[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

2019-02-22 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis marked an inline comment as done.
zinovy.nis added inline comments.



Comment at: clang-tidy/tool/clang-tidy-diff.py:133
+if args.export_fixes:
+  print("error: -export-fixes and -j are mutually exclusive.")
+  sys.exit(1)

alexfh wrote:
> What's wrong with -export-fixes and -j? Different clang-tidy processes could 
> export the fixes to different files (e.g. the -export-fixes= value could be 
> used as a prefix of the filename with a sequential or random suffix. WDYT?
I can copy/paste (shame on me!) a code from `run-clang-tidy.py` that does just 
what we need.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57662/new/

https://reviews.llvm.org/D57662



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


[PATCH] D58278: Prepare ground for re-lexing modular headers.

2019-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang/include/clang/Lex/PPCallbacks.h:346
+
+  virtual void setPreprocessor(Preprocessor *preprocessor) {
+preprocessor_ = preprocessor;

This seems pretty tangly from a layering point of view, giving each object a 
reference to the other. I always find it hard to reason about ownership in such 
cases.

What about lifecycle methods `beginPreprocessing(Preprocessor*)` and 
`endPreprocessing(Preprocessor*)`? It will look similar in practice: subclasses 
that need it can still track the PP instance, but the circular references will 
only be there when needed, and will be explicit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58278/new/

https://reviews.llvm.org/D58278



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


[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

2019-02-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

This one was used in a few different places in clang, for ex: 
https://github.com/llvm-mirror/clang/blob/master/tools/libclang/CIndex.cpp#L8713.
 Therefore I've used the same funcitonality.

Regarding `pthread_setschedparam` does setting prioirty to 
`PTHREAD_MIN_PRIORITY` on either `SCHED_FIFO` or `SCHED_RR` policy corresponds 
to `SCHED_IDLE` in linux(http://man7.org/linux/man-pages/man7/sched.7.html)?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58492/new/

https://reviews.llvm.org/D58492



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


[PATCH] D55250: [clangd] Enhance macro hover to see full definition

2019-02-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!




Comment at: clangd/ClangdLSPServer.cpp:816
+// If the client supports Markdown, convert from plaintext here.
+if (*H && HoverSupportsMarkdown) {
+  (*H)->contents.kind = MarkupKind::Markdown;

malaperle wrote:
> ilya-biryukov wrote:
> > malaperle wrote:
> > > I don't know if you meant plain text as non-language-specific markdown or 
> > > no markdown at all. In VSCode, non-markdown for multiline macros looks 
> > > bad because the newlines are ignored.
> > Did not know about that, thanks for pointing it out.
> > It does not ignore double newlines though (that's what we use in 
> > declarations). I suspect it treats plaintext as markdown, but can't be sure.
> > 
> > In any case, converting to a markdown code block here looks incredibly 
> > hacky and shaky.
> > Could we use double-newlines for line breaks in our implementation instead?
> > 
> > This aligns with what we did before this patch for declarations.
> > If that doesn't work, breaking the multi-line macros in VSCode looks ok, 
> > this really feels like a bug in VSCode.
> > In any case, converting to a markdown code block here looks incredibly 
> > hacky and shaky.
> 
> I'm not sure why? ClangdLSPServer knows about client capabilities so it made 
> sense to me to convert it there. Either way, it sounds like I should just 
> remove "HoverSupportsMarkdown" altogether if we're not going to use Markdown.
> 
> > Could we use double-newlines for line breaks in our implementation instead?
> 
> It looks odd. When I double them in the hover contents, the lines are 
> displayed as doubled and some extra backslashes are added on all non-empty 
> lines except the first line. Single new lines and correct backslashes are 
> displayed correctly when Markdown is used. We can just display multiline line 
> macros as one line until we want to handle Markdown (which is how exactly?). 
> I'm not sure why? ClangdLSPServer knows about client capabilities so it made 
> sense to me to convert it there. Either way, it sounds like I should just 
> remove "HoverSupportsMarkdown" altogether if we're not going to use Markdown.

Don't get me wrong, adding markdown support seems nice, but let's do it as a 
separate patch. Here's a few thoughts on this.

Currently ClangdServer already already returns a hover with proper kind and 
contents, so it is ClangdServer's responsibility to produce markdown or 
plaintext as of today.
We could consider moving the responsibility to decide whether we should return 
markdown or plaintext to LSPServer, but then we should also change the types of 
values ClangdServer returns (to a structured representation that would produce 
plaintext or markdown, depending on the client settings).

Assuming the result is a code block looks wrong, we do return plain text bits 
like "declared in namespace std" as a result of hover.

> It looks odd. When I double them in the hover contents, the lines are 
> displayed as doubled and some extra backslashes are added on all non-empty 
> lines except the first line. Single new lines and correct backslashes are 
> displayed correctly when Markdown is used. We can just display multiline line 
> macros as one line until we want to handle Markdown (which is how exactly?).
Totally on board with this, multiline marcos are not exactly rare, but we could 
live with this oddity in VSCode for some time.
Adding basic markdown support shouldn't be too hard, but let's block this patch 
on it.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55250/new/

https://reviews.llvm.org/D55250



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


[PATCH] D58525: [clangd] Don't attach FixIt to the source code in macro.

2019-02-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58525/new/

https://reviews.llvm.org/D58525



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


[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

2019-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: klimek.
sammccall added a comment.

Sorry to be a pain here, but I'm not sure introducing Javascript is a good idea 
unless there's a strong reason for it.
All LLVM developers have python installed, many are comfortable with the 
language - it seems less likely to be a maintenance hazard. I can recommend 
BeautifulSoup for the HTML parsing stuff.
(For the record, I kind of hate python, but I'm more comfortable reviewing and 
maintaining it than JS).
Mitigating points: we have some in the VSCode plugin by necessity, and this 
isn't a build-time dependency.

Happy to get another opinion here.

> Added the tool in this patch, but I think a separate repository under 
> GitHub's clangd org might be a more suitable place.

I'm somewhat sympathetic to this idea, but I don't think we can do it as things 
stand. @klimek can confirm.




Comment at: clangd/StdGen/Readme.md:1
+# StdGen
+

can we add the caveats here?
 - only symbols directly in namespace std are added
 - symbols with multiple variants or defined in multiple headers aren't added



Comment at: clangd/StdGen/Readme.md:3
+
+StdGen is a tool to generate headers for C++ Standard Library symbols by 
parsing
+archieved HTML files from [cppreference](http://cppreference.com/).

Can we be a bit more specific about what it generates?

From this text I'd have guess it generates headers like `namespace std { int 
printf(char*, ...); }`.



Comment at: clangd/StdGen/Readme.md:8
+
+### Requriement
+

Requriement -> Requirements



Comment at: clangd/StdGen/Readme.md:11
+* Download the cppreference [offline HTML 
files](https://en.cppreference.com/w/Cppreference:Archives),
+and unzip it to the `/reference` directory.
+

nit: can we make this a command-line arg instead of requiring people to put it 
in their source tree?



Comment at: clangd/StdGen/lib/main.js:1
+const $ = require('cheerio')
+const fs = require('fs')

note: I haven't reviewed the JS files for style/readability as I'm weak on JS 
and it's unclear what style would govern here. e.g. I suspect when used as a 
niche language in a C++ project by people who don't know it well, we should 
probably have semicolons rather than relying on ASI.
But see code review comment - this may be a moot point.



Comment at: clangd/StdGen/lib/parse.js:39
+  })
+  // Skip C++20 symbols as C++20 is not finalized yet.
+  if (revision == 'C++20')

Not sure this is necessary - it seems preferable to be forwards-compatible even 
if not 100% accurate, no?

(And it would simplify the code, as we don't need to parse out the version)



Comment at: clangd/index/CanonicalIncludes.cpp:110
   static const std::vector> SymbolMap = {
   {"std::addressof", ""},
   // Map symbols in  to their preferred includes.

can you remove the ones that duplicate the .inc file?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58345/new/

https://reviews.llvm.org/D58345



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


[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

2019-02-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clangd/Threading.cpp:125
+#elif defined(__APPLE__)
+  // 
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,

ilya-biryukov wrote:
> This points into `iPhoneOS` docs, might be confusing.
> Is there a link for the desktop version of the OS somewhere?
> Could also leave it out, I'm sure setpriority is easy to google.
It still says `This document is a Mac OS X manual page.` on top of the page, 
couldn't find any url that contains something like `MacOS` instead of `iPhoneOS`


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58492/new/

https://reviews.llvm.org/D58492



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


[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

2019-02-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 187908.
kadircet marked an inline comment as done.
kadircet added a comment.

- Add required header


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58492/new/

https://reviews.llvm.org/D58492

Files:
  clangd/Threading.cpp


Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -7,6 +7,8 @@
 #include 
 #ifdef __USE_POSIX
 #include 
+#elif defined(__APPLE__)
+#include 
 #endif
 
 namespace clang {
@@ -121,6 +123,12 @@
   Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
 : SCHED_OTHER,
   &priority);
+#elif defined(__APPLE__)
+  // 
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,
+  Priority == ThreadPriority::Low && !AvoidThreadStarvation
+  ? PRIO_DARWIN_BG
+  : 0);
 #endif
 }
 


Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -7,6 +7,8 @@
 #include 
 #ifdef __USE_POSIX
 #include 
+#elif defined(__APPLE__)
+#include 
 #endif
 
 namespace clang {
@@ -121,6 +123,12 @@
   Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
 : SCHED_OTHER,
   &priority);
+#elif defined(__APPLE__)
+  // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
+  setpriority(PRIO_DARWIN_THREAD, 0,
+  Priority == ThreadPriority::Low && !AvoidThreadStarvation
+  ? PRIO_DARWIN_BG
+  : 0);
 #endif
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r354664 - [clangd] Don't attach FixIt to the source code in macro.

2019-02-22 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Feb 22 01:43:56 2019
New Revision: 354664

URL: http://llvm.org/viewvc/llvm-project?rev=354664&view=rev
Log:
[clangd] Don't attach FixIt to the source code in macro.

Summary:
We are less certain it is the correct fix. Also, clang doesn't apply FixIt to
the source code in macro.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=354664&r1=354663&r2=354664&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Fri Feb 22 01:43:56 2019
@@ -335,6 +335,11 @@ void StoreDiags::HandleDiagnostic(Diagno
 
 llvm::SmallVector Edits;
 for (auto &FixIt : Info.getFixItHints()) {
+  // Follow clang's behavior, don't apply FixIt to the code in macros,
+  // we are less certain it is the right fix.
+  if (FixIt.RemoveRange.getBegin().isMacroID() ||
+  FixIt.RemoveRange.getEnd().isMacroID())
+return false;
   if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
 Info.getSourceManager()))
 return false;

Modified: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp?rev=354664&r1=354663&r2=354664&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp Fri Feb 22 
01:43:56 2019
@@ -215,6 +215,18 @@ TEST(DiagnosticsTest, InsideMacros) {
"'int *' with an rvalue of type 'int'")));
 }
 
+TEST(DiagnosticsTest, NoFixItInMacro) {
+  Annotations Test(R"cpp(
+#define Define(name) void name() {}
+
+[[Define]](main)
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
+Not(WithFix(_);
+}
+
 TEST(DiagnosticsTest, ToLSP) {
   clangd::Diag D;
   D.Message = "something terrible happened";


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


[PATCH] D58525: [clangd] Don't attach FixIt to the source code in macro.

2019-02-22 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL354664: [clangd] Don't attach FixIt to the source code 
in macro. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58525/new/

https://reviews.llvm.org/D58525

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


Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -335,6 +335,11 @@
 
 llvm::SmallVector Edits;
 for (auto &FixIt : Info.getFixItHints()) {
+  // Follow clang's behavior, don't apply FixIt to the code in macros,
+  // we are less certain it is the right fix.
+  if (FixIt.RemoveRange.getBegin().isMacroID() ||
+  FixIt.RemoveRange.getEnd().isMacroID())
+return false;
   if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
 Info.getSourceManager()))
 return false;
Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
@@ -215,6 +215,18 @@
"'int *' with an rvalue of type 'int'")));
 }
 
+TEST(DiagnosticsTest, NoFixItInMacro) {
+  Annotations Test(R"cpp(
+#define Define(name) void name() {}
+
+[[Define]](main)
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
+Not(WithFix(_);
+}
+
 TEST(DiagnosticsTest, ToLSP) {
   clangd::Diag D;
   D.Message = "something terrible happened";


Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -335,6 +335,11 @@
 
 llvm::SmallVector Edits;
 for (auto &FixIt : Info.getFixItHints()) {
+  // Follow clang's behavior, don't apply FixIt to the code in macros,
+  // we are less certain it is the right fix.
+  if (FixIt.RemoveRange.getBegin().isMacroID() ||
+  FixIt.RemoveRange.getEnd().isMacroID())
+return false;
   if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
 Info.getSourceManager()))
 return false;
Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
@@ -215,6 +215,18 @@
"'int *' with an rvalue of type 'int'")));
 }
 
+TEST(DiagnosticsTest, NoFixItInMacro) {
+  Annotations Test(R"cpp(
+#define Define(name) void name() {}
+
+[[Define]](main)
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
+Not(WithFix(_);
+}
+
 TEST(DiagnosticsTest, ToLSP) {
   clangd::Diag D;
   D.Message = "something terrible happened";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58278: Prepare ground for re-lexing modular headers.

2019-02-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh marked an inline comment as done.
alexfh added inline comments.



Comment at: clang/include/clang/Lex/PPCallbacks.h:346
+
+  virtual void setPreprocessor(Preprocessor *preprocessor) {
+preprocessor_ = preprocessor;

sammccall wrote:
> This seems pretty tangly from a layering point of view, giving each object a 
> reference to the other. I always find it hard to reason about ownership in 
> such cases.
> 
> What about lifecycle methods `beginPreprocessing(Preprocessor*)` and 
> `endPreprocessing(Preprocessor*)`? It will look similar in practice: 
> subclasses that need it can still track the PP instance, but the circular 
> references will only be there when needed, and will be explicit.
We can do that as well. However, adding another couple of overrides to each 
PPCallbacks implementation will result in quite some boilerplate. How would you 
look at creating an intermediate class in the `PPCallbacks` ->  hierarchy that will handle 
`beginPreprocessing`/`endPreprocessing` and expose the preprocessor pointer via 
a protected `getPreprocessor()` method?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58278/new/

https://reviews.llvm.org/D58278



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


[PATCH] D58278: Prepare ground for re-lexing modular headers.

2019-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang/include/clang/Lex/PPCallbacks.h:346
+
+  virtual void setPreprocessor(Preprocessor *preprocessor) {
+preprocessor_ = preprocessor;

alexfh wrote:
> sammccall wrote:
> > This seems pretty tangly from a layering point of view, giving each object 
> > a reference to the other. I always find it hard to reason about ownership 
> > in such cases.
> > 
> > What about lifecycle methods `beginPreprocessing(Preprocessor*)` and 
> > `endPreprocessing(Preprocessor*)`? It will look similar in practice: 
> > subclasses that need it can still track the PP instance, but the circular 
> > references will only be there when needed, and will be explicit.
> We can do that as well. However, adding another couple of overrides to each 
> PPCallbacks implementation will result in quite some boilerplate. How would 
> you look at creating an intermediate class in the `PPCallbacks` ->  PPCallbacks handlers> hierarchy that will handle 
> `beginPreprocessing`/`endPreprocessing` and expose the preprocessor pointer 
> via a protected `getPreprocessor()` method?
I think that would probably obscure the intent, I really think this is subtle 
enough to be explicit, and would be only about as boilerplatey as today (the PP 
pointer is set by a method rather than in the constructor).

But I'm more concerned about the interfaces in Lex/ than the specific callsites 
in clang-tidy, so if such a helper class were private to clang-tidy, that seems 
ok.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58278/new/

https://reviews.llvm.org/D58278



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


[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-22 Thread pierre gousseau via Phabricator via cfe-commits
pgousseau updated this revision to Diff 187913.
pgousseau added a comment.

Fix odr violation issue using static data member of a class template as 
suggested.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57914/new/

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6211,7 +6211,8 @@
 if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
   return;
 
-if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) == 0)
+if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
+SanitizerMask())
   S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
 else if (isGlobalVar(D) && SanitizerName != "address")
   S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -551,7 +551,7 @@
 DiagnosticsEngine &Diags, SanitizerSet &S) {
   for (const auto &Sanitizer : Sanitizers) {
 SanitizerMask K = parseSanitizerValue(Sanitizer, /*AllowGroups=*/false);
-if (K == 0)
+if (K == SanitizerMask())
   Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
 else
   S.set(K, true);
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -25,29 +25,32 @@
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+static const SanitizerMask NeedsUbsanRt =
+Undefined | Integer | ImplicitConversion | Nullability | CFI;
+static const SanitizerMask NeedsUbsanCxxRt = Vptr | CFI;
+static const SanitizerMask NotAllowedWithTrap = Vptr;
+static const SanitizerMask NotAllowedWithMinimalRuntime = Vptr;
+static const SanitizerMask RequiresPIE = DataFlow | HWAddress | Scudo;
+static const SanitizerMask NeedsUnwindTables =
+Address | HWAddress | Thread | Memory | DataFlow;
+static const SanitizerMask SupportsCoverage =
+Address | HWAddress | KernelAddress | KernelHWAddress | Memory |
+KernelMemory | Leak | Undefined | Integer | ImplicitConversion |
+Nullability | DataFlow | Fuzzer | FuzzerNoLink;
+static const SanitizerMask RecoverableByDefault =
+Undefined | Integer | ImplicitConversion | Nullability;
+static const SanitizerMask Unrecoverable = Unreachable | Return;
+static const SanitizerMask AlwaysRecoverable = KernelAddress | KernelHWAddress;
+static const SanitizerMask LegacyFsanitizeRecoverMask = Undefined | Integer;
+static const SanitizerMask NeedsLTO = CFI;
+static const SanitizerMask TrappingSupported =
+(Undefined & ~Vptr) | UnsignedIntegerOverflow | ImplicitConversion |
+Nullability | LocalBounds | CFI;
+static const SanitizerMask TrappingDefault = CFI;
+static const SanitizerMask CFIClasses =
+CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast;
+static const SanitizerMask CompatibleWithMinimalRuntime =
+TrappingSupported | Scudo | ShadowCallStack;
 
 enum CoverageFeature {
   CoverageFunc = 1 << 0,
@@ -136,10 +139,10 @@
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
const llvm::opt::ArgList &Args

[PATCH] D58541: [CodeComplete] Propagate preferred type for function arguments in more cases

2019-02-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

See the added test for some new cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58541

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/unittests/Sema/CodeCompleteTest.cpp

Index: clang/unittests/Sema/CodeCompleteTest.cpp
===
--- clang/unittests/Sema/CodeCompleteTest.cpp
+++ clang/unittests/Sema/CodeCompleteTest.cpp
@@ -442,4 +442,22 @@
   )cpp";
   EXPECT_THAT(collectPreferredTypes(Code), Each("const int *"));
 }
+
+TEST(PreferredTypeTest, FunctionArguments) {
+  StringRef Code = R"cpp(
+void foo(const int*);
+
+void bar(const int*);
+void bar(const int*, int b);
+
+struct vector {
+  const int *data();
+};
+void test() {
+  foo(^(^(^(^vector().^data();
+  bar(^(^(^(^vector().^data();
+}
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(Code), Each("const int *"));
+}
 } // namespace
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -350,13 +350,16 @@
 void PreferredTypeBuilder::enterReturn(Sema &S, SourceLocation Tok) {
   if (isa(S.CurContext)) {
 if (sema::BlockScopeInfo *BSI = S.getCurBlock()) {
+  ComputeType = nullptr;
   Type = BSI->ReturnType;
   ExpectedLoc = Tok;
 }
   } else if (const auto *Function = dyn_cast(S.CurContext)) {
+ComputeType = nullptr;
 Type = Function->getReturnType();
 ExpectedLoc = Tok;
   } else if (const auto *Method = dyn_cast(S.CurContext)) {
+ComputeType = nullptr;
 Type = Method->getReturnType();
 ExpectedLoc = Tok;
   }
@@ -364,10 +367,18 @@
 
 void PreferredTypeBuilder::enterVariableInit(SourceLocation Tok, Decl *D) {
   auto *VD = llvm::dyn_cast_or_null(D);
+  ComputeType = nullptr;
   Type = VD ? VD->getType() : QualType();
   ExpectedLoc = Tok;
 }
 
+void PreferredTypeBuilder::enterFunctionArgument(
+SourceLocation Tok, llvm::function_ref ComputeType) {
+  this->ComputeType = ComputeType;
+  Type = QualType();
+  ExpectedLoc = Tok;
+}
+
 void PreferredTypeBuilder::enterParenExpr(SourceLocation Tok,
   SourceLocation LParLoc) {
   // expected type for parenthesized expression does not change.
@@ -487,6 +498,7 @@
 
 void PreferredTypeBuilder::enterBinary(Sema &S, SourceLocation Tok, Expr *LHS,
tok::TokenKind Op) {
+  ComputeType = nullptr;
   Type = getPreferredTypeOfBinaryRHS(S, LHS, Op);
   ExpectedLoc = Tok;
 }
@@ -495,30 +507,38 @@
   Expr *Base) {
   if (!Base)
 return;
-  Type = this->get(Base->getBeginLoc());
+  // Do we have expected type for Base?
+  if (ExpectedLoc != Base->getBeginLoc())
+return;
+  // Keep the expected type, only update the location.
   ExpectedLoc = Tok;
+  return;
 }
 
 void PreferredTypeBuilder::enterUnary(Sema &S, SourceLocation Tok,
   tok::TokenKind OpKind,
   SourceLocation OpLoc) {
+  ComputeType = nullptr;
   Type = getPreferredTypeOfUnaryArg(S, this->get(OpLoc), OpKind);
   ExpectedLoc = Tok;
 }
 
 void PreferredTypeBuilder::enterSubscript(Sema &S, SourceLocation Tok,
   Expr *LHS) {
+  ComputeType = nullptr;
   Type = S.getASTContext().IntTy;
   ExpectedLoc = Tok;
 }
 
 void PreferredTypeBuilder::enterTypeCast(SourceLocation Tok,
  QualType CastType) {
+  ComputeType = nullptr;
   Type = !CastType.isNull() ? CastType.getCanonicalType() : QualType();
   ExpectedLoc = Tok;
 }
 
 void PreferredTypeBuilder::enterCondition(Sema &S, SourceLocation Tok) {
+  ComputeType = nullptr;
   Type = S.getASTContext().BoolTy;
   ExpectedLoc = Tok;
 }
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -424,21 +424,19 @@
 CommaLocsTy CommaLocs;
 
 SourceLocation LParLoc = T.getOpenLocation();
-if (ParseExpressionList(
-Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, &Exprs] {
-  QualType PreferredType = Actions.ProduceConstructorSignatureHelp(
-  getCurScope(),
-  OmpPrivParm->getType()->getCanonicalTypeInternal(),
-  OmpPrivParm->getLocation(), Exprs, LParLoc);
-  CalledSignatureHelp = true;
-  Actions.CodeCompleteExpression(getCurSc

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-22 Thread pierre gousseau via Phabricator via cfe-commits
pgousseau updated this revision to Diff 187917.
pgousseau added a comment.

Rework FIXME comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57914/new/

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6211,7 +6211,8 @@
 if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
   return;
 
-if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) == 0)
+if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
+SanitizerMask())
   S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
 else if (isGlobalVar(D) && SanitizerName != "address")
   S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -551,7 +551,7 @@
 DiagnosticsEngine &Diags, SanitizerSet &S) {
   for (const auto &Sanitizer : Sanitizers) {
 SanitizerMask K = parseSanitizerValue(Sanitizer, /*AllowGroups=*/false);
-if (K == 0)
+if (K == SanitizerMask())
   Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
 else
   S.set(K, true);
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -25,29 +25,32 @@
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+static const SanitizerMask NeedsUbsanRt =
+Undefined | Integer | ImplicitConversion | Nullability | CFI;
+static const SanitizerMask NeedsUbsanCxxRt = Vptr | CFI;
+static const SanitizerMask NotAllowedWithTrap = Vptr;
+static const SanitizerMask NotAllowedWithMinimalRuntime = Vptr;
+static const SanitizerMask RequiresPIE = DataFlow | HWAddress | Scudo;
+static const SanitizerMask NeedsUnwindTables =
+Address | HWAddress | Thread | Memory | DataFlow;
+static const SanitizerMask SupportsCoverage =
+Address | HWAddress | KernelAddress | KernelHWAddress | Memory |
+KernelMemory | Leak | Undefined | Integer | ImplicitConversion |
+Nullability | DataFlow | Fuzzer | FuzzerNoLink;
+static const SanitizerMask RecoverableByDefault =
+Undefined | Integer | ImplicitConversion | Nullability;
+static const SanitizerMask Unrecoverable = Unreachable | Return;
+static const SanitizerMask AlwaysRecoverable = KernelAddress | KernelHWAddress;
+static const SanitizerMask LegacyFsanitizeRecoverMask = Undefined | Integer;
+static const SanitizerMask NeedsLTO = CFI;
+static const SanitizerMask TrappingSupported =
+(Undefined & ~Vptr) | UnsignedIntegerOverflow | ImplicitConversion |
+Nullability | LocalBounds | CFI;
+static const SanitizerMask TrappingDefault = CFI;
+static const SanitizerMask CFIClasses =
+CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast;
+static const SanitizerMask CompatibleWithMinimalRuntime =
+TrappingSupported | Scudo | ShadowCallStack;
 
 enum CoverageFeature {
   CoverageFunc = 1 << 0,
@@ -136,10 +139,10 @@
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
const llvm::opt::ArgList &Args) {
-  SanitizerMask TrapRemove = 0; // During the loop below,

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-22 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: include/clang/Basic/Sanitizers.h:148
+// workaround from n4424 to avoid odr issues.
+// FIXME: Can be replaced by constexpr once c++14 can be used in llvm.
+template  struct SanitizerMasks {

Not replaced. `constexpr` is nearly orthogonal to the odr issue. It can be 
replaced by `inline` variables in C++17. I think that you should leave two 
FIXME here: the first one about marking the masks`constexpr` in C++14, and the 
second one about replacing the workaround by marking the masks `inline`.



Comment at: include/clang/Basic/Sanitizers.h:173
+  SanitizerMask::bitPosToMask(SO_##ID##Group); 
\
+  static const SanitizerMask &ID##Group = SanitizerMasks<>::ID##Group;
 #include "clang/Basic/Sanitizers.def"

You are back to the same issue with the references.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57914/new/

https://reviews.llvm.org/D57914



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


[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-02-22 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment.

I am wondering about the semantics of this bit. I don't think that you can know 
for sure within clang whether a variable has been modified. The best you can do 
is know for sure that some variable has been modified, but I don't think you 
can prove that it has not been modified.

Consequently I am wondering if you should change the name of this flag to 
something like `IsArgumentKnownToBeModified`.

Orthogonal to this you need to also add tests for templates.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035



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


[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-22 Thread pierre gousseau via Phabricator via cfe-commits
pgousseau marked 2 inline comments as done.
pgousseau added inline comments.



Comment at: include/clang/Basic/Sanitizers.h:148
+// workaround from n4424 to avoid odr issues.
+// FIXME: Can be replaced by constexpr once c++14 can be used in llvm.
+template  struct SanitizerMasks {

riccibruno wrote:
> Not replaced. `constexpr` is nearly orthogonal to the odr issue. It can be 
> replaced by `inline` variables in C++17. I think that you should leave two 
> FIXME here: the first one about marking the masks`constexpr` in C++14, and 
> the second one about replacing the workaround by marking the masks `inline`.
Makes sense thanks!



Comment at: include/clang/Basic/Sanitizers.h:173
+  SanitizerMask::bitPosToMask(SO_##ID##Group); 
\
+  static const SanitizerMask &ID##Group = SanitizerMasks<>::ID##Group;
 #include "clang/Basic/Sanitizers.def"

riccibruno wrote:
> You are back to the same issue with the references.
A yes I should have read the odr definition more carefully! I am trying to find 
a way to not have to add `SanitizerMasks<>::` all over the place, any ideas?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57914/new/

https://reviews.llvm.org/D57914



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


[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-22 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: include/clang/Basic/Sanitizers.h:173
+  SanitizerMask::bitPosToMask(SO_##ID##Group); 
\
+  static const SanitizerMask &ID##Group = SanitizerMasks<>::ID##Group;
 #include "clang/Basic/Sanitizers.def"

pgousseau wrote:
> riccibruno wrote:
> > You are back to the same issue with the references.
> A yes I should have read the odr definition more carefully! I am trying to 
> find a way to not have to add `SanitizerMasks<>::` all over the place, any 
> ideas?
Nope, unfortunately.

(And I am wondering how many other similar issues exist in clang/llvm; I would 
bet it is > 0).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57914/new/

https://reviews.llvm.org/D57914



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


[PATCH] D58544: [AST] Improve support of external layouts in `MicrosoftRecordLayoutBuilder`

2019-02-22 Thread Aleksandr Urakov via Phabricator via cfe-commits
aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: rnk, zturner, rsmith.
aleksandr.urakov added a project: clang.
Herald added subscribers: cfe-commits, jdoerfert.

This patch fixes several small problems with external layouts support in 
`MicrosoftRecordLayoutBuilder`:

- aligns properly the size of a struct that ends with a bit field. It was 
aligned on byte before, not on the size of the field, so the struct size was 
smaller than it should be;
- adjusts the struct size when injecting a vbptr in the case when there were no 
bases or fields allocated after the vbptr. Similarly, without the adjustment 
the struct was smaller than it should be;
- the same fix as above for the vfptr.

All these fixes affect the non-virtual size of a struct, so they are tested 
through non-virtual inheritance.


Repository:
  rC Clang

https://reviews.llvm.org/D58544

Files:
  lib/AST/RecordLayoutBuilder.cpp
  test/CodeGenCXX/Inputs/override-bit-field-layout.layout
  test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
  test/CodeGenCXX/override-bit-field-layout.cpp
  test/CodeGenCXX/override-layout-virtual-base.cpp

Index: test/CodeGenCXX/override-layout-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/override-layout-virtual-base.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
+
+struct S1 {
+  int a;
+};
+
+struct S2 : virtual S1 {
+  virtual void foo() {}
+};
+
+// CHECK: Type: struct S3
+// CHECK:   FieldOffsets: [128]
+struct S3 : S2 {
+  char b;
+};
+
+void use_structs() {
+  S1 s1s[sizeof(S1)];
+  S2 s2s[sizeof(S2)];
+  S3 s3s[sizeof(S3)];
+}
Index: test/CodeGenCXX/override-bit-field-layout.cpp
===
--- test/CodeGenCXX/override-bit-field-layout.cpp
+++ test/CodeGenCXX/override-bit-field-layout.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s
 
 // CHECK: Type: struct S1
 // CHECK:   FieldOffsets: [0, 11]
@@ -14,7 +14,23 @@
   short a : 3;
 };
 
+// CHECK: Type: struct S3
+// CHECK:   Size:32
+// CHECK:   FieldOffsets: [0, 1]
+struct S3 {
+  int a : 1;
+  int b : 2;
+};
+
+// CHECK: Type: struct S4
+// CHECK:   FieldOffsets: [32]
+struct S4 : S3 {
+  char c;
+};
+
 void use_structs() {
   S1 s1s[sizeof(S1)];
   S2 s2s[sizeof(S2)];
+  S3 s3s[sizeof(S3)];
+  S4 s4s[sizeof(S4)];
 }
Index: test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
===
--- /dev/null
+++ test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
@@ -0,0 +1,8 @@
+
+*** Dumping AST Record Layout
+Type: struct S2
+
+Layout: 
Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout
===
--- test/CodeGenCXX/Inputs/override-bit-field-layout.layout
+++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout
@@ -14,3 +14,11 @@
   Size:128
   Alignment:64
   FieldOffsets: [64]>
+
+*** Dumping AST Record Layout
+Type: struct S3
+
+Layout: 
Index: lib/AST/RecordLayoutBuilder.cpp
===
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -2692,7 +2692,8 @@
 auto FieldBitOffset = External.getExternalFieldOffset(FD);
 placeFieldAtBitOffset(FieldBitOffset);
 auto NewSize = Context.toCharUnitsFromBits(
-llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth()));
+llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
+Context.toBits(Info.Size));
 Size = std::max(Size, NewSize);
 Alignment = std::max(Alignment, Info.Alignment);
   } else if (IsUnion) {
@@ -2741,12 +2742,17 @@
   CharUnits InjectionSite = VBPtrOffset;
   // But before we do, make sure it's properly aligned.
   VBPtrOffset = VBPtrOffset.alignTo(PointerInfo.Alignment);
+  // Determine where the first field should be laid out after the vbptr.
+  CharUnits FieldStart = VBPtrOffset + PointerInfo.Size;
   // Shift everything after the vbptr down, unless we're using an external
   // layout.
-  if (UseExternalLayout)
+  if (UseExternalLayout) {
+// It is possible that there were no fields or bases located after vbptr,
+// so the size was not adjusted before.
+if (Size < FieldStart)
+  Size = FieldStart;
 return;
-  // Determine where the first field should be laid out after the vbptr.
-  CharUnits FieldStart = VBPtrOffset + PointerInfo.Size;
+  }
   // Make sure that the amount we push the fields back by is a m

[PATCH] D58492: [clangd] Add thread priority lowering for MacOS as well

2019-02-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision.
gribozavr added a comment.
This revision is now accepted and ready to land.

I see, there's no SCHED_IDLE in the macOS SDK.  OK then, I trust that people 
who wrote CIndex.cpp support for macOS probably have got it correctly working 
for macOS, so it makes sense to do the same in clangd.  Please consider 
factoring a function in libSupport though.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58492/new/

https://reviews.llvm.org/D58492



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


[PATCH] D58545: fix the message

2019-02-22 Thread Mikhail Goncharov via Phabricator via cfe-commits
goncharov created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

please ignore


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58545

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/warn-infinite-recursion.cpp

Index: clang/test/SemaCXX/warn-infinite-recursion.cpp
===
--- clang/test/SemaCXX/warn-infinite-recursion.cpp
+++ clang/test/SemaCXX/warn-infinite-recursion.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -Winfinite-recursion
 
-void a() {  // expected-warning{{call itself}}
+void a() {  // expected-warning{{recursion!}}
   a();
 }
 
-void b(int x) {  // expected-warning{{call itself}}
+void b(int x) {  // expected-warning{{recursion!}}
   if (x)
 b(x);
   else
@@ -16,7 +16,7 @@
 c(5);
 }
 
-void d(int x) {  // expected-warning{{call itself}}
+void d(int x) {  // expected-warning{{recursion!}}
   if (x)
 ++x;
   return d(x);
@@ -29,7 +29,7 @@
 void e() { f(); }
 void f() { e(); }
 
-void g() {  // expected-warning{{call itself}}
+void g() {  // expected-warning{{recursion!}}
   while (true)
 g();
 
@@ -42,14 +42,14 @@
   }
 }
 
-void i(int x) {  // expected-warning{{call itself}}
+void i(int x) {  // expected-warning{{recursion!}}
   while (x < 5) {
 --x;
   }
   i(0);
 }
 
-int j() {  // expected-warning{{call itself}}
+int j() {  // expected-warning{{recursion!}}
   return 5 + j();
 }
 
@@ -80,11 +80,11 @@
   void b();
 };
 
-void S::a() {  // expected-warning{{call itself}}
+void S::a() {  // expected-warning{{recursion!}}
   return a();
 }
 
-void S::b() {  // expected-warning{{call itself}}
+void S::b() {  // expected-warning{{recursion!}}
   int i = 0;
   do {
 ++i;
@@ -95,8 +95,8 @@
 template
 struct T {
   member m;
-  void a() { return a(); }  // expected-warning{{call itself}}
-  static void b() { return b(); }  // expected-warning{{call itself}}
+  void a() { return a(); }  // expected-warning{{recursion!}}
+  static void b() { return b(); }  // expected-warning{{recursion!}}
 };
 
 void test_T() {
@@ -107,13 +107,13 @@
 
 class U {
   U* u;
-  void Fun() {  // expected-warning{{call itself}}
+  void Fun() {  // expected-warning{{recursion!}}
 u->Fun();
   }
 };
 
 // No warnings on templated functions
-// sum<0>() is instantiated, does recursively call itself, but never runs.
+// sum<0>() is instantiated, does recursively recursion!, but never runs.
 template 
 int sum() {
   return value + sum();
@@ -157,7 +157,7 @@
   return 0;
 return Wrapper::run();
   }
-  static int run2() {  // expected-warning{{call itself}}
+  static int run2() {  // expected-warning{{recursion!}}
 return run2();
   }
 };
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -61,7 +61,7 @@
   "remove call to max function and unsigned zero argument">;
 
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "oh my, recursion!">,
   InGroup, DefaultIgnore;
 
 def warn_comma_operator : Warning<"possible misuse of comma operator here">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D16351: [FIX] Bug 25404 - Crash on typedef in OpenCL 2.0

2019-02-22 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia commandeered this revision.
Anastasia added a reviewer: ichesnokov.
Anastasia added a comment.
Herald added a subscriber: ebevhan.

I just closed the bug because it's no longer failing on the master branch. So I 
don't think this is needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D16351/new/

https://reviews.llvm.org/D16351



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


[PATCH] D16351: [FIX] Bug 25404 - Crash on typedef in OpenCL 2.0

2019-02-22 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia abandoned this revision.
Anastasia added a comment.

I just took the ownership of this to be able to close it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D16351/new/

https://reviews.llvm.org/D16351



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


[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to and from clauses with mapper modifier

2019-02-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: lib/AST/OpenMPClause.cpp:1469
 
 void OMPClausePrinter::VisitOMPFromClause(OMPFromClause *Node) {
   if (!Node->varlist_empty()) {

lildmh wrote:
> ABataev wrote:
> > Better to split this patch into 2: one for `to` clause and another one for 
> > `from` clause
> Hi Alexey,
> 
> Thanks for the quick review! The code for `to` and `from` clauses are almost 
> identical, so I prefer to have them in the same patch. Besides I don't have 
> commit privilege, so it causes more time to me to have multiple patches. But 
> if you think it's crucial to split it. I can do it. What do you think?
Hi Lingda, it is clang/LLVM policy to make patches as small as possible (for 
better "reviewability", reliability, trackability etc.). If it is possible to 
split the patch into several parts, this must be done.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523



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


[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-02-22 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro marked 2 inline comments as done.
djtodoro added a comment.

> I was under the impression that space inside VarDecl was quite constrained. 
> Pardon the likely naive question, but: is there any way to make the 
> representation more compact (maybe sneak a bit into ParmVarDeclBitfields)?

@vsk  Thanks for the comment! Sure, it is better idea. Initially, we thought 
this could be used even for local variables, but since we are using it only for 
parameters it makes more sense.




Comment at: lib/Sema/SemaExpr.cpp:11282
+static void EmitArgumentsValueModification(Expr *E) {
+  if (const DeclRefExpr *LHSDeclRef =
+  dyn_cast(E->IgnoreParenCasts()))

probinson wrote:
> Does this fit on one line if you use `const auto *LHSDeclRef ...`? Given you 
> have the dyn_cast on the RHS there's no real need to give the type name twice.
>Does this fit on one line if you use const auto *LHSDeclRef ...? Given you 
>have the dyn_cast on the RHS there's no real need to give the type name twice.

@probinson Thanks for the comment! Sure, it will be refactored.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035



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


[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-02-22 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro marked an inline comment as done.
djtodoro added a comment.

@riccibruno Thanks for your comments!

> Oh and I think that you will also have to update the serialization 
> code/de-serialization code in ASTReaderDecl.cpp / ASTWriterDecl.cpp. You 
> might also have to update TreeTransform but I am less familiar with this.

I've added `ASTReaderDecl.cpp` / `ASTWriterDecl.cpp`, but looking at 
`TreeTransform ` I'm not sure if there is a place for adding something 
regarding this.

> I am wondering about the semantics of this bit. I don't think that you can 
> know for sure within clang whether a variable has been modified. The best you 
> can do is know for sure that some variable has been modified, but I don't 
> think you can prove that it has not been modified.



> Consequently I am wondering if you should change the name of this flag to 
> something like IsArgumentKnownToBeModified.



> Orthogonal to this you need to also add tests for templates.

I agree! You are right, good idea!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035



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


[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

2019-02-22 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

In D58345#1406892 , @sammccall wrote:

> Sorry to be a pain here, but I'm not sure introducing Javascript is a good 
> idea unless there's a strong reason for it.
>  All LLVM developers have python installed, many are comfortable with the 
> language - it seems less likely to be a maintenance hazard. I can recommend 
> BeautifulSoup for the HTML parsing stuff.
>  (For the record, I kind of hate python, but I'm more comfortable reviewing 
> and maintaining it than JS).
>  Mitigating points: we have some in the VSCode plugin by necessity, and this 
> isn't a build-time dependency.
>
> Happy to get another opinion here.
>
> > Added the tool in this patch, but I think a separate repository under 
> > GitHub's clangd org might be a more suitable place.
>
> I'm somewhat sympathetic to this idea, but I don't think we can do it as 
> things stand. @klimek can confirm.


I unfortunately agree with Sam on this.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58345/new/

https://reviews.llvm.org/D58345



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


[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-02-22 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro updated this revision to Diff 187931.
djtodoro added a comment.

- Add a field in `ParmVarDecl` instead of `VarDecl`
- Use a bit in `ParmVarDeclBitfields` to indicate parameter modification
- Add support for the bit in  `ASTReaderDecl.cpp` / `ASTWriterDecl.cpp`
- Add test case for templates


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035

Files:
  include/clang/AST/Decl.h
  lib/CodeGen/CGDebugInfo.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGen/dbginfo-var-change-templates.cpp
  test/CodeGen/debug-info-varchange.c

Index: test/CodeGen/debug-info-varchange.c
===
--- /dev/null
+++ test/CodeGen/debug-info-varchange.c
@@ -0,0 +1,35 @@
+// RUN: %clang -femit-param-entry-values -emit-llvm -S -g %s -o - | FileCheck %s
+
+// CHECK: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+// CHECK: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "test_s", arg: 3, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+// CHECK: !DILocalVariable(name: "test_s2", arg: 4, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "x", scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "y", scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+
+typedef struct s {
+  int m;
+  int n;
+}S;
+
+void foo (int a, int b,
+  S test_s, S test_s2)
+{
+  b++;
+  b = a + 1;
+  if (b>4)
+test_s2.m = 434;
+}
+
+int main()
+{
+  S test_s = {4, 5};
+
+  int x = 5;
+  int y = 6;
+
+  foo(x , y, test_s, test_s);
+
+  return 0;
+}
+
Index: test/CodeGen/dbginfo-var-change-templates.cpp
===
--- /dev/null
+++ test/CodeGen/dbginfo-var-change-templates.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang -femit-param-entry-values -emit-llvm -S -g %s -o - | FileCheck %s
+// CHECK: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+
+template 
+__attribute__((noinline))
+T GetMin (T a, T b) {
+  T result;
+  ++a;
+  result = (a < b) ? a : b;
+  return result;
+}
+
+int baa () {
+  int i=5, j=6, k;
+  k=GetMin(i,j);
+
+  if (i == k)
+++k;
+  else
+--k;
+
+  return k;
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -1033,6 +1033,7 @@
   Record.push_back(D->hasUninstantiatedDefaultArg());
   if (D->hasUninstantiatedDefaultArg())
 Record.AddStmt(D->getUninstantiatedDefaultArg());
+  Record.push_back(D->isArgumentKnownToBeModified());
   Code = serialization::DECL_PARM_VAR;
 
   assert(!D->isARCPseudoStrong()); // can be true of ImplicitParamDecl
@@ -1046,6 +1047,7 @@
   !D->isImplicit() &&
   !D->isUsed(false) &&
   !D->isInvalidDecl() &&
+  !D->isArgumentKnownToBeModified() &&
   !D->isReferenced() &&
   D->getAccess() == AS_none &&
   !D->isModulePrivate() &&
@@ -2011,6 +2013,7 @@
   Abv->Add(BitCodeAbbrevOp(0));   // KNRPromoted
   Abv->Add(BitCodeAbbrevOp(0));   // HasInheritedDefaultArg
   Abv->Add(BitCodeAbbrevOp(0));   // HasUninstantiatedDefaultArg
+  Abv->Add(BitCodeAbbrevOp(0));   // IsArgumentKnownToBeModified
   // Type Source Info
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // TypeLoc
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1441,7 +1441,7 @@
   PD->ParmVarDeclBits.HasInheritedDefaultArg = Record.readInt();
   if (Record.readInt()) // hasUninstantiatedDefaultArg.
 PD->setUninstantiatedDefaultArg(Record.readExpr());
-
+  PD->ParmVarDeclBits.IsArgumentKnownToBeModified = Record.readInt();
   // FIXME: If this is a redeclaration of a function from another module, handle
   // inheritance of default arguments.
 }
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -16,6 +16,7 @@
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
@@ -11276,6 +11277,15 @@
 DiagnoseCons

r354671 - [CUDA]Delayed diagnostics for the asm instructions.

2019-02-22 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Fri Feb 22 06:42:48 2019
New Revision: 354671

URL: http://llvm.org/viewvc/llvm-project?rev=354671&view=rev
Log:
[CUDA]Delayed diagnostics for the asm instructions.

Adapted targetDiag for the CUDA and used for the delayed diagnostics in
asm constructs. Works for both host and device compilation sides.

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

Added:
cfe/trunk/test/SemaCUDA/asm_delayed_diags.cu
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=354671&r1=354670&r2=354671&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb 22 06:42:48 2019
@@ -10208,8 +10208,9 @@ public:
const T &Value) {
   if (Diag.ImmediateDiag.hasValue())
 *Diag.ImmediateDiag << Value;
-  else if (Diag.PartialDiag.hasValue())
-*Diag.PartialDiag << Value;
+  else if (Diag.PartialDiagId.hasValue())
+Diag.S.DeviceDeferredDiags[Diag.Fn][*Diag.PartialDiagId].second
+<< Value;
   return Diag;
 }
 
@@ -10223,7 +10224,7 @@ public:
 // Invariant: At most one of these Optionals has a value.
 // FIXME: Switch these to a Variant once that exists.
 llvm::Optional ImmediateDiag;
-llvm::Optional PartialDiag;
+llvm::Optional PartialDiagId;
   };
 
   /// Indicate that this function (and thus everything it transtively calls)

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=354671&r1=354670&r2=354671&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Fri Feb 22 06:42:48 2019
@@ -1402,7 +1402,9 @@ Sema::DeviceDiagBuilder::DeviceDiagBuild
 break;
   case K_Deferred:
 assert(Fn && "Must have a function to attach the deferred diag to.");
-PartialDiag.emplace(S.PDiag(DiagID));
+auto &Diags = S.DeviceDeferredDiags[Fn];
+PartialDiagId.emplace(Diags.size());
+Diags.emplace_back(Loc, S.PDiag(DiagID));
 break;
   }
 }
@@ -1416,9 +1418,9 @@ Sema::DeviceDiagBuilder::~DeviceDiagBuil
 ImmediateDiag.reset(); // Emit the immediate diag.
 if (IsWarningOrError && ShowCallStack)
   emitCallStackNotes(S, Fn);
-  } else if (PartialDiag) {
-assert(ShowCallStack && "Must always show call stack for deferred diags.");
-S.DeviceDeferredDiags[Fn].push_back({Loc, std::move(*PartialDiag)});
+  } else {
+assert((!PartialDiagId || ShowCallStack) &&
+   "Must always show call stack for deferred diags.");
   }
 }
 
@@ -1487,10 +1489,12 @@ void Sema::markKnownEmitted(
   }
 }
 
-Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc,
- unsigned DiagID) {
+Sema::DeviceDiagBuilder Sema::targetDiag(SourceLocation Loc, unsigned DiagID) {
   if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice)
 return diagIfOpenMPDeviceCode(Loc, DiagID);
+  if (getLangOpts().CUDA)
+return getLangOpts().CUDAIsDevice ? CUDADiagIfDeviceCode(Loc, DiagID)
+  : CUDADiagIfHostCode(Loc, DiagID);
   return DeviceDiagBuilder(DeviceDiagBuilder::K_Immediate, Loc, DiagID,
getCurFunctionDecl(), *this);
 }

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=354671&r1=354670&r2=354671&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Feb 22 06:42:48 2019
@@ -750,7 +750,7 @@ ExprResult Sema::BuildCXXThrow(SourceLoc
bool IsThrownVarInScope) {
   // Don't report an error if 'throw' is used in system headers.
   if (!getLangOpts().CXXExceptions &&
-  !getSourceManager().isInSystemHeader(OpLoc)) {
+  !getSourceManager().isInSystemHeader(OpLoc) && !getLangOpts().CUDA) {
 // Delay error emission for the OpenMP device code.
 targetDiag(OpLoc, diag::err_exceptions_disabled) << "throw";
   }

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=354671&r1=354670&r2=354671&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Fri Feb 22 06:42:48 2019
@@ -3993,7 +3993,7 @@ StmtResult Sema::ActOnCXXTryBlock(Source
   ArrayRef Handlers) {
   // Don't report an error if '

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-02-22 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:11301
+EmitArgumentsValueModification(E);
+
   SourceLocation OrigLoc = Loc;

Comments:

1. Shouldn't you mark the variable to be modified only if 
`CheckForModifiableLvalue` returns true ?

2. I think that you need to handle way more than just member expressions. For 
example are you handling `(x, y)` (comma operator) ? But hard-coding every 
cases here is probably not ideal. It would be nice if there was already some 
code somewhere that could help you do this.

3. I believe that a `MemberExpr` has always a base. Similarly `DeclRefExpr`s 
always have a referenced declaration (so you can omit the `if`).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035



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


[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-02-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:11301
+EmitArgumentsValueModification(E);
+
   SourceLocation OrigLoc = Loc;

riccibruno wrote:
> Comments:
> 
> 1. Shouldn't you mark the variable to be modified only if 
> `CheckForModifiableLvalue` returns true ?
> 
> 2. I think that you need to handle way more than just member expressions. For 
> example are you handling `(x, y)` (comma operator) ? But hard-coding every 
> cases here is probably not ideal. It would be nice if there was already some 
> code somewhere that could help you do this.
> 
> 3. I believe that a `MemberExpr` has always a base. Similarly `DeclRefExpr`s 
> always have a referenced declaration (so you can omit the `if`).
I'm not quite sure what this differential is about, but i feel like mentioning 
ExprMutationAnalyzer lib in clang-tidy / clang-tools-extra.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58035/new/

https://reviews.llvm.org/D58035



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


[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

2019-02-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added reviewers: malaperle, sammccall, ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
Herald added a project: clang.

That can render to markdown or plain text. Used for findHover requests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/unittests/clangd/XRefsTests.cpp

Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -1154,7 +1154,7 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  EXPECT_EQ(H->renderAsPlainText(), Test.ExpectedHover.str()) << Test.Input;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
 
 #include "ClangdUnit.h"
+#include "FormattedString.h"
 #include "Protocol.h"
 #include "index/Index.h"
 #include "llvm/ADT/Optional.h"
@@ -47,7 +48,7 @@
   Position Pos);
 
 /// Get the hover information when hovering at \p Pos.
-llvm::Optional getHover(ParsedAST &AST, Position Pos);
+llvm::Optional getHover(ParsedAST &AST, Position Pos);
 
 /// Returns reference locations of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -7,6 +7,7 @@
 //===--===//
 #include "XRefs.h"
 #include "AST.h"
+#include "FormattedString.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -515,17 +516,17 @@
 }
 
 /// Generate a \p Hover object given the declaration \p D.
-static Hover getHoverContents(const Decl *D) {
-  Hover H;
+static FormattedString getHoverContents(const Decl *D) {
+  FormattedString R;
   llvm::Optional NamedScope = getScopeName(D);
 
   // Generate the "Declared in" section.
   if (NamedScope) {
 assert(!NamedScope->empty());
 
-H.contents.value += "Declared in ";
-H.contents.value += *NamedScope;
-H.contents.value += "\n\n";
+R.appendText("Declared in ");
+R.appendText(*NamedScope);
+R.appendText("\n");
   }
 
   // We want to include the template in the Hover.
@@ -537,35 +538,30 @@
 
   PrintingPolicy Policy =
   printingPolicyForDecls(D->getASTContext().getPrintingPolicy());
-
   D->print(OS, Policy);
 
-  OS.flush();
-
-  H.contents.value += DeclText;
-  return H;
+  R.appendCodeBlock(OS.str());
+  return R;
 }
 
 /// Generate a \p Hover object given the type \p T.
-static Hover getHoverContents(QualType T, ASTContext &ASTCtx) {
-  Hover H;
-  std::string TypeText;
-  llvm::raw_string_ostream OS(TypeText);
+static FormattedString getHoverContents(QualType T, ASTContext &ASTCtx) {
+  FormattedString R;
+
+  std::string Code;
+  llvm::raw_string_ostream OS(Code);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
-  OS.flush();
-  H.contents.value += TypeText;
-  return H;
+
+  R.appendCodeBlock(OS.str());
+  return R;
 }
 
 /// Generate a \p Hover object given the macro \p MacroInf.
-static Hover getHoverContents(llvm::StringRef MacroName) {
-  Hover H;
-
-  H.contents.value = "#define ";
-  H.contents.value += MacroName;
-
-  return H;
+static FormattedString getHoverContents(llvm::StringRef MacroName) {
+  FormattedString S;
+  S.appendCodeBlock("#define " + MacroName.str());
+  return S;
 }
 
 namespace {
@@ -680,7 +676,7 @@
   return V.getDeducedType();
 }
 
-llvm::Optional getHover(ParsedAST &AST, Position Pos) {
+llvm::Optional getHover(ParsedAST &AST, Position Pos) {
   const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
   SourceLocation SourceLocationBeg =
   getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
Index: clang-tools-extra/clangd/FormattedString.h
===
--- /dev/null
+++ clang-tools-extra/clangd/FormattedString.h
@@ -0,0 +1,56 @@
+//===-

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

2019-02-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

More concretely, here's the proposed API for the intermediate representation of 
formatted string: D58547 


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55250/new/

https://reviews.llvm.org/D55250



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


[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

2019-02-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

This is a follow-up for a discussion from D55250 
.
Still missing test, wanted to get some input on the API first.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58547/new/

https://reviews.llvm.org/D58547



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


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision.
jyknight added a reviewer: philip.pfaffe.
Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, steven_wu, 
hiraditya, mehdi_amini.
Herald added projects: clang, LLVM.

Just as as llvm IR supports explicitly specifying numeric value ids
for instructions, and emits them by default in textual output, now do
the same for blocks.

This is a slightly incompatible change in the textual IR format.

Previously, llvm would parse numeric labels as string names. E.g.

  define void @f() {
br label %"55"
  55:
ret void
  }

defined a label *named* "55", even without needing to be quoted, while
the reference required quoting. Now, if you intend a block label which
looks like a value number to be a name, you must quote it in the
definition too (e.g. `"55":`).

Previously, llvm would print nameless blocks only as a comment, and
would omit it if there was no predecessor. This could cause confusion
for readers of the IR, just as unnamed instructions did prior to the
addition of "%5 = " syntax, back in 2008 (PR2480).

Now, it will always print a label for an unnamed block, with the
exception of the entry block. (IMO it may be better to print it for
the entry-block as well. However, that requires updating many more
tests.)

Thus, the following is supported, and is the canonical printing:

  define i32 @f(i32, i32) {
%3 = add i32 %0, %1
br label %4
  
  4:
ret i32 %3
  }

New test cases covering this behavior are added, and other tests
updated as required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

I like this idea, and I don’t think the textual IR central is too important.  A 
few things:

- Changes to the IR should always be discussed on llvm-dev.  Did this already 
happen?
- Please update LangRef.
- Did you write a script for updating the test cases?  If so, you might 
consider attaching it to the RFC and linking to it from the commit message as a 
courtesy to downstream maintainers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In D58548#1407164 , @dexonsmith wrote:

> I like this idea, and I don’t think the textual IR central is too important.


Textual IR *change*; typing on a phone while walking :/.  By which I mean that 
IMO it’s fine to break/change this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


r354678 - CodeGen: use COMDAT for block copy/destroy helpers

2019-02-22 Thread Saleem Abdulrasool via cfe-commits
Author: compnerd
Date: Fri Feb 22 08:29:50 2019
New Revision: 354678

URL: http://llvm.org/viewvc/llvm-project?rev=354678&view=rev
Log:
CodeGen: use COMDAT for block copy/destroy helpers

SVN r339438 added support to deduplicate the helpers by using a consistent
naming scheme and using LinkOnceODR semantics.  This works on ELF by means of
weak linking semantics, and entirely does not work on PE/COFF where you end up
with multiply defined strong symbols, which is a strong error on PE/COFF.
Assign the functions a COMDAT group so that they can be uniqued by the linker.
This fixes the use of blocks in CoreFoundation on Windows.

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/test/CodeGen/blocks-1.c

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=354678&r1=354677&r2=354678&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Fri Feb 22 08:29:50 2019
@@ -2016,6 +2016,8 @@ CodeGenFunction::GenerateCopyHelperFunct
   llvm::Function *Fn =
 llvm::Function::Create(LTy, llvm::GlobalValue::LinkOnceODRLinkage,
FuncName, &CGM.getModule());
+  if (CGM.supportsCOMDAT())
+Fn->setComdat(CGM.getModule().getOrInsertComdat(FuncName));
 
   IdentifierInfo *II = &C.Idents.get(FuncName);
 
@@ -2207,6 +2209,8 @@ CodeGenFunction::GenerateDestroyHelperFu
   llvm::Function *Fn =
 llvm::Function::Create(LTy, llvm::GlobalValue::LinkOnceODRLinkage,
FuncName, &CGM.getModule());
+  if (CGM.supportsCOMDAT())
+Fn->setComdat(CGM.getModule().getOrInsertComdat(FuncName));
 
   IdentifierInfo *II = &C.Idents.get(FuncName);
 

Modified: cfe/trunk/test/CodeGen/blocks-1.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/blocks-1.c?rev=354678&r1=354677&r2=354678&view=diff
==
--- cfe/trunk/test/CodeGen/blocks-1.c (original)
+++ cfe/trunk/test/CodeGen/blocks-1.c Fri Feb 22 08:29:50 2019
@@ -1,10 +1,19 @@
-// RUN: %clang_cc1 %s -emit-llvm -o %t -fblocks
+// RUN: %clang_cc1 -triple thumbv7-apple-ios %s -emit-llvm -o %t -fblocks
 // RUN: grep "_Block_object_dispose" %t | count 12
 // RUN: grep "__copy_helper_block_" %t | count 9
 // RUN: grep "__destroy_helper_block_" %t | count 9
 // RUN: grep "__Block_byref_object_copy_" %t | count 2
 // RUN: grep "__Block_byref_object_dispose_" %t | count 2
 // RUN: grep "i32 135)" %t | count 2
+// RUN: grep "_Block_object_assign" %t | count 5
+
+// RUN: %clang_cc1 -triple thumbv7-unknown-windows %s -emit-llvm -o %t -fblocks
+// RUN: grep "_Block_object_dispose" %t | count 12
+// RUN: grep "__copy_helper_block_" %t | count 11
+// RUN: grep "__destroy_helper_block_" %t | count 11
+// RUN: grep "__Block_byref_object_copy_" %t | count 2
+// RUN: grep "__Block_byref_object_dispose_" %t | count 2
+// RUN: grep "i32 135)" %t | count 2
 // RUN: grep "_Block_object_assign" %t | count 5
 
 int printf(const char *, ...);


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


[PATCH] D57716: [CUDA][HIP] Check calling convention based on function target

2019-02-22 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: test/SemaCUDA/amdgpu-windows-vectorcall.cu:9-10
+
+template struct A<_Ret (__cdecl 
_Arg0::*)(_Types) > { };
+template struct A<_Ret (__vectorcall 
_Arg0::*)(_Types) > {};
+

tra wrote:
> I don't think we need templates here. We're only making sure that the 
> function attributes are handled correctly.
> Can we get by with a regular function declaration or definition?
> 
> 
Here we want to make sure `_Ret (__cdecl _Arg0::*)(_Types)` and `_Ret 
(__vectorcall _Arg0::*)(_Types)` are different types because calling convention 
is part of type, otherwise it results in compilation error due to duplicate 
template instantation. I can come up with a test without template.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57716/new/

https://reviews.llvm.org/D57716



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


[PATCH] D57716: [CUDA][HIP] Check calling convention based on function target

2019-02-22 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 187943.
yaxunl added a comment.

modify test to use non-template functions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57716/new/

https://reviews.llvm.org/D57716

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCUDA/amdgpu-windows-vectorcall.cu


Index: test/SemaCUDA/amdgpu-windows-vectorcall.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-windows-vectorcall.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only 
-verify %s
+
+__cdecl void hostf1();
+__vectorcall void (*hostf2)() = hostf1; // expected-error {{cannot initialize 
a variable of type 'void ((*))() __attribute__((vectorcall))' with an lvalue of 
type 'void () __attribute__((cdecl))'}}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4615,8 +4615,36 @@
   default: llvm_unreachable("unexpected attribute kind");
   }
 
+  TargetInfo::CallingConvCheckResult A = TargetInfo::CCCR_OK;
   const TargetInfo &TI = Context.getTargetInfo();
-  TargetInfo::CallingConvCheckResult A = TI.checkCallingConvention(CC);
+  auto *Aux = Context.getAuxTargetInfo();
+  if (LangOpts.CUDA) {
+auto CudaTarget = IdentifyCUDATarget(FD);
+bool CheckHost = false, CheckDevice = false;
+switch (CudaTarget) {
+case CFT_HostDevice:
+  CheckHost = true;
+  CheckDevice = true;
+  break;
+case CFT_Host:
+  CheckHost = true;
+  break;
+case CFT_Device:
+case CFT_Global:
+  CheckDevice = true;
+  break;
+case CFT_InvalidTarget:
+  llvm_unreachable("unexpected cuda target");
+}
+auto *HostTI = LangOpts.CUDAIsDevice ? Aux : &TI;
+auto *DeviceTI = LangOpts.CUDAIsDevice ? &TI : Aux;
+if (CheckHost && HostTI)
+  A = HostTI->checkCallingConvention(CC);
+if (A == TargetInfo::CCCR_OK && CheckDevice && DeviceTI)
+  A = DeviceTI->checkCallingConvention(CC);
+  } else {
+A = TI.checkCallingConvention(CC);
+  }
   if (A != TargetInfo::CCCR_OK) {
 if (A == TargetInfo::CCCR_Warning)
   Diag(Attrs.getLoc(), diag::warn_cconv_ignored) << Attrs;


Index: test/SemaCUDA/amdgpu-windows-vectorcall.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-windows-vectorcall.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+__cdecl void hostf1();
+__vectorcall void (*hostf2)() = hostf1; // expected-error {{cannot initialize a variable of type 'void ((*))() __attribute__((vectorcall))' with an lvalue of type 'void () __attribute__((cdecl))'}}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4615,8 +4615,36 @@
   default: llvm_unreachable("unexpected attribute kind");
   }
 
+  TargetInfo::CallingConvCheckResult A = TargetInfo::CCCR_OK;
   const TargetInfo &TI = Context.getTargetInfo();
-  TargetInfo::CallingConvCheckResult A = TI.checkCallingConvention(CC);
+  auto *Aux = Context.getAuxTargetInfo();
+  if (LangOpts.CUDA) {
+auto CudaTarget = IdentifyCUDATarget(FD);
+bool CheckHost = false, CheckDevice = false;
+switch (CudaTarget) {
+case CFT_HostDevice:
+  CheckHost = true;
+  CheckDevice = true;
+  break;
+case CFT_Host:
+  CheckHost = true;
+  break;
+case CFT_Device:
+case CFT_Global:
+  CheckDevice = true;
+  break;
+case CFT_InvalidTarget:
+  llvm_unreachable("unexpected cuda target");
+}
+auto *HostTI = LangOpts.CUDAIsDevice ? Aux : &TI;
+auto *DeviceTI = LangOpts.CUDAIsDevice ? &TI : Aux;
+if (CheckHost && HostTI)
+  A = HostTI->checkCallingConvention(CC);
+if (A == TargetInfo::CCCR_OK && CheckDevice && DeviceTI)
+  A = DeviceTI->checkCallingConvention(CC);
+  } else {
+A = TI.checkCallingConvention(CC);
+  }
   if (A != TargetInfo::CCCR_OK) {
 if (A == TargetInfo::CCCR_Warning)
   Diag(Attrs.getLoc(), diag::warn_cconv_ignored) << Attrs;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r354679 - [OPENMP] Delayed diagnostics for VLA support.

2019-02-22 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Fri Feb 22 08:49:13 2019
New Revision: 354679

URL: http://llvm.org/viewvc/llvm-project?rev=354679&view=rev
Log:
[OPENMP] Delayed diagnostics for VLA support.

Generalized processing of the deferred diagnostics for OpenMP/CUDA code.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/OpenMP/target_vla_messages.cpp
cfe/trunk/test/SemaCUDA/vla.cu

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=354679&r1=354678&r2=354679&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri Feb 22 08:49:13 2019
@@ -2250,15 +2250,13 @@ QualType Sema::BuildArrayType(QualType T
   }
 
   if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
-if (getLangOpts().CUDA) {
-  // CUDA device code doesn't support VLAs.
-  CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
-} else if (!getLangOpts().OpenMP ||
-   shouldDiagnoseTargetSupportFromOpenMP()) {
-  // Some targets don't support VLAs.
-  Diag(Loc, diag::err_vla_unsupported);
-  return QualType();
-}
+// CUDA device code and some other targets don't support VLAs.
+targetDiag(Loc, (getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
+? diag::err_cuda_vla
+: diag::err_vla_unsupported)
+<< ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
+? CurrentCUDATarget()
+: CFT_InvalidTarget);
   }
 
   // If this is not C99, extwarn about VLA's and C99 array size modifiers.

Modified: cfe/trunk/test/OpenMP/target_vla_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_vla_messages.cpp?rev=354679&r1=354678&r2=354679&view=diff
==
--- cfe/trunk/test/OpenMP/target_vla_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/target_vla_messages.cpp Fri Feb 22 08:49:13 2019
@@ -47,7 +47,7 @@ void target_template(int arg) {
 #pragma omp target
   {
 #ifdef NO_VLA
-// expected-error@+2 {{variable length arrays are not supported for the 
current target}}
+// expected-error@+2 2 {{variable length arrays are not supported for the 
current target}}
 #endif
 T vla[arg];
   }
@@ -73,6 +73,9 @@ void target(int arg) {
 }
   }
 
+#ifdef NO_VLA
+// expected-note@+2 {{in instantiation of function template specialization 
'target_template' requested here}}
+#endif
   target_template(arg);
 }
 

Modified: cfe/trunk/test/SemaCUDA/vla.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/vla.cu?rev=354679&r1=354678&r2=354679&view=diff
==
--- cfe/trunk/test/SemaCUDA/vla.cu (original)
+++ cfe/trunk/test/SemaCUDA/vla.cu Fri Feb 22 08:49:13 2019
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
-// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -verify -DHOST %s
+
+#ifndef __CUDA_ARCH__
+// expected-no-diagnostics
+#endif
 
 #include "Inputs/cuda.h"
 
@@ -8,7 +12,10 @@ void host(int n) {
 }
 
 __device__ void device(int n) {
-  int x[n];  // expected-error {{cannot use variable-length arrays in 
__device__ functions}}
+  int x[n];
+#ifdef __CUDA_ARCH__
+  // expected-error@-2 {{cannot use variable-length arrays in __device__ 
functions}}
+#endif
 }
 
 __host__ __device__ void hd(int n) {


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


r354680 - Revert "[OPENMP] Delayed diagnostics for VLA support."

2019-02-22 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Fri Feb 22 09:16:50 2019
New Revision: 354680

URL: http://llvm.org/viewvc/llvm-project?rev=354680&view=rev
Log:
Revert "[OPENMP] Delayed diagnostics for VLA support."

This reverts commit r354679 to fix the problem with the Windows
buildbots

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/OpenMP/target_vla_messages.cpp
cfe/trunk/test/SemaCUDA/vla.cu

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=354680&r1=354679&r2=354680&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri Feb 22 09:16:50 2019
@@ -2250,13 +2250,15 @@ QualType Sema::BuildArrayType(QualType T
   }
 
   if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
-// CUDA device code and some other targets don't support VLAs.
-targetDiag(Loc, (getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
-? diag::err_cuda_vla
-: diag::err_vla_unsupported)
-<< ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
-? CurrentCUDATarget()
-: CFT_InvalidTarget);
+if (getLangOpts().CUDA) {
+  // CUDA device code doesn't support VLAs.
+  CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
+} else if (!getLangOpts().OpenMP ||
+   shouldDiagnoseTargetSupportFromOpenMP()) {
+  // Some targets don't support VLAs.
+  Diag(Loc, diag::err_vla_unsupported);
+  return QualType();
+}
   }
 
   // If this is not C99, extwarn about VLA's and C99 array size modifiers.

Modified: cfe/trunk/test/OpenMP/target_vla_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_vla_messages.cpp?rev=354680&r1=354679&r2=354680&view=diff
==
--- cfe/trunk/test/OpenMP/target_vla_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/target_vla_messages.cpp Fri Feb 22 09:16:50 2019
@@ -47,7 +47,7 @@ void target_template(int arg) {
 #pragma omp target
   {
 #ifdef NO_VLA
-// expected-error@+2 2 {{variable length arrays are not supported for the 
current target}}
+// expected-error@+2 {{variable length arrays are not supported for the 
current target}}
 #endif
 T vla[arg];
   }
@@ -73,9 +73,6 @@ void target(int arg) {
 }
   }
 
-#ifdef NO_VLA
-// expected-note@+2 {{in instantiation of function template specialization 
'target_template' requested here}}
-#endif
   target_template(arg);
 }
 

Modified: cfe/trunk/test/SemaCUDA/vla.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/vla.cu?rev=354680&r1=354679&r2=354680&view=diff
==
--- cfe/trunk/test/SemaCUDA/vla.cu (original)
+++ cfe/trunk/test/SemaCUDA/vla.cu Fri Feb 22 09:16:50 2019
@@ -1,9 +1,5 @@
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -verify -DHOST %s
-
-#ifndef __CUDA_ARCH__
-// expected-no-diagnostics
-#endif
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
 
 #include "Inputs/cuda.h"
 
@@ -12,10 +8,7 @@ void host(int n) {
 }
 
 __device__ void device(int n) {
-  int x[n];
-#ifdef __CUDA_ARCH__
-  // expected-error@-2 {{cannot use variable-length arrays in __device__ 
functions}}
-#endif
+  int x[n];  // expected-error {{cannot use variable-length arrays in 
__device__ functions}}
 }
 
 __host__ __device__ void hd(int n) {


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


r354681 - Fix "not all control paths return" warning. NFCI.

2019-02-22 Thread Simon Pilgrim via cfe-commits
Author: rksimon
Date: Fri Feb 22 09:37:59 2019
New Revision: 354681

URL: http://llvm.org/viewvc/llvm-project?rev=354681&view=rev
Log:
Fix "not all control paths return" warning. NFCI.

Modified:
cfe/trunk/include/clang/Analysis/AnyCall.h

Modified: cfe/trunk/include/clang/Analysis/AnyCall.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnyCall.h?rev=354681&r1=354680&r2=354681&view=diff
==
--- cfe/trunk/include/clang/Analysis/AnyCall.h (original)
+++ cfe/trunk/include/clang/Analysis/AnyCall.h Fri Feb 22 09:37:59 2019
@@ -173,6 +173,7 @@ public:
 case Deallocator:
   return cast(D)->getReturnType();
 }
+llvm_unreachable("Unknown AnyCall::Kind");
   }
 
   /// \returns Function identifier if it is a named declaration,


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


[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

2019-02-22 Thread Nolan O'Brien via Phabricator via cfe-commits
NSProgrammer added a comment.

We would prefer a macro like `__FILE_NAME__` over a build flag for code reading 
consistency (they would clearly do different things vs varying based on an 
obscure flag being present/absent).

This patch is languishing, unless the original author thinks otherwise, a new 
patch to push this through would be great.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



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


[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

2019-02-22 Thread Micah S. via Phabricator via cfe-commits
micah-s added a comment.

ClamAV recently started using clang-format.  We published this blog post about 
how we're using it: 
https://blog.clamav.net/2019/02/clamav-adopts-clang-format.html  One of the 
things I called out in the blog post is that we really want this feature, and 
presently we have to use the "// clang-format on/off" markers throughout our 
code mostly to keep our consecutive macros aligned.

I haven't found time to build clang-format with this patch to test it on our 
code base with the markers disabled.  If a review of my experience doing so 
will help get traction for this review in any way, let me know and I'll make 
the effort.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D28462/new/

https://reviews.llvm.org/D28462



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


[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

2019-02-22 Thread Mandeep Singh Grang via Phabricator via cfe-commits
mgrang added a comment.

In D50488#1405094 , @mgrang wrote:

> So I was able compile a couple of C++ code bases through csa-testbench. I 
> built cppcheck and tinyxml2 without any problems. cppcheck has one invocation 
> std::sort but the keys are not pointers whereas tinyxml2 does not use 
> std::sort. I tried bitcoin, rtags, xerces but run into a lot of 
> configure/build errors.


@Szelethus Are you able to build bitcoin, xerces, etc? I seem to hit "Could not 
link against boost_filesystem" errors with bitcoin. I have installed 
dependencies mentioned in 
https://stackoverflow.com/questions/9132335/configure-error-could-not-link-against-boost-system/47773206
 and https://github.com/bitcoin/bitcoin/issues/8749.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50488/new/

https://reviews.llvm.org/D50488



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


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread David Greene via Phabricator via cfe-commits
greened added a comment.

+1.  Is there any reason not to use "%4" in the definition?

  define i32 @f(i32, i32) {
%3 = add i32 %0, %1
br label %4
  
  %4:
ret i32 %3
  }

Maybe it creates an ambiguity in the grammar or something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In D58548#1407331 , @greened wrote:

> +1.  Is there any reason not to use "%4" in the definition?
>
>   define i32 @f(i32, i32) {
> %3 = add i32 %0, %1
> br label %4
>  
>   %4:
> ret i32 %3
>   }
>
>
> Maybe it creates an ambiguity in the grammar or something.


A `%` isn't used for labels that do have names, so David's approach seems 
consistent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


[PATCH] D58321: [WIP] Support for relative vtables

2019-02-22 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

Can we start with a patch that just exposes a flag that enables the relative 
ABI unconditionally, and remove all the platform ABI compatibility stuff?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58321/new/

https://reviews.llvm.org/D58321



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


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In D58548#1407355 , @dexonsmith wrote:

> In D58548#1407331 , @greened wrote:
>
> > +1.  Is there any reason not to use "%4" in the definition?
> >
> >   define i32 @f(i32, i32) {
> > %3 = add i32 %0, %1
> > br label %4
> >  
> >   %4:
> > ret i32 %3
> >   }
> >
> >
> > Maybe it creates an ambiguity in the grammar or something.
>
>
> A `%` isn't used for labels that do have names, so David's approach seems 
> consistent.


s/David/James/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


[PATCH] D20749: Introduce support for relative C++ ABI gated on LTO visibility.

2019-02-22 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added a comment.
Herald added a subscriber: jdoerfert.

Hi @pcc , I'm working on revisiting this to see if this could help when 
building Fuchsia (D58321 ) and had a few 
questions I left inline.




Comment at: lib/CodeGen/CGVTables.cpp:532
+  for (auto &Comp : VTLayout.vtable_components()) {
+if (Comp.isFunctionPointerKind())
+  Types.push_back(CGM.Int32Ty);

It seems that the condition for treating a vtable component as an i32 vs i8* is 
slightly different here than in `SetVTableInitializer`. `SetVTableInitializer` 
checks

```
Component.isFunctionPointerKind() && (I == 0 || !Components[I - 
1].isFunctionPointerKind())
```

where `GetVTableType` just checks `Component.isFunctionPointerKind()`.

Should this also check if the first or last component is a function pointer, 
and does this check in `SetVTableInitializer` have any importance with the 
FIXME below it?



Comment at: test/CodeGenCXX/vtable-relative-abi.cpp:5
+
+// CHECK-ITANIUM: @_ZTV1S = hidden unnamed_addr constant { i8*, i8*, i32, i32 
} { i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1S to i8*), i32 trunc (i64 sub 
(i64 ptrtoint (void (%struct.S*)* @_ZN1S2f1Ev to i64), i64 ptrtoint (i32* 
getelementptr inbounds ({ i8*, i8*, i32, i32 }, { i8*, i8*, i32, i32 }* 
@_ZTV1S, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (void 
(%struct.S*)* @_ZN1S2f2Ev to i64), i64 ptrtoint (i32* getelementptr inbounds ({ 
i8*, i8*, i32, i32 }, { i8*, i8*, i32, i32 }* @_ZTV1S, i32 0, i32 2) to i64)) 
to i32) }, align 8
+// CHECK-MS: @0 = private unnamed_addr constant { i8*, i32, i32 } { i8* 
bitcast (%rtti.CompleteObjectLocator* @"\01??_R4S@@6B@" to i8*), i32 trunc (i64 
sub (i64 ptrtoint (void (%struct.S*)* @"\01?f1@S@@UEAAXXZ" to i64), i64 
ptrtoint (i32* getelementptr inbounds ({ i8*, i32, i32 }, { i8*, i32, i32 }* 
@0, i32 0, i32 1) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (void 
(%struct.S*)* @"\01?f2@S@@UEAAXXZ" to i64), i64 ptrtoint (i32* getelementptr 
inbounds ({ i8*, i32, i32 }, { i8*, i32, i32 }* @0, i32 0, i32 1) to i64)) to 
i32) }, comdat($"\01??_7S@@6B@")

Should the second `getelementptr` access be `i32 0, i32 3` instead since it’s 
for the 4th element in the struct?

If so, I think this is due to some referencing problem with the 
`maybeMakeRelative` lambda. I ran into this issue when I moved `llvm::Type 
*VTableTy = VTable->getValueType();` into the following if block that 
initializes `AddrPointInt`. This also seems to occur for other test classes 
that contain more than one virtual function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D20749/new/

https://reviews.llvm.org/D20749



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


[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 187958.
lildmh retitled this revision from "[OpenMP 5.0] Parsing/sema support for to 
and from clauses with mapper modifier" to "[OpenMP 5.0] Parsing/sema support 
for to clause with mapper modifier".
lildmh edited the summary of this revision.
lildmh added a comment.

get rid of the from clause part.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_codegen.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp

Index: test/OpenMP/declare_mapper_messages.cpp
===
--- test/OpenMP/declare_mapper_messages.cpp
+++ test/OpenMP/declare_mapper_messages.cpp
@@ -29,7 +29,7 @@
 #pragma omp declare mapper(bb: vec v) private(v)// expected-error {{expected at least one clause on '#pragma omp declare mapper' directive}} // expected-error {{unexpected OpenMP clause 'private' in directive '#pragma omp declare mapper'}}
 #pragma omp declare mapper(cc: vec v) map(v) (  // expected-warning {{extra tokens at the end of '#pragma omp declare mapper' are ignored}}
 
-#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal OpenMP user-defined mapper identifier}}
 #pragma omp declare mapper(id1: vec v) map(v.len, temp) // expected-error {{only variable v is allowed in map clauses of this 'omp declare mapper' directive}}
 #pragma omp declare mapper(default : vec kk) map(kk.data[0:2])  // expected-note {{previous definition is here}}
 #pragma omp declare mapper(vec v) map(v.len)// expected-error {{redefinition of user-defined mapper for type 'vec' with name 'default'}}
@@ -74,9 +74,9 @@
   {}
 #pragma omp target map(mapper(ab) :vv)  // expected-error {{missing map type}} expected-error {{cannot find a valid user-defined mapper for type 'vec' with name 'ab'}}
   {}
-#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
-#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
 #pragma omp target map(mapper(aa) :vv)  // expected-error {{missing map type}}
   {}
@@ -86,6 +86,19 @@
   {}
 #pragma omp target map(mapper(N1::stack::id) to:vv)
   {}
+
+#pragma omp target update to(mapper)// expected-error {{expected '(' after 'mapper'}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper()   // expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper:vv) // expected-error {{expected '(' after 'mapper'}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(:vv)// expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(aa :vv) // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(N2:: :vv) 

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

2019-02-22 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

One comment, but otherwise LGTM.




Comment at: lib/Sema/SemaCast.cpp:2309
+auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType(
+DestPointeeType.getCanonicalType());
+return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,

ebevhan wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > ebevhan wrote:
> > > > Maybe I'm mistaken, but won't getting the canonical type here drop 
> > > > qualifiers (like cv) in nested pointers? If so, an addrspace_cast might 
> > > > strip qualifiers by mistake.
> > > Yes, indeed I will need to extend this to nested pointers when we are 
> > > ready. But for now I can try to change this bit... however I am not sure 
> > > it will work w/o canonical types when we have typedef. I will try to 
> > > create an example and see.
> > I checked the canonical type does preserve the qualifiers correctly.
> > 
> > Here is the AST dump of the following C type `mytype const __generic*` 
> > where  `typedef  __generic int* mytype;`.
> > 
> > 
> > ```
> > PointerType 0x204d3b0 'const __generic mytype *'
> > `-QualType 0x204d369 'const __generic mytype' const __generic
> >   `-TypedefType 0x204d320 'mytype' sugar
> > |-Typedef 0x204d1b0 'mytype'
> > `-PointerType 0x204d170 '__generic int *'
> >   `-QualType 0x204d158 '__generic int' __generic
> > `-BuiltinType 0x2009750 'int'
> > ```
> > 
> > and it's canonical representation in AST is:
> > 
> > ```
> > PointerType 0x204d380 '__generic int *const __generic *'
> > `-QualType 0x204d349 '__generic int *const __generic' const __generic
> >   `-PointerType 0x204d170 '__generic int *'
> > `-QualType 0x204d158 '__generic int' __generic
> >   `-BuiltinType 0x2009750 'int'
> > ```
> > 
> > So using canonical type will just simply handling of nested pointer chain 
> > by avoiding special casing typedefs. We won't loose any qualifiers.
> Okay, seems good then!
It seems to me that the rules in this function are the reasonable 
cross-language rules.  If you want to check for OpenCL at the top as a 
fast-path check (taking advantage of that fact that no other language modes 
currently have overlapping address spaces), that might be reasonable, but the 
comment and indentation should reflect that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58346/new/

https://reviews.llvm.org/D58346



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


[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:13195
-  } else {
-MVLI.UDMapperList.push_back(nullptr);
   }

Is this correct for `from` clause?



Comment at: lib/Sema/SemaOpenMP.cpp:13237
-  } else {
-MVLI.UDMapperList.push_back(nullptr);
   }

Same here: what about `from`?



Comment at: lib/Sema/TreeTransform.h:8821
+TreeTransform &TT, OMPMappableExprListClause *C,
+llvm::SmallVector &Vars, CXXScopeSpec &MapperIdScopeSpec,
+DeclarationNameInfo &MapperIdInfo,

`llvm::SmallVector &` -> `llvm::SmallVectorImpl &`



Comment at: lib/Sema/TreeTransform.h:8823
+DeclarationNameInfo &MapperIdInfo,
+llvm::SmallVector &UnresolvedMappers) {
+  // Transform expressions in the list.

llvm::SmallVector & -> llvm::SmallVectorImpl &


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523



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


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 187963.
jyknight added a comment.

Add some wording to LangRef and clang-format.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/Sanitize

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D58548#1407164 , @dexonsmith wrote:

> I like this idea, and I don’t think the textual IR central is too important.  
> A few things:
>
> - Changes to the IR should always be discussed on llvm-dev.  Did this already 
> happen?


I sent a message ("Improving textual IR format for nameless blocks") 
concurrently with sending this review, and will wait a bit to make sure there's 
no objections.

> - Please update LangRef.

Done.

> - Did you write a script for updating the test cases?  If so, you might 
> consider attaching it to the RFC and linking to it from the commit message as 
> a courtesy to downstream maintainers.

Nope, I didn't; there were few enough instances that it didn't seem worth 
scripting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Lingda Li via Phabricator via cfe-commits
lildmh added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:13195
-  } else {
-MVLI.UDMapperList.push_back(nullptr);
   }

ABataev wrote:
> Is this correct for `from` clause?
Yes, it's correct for `from`, which will never use `MVLI.UDMapperList`. 
Regression test is also correct.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523



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


[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 187964.
lildmh marked 4 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_codegen.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp

Index: test/OpenMP/declare_mapper_messages.cpp
===
--- test/OpenMP/declare_mapper_messages.cpp
+++ test/OpenMP/declare_mapper_messages.cpp
@@ -29,7 +29,7 @@
 #pragma omp declare mapper(bb: vec v) private(v)// expected-error {{expected at least one clause on '#pragma omp declare mapper' directive}} // expected-error {{unexpected OpenMP clause 'private' in directive '#pragma omp declare mapper'}}
 #pragma omp declare mapper(cc: vec v) map(v) (  // expected-warning {{extra tokens at the end of '#pragma omp declare mapper' are ignored}}
 
-#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal OpenMP user-defined mapper identifier}}
 #pragma omp declare mapper(id1: vec v) map(v.len, temp) // expected-error {{only variable v is allowed in map clauses of this 'omp declare mapper' directive}}
 #pragma omp declare mapper(default : vec kk) map(kk.data[0:2])  // expected-note {{previous definition is here}}
 #pragma omp declare mapper(vec v) map(v.len)// expected-error {{redefinition of user-defined mapper for type 'vec' with name 'default'}}
@@ -74,9 +74,9 @@
   {}
 #pragma omp target map(mapper(ab) :vv)  // expected-error {{missing map type}} expected-error {{cannot find a valid user-defined mapper for type 'vec' with name 'ab'}}
   {}
-#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
-#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
 #pragma omp target map(mapper(aa) :vv)  // expected-error {{missing map type}}
   {}
@@ -86,6 +86,19 @@
   {}
 #pragma omp target map(mapper(N1::stack::id) to:vv)
   {}
+
+#pragma omp target update to(mapper)// expected-error {{expected '(' after 'mapper'}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper()   // expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper:vv) // expected-error {{expected '(' after 'mapper'}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(:vv)// expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(aa :vv) // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(N2:: :vv)   // expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pra

[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

2019-02-22 Thread Hyrum Wright via Phabricator via cfe-commits
hwright updated this revision to Diff 187967.
hwright marked 15 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58137/new/

https://reviews.llvm.org/D58137

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tidy/abseil/TimeSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-time-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-time-subtraction.cpp

Index: test/clang-tidy/abseil-time-subtraction.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-time-subtraction.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s abseil-time-subtraction %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void g(absl::Duration d);
+
+void f() {
+  absl::Time t;
+  int x, y;
+  absl::Duration d;
+
+  d = absl::Hours(absl::ToUnixHours(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixHours(x));
+  d = absl::Minutes(absl::ToUnixMinutes(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixMinutes(x));
+  d = absl::Seconds(absl::ToUnixSeconds(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixSeconds(x));
+  d = absl::Milliseconds(absl::ToUnixMillis(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixMillis(x));
+  d = absl::Microseconds(absl::ToUnixMicros(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixMicros(x));
+  d = absl::Nanoseconds(absl::ToUnixNanos(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixNanos(x));
+
+  y = x - absl::ToUnixHours(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Hours(absl::FromUnixHours(x) - t);
+  y = x - absl::ToUnixMinutes(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Minutes(absl::FromUnixMinutes(x) - t);
+  y = x - absl::ToUnixSeconds(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Seconds(absl::FromUnixSeconds(x) - t);
+  y = x - absl::ToUnixMillis(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Milliseconds(absl::FromUnixMillis(x) - t);
+  y = x - absl::ToUnixMicros(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Microseconds(absl::FromUnixMicros(x) - t);
+  y = x - absl::ToUnixNanos(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Nanoseconds(absl::FromUnixNanos(x) - t);
+
+  // Check parenthesis placement
+  d = 5 * absl::Seconds(absl::ToUnixSeconds(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = 5 * (t - absl::FromUnixSeconds(x));
+  d = absl::Seconds(absl::ToUnixSeconds(t) - x) / 5;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixSeconds(x)) / 5;
+
+  // No extra parens around arguments
+  g(absl::Seconds(absl::ToUnixSeconds(t) - x));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: g(t - absl::FromUnixSeconds(x));
+  g(absl::Seconds(x - absl::ToUnixSeconds(t)));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: g(absl::FromUnixSeconds(x) - t);
+
+  // These should not trigger; they are likely bugs
+  d = absl::Milliseconds(absl::ToUnixSeconds(t) - x);
+  d = absl::Seconds(absl::ToUnixMicros(t) - x);
+
+  // Various macro scenarios
+#define SUB(z, t1) z - absl::ToUnixSeconds(t1)
+  y = SUB(x, t);
+#undef SUB
+
+#define MILLIS(t1) absl::ToUnixMillis(t1)
+  y = x - MILLIS(t);
+#undef MILLIS
+
+#define HOURS(z) absl::Hours(z

[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523



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


[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

2019-02-22 Thread Hyrum Wright via Phabricator via cfe-commits
hwright added inline comments.



Comment at: clang-tidy/abseil/TimeSubtractionCheck.cpp:97
+void TimeSubtractionCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *BinOp = Result.Nodes.getNodeAs("binop");
+  std::string inverse_name =

JonasToth wrote:
> Could you please split this function up into smaller ones. There are three or 
> four distinct cases that are easier to comprehend in isolation.
The actual bodies of these if-statements are only one or two separate 
statements themselves.  Moving those to separate functions seems like it would 
just obfuscate things a bit.



Comment at: clang-tidy/abseil/TimeSubtractionCheck.h:19
+/// Finds and fixes `absl::Time` subtraction expressions to do subtraction
+/// in the Time domain instead of the numeric domain.
+///

JonasToth wrote:
> nit: 'Time' domain
This doesn't refer to a type, but a library system, so it probably isn't 
appropriate to quote it.

(Just has how one wouldn't quote "frequency" when talking about "the frequency 
domain" of a Fourier transform.)



Comment at: test/clang-tidy/abseil-time-subtraction.cpp:12
+
+  d = absl::Hours(absl::ToUnixHours(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time 
domain [abseil-time-subtraction]

JonasToth wrote:
> please add tests where `x` itself is a calculation with different precedence 
> of its operators (multiplication, addition) to ensure these cases are 
> transformed properly as well.
This doesn't actually matter in this case: `x` will be wrapped in a function 
call.

It does matter in the case where we //unwrap// the first argument (below) and 
I've already got a test which uses multiplication in this case.  I've also 
added one for division.



Comment at: test/clang-tidy/abseil-time-subtraction.cpp:78
+  // CHECK-FIXES: return absl::FromUnixSeconds(x) - t;
+}

JonasToth wrote:
> please add tests for templates and macros.
I've add tests for macros, though I'm not sure what cases you have in mind 
regarding templates.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58137/new/

https://reviews.llvm.org/D58137



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


[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Lingda Li via Phabricator via cfe-commits
lildmh added a comment.

Thanks a lot!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523



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


[PATCH] D58404: [clang-format] Add basic support for formatting C# files

2019-02-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 187969.
MyDeveloperDay added a comment.

Increase C# formatting capabilities

- don't split regions markers across lines
- lexer support for verbatim string literals
- support for interpolated string literals (C#6)
- support for  interpolated verbatim string literals
- support for keyword escaping @enum, @class etc..
  - support for null conditionals

Add additional unit tests to support the above


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58404/new/

https://reviews.llvm.org/D58404

Files:
  docs/ClangFormat.rst
  docs/ClangFormatStyleOptions.rst
  docs/ReleaseNotes.rst
  include/clang/Basic/LangOptions.def
  include/clang/Basic/TokenKinds.def
  include/clang/Basic/TokenKinds.h
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/LiteralSupport.cpp
  lib/Lex/TokenConcatenation.cpp
  tools/clang-format/ClangFormat.cpp
  unittests/Format/CMakeLists.txt
  unittests/Format/FormatTestCSharp.cpp

Index: unittests/Format/FormatTestCSharp.cpp
===
--- /dev/null
+++ unittests/Format/FormatTestCSharp.cpp
@@ -0,0 +1,170 @@
+//===- unittest/Format/FormatTestCSharp.cpp - Formatting tests for CSharp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestCSharp : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle &Style) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  format(llvm::StringRef Code,
+ const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
+FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_CSharp)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestCSharp, CSharpClass) {
+  verifyFormat("public class SomeClass {\n"
+   "  void f() {}\n"
+   "  int g() { return 0; }\n"
+   "  void h() {\n"
+   "while (true) f();\n"
+   "for (;;) f();\n"
+   "if (true) f();\n"
+   "  }\n"
+   "}");
+}
+
+TEST_F(FormatTestCSharp, AccessModifiers) {
+  verifyFormat("public String toString() {}");
+  verifyFormat("private String toString() {}");
+  verifyFormat("protected String toString() {}");
+  verifyFormat("internal String toString() {}");
+
+  verifyFormat("public override String toString() {}");
+  verifyFormat("private override String toString() {}");
+  verifyFormat("protected override String toString() {}");
+  verifyFormat("internal override String toString() {}");
+
+  verifyFormat("internal static String toString() {}");
+}
+
+TEST_F(FormatTestCSharp, NoStringLiteralBreaks) {
+  verifyFormat("foo("
+   "\"a"
+   "aa\");");
+}
+
+TEST_F(FormatTestCSharp, CSharpVerbatiumStringLiterals) {
+  verifyFormat("foo(@\"\\abc\\\");");
+  // @"ABC\" + ToString("B") - handle embedded \ in literal string at
+  // the end
+  verifyFormat("string s = @\"ABC\\\" + ToString(\"B\");");
+}
+
+TEST_F(FormatTestCSharp, CSharpInterpolatedStringLiterals) {
+  verifyFormat("foo($\"{aaa}\");");
+  verifyFormat("foo($\"{A}\");");
+  verifyFormat(
+  "foo($\"{A}"
+  "a\");");
+  verifyFormat("Name = $\"{firstName} {lastName}\";");
+
+  // $"ABC\" + ToString

r354690 - [OPENMP] Delayed diagnostics for VLA support.

2019-02-22 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Fri Feb 22 12:36:10 2019
New Revision: 354690

URL: http://llvm.org/viewvc/llvm-project?rev=354690&view=rev
Log:
[OPENMP] Delayed diagnostics for VLA support.

Generalized processing of the deferred diagnostics for OpenMP/CUDA code.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/OpenMP/target_vla_messages.cpp
cfe/trunk/test/SemaCUDA/vla.cu

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=354690&r1=354689&r2=354690&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb 22 12:36:10 2019
@@ -10189,6 +10189,8 @@ public:
 
 DeviceDiagBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   FunctionDecl *Fn, Sema &S);
+DeviceDiagBuilder(DeviceDiagBuilder &&D);
+DeviceDiagBuilder(const DeviceDiagBuilder &) = default;
 ~DeviceDiagBuilder();
 
 /// Convertible to bool: True if we immediately emitted an error, false if

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=354690&r1=354689&r2=354690&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Fri Feb 22 12:36:10 2019
@@ -1409,6 +1409,16 @@ Sema::DeviceDiagBuilder::DeviceDiagBuild
   }
 }
 
+Sema::DeviceDiagBuilder::DeviceDiagBuilder(DeviceDiagBuilder &&D)
+: S(D.S), Loc(D.Loc), DiagID(D.DiagID), Fn(D.Fn),
+  ShowCallStack(D.ShowCallStack), ImmediateDiag(D.ImmediateDiag),
+  PartialDiagId(D.PartialDiagId) {
+  // Clean the previous diagnostics.
+  D.ShowCallStack = false;
+  D.ImmediateDiag.reset();
+  D.PartialDiagId.reset();
+}
+
 Sema::DeviceDiagBuilder::~DeviceDiagBuilder() {
   if (ImmediateDiag) {
 // Emit our diagnostic and, if it was a warning or error, output a 
callstack

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=354690&r1=354689&r2=354690&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri Feb 22 12:36:10 2019
@@ -2250,15 +2250,13 @@ QualType Sema::BuildArrayType(QualType T
   }
 
   if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
-if (getLangOpts().CUDA) {
-  // CUDA device code doesn't support VLAs.
-  CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
-} else if (!getLangOpts().OpenMP ||
-   shouldDiagnoseTargetSupportFromOpenMP()) {
-  // Some targets don't support VLAs.
-  Diag(Loc, diag::err_vla_unsupported);
-  return QualType();
-}
+// CUDA device code and some other targets don't support VLAs.
+targetDiag(Loc, (getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
+? diag::err_cuda_vla
+: diag::err_vla_unsupported)
+<< ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
+? CurrentCUDATarget()
+: CFT_InvalidTarget);
   }
 
   // If this is not C99, extwarn about VLA's and C99 array size modifiers.

Modified: cfe/trunk/test/OpenMP/target_vla_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_vla_messages.cpp?rev=354690&r1=354689&r2=354690&view=diff
==
--- cfe/trunk/test/OpenMP/target_vla_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/target_vla_messages.cpp Fri Feb 22 12:36:10 2019
@@ -47,7 +47,7 @@ void target_template(int arg) {
 #pragma omp target
   {
 #ifdef NO_VLA
-// expected-error@+2 {{variable length arrays are not supported for the 
current target}}
+// expected-error@+2 2 {{variable length arrays are not supported for the 
current target}}
 #endif
 T vla[arg];
   }
@@ -73,6 +73,9 @@ void target(int arg) {
 }
   }
 
+#ifdef NO_VLA
+// expected-note@+2 {{in instantiation of function template specialization 
'target_template' requested here}}
+#endif
   target_template(arg);
 }
 

Modified: cfe/trunk/test/SemaCUDA/vla.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/vla.cu?rev=354690&r1=354689&r2=354690&view=diff
==
--- cfe/trunk/test/SemaCUDA/vla.cu (original)
+++ cfe/trunk/test/SemaCUDA/vla.cu Fri Feb 22 12:36:10 2019
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
-// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -verify -DHOST %s
+
+#ifndef __CUDA_ARCH__
+// expected-no-diagno

[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 187970.
lildmh added a comment.

Rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_codegen.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp

Index: test/OpenMP/declare_mapper_messages.cpp
===
--- test/OpenMP/declare_mapper_messages.cpp
+++ test/OpenMP/declare_mapper_messages.cpp
@@ -29,7 +29,7 @@
 #pragma omp declare mapper(bb: vec v) private(v)// expected-error {{expected at least one clause on '#pragma omp declare mapper' directive}} // expected-error {{unexpected OpenMP clause 'private' in directive '#pragma omp declare mapper'}}
 #pragma omp declare mapper(cc: vec v) map(v) (  // expected-warning {{extra tokens at the end of '#pragma omp declare mapper' are ignored}}
 
-#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp declare mapper(++: vec v) map(v.len)// expected-error {{illegal OpenMP user-defined mapper identifier}}
 #pragma omp declare mapper(id1: vec v) map(v.len, temp) // expected-error {{only variable v is allowed in map clauses of this 'omp declare mapper' directive}}
 #pragma omp declare mapper(default : vec kk) map(kk.data[0:2])  // expected-note {{previous definition is here}}
 #pragma omp declare mapper(vec v) map(v.len)// expected-error {{redefinition of user-defined mapper for type 'vec' with name 'default'}}
@@ -74,9 +74,9 @@
   {}
 #pragma omp target map(mapper(ab) :vv)  // expected-error {{missing map type}} expected-error {{cannot find a valid user-defined mapper for type 'vec' with name 'ab'}}
   {}
-#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N2::) :vv)// expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
-#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal identifier on 'omp declare mapper' directive}}
+#pragma omp target map(mapper(N1::) :vv)// expected-error {{illegal OpenMP user-defined mapper identifier}}
   {}
 #pragma omp target map(mapper(aa) :vv)  // expected-error {{missing map type}}
   {}
@@ -86,6 +86,19 @@
   {}
 #pragma omp target map(mapper(N1::stack::id) to:vv)
   {}
+
+#pragma omp target update to(mapper)// expected-error {{expected '(' after 'mapper'}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper()   // expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected expression}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper:vv) // expected-error {{expected '(' after 'mapper'}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(:vv)// expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(aa :vv) // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(mapper(N2:: :vv)   // expected-error {{use of undeclared identifier 'N2'}} expected-error {{illegal OpenMP user-defined mapper identifier}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp 

r354691 - [clang] Only provide C11 features in starting with C++17

2019-02-22 Thread Louis Dionne via cfe-commits
Author: ldionne
Date: Fri Feb 22 12:48:54 2019
New Revision: 354691

URL: http://llvm.org/viewvc/llvm-project?rev=354691&view=rev
Log:
[clang] Only provide C11 features in  starting with C++17

Summary:
In r353970, I enabled those features in C++11 and above. To be strictly
conforming, those features should only be enabled in C++17 and above.

Reviewers: jfb, eli.friedman

Subscribers: jkorous, dexonsmith, libcxx-commits

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

Modified:
cfe/trunk/lib/Headers/float.h
cfe/trunk/test/Headers/float.c

Modified: cfe/trunk/lib/Headers/float.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/float.h?rev=354691&r1=354690&r2=354691&view=diff
==
--- cfe/trunk/lib/Headers/float.h (original)
+++ cfe/trunk/lib/Headers/float.h Fri Feb 22 12:48:54 2019
@@ -78,7 +78,7 @@
 #  undef FLT_MIN
 #  undef DBL_MIN
 #  undef LDBL_MIN
-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201103L
+#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201703L
 #undef FLT_TRUE_MIN
 #undef DBL_TRUE_MIN
 #undef LDBL_TRUE_MIN
@@ -137,7 +137,7 @@
 #define DBL_MIN __DBL_MIN__
 #define LDBL_MIN __LDBL_MIN__
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201703L
 #  define FLT_TRUE_MIN __FLT_DENORM_MIN__
 #  define DBL_TRUE_MIN __DBL_DENORM_MIN__
 #  define LDBL_TRUE_MIN __LDBL_DENORM_MIN__

Modified: cfe/trunk/test/Headers/float.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/float.c?rev=354691&r1=354690&r2=354691&view=diff
==
--- cfe/trunk/test/Headers/float.c (original)
+++ cfe/trunk/test/Headers/float.c Fri Feb 22 12:48:54 2019
@@ -45,7 +45,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201703L
 #ifndef FLT_DECIMAL_DIG
 #error "Mandatory macro FLT_DECIMAL_DIG is missing."
 #elif   FLT_DECIMAL_DIG < 6
@@ -215,7 +215,7 @@ _Static_assert(FLT_MANT_DIG == __FLT_MAN
 _Static_assert(DBL_MANT_DIG == __DBL_MANT_DIG__, "");
 _Static_assert(LDBL_MANT_DIG == __LDBL_MANT_DIG__, "");
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201703L
 _Static_assert(FLT_DECIMAL_DIG == __FLT_DECIMAL_DIG__, "");
 _Static_assert(DBL_DECIMAL_DIG == __DBL_DECIMAL_DIG__, "");
 _Static_assert(LDBL_DECIMAL_DIG == __LDBL_DECIMAL_DIG__, "");


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


[PATCH] D58289: [clang] Only provide C11 features in starting with C++17

2019-02-22 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL354691: [clang] Only provide C11 features in  
starting with C++17 (authored by ldionne, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58289?vs=187032&id=187972#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58289/new/

https://reviews.llvm.org/D58289

Files:
  cfe/trunk/lib/Headers/float.h
  cfe/trunk/test/Headers/float.c


Index: cfe/trunk/test/Headers/float.c
===
--- cfe/trunk/test/Headers/float.c
+++ cfe/trunk/test/Headers/float.c
@@ -45,7 +45,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201703L
 #ifndef FLT_DECIMAL_DIG
 #error "Mandatory macro FLT_DECIMAL_DIG is missing."
 #elif   FLT_DECIMAL_DIG < 6
@@ -215,7 +215,7 @@
 _Static_assert(DBL_MANT_DIG == __DBL_MANT_DIG__, "");
 _Static_assert(LDBL_MANT_DIG == __LDBL_MANT_DIG__, "");
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201703L
 _Static_assert(FLT_DECIMAL_DIG == __FLT_DECIMAL_DIG__, "");
 _Static_assert(DBL_DECIMAL_DIG == __DBL_DECIMAL_DIG__, "");
 _Static_assert(LDBL_DECIMAL_DIG == __LDBL_DECIMAL_DIG__, "");
Index: cfe/trunk/lib/Headers/float.h
===
--- cfe/trunk/lib/Headers/float.h
+++ cfe/trunk/lib/Headers/float.h
@@ -78,7 +78,7 @@
 #  undef FLT_MIN
 #  undef DBL_MIN
 #  undef LDBL_MIN
-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201103L
+#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus 
>= 201703L
 #undef FLT_TRUE_MIN
 #undef DBL_TRUE_MIN
 #undef LDBL_TRUE_MIN
@@ -137,7 +137,7 @@
 #define DBL_MIN __DBL_MIN__
 #define LDBL_MIN __LDBL_MIN__
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 
201703L
 #  define FLT_TRUE_MIN __FLT_DENORM_MIN__
 #  define DBL_TRUE_MIN __DBL_DENORM_MIN__
 #  define LDBL_TRUE_MIN __LDBL_DENORM_MIN__


Index: cfe/trunk/test/Headers/float.c
===
--- cfe/trunk/test/Headers/float.c
+++ cfe/trunk/test/Headers/float.c
@@ -45,7 +45,7 @@
 #endif
 
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201703L
 #ifndef FLT_DECIMAL_DIG
 #error "Mandatory macro FLT_DECIMAL_DIG is missing."
 #elif   FLT_DECIMAL_DIG < 6
@@ -215,7 +215,7 @@
 _Static_assert(DBL_MANT_DIG == __DBL_MANT_DIG__, "");
 _Static_assert(LDBL_MANT_DIG == __LDBL_MANT_DIG__, "");
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201703L
 _Static_assert(FLT_DECIMAL_DIG == __FLT_DECIMAL_DIG__, "");
 _Static_assert(DBL_DECIMAL_DIG == __DBL_DECIMAL_DIG__, "");
 _Static_assert(LDBL_DECIMAL_DIG == __LDBL_DECIMAL_DIG__, "");
Index: cfe/trunk/lib/Headers/float.h
===
--- cfe/trunk/lib/Headers/float.h
+++ cfe/trunk/lib/Headers/float.h
@@ -78,7 +78,7 @@
 #  undef FLT_MIN
 #  undef DBL_MIN
 #  undef LDBL_MIN
-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L
+#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201703L
 #undef FLT_TRUE_MIN
 #undef DBL_TRUE_MIN
 #undef LDBL_TRUE_MIN
@@ -137,7 +137,7 @@
 #define DBL_MIN __DBL_MIN__
 #define LDBL_MIN __LDBL_MIN__
 
-#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201703L
 #  define FLT_TRUE_MIN __FLT_DENORM_MIN__
 #  define DBL_TRUE_MIN __DBL_DENORM_MIN__
 #  define LDBL_TRUE_MIN __LDBL_DENORM_MIN__
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

I have a few nitpicks, but otherwise this seems right.  I'll wait for the 
llvm-dev discussion before LGTM'ing though.




Comment at: llvm/lib/AsmParser/LLParser.cpp:2930-2931
+if (NameID != -1 && unsigned(NameID) != NumberedVals.size()) {
+  P.Error(Loc, "label expected to be numbered '%" +
+   Twine(NumberedVals.size()) + "'");
+  return nullptr;

Since the label doesn't include a `%`, I think you should drop that part of the 
error message.



Comment at: llvm/lib/AsmParser/LLParser.cpp:2936-2937
+if (!BB) {
+  P.Error(Loc,
+  "unable to create block numbered " + Twine(NumberedVals.size()));
+  return nullptr;

I think single quotes around the number would be better here.



Comment at: llvm/lib/AsmParser/LLParser.cpp:5580-5581
   switch (Token) {
-  default:return Error(Loc, "expected instruction opcode");
+  default:
+return Error(Loc, "expected instruction opcode");
   // Terminator Instructions.

Looks unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



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


[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain

2019-02-22 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 187979.
gtbercea added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

- Update.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47394/new/

https://reviews.llvm.org/D47394

Files:
  include/clang/Driver/Action.h
  include/clang/Driver/Compilation.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Action.cpp
  lib/Driver/Compilation.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Clang.h
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload-gpu-linux.c
  test/Driver/openmp-offload-gpu.c
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -480,13 +480,13 @@
 // Create host object and bundle.
 // CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-unknown-linux" {{.*}}"-emit-obj" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-BUJOBS-SAME: [[HOSTOBJ:[^\\/]+\.o]]" "-x" "ir" "{{.*}}[[HOSTBC]]"
-// CHK-BUJOBS: clang-offload-bundler{{.*}}" "-type=o" "-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le-unknown-linux" "-outputs=
+// CHK-BUJOBS: clang-offload-bundler{{.*}}" "-type=o"{{.*}}"-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le-unknown-linux" "-outputs=
 // CHK-BUJOBS-SAME: [[RES:[^\\/]+\.o]]" "-inputs={{.*}}[[T1OBJ]],{{.*}}[[T2OBJ]],{{.*}}[[HOSTOBJ]]"
 // CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-unknown-linux" {{.*}}"-S" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-BUJOBS-ST-SAME: [[HOSTASM:[^\\/]+\.s]]" "-x" "ir" "{{.*}}[[HOSTBC]]"
 // CHK-BUJOBS-ST: clang{{.*}}" "-cc1as" "-triple" "powerpc64le-unknown-linux" "-filetype" "obj" {{.*}}"-o" "
 // CHK-BUJOBS-ST-SAME: [[HOSTOBJ:[^\\/]+\.o]]" "{{.*}}[[HOSTASM]]"
-// CHK-BUJOBS-ST: clang-offload-bundler{{.*}}" "-type=o" "-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le-unknown-linux" "-outputs=
+// CHK-BUJOBS-ST: clang-offload-bundler{{.*}}" "-type=o"{{.*}}"-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le-unknown-linux" "-outputs=
 // CHK-BUJOBS-ST-SAME: [[RES:[^\\/]+\.o]]" "-inputs={{.*}}[[T1OBJ]],{{.*}}[[T2OBJ]],{{.*}}[[HOSTOBJ]]"
 
 /// ###
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -77,7 +77,7 @@
 
 /// Check cubin file generation and bundling
 // RUN:   %clang -### -target powerpc64le-unknown-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
-// RUN:  -no-canonical-prefixes -save-temps %s -c 2>&1 \
+// RUN:  -no-canonical-prefixes -save-temps %s -c -fopenmp-use-target-bundling 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-PTXAS-CUBIN-BUNDLING %s
 
 // CHK-PTXAS-CUBIN-BUNDLING: clang{{.*}}" "-o" "[[PTX:.*\.s]]"
@@ -89,7 +89,7 @@
 /// Check cubin file unbundling and usage by nvlink
 // RUN:   touch %t.o
 // RUN:   %clang -### -target powerpc64le-unknown-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
-// RUN:  -no-canonical-prefixes -save-temps %t.o %S/Inputs/in.so 2>&1 \
+// RUN:  -no-canonical-prefixes -save-temps %t.o %S/Inputs/in.so -fopenmp-use-target-bundling 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-CUBIN-UNBUNDLING-NVLINK %s
 
 /// Use DAG to ensure that cubin file has been unbundled.
@@ -105,11 +105,11 @@
 // RUN:   touch %t1.o
 // RUN:   touch %t2.o
 // RUN:   %clang -### -no-canonical-prefixes -target powerpc64le-unknown-linux-gnu -fopenmp=libomp \
-// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o 2>&1 \
+// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o -fopenmp-use-target-bundling 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-TWOCUBIN %s
 /// Check cubin file generation and usage by nvlink when toolchain has BindArchAction
 // RUN:   %clang -### -no-canonical-prefixes -target x86_64-apple-darwin17.0.0 -fopenmp=libomp \
-// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o 2>&1 \
+// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o -fopenmp-use-target-bundling 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-TWOCUBIN %s
 
 // CHK-TWOCUBIN: nvlink{{.*}}openmp-offload-{{.*}}.cubin" "{{.*}}openmp-offload-{{.*}}.cubin"
Index: test/Driver/openmp-offload-gpu-linux.c
===
--- /dev/null
+++ test/Driver/openmp-offload-gpu-linux.c
@@ -0,0 +1,52 @@
+///
+/// Perform driver tests for OpenMP offloading on Linux systems
+///
+
+// UNSUPPORTED: system-windows
+
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: powerpc-registered-t

[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain

2019-02-22 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

ping


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47394/new/

https://reviews.llvm.org/D47394



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


[PATCH] D58518: [HIP] change kernel stub name

2019-02-22 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 187980.
yaxunl added a comment.

Fixed regressions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58518/new/

https://reviews.llvm.org/D58518

Files:
  lib/CodeGen/CGCUDANV.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCUDA/kernel-stub-name.cu


Index: test/CodeGenCUDA/kernel-stub-name.cu
===
--- /dev/null
+++ test/CodeGenCUDA/kernel-stub-name.cu
@@ -0,0 +1,20 @@
+// RUN: echo "GPU binary would be here" > %t
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
+// RUN: -fcuda-include-gpubinary %t -o - -x hip\
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=CHECK
+
+#include "Inputs/cuda.h"
+
+template
+__global__ void kernelfunc() {}
+
+// CHECK-LABEL: define{{.*}}@_Z8hostfuncv()
+// CHECK: call void @[[STUB:_Z10kernelfuncIiEvv.stub]]()
+void hostfunc(void) { kernelfunc<<<1, 1>>>(); }
+
+// CHECK: define{{.*}}@[[STUB]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[STUB]]
+
+// CHECK-LABEL: define{{.*}}@__hip_register_globals
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[STUB]]
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1048,8 +1048,17 @@
 
   // Keep the first result in the case of a mangling collision.
   const auto *ND = cast(GD.getDecl());
-  auto Result =
-  Manglings.insert(std::make_pair(getMangledNameImpl(*this, GD, ND), GD));
+  std::string MangledName = getMangledNameImpl(*this, GD, ND);
+
+  // Postfix kernel stub names with .stub to differentiate them from kernel
+  // names in device binaries. This is to facilitate the debugger to find
+  // the correct symbols for kernels in the device binary.
+  if (auto *FD = dyn_cast(GD.getDecl()))
+if (getLangOpts().HIP && !getLangOpts().CUDAIsDevice &&
+FD->hasAttr())
+  MangledName = MangledName + ".stub";
+
+  auto Result = Manglings.insert(std::make_pair(MangledName, GD));
   return MangledDeclNames[CanonicalGD] = Result.first->first();
 }
 
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -218,6 +218,7 @@
 void CGNVCUDARuntime::emitDeviceStub(CodeGenFunction &CGF,
  FunctionArgList &Args) {
   assert(getDeviceSideName(CGF.CurFuncDecl) == CGF.CurFn->getName() ||
+ getDeviceSideName(CGF.CurFuncDecl) + ".stub" == CGF.CurFn->getName() 
||
  CGF.CGM.getContext().getTargetInfo().getCXXABI() !=
  CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI());
 


Index: test/CodeGenCUDA/kernel-stub-name.cu
===
--- /dev/null
+++ test/CodeGenCUDA/kernel-stub-name.cu
@@ -0,0 +1,20 @@
+// RUN: echo "GPU binary would be here" > %t
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
+// RUN: -fcuda-include-gpubinary %t -o - -x hip\
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=CHECK
+
+#include "Inputs/cuda.h"
+
+template
+__global__ void kernelfunc() {}
+
+// CHECK-LABEL: define{{.*}}@_Z8hostfuncv()
+// CHECK: call void @[[STUB:_Z10kernelfuncIiEvv.stub]]()
+void hostfunc(void) { kernelfunc<<<1, 1>>>(); }
+
+// CHECK: define{{.*}}@[[STUB]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[STUB]]
+
+// CHECK-LABEL: define{{.*}}@__hip_register_globals
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[STUB]]
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1048,8 +1048,17 @@
 
   // Keep the first result in the case of a mangling collision.
   const auto *ND = cast(GD.getDecl());
-  auto Result =
-  Manglings.insert(std::make_pair(getMangledNameImpl(*this, GD, ND), GD));
+  std::string MangledName = getMangledNameImpl(*this, GD, ND);
+
+  // Postfix kernel stub names with .stub to differentiate them from kernel
+  // names in device binaries. This is to facilitate the debugger to find
+  // the correct symbols for kernels in the device binary.
+  if (auto *FD = dyn_cast(GD.getDecl()))
+if (getLangOpts().HIP && !getLangOpts().CUDAIsDevice &&
+FD->hasAttr())
+  MangledName = MangledName + ".stub";
+
+  auto Result = Manglings.insert(std::make_pair(MangledName, GD));
   return MangledDeclNames[CanonicalGD] = Result.first->first();
 }
 
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -218,6 +218,7 @@
 void CGNVCUDARuntime::emitDeviceStub(CodeGenFunction &CGF,
  FunctionArgList &Args) {
   assert(getDeviceSideName(CGF.CurFuncDecl) == CGF.CurFn->getName() ||
+ getDeviceSideName(CGF.CurFuncDecl) + ".stu

[PATCH] D56924: Handle ObjCCategoryDecl class extensions for print

2019-02-22 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment.

ping, looking to get this in to fix a clangd assertion failure


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56924/new/

https://reviews.llvm.org/D56924



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


[PATCH] D58518: [HIP] change kernel stub name

2019-02-22 Thread Artem Belevich via Phabricator via cfe-commits
tra requested changes to this revision.
tra added a subscriber: echristo.
tra added inline comments.
This revision now requires changes to proceed.



Comment at: lib/CodeGen/CodeGenModule.cpp:1059
+FD->hasAttr())
+  MangledName = MangledName + ".stub";
+

Changing mangled name exposes this change to a wider scope of potential issues. 
Is the mangled name still valid after this change? I.e. will external 
demanglers have problem with it? Is `.` a valid symbol in mangled names on all 
platforms we support?

I think changing the name here is way too late and we should figure out how to 
change the stub name when we generate it.

@echristo Eric, what do you think?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58518/new/

https://reviews.llvm.org/D58518



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


r354698 - [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier.

2019-02-22 Thread Michael Kruse via cfe-commits
Author: meinersbur
Date: Fri Feb 22 14:29:42 2019
New Revision: 354698

URL: http://llvm.org/viewvc/llvm-project?rev=354698&view=rev
Log:
[OpenMP 5.0] Parsing/sema support for to clause with mapper modifier.

This patch implements the parsing and sema support for OpenMP to clause
with potential user-defined mappers attached. User defined mapper is a
new feature in OpenMP 5.0. A to/from clause can have an explicit or
implicit associated mapper, which instructs the compiler to generate and
use customized mapping functions. An example is shown below:

struct S { int len; int *d; };
#pragma omp declare mapper(id: struct S s) map(s, s.d[0:s.len])
struct S ss;
#pragma omp target update to(mapper(id): ss) // use the mapper with name 
'id' to map ss to device

Contributed-by: 

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

Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/include/clang/Basic/OpenMPKinds.h
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/OpenMP/declare_mapper_ast_print.c
cfe/trunk/test/OpenMP/declare_mapper_ast_print.cpp
cfe/trunk/test/OpenMP/declare_mapper_codegen.cpp
cfe/trunk/test/OpenMP/declare_mapper_messages.c
cfe/trunk/test/OpenMP/declare_mapper_messages.cpp

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=354698&r1=354697&r2=354698&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Fri Feb 22 14:29:42 2019
@@ -158,8 +158,11 @@ public:
 
 /// This structure contains most locations needed for by an OMPVarListClause.
 struct OMPVarListLocTy {
+  /// Starting location of the clause (the clause keyword).
   SourceLocation StartLoc;
+  /// Location of '('.
   SourceLocation LParenLoc;
+  /// Ending location of the clause.
   SourceLocation EndLoc;
   OMPVarListLocTy() = default;
   OMPVarListLocTy(SourceLocation StartLoc, SourceLocation LParenLoc,
@@ -3609,9 +3612,13 @@ protected:
 /// This structure contains all sizes needed for by an
 /// OMPMappableExprListClause.
 struct OMPMappableExprListSizeTy {
+  /// Number of expressions listed.
   unsigned NumVars;
+  /// Number of unique base declarations.
   unsigned NumUniqueDeclarations;
+  /// Number of component lists.
   unsigned NumComponentLists;
+  /// Total number of expression components.
   unsigned NumComponents;
   OMPMappableExprListSizeTy() = default;
   OMPMappableExprListSizeTy(unsigned NumVars, unsigned NumUniqueDeclarations,
@@ -4969,6 +4976,10 @@ class OMPToClause final : public OMPMapp
 
   /// Build clause with number of variables \a NumVars.
   ///
+  /// \param MapperQualifierLoc C++ nested name specifier for the associated
+  /// user-defined mapper.
+  /// \param MapperIdInfo The identifier of associated user-defined mapper.
+  /// \param MapType Map type.
   /// \param Locs Locations needed to build a mappable clause. It includes 1)
   /// StartLoc: starting location of the clause (the clause keyword); 2)
   /// LParenLoc: location of '('; 3) EndLoc: ending location of the clause.
@@ -4977,9 +4988,12 @@ class OMPToClause final : public OMPMapp
   /// NumUniqueDeclarations: number of unique base declarations in this clause;
   /// 3) NumComponentLists: number of component lists in this clause; and 4)
   /// NumComponents: total number of expression components in the clause.
-  explicit OMPToClause(const OMPVarListLocTy &Locs,
+  explicit OMPToClause(NestedNameSpecifierLoc MapperQualifierLoc,
+   DeclarationNameInfo MapperIdInfo,
+   const OMPVarListLocTy &Locs,
const OMPMappableExprListSizeTy &Sizes)
-  : OMPMappableExprListClause(OMPC_to, Locs, Sizes) {}
+  : OMPMappableExprListClause(OMPC_to, Locs, Sizes, &MapperQualifierLoc,
+  &MapperIdInfo) {}
 
   /// Build an empty clause.
   ///
@@ -4994,7 +5008,9 @@ class OMPToClause final : public OMPMapp
   /// Define the sizes of each trailing object array except the last one. This
   /// is required for TrailingObjects to work properly.
   size_t numTrailingObjects(OverloadToken) const {
-return varlist_size();
+// There are varlist_size() of expressions, and varlist_size() of
+// user-defined mappers.
+return 2 * varlist_size();
   }
   size_t numTrailingObjects(OverloadToken) const {
 return getUniqueDeclarationsNum();
@

[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

2019-02-22 Thread Michael Kruse via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL354698: [OpenMP 5.0] Parsing/sema support for to clause with 
mapper modifier. (authored by Meinersbur, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58523?vs=187970&id=187988#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58523/new/

https://reviews.llvm.org/D58523

Files:
  cfe/trunk/include/clang/AST/OpenMPClause.h
  cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
  cfe/trunk/include/clang/Basic/OpenMPKinds.def
  cfe/trunk/include/clang/Basic/OpenMPKinds.h
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/AST/OpenMPClause.cpp
  cfe/trunk/lib/Basic/OpenMPKinds.cpp
  cfe/trunk/lib/Parse/ParseOpenMP.cpp
  cfe/trunk/lib/Sema/SemaOpenMP.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/test/OpenMP/declare_mapper_ast_print.c
  cfe/trunk/test/OpenMP/declare_mapper_ast_print.cpp
  cfe/trunk/test/OpenMP/declare_mapper_codegen.cpp
  cfe/trunk/test/OpenMP/declare_mapper_messages.c
  cfe/trunk/test/OpenMP/declare_mapper_messages.cpp

Index: cfe/trunk/lib/AST/OpenMPClause.cpp
===
--- cfe/trunk/lib/AST/OpenMPClause.cpp
+++ cfe/trunk/lib/AST/OpenMPClause.cpp
@@ -845,11 +845,11 @@
   return new (Mem) OMPMapClause(Sizes);
 }
 
-OMPToClause *OMPToClause::Create(const ASTContext &C,
- const OMPVarListLocTy &Locs,
- ArrayRef Vars,
- ArrayRef Declarations,
- MappableExprComponentListsRef ComponentLists) {
+OMPToClause *OMPToClause::Create(
+const ASTContext &C, const OMPVarListLocTy &Locs, ArrayRef Vars,
+ArrayRef Declarations,
+MappableExprComponentListsRef ComponentLists, ArrayRef UDMapperRefs,
+NestedNameSpecifierLoc UDMQualifierLoc, DeclarationNameInfo MapperId) {
   OMPMappableExprListSizeTy Sizes;
   Sizes.NumVars = Vars.size();
   Sizes.NumUniqueDeclarations = getUniqueDeclarationsTotalNumber(Declarations);
@@ -857,8 +857,8 @@
   Sizes.NumComponents = getComponentsTotalNumber(ComponentLists);
 
   // We need to allocate:
-  // NumVars x Expr* - we have an original list expression for each clause list
-  // entry.
+  // 2 x NumVars x Expr* - we have an original list expression and an associated
+  // user-defined mapper for each clause list entry.
   // NumUniqueDeclarations x ValueDecl* - unique base declarations associated
   // with each component list.
   // (NumUniqueDeclarations + NumComponentLists) x unsigned - we specify the
@@ -869,13 +869,14 @@
   void *Mem = C.Allocate(
   totalSizeToAlloc(
-  Sizes.NumVars, Sizes.NumUniqueDeclarations,
+  2 * Sizes.NumVars, Sizes.NumUniqueDeclarations,
   Sizes.NumUniqueDeclarations + Sizes.NumComponentLists,
   Sizes.NumComponents));
 
-  OMPToClause *Clause = new (Mem) OMPToClause(Locs, Sizes);
+  auto *Clause = new (Mem) OMPToClause(UDMQualifierLoc, MapperId, Locs, Sizes);
 
   Clause->setVarRefs(Vars);
+  Clause->setUDMapperRefs(UDMapperRefs);
   Clause->setClauseInfo(Declarations, ComponentLists);
   return Clause;
 }
@@ -885,7 +886,7 @@
   void *Mem = C.Allocate(
   totalSizeToAlloc(
-  Sizes.NumVars, Sizes.NumUniqueDeclarations,
+  2 * Sizes.NumVars, Sizes.NumUniqueDeclarations,
   Sizes.NumUniqueDeclarations + Sizes.NumComponentLists,
   Sizes.NumComponents));
   return new (Mem) OMPToClause(Sizes);
@@ -1444,7 +1445,19 @@
 void OMPClausePrinter::VisitOMPToClause(OMPToClause *Node) {
   if (!Node->varlist_empty()) {
 OS << "to";
-VisitOMPClauseList(Node, '(');
+DeclarationNameInfo MapperId = Node->getMapperIdInfo();
+if (MapperId.getName() && !MapperId.getName().isEmpty()) {
+  OS << '(';
+  OS << "mapper(";
+  NestedNameSpecifier *MapperNNS =
+  Node->getMapperQualifierLoc().getNestedNameSpecifier();
+  if (MapperNNS)
+MapperNNS->print(OS, Policy);
+  OS << MapperId << "):";
+  VisitOMPClauseList(Node, ' ');
+} else {
+  VisitOMPClauseList(Node, '(');
+}
 OS << ")";
   }
 }
Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp
@@ -9802,7 +9802,8 @@
DepLinMapLoc, ColonLoc, VarList, Locs);
 break;
   case OMPC_to:
-Res = ActOnOpenMPToClause(VarList, Locs);
+Res = ActOnOpenMPToClause(VarList, ReductionOrMapperIdScopeSpec,
+  ReductionOrMapperId, Locs);
 break;
   case OMPC_from:
 Res = ActOnOpenMPFromClause(VarList, Locs);
@@ -13140,17 +13141,23 @@
 static v

[PATCH] D58556: [LibTooling] Add "smart" retrieval of AST-node source to FixIt library

2019-02-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
ymandel added a project: clang.
Herald added a subscriber: jdoerfert.

Introduces variants of `getText` and `getSourceRange` that extract the source 
text of an AST node based on contextual considerations.  This revision 
introduces the relevant functions, but, as a start, only supports statements 
and trailing semicolons.

Some of the new functions manipulate `CharSourceRange`s, rather than 
`SourceRange`s, because they document and dynamically enforce their type.  So, 
this revision also updates the corresponding existing FixIt functions to 
manipulate `CharSourceRange`s.  This change is not strictly necessary, but 
seems like the correct choice, to keep the API self-consistent.

This revision is the first in a series intended to improve the abstractions 
available to users for writing source-to-source transformations.  A full 
discussion of the end goal can be found on the cfe-dev list with subject "[RFC] 
Easier source-to-source transformations with clang tooling".


Repository:
  rC Clang

https://reviews.llvm.org/D58556

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

Index: clang/unittests/Tooling/FixItTest.cpp
===
--- clang/unittests/Tooling/FixItTest.cpp
+++ clang/unittests/Tooling/FixItTest.cpp
@@ -13,6 +13,7 @@
 using namespace clang;
 
 using tooling::fixit::getText;
+using tooling::fixit::getTextAuto;
 using tooling::fixit::createRemoval;
 using tooling::fixit::createReplacement;
 
@@ -27,6 +28,24 @@
   std::function OnCall;
 };
 
+struct IfVisitor : TestVisitor {
+  bool VisitIfStmt(IfStmt* S) {
+OnIfStmt(S, Context);
+return true;
+  }
+
+  std::function OnIfStmt;
+};
+
+struct VarDeclVisitor : TestVisitor {
+  bool VisitVarDecl(VarDecl* Decl) {
+OnVarDecl(Decl, Context);
+return true;
+  }
+
+  std::function OnVarDecl;
+};
+
 std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
   return Loc.printToString(Context->getSourceManager());
 }
@@ -77,6 +96,57 @@
   "void foo(int x, int y) { FOO(x,y) }");
 }
 
+TEST(FixItTest, getTextAuto) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y);", getTextAuto(*CE, *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getTextAuto(*P0, *Context));
+EXPECT_EQ("y", getTextAuto(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { if (true) foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { switch(x) foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { switch(x) case 3: foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { switch(x) default: foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { while (true) foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { do foo(x, y); while (true); }");
+  Visitor.runOver("void foo(int x, int y) { for (;;) foo(x, y); }");
+  Visitor.runOver("void foo(int x, int y) { bar: foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo()", getTextAuto(*CE, *Context));
+  };
+  Visitor.runOver("int foo() { return foo(); }");
+  Visitor.runOver("int foo() { 3 + foo(); return 0; }");
+  Visitor.runOver("bool foo() { if (foo()) true; return true; }");
+  Visitor.runOver("bool foo() { switch(foo()) true; return true; }");
+  Visitor.runOver("bool foo() { while (foo()) true; return true; }");
+  Visitor.runOver("bool foo() { do true; while (foo()); return true; }");
+  Visitor.runOver("void foo() { for (foo();;) true; }");
+  Visitor.runOver("bool foo() { for (;foo();) true; return true; }");
+  Visitor.runOver("void foo() { for (;; foo()) true; }");
+}
+
+TEST(FixItTest, getTextAutoNonStatement) {
+  VarDeclVisitor Visitor;
+  Visitor.OnVarDecl = [](VarDecl *D, ASTContext *Context) {
+EXPECT_EQ(getText(*D, *Context), getTextAuto(*D, *Context));
+  };
+  Visitor.runOver("int foo() { int x = 3; return x; }");
+}
+
+TEST(FixItTest, getTextAutoNonExprStatement) {
+  IfVisitor Visitor;
+  Visitor.OnIfStmt = [](IfStmt *S, ASTContext *Context) {
+EXPECT_EQ(getText(*S, *Context), getTextAuto(*S, *Context));
+  };
+  Visitor.runOver("int foo() { int x = 3; return x; }");
+}
+
 TEST(FixItTest, createRemoval) {
   CallsVisitor Visitor;
 
Index: clang/lib/Tooling/FixIt.cpp
===
--- clang/lib/Tooling/FixIt.cpp
+++ clang/lib/Tooling/FixIt.cpp
@@ -11,6 +11,8 @@
 //
 //===--===//
 #include "clang/Tooling/FixIt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 
 namespace clang {
@@ -18,13 +20,66 @@
 namespace fixit {
 
 namespa

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

2019-02-22 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder updated this revision to Diff 187989.
tmroeder added a comment.

Added more unit tests.

Sorry for the delay; I had to dig into the details to figure out where to put 
these tests and how to structure them. Please let me know if there are better 
ways to do this.

I don't know any way to write a matcher that checks the condition of 
__builtin_choose_expr before and after import, so I added a unit test that 
matches the expr and compares the From and To ChooseExprs directly.

I've confirmed that without the negation in the condition in VisitChooseExpr, 
the new tests cause failures and with the negation, everything is clean.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58292/new/

https://reviews.llvm.org/D58292

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  test/ASTMerge/choose-expr/Inputs/choose.c
  test/ASTMerge/choose-expr/test.c
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -754,6 +754,11 @@
   EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr()));
 }
 
+TEST(Matcher, ChooseExpr) {
+  EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }",
+   chooseExpr()));
+}
+
 TEST(Matcher, GNUNullExpr) {
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,35 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier Verifier;
+
+  // This case tests C code that is not condition-dependent and has a true
+  // condition.
+  testImport(
+"void declToImport() { (void)__builtin_choose_expr(1, 2, 3); }",
+Lang_C, "", Lang_C, Verifier,
+functionDecl(hasDescendant(chooseExpr(;
+
+  ArgVector Args = getExtraArgs();
+  BindableMatcher Matcher =
+  functionTemplateDecl(hasDescendant(chooseExpr()));
+
+  // Don't try to match the template contents if template parsing is delayed.
+  if (std::find(std::begin(Args), std::end(Args),
+"-fdelayed-template-parsing") != Args.end()) {
+Matcher = functionTemplateDecl();
+  }
+
+  // Make sure that uses of (void)__builtin_choose_expr with dependent types in
+  // the condition are handled properly. This test triggers an assertion if the
+  // ASTImporter incorrectly tries to access isConditionTrue() when
+  // isConditionDependent() is true.
+  testImport("template void declToImport() { "
+ "(void)__builtin_choose_expr(N, 1, 0); }",
+ Lang_CXX, "", Lang_CXX, Verifier, Matcher);
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
@@ -1312,6 +1341,27 @@
   ASSERT_EQ(ToTemplated1, ToTemplated);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportChooseExpr) {
+  // This tests the import of isConditionTrue directly to make sure the importer
+  // gets it right.
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+"void declToImport() { (void)__builtin_choose_expr(1, 0, 1); }",
+Lang_C, "", Lang_C);
+
+  auto ToResults = match(chooseExpr().bind("choose"), To->getASTContext());
+  auto FromResults = match(chooseExpr().bind("choose"), From->getASTContext());
+
+  const ChooseExpr *FromChooseExpr =
+  selectFirst("choose", FromResults);
+  ASSERT_TRUE(FromChooseExpr);
+
+  const ChooseExpr* ToChooseExpr = selectFirst("choose", ToResults);
+  ASSERT_TRUE(ToChooseExpr);
+
+  EXPECT_EQ(FromChooseExpr->isConditionTrue(), ToChooseExpr->isConditionTrue());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportFunctionWithBackReferringParameter) {
   Decl *From, *To;
Index: test/ASTMerge/choose-expr/test.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: test/ASTMerge/choose-expr/Inputs/choose.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/Inputs/choose.c
@@ -0,0 +1,2 @@
+_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr");
+_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr");
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

2019-02-22 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder updated this revision to Diff 187990.
tmroeder added a comment.

Fixed a minor style typo.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58292/new/

https://reviews.llvm.org/D58292

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  test/ASTMerge/choose-expr/Inputs/choose.c
  test/ASTMerge/choose-expr/test.c
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -754,6 +754,11 @@
   EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr()));
 }
 
+TEST(Matcher, ChooseExpr) {
+  EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }",
+   chooseExpr()));
+}
+
 TEST(Matcher, GNUNullExpr) {
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,35 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier Verifier;
+
+  // This case tests C code that is not condition-dependent and has a true
+  // condition.
+  testImport(
+"void declToImport() { (void)__builtin_choose_expr(1, 2, 3); }",
+Lang_C, "", Lang_C, Verifier,
+functionDecl(hasDescendant(chooseExpr(;
+
+  ArgVector Args = getExtraArgs();
+  BindableMatcher Matcher =
+  functionTemplateDecl(hasDescendant(chooseExpr()));
+
+  // Don't try to match the template contents if template parsing is delayed.
+  if (std::find(std::begin(Args), std::end(Args),
+"-fdelayed-template-parsing") != Args.end()) {
+Matcher = functionTemplateDecl();
+  }
+
+  // Make sure that uses of (void)__builtin_choose_expr with dependent types in
+  // the condition are handled properly. This test triggers an assertion if the
+  // ASTImporter incorrectly tries to access isConditionTrue() when
+  // isConditionDependent() is true.
+  testImport("template void declToImport() { "
+ "(void)__builtin_choose_expr(N, 1, 0); }",
+ Lang_CXX, "", Lang_CXX, Verifier, Matcher);
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
@@ -1312,6 +1341,27 @@
   ASSERT_EQ(ToTemplated1, ToTemplated);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportChooseExpr) {
+  // This tests the import of isConditionTrue directly to make sure the importer
+  // gets it right.
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+"void declToImport() { (void)__builtin_choose_expr(1, 0, 1); }",
+Lang_C, "", Lang_C);
+
+  auto ToResults = match(chooseExpr().bind("choose"), To->getASTContext());
+  auto FromResults = match(chooseExpr().bind("choose"), From->getASTContext());
+
+  const ChooseExpr *FromChooseExpr =
+  selectFirst("choose", FromResults);
+  ASSERT_TRUE(FromChooseExpr);
+
+  const ChooseExpr *ToChooseExpr = selectFirst("choose", ToResults);
+  ASSERT_TRUE(ToChooseExpr);
+
+  EXPECT_EQ(FromChooseExpr->isConditionTrue(), ToChooseExpr->isConditionTrue());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportFunctionWithBackReferringParameter) {
   Decl *From, *To;
Index: test/ASTMerge/choose-expr/test.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: test/ASTMerge/choose-expr/Inputs/choose.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/Inputs/choose.c
@@ -0,0 +1,2 @@
+_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr");
+_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr");
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -147,6 +147,7 @@
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
+  REGISTER_MATCHER(chooseExpr);
   REGISTER_MATCHER(classTemplateDecl);
   REGISTER_MATCHER(classTemplateSpecializationDecl);
   REGISTER_MATCHER(complexType);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 187994.
jyknight marked 4 inline comments as done.
jyknight added a comment.

Minor tweaks per comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/

[PATCH] D58095: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

2019-02-22 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore added a comment.

Thanks for the review!


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58095/new/

https://reviews.llvm.org/D58095



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


[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

2019-02-22 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 187998.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.

Address review comments. Add CodeGen test cases for parentheses expressions.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58514/new/

https://reviews.llvm.org/D58514

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclBase.h
  lib/AST/Decl.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenObjC/arc-block-copy-escape.m
  test/CodeGenObjC/arc-blocks.m
  test/CodeGenObjCXX/arc-blocks.mm
  test/PCH/arc-blocks.mm

Index: test/PCH/arc-blocks.mm
===
--- /dev/null
+++ test/PCH/arc-blocks.mm
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fobjc-arc -fblocks -std=c++1y -emit-pch %s -o %t
+// RUN: %clang_cc1 -fobjc-arc -fblocks -std=c++1y -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+namespace test_block_retain {
+  typedef void (^BlockTy)();
+  void foo1(id);
+
+  inline void initialization(id a) {
+// Call to @llvm.objc.retainBlock isn't needed.
+BlockTy b0 = ^{ foo1(a); };
+b0();
+  }
+
+  inline void assignmentConditional(id a, bool c) {
+BlockTy b0;
+if (c)
+  // @llvm.objc.retainBlock is called since 'b0' is declared in the outer scope.
+  b0 = ^{ foo1(a); };
+b0();
+  }
+}
+
+#else
+
+// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+
+namespace test_block_retain {
+// CHECK-LABEL: define linkonce_odr void @_ZN17test_block_retain14initializationEP11objc_object(
+// CHECK-NOT: call i8* @llvm.objc.retainBlock(
+
+  void test_initialization(id a) {
+initialization(a);
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain26test_assignmentConditionalEP11objc_objectb(
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>, align 8
+// CHECK: %[[V4:.*]] = bitcast <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>* %[[BLOCK]] to void ()*
+// CHECK: %[[V5:.*]] = bitcast void ()* %[[V4]] to i8*
+// CHECK: call i8* @llvm.objc.retainBlock(i8* %[[V5]])
+
+  void test_assignmentConditional(id a, bool c) {
+assignmentConditional(a, c);
+  }
+}
+
+#endif
Index: test/CodeGenObjCXX/arc-blocks.mm
===
--- test/CodeGenObjCXX/arc-blocks.mm
+++ test/CodeGenObjCXX/arc-blocks.mm
@@ -201,3 +201,123 @@
   ^{ (void)t0; (void)t1; (void)t2; (void)t3; (void)t4; (void)t5; };
 }
 }
+
+// Test that calls to @llvm.objc.retainBlock aren't emitted in some cases.
+
+namespace test_block_retain {
+  typedef void (^BlockTy)();
+
+  void foo1(id);
+
+// CHECK-LABEL: define void @_ZN17test_block_retain14initializationEP11objc_object(
+// CHECK-NOT: @llvm.objc.retainBlock(
+  void initialization(id a) {
+BlockTy b0 = ^{ foo1(a); };
+BlockTy b1 = (^{ foo1(a); });
+b0();
+b1();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain20initializationStaticEP11objc_object(
+// CHECK: @llvm.objc.retainBlock(
+  void initializationStatic(id a) {
+static BlockTy b0 = ^{ foo1(a); };
+b0();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain15initialization2EP11objc_object
+// CHECK: %[[B0:.*]] = alloca void ()*, align 8
+// CHECK: %[[B1:.*]] = alloca void ()*, align 8
+// CHECK: load void ()*, void ()** %[[B0]], align 8
+// CHECK-NOT: @llvm.objc.retainBlock
+// CHECK: %[[V9:.*]] = load void ()*, void ()** %[[B0]], align 8
+// CHECK: %[[V10:.*]] = bitcast void ()* %[[V9]] to i8*
+// CHECK: %[[V11:.*]] = call i8* @llvm.objc.retainBlock(i8* %[[V10]])
+// CHECK: %[[V12:.*]] = bitcast i8* %[[V11]] to void ()*
+// CHECK: store void ()* %[[V12]], void ()** %[[B1]], align 8
+  void initialization2(id a) {
+BlockTy b0 = ^{ foo1(a); };
+b0();
+BlockTy b1 = b0; // can't optimize this yet.
+b1();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain10assignmentEP11objc_object(
+// CHECK-NOT: @llvm.objc.retainBlock(
+  void assignment(id a) {
+BlockTy b0;
+b0 = ^{ foo1(a); };
+b0();
+b0 = (^{ foo1(a); });
+b0();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain16assignmentStaticEP11objc_object(
+// CHECK: @llvm.objc.retainBlock(
+  void assignmentStatic(id a) {
+static BlockTy b0;
+b0 = ^{ foo1(a); };
+b0();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain21assignmentConditionalEP11objc_objectb(
+// CHECK: @llvm.objc.retainBlock(
+  void assignmentConditional(id a, bool c) {
+BlockTy b0;
+if (c)
+  // can't optimize this since 'b0' is declared in the outer scope.
+  b0 = ^{ foo1(a); };
+b0();
+  }
+
+// CHECK-LABEL: define void @_ZN17test_block_retain11assignment2EP11objc_object(
+// CHECK: %[[B0:.*]] = alloca void ()*, align 8
+// CHECK: %[[B1:.*]] = alloca void ()*, align 8
+// CHECK-NOT: @llv

[PATCH] D58518: [HIP] change kernel stub name

2019-02-22 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:1059
+FD->hasAttr())
+  MangledName = MangledName + ".stub";
+

tra wrote:
> Changing mangled name exposes this change to a wider scope of potential 
> issues. Is the mangled name still valid after this change? I.e. will external 
> demanglers have problem with it? Is `.` a valid symbol in mangled names on 
> all platforms we support?
> 
> I think changing the name here is way too late and we should figure out how 
> to change the stub name when we generate it.
> 
> @echristo Eric, what do you think?
The external demangler can still demangle this name. e.g. c++filt will demangle 
this name and add [clone .stub] after that.

As far as I can see this function is only called in codegen to map FunctionDecl 
names to LLVM function names. I've tried this change with real ML frameworks 
and it works.

Changing at this place is not too late. The stub function name is requested at 
multiple places in codegen, not just at the emitting of stub function 
definition. For template kernel function, the emitting of stub function 
definition is deferred after emitting of the call of the stub function. 
Basically, codegen needs to find the corresponding LLVM stub function by 
getMangledName first, then by GetOrCreateLLVMFunction. If we do not change 
getMangledName, codegen will not get the correct stub function name 
consistently at all places. That's why the previous patch does not work.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58518/new/

https://reviews.llvm.org/D58518



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


[PATCH] D58559: emit '(assertions enabled)' in the version string for a build of clang with assertions enabled

2019-02-22 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman created this revision.
arphaman added a reviewer: dexonsmith.
Herald added a subscriber: jkorous.
Herald added a project: clang.

This patch adds an additional '(assertions enabled)' in the version string for 
a build of clang with assertions enabled.

This is a useful way to differentiate between clang binaries that have 
assertions enabled and the ones that don't if they are based on the same 
version.


Repository:
  rC Clang

https://reviews.llvm.org/D58559

Files:
  lib/Basic/Version.cpp


Index: lib/Basic/Version.cpp
===
--- lib/Basic/Version.cpp
+++ lib/Basic/Version.cpp
@@ -111,6 +111,11 @@
   OS << LLVMRepo << ' ';
 OS << LLVMRev << ')';
   }
+
+  // Differentiate between the version with and without assertions.
+#ifndef NDEBUG
+  OS << " (assertions enabled)";
+#endif
   return OS.str();
 }
 


Index: lib/Basic/Version.cpp
===
--- lib/Basic/Version.cpp
+++ lib/Basic/Version.cpp
@@ -111,6 +111,11 @@
   OS << LLVMRepo << ' ';
 OS << LLVMRev << ')';
   }
+
+  // Differentiate between the version with and without assertions.
+#ifndef NDEBUG
+  OS << " (assertions enabled)";
+#endif
   return OS.str();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

2019-02-22 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment.

bitcoin:

  sudo apt-get install build-essential libtool autotools-dev automake 
pkg-config libssl-dev libevent-dev bsdmainutils python3 libboost-all-dev 
software-properties-common libminiupnpc-dev libzmq3-dev libqt5gui5 libqt5core5a 
libqt5dbus5 qttools5-dev qttools5-dev-tools libprotobuf-dev protobuf-compiler
  sudo add-apt-repository ppa:bitcoin/bitcoin
  sudo apt-get update
  sudo apt-get install libdb4.8-dev libdb4.8++-dev
  
  git clone https://github.com/bitcoin/bitcoin.git
  cd bitcoin
  
  ./autogen.sh
  ./configure
  CodeChecker log -b "make -j13" -o compile_commands.json

xerces:

  wget http://www-eu.apache.org/dist//xerces/c/3/sources/xerces-c-3.2.2.tar.gz
  tar xf xerces-c-3.2.2.tar.gz
  mv xerces-c-3.2.2 xerces
  rm xerces-c-3.2.2.tar.gz
  cd xerces
  
  ./configure
  CodeChecker log -b "make -j13" -o compile_commands.json

where `-j13` means 13 threads.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50488/new/

https://reviews.llvm.org/D50488



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


[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

2019-02-22 Thread Mandeep Singh Grang via Phabricator via cfe-commits
mgrang added a comment.

In D50488#1407695 , @Charusso wrote:

> bitcoin:
>
>   sudo apt-get install build-essential libtool autotools-dev automake 
> pkg-config libssl-dev libevent-dev bsdmainutils python3 libboost-all-dev 
> software-properties-common libminiupnpc-dev libzmq3-dev libqt5gui5 
> libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools libprotobuf-dev 
> protobuf-compiler
>   sudo add-apt-repository ppa:bitcoin/bitcoin
>   sudo apt-get update
>   sudo apt-get install libdb4.8-dev libdb4.8++-dev
>  
>   git clone https://github.com/bitcoin/bitcoin.git
>   cd bitcoin
>  
>   ./autogen.sh
>   ./configure
>   CodeChecker log -b "make -j13" -o compile_commands.json
>
>
> xerces:
>
>   wget http://www-eu.apache.org/dist//xerces/c/3/sources/xerces-c-3.2.2.tar.gz
>   tar xf xerces-c-3.2.2.tar.gz
>   mv xerces-c-3.2.2 xerces
>   rm xerces-c-3.2.2.tar.gz
>   cd xerces
>  
>   ./configure
>   CodeChecker log -b "make -j13" -o compile_commands.json
>
>
> where `-j13` means 13 threads.


Thanks @Charusso . I have already tried these steps.

By running just ./configure I get this error:

  configure: error: cannot figure out how to use std::atomic

Instead, this is the configure command I use for bitcoin:

  ./configure --disable-wallet CFLAGS='-target x86_64-linux-gnu -L 
/usr/lib/x86_64-linux-gnu -B /usr/lib/x86_64-linux-gnu -B 
/usr/lib/gcc/x86_64-linux-gnu/5 -L /usr/lib/gcc/x86_64-linux-gnu/5 -I 
/usr/include/c++/5 -I /usr/include/x86_64-linux-gnu/c++/5' CXXFLAGS='-target 
x86_64-linux-gnu -L /usr/lib/x86_64-linux-gnu -B /usr/lib/x86_64-linux-gnu -B 
/usr/lib/gcc/x86_64-linux-gnu/5 -L /usr/lib/gcc/x86_64-linux-gnu/5 -I 
/usr/include/c++/5 -I /usr/include/x86_64-linux-gnu/c++/5'

And then I hit this:

  configure: error: Could not link against boost_filesystem !


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50488/new/

https://reviews.llvm.org/D50488



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


[PATCH] D58559: emit '(assertions enabled)' in the version string for a build of clang with assertions enabled

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

I like this.  Can you start a discussion on cfe-dev (if you haven't already)?  
This is user-visible so I want to be sure other vendors are happy with this; if 
not, we can hide it behind a CMake flag.




Comment at: lib/Basic/Version.cpp:117
+#ifndef NDEBUG
+  OS << " (assertions enabled)";
+#endif

Can we just make this `" (+asserts)"` or `" (-NDEBUG)"`?  If we wanted to 
generalize this, I'd expect to want a list of compiler features similar to CPU 
features.



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58559/new/

https://reviews.llvm.org/D58559



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


LLVM buildmaster will be updated and restarted soon

2019-02-22 Thread Galina Kistanova via cfe-commits
 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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


[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-22 Thread Jan Korous via Phabricator via cfe-commits
jkorous marked 4 inline comments as done.
jkorous added inline comments.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:116
+
+  DirectoryWatcher::EventKind K = DirectoryWatcher::EventKind::Added;
+  if (ievt->mask & IN_MODIFY) {

mgorny wrote:
> I don't think I understand this assumption. Have you any specific event in 
> mind that is supposed to be treated as added here? If not, maybe it'd be 
> better to have no default and instead assert that one of the conditions below 
> matched.
SGTM. Will rewrite to Optional<> and assert.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135
+if (!statusOpt.hasValue())
+  K = DirectoryWatcher::EventKind::Removed;
+  }

mgorny wrote:
> Why? I suppose this deserves a comment.
I'll add this comment:

// The file might have been removed just after we received the event.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:154
+errorMsg += llvm::sys::StrError();
+return true;
+  };

mgorny wrote:
> The return values seem to be confusing. Any reason not to use `true` for 
> success?
It seems there's no real consensus * about error/success mapping to bool and 
since this is just implementation detail (not a public interface) I think it's 
fine. `llvm::Error` has better semantics but seems a bit heavyweight for this.

* Just tried this: grep -nr "returns true" clang | grep -i error



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher.cpp:95
+
+#if !defined(__has_include)
+#define __has_include(x) 0

mgorny wrote:
> Why not use CMake checks? You're already partially using them for frameworks.
Do I understand correctly that you suggest doing the check in CMake and 
propagating the result to preprocessor?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58418/new/

https://reviews.llvm.org/D58418



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


[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-22 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 188021.
jyu2 added a comment.

Rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56571/new/

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/asm-goto.cpp
  test/CodeGen/asm-goto.c
  test/CodeGen/asm.c
  test/CodeGen/inline-asm-mixed-style.c
  test/Coverage/c-language-features.inc
  test/PCH/asm.h
  test/Parser/asm.c
  test/Parser/asm.cpp
  test/Sema/asm-goto.cpp
  test/Sema/asm.c
  test/Sema/inline-asm-validate-tmpl.cpp
  test/Sema/scope-check.c

Index: test/Sema/scope-check.c
===
--- test/Sema/scope-check.c
+++ test/Sema/scope-check.c
@@ -232,3 +232,19 @@
 
 // rdar://9024687
 int test16(int [sizeof &&z]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+//Asm goto:
+int test16(int n)
+{
+  // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+2 {{jump bypasses initialization of variable length array}}
+  // expected-note@+1 {{possible target of asm goto statement}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+// expected-note@+1 {{possible target of asm goto statement}}
+loop:
+  return 0;
+}
Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,24 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  asm goto ("jne %l0"lab);
+}
Index: test/Sema/asm-goto.cpp
===
--- /dev/null
+++ test/Sema/asm-goto.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
+
+struct NonTrivial {
+  ~NonTrivial();
+  int f(int);
+private:
+  int k;
+};
+void JumpDiagnostics(int n) {
+// expected-error@+1 {{cannot jump from this goto statement to its label}}
+  goto DirectJump;
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp1;
+
+DirectJump:
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" Later);
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp2;
+// expected-note@+1 {{possible target of asm goto statement}}
+Later:
+  return;
+}
+
+struct S { ~S(); };
+void foo(int a) {
+  if (a) {
+FOO:
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+S s;
+void *p = &&BAR;
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" BAR);
+// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
+goto *p;
+p = &&FOO;
+goto *p;
+re

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:154
+errorMsg += llvm::sys::StrError();
+return true;
+  };

jkorous wrote:
> mgorny wrote:
> > The return values seem to be confusing. Any reason not to use `true` for 
> > success?
> It seems there's no real consensus * about error/success mapping to bool and 
> since this is just implementation detail (not a public interface) I think 
> it's fine. `llvm::Error` has better semantics but seems a bit heavyweight for 
> this.
> 
> * Just tried this: grep -nr "returns true" clang | grep -i error
Agreed that much of LLVM/Clang uses the C convention where `false`/`0` is 
success.

However, I'm curious whether it might be better to thread `llvm::Error` (and/or 
`llvm::Expected`) through, since you bring it up.  They provide nice assertions 
when the caller forgets to check the result, they're cheap on success, and 
there's no true/false ambiguity.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:94
+   std::shared_ptr evtQueue) {
+#define EVT_BUF_LEN (30 * (sizeof(struct inotify_event) + NAME_MAX + 1))
+  char buf[EVT_BUF_LEN] __attribute__((aligned(8)));

Can this be `constexpr int EventBufferLength` instead of `#define EVT_BUF_LEN`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58418/new/

https://reviews.llvm.org/D58418



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


[PATCH] D58559: emit '(assertions enabled)' in the version string for a build of clang with assertions enabled

2019-02-22 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 188023.
arphaman marked an inline comment as done.
arphaman added a comment.

Use `"(+asserts)"` as suggested.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58559/new/

https://reviews.llvm.org/D58559

Files:
  lib/Basic/Version.cpp


Index: lib/Basic/Version.cpp
===
--- lib/Basic/Version.cpp
+++ lib/Basic/Version.cpp
@@ -111,6 +111,11 @@
   OS << LLVMRepo << ' ';
 OS << LLVMRev << ')';
   }
+
+  // Differentiate between the version with and without assertions.
+#ifndef NDEBUG
+  OS << " (+asserts)";
+#endif
   return OS.str();
 }
 


Index: lib/Basic/Version.cpp
===
--- lib/Basic/Version.cpp
+++ lib/Basic/Version.cpp
@@ -111,6 +111,11 @@
   OS << LLVMRepo << ' ';
 OS << LLVMRev << ')';
   }
+
+  // Differentiate between the version with and without assertions.
+#ifndef NDEBUG
+  OS << " (+asserts)";
+#endif
   return OS.str();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D56924: Handle ObjCCategoryDecl class extensions for print

2019-02-22 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

Please add a test that covers the '(class extension)' output as well.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56924/new/

https://reviews.llvm.org/D56924



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


[PATCH] D58559: emit '(assertions enabled)' in the version string for a build of clang with assertions enabled

2019-02-22 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

In D58559#1407742 , @dexonsmith wrote:

> I like this.  Can you start a discussion on cfe-dev (if you haven't already)? 
>  This is user-visible so I want to be sure other vendors are happy with this; 
> if not, we can hide it behind a CMake flag.


Sure, I started a cfe-dev thread.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58559/new/

https://reviews.llvm.org/D58559



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


[PATCH] D58569: [libclang] Avoid crashing when getting layout info of an undeduced type.

2019-02-22 Thread Emilio Cobos Álvarez via Phabricator via cfe-commits
emilio created this revision.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

When the type is not deducible, return an error instead of crashing.

This fixes https://bugs.llvm.org/show_bug.cgi?id=40813.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58569

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/print-type-size.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -891,6 +891,9 @@
 return CXTypeLayoutError_Incomplete;
   if (QT->isDependentType())
 return CXTypeLayoutError_Dependent;
+  if (const auto *Deduced = dyn_cast(QT))
+if (Deduced->getDeducedType().isNull())
+  return CXTypeLayoutError_Undeduced;
   // Exceptions by GCC extension - see ASTContext.cpp:1313 getTypeInfoImpl
   // if (QT->isFunctionType()) return 4; // Bug #15511 - should be 1
   // if (QT->isVoidType()) return 1;
@@ -928,6 +931,9 @@
 return CXTypeLayoutError_Dependent;
   if (!QT->isConstantSizeType())
 return CXTypeLayoutError_NotConstantSize;
+  if (const auto *Deduced = dyn_cast(QT))
+if (Deduced->getDeducedType().isNull())
+  return CXTypeLayoutError_Undeduced;
   // [gcc extension] lib/AST/ExprConstant.cpp:1372
   // HandleSizeof : {voidtype,functype} == 1
   // not handled by ASTContext.cpp:1313 getTypeInfoImpl
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -1670,29 +1670,44 @@
   return CXChildVisit_Recurse;
 }
 
-static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
- CXClientData d) {
-  CXType T;
-  enum CXCursorKind K = clang_getCursorKind(cursor);
-  if (clang_isInvalid(K))
-return CXChildVisit_Recurse;
-  T = clang_getCursorType(cursor);
-  PrintCursor(cursor, NULL);
-  PrintTypeAndTypeKind(T, " [type=%s] [typekind=%s]");
+static void PrintSingleTypeSize(CXType T, const char *TypeKindFormat,
+const char *SizeFormat,
+const char *AlignFormat) {
+  PrintTypeAndTypeKind(T, TypeKindFormat);
   /* Print the type sizeof if applicable. */
   {
 long long Size = clang_Type_getSizeOf(T);
 if (Size >= 0 || Size < -1 ) {
-  printf(" [sizeof=%lld]", Size);
+  printf(SizeFormat, Size);
 }
   }
   /* Print the type alignof if applicable. */
   {
 long long Align = clang_Type_getAlignOf(T);
 if (Align >= 0 || Align < -1) {
-  printf(" [alignof=%lld]", Align);
+  printf(AlignFormat, Align);
 }
   }
+
+  /* Print the return type if it exists. */
+  {
+CXType RT = clang_getResultType(T);
+if (RT.kind != CXType_Invalid)
+  PrintSingleTypeSize(RT, " [resulttype=%s] [resulttypekind=%s]",
+  " [resultsizeof=%lld]", " [resultalignof=%lld]");
+  }
+}
+
+static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
+ CXClientData d) {
+  CXType T;
+  enum CXCursorKind K = clang_getCursorKind(cursor);
+  if (clang_isInvalid(K))
+return CXChildVisit_Recurse;
+  T = clang_getCursorType(cursor);
+  PrintCursor(cursor, NULL);
+  PrintSingleTypeSize(T, " [type=%s] [typekind=%s]", " [sizeof=%lld]",
+  " [alignof=%lld]");
   /* Print the record field offset if applicable. */
   {
 CXString FieldSpelling = clang_getCursorSpelling(cursor);
@@ -1730,7 +1745,9 @@
 if (IsBitfield)
   printf(" [BitFieldSize=%d]", clang_getFieldDeclBitWidth(cursor));
   }
+
   printf("\n");
+
   return CXChildVisit_Recurse;
 }
 
Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -391,6 +391,10 @@
 Foo t1;
 Foo t2;
 
+class BrowsingContext {
+  auto Tie(void*) const;
+};
+
 void c;
 
 plopplop;
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -3839,7 +3839,11 @@
   /**
* The Field name is not valid for this record.
*/
-  CXTypeLayoutError_InvalidFieldName = -5
+  CXTypeLayoutError_InvalidFieldName = -5,
+  /**
+   * The type is undeduced.
+   */
+  CXTypeLayoutError_Undeduced = -6
 };
 
 /**
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58570: [libclang] Expose warn_unused and warn_unused_result attributes.

2019-02-22 Thread Emilio Cobos Álvarez via Phabricator via cfe-commits
emilio created this revision.
emilio added reviewers: arphaman, Anastasia.
Herald added subscribers: cfe-commits, JDevlieghere.
Herald added a project: clang.

This is helpful to properly detect them, and fixing issues like
https://github.com/rust-lang/rust-bindgen/issues/1518.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58570

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


Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -79,6 +79,8 @@
 case attr::ObjCBoxable: return CXCursor_ObjCBoxable;
 case attr::FlagEnum: return CXCursor_FlagEnum;
 case attr::Convergent: return CXCursor_ConvergentAttr;
+case attr::WarnUnused: return CXCursor_WarnUnusedAttr;
+case attr::WarnUnusedResult: return CXCursor_WarnUnusedResultAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5477,6 +5477,10 @@
   return cxstring::createRef("FriendDecl");
   case CXCursor_ConvergentAttr:
   return cxstring::createRef("attribute(convergent)");
+  case CXCursor_WarnUnusedAttr:
+  return cxstring::createRef("attribute(warn_unused)");
+  case CXCursor_WarnUnusedResultAttr:
+  return cxstring::createRef("attribute(warn_unused_result)");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");
Index: clang/test/Index/attributes.c
===
--- clang/test/Index/attributes.c
+++ clang/test/Index/attributes.c
@@ -14,6 +14,12 @@
 
 void convergent_fn() __attribute__((convergent));
 
+int warn_unused_result_fn() __attribute__((warn_unused_result));
+
+struct __attribute__((warn_unused)) WarnUnused {
+  int b;
+};
+
 // CHECK: attributes.c:3:32: StructDecl=Test2:3:32 (Definition) Extent=[3:1 - 
5:2]
 // CHECK: attributes.c:3:23: attribute(packed)=packed Extent=[3:23 - 3:29]
 // CHECK: attributes.c:4:8: FieldDecl=a:4:8 (Definition) Extent=[4:3 - 4:9] 
[access=public]
@@ -29,3 +35,7 @@
 // CHECK: attributes.c:12:3: EnumConstantDecl=Foo:12:3 (Definition) 
Extent=[12:3 - 12:6]
 // CHECK: attributes.c:15:6: FunctionDecl=convergent_fn:15:6 Extent=[15:1 - 
15:49]
 // CHECK: attributes.c:15:37: attribute(convergent)= Extent=[15:37 - 15:47]
+// CHECK: attributes.c:17:5: FunctionDecl=warn_unused_result_fn:17:5 
Extent=[17:1 - 17:64]
+// CHECK: attributes.c:17:44: attribute(warn_unused_result)= Extent=[17:44 - 
17:62]
+// CHECK: attributes.c:19:37: StructDecl=WarnUnused:19:37 (Definition) 
Extent=[19:1 - 21:2]
+// CHECK: attributes.c:19:23: attribute(warn_unused)= Extent=[19:23 - 19:34]
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 51
+#define CINDEX_VERSION_MINOR 52
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -2587,7 +2587,9 @@
   CXCursor_ObjCBoxable   = 436,
   CXCursor_FlagEnum  = 437,
   CXCursor_ConvergentAttr= 438,
-  CXCursor_LastAttr  = CXCursor_ConvergentAttr,
+  CXCursor_WarnUnusedAttr= 439,
+  CXCursor_WarnUnusedResultAttr  = 440,
+  CXCursor_LastAttr  = CXCursor_WarnUnusedResultAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,


Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -79,6 +79,8 @@
 case attr::ObjCBoxable: return CXCursor_ObjCBoxable;
 case attr::FlagEnum: return CXCursor_FlagEnum;
 case attr::Convergent: return CXCursor_ConvergentAttr;
+case attr::WarnUnused: return CXCursor_WarnUnusedAttr;
+case attr::WarnUnusedResult: return CXCursor_WarnUnusedResultAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5477,6 +5477,10 @@
   return cxstring::createRef("FriendDecl");
   case CXCursor_ConvergentAttr:
   return cxstring::createRef("attribute(convergent)");
+  case CXCursor_WarnUnusedAttr:
+  return cxstring::createRef("attribute(warn_unused)");
+  case CXCursor_WarnUnusedResultAttr:
+  return cxstring::createRef("attribute(warn_unused_result)");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");
In

[PATCH] D58571: [libclang] Fix a trivial error introduced in D57946.

2019-02-22 Thread Emilio Cobos Álvarez via Phabricator via cfe-commits
emilio created this revision.
emilio added reviewers: Anastasia, arphaman.
Herald added a reviewer: serge-sans-paille.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The value for CXCursor_ConvergentAttr is not 420. I'm not really sure how easy
it is to test this, and I'm not familiar with the python bindings, just noticed
the error while looking at D57946  to write 
D58570 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58571

Files:
  clang/bindings/python/clang/cindex.py


Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -1342,7 +1342,7 @@
 
 CursorKind.DLLEXPORT_ATTR = CursorKind(418)
 CursorKind.DLLIMPORT_ATTR = CursorKind(419)
-CursorKind.CONVERGENT_ATTR = CursorKind(420)
+CursorKind.CONVERGENT_ATTR = CursorKind(438)
 
 ###
 # Preprocessing


Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -1342,7 +1342,7 @@
 
 CursorKind.DLLEXPORT_ATTR = CursorKind(418)
 CursorKind.DLLIMPORT_ATTR = CursorKind(419)
-CursorKind.CONVERGENT_ATTR = CursorKind(420)
+CursorKind.CONVERGENT_ATTR = CursorKind(438)
 
 ###
 # Preprocessing
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r354721 - Remove sanitizer context workaround no longer necessary

2019-02-22 Thread Brad Smith via cfe-commits
Author: brad
Date: Fri Feb 22 22:19:28 2019
New Revision: 354721

URL: http://llvm.org/viewvc/llvm-project?rev=354721&view=rev
Log:
Remove sanitizer context workaround no longer necessary

The base linker is now lld.

Modified:
cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp?rev=354721&r1=354720&r2=354721&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp Fri Feb 22 22:19:28 2019
@@ -226,9 +226,7 @@ void openbsd::Linker::ConstructJob(Compi
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtend)));
   }
 
-  const char *Exec = Args.MakeArgString(
-  !NeedsSanitizerDeps ? ToolChain.GetLinkerPath()
-  : ToolChain.GetProgramPath("ld.lld"));
+  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 


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


[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

2019-02-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D50488#1405094 , @mgrang wrote:

> So I was able compile a couple of C++ code bases through csa-testbench. I 
> built cppcheck and tinyxml2 without any problems. cppcheck has one invocation 
> std::sort but the keys are not pointers whereas tinyxml2 does not use 
> std::sort. I tried bitcoin, rtags, xerces but run into a lot of 
> configure/build errors.


Thats great. How about LLVM+Clang? That'll be a pain in the butt to analyze, 
but is pretty great for testing. Also, did you clone rtags with the git option 
`--recursive`?




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:97
 
+def NonDeterminismAlpha : Package<"nondeterminism">, ParentPackage;
+

Hmmm, okay, so your checker ks C++ exclusive I belive? How about making this 
checker reside in `alpha.cplusplus`? Rgard this one as more of a question.



Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:1
+//===-- PointerSortingChecker.cpp 
-===//
+//

Missing emacs thingie here?



Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:30
+// ID of a node at which the diagnostic would be emitted.
+const char *WarnAtNode = "sort";
+

I heard somewhere that LLVM::StringLiteral is used for global constexpr 
strings? Anyway, constexpr could be added here if I'm wrong sbout that :)



Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:110
+void ento::registerPointerSortingChecker(CheckerManager &Mgr) {
+  if (Mgr.getLangOpts().CPlusPlus)
+Mgr.registerChecker();

This is no longer needed, your checker wont be registered if the shouldRegister 
function returns false.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50488/new/

https://reviews.llvm.org/D50488



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


[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-22 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:196
+  while (true) {
+if (close(inotifyFD) == -1 && errno == EINTR)
+  continue;

There's some fancy function for this in LLVM. `RetryAfterSignal`, I think.



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135
+if (!statusOpt.hasValue())
+  K = DirectoryWatcher::EventKind::Removed;
+  }

jkorous wrote:
> mgorny wrote:
> > Why? I suppose this deserves a comment.
> I'll add this comment:
> 
> // The file might have been removed just after we received the event.
Wouldn't that cause removals to be reported twice?



Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher.cpp:95
+
+#if !defined(__has_include)
+#define __has_include(x) 0

jkorous wrote:
> mgorny wrote:
> > Why not use CMake checks? You're already partially using them for 
> > frameworks.
> Do I understand correctly that you suggest doing the check in CMake and 
> propagating the result to preprocessor?
Yes, using `check_include_file` and declaring `HAVE_SOMETHING` in config.h or 
alike.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58418/new/

https://reviews.llvm.org/D58418



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


  1   2   >