[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2019-12-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.
Herald added a subscriber: rnkovacs.

@rsmith, could you please take a look and let me know whether you think adding 
a new type for this makes sense?

On one hand, I feel it's good to have a type to represent "dependencies" and it 
allow to write helper functions and easily add new bits into the dependencies 
without rewriting all the code propagating those flags (error bit).

OTOH, using this for some things might be confusing:

- a type cannot be value-dependent, but dependency flags allow for that.
- a template argument can currently be "dependent" after can be either type- or 
value-dependent, also confusing.
- ...

Note there are still some rough edges here, see the added FIXMEs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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


[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2019-12-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:17
 
+using namespace clang::ast_matchers;
+

NOTE:
We need this to fix compiler errors down below. There's an AST matcher called 
`isTypeDependent` and we add a function 
`clang::isTypeDependent(DependencyFlags)` in this change.

Overload resolution does its job, but we have to make sure they're in the same 
namespace, so we need to put `using namespace` somewhere inside the `clang` 
namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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


[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2019-12-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Also note that this change does not move any code around to make sure this 
change is easy to review and validate that the code is doing the same thing.
I'm also planning to move all the code that computes dependencies into one 
place (it's currently scattered around many files and it certainly makes it 
quite hard to find bugs in the code and update it). But I would really like to 
get everyone aligned on the direction we're taking here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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


[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2019-12-27 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61128 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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


[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

2019-12-27 Thread Eugene Leviant via Phabricator via cfe-commits
evgeny777 added inline comments.



Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:794
+  }
+  GlobalObject::VCallVisibility vCallVisibility() const {
+return (GlobalObject::VCallVisibility)VarFlags.VCallVisibility;

getVCallVisibility() ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71911



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


[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

2019-12-27 Thread Eugene Leviant via Phabricator via cfe-commits
evgeny777 added a comment.

Looks good at first sight. Do you have linker patch for me to try this out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71907



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


[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

2019-12-27 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

In D71907#1797218 , @evgeny777 wrote:

> Looks good at first sight. Do you have linker patch for me to try this out?


The linker changes are in D71913 . This one 
should be a noop by itself (other than just getting the additional metadata). 
You need all three (this one, D71911  and 
D71913 ) to implement the RFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71907



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


[clang-tools-extra] f072233 - Allow newlines in AST Matchers in clang-query files

2019-12-27 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2019-12-27T15:25:57Z
New Revision: f0722333dd167245eb3c2b4263529a1ce3679b5c

URL: 
https://github.com/llvm/llvm-project/commit/f0722333dd167245eb3c2b4263529a1ce3679b5c
DIFF: 
https://github.com/llvm/llvm-project/commit/f0722333dd167245eb3c2b4263529a1ce3679b5c.diff

LOG: Allow newlines in AST Matchers in clang-query files

Reviewers: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-query/Query.cpp
clang-tools-extra/clang-query/Query.h
clang-tools-extra/clang-query/QueryParser.cpp
clang-tools-extra/clang-query/tool/ClangQuery.cpp
clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
clang/include/clang/ASTMatchers/Dynamic/Parser.h
clang/lib/ASTMatchers/Dynamic/Parser.cpp
clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-query/Query.cpp 
b/clang-tools-extra/clang-query/Query.cpp
index 675fd968f46e..8eafc5eed750 100644
--- a/clang-tools-extra/clang-query/Query.cpp
+++ b/clang-tools-extra/clang-query/Query.cpp
@@ -101,9 +101,24 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession 
&QS) const {
 Finder.matchAST(AST->getASTContext());
 
 if (QS.PrintMatcher) {
-  std::string prefixText = "Matcher: ";
-  OS << "\n  " << prefixText << Source << "\n";
-  OS << "  " << std::string(prefixText.size() + Source.size(), '=') << 
'\n';
+  SmallVector Lines;
+  Source.split(Lines, "\n");
+  auto FirstLine = Lines[0];
+  Lines.erase(Lines.begin(), Lines.begin() + 1);
+  while (!Lines.empty() && Lines.back().empty()) {
+Lines.resize(Lines.size() - 1);
+  }
+  unsigned MaxLength = FirstLine.size();
+  std::string PrefixText = "Matcher: ";
+  OS << "\n  " << PrefixText << FirstLine;
+
+  for (auto Line : Lines) {
+OS << "\n" << std::string(PrefixText.size() + 2, ' ') << Line;
+MaxLength = std::max(MaxLength, Line.rtrim().size());
+  }
+
+  OS << "\n"
+ << "  " << std::string(PrefixText.size() + MaxLength, '=') << "\n\n";
 }
 
 for (auto MI = Matches.begin(), ME = Matches.end(); MI != ME; ++MI) {

diff  --git a/clang-tools-extra/clang-query/Query.h 
b/clang-tools-extra/clang-query/Query.h
index 56af486984ee..78bcbc79cdf8 100644
--- a/clang-tools-extra/clang-query/Query.h
+++ b/clang-tools-extra/clang-query/Query.h
@@ -44,6 +44,7 @@ struct Query : llvm::RefCountedBase {
   /// \return false if an error occurs, otherwise return true.
   virtual bool run(llvm::raw_ostream &OS, QuerySession &QS) const = 0;
 
+  StringRef RemainingContent;
   const QueryKind Kind;
 };
 

diff  --git a/clang-tools-extra/clang-query/QueryParser.cpp 
b/clang-tools-extra/clang-query/QueryParser.cpp
index 4da2f5da79d4..a980722de9e6 100644
--- a/clang-tools-extra/clang-query/QueryParser.cpp
+++ b/clang-tools-extra/clang-query/QueryParser.cpp
@@ -26,7 +26,10 @@ namespace query {
 // is found before End, return StringRef().  Begin is adjusted to exclude the
 // lexed region.
 StringRef QueryParser::lexWord() {
-  Line = Line.ltrim();
+  Line = Line.drop_while([](char c) {
+// Don't trim newlines.
+return StringRef(" \t\v\f\r").contains(c);
+  });
 
   if (Line.empty())
 // Even though the Line is empty, it contains a pointer and
@@ -34,12 +37,12 @@ StringRef QueryParser::lexWord() {
 // code completion.
 return Line;
 
-  if (Line.front() == '#') {
-Line = {};
-return StringRef();
-  }
+  StringRef Word;
+  if (Line.front() == '#')
+Word = Line.substr(0, 1);
+  else
+Word = Line.take_until(isWhitespace);
 
-  StringRef Word = Line.take_until(isWhitespace);
   Line = Line.drop_front(Word.size());
   return Word;
 }
@@ -125,9 +128,25 @@ template  QueryRef 
QueryParser::parseSetOutputKind() {
 }
 
 QueryRef QueryParser::endQuery(QueryRef Q) {
-  const StringRef Extra = Line;
-  if (!lexWord().empty())
-return new InvalidQuery("unexpected extra input: '" + Extra + "'");
+  StringRef Extra = Line;
+  StringRef ExtraTrimmed = Extra.drop_while(
+  [](char c) { return StringRef(" \t\v\f\r").contains(c); });
+
+  if ((!ExtraTrimmed.empty() && ExtraTrimmed[0] == '\n') ||
+  (ExtraTrimmed.size() >= 2 && ExtraTrimmed[0] == '\r' &&
+   ExtraTrimmed[1] == '\n'))
+Q->RemainingContent = Extra;
+  else {
+StringRef TrailingWord = lexWord();
+if (!TrailingWord.empty() && TrailingWord.front() == '#') {
+  Line = Line.drop_until([](char c) { return c == '\n'; });
+  Line = Line.drop_while([](char c) { return c == '\n'; });
+  return endQuery(Q);
+}
+if (!TrailingWord.empty()) {
+  return new InvalidQuery("unexpected extra input: '" + Extra + "'");
+}
+  }
   return Q;
 }
 
@@ -193,7 +212,11 @@ QueryRef QueryParser::doParse() {
   switch (QKind) {
   case PQK_Commen

[PATCH] D71725: [OpenCL] Fix inconsistency between opencl and c11 atomic fetch max/min

2019-12-27 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks!


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

https://reviews.llvm.org/D71725



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


[PATCH] D71460: [OpenCL] Fix support for cl_khr_mipmap_image_writes

2019-12-27 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/lib/Headers/opencl-c.h:14686
+#if defined(cl_khr_mipmap_image_writes)
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : begin
 void __ovld write_imagef(write_only image1d_t image, int coord, int lod, 
float4 color);

AlexeySotkin wrote:
> Anastasia wrote:
> > Do we actually need pragma for this extension? I.e. does it need to 
> > activate any special mode in the compiler?
> I'm not aware of any special mode required for this extension. These 
> begin/end pragma lines were added to disable the extension by default as it 
> is [[ 
> https://github.com/KhronosGroup/OpenCL-Docs/blob/master/ext/introduction.asciidoc#compiler-directives-for-optional-extensions
>  | required ]] by the OpenCL extension spec. Is there any other mechanism 
> which should be used for this purpose?
> Probably, we should do the same for `cl_khr_mipmap_image`(and maybe others?), 
> because with the current compiler, built-ins from this extension can be 
> compiled successfully even if `#pragma OPENCL EXTENSION cl_khr_mipmap_image : 
> disable` is specified. See https://godbolt.org/z/fNEWuG
What I am saying is that `pragma` is only typically used to activate some 
special mode in the compiler. If we don't need to activate anything perhaps we 
don't need to add `pragma` at all? It simplifies compiler and application code 
too. Would regular include mechanisms be enough to guard the availability of 
those functions?

Maybe this thread will help to understand the topic better: 
https://github.com/KhronosGroup/OpenCL-Docs/issues/82


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

https://reviews.llvm.org/D71460



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


[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits
pastey created this revision.
pastey added a reviewer: MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

Found a bug introduced with BraceWrappingFlags AfterControlStatement MultiLine. 
This feature conflicts with the existing BeforeCatch and BeforeElse flags.

For example, our team uses BeforeElse.

if (foo ||

bar) {
  doSomething();

}
else {

  doSomethingElse();

}

If we enable MultiLine (which we'd really love to do) we expect it to work like 
this:

if (foo ||

  bar)

{

  doSomething();

}
else {

  doSomethingElse();

}

What we actually get is:

if (foo ||

  bar)

{

  doSomething();

}
else
{

  doSomethingElse();

}

I added two tests that show this issue. One for if/else and one for try/catch 
(which is affected in the same way as if/else).

One more test with ColumnLimit set to 40 is to check that a suggested fix 
doesn't break existing cases. This test is the copy of an existing test from 
the same case, but it removes line wrap from the stage.

I suggest the fix, but I'm new to this code, so maybe you could suggest 
something better.

As far as I understood, the problem is caused by the following code:

  CompoundStatementIndenter(UnwrappedLineParser *Parser,
const FormatStyle &Style, unsigned &LineLevel)
  : CompoundStatementIndenter(Parser, LineLevel,
  Style.BraceWrapping.AfterControlStatement,  
// <- here
  Style.BraceWrapping.IndentBraces) {}
  CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned &LineLevel,
bool WrapBrace, bool IndentBrace)
  : LineLevel(LineLevel), OldLineLevel(LineLevel) {
if (WrapBrace)
// <- and here
  Parser->addUnwrappedLine();
if (IndentBrace)
  ++LineLevel;
  }

Style.BraceWrapping.AfterControlStatement used to be boolean, now it's an enum. 
MultiLine and Always both turn into true, thus MultiLine leads to a call of 
addUnwrappedLine().
I tried to place Style.BraceWrapping.AfterControlStatement == 
FormatStyle::BWACS_Always right in the CompoundStatementIndenter constructor, 
but that breaks MultiLine.

As I said, I'm new to this code, so maybe my fix is not at the right place, so 
I would be glad if we find a better fix.


Repository:
  rC Clang

https://reviews.llvm.org/D71939

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception&bar){baz();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column 
limit
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"} catch (Exception &bar) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception&bar){baz();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column 
limit
+
+  Style.BraceWrapping.BeforeElse = true;
+  EXPECT_EQ(
+  "if (foo) {\n"
+  "  bar();\n"
+  "}\n"
+  "else if (baz ||\n"
+  " quux)\n"
+  "{\n"
+  "  foobar();\n"
+  "}\n"
+  "else {\n"
+  "  barbaz();\n"
+  "}",
+  format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
+ Style));
+
+  Style.BraceWrapping.BeforeCatch = true;
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"}\n"
+"catch (...) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(...){baz();}", Style));
 }
 
 
//===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
- TEST_F(FormatTest, SpacesInConditionalStatement) {
+TEST_F(FormatTest, SpacesInConditionalStatement) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInConditionalStatement = true;
   verifyFormat("for ( int i = 0; i; i++ )\n  continue;", Spaces);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1785,7 +1785,11 @@
   if (FormatTok->Tok.is(tok::kw_else)) {
 nextToken();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  CompoundStatementIndenter Indenter(this, Style, Line->Level);
+  CompoundStatementIndenter Indenter(
+  this, Line->Level,
+  Style.BraceWrapping.AfterControlStatement ==
+  FormatStyle::BWACS_Always,
+  Style.BraceWrapping.IndentBraces);
   parseBlock(/*MustBeDecl

[clang] 134ef0f - [OpenCL] Fix inconsistency between opencl and c11 atomic fetch max/min

2019-12-27 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2019-12-27T11:29:04-05:00
New Revision: 134ef0fb4b92718477a1dc9da0118f9b2dd77237

URL: 
https://github.com/llvm/llvm-project/commit/134ef0fb4b92718477a1dc9da0118f9b2dd77237
DIFF: 
https://github.com/llvm/llvm-project/commit/134ef0fb4b92718477a1dc9da0118f9b2dd77237.diff

LOG: [OpenCL] Fix inconsistency between opencl and c11 atomic fetch max/min

There is some inconsistency between opencl and c11 atomic fetch max/min after

https://reviews.llvm.org/D46386

https://reviews.llvm.org/D55562

It is not reasonable to have such inconsistencies. This patch fixes that.

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaOpenCL/atomic-ops.cl

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index b8295bc741eb..d8711fb6bcab 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -4642,8 +4642,6 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
   case AtomicExpr::AO__c11_atomic_fetch_sub:
   case AtomicExpr::AO__opencl_atomic_fetch_add:
   case AtomicExpr::AO__opencl_atomic_fetch_sub:
-  case AtomicExpr::AO__opencl_atomic_fetch_min:
-  case AtomicExpr::AO__opencl_atomic_fetch_max:
   case AtomicExpr::AO__atomic_fetch_add:
   case AtomicExpr::AO__atomic_fetch_sub:
   case AtomicExpr::AO__atomic_add_fetch:
@@ -4666,6 +4664,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
   case AtomicExpr::AO__atomic_nand_fetch:
   case AtomicExpr::AO__c11_atomic_fetch_min:
   case AtomicExpr::AO__c11_atomic_fetch_max:
+  case AtomicExpr::AO__opencl_atomic_fetch_min:
+  case AtomicExpr::AO__opencl_atomic_fetch_max:
   case AtomicExpr::AO__atomic_min_fetch:
   case AtomicExpr::AO__atomic_max_fetch:
   case AtomicExpr::AO__atomic_fetch_min:

diff  --git a/clang/test/SemaOpenCL/atomic-ops.cl 
b/clang/test/SemaOpenCL/atomic-ops.cl
index c95c73d52e52..5a8c6fc4fbda 100644
--- a/clang/test/SemaOpenCL/atomic-ops.cl
+++ b/clang/test/SemaOpenCL/atomic-ops.cl
@@ -77,8 +77,8 @@ void f(atomic_int *i, const atomic_int *ci,
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, 
memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, 
memory_scope_work_group);
-  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer or pointer ('__generic 
atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
-  __opencl_atomic_fetch_max(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer or pointer ('__generic 
atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer ('__generic atomic_float *' (aka 
'__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_max(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer ('__generic atomic_float *' (aka 
'__generic _Atomic(float) *') invalid)}}
 
   bool cmpexch_1 = __opencl_atomic_compare_exchange_strong(i, I, 1, 
memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);
   bool cmpexch_2 = __opencl_atomic_compare_exchange_strong(p, P, 1, 
memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);



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


[PATCH] D71725: [OpenCL] Fix inconsistency between opencl and c11 atomic fetch max/min

2019-12-27 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG134ef0fb4b92: [OpenCL] Fix inconsistency between opencl and 
c11 atomic fetch max/min (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71725

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaOpenCL/atomic-ops.cl


Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -77,8 +77,8 @@
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, 
memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, 
memory_scope_work_group);
-  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer or pointer ('__generic 
atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
-  __opencl_atomic_fetch_max(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer or pointer ('__generic 
atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer ('__generic atomic_float *' (aka 
'__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_max(d, 1, memory_order_seq_cst, 
memory_scope_work_group); // expected-error {{address argument to atomic 
operation must be a pointer to atomic integer ('__generic atomic_float *' (aka 
'__generic _Atomic(float) *') invalid)}}
 
   bool cmpexch_1 = __opencl_atomic_compare_exchange_strong(i, I, 1, 
memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);
   bool cmpexch_2 = __opencl_atomic_compare_exchange_strong(p, P, 1, 
memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4642,8 +4642,6 @@
   case AtomicExpr::AO__c11_atomic_fetch_sub:
   case AtomicExpr::AO__opencl_atomic_fetch_add:
   case AtomicExpr::AO__opencl_atomic_fetch_sub:
-  case AtomicExpr::AO__opencl_atomic_fetch_min:
-  case AtomicExpr::AO__opencl_atomic_fetch_max:
   case AtomicExpr::AO__atomic_fetch_add:
   case AtomicExpr::AO__atomic_fetch_sub:
   case AtomicExpr::AO__atomic_add_fetch:
@@ -4666,6 +4664,8 @@
   case AtomicExpr::AO__atomic_nand_fetch:
   case AtomicExpr::AO__c11_atomic_fetch_min:
   case AtomicExpr::AO__c11_atomic_fetch_max:
+  case AtomicExpr::AO__opencl_atomic_fetch_min:
+  case AtomicExpr::AO__opencl_atomic_fetch_max:
   case AtomicExpr::AO__atomic_min_fetch:
   case AtomicExpr::AO__atomic_max_fetch:
   case AtomicExpr::AO__atomic_fetch_min:


Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -77,8 +77,8 @@
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
-  __opencl_atomic_fetch_max(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_min(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_max(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
 
   bool cmpexch_1 = __opencl_atomic_compare_exchange_strong(i, I, 1, memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);
   bool cmpexch_2 = __opencl_atomic_compare_exchange_strong(p, P, 1, memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4642,8 +4642,6 @@
   case AtomicExpr::

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

2019-12-27 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

It looks good, I was just thinking whether it would be possible to share more 
common infrastructure. There is `Sema::CheckVectorOperands` that corresponding 
OpenCL methods are using internally. Do you think it is possible to share the 
code more?




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6880
+def err_conditional_vector_has_void : Error<
+  "GNU vector conditional operand cannot be %select{void|a throw 
expression}0">;
+def err_conditional_vector_operand_type

Would this only apply to GNU extension? I am just wondering if this can be 
generalized.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5797
+// If both are vector types, they must be the same type.
+if (!Context.hasSameType(LHSType, RHSType)) {
+  Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors)

Don't we already have a diagnostic for this used in OpenCL?


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

https://reviews.llvm.org/D71463



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


[clang] 752220e - [OpenCL] Fixed printing of __private in AMDGPU test

2019-12-27 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2019-12-27T17:08:42Z
New Revision: 752220ea2664c814eb1eb046d755fe63ade9c32e

URL: 
https://github.com/llvm/llvm-project/commit/752220ea2664c814eb1eb046d755fe63ade9c32e
DIFF: 
https://github.com/llvm/llvm-project/commit/752220ea2664c814eb1eb046d755fe63ade9c32e.diff

LOG: [OpenCL] Fixed printing of __private in AMDGPU test

Tags: #clang

Added: 


Modified: 
clang/test/SemaOpenCL/numbered-address-space.cl

Removed: 




diff  --git a/clang/test/SemaOpenCL/numbered-address-space.cl 
b/clang/test/SemaOpenCL/numbered-address-space.cl
index 8a24b3ce979c..ab824ac531ff 100644
--- a/clang/test/SemaOpenCL/numbered-address-space.cl
+++ b/clang/test/SemaOpenCL/numbered-address-space.cl
@@ -11,7 +11,7 @@ void 
test_numeric_as_to_generic_explicit_cast(__attribute__((address_space(3)))
 
 void test_generic_to_numeric_as_implicit_cast() {
   generic int* generic_ptr = 0;
-  __attribute__((address_space(3))) int *as3_ptr = generic_ptr; // 
expected-error{{initializing '__attribute__((address_space(3))) int *' with an 
expression of type '__generic int *' changes address space of pointer}}
+  __attribute__((address_space(3))) int *as3_ptr = generic_ptr; // 
expected-error{{initializing '__attribute__((address_space(3))) int *__private' 
with an expression of type '__generic int *__private' changes address space of 
pointer}}
 }
 
 void test_generic_to_numeric_as_explicit_cast() {
@@ -26,6 +26,6 @@ void 
test_generic_as_to_builtin_parameter_explicit_cast_numeric(__attribute__((a
 
 void 
test_generic_as_to_builtin_parameterimplicit_cast_numeric(__attribute__((address_space(3)))
 int *as3_ptr, float src) {
   generic int* generic_ptr = as3_ptr;
-  volatile float result = __builtin_amdgcn_ds_fmaxf(generic_ptr, src, 0, 0, 
false); // expected-error {{passing '__generic int *' to parameter of type 
'__local float *' changes address space of pointer}}
+  volatile float result = __builtin_amdgcn_ds_fmaxf(generic_ptr, src, 0, 0, 
false); // expected-error {{passing '__generic int *__private' to parameter of 
type '__local float *' changes address space of pointer}}
 }
 



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


[PATCH] D71463: Implement VectorType conditional operator GNU extension.

2019-12-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 2 inline comments as done.
erichkeane added a comment.

In D71463#1797436 , @Anastasia wrote:

> It looks good, I was just thinking whether it would be possible to share more 
> common infrastructure. There is `Sema::CheckVectorOperands` that 
> corresponding OpenCL methods are using internally. Do you think it is 
> possible to share the code more?


I tried in my initial implementation.  Unfortunately CheckVectorOperands and 
the OpenCL vectors have significantly different semantic rules from this, these 
aren't even consistent with the normal GCC vector conversion rules. The 
similarities/differences between all of these are frustrating, any attempt I 
made at unifying the logic made both really difficult to follow.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6880
+def err_conditional_vector_has_void : Error<
+  "GNU vector conditional operand cannot be %select{void|a throw 
expression}0">;
+def err_conditional_vector_operand_type

Anastasia wrote:
> Would this only apply to GNU extension? I am just wondering if this can be 
> generalized.
To be honest, I limited the scope of this a bit since the ext_vector type isn't 
something I have a great knowledge of.  The OpenCL vector types end up having 
frustratingly similar/different behavior that it required significant 
differences in logic.

Does OpenCL prohibit void/throw expressions in their vector conditionals?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5797
+// If both are vector types, they must be the same type.
+if (!Context.hasSameType(LHSType, RHSType)) {
+  Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors)

Anastasia wrote:
> Don't we already have a diagnostic for this used in OpenCL?
The one I found was less specific than just 'mismatched'.  It ended up 
containing info about not convertible.  I tried a set of 'selects' on it but it 
ended up getting hacked up more than I was comfortable with.  Unless you're 
referring to a different one that I missed?


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

https://reviews.llvm.org/D71463



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


[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

2019-12-27 Thread Brian Gesiak via Phabricator via cfe-commits
modocache marked an inline comment as done.
modocache added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1227
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));
+  MPM.addPass(createModuleToFunctionPassAdaptor(CoroElidePass()));
+  
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass()));

junparser wrote:
> Since coro elision depends on other optimization pass(inline and so on)  
> implicitly,  how can we adjust the pipeline to achieve this.
One option would be to use the new pass manager's registration callbacks, like 
`PassBuilder::registerPipelineStartEPCallback` or 
`PassBuilder::registerOptimizerLastEPCallback`. These work similarly to the old 
pass manager's `PassManagerBuilder::addExtension`. That's something that I 
think would be good to improve in a follow-up patch, but let me know if you'd 
rather see it in this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71903



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


[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska updated this revision to Diff 235435.
Bouska added a comment.

Set line length to column limit + 1 (41) in tests


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

https://reviews.llvm.org/D71512

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -583,7 +583,7 @@
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true) { //\n"
@@ -659,7 +659,7 @@
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
@@ -721,7 +721,9 @@
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
-"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"  {\\\n"
+"RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier;   \\\n"
+"  }\n"
 "X;",
 format("#define A \\\n"
"   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -327,21 +327,6 @@
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
 }
-// Try to merge either empty or one-line block if is precedeed by control
-// statement token
-if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
-I != AnnotatedLines.begin() &&
-I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  unsigned MergedLines = 0;
-  if (Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never) {
-MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
-// If we managed to merge the block, discard the first merged line
-// since we are merging starting from I.
-if (MergedLines > 0)
-  --MergedLines;
-  }
-  return MergedLines;
-}
 // Don't merge block with left brace wrapped after ObjC special blocks
 if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
 I[-1]->First->is(tok::at) && I[-1]->First->Next) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -583,7 +583,7 @@
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true) { //\n"
@@ -659,7 +659,7 @@
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
@@ -721,7 +721,9 @@
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
-"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"  {\\\n"
+"RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier;   \\\n"
+"  }\n"
 "X;",
 format("#define A \\\n"
"   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -327,21 +327,6 @@
  ? tryMergeSimpleBlock(I, E, Limit)
 

[clang] 780d306 - [VFS] Don't run symlink test on Windows, it may pass or fail

2019-12-27 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2019-12-27T10:39:47-08:00
New Revision: 780d30660e965992cf10d390e8ff102e4bf82aa4

URL: 
https://github.com/llvm/llvm-project/commit/780d30660e965992cf10d390e8ff102e4bf82aa4
DIFF: 
https://github.com/llvm/llvm-project/commit/780d30660e965992cf10d390e8ff102e4bf82aa4.diff

LOG: [VFS] Don't run symlink test on Windows, it may pass or fail

This test was XFAILed because of symlinks, but some versions of ln -s
seem to work on Windows, so the test was unexpectedly passing on our
bot:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/13233
Unexpected Passing Tests (1):
Clang :: VFS/subframework-symlink.m

I don't know how or why, but it seems dependent on system configuration,
and is not something worth debugging. Avoid the problem by marking the
test UNSUPPORTED: system-windows instead of XFAIL: system-windows.

Added: 


Modified: 
clang/test/VFS/subframework-symlink.m

Removed: 




diff  --git a/clang/test/VFS/subframework-symlink.m 
b/clang/test/VFS/subframework-symlink.m
index 77e8572aa457..88946e86d43c 100644
--- a/clang/test/VFS/subframework-symlink.m
+++ b/clang/test/VFS/subframework-symlink.m
@@ -1,5 +1,5 @@
 // FIXME: PR44221
-// XFAIL: system-windows
+// UNSUPPORTED: system-windows
 
 // Test that when a subframework is a symlink to another framework, we don't
 // add it as a submodule to the enclosing framework. We also need to make clang



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


Re: [clang] ce1f95a - Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"

2019-12-27 Thread David Blaikie via cfe-commits
On Thu, Dec 26, 2019 at 11:58 PM Djordje Todorovic <
djordje.todoro...@rt-rk.com> wrote:

> Hi David,
>
> It's a good question.
>
> Current approach of the debug entry values will consider an entry value as
> a valid value until the variable gets modified.
>
> Please consider this.
>
> void fn(int a) {
>   ...
>   a++;
> }
>
> If there is an instruction that does not affect the code generated, e.g.
> an ADD instruction that gets optimized out from the case above, it won't
> force us to invalidate all the entry values before, since the instruction
> is not there in the final code generated. The GCC does the same thing in
> that situation. But if the instruction were at the beginning of the
> function (or somewhere else), we believe that there is an DBG_VALUE
> representing that variable's change (e.g. generated from the Salvage Debug
> Info), so the entry value would not be used any more.
>
> If we come up with a case where a dead store causing an invalid use of the
> entry values, that will be good point for improvements.
>

Ah, OK, so you actually want to know whether the entry value gets really
modified, makes sense to do that in the backend then - thanks for
explaining!


>
> Best regards,
> Djordje
>
> On 26.12.19. 22:33, David Blaikie wrote:
> >
> >
> > On Wed, Nov 20, 2019 at 1:08 AM Djordje Todorovic via cfe-commits <
> cfe-commits@lists.llvm.org > wrote:
> >
> >
> > Author: Djordje Todorovic
> > Date: 2019-11-20T10:08:07+01:00
> > New Revision: ce1f95a6e077693f93d8869245f911aff3eb7e4c
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c
> > DIFF:
> https://github.com/llvm/llvm-project/commit/ce1f95a6e077693f93d8869245f911aff3eb7e4c.diff
> >
> > LOG: Reland "[clang] Remove the DIFlagArgumentNotModified debug info
> flag"
> >
> > It turns out that the ExprMutationAnalyzer can be very slow when AST
> > gets huge in some cases. The idea is to move this analysis to the
> LLVM
> > back-end level (more precisely, in the LiveDebugValues pass). The new
> > approach will remove the performance regression, simplify the
> > implementation and give us front-end independent implementation.
> >
> >
> > What if the LLVM backend optimized out a dead store? (then we might
> concnlude that the argument is not modified, when it actually is modified?)
> >
> >
> >
> > Differential Revision: https://reviews.llvm.org/D68206
> >
> > Added:
> >
> >
> > Modified:
> > clang/lib/CodeGen/CGDebugInfo.cpp
> > clang/lib/CodeGen/CGDebugInfo.h
> >
> lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
> >
> > Removed:
> > clang/test/CodeGen/debug-info-param-modification.c
> >
> >
> >
>  
> 
> > diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> > index 116517a9cb99..a9b3831aa0b5 100644
> > --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> > +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> > @@ -18,7 +18,6 @@
> >  #include "CodeGenFunction.h"
> >  #include "CodeGenModule.h"
> >  #include "ConstantEmitter.h"
> > -#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
> >  #include "clang/AST/ASTContext.h"
> >  #include "clang/AST/DeclFriend.h"
> >  #include "clang/AST/DeclObjC.h"
> > @@ -3686,15 +3685,6 @@ void
> CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,
> >if (HasDecl && isa(D))
> >  DeclCache[D->getCanonicalDecl()].reset(SP);
> >
> > -  // We use the SPDefCache only in the case when the debug entry
> values option
> > -  // is set, in order to speed up parameters modification analysis.
> > -  //
> > -  // FIXME: Use AbstractCallee here to support ObjCMethodDecl.
> > -  if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl)
> > -if (auto *FD = dyn_cast(D))
> > -  if (FD->hasBody() && !FD->param_empty())
> > -SPDefCache[FD].reset(SP);
> > -
> >// Push the function onto the lexical block stack.
> >LexicalBlockStack.emplace_back(SP);
> >
> > @@ -4097,11 +4087,6 @@ llvm::DILocalVariable
> *CGDebugInfo::EmitDeclare(const VarDecl *VD,
> >   llvm::DebugLoc::get(Line, Column, Scope,
> CurInlinedAt),
> >   Builder.GetInsertBlock());
> >
> > -  if (CGM.getCodeGenOpts().EnableDebugEntryValues && ArgNo) {
> > -if (auto *PD = dyn_cast(VD))
> > -  ParamCache[PD].reset(D);
> > -  }
> > -
> >return D;
> >  }
> >
> > @@ -4717,29 +4702,6 @@ void CGDebugInfo::setDwoId(uint64_t
> Signature) {
> >TheCU->setDWOId(Signature);
> >  }
> >
> > -/// Analyzes each function parameter to determine whether it is
> constant
> > -/// throughout t

[clang] d801823 - Revert "CWG2352: Allow qualification conversions during reference binding."

2019-12-27 Thread David Blaikie via cfe-commits

Author: David Blaikie
Date: 2019-12-27T12:27:20-08:00
New Revision: d8018233d1ea4234de68d5b4593abd773db79484

URL: 
https://github.com/llvm/llvm-project/commit/d8018233d1ea4234de68d5b4593abd773db79484
DIFF: 
https://github.com/llvm/llvm-project/commit/d8018233d1ea4234de68d5b4593abd773db79484.diff

LOG: Revert "CWG2352: Allow qualification conversions during reference binding."

This reverts commit de21704ba96fa80d3e9402f12c6505917a3885f4.

Regressed/causes this to error due to ambiguity:

  void f(const int * const &);
  void f(int *);
  int main() {
int * x;
f(x);
  }

(in case it's important - the original case where this turned up was a
member function overload in a class template with, essentially:

  f(const T1&)
  f(T2*)

(where T1 == X const *, T2 == X))

It's not super clear to me if this ^ is expected behavior, in which case
I'm sorry about the revert & happy to look into ways to fix the original
code.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/drs/dr23xx.cpp
clang/test/CXX/drs/dr4xx.cpp
clang/test/SemaObjCXX/arc-overloading.mm
clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
clang/www/cxx_dr_status.html
clang/www/make_cxx_dr_status

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b86abd0db73e..54299a0409fd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1933,8 +1933,7 @@ def err_lvalue_reference_bind_to_unrelated : Error<
   "cannot bind to a value of unrelated type}1,2">;
 def err_reference_bind_drops_quals : Error<
   "binding reference %
diff {of type $ to value of type $|to value}0,1 "
-  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space|"
-  "not permitted due to incompatible qualifiers}2">;
+  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space}2">;
 def err_reference_bind_failed : Error<
   "reference %
diff {to %select{type|incomplete type}1 $ could not bind to an "
   "%select{rvalue|lvalue}2 of type $|could not bind to %select{rvalue|lvalue}2 
of "

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index cd78f096bb22..cfb3a05e9c14 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5864,8 +5864,6 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult 
&Cond, ExprResult &LHS,
   //   one of the operands is reference-compatible with the other, in order
   //   to support conditionals between functions 
diff ering in noexcept. This
   //   will similarly cover 
diff erence in array bounds after P0388R4.
-  // FIXME: If LTy and RTy have a composite pointer type, should we convert to
-  //   that instead?
   ExprValueKind LVK = LHS.get()->getValueKind();
   ExprValueKind RVK = RHS.get()->getValueKind();
   if (!Context.hasSameType(LTy, RTy) &&

diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ef4fa827064e..94d524a63f5a 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -8919,17 +8919,11 @@ bool InitializationSequence::Diagnose(Sema &S,
   S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
   << NonRefType << SourceType << 1 /*addr space*/
   << Args[0]->getSourceRange();
-else if (DroppedQualifiers.hasQualifiers())
+else
   S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
   << NonRefType << SourceType << 0 /*cv quals*/
   << Qualifiers::fromCVRMask(DroppedQualifiers.getCVRQualifiers())
   << DroppedQualifiers.getCVRQualifiers() << Args[0]->getSourceRange();
-else
-  // FIXME: Consider decomposing the type and explaining which qualifiers
-  // were dropped where, or on which level a 'const' is missing, etc.
-  S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
-  << NonRefType << SourceType << 2 /*incompatible quals*/
-  << Args[0]->getSourceRange();
 break;
   }
 

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 92058c3aa5fd..74a0bc7c78ff 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -3153,70 +3153,6 @@ static bool 
isNonTrivialObjCLifetimeConversion(Qualifiers FromQuals,
   return true;
 }
 
-/// Perform a single iteration of the loop for checking if a qualification
-/// conversion is valid.
-///
-/// Specifically, check whether any change between the qualifiers of \p
-/// FromType and \p ToType is permissible, given knowledge about whether every
-/// outer layer is const-qualified.
-static bool isQualificationConversionStep(QualType FromType, QualType ToType,
-   

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment.

Your code still breaks MultiLine

  [ RUN  ] FormatTest.MultiLineControlStatements
  /home/pablomg/dev/llvm-project/clang/unittests/Format/FormatTest.cpp:1562: 
Failure
Expected: "try {\n" "  foo();\n" "} catch (\n" "Exception &bar)\n" 
"{\n" "  baz();\n" "}"
Which is: "try {\n  foo();\n} catch (\nException &bar)\n{\n  
baz();\n}"
  To be equal to: format("try{foo();}catch(Exception&bar){baz();}", Style)
Which is: "try {\n  foo();\n} catch (Exception\n &bar) {\n  
baz();\n}"
  With diff:
  @@ -1,7 +1,6 @@
   try {
 foo();
  -} catch (
  -Exception &bar)
  -{
  +} catch (Exception
  + &bar) {
 baz();
   }
  
  [  FAILED  ] FormatTest.MultiLineControlStatements (28 ms)


Repository:
  rC Clang

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

https://reviews.llvm.org/D71939



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


Re: [clang] de21704 - CWG2352: Allow qualification conversions during reference binding.

2019-12-27 Thread David Blaikie via cfe-commits
Sorry if this is expected behavior, but saw a regression that looks
/plausibly/ correct & is rendered ambiguous by this change so I've gone
ahead and reverted this patch in d8018233d1ea4234de68d5b4593abd773db79484

Comment from the original:

Regressed/causes this to error due to ambiguity:

  void f(const int * const &);
  void f(int *);
  int main() {
int * x;
f(x);
  }

(in case it's important - the original case where this turned up was a
member function overload in a class template with, essentially:

  f(const T1&)
  f(T2*)

(where T1 == X const *, T2 == X))

On Thu, Dec 19, 2019 at 6:56 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Richard Smith
> Date: 2019-12-19T18:37:55-08:00
> New Revision: de21704ba96fa80d3e9402f12c6505917a3885f4
>
> URL:
> https://github.com/llvm/llvm-project/commit/de21704ba96fa80d3e9402f12c6505917a3885f4
> DIFF:
> https://github.com/llvm/llvm-project/commit/de21704ba96fa80d3e9402f12c6505917a3885f4.diff
>
> LOG: CWG2352: Allow qualification conversions during reference binding.
>
> The language wording change forgot to update overload resolution to rank
> implicit conversion sequences based on qualification conversions in
> reference bindings. The anticipated resolution for that oversight is
> implemented here -- we order candidates based on qualification
> conversion, not only on top-level cv-qualifiers.
>
> For OpenCL/C++, this allows reference binding between pointers with
> differing (nested) address spaces. This makes the behavior of reference
> binding consistent with that of implicit pointer conversions, as is the
> purpose of this change, but that pre-existing behavior for pointer
> conversions is itself probably not correct. In any case, it's now
> consistently the same behavior and implemented in only one place.
>
> Added:
>
>
> Modified:
> clang/include/clang/Basic/DiagnosticSemaKinds.td
> clang/lib/Sema/SemaExprCXX.cpp
> clang/lib/Sema/SemaInit.cpp
> clang/lib/Sema/SemaOverload.cpp
> clang/test/CXX/drs/dr23xx.cpp
> clang/test/CXX/drs/dr4xx.cpp
> clang/test/SemaObjCXX/arc-overloading.mm
> clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
> clang/www/cxx_dr_status.html
> clang/www/make_cxx_dr_status
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
> b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> index c7b501b9bb2b..315e836cd397 100644
> --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1927,7 +1927,8 @@ def err_lvalue_reference_bind_to_unrelated : Error<
>"cannot bind to a value of unrelated type}1,2">;
>  def err_reference_bind_drops_quals : Error<
>"binding reference %
> diff {of type $ to value of type $|to value}0,1 "
> -  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address
> space}2">;
> +  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space|"
> +  "not permitted due to incompatible qualifiers}2">;
>  def err_reference_bind_failed : Error<
>"reference %
> diff {to %select{type|incomplete type}1 $ could not bind to an "
>"%select{rvalue|lvalue}2 of type $|could not bind to
> %select{rvalue|lvalue}2 of "
>
> diff  --git a/clang/lib/Sema/SemaExprCXX.cpp
> b/clang/lib/Sema/SemaExprCXX.cpp
> index cfb3a05e9c14..cd78f096bb22 100644
> --- a/clang/lib/Sema/SemaExprCXX.cpp
> +++ b/clang/lib/Sema/SemaExprCXX.cpp
> @@ -5864,6 +5864,8 @@ QualType
> Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
>//   one of the operands is reference-compatible with the other, in
> order
>//   to support conditionals between functions
> diff ering in noexcept. This
>//   will similarly cover
> diff erence in array bounds after P0388R4.
> +  // FIXME: If LTy and RTy have a composite pointer type, should we
> convert to
> +  //   that instead?
>ExprValueKind LVK = LHS.get()->getValueKind();
>ExprValueKind RVK = RHS.get()->getValueKind();
>if (!Context.hasSameType(LTy, RTy) &&
>
> diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
> index 94d524a63f5a..ef4fa827064e 100644
> --- a/clang/lib/Sema/SemaInit.cpp
> +++ b/clang/lib/Sema/SemaInit.cpp
> @@ -8919,11 +8919,17 @@ bool InitializationSequence::Diagnose(Sema &S,
>S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
><< NonRefType << SourceType << 1 /*addr space*/
><< Args[0]->getSourceRange();
> -else
> +else if (DroppedQualifiers.hasQualifiers())
>S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
><< NonRefType << SourceType << 0 /*cv quals*/
><< Qualifiers::fromCVRMask(DroppedQualifiers.getCVRQualifiers())
><< DroppedQualifiers.getCVRQualifiers() <<
> Args[0]->getSourceRange();
> +else

[PATCH] D71026: Fix LLVM_ENABLE_MODULES=ON + BUILD_SHARED_LIBS=ON build

2019-12-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D71026#1785025 , @arichardson wrote:

> If I build on macOS with `cmake -GNinja -DBUILD_SHARED_LIBS=ON 
> -DCMAKE_BUILD_TYPE=Debug 
> -DLLVM_ENABLE_PROJECTS=llvm;clang;lld;compiler-rt;libcxx;libcxxabi 
> -DBUILD_SHARED_LIBS=ON -DLLVM_ENABLE_MODULES=ON`, I get lots of linker errors 
> building e.g. libclangASTDiff.dylib due to using the internal ASTMatchers 
> header inside the Tooling/Transformer headers.
>
> This is probably due to https://bugs.llvm.org/show_bug.cgi?id=23521 mentioned 
> in the previous commits that added the exclude directives.


Could we instead workaround this by adhering to the LLVM style guide's 
"requirement" (it's clearly not universally applied) have strong vtables? 
https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
 that way this would come up less often, by the sounds of it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71026



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


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

2019-12-27 Thread Derek Argueta via Phabricator via cfe-commits
derek added a comment.
Herald added a subscriber: mgehre.

@zinovy.nis @alexfh @JonasToth @MyDeveloperDay

Hi folks, please correct me if I'm wrong but it appears that an effect of this 
change is that this script will no longer exit non-zero if `clang-tidy` 
discovers any errors, which was the previous functionality with 
`sys.exit(subprocess.call(' '.join(command), shell=True))`. As a result, when 
we upgraded a project to LLVM 9 our CI began having false-positives, as we 
relied on the exit code of this script to indicate success/failure. Would it be 
possible to restore that functionality? I'd be happy to provide a patch.


Repository:
  rCTE Clang Tools Extra

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] D71948: [OpenMP] Use the OpenMPIRBuilder for `cancel` directives

2019-12-27 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert created this revision.
jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, 
gtbercea, grokos, sdmitriev, JonChesterfield, fghanim.
Herald added subscribers: guansong, bollu, hiraditya.
Herald added a project: LLVM.
jdoerfert added a child revision: D70290: [OpenMP] Use the OpenMPIRBuilder for 
"omp parallel".

An `omp cancel parallel` needs to be emitted by the OpenMPIRBuilder if
the `parallel` was emitted by the OpenMPIRBuilder. This patch makes
this possible. The cancel logic is shared with the cancel barriers.
Testing is done via unit tests and the clang cancel_codegen.cpp file
once D70290  lands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71948

Files:
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -99,6 +99,122 @@
   EXPECT_FALSE(verifyModule(*M));
 }
 
+TEST_F(OpenMPIRBuilderTest, CreateCancel) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+
+  BasicBlock *CBB = BasicBlock::Create(Ctx, "", F);
+  new UnreachableInst(Ctx, CBB);
+  auto FiniCB = [&](InsertPointTy IP) {
+ASSERT_NE(IP.getBlock(), nullptr);
+ASSERT_EQ(IP.getBlock()->end(), IP.getPoint());
+BranchInst::Create(CBB, IP.getBlock());
+  };
+  OMPBuilder.pushFinalizationCB({FiniCB, OMPD_parallel, true});
+
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP()});
+  auto NewIP = OMPBuilder.CreateCancel(Loc, nullptr, OMPD_parallel);
+  Builder.restoreIP(NewIP);
+  EXPECT_FALSE(M->global_empty());
+  EXPECT_EQ(M->size(), 3U);
+  EXPECT_EQ(F->size(), 4U);
+  EXPECT_EQ(BB->size(), 4U);
+
+  CallInst *GTID = dyn_cast(&BB->front());
+  EXPECT_NE(GTID, nullptr);
+  EXPECT_EQ(GTID->getNumArgOperands(), 1U);
+  EXPECT_EQ(GTID->getCalledFunction()->getName(), "__kmpc_global_thread_num");
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotAccessMemory());
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotFreeMemory());
+
+  CallInst *Cancel = dyn_cast(GTID->getNextNode());
+  EXPECT_NE(Cancel, nullptr);
+  EXPECT_EQ(Cancel->getNumArgOperands(), 3U);
+  EXPECT_EQ(Cancel->getCalledFunction()->getName(), "__kmpc_cancel");
+  EXPECT_FALSE(Cancel->getCalledFunction()->doesNotAccessMemory());
+  EXPECT_FALSE(Cancel->getCalledFunction()->doesNotFreeMemory());
+  EXPECT_EQ(Cancel->getNumUses(), 1U);
+  Instruction *CancelBBTI = Cancel->getParent()->getTerminator();
+  EXPECT_EQ(CancelBBTI->getNumSuccessors(), 2U);
+  EXPECT_EQ(CancelBBTI->getSuccessor(0), NewIP.getBlock());
+  EXPECT_EQ(CancelBBTI->getSuccessor(1)->size(), 1U);
+  EXPECT_EQ(CancelBBTI->getSuccessor(1)->getTerminator()->getNumSuccessors(),
+1U);
+  EXPECT_EQ(CancelBBTI->getSuccessor(1)->getTerminator()->getSuccessor(0),
+CBB);
+
+  EXPECT_EQ(cast(Cancel)->getArgOperand(1), GTID);
+
+  OMPBuilder.popFinalizationCB();
+
+  Builder.CreateUnreachable();
+  EXPECT_FALSE(verifyModule(*M));
+}
+
+TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+
+  BasicBlock *CBB = BasicBlock::Create(Ctx, "", F);
+  new UnreachableInst(Ctx, CBB);
+  auto FiniCB = [&](InsertPointTy IP) {
+ASSERT_NE(IP.getBlock(), nullptr);
+ASSERT_EQ(IP.getBlock()->end(), IP.getPoint());
+BranchInst::Create(CBB, IP.getBlock());
+  };
+  OMPBuilder.pushFinalizationCB({FiniCB, OMPD_parallel, true});
+
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP()});
+  auto NewIP = OMPBuilder.CreateCancel(Loc, Builder.getTrue(), OMPD_parallel);
+  Builder.restoreIP(NewIP);
+  EXPECT_FALSE(M->global_empty());
+  EXPECT_EQ(M->size(), 3U);
+  EXPECT_EQ(F->size(), 7U);
+  EXPECT_EQ(BB->size(), 1U);
+  ASSERT_TRUE(isa(BB->getTerminator()));
+  ASSERT_EQ(BB->getTerminator()->getNumSuccessors(), 2U);
+  BB = BB->getTerminator()->getSuccessor(0);
+  EXPECT_EQ(BB->size(), 4U);
+
+
+  CallInst *GTID = dyn_cast(&BB->front());
+  EXPECT_NE(GTID, nullptr);
+  EXPECT_EQ(GTID->getNumArgOperands(), 1U);
+  EXPECT_EQ(GTID->getCalledFunction()->getName(), "__kmpc_global_thread_num");
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotAccessMemory());
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotFreeMemory());
+
+  CallInst *Cancel = dyn_cast(GTID->getNextNode());
+  EXPECT_NE(Cancel, nullptr);
+  EXPECT_EQ(Cancel->getNumArgOperands(), 3U);
+  EXPECT_EQ(Cancel->getCalledFunction()->getName(), "__kmpc_cancel");
+  EXPECT_FALSE(Cancel->getCalledFunction()->doesNotAccessMemory());
+  EXPECT_FAL

[PATCH] D70290: [OpenMP] Use the OpenMPIRBuilder for "omp parallel"

2019-12-27 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert updated this revision to Diff 235449.
jdoerfert added a comment.

Rebased on D71948  and combined with 
OpenMPIRBuilder cancel code gen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70290

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -376,6 +376,10 @@
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
   EXPECT_FALSE(verifyModule(*M));
+  EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
+  EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
+  EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
+  EXPECT_TRUE(OutlinedFn->hasParamAttribute(1, Attribute::NoAlias));
 
   EXPECT_TRUE(OutlinedFn->hasInternalLinkage());
   EXPECT_EQ(OutlinedFn->arg_size(), 3U);
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
@@ -510,10 +511,21 @@
   dbgs() << " PBR: " << BB->getName() << "\n";
   });
 
+  // Add some known attributes to the outlined function.
   Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);
+  OutlinedFn->addParamAttr(0, Attribute::NoAlias);
+  OutlinedFn->addParamAttr(1, Attribute::NoAlias);
+  OutlinedFn->addFnAttr(Attribute::NoUnwind);
+  OutlinedFn->addFnAttr(Attribute::NoRecurse);
+
   LLVM_DEBUG(dbgs() << "After  outlining: " << *UI->getFunction() << "\n");
   LLVM_DEBUG(dbgs() << "   Outlined function: " << *OutlinedFn << "\n");
 
+  // For compability with the clang CG we move the outlined function after the
+  // one with the parallel region.
+  OutlinedFn->removeFromParent();
+  M.getFunctionList().insertAfter(OuterFn->getIterator(), OutlinedFn);
+
   // Remove the artificial entry introduced by the extractor right away, we
   // made our own entry block after all.
   {
@@ -544,6 +556,23 @@
   RealArgs.append(CI->arg_begin() + /* tid & bound tid */ 2, CI->arg_end());
 
   FunctionCallee RTLFn = getOrCreateRuntimeFunction(OMPRTL___kmpc_fork_call);
+  if (auto *F = dyn_cast(RTLFn.getCallee())) {
+if (!F->hasMetadata(llvm::LLVMContext::MD_callback)) {
+  llvm::LLVMContext &Ctx = F->getContext();
+  MDBuilder MDB(Ctx);
+  // Annotate the callback behavior of the __kmpc_fork_call:
+  //  - The callback callee is argument number 2 (microtask).
+  //  - The first two arguments of the callback callee are unknown (-1).
+  //  - All variadic arguments to the __kmpc_fork_call are passed to the
+  //callback callee.
+  F->addMetadata(
+  llvm::LLVMContext::MD_callback,
+  *llvm::MDNode::get(Ctx, {MDB.createCallbackEncoding(
+  2, {-1, -1},
+  /* VarArgsArePassed */ true)}));
+}
+  }
+
   Builder.CreateCall(RTLFn, RealArgs);
 
   LLVM_DEBUG(dbgs() << "With fork_call placed: "
Index: llvm/lib/Frontend/OpenMP/OMPConstants.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPConstants.cpp
+++ llvm/lib/Frontend/OpenMP/OMPConstants.cpp
@@ -65,7 +65,7 @@
 #define OMP_TYPE(VarName, InitValue) VarName = InitValue;
 #define OMP_FUNCTION_TYPE(VarName, IsVarArg, ReturnType, ...)  \
   VarName = FunctionType::get(ReturnType, {__VA_ARGS__}, IsVarArg);\
-  VarName##Ptr = PointerType::getUnqual(T);
+  VarName##Ptr = PointerType::getUnqual(VarName);
 #define OMP_STRUCT_TYPE(VarName, StructName, ...)  \
   T = M.getTypeByName(StructName); \
   if (!T)  \
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -204,6 +204,9 @@
 #define __OMP_RTL_ATTRS(Name, FnAttrSet, RetAttrSet, ArgAttrSets)  \
   OMP_RTL_ATTRS(OMPRTL_##Name, FnAttrSet, RetAttrSet, ArgAttrSets)
 
+__OMP_RTL_ATTRS(__kmpc_fork_call, AttributeS

[PATCH] D70290: [OpenMP] Use the OpenMPIRBuilder for "omp parallel"

2019-12-27 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 console-log.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70290



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


[PATCH] D71948: [OpenMP] Use the OpenMPIRBuilder for `cancel` directives

2019-12-27 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 61126 tests passed, 1 failed 
and 728 were skipped.

  failed: 
libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_class/try_lock.pass.cpp

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71948



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


[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska requested changes to this revision.
Bouska added a comment.
This revision now requires changes to proceed.

So, you are trying to fix the issue at the wrong place. Contrary from what we 
should expect from a UnwrappedLine, BWACS_Multiline works by always wrapping 
the brace in UnwrappedLineParser, and afterwards in UnwrappedLineFormatter 
merging it back to the control statement if it's not in multi-line. The bug is 
on the control statement @ line 308 on UnwrappedLineFormatter.cpp 
.
 That's the block that merge the brace back to its control statement if it is 
not a multiline and it's executed only with a line with a else or a catch that 
begins with right curly brace.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71939



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


Re: [clang] d801823 - Revert "CWG2352: Allow qualification conversions during reference binding."

2019-12-27 Thread Richard Smith via cfe-commits
On Fri, 27 Dec 2019 at 12:27, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: David Blaikie
> Date: 2019-12-27T12:27:20-08:00
> New Revision: d8018233d1ea4234de68d5b4593abd773db79484
>
> URL:
> https://github.com/llvm/llvm-project/commit/d8018233d1ea4234de68d5b4593abd773db79484
> DIFF:
> https://github.com/llvm/llvm-project/commit/d8018233d1ea4234de68d5b4593abd773db79484.diff
>
> LOG: Revert "CWG2352: Allow qualification conversions during reference
> binding."
>
> This reverts commit de21704ba96fa80d3e9402f12c6505917a3885f4.
>
> Regressed/causes this to error due to ambiguity:
>
>   void f(const int * const &);
>   void f(int *);
>   int main() {
> int * x;
> f(x);
>   }
>
> (in case it's important - the original case where this turned up was a
> member function overload in a class template with, essentially:
>
>   f(const T1&)
>   f(T2*)
>
> (where T1 == X const *, T2 == X))
>
> It's not super clear to me if this ^ is expected behavior, in which case
> I'm sorry about the revert & happy to look into ways to fix the original
> code.
>

I believe the new Clang behavior here is correct according to the standard
wording. However, GCC trunk also implements CWG2352 and accepts this, and
there's a natural-seeming way to get that result, so I'm going to take this
back to the core reflector and see if people agree that this ought to
remain valid.


> Added:
>
>
> Modified:
> clang/include/clang/Basic/DiagnosticSemaKinds.td
> clang/lib/Sema/SemaExprCXX.cpp
> clang/lib/Sema/SemaInit.cpp
> clang/lib/Sema/SemaOverload.cpp
> clang/test/CXX/drs/dr23xx.cpp
> clang/test/CXX/drs/dr4xx.cpp
> clang/test/SemaObjCXX/arc-overloading.mm
> clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
> clang/www/cxx_dr_status.html
> clang/www/make_cxx_dr_status
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
> b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> index b86abd0db73e..54299a0409fd 100644
> --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1933,8 +1933,7 @@ def err_lvalue_reference_bind_to_unrelated : Error<
>"cannot bind to a value of unrelated type}1,2">;
>  def err_reference_bind_drops_quals : Error<
>"binding reference %
> diff {of type $ to value of type $|to value}0,1 "
> -  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space|"
> -  "not permitted due to incompatible qualifiers}2">;
> +  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address
> space}2">;
>  def err_reference_bind_failed : Error<
>"reference %
> diff {to %select{type|incomplete type}1 $ could not bind to an "
>"%select{rvalue|lvalue}2 of type $|could not bind to
> %select{rvalue|lvalue}2 of "
>
> diff  --git a/clang/lib/Sema/SemaExprCXX.cpp
> b/clang/lib/Sema/SemaExprCXX.cpp
> index cd78f096bb22..cfb3a05e9c14 100644
> --- a/clang/lib/Sema/SemaExprCXX.cpp
> +++ b/clang/lib/Sema/SemaExprCXX.cpp
> @@ -5864,8 +5864,6 @@ QualType
> Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
>//   one of the operands is reference-compatible with the other, in
> order
>//   to support conditionals between functions
> diff ering in noexcept. This
>//   will similarly cover
> diff erence in array bounds after P0388R4.
> -  // FIXME: If LTy and RTy have a composite pointer type, should we
> convert to
> -  //   that instead?
>ExprValueKind LVK = LHS.get()->getValueKind();
>ExprValueKind RVK = RHS.get()->getValueKind();
>if (!Context.hasSameType(LTy, RTy) &&
>
> diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
> index ef4fa827064e..94d524a63f5a 100644
> --- a/clang/lib/Sema/SemaInit.cpp
> +++ b/clang/lib/Sema/SemaInit.cpp
> @@ -8919,17 +8919,11 @@ bool InitializationSequence::Diagnose(Sema &S,
>S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
><< NonRefType << SourceType << 1 /*addr space*/
><< Args[0]->getSourceRange();
> -else if (DroppedQualifiers.hasQualifiers())
> +else
>S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
><< NonRefType << SourceType << 0 /*cv quals*/
><< Qualifiers::fromCVRMask(DroppedQualifiers.getCVRQualifiers())
><< DroppedQualifiers.getCVRQualifiers() <<
> Args[0]->getSourceRange();
> -else
> -  // FIXME: Consider decomposing the type and explaining which
> qualifiers
> -  // were dropped where, or on which level a 'const' is missing, etc.
> -  S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
> -  << NonRefType << SourceType << 2 /*incompatible quals*/
> -  << Args[0]->getSourceRange();
>  break;
>}
>
>
> diff  --git a/clang/lib/Sema/SemaOverload.cpp
> b/clang/lib/Sema/SemaOverload.cpp
> index 92058c3aa5

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment.

Is there a reason why this option adds space before and after the tok::arrow 
token but not the tok::arrowstar token? That seems inconsistent to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71659



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


Re: [clang] d801823 - Revert "CWG2352: Allow qualification conversions during reference binding."

2019-12-27 Thread David Blaikie via cfe-commits
On Fri, Dec 27, 2019 at 3:48 PM Richard Smith  wrote:

> On Fri, 27 Dec 2019 at 12:27, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: David Blaikie
>> Date: 2019-12-27T12:27:20-08:00
>> New Revision: d8018233d1ea4234de68d5b4593abd773db79484
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/d8018233d1ea4234de68d5b4593abd773db79484
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/d8018233d1ea4234de68d5b4593abd773db79484.diff
>>
>> LOG: Revert "CWG2352: Allow qualification conversions during reference
>> binding."
>>
>> This reverts commit de21704ba96fa80d3e9402f12c6505917a3885f4.
>>
>> Regressed/causes this to error due to ambiguity:
>>
>>   void f(const int * const &);
>>   void f(int *);
>>   int main() {
>> int * x;
>> f(x);
>>   }
>>
>> (in case it's important - the original case where this turned up was a
>> member function overload in a class template with, essentially:
>>
>>   f(const T1&)
>>   f(T2*)
>>
>> (where T1 == X const *, T2 == X))
>>
>> It's not super clear to me if this ^ is expected behavior, in which case
>> I'm sorry about the revert & happy to look into ways to fix the original
>> code.
>>
>
> I believe the new Clang behavior here is correct according to the standard
> wording.
>

Ah, OK. Thanks for looking/checking!


> However, GCC trunk also implements CWG2352 and accepts this,
>

Right, I should've mentioned I checked against GCC trunk. The other case
that showed up with this change:

template
void f(T&&);
void f() {
  int *i;
  f(i);
}

that one failed with both GCC and Clang (& I've written up a fix for it
internally) & /seems/ roughly like what I believe is to be correct behavior
form the compiler/new patch here (& consistent with GCC, giving it a bit
more a vote for "this seems like good behavior")


> and there's a natural-seeming way to get that result, so I'm going to take
> this back to the core reflector and see if people agree that this ought to
> remain valid.
>

OK - happy to help with internal fixes if/when that's needed (I think it
might just be the one, though - so maybe that goes to show this isn't the
worst behavior)


>
>
>> Added:
>>
>>
>> Modified:
>> clang/include/clang/Basic/DiagnosticSemaKinds.td
>> clang/lib/Sema/SemaExprCXX.cpp
>> clang/lib/Sema/SemaInit.cpp
>> clang/lib/Sema/SemaOverload.cpp
>> clang/test/CXX/drs/dr23xx.cpp
>> clang/test/CXX/drs/dr4xx.cpp
>> clang/test/SemaObjCXX/arc-overloading.mm
>> clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
>> clang/www/cxx_dr_status.html
>> clang/www/make_cxx_dr_status
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
>> b/clang/include/clang/Basic/DiagnosticSemaKinds.td
>> index b86abd0db73e..54299a0409fd 100644
>> --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
>> +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
>> @@ -1933,8 +1933,7 @@ def err_lvalue_reference_bind_to_unrelated : Error<
>>"cannot bind to a value of unrelated type}1,2">;
>>  def err_reference_bind_drops_quals : Error<
>>"binding reference %
>> diff {of type $ to value of type $|to value}0,1 "
>> -  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address
>> space|"
>> -  "not permitted due to incompatible qualifiers}2">;
>> +  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address
>> space}2">;
>>  def err_reference_bind_failed : Error<
>>"reference %
>> diff {to %select{type|incomplete type}1 $ could not bind to an "
>>"%select{rvalue|lvalue}2 of type $|could not bind to
>> %select{rvalue|lvalue}2 of "
>>
>> diff  --git a/clang/lib/Sema/SemaExprCXX.cpp
>> b/clang/lib/Sema/SemaExprCXX.cpp
>> index cd78f096bb22..cfb3a05e9c14 100644
>> --- a/clang/lib/Sema/SemaExprCXX.cpp
>> +++ b/clang/lib/Sema/SemaExprCXX.cpp
>> @@ -5864,8 +5864,6 @@ QualType
>> Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
>>//   one of the operands is reference-compatible with the other, in
>> order
>>//   to support conditionals between functions
>> diff ering in noexcept. This
>>//   will similarly cover
>> diff erence in array bounds after P0388R4.
>> -  // FIXME: If LTy and RTy have a composite pointer type, should we
>> convert to
>> -  //   that instead?
>>ExprValueKind LVK = LHS.get()->getValueKind();
>>ExprValueKind RVK = RHS.get()->getValueKind();
>>if (!Context.hasSameType(LTy, RTy) &&
>>
>> diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
>> index ef4fa827064e..94d524a63f5a 100644
>> --- a/clang/lib/Sema/SemaInit.cpp
>> +++ b/clang/lib/Sema/SemaInit.cpp
>> @@ -8919,17 +8919,11 @@ bool InitializationSequence::Diagnose(Sema &S,
>>S.Diag(Kind.getLocation(), diag::err_reference_bind_drops_quals)
>><< NonRefType << SourceType << 1 /*addr space*/
>><< Args[0]->getSourceRange();
>> -else if

[PATCH] D71953: [Tooling] Infer driver mode and target for FixedCompilationDatabase

2019-12-27 Thread Hanjiang Yu via Phabricator via cfe-commits
de1acr0ix created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When creating FixedCompilationDatabase from command line, compiler
command is often provided. This commit tries to infer driver mode and
target from that so that clang tools does not need to specify them
explicitly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71953

Files:
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -641,6 +641,43 @@
   EXPECT_EQ(2, Argc);
 }
 
+TEST(ParseFixedCompilationDatabase, InferDriverMode) {
+  const char *Argv[] = {"1", "2", "--", "cl.exe", "-nologo", "somefile.cpp"};
+  int Argc = sizeof(Argv) / sizeof(char *);
+  std::string ErrorMsg;
+  std::unique_ptr Database =
+  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
+  ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
+  std::vector Result = Database->getCompileCommands("source");
+  ASSERT_EQ(1ul, Result.size());
+  ASSERT_EQ(".", Result[0].Directory);
+  std::vector Expected;
+  ASSERT_THAT(Result[0].CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "--driver-mode=cl", 
"-nologo",
+  "source"));
+  EXPECT_EQ(2, Argc);
+}
+
+TEST(ParseFixedCompilationDatabase, InferTarget) {
+  const char *Argv[] = {"1", "2", "--", "x86_64-linux-gnu-gcc", 
"somefile.cpp"};
+  int Argc = sizeof(Argv) / sizeof(char *);
+  std::string ErrorMsg;
+  LLVMInitializeX86TargetInfo();
+  std::unique_ptr Database =
+  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
+  ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
+  std::vector Result = Database->getCompileCommands("source");
+  ASSERT_EQ(1ul, Result.size());
+  ASSERT_EQ(".", Result[0].Directory);
+  std::vector Expected;
+  ASSERT_THAT(Result[0].CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-target", 
"x86_64-linux-gnu",
+  "source"));
+  EXPECT_EQ(2, Argc);
+}
+
 struct MemCDB : public CompilationDatabase {
   using EntryMap = llvm::StringMap>;
   EntryMap Entries;
Index: clang/lib/Tooling/CompilationDatabase.cpp
===
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -275,6 +275,20 @@
   Diagnostics));
   NewDriver->setCheckInputsExist(false);
 
+  // Try to infer driver mode and target from the original argv[0].
+  driver::ParsedClangName NameParts;
+  if (!Args.empty()) {
+NameParts = driver::ToolChain::getTargetAndModeFromProgramName(Args[0]);
+if (NameParts.DriverMode) {
+  Args.insert(Args.begin(), NameParts.DriverMode);
+}
+
+if (NameParts.TargetIsValid) {
+  const char *arr[] = {"-target", NameParts.TargetPrefix.c_str()};
+  Args.insert(Args.begin(), std::begin(arr), std::end(arr));
+}
+  }
+
   // This becomes the new argv[0]. The value is used to detect libc++ include
   // dirs on Mac, it isn't used for other platforms.
   std::string Argv0 = GetClangToolCommand();


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -641,6 +641,43 @@
   EXPECT_EQ(2, Argc);
 }
 
+TEST(ParseFixedCompilationDatabase, InferDriverMode) {
+  const char *Argv[] = {"1", "2", "--", "cl.exe", "-nologo", "somefile.cpp"};
+  int Argc = sizeof(Argv) / sizeof(char *);
+  std::string ErrorMsg;
+  std::unique_ptr Database =
+  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
+  ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
+  std::vector Result = Database->getCompileCommands("source");
+  ASSERT_EQ(1ul, Result.size());
+  ASSERT_EQ(".", Result[0].Directory);
+  std::vector Expected;
+  ASSERT_THAT(Result[0].CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "--driver-mode=cl", "-nologo",
+  "source"));
+  EXPECT_EQ(2, Argc);
+}
+
+TEST(ParseFixedCompilationDatabase, InferTarget) {
+  const char *Argv[] = {"1", "2", "--", "x86_64-linux-gnu-gcc", "somefile.cpp"};
+  int Argc = sizeof(Argv) / sizeof(char *);
+  std::string ErrorMsg;
+  LLVMInitializeX86TargetInfo();
+  std::unique_ptr Database =
+  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
+  ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
+  std::vector Result = Database->getCompileCommands("source");
+  ASSERT_EQ(1ul, Result.size());
+  ASSERT_EQ(".", Result[0].Directory);
+  std::vector Expected;
+  ASSERT_THAT(Result[0].CommandLine,
+  E

[PATCH] D71953: [Tooling] Infer driver mode and target for FixedCompilationDatabase

2019-12-27 Thread Hanjiang Yu via Phabricator via cfe-commits
de1acr0ix planned changes to this revision.
de1acr0ix added a comment.

I realized an obvious problem: if the first argument is "-cc" or something 
compiler option ends with a possible driver mode suffix it would be treated as 
compiler binary name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71953



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


[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2019-12-27 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas marked an inline comment as done.
ztamas added inline comments.
Herald added a subscriber: whisperity.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:43-44
+
+  const auto SignedCharType = expr(hasType(qualType(
+  allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef);
+

aaron.ballman wrote:
> Does this properly handle architectures where `char` is signed rather than 
> unsigned? Or does this only handle the case where the user wrote `signed 
> char`?
It catches plain char too. I tested on 64 bit linux where char is signed by 
default.
The funsigned-char and fsigned-char options can be used to override the default 
behavior. Added some new test cases and also extended the documentation to make 
this clear.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:43-44
+
+  const auto SignedCharType = expr(hasType(qualType(
+  allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef);
+

ztamas wrote:
> aaron.ballman wrote:
> > Does this properly handle architectures where `char` is signed rather than 
> > unsigned? Or does this only handle the case where the user wrote `signed 
> > char`?
> It catches plain char too. I tested on 64 bit linux where char is signed by 
> default.
> The funsigned-char and fsigned-char options can be used to override the 
> default behavior. Added some new test cases and also extended the 
> documentation to make this clear.
Most of the catches I found in LibreOffice / LLVM code had `char` type.
I found isSignedCharDefault() method in LLVM code where clang handles platform 
differences, as I see, so I assume clang-tidy works based on that. With the 
compilation options, the char signedness can be controlled anyway, so I think 
this will be OK.
I could test only with a 64 bit linux. There plain char is caught by default, 
as I mentioned, and I could change this behavior with the -funsigned-char 
option.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:25
+See also:
+`STR34-C. Cast characters to unsigned char before converting to larger integer 
sizes
+`_

aaron.ballman wrote:
> Should this check also be registered in the CERT module?
This check does not cover the whole CERT description. I guess a cert check 
should catch all bugous code related to the CERT issue, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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


[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2019-12-27 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

I added the above comments two weeks ago, I just forget to push the submit 
button.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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


[clang-tools-extra] 14e1100 - [clangd] Fix crash in hover

2019-12-27 Thread Ilya Biryukov via cfe-commits

Author: Ilya Biryukov
Date: 2019-12-27T09:15:15+01:00
New Revision: 14e11005d1a6ac1fecb230c470e9011d6956b8e4

URL: 
https://github.com/llvm/llvm-project/commit/14e11005d1a6ac1fecb230c470e9011d6956b8e4
DIFF: 
https://github.com/llvm/llvm-project/commit/14e11005d1a6ac1fecb230c470e9011d6956b8e4.diff

LOG: [clangd] Fix crash in hover

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 94a532b1ac47..95ebe08a9281 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -195,7 +195,7 @@ const NamedDecl *getDeclForComment(const NamedDecl *D) {
   return VTSD->getTemplateInstantiationPattern();
   if (auto *FD = D->getAsFunction())
 if (FD->isTemplateInstantiation())
-  return FD->getTemplateSpecializationInfo()->getTemplate();
+  return FD->getTemplateInstantiationPattern();
   return D;
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 0c71d76e3c03..2ed9a72ca138 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -619,24 +619,24 @@ TEST(Hover, All) {
 const char *const Code;
 const std::function ExpectedBuilder;
   } Cases[] = {
-  {
-  R"cpp(// Local variable
+{
+R"cpp(// Local variable
 int main() {
   int bonjour;
   ^[[bonjour]] = 2;
   int test1 = bonjour;
 }
   )cpp",
-  [](HoverInfo &HI) {
-HI.Name = "bonjour";
-HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "";
-HI.LocalScope = "main::";
-HI.Type = "int";
-HI.Definition = "int bonjour";
-  }},
-  {
-  R"cpp(// Local variable in method
+[](HoverInfo &HI) {
+  HI.Name = "bonjour";
+  HI.Kind = index::SymbolKind::Variable;
+  HI.NamespaceScope = "";
+  HI.LocalScope = "main::";
+  HI.Type = "int";
+  HI.Definition = "int bonjour";
+}},
+{
+R"cpp(// Local variable in method
 struct s {
   void method() {
 int bonjour;
@@ -644,16 +644,16 @@ TEST(Hover, All) {
   }
 };
   )cpp",
-  [](HoverInfo &HI) {
-HI.Name = "bonjour";
-HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "";
-HI.LocalScope = "s::method::";
-HI.Type = "int";
-HI.Definition = "int bonjour";
-  }},
-  {
-  R"cpp(// Struct
+[](HoverInfo &HI) {
+  HI.Name = "bonjour";
+  HI.Kind = index::SymbolKind::Variable;
+  HI.NamespaceScope = "";
+  HI.LocalScope = "s::method::";
+  HI.Type = "int";
+  HI.Definition = "int bonjour";
+}},
+{
+R"cpp(// Struct
 namespace ns1 {
   struct MyClass {};
 } // namespace ns1
@@ -661,14 +661,14 @@ TEST(Hover, All) {
   ns1::[[My^Class]]* Params;
 }
   )cpp",
-  [](HoverInfo &HI) {
-HI.Name = "MyClass";
-HI.Kind = index::SymbolKind::Struct;
-HI.NamespaceScope = "ns1::";
-HI.Definition = "struct MyClass {}";
-  }},
-  {
-  R"cpp(// Class
+[](HoverInfo &HI) {
+  HI.Name = "MyClass";
+  HI.Kind = index::SymbolKind::Struct;
+  HI.NamespaceScope = "ns1::";
+  HI.Definition = "struct MyClass {}";
+}},
+{
+R"cpp(// Class
 namespace ns1 {
   class MyClass {};
 } // namespace ns1
@@ -676,14 +676,14 @@ TEST(Hover, All) {
   ns1::[[My^Class]]* Params;
 }
   )cpp",
-  [](HoverInfo &HI) {
-HI.Name = "MyClass";
-HI.Kind = index::SymbolKind::Class;
-HI.NamespaceScope = "ns1::";
-HI.Definition = "class MyClass {}";
-  }},
-  {
-  R"cpp(// Union
+[](HoverInfo &HI) {
+  HI.Name = "MyClass";
+  HI.Kind = index::SymbolKind::Class;
+  HI.NamespaceScope = "ns1::";
+  HI.Definition = "class MyClass {}";
+}},
+{
+R"cpp(// Union
 namespace ns1 {
   union MyUnion { int x; int y; };
 } // namespace ns1
@@ -691,223 +691,223 @@ TEST(Hover, All) {
   ns1::[[My^Union]] Params;
 }
   )cpp",
-  [](HoverInfo &HI) {
-HI.Name = "MyUnion";
-HI.Kind = index::SymbolKind::Union;
-HI.NamespaceScope = "ns1::";
-  

[clang-tools-extra] e2d9f4e - [clangd] Reformat `HoverTests.cpp` NFC

2019-12-27 Thread Ilya Biryukov via cfe-commits

Author: Ilya Biryukov
Date: 2019-12-27T09:35:46+01:00
New Revision: e2d9f4e6a284992388a82df388e36f6491b9ec66

URL: 
https://github.com/llvm/llvm-project/commit/e2d9f4e6a284992388a82df388e36f6491b9ec66
DIFF: 
https://github.com/llvm/llvm-project/commit/e2d9f4e6a284992388a82df388e36f6491b9ec66.diff

LOG: [clangd] Reformat `HoverTests.cpp` NFC

I accidentally broke formatting in the previous revision.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 2ed9a72ca138..ba8f773fdf83 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -619,24 +619,24 @@ TEST(Hover, All) {
 const char *const Code;
 const std::function ExpectedBuilder;
   } Cases[] = {
-{
-R"cpp(// Local variable
+  {
+  R"cpp(// Local variable
 int main() {
   int bonjour;
   ^[[bonjour]] = 2;
   int test1 = bonjour;
 }
   )cpp",
-[](HoverInfo &HI) {
-  HI.Name = "bonjour";
-  HI.Kind = index::SymbolKind::Variable;
-  HI.NamespaceScope = "";
-  HI.LocalScope = "main::";
-  HI.Type = "int";
-  HI.Definition = "int bonjour";
-}},
-{
-R"cpp(// Local variable in method
+  [](HoverInfo &HI) {
+HI.Name = "bonjour";
+HI.Kind = index::SymbolKind::Variable;
+HI.NamespaceScope = "";
+HI.LocalScope = "main::";
+HI.Type = "int";
+HI.Definition = "int bonjour";
+  }},
+  {
+  R"cpp(// Local variable in method
 struct s {
   void method() {
 int bonjour;
@@ -644,16 +644,16 @@ TEST(Hover, All) {
   }
 };
   )cpp",
-[](HoverInfo &HI) {
-  HI.Name = "bonjour";
-  HI.Kind = index::SymbolKind::Variable;
-  HI.NamespaceScope = "";
-  HI.LocalScope = "s::method::";
-  HI.Type = "int";
-  HI.Definition = "int bonjour";
-}},
-{
-R"cpp(// Struct
+  [](HoverInfo &HI) {
+HI.Name = "bonjour";
+HI.Kind = index::SymbolKind::Variable;
+HI.NamespaceScope = "";
+HI.LocalScope = "s::method::";
+HI.Type = "int";
+HI.Definition = "int bonjour";
+  }},
+  {
+  R"cpp(// Struct
 namespace ns1 {
   struct MyClass {};
 } // namespace ns1
@@ -661,14 +661,14 @@ TEST(Hover, All) {
   ns1::[[My^Class]]* Params;
 }
   )cpp",
-[](HoverInfo &HI) {
-  HI.Name = "MyClass";
-  HI.Kind = index::SymbolKind::Struct;
-  HI.NamespaceScope = "ns1::";
-  HI.Definition = "struct MyClass {}";
-}},
-{
-R"cpp(// Class
+  [](HoverInfo &HI) {
+HI.Name = "MyClass";
+HI.Kind = index::SymbolKind::Struct;
+HI.NamespaceScope = "ns1::";
+HI.Definition = "struct MyClass {}";
+  }},
+  {
+  R"cpp(// Class
 namespace ns1 {
   class MyClass {};
 } // namespace ns1
@@ -676,14 +676,14 @@ TEST(Hover, All) {
   ns1::[[My^Class]]* Params;
 }
   )cpp",
-[](HoverInfo &HI) {
-  HI.Name = "MyClass";
-  HI.Kind = index::SymbolKind::Class;
-  HI.NamespaceScope = "ns1::";
-  HI.Definition = "class MyClass {}";
-}},
-{
-R"cpp(// Union
+  [](HoverInfo &HI) {
+HI.Name = "MyClass";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns1::";
+HI.Definition = "class MyClass {}";
+  }},
+  {
+  R"cpp(// Union
 namespace ns1 {
   union MyUnion { int x; int y; };
 } // namespace ns1
@@ -691,223 +691,223 @@ TEST(Hover, All) {
   ns1::[[My^Union]] Params;
 }
   )cpp",
-[](HoverInfo &HI) {
-  HI.Name = "MyUnion";
-  HI.Kind = index::SymbolKind::Union;
-  HI.NamespaceScope = "ns1::";
-  HI.Definition = "union MyUnion {}";
-}},
-{
-R"cpp(// Function definition via pointer
+  [](HoverInfo &HI) {
+HI.Name = "MyUnion";
+HI.Kind = index::SymbolKind::Union;
+HI.NamespaceScope = "ns1::";
+HI.Definition = "union MyUnion {}";
+  }},
+  {
+  R"cpp(// Function definition via pointer
 void foo(int) {}
 int main() {
   auto *X = &^[[foo]];
 }
   )cpp",
-[](Ho

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2019-12-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: rsmith.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added a project: clang.

This changes introduces an enum to represent dependencies as a bitmask
and extract common patterns from code that computes dependency bits into
helper functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71920

Files:
  clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  clang/include/clang/AST/DependencyFlags.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp

Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -511,10 +511,23 @@
 void ASTStmtReader::VisitExpr(Expr *E) {
   VisitStmt(E);
   E->setType(Record.readType());
-  E->setTypeDependent(Record.readInt());
-  E->setValueDependent(Record.readInt());
-  E->setInstantiationDependent(Record.readInt());
-  E->ExprBits.ContainsUnexpandedParameterPack = Record.readInt();
+
+  // FIXME: write and read all DependentFlags with a single call.
+  bool TypeDependent = Record.readInt();
+  bool ValueDependent = Record.readInt();
+  bool InstantiationDependent = Record.readInt();
+  bool ContainsUnexpandedTemplateParameters = Record.readInt();
+  DependencyFlags Deps = DependencyFlags::None;
+  if (TypeDependent)
+Deps = Deps | DependencyFlags::Type;
+  if (ValueDependent)
+Deps = Deps | DependencyFlags::Value;
+  if (InstantiationDependent)
+Deps = Deps | DependencyFlags::Instantiation;
+  if (ContainsUnexpandedTemplateParameters)
+Deps = Deps | DependencyFlags::UnexpandedPack;
+  E->setDependencies(Deps);
+
   E->setValueKind(static_cast(Record.readInt()));
   E->setObjectKind(static_cast(Record.readInt()));
   assert(Record.getIdx() == NumExprFields &&
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12420,9 +12420,7 @@
   // base classes.
   CallExpr *CE = CallExpr::Create(Context, Fn, Args, Context.DependentTy,
   VK_RValue, RParenLoc);
-  CE->setTypeDependent(true);
-  CE->setValueDependent(true);
-  CE->setInstantiationDependent(true);
+  CE->addDependencies(DependencyFlags::TypeValueInstantiation);
   *Result = CE;
   return true;
 }
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -111,84 +111,65 @@
   return TemplateArgument(Args.copy(Context));
 }
 
-bool TemplateArgument::isDependent() const {
+DependencyFlags TemplateArgument::getDependencies() const {
+  DependencyFlags Deps = DependencyFlags::None;
   switch (getKind()) {
   case Null:
 llvm_unreachable("Should not have a NULL template argument");
 
   case Type:
-return getAsType()->isDependentType() ||
-   isa(getAsType());
+Deps = getAsType()->getDependencies();
+if (isa(getAsType()))
+  Deps = Deps | DependencyFlags::Type;
+return Deps;
 
   case Template:
-return getAsTemplate().isDependent();
+// FIXME: add TemplateName::getDependencies.
+if (getAsTemplate().isDependent())
+  Deps = Deps | DependencyFlags::TypeValue;
+if (getAsTemplate().isInstantiationDependent())
+  Deps = Deps | DependencyFlags::Instantiation;
+if (getAsTemplate().containsUnexpandedParameterPack())
+  Deps = Deps | DependencyFlags::UnexpandedPack;
+return Deps;
 
   case TemplateExpansion:
-return true;
+return DependencyFlags::TypeValueInstantiation;
 
-  case Declaration:
-if (DeclContext *DC = dyn_cast(getAsDecl()))
-  return DC->isDependentContext();
-return getAsDecl()->getDeclContext()->isDependentContext();
+  case Declaration: {
+auto *DC = dyn_cast(getAsDecl());
+if (!DC)
+  DC = getAsDecl()->getDeclContext();
+if (DC->isDependentContext())
+  Deps = DependencyFlags::TypeValueInstantiation;
+return Deps;
+  }
 
   case NullPtr:
-return false;
-
   case Integral:
-// Never dependent
-return false;
+return DependencyFlags::None;
 
   case Expression:
-return (getAsExpr()->isTypeDependent() || getAsExpr()->isValueDependent() ||
-isa(getAsExpr()));
+Deps = getAsExpr()->getDependencies();
+if (isa(getAsExpr()))
+  Deps = Deps | DependencyFlags::TypeValueInstantiation;
+return Deps;
 
   case Pack:
 for (const auto &P : pack_elements())
-