[PATCH] D33440: clang-format: better handle statement macros

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments.



Comment at: lib/Format/FormatTokenLexer.cpp:642
   tok::pp_define) &&
-std::find(ForEachMacros.begin(), ForEachMacros.end(),
-  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
-  FormatTok->Type = TT_ForEachMacro;
+(it = std::find(Macros.begin(), Macros.end(),
+FormatTok->Tok.getIdentifierInfo())) != Macros.end()) {

djasper wrote:
> This does a binary search. Why aren't you implementing it with a hashtable?
It was already done this way, so I did not change it to avoid any impact on 
performance.
But I can change it if you prefer.


https://reviews.llvm.org/D33440



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


[PATCH] D33440: clang-format: better handle statement macros

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 115801.
Typz marked 5 inline comments as done.
Typz added a comment.

Add tests.
Replace sorted list with hashtable.


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2385,6 +2385,42 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10185,6 +10221,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeCategories.clear();
   std::vector ExpectedCategories = {{"abc/.*", 2},
   {".*", 1}};
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1089,6 +1089,15 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1238,6 +1247,16 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+nextToken();
+if (FormatTok->is(tok::l_paren))
+  parseParens();
+if (FormatTok->is(tok::semi))
+  nextToken();
+addUnwrappedLine();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include 
 
 #include 
 
@@ -97,7 +98,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -37,8 +37,9 @@
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string &ForEachMacro : Style.ForEachMacros)
-ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
-  std::sort(ForEachMacros.begin(), ForEachMacros.end());
+Macros.insert({&IdentTable.get(ForEachMacro), TT_ForEachMacro});
+  for (const std::string &StatementMacro : Style.StatementMacros)
+Macros.insert({&IdentTable.get(StatementMacro), TT_StatementMacro});
 }
 
 ArrayRef FormatTokenLexer::lex() {
@@ -633,12 +634,13 @@
   }
 
   if (Style.isCpp()) {
+decltype(Macros)::iterator it;
 if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
-std::find(ForEachMacros.begin(), ForEachMacros.end(),
-   

[PATCH] D37813: clang-format: better handle namespace macros

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 115802.
Typz added a comment.

rebase


https://reviews.llvm.org/D37813

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"} /* end of TESTSUITE(A) */",
+

Re: r310983 - PR19668, PR23034: Fix handling of move constructors and deleted copy

2017-09-19 Thread Malcolm Parsons via cfe-commits
On 16 August 2017 at 02:49, Richard Smith via cfe-commits
 wrote:

> +  /// \brief \c true if a defaulted destructor for this class would be 
> deleted.
> +  bool defaultedDestructorIsDeleted() const {
> +return !data().DefaultedDestructorIsDeleted;
> +  }

Is the ! intentional?

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


[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings

2017-09-19 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

So we actually didn't need to change the UnwrappedLineParser? I assume we still 
incorrectly assume the structured bindings are labmdas then?


https://reviews.llvm.org/D35743



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


[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

There is one big missing thing here: `PointerAlignment`. Actually only 
`PAS_Left` is taken into account.
There are 3 possible options:
Left: `auto& [a, b] = f();`
Middle: `auto & [a, b] = f();`
Right: `auto &[a, b] = f();`


https://reviews.llvm.org/D35743



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


[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 115814.
curdeius added a comment.

Minor: use `FormatToken::isNot` instead of `!FormatToken::is`.


https://reviews.llvm.org/D37980

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1413,7 +1413,7 @@
   verifyFormat("typedef enum {} EmptyEnum;");
   verifyFormat("typedef enum { A, B, C } ShortEnum;");
   verifyFormat("typedef enum {\n"
-   "  ZERO = 0,\n" 
+   "  ZERO = 0,\n"
"  ONE = 1,\n"
"  TWO = 2,\n"
"  THREE = 3\n"
@@ -1425,7 +1425,7 @@
   verifyFormat("typedef enum { A, B, C } ShortEnum;");
   verifyFormat("typedef enum\n"
"{\n"
-   "  ZERO = 0,\n" 
+   "  ZERO = 0,\n"
"  ONE = 1,\n"
"  TWO = 2,\n"
"  THREE = 3\n"
@@ -1574,8 +1574,8 @@
   Style.CompactNamespaces = true;
 
   verifyFormat("namespace A { namespace B {\n"
-			   "}} // namespace A::B",
-			   Style);
+   "}} // namespace A::B",
+   Style);
 
   EXPECT_EQ("namespace out { namespace in {\n"
 "}} // namespace out::in",
@@ -3428,22 +3428,22 @@
   verifyFormat(
   "SomeClass::Constructor() :\n"
   "a(aa), aaa() {}",
-	  Style);
+  Style);
 
   verifyFormat(
   "SomeClass::Constructor() :\n"
   "a(aa), a(aa),\n"
   "a(aa) {}",
-	  Style);
+  Style);
   verifyFormat(
   "SomeClass::Constructor() :\n"
   "aa(aa),\n"
   "aaa() {}",
-	  Style);
+  Style);
   verifyFormat("Constructor(aa ,\n"
"aa ) :\n"
"aa(aa) {}",
-			   Style);
+   Style);
 
   verifyFormat("Constructor() :\n"
"(aaa),\n"
@@ -3450,17 +3450,17 @@
"(aaa,\n"
" aaa),\n"
"aaa() {}",
-			   Style);
+   Style);
 
   verifyFormat("Constructor() :\n"
"(\n"
"a) {}",
-			   Style);
+   Style);
 
   verifyFormat("Constructor(int Parameter = 0) :\n"
"aa(a),\n"
"(a) {}",
-			   Style);
+   Style);
   verifyFormat("Constructor() :\n"
"aa(a), (b) {\n"
"}",
@@ -3468,7 +3468,7 @@
   verifyFormat("Constructor() :\n"
"a(\n"
"a(, )) {}",
-			   Style);
+   Style);
 
   // Here a line could be saved by splitting the second initializer onto two
   // lines, but that is not desirable.
@@ -3476,7 +3476,7 @@
"(),\n"
"aaa(aaa),\n"
"at() {}",
-			   Style);
+   Style);
 
   FormatStyle OnePerLine = Style;
   OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
@@ -3526,7 +3526,7 @@
 format("Constructor() :\n"
"// Comment forcing unwanted break.\n"
"() {}",
-   Style));
+   Style));
 
   Style.ColumnLimit = 0;
   verifyFormat("SomeClass::Constructor() :\n"
@@ -3536,7 +3536,7 @@
"a(a) {}",
Style);
   verifyFormat("SomeClass::Constructor() :\n"
-			   "a(a), b(b), c(c) {}",
+   "a(a), b(b), c(c) {}",
Style);
   verifyFormat("SomeClass::Constructor() :\n"
"a(a) {\n"
@@ -3547,12 +3547,12 @@
 
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   verifyFormat("SomeClass::Constructor() :\n"
-			   "a(a), b(b), c(c) {\n"
-			   "}",
+   "a(a), b(b), c(c) {\n"
+   "}",
Style);
   verifyFormat("SomeClass::Constructor() :\n"
"a(a) {\n"
-			   "}",
+   "}",
Style);
 
   Style.ColumnLimit = 80;
@@ -6519,7 +6519,7 @@

r313622 - Fix formatting of lambda introducers with initializers.

2017-09-19 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Tue Sep 19 02:59:30 2017
New Revision: 313622

URL: http://llvm.org/viewvc/llvm-project?rev=313622&view=rev
Log:
Fix formatting of lambda introducers with initializers.

Most of the work was already done when we introduced a look-behind based
lambda introducer detection.

This patch finishes the transition by completely relying on the simple
lambda introducer detection and simply recursing into normal
brace-parsing code to parse until the end of the introducer.

This fixes initializers in lambdas, including nested lambdas.

Before:
  auto a = [b = [c = 42]{}]{};
  auto b = [c = &i + 23]{};

After:
  auto a = [b = [c = 42] {}] {};
  auto b = [c = &i + 23] {};

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=313622&r1=313621&r2=313622&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Sep 19 02:59:30 2017
@@ -1314,14 +1314,6 @@ bool UnwrappedLineParser::tryToParseLamb
 nextToken();
 return false;
   }
-  const FormatToken* Previous = getPreviousToken();
-  if (Previous &&
-  (Previous->isOneOf(tok::identifier, tok::kw_operator, tok::kw_new,
- tok::kw_delete) ||
-   Previous->closesScope() || Previous->isSimpleTypeSpecifier())) {
-nextToken();
-return false;
-  }
   assert(FormatTok->is(tok::l_square));
   FormatToken &LSquare = *FormatTok;
   if (!tryToParseLambdaIntroducer())
@@ -1364,49 +1356,17 @@ bool UnwrappedLineParser::tryToParseLamb
 }
 
 bool UnwrappedLineParser::tryToParseLambdaIntroducer() {
-  nextToken();
-  if (FormatTok->is(tok::equal)) {
-nextToken();
-if (FormatTok->is(tok::r_square)) {
-  nextToken();
-  return true;
-}
-if (FormatTok->isNot(tok::comma))
-  return false;
-nextToken();
-  } else if (FormatTok->is(tok::amp)) {
-nextToken();
-if (FormatTok->is(tok::r_square)) {
-  nextToken();
-  return true;
-}
-if (!FormatTok->isOneOf(tok::comma, tok::identifier)) {
-  return false;
-}
-if (FormatTok->is(tok::comma))
-  nextToken();
-  } else if (FormatTok->is(tok::r_square)) {
+  const FormatToken* Previous = getPreviousToken();
+  if (Previous &&
+  (Previous->isOneOf(tok::identifier, tok::kw_operator, tok::kw_new,
+ tok::kw_delete) ||
+   Previous->closesScope() || Previous->isSimpleTypeSpecifier())) {
 nextToken();
-return true;
+return false;
   }
-  do {
-if (FormatTok->is(tok::amp))
-  nextToken();
-if (!FormatTok->isOneOf(tok::identifier, tok::kw_this))
-  return false;
-nextToken();
-if (FormatTok->is(tok::ellipsis))
-  nextToken();
-if (FormatTok->is(tok::comma)) {
-  nextToken();
-} else if (FormatTok->is(tok::r_square)) {
-  nextToken();
-  return true;
-} else {
-  return false;
-}
-  } while (!eof());
-  return false;
+  nextToken();
+  parseSquare(/*LambdaIntroducer=*/true);
+  return true;
 }
 
 void UnwrappedLineParser::tryToParseJSFunction() {
@@ -1608,10 +1568,12 @@ void UnwrappedLineParser::parseParens()
   } while (!eof());
 }
 
-void UnwrappedLineParser::parseSquare() {
-  assert(FormatTok->Tok.is(tok::l_square) && "'[' expected.");
-  if (tryToParseLambda())
-return;
+void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) {
+  if (!LambdaIntroducer) {
+assert(FormatTok->Tok.is(tok::l_square) && "'[' expected.");
+if (tryToParseLambda())
+  return;
+  }
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=313622&r1=313621&r2=313622&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Tue Sep 19 02:59:30 2017
@@ -96,7 +96,7 @@ private:
   bool parseBracedList(bool ContinueOnSemicolons = false,
tok::TokenKind ClosingBraceKind = tok::r_brace);
   void parseParens();
-  void parseSquare();
+  void parseSquare(bool LambdaIntroducer = false);
   void parseIfThenElse();
   void parseTryCatch();
   void parseForOrWhileLoop();

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=313622&r1=313621&r2=313622&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Sep 19 02:59:30 2

[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.

2017-09-19 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

Thanks for the patch and the discussion around this. I fixed this in r313622 in 
what I think is a more principled approach that also works for nested lambdas 
(and gets rid of a lot of now-obsolete code).
The big problem with this code was that it evolved a bit to the point where we 
were able to generalize it, but we never did. Let me know if you find any 
problems with the aforementioned change.


https://reviews.llvm.org/D37980



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


[PATCH] D38032: [clangd] Serialize onDiagnosticsReady callbacks for the same file.

2017-09-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.

Calls to onDiagnosticsReady were done concurrently before. This sometimes
led to older versions of diagnostics being reported to the user after
the newer versions.


https://reviews.llvm.org/D38032

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -892,5 +893,67 @@
   }
 }
 
+TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
+  class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
+  public:
+NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse)
+: StartSecondReparse(std::move(StartSecondReparse)) {}
+
+void onDiagnosticsReady(
+PathRef File,
+Tagged> Diagnostics) override {
+
+  std::unique_lock Lock(Mutex, std::try_to_lock_t());
+  ASSERT_TRUE(Lock.owns_lock())
+  << "Detected concurrent onDiagnosticsReady calls for the same file.";
+  if (FirstRequest) {
+FirstRequest = false;
+StartSecondReparse.set_value();
+// Sleep long enough for the second request to be processed.
+std::this_thread::sleep_for(std::chrono::milliseconds(50));
+  }
+  Mutex.unlock();
+}
+
+  private:
+std::mutex Mutex;
+bool FirstRequest = true;
+std::promise StartSecondReparse;
+  };
+
+  const auto SourceContentsWithoutErrors = R"cpp(
+int a;
+int b;
+int c;
+int d;
+)cpp";
+
+  const auto SourceContentsWithErrors = R"cpp(
+int a = x;
+int b;
+int c;
+int d;
+)cpp";
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  llvm::StringMap FileContents;
+  FileContents[FooCpp] = "";
+  ConstantFSProvider FS(buildTestFS(FileContents));
+
+  std::promise StartSecondReparsePromise;
+  std::future StartSecondReparse = StartSecondReparsePromise.get_future();
+
+  NoConcurrentAccessDiagConsumer DiagConsumer(
+  std::move(StartSecondReparsePromise));
+
+  MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
+  ClangdServer Server(CDB, DiagConsumer, FS, 4, /*SnippetCompletions=*/false);
+  Server.addDocument(FooCpp, SourceContentsWithErrors);
+  StartSecondReparse.wait();
+
+  auto Future = Server.addDocument(FooCpp, SourceContentsWithoutErrors);
+  Future.wait();
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/DraftStore.h
===
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -13,15 +13,16 @@
 #include "Path.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
+#include 
 #include 
 #include 
 #include 
 
 namespace clang {
 namespace clangd {
 
-/// Using 'unsigned' here to avoid undefined behaviour on overflow.
-typedef unsigned DocVersion;
+/// Using unsigned int type here to avoid undefined behaviour on overflow.
+typedef uint64_t DocVersion;
 
 /// Document draft with a version of this draft.
 struct VersionedDraft {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -279,6 +279,12 @@
   // ClangdServer
   ClangdScheduler WorkScheduler;
   bool SnippetCompletions;
+
+  /// Used to serialize diagnostic callbacks.
+  /// FIXME(ibiryukov): get rid of an extra map and put all version counters
+  /// into CppFile.
+  std::mutex DiagnosticsMutex;
+  llvm::StringMap ReportedDiagnosticVersions;
 };
 
 } // namespace clangd
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -313,6 +313,19 @@
 auto Diags = DeferredRebuild.get();
 if (!Diags)
   return; // A new reparse was requested before this one completed.
+
+// We need to serialize access to resulting diagnostics to avoid calling
+// `onDiagnosticsReady` in the wrong order.
+std::lock_guard DiagsLock(DiagnosticsMutex);
+DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr];
+// FIXME(ibiryukov): get rid of '<' comparison here. In the current
+// implementation diagnostics will not be reported after version counters'
+// overflow. This should not happen in practice, since DocVersion is a
+// 64-bit unsigned integer.
+if (Version < LastReportedDiagsVersion)
+  return;
+LastReportedDiagsVersion = Version;
+
 DiagConsumer.onDiagnosticsReady(FileStr,
 make_tagged(std::move(*Diags), Tag));
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37263: [clang-format] Ignore case when sorting using-declarations

2017-09-19 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment.

As discussed offline, the sorting should also be stable to avoid no-op 
replacements for identical using declarations and randomizing the order of 
using declarations differing only in case (not that I'd expect this to be a 
frequent thing, but it's better to handle it properly anyway).


https://reviews.llvm.org/D37263



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


[PATCH] D37972: [clangd] Introduced Logger interface.

2017-09-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 115818.
ilya-biryukov added a comment.

- Replaced JSONOutput in Protocol.h/Protocol.cpp with Logger.


https://reviews.llvm.org/D37972

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/JSONRPCDispatcher.h
  clangd/Logger.cpp
  clangd/Logger.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "ClangdServer.h"
+#include "Logger.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -302,7 +303,8 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-/*SnippetCompletions=*/false);
+/*SnippetCompletions=*/false,
+EmptyLogger::getInstance());
 for (const auto &FileWithContents : ExtraFiles)
   FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
   FileWithContents.second;
@@ -365,7 +367,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*SnippetCompletions=*/false);
+  /*SnippetCompletions=*/false, EmptyLogger::getInstance());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -410,7 +412,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*SnippetCompletions=*/false);
+  /*SnippetCompletions=*/false, EmptyLogger::getInstance());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -457,7 +459,8 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false);
+  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false,
+  EmptyLogger::getInstance());
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
   const auto SourceContents = "int a;";
@@ -490,7 +493,8 @@
   "-stdlib=libstdc++"});
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false);
+  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false,
+  EmptyLogger::getInstance());
 
   // Just a random gcc version string
   SmallString<8> Version("4.9.3");
@@ -538,7 +542,8 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false);
+  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false,
+  EmptyLogger::getInstance());
   // No need to sync reparses, because reparses are performed on the calling
   // thread to true.
 
@@ -597,7 +602,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*SnippetCompletions=*/false);
+  /*SnippetCompletions=*/false, EmptyLogger::getInstance());
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
   const auto SourceContents = R"cpp(
@@ -745,7 +750,8 @@
   {
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-/*SnippetCompletions=*/false);
+/*SnippetCompletions=*/false,
+EmptyLogger::getInstance());
 
 // Prepare some random distributions for the test.
 std::random_device RandGen;
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -29,7 +29,7 @@
 namespace clang {
 namespace clangd {
 
-class JSONOutput;
+class Logger;
 
 struct URI {
   std::string uri;
@@ -59,7 +59,7 @@
   URI uri;
 
   static llvm::Optional
-  parse(llvm::yaml::MappingNode *Params, JSONOutput &Output);
+  parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
 

[PATCH] D38032: [clangd] Serialize onDiagnosticsReady callbacks for the same file.

2017-09-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 115821.
ilya-biryukov added a comment.

- Removed an incidental `Mutex.unlock()`, a leftover from previous drafts.


https://reviews.llvm.org/D38032

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -892,5 +893,66 @@
   }
 }
 
+TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
+  class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
+  public:
+NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse)
+: StartSecondReparse(std::move(StartSecondReparse)) {}
+
+void onDiagnosticsReady(
+PathRef File,
+Tagged> Diagnostics) override {
+
+  std::unique_lock Lock(Mutex, std::try_to_lock_t());
+  ASSERT_TRUE(Lock.owns_lock())
+  << "Detected concurrent onDiagnosticsReady calls for the same file.";
+  if (FirstRequest) {
+FirstRequest = false;
+StartSecondReparse.set_value();
+// Sleep long enough for the second request to be processed.
+std::this_thread::sleep_for(std::chrono::milliseconds(50));
+  }
+}
+
+  private:
+std::mutex Mutex;
+bool FirstRequest = true;
+std::promise StartSecondReparse;
+  };
+
+  const auto SourceContentsWithoutErrors = R"cpp(
+int a;
+int b;
+int c;
+int d;
+)cpp";
+
+  const auto SourceContentsWithErrors = R"cpp(
+int a = x;
+int b;
+int c;
+int d;
+)cpp";
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  llvm::StringMap FileContents;
+  FileContents[FooCpp] = "";
+  ConstantFSProvider FS(buildTestFS(FileContents));
+
+  std::promise StartSecondReparsePromise;
+  std::future StartSecondReparse = StartSecondReparsePromise.get_future();
+
+  NoConcurrentAccessDiagConsumer DiagConsumer(
+  std::move(StartSecondReparsePromise));
+
+  MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
+  ClangdServer Server(CDB, DiagConsumer, FS, 4, /*SnippetCompletions=*/false);
+  Server.addDocument(FooCpp, SourceContentsWithErrors);
+  StartSecondReparse.wait();
+
+  auto Future = Server.addDocument(FooCpp, SourceContentsWithoutErrors);
+  Future.wait();
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/DraftStore.h
===
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -13,15 +13,16 @@
 #include "Path.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
+#include 
 #include 
 #include 
 #include 
 
 namespace clang {
 namespace clangd {
 
-/// Using 'unsigned' here to avoid undefined behaviour on overflow.
-typedef unsigned DocVersion;
+/// Using unsigned int type here to avoid undefined behaviour on overflow.
+typedef uint64_t DocVersion;
 
 /// Document draft with a version of this draft.
 struct VersionedDraft {
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -279,6 +279,12 @@
   // ClangdServer
   ClangdScheduler WorkScheduler;
   bool SnippetCompletions;
+
+  /// Used to serialize diagnostic callbacks.
+  /// FIXME(ibiryukov): get rid of an extra map and put all version counters
+  /// into CppFile.
+  std::mutex DiagnosticsMutex;
+  llvm::StringMap ReportedDiagnosticVersions;
 };
 
 } // namespace clangd
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -313,6 +313,19 @@
 auto Diags = DeferredRebuild.get();
 if (!Diags)
   return; // A new reparse was requested before this one completed.
+
+// We need to serialize access to resulting diagnostics to avoid calling
+// `onDiagnosticsReady` in the wrong order.
+std::lock_guard DiagsLock(DiagnosticsMutex);
+DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr];
+// FIXME(ibiryukov): get rid of '<' comparison here. In the current
+// implementation diagnostics will not be reported after version counters'
+// overflow. This should not happen in practice, since DocVersion is a
+// 64-bit unsigned integer.
+if (Version < LastReportedDiagsVersion)
+  return;
+LastReportedDiagsVersion = Version;
+
 DiagConsumer.onDiagnosticsReady(FileStr,
 make_tagged(std::move(*Diags), Tag));
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r313624 - Lowering Mask Set1 intrinsics to LLVM IR

2017-09-19 Thread Jina Nahias via cfe-commits
Author: jina.nahias
Date: Tue Sep 19 04:00:27 2017
New Revision: 313624

URL: http://llvm.org/viewvc/llvm-project?rev=313624&view=rev
Log:
Lowering Mask Set1 intrinsics to LLVM IR

This patch, together with a matching llvm patch 
(https://reviews.llvm.org/D37669), implements the lowering of X86 mask set1 
intrinsics to IR.

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
cfe/trunk/lib/Headers/avx512bwintrin.h
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vlbwintrin.h
cfe/trunk/lib/Headers/avx512vlintrin.h
cfe/trunk/test/CodeGen/avx512bw-builtins.c
cfe/trunk/test/CodeGen/avx512f-builtins.c
cfe/trunk/test/CodeGen/avx512vl-builtins.c
cfe/trunk/test/CodeGen/avx512vlbw-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=313624&r1=313623&r2=313624&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Sep 19 04:00:27 2017
@@ -973,7 +973,6 @@ TARGET_BUILTIN(__builtin_ia32_pmuldq512,
 TARGET_BUILTIN(__builtin_ia32_pmuludq512, "V8LLiV16iV16i", "", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_ptestmd512, "UsV16iV16iUs", "", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_ptestmq512, "UcV8LLiV8LLiUc", "", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastd512_gpr_mask, "V16iiV16iUs", "", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_loaddqusi512_mask, "V16iiC*V16iUs", "", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_loaddqudi512_mask, "V8LLiLLiC*V8LLiUc", "", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_loadups512_mask, "V16ffC*V16fUs", "", "avx512f")
@@ -1374,11 +1373,6 @@ TARGET_BUILTIN(__builtin_ia32_movdqa64lo
 TARGET_BUILTIN(__builtin_ia32_movdqa64load256_mask, 
"V4LLiV4LLiC*V4LLiUc","","avx512vl")
 TARGET_BUILTIN(__builtin_ia32_movdqa64store128_mask, 
"vV2LLi*V2LLiUc","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_movdqa64store256_mask, 
"vV4LLi*V4LLiUc","","avx512f")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastb512_gpr_mask, 
"V64ccV64cULLi","","avx512bw")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastb128_gpr_mask, 
"V16ccV16cUs","","avx512bw,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastb256_gpr_mask, 
"V32ccV32cUi","","avx512bw,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastd128_gpr_mask, 
"V4iiV4iUc","","avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastd256_gpr_mask, 
"V8iiV8iUc","","avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512_mask, 
"V8LLiV8LLiV8LLiV8LLiUc","","avx512ifma")
 TARGET_BUILTIN(__builtin_ia32_vpmadd52huq512_maskz, 
"V8LLiV8LLiV8LLiV8LLiUc","","avx512ifma")
 TARGET_BUILTIN(__builtin_ia32_vpmadd52luq512_mask, 
"V8LLiV8LLiV8LLiV8LLiUc","","avx512ifma")
@@ -1589,9 +1583,6 @@ TARGET_BUILTIN(__builtin_ia32_broadcastm
 TARGET_BUILTIN(__builtin_ia32_broadcastmb256, "V4LLiUc","","avx512cd,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_broadcastmw128, "V4iUs","","avx512cd,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_broadcastmw256, "V8iUs","","avx512cd,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastw512_gpr_mask, 
"V32shV32sUi","","avx512bw")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastw256_gpr_mask, 
"V16shV16sUs","","avx512bw,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastw128_gpr_mask, 
"V8ssV8sUc","","avx512bw,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_pmovsdb512_mask, "V16cV16iV16cUs","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmovsdb512mem_mask, "vV16c*V16iUs","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmovswb512mem_mask, "vV32c*V32sUi","","avx512bw")

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=313624&r1=313623&r2=313624&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Tue Sep 19 04:00:27 2017
@@ -71,9 +71,6 @@ TARGET_BUILTIN(__builtin_ia32_pext_di, "
 TARGET_BUILTIN(__builtin_ia32_bextri_u64, "ULLiULLiIULLi", "", "tbm")
 TARGET_BUILTIN(__builtin_ia32_lwpins64, "UcULLiUiUi", "", "lwp")
 TARGET_BUILTIN(__builtin_ia32_lwpval64, "vULLiUiUi", "", "lwp")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastq512_gpr_mask, "V8LLiLLiV8LLiUc", "", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastq128_gpr_mask, 
"V2LLiULLiV2LLiUc","","avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pbroadcastq256_gpr_mask, 
"V4LLiULLiV4LLiUc","","avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vcvtsd2si64, "LLiV2dIi","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_vcvtsd2usi64, "ULLiV2dIi","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_vcvtss2si64, "LLiV4fIi","","avx512f")

Modified: cfe/trunk/lib/Headers/avx512bwintrin.h
URL: 
http://llvm.org/viewvc/llvm-pro

[PATCH] D37668: [X86][intrinsics] lower _mm[256|512]_mask[z]_set1_epi[8|16|32|64] intrinsic to IR

2017-09-19 Thread jina via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313624: Lowering Mask Set1 intrinsics to LLVM IR (authored 
by jina.nahias).

Changed prior to commit:
  https://reviews.llvm.org/D37668?vs=115622&id=115823#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37668

Files:
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
  cfe/trunk/lib/Headers/avx512bwintrin.h
  cfe/trunk/lib/Headers/avx512fintrin.h
  cfe/trunk/lib/Headers/avx512vlbwintrin.h
  cfe/trunk/lib/Headers/avx512vlintrin.h
  cfe/trunk/test/CodeGen/avx512bw-builtins.c
  cfe/trunk/test/CodeGen/avx512f-builtins.c
  cfe/trunk/test/CodeGen/avx512vl-builtins.c
  cfe/trunk/test/CodeGen/avx512vlbw-builtins.c

Index: cfe/trunk/lib/Headers/avx512vlintrin.h
===
--- cfe/trunk/lib/Headers/avx512vlintrin.h
+++ cfe/trunk/lib/Headers/avx512vlintrin.h
@@ -5723,59 +5723,72 @@
   (__v4df)_mm256_setzero_pd());
 }
 
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_mask_set1_epi32(__m128i __O, __mmask8 __M, int __A)
+{
+   return (__m128i)__builtin_ia32_selectd_128(__M,
+  (__v4si) _mm_set1_epi32(__A),
+  (__v4si)__O);
+}
+
+static __inline__ __m128i __DEFAULT_FN_ATTRS
+_mm_maskz_set1_epi32( __mmask8 __M, int __A)
+{
+   return (__m128i)__builtin_ia32_selectd_128(__M,
+  (__v4si) _mm_set1_epi32(__A),
+  (__v4si)_mm_setzero_si128());
+}
+
+static __inline__ __m256i __DEFAULT_FN_ATTRS
+_mm256_mask_set1_epi32(__m256i __O, __mmask8 __M, int __A)
+{
+   return (__m256i)__builtin_ia32_selectd_256(__M,
+  (__v8si) _mm256_set1_epi32(__A),
+  (__v8si)__O);
+}
+
+static __inline__ __m256i __DEFAULT_FN_ATTRS
+_mm256_maskz_set1_epi32( __mmask8 __M, int __A)
+{
+   return (__m256i)__builtin_ia32_selectd_256(__M,
+  (__v8si) _mm256_set1_epi32(__A),
+  (__v8si)_mm256_setzero_si256());
+}
 
-#define _mm_mask_set1_epi32(O, M, A) __extension__ ({ \
-  (__m128i)__builtin_ia32_pbroadcastd128_gpr_mask((int)(A), \
-  (__v4si)(__m128i)(O), \
-  (__mmask8)(M)); })
-
-#define _mm_maskz_set1_epi32(M, A) __extension__ ({ \
-  (__m128i)__builtin_ia32_pbroadcastd128_gpr_mask((int)(A), \
-  (__v4si)_mm_setzero_si128(), \
-  (__mmask8)(M)); })
-
-#define _mm256_mask_set1_epi32(O, M, A) __extension__ ({ \
-  (__m256i)__builtin_ia32_pbroadcastd256_gpr_mask((int)(A), \
-  (__v8si)(__m256i)(O), \
-  (__mmask8)(M)); })
-
-#define _mm256_maskz_set1_epi32(M, A) __extension__ ({ \
-  (__m256i)__builtin_ia32_pbroadcastd256_gpr_mask((int)(A), \
-  (__v8si)_mm256_setzero_si256(), \
-  (__mmask8)(M)); })
 
 #ifdef __x86_64__
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_mask_set1_epi64 (__m128i __O, __mmask8 __M, long long __A)
 {
-  return (__m128i) __builtin_ia32_pbroadcastq128_gpr_mask (__A, (__v2di) __O,
- __M);
+  return (__m128i) __builtin_ia32_selectq_128(__M,
+  (__v2di) _mm_set1_epi8(__A),
+  (__v2di) __O);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_maskz_set1_epi64 (__mmask8 __M, long long __A)
 {
-  return (__m128i) __builtin_ia32_pbroadcastq128_gpr_mask (__A,
- (__v2di)
- _mm_setzero_si128 (),
- __M);
+  return (__m128i) __builtin_ia32_selectq_128(__M,
+  (__v2di) _mm_set1_epi8(__A),
+  (__v2di) _mm_setzero_si128());
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS
 _mm256_mask_set1_epi64 (__m256i __O, __mmask8 __M, long long __A)
 {
-  return (__m256i) __builtin_ia32_pbroadcastq256_gpr_mask (__A, (__v4di) __O,
- __M);
+  return (__m256i) __builtin_ia32_selectq_256(__M,
+  (__v4di) _mm256_set1_epi64x(__A),
+  (__v4di) __O) ;
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS
 _mm256_maskz_set1_epi64 (__mmask8 __M, long long __A)
 {
-  return (__m256i) __builtin_ia32_pbroadcastq256_gpr_mask (__A,
- (__v4di)
- _mm256_setzero_si256 (),
- __M);
+   return (__m256i) __builtin_i

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

2017-09-19 Thread Bjorn Pettersson via Phabricator via cfe-commits
bjope accepted this revision.
bjope added a comment.
This revision is now accepted and ready to land.

Nobody else seems to bother. At least no objections, so I think we can land 
this now.


https://reviews.llvm.org/D37254



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


[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

For one thing, we need to update the state to store the "decision" of the 
reflowing mode, which is performed only in DryRun=true mode, to avoid doing the 
computation multiple times.

Apart from this, the decision is conceptually internal to 
breakProtrudingToken(). But the 'computation' of either mode is very similar, 
and updates the state variable anyway (State.Column, Stack.back().LastSpace, 
State.Stack[i].BreakBeforeParameter, Token->updateNextToken(State)) : so it is 
simpler (code-wise) to split the function and call it twice, then decide which 
'State' variable to use.


https://reviews.llvm.org/D33589



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


[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-09-19 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment.

I think doing the computation twice is fine. Or at least, I'd need a test case 
where it actually shows substantial overhead before doing what you are doing 
here. Understand that creating more States and making the State object itself 
larger also has cost and that cost occurs in the combinatorial exploration of 
the solution space. Doing an additional computation at the end should be 
comparatively cheap. Look at it this way: During the exploration of the 
solution space, we might enter breakProtrudingToken many times for the same 
comment. One more time during reconstruction of the solution is not that 
harmful.


https://reviews.llvm.org/D33589



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


[PATCH] D37903: Fix assume-filename handling in clang-format.el

2017-09-19 Thread Micah Werbitt via Phabricator via cfe-commits
werbitt updated this revision to Diff 115827.
werbitt added a comment.

Clean up call-process-region

- Quote call-process-region with #', this will cause a compile time error if 
call-process-region is undefined
- Pass positional arguments normally (exclude from the list)
- Instead of using append use a backquoted list
- Use ,@ to splice the assume file logic into the list
- Use 'and' instead of 'if'


https://reviews.llvm.org/D37903

Files:
  tools/clang-format/clang-format.el


Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -147,16 +147,15 @@
 ;; always use ‘utf-8-unix’ and ignore the buffer coding system.
 (default-process-coding-system '(utf-8-unix . utf-8-unix)))
 (unwind-protect
-(let ((status (apply 'call-process-region
- (append `(nil nil ,clang-format-executable
-   nil (,temp-buffer ,temp-file) nil)
- '("-output-replacements-xml")
- (if assume-file
- `("-assume-filename" ,assume-file) 
nil)
- `("-style" ,style
-   "-offset" ,(number-to-string file-start)
-   "-length" ,(number-to-string (- 
file-end file-start))
-   "-cursor" ,(number-to-string cursor)
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+   ,@(and assume-file (list "-assume-filename" 
assume-file))
+   "-style" ,style
+   "-offset" ,(number-to-string file-start)
+   "-length" ,(number-to-string (- file-end 
file-start))
+   "-cursor" ,(number-to-string cursor
   (stderr (with-temp-buffer
 (unless (zerop (cadr (insert-file-contents temp-file)))
   (insert ": "))


Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -147,16 +147,15 @@
 ;; always use ‘utf-8-unix’ and ignore the buffer coding system.
 (default-process-coding-system '(utf-8-unix . utf-8-unix)))
 (unwind-protect
-(let ((status (apply 'call-process-region
- (append `(nil nil ,clang-format-executable
-   nil (,temp-buffer ,temp-file) nil)
- '("-output-replacements-xml")
- (if assume-file
- `("-assume-filename" ,assume-file) nil)
- `("-style" ,style
-   "-offset" ,(number-to-string file-start)
-   "-length" ,(number-to-string (- file-end file-start))
-   "-cursor" ,(number-to-string cursor)
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+   ,@(and assume-file (list "-assume-filename" assume-file))
+   "-style" ,style
+   "-offset" ,(number-to-string file-start)
+   "-length" ,(number-to-string (- file-end file-start))
+   "-cursor" ,(number-to-string cursor
   (stderr (with-temp-buffer
 (unless (zerop (cadr (insert-file-contents temp-file)))
   (insert ": "))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37903: Fix assume-filename handling in clang-format.el

2017-09-19 Thread Micah Werbitt via Phabricator via cfe-commits
werbitt added a comment.

Hi,

Thank you very much for your feedback.

I submitted a bug here:
https://bugs.llvm.org/show_bug.cgi?id=34667

I made the changes you suggested, but I left the assume-filename optional 
argument for now.

My current use-case is that when I'm editing a source block in an orgmode file, 
the code is in new buffer that doesn't have a buffer-file-name. If I'm able to 
pass in the base orgmode buffer-file-name, I can apply the .clang-format file 
from the correct directory. Without it, I'll need to locate the .clang-format 
file and use it to populate the style argument, which would be a lot of extra 
code on my end.

If you still think it's not worth it to have the assume-file argument let me 
know and I'll remove it.

Thanks,
Micah


https://reviews.llvm.org/D37903



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


[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

In https://reviews.llvm.org/D33589#875039, @djasper wrote:

> I think doing the computation twice is fine. Or at least, I'd need a test 
> case where it actually shows substantial overhead before doing what you are 
> doing here. Understand that creating more States and making the State object 
> itself larger also has cost and that cost occurs in the combinatorial 
> exploration of the solution space. Doing an additional computation at the end 
> should be comparatively cheap. Look at it this way: During the exploration of 
> the solution space, we might enter breakProtrudingToken many times for the 
> same comment. One more time during reconstruction of the solution is not that 
> harmful.


I 've just tried to implement this (e.g. make the 2 calls also in DryRun), and 
I am running into an assertion in WhitespaceManager :

  [ RUN  ] FormatTest.ConfigurableUseOfTab
  FormatTests: 
/workspace/llvm/tools/clang/lib/Format/WhitespaceManager.cpp:112: void 
clang::format::WhitespaceManager::calculateLineBreakInformation(): Assertion 
`PreviousOriginalWhitespaceEndOffset <= OriginalWhitespaceStartOffset' failed.

This remind there was another "reason" for limiting this to DryRun (not sure it 
is a good or bad reason): it happens only in the optimizer, all other cases 
where the indenter is used are not affected.

I am still trying to get to the bottom of this assertion, any hint where to 
look for?


https://reviews.llvm.org/D33589



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


[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

> I am still trying to get to the bottom of this assertion, any hint where to 
> look for?

OK, got it. The issue is that we will actually need to run the wrapping 3 times 
when DryRun = false : call reflowProtrudingToken() twice with DryRun=true to 
find out the better option, then call it once more with DryRun=false.


https://reviews.llvm.org/D33589



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


[PATCH] D37972: [clangd] Introduced Logger interface.

2017-09-19 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle accepted this revision.
malaperle added a comment.
This revision is now accepted and ready to land.

Look good. Thank you!


https://reviews.llvm.org/D37972



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


[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-09-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 115830.
Typz added a comment.

Remove `Reflow` from LineState, and perform the decision again during 
reconstruction phase.


https://reviews.llvm.org/D33589

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9809,6 +9809,60 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+   "   // comment", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+			format("int a; /* first line\n"
+   "* second\n"
+   "* line third\n"
+   "* line\n"
+   "*/", Style));
+  EXPECT_EQ("int a; // first line\n"
+"   // second\n"
+"   // line third\n"
+"   // line",
+format("int a; // first line\n"
+   "   // second line\n"
+   "   // third line",
+   Style));
+
+  Style.PenaltyExcessCharacter = 90;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  EXPECT_EQ("int a; /* first line\n"
+"* second line\n"
+"* third line\n"
+"*/",
+			format("int a; /* first line\n"
+	   "* second line\n"
+			   "* third line\n"
+	   "*/", Style));
+  EXPECT_EQ("int a; // first line\n"
+"   // second line\n"
+"   // third line",
+format("int a; // first line\n"
+   "   // second line\n"
+   "   // third line",
+   Style));
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line"
+   "\n*/", Style));
+}
+
 #define EXPECT_ALL_STYLES_EQUAL(Styles)\
   for (size_t i = 1; i < Styles.size(); ++i)   \
   EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -27,6 +27,7 @@
 namespace format {
 
 class AnnotatedLine;
+class BreakableToken;
 struct FormatToken;
 struct LineState;
 struct ParenState;
@@ -100,6 +101,16 @@
   unsigned breakProtrudingToken(const FormatToken &Current, LineState &State,
 bool DryRun);
 
+  /// \brief Perform the reflowing of a BreakableToken.
+  /// Reflow=true, then the function will reflow the token as needed. Otherwise, it simply computes
+  /// the penalty caused by this tokens characters.
+  ///
+  /// \returns The penalty of reflowing the token if State.Reflow=true; otherwise
+  /// the penalty of characters going beyond the column limit.
+  unsigned reflowProtrudingToken(const FormatToken & Current, LineState & State,
+ std::unique_ptr & Token,
+ unsigned ColumnLimit, bool DryRun, bool Reflow);
+
   /// \brief Appends the next token to \p State and updates information
   /// necessary for indentation.
   ///
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1313,6 +1313,40 @@
   if (Current.UnbreakableTailLength >= ColumnLimit)
 return 0;
 
+  // Verify if the comment should be reflown
+  LineState PrevState = State;
+  unsigned Penalty = reflowProtrudingToken(Current, State, Token, ColumnLimit, true, true);
+  bool Reflow = true;
+  if (Penalty > 0) {
+LineState NoReflowState = PrevState;
+unsigned NoReflowPenalty = reflowProtrudingToken(Current, NoReflowState, Token, ColumnLimit,
+ true, false);
+if (NoReflowPenalty <= Penalty) {
+  Reflow = false;
+  State = NoReflowState;
+  Penalty = NoReflowPenalty;
+}
+  }
+
+  // Actually do the reflow, if DryRun=false
+  if (!DryRun)
+reflowProtrudingToken(Current, PrevState, Token, ColumnLimit, false, Reflow);
+
+  // Do not count the penalty twice, it will be 

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

2017-09-19 Thread Bjorn Pettersson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313628: [Sema] Disallow assigning record lvalues with nested 
const-qualified fields. (authored by bjope).

Changed prior to commit:
  https://reviews.llvm.org/D37254?vs=114340&id=115831#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37254

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/ExprClassification.cpp
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/assign.c

Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -3791,10 +3791,9 @@
 return reinterpret_cast(TagType::getDecl());
   }
 
-  // FIXME: This predicate is a helper to QualType/Type. It needs to
-  // recursively check all fields for const-ness. If any field is declared
-  // const, it needs to return false.
-  bool hasConstFields() const { return false; }
+  /// Recursively check all fields in the record for const-ness. If any field
+  /// is declared const, return true. Otherwise, return false.
+  bool hasConstFields() const;
 
   bool isSugared() const { return false; }
   QualType desugar() const { return QualType(this, 0); }
Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -275,6 +275,7 @@
 MLV_LValueCast,   // Specialized form of MLV_InvalidExpression.
 MLV_IncompleteType,
 MLV_ConstQualified,
+MLV_ConstQualifiedField,
 MLV_ConstAddrSpace,
 MLV_ArrayType,
 MLV_NoSetterProperty,
@@ -324,6 +325,7 @@
   CM_LValueCast, // Same as CM_RValue, but indicates GCC cast-as-lvalue ext
   CM_NoSetterProperty,// Implicit assignment to ObjC property without setter
   CM_ConstQualified,
+  CM_ConstQualifiedField,
   CM_ConstAddrSpace,
   CM_ArrayType,
   CM_IncompleteType
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5888,14 +5888,17 @@
   "cannot assign to %select{non-|}1static data member %2 "
   "with const-qualified type %3|"
   "cannot assign to non-static data member within const member function %1|"
+  "cannot assign to %select{variable %2|non-static data member %2|lvalue}1 "
+  "with %select{|nested }3const-qualified data member %4|"
   "read-only variable is not assignable}0">;
 
 def note_typecheck_assign_const : Note<
   "%select{"
   "function %1 which returns const-qualified type %2 declared here|"
   "variable %1 declared const here|"
   "%select{non-|}1static data member %2 declared const here|"
-  "member function %q1 is declared const here}0">;
+  "member function %q1 is declared const here|"
+  "%select{|nested }1data member %2 declared const here}0">;
 
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of different signs: %0 and %1">,
Index: cfe/trunk/test/Sema/assign.c
===
--- cfe/trunk/test/Sema/assign.c
+++ cfe/trunk/test/Sema/assign.c
@@ -16,3 +16,46 @@
   b[4] = 1; // expected-error {{read-only variable is not assignable}}
   b2[4] = 1; // expected-error {{read-only variable is not assignable}}
 }
+
+typedef struct I {
+  const int a; // expected-note 4{{nested data member 'a' declared const here}} \
+  expected-note 6{{data member 'a' declared const here}}
+} I;
+typedef struct J {
+  struct I i;
+} J;
+typedef struct K {
+  struct J *j;
+} K;
+
+void testI(struct I i1, struct I i2) {
+  i1 = i2; // expected-error {{cannot assign to variable 'i1' with const-qualified data member 'a'}}
+}
+void testJ1(struct J j1, struct J j2) {
+  j1 = j2; // expected-error {{cannot assign to variable 'j1' with nested const-qualified data member 'a'}}
+}
+void testJ2(struct J j, struct I i) {
+  j.i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
+}
+void testK1(struct K k, struct J j) {
+  *(k.j) = j; // expected-error {{cannot assign to lvalue with nested const-qualified data member 'a'}}
+}
+void testK2(struct K k, struct I i) {
+  k.j->i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
+}
+
+void testI_(I i1, I i2) {
+  i1 = i2; // expected-error {{cannot assign to variable 'i1' with const-qualified data member 'a'}}
+}
+void testJ1_(J j1, J j2) {
+  j1 = j2; // expected-error {{cannot assign to variable 'j1' with nested const-qualified data member 'a'}}
+}
+void testJ2_(J j, I i) {
+  j.i = i; // expected-error {{cannot assign to non-static data m

r313628 - [Sema] Disallow assigning record lvalues with nested const-qualified fields.

2017-09-19 Thread Bjorn Pettersson via cfe-commits
Author: bjope
Date: Tue Sep 19 06:10:30 2017
New Revision: 313628

URL: http://llvm.org/viewvc/llvm-project?rev=313628&view=rev
Log:
[Sema] Disallow assigning record lvalues with nested const-qualified fields.

Summary:
According to C99 6.3.2.1p1, structs and unions with nested
const-qualified fields (that is, const-qualified fields
declared at some recursive level of the aggregate) are not
modifiable lvalues. However, Clang permits assignments of
these lvalues.

With this patch, we both prohibit the assignment of records
with const-qualified fields and emit a best-effort diagnostic.
This fixes https://bugs.llvm.org/show_bug.cgi?id=31796 .

Committing on behalf of bevinh (Bevin Hansson).

Reviewers: rtrieu, rsmith, bjope

Reviewed By: bjope

Subscribers: Ka-Ka, rogfer01, bjope, fhahn, cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ExprClassification.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/assign.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=313628&r1=313627&r2=313628&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Sep 19 06:10:30 2017
@@ -275,6 +275,7 @@ public:
 MLV_LValueCast,   // Specialized form of MLV_InvalidExpression.
 MLV_IncompleteType,
 MLV_ConstQualified,
+MLV_ConstQualifiedField,
 MLV_ConstAddrSpace,
 MLV_ArrayType,
 MLV_NoSetterProperty,
@@ -324,6 +325,7 @@ public:
   CM_LValueCast, // Same as CM_RValue, but indicates GCC cast-as-lvalue ext
   CM_NoSetterProperty,// Implicit assignment to ObjC property without 
setter
   CM_ConstQualified,
+  CM_ConstQualifiedField,
   CM_ConstAddrSpace,
   CM_ArrayType,
   CM_IncompleteType

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=313628&r1=313627&r2=313628&view=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Tue Sep 19 06:10:30 2017
@@ -3791,10 +3791,9 @@ public:
 return reinterpret_cast(TagType::getDecl());
   }
 
-  // FIXME: This predicate is a helper to QualType/Type. It needs to
-  // recursively check all fields for const-ness. If any field is declared
-  // const, it needs to return false.
-  bool hasConstFields() const { return false; }
+  /// Recursively check all fields in the record for const-ness. If any field
+  /// is declared const, return true. Otherwise, return false.
+  bool hasConstFields() const;
 
   bool isSugared() const { return false; }
   QualType desugar() const { return QualType(this, 0); }

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313628&r1=313627&r2=313628&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 06:10:30 
2017
@@ -5888,6 +5888,8 @@ def err_typecheck_assign_const : Error<
   "cannot assign to %select{non-|}1static data member %2 "
   "with const-qualified type %3|"
   "cannot assign to non-static data member within const member function %1|"
+  "cannot assign to %select{variable %2|non-static data member %2|lvalue}1 "
+  "with %select{|nested }3const-qualified data member %4|"
   "read-only variable is not assignable}0">;
 
 def note_typecheck_assign_const : Note<
@@ -5895,7 +5897,8 @@ def note_typecheck_assign_const : Note<
   "function %1 which returns const-qualified type %2 declared here|"
   "variable %1 declared const here|"
   "%select{non-|}1static data member %2 declared const here|"
-  "member function %q1 is declared const here}0">;
+  "member function %q1 is declared const here|"
+  "%select{|nested }1data member %2 declared const here}0">;
 
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of different signs: %0 and %1">,

Modified: cfe/trunk/lib/AST/ExprClassification.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprClassification.cpp?rev=313628&r1=313627&r2=313628&view=diff
==
--- cfe/trunk/lib/AST/ExprClassification.cpp (original)
+++ cfe/trunk/lib/AST/ExprClassification.cpp Tue Sep 19 06:10:30 2017
@@ -641,7 +641,7 @@ static Cl::ModifiableType IsModifiable(A
   // Records with any const fields (recursively) are not modifiable.
   if (const RecordType *R = CT->getAs())

[PATCH] D37903: Fix assume-filename handling in clang-format.el

2017-09-19 Thread Philipp via Phabricator via cfe-commits
phst added inline comments.



Comment at: tools/clang-format/clang-format.el:125
 If called interactively uses the region or the current statement if there
 is no active region.  If no style is given uses `clang-format-style'."
   (interactive

Please document the ASSUME-FILE parameter.



Comment at: tools/clang-format/clang-format.el:187
 ;;;###autoload
-(defun clang-format-buffer (&optional style)
+(defun clang-format-buffer (&optional style assume-file)
   "Use clang-format to format the current buffer according to STYLE."

Nit: consider renaming the parameter to ASSUME-FILENAME so that it's clear that 
it's a filename. Same above.



Comment at: tools/clang-format/clang-format.el:188
+(defun clang-format-buffer (&optional style assume-file)
   "Use clang-format to format the current buffer according to STYLE."
   (interactive)

Please document the new parameter here as well.


https://reviews.llvm.org/D37903



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


[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-09-19 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

I find the current semantics of the functions a bit surprising, specifically:
... reflowProtrudingToken(..., bool Reflow)
is really confusing me :)

I'd have expected something like this where we currently call 
breakProtrudingToken():

  if (CanBreak) {
ReflowPenalty = breakProtrudingToken(Current, State, /*DryRun=*/true);
FixedPenalty = getExcessPenalty(Current, State);
Penalty = ReflowPenalty < FixedPenalty ? ReflowPenalty : FixedPenalty;
if (!DryRun && ReflowPenalty < FixedPenalty
breakProtrudingToken(Current, State, /*DryRun=*/false);
  }

I haven't looked how we calculate the penalty currently if we can't break, as 
if we don't, that also seems ... wrong.
getExcessPenalty would be a new function that just focuses on getting the 
excess penalty for a breakable token.


https://reviews.llvm.org/D33589



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


[PATCH] D37903: Fix assume-filename handling in clang-format.el

2017-09-19 Thread Philipp via Phabricator via cfe-commits
phst accepted this revision.
phst added a comment.
This revision is now accepted and ready to land.

Your use case sounds good to me. Please be sure to document the new parameter, 
though.




Comment at: tools/clang-format/clang-format.el:154
+ `("-output-replacements-xml"
+   ,@(and assume-file (list "-assume-filename" 
assume-file))
+   "-style" ,style

Please add a comment referencing the LLVM bug you've just filed so that others 
aren't surprised about this.


https://reviews.llvm.org/D37903



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


[PATCH] D37955: [libcxx] Fix invert negative bracket match.

2017-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists requested changes to this revision.
mclow.lists added a comment.
This revision now requires changes to proceed.

When I applied this patch locally, some of the other tests started failing.
Specifically:

  Assertion failed: (!std::regex_match(s, m, std::regex("^[a-f]$", 
std::regex_constants::basic))), function main, file 
test/std/re/re.alg/re.alg.match/basic.pass.cpp, line 499.
  Assertion failed: (!std::regex_match(s, m, std::regex("^[a-f]$", 
std::regex_constants::extended))), function main, file 
test/std/re/re.alg/re.alg.match/extended.pass.cpp, line 497.
  Assertion failed: (!std::regex_match(s, m, std::regex("^[a-f]$"))), function 
main, file test/std/re/re.alg/re.alg.match/ecma.pass.cpp, line 493.
  Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$", 
std::regex_constants::awk))), function main, file 
test/std/re/re.alg/re.alg.search/awk.pass.cpp, line 560.
  Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$", 
std::regex_constants::basic))), function main, file 
test/std/re/re.alg/re.alg.search/basic.pass.cpp, line 562.
  Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$"))), function 
main, file test/std/re/re.alg/re.alg.search/ecma.pass.cpp, line 553.
  Assertion failed: (!std::regex_search(s, m, std::regex("^[a-f]$", 
std::regex_constants::extended))), function main, file 
test/std/re/re.alg/re.alg.search/extended.pass.cpp, line 560.

all now fail for me (Mac OS 10.12)


https://reviews.llvm.org/D37955



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


[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius abandoned this revision.
curdeius added a comment.

Ok. Nice patch. You can close https://bugs.llvm.org/show_bug.cgi?id=19986 now.


https://reviews.llvm.org/D37980



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


[PATCH] D37861: preserving #pragma clang assume_nonnull in preprocessed output

2017-09-19 Thread Zbigniew Sarbinowski via Phabricator via cfe-commits
zibi updated this revision to Diff 115838.
zibi added a comment.

original + review changes


https://reviews.llvm.org/D37861

Files:
  include/clang/Lex/PPCallbacks.h
  lib/Frontend/PrintPreprocessedOutput.cpp
  lib/Lex/Pragma.cpp
  test/Preprocessor/pragma_assume_nonnull.c

Index: test/Preprocessor/pragma_assume_nonnull.c
===
--- /dev/null
+++ test/Preprocessor/pragma_assume_nonnull.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -E %s | FileCheck %s
+
+// CHECK: #pragma clang assume_nonnull begin
+#pragma clang assume_nonnull begin
+
+int bar(int * ip) { return *ip; }
+
+// CHECK: #pragma clang assume_nonnull end
+#pragma clang assume_nonnull end
+
+int foo(int * _Nonnull ip) { return *ip; }
+
+int main() {
+#if __has_feature(assume_nonnull)
+   return bar(0) + foo(0); // expected-warning 2 {{null passed to a callee that requires a non-null argument}}
+#else
+   return bar(0) + foo(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
+#endif
+}
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1725,21 +1725,26 @@
 
 // The start location we want after processing this.
 SourceLocation NewLoc;
+PPCallbacks *Callbacks = PP.getPPCallbacks();
 
 if (IsBegin) {
   // Complain about attempts to re-enter an audit.
   if (BeginLoc.isValid()) {
 PP.Diag(Loc, diag::err_pp_double_begin_of_assume_nonnull);
 PP.Diag(BeginLoc, diag::note_pragma_entered_here);
   }
   NewLoc = Loc;
+  if (Callbacks)
+Callbacks->PragmaAssumeNonNullBegin(NewLoc);
 } else {
   // Complain about attempts to leave an audit that doesn't exist.
   if (!BeginLoc.isValid()) {
 PP.Diag(Loc, diag::err_pp_unmatched_end_of_assume_nonnull);
 return;
   }
   NewLoc = SourceLocation();
+  if (Callbacks)
+Callbacks->PragmaAssumeNonNullEnd(NewLoc);
 }
 
 PP.setPragmaAssumeNonNullLoc(NewLoc);
Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -143,6 +143,8 @@
  ArrayRef Ids) override;
   void PragmaWarningPush(SourceLocation Loc, int Level) override;
   void PragmaWarningPop(SourceLocation Loc) override;
+  void PragmaAssumeNonNullBegin(SourceLocation Loc) override;
+  void PragmaAssumeNonNullEnd(SourceLocation Loc) override;
 
   bool HandleFirstTokOnLine(Token &Tok);
 
@@ -549,6 +551,22 @@
   setEmittedDirectiveOnThisLine();
 }
 
+void PrintPPOutputPPCallbacks::
+PragmaAssumeNonNullBegin(SourceLocation Loc) {
+  startNewLineIfNeeded();
+  MoveToLine(Loc);
+  OS << "#pragma clang assume_nonnull begin";
+  setEmittedDirectiveOnThisLine();
+}
+
+void PrintPPOutputPPCallbacks::
+PragmaAssumeNonNullEnd(SourceLocation Loc) {
+  startNewLineIfNeeded();
+  MoveToLine(Loc);
+  OS << "#pragma clang assume_nonnull end";
+  setEmittedDirectiveOnThisLine();
+}
+
 /// HandleFirstTokOnLine - When emitting a preprocessed file in -E mode, this
 /// is called for the first token on each new line.  If this really is the start
 /// of a new logical line, handle it and return true, otherwise return false.
Index: include/clang/Lex/PPCallbacks.h
===
--- include/clang/Lex/PPCallbacks.h
+++ include/clang/Lex/PPCallbacks.h
@@ -235,6 +235,14 @@
   virtual void PragmaWarningPop(SourceLocation Loc) {
   }
 
+  /// \brief Callback invoked when a \#pragma clang assume_nonnull begin directive
+  /// is read.
+  virtual void PragmaAssumeNonNullBegin(SourceLocation Loc) {}
+
+  /// \brief Callback invoked when a \#pragma clang assume_nonnull end directive
+  /// is read.
+  virtual void PragmaAssumeNonNullEnd(SourceLocation Loc) {}
+
   /// \brief Called by Preprocessor::HandleMacroExpandedIdentifier when a
   /// macro invocation is found.
   virtual void MacroExpands(const Token &MacroNameTok,
@@ -443,6 +451,16 @@
 Second->PragmaWarningPop(Loc);
   }
 
+  void PragmaAssumeNonNullBegin(SourceLocation Loc) override {
+First->PragmaAssumeNonNullBegin(Loc);
+Second->PragmaAssumeNonNullBegin(Loc);
+  }
+
+  void PragmaAssumeNonNullEnd(SourceLocation Loc) override {
+First->PragmaAssumeNonNullEnd(Loc);
+Second->PragmaAssumeNonNullEnd(Loc);
+  }
+
   void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
 SourceRange Range, const MacroArgs *Args) override {
 First->MacroExpands(MacroNameTok, MD, Range, Args);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37861: preserving #pragma clang assume_nonnull in preprocessed output

2017-09-19 Thread Zbigniew Sarbinowski via Phabricator via cfe-commits
zibi added a comment.

Please be aware that I don't have the commit permission yet since this is my 
first patch. I will rely on somebody to push it to the trunk.


https://reviews.llvm.org/D37861



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


[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

2017-09-19 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

Hey @jyknight I heard from @erichkeane that you may want a couple more changes 
to this patch.  Please let me know if you have some changes to recommend. 
@joerg thought this was OK for submission. Thanks --Melanie


https://reviews.llvm.org/D34158



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


[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

2017-09-19 Thread Alexander Shukaev via Phabricator via cfe-commits
Alexander-Shukaev added a comment.

Mark, just wanted to check if the review is still somewhere on your radar.


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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


[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

I confirm what I observed before: when invoking tests in 
`unittests/Format/NamespaceEndCommentsFixerTest.cpp`, the `const AnnotatedLine 
*line` parameter in `getNamespaceToken` gets one big line that includes both 
`namespace` and `{` (something like `namespace\n{\n...`, whereas in tests 
invoked from `unittests/Format/FormatTests.cpp`, there is a separate line with 
`namespace\n` and another one with `{\n`.

I haven't looked at what provokes this behavior, but I imagine that there are 
additional passes in `FormatTests`.


https://reviews.llvm.org/D37904



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


[PATCH] D36101: Fix usage of right shift operator in fold expressions

2017-09-19 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 115842.
Rakete added a project: clang.
Rakete added a comment.

Added the tests to the existing test file for fold operators that I didn't 
notice before :)


https://reviews.llvm.org/D36101

Files:
  lib/Parse/ParseExpr.cpp
  test/Parser/cxx1z-fold-expressions.cpp


Index: test/Parser/cxx1z-fold-expressions.cpp
===
--- test/Parser/cxx1z-fold-expressions.cpp
+++ test/Parser/cxx1z-fold-expressions.cpp
@@ -43,3 +43,8 @@
 (int)(undeclared_junk + ...) + // expected-error {{undeclared}}
 (int)(a + ...); // expected-error {{does not contain any unexpanded}}
 }
+
+// fold-operator can be '>' or '>>'.
+template bool greater_than() { return (N > ...); }
+template bool right_shift() { return (N >> ...); }
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -270,7 +270,7 @@
   return Level > prec::Unknown && Level != prec::Conditional;
 }
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }
 
 /// \brief Parse a binary expression that starts with \p LHS and has a


Index: test/Parser/cxx1z-fold-expressions.cpp
===
--- test/Parser/cxx1z-fold-expressions.cpp
+++ test/Parser/cxx1z-fold-expressions.cpp
@@ -43,3 +43,8 @@
 (int)(undeclared_junk + ...) + // expected-error {{undeclared}}
 (int)(a + ...); // expected-error {{does not contain any unexpanded}}
 }
+
+// fold-operator can be '>' or '>>'.
+template bool greater_than() { return (N > ...); }
+template bool right_shift() { return (N >> ...); }
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -270,7 +270,7 @@
   return Level > prec::Unknown && Level != prec::Conditional;
 }
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }
 
 /// \brief Parse a binary expression that starts with \p LHS and has a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings

2017-09-19 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

Ok, we still need to fix structured bindings in the UnwrappedLineParser. 
Unfortunately isCppStructuredBinding requires a "previous token" function, 
which we don't really have in the UnwrappedLineParser. /me goes thinking more 
about that part of the problem. That should be largely independent of this 
patch, though, so LG from my side.


https://reviews.llvm.org/D35743



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


[PATCH] D38040: [OpenMP] Add an additional test for D34888

2017-09-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea created this revision.

Test for checking if the mapping is performed correctly. This is a test 
initially included in Patch https://reviews.llvm.org/D29905


Repository:
  rL LLVM

https://reviews.llvm.org/D38040

Files:
  test/OpenMP/target_map_codegen.cpp


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4840,3 +4840,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) 
%ParamToKernel) #0 {
+
+#endif


Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4840,3 +4840,20 @@
 }
 #endif
 #endif
+
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck -check-prefix=CK30 %s
+
+#ifdef CK30
+
+void target_maps_parallel_integer(int a){
+  int ParamToKernel = a;
+#pragma omp target map(tofrom: ParamToKernel)
+  {
+ParamToKernel += 1;
+  }
+}
+
+// CK30: {{.*}}void @__omp_offloading_{{.*}}(i32* dereferenceable(4) %ParamToKernel) #0 {
+
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36101: Fix usage of right shift operator in fold expressions

2017-09-19 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 115846.
Rakete added a comment.

Used the correct return type, even if it doesn't really matter to the compiler.


https://reviews.llvm.org/D36101

Files:
  lib/Parse/ParseExpr.cpp
  test/Parser/cxx1z-fold-expressions.cpp


Index: test/Parser/cxx1z-fold-expressions.cpp
===
--- test/Parser/cxx1z-fold-expressions.cpp
+++ test/Parser/cxx1z-fold-expressions.cpp
@@ -43,3 +43,8 @@
 (int)(undeclared_junk + ...) + // expected-error {{undeclared}}
 (int)(a + ...); // expected-error {{does not contain any unexpanded}}
 }
+
+// fold-operator can be '>' or '>>'.
+template bool greater_than() { return (N > ...); }
+template int right_shift() { return (N >> ...); }
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -270,7 +270,7 @@
   return Level > prec::Unknown && Level != prec::Conditional;
 }
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }
 
 /// \brief Parse a binary expression that starts with \p LHS and has a


Index: test/Parser/cxx1z-fold-expressions.cpp
===
--- test/Parser/cxx1z-fold-expressions.cpp
+++ test/Parser/cxx1z-fold-expressions.cpp
@@ -43,3 +43,8 @@
 (int)(undeclared_junk + ...) + // expected-error {{undeclared}}
 (int)(a + ...); // expected-error {{does not contain any unexpanded}}
 }
+
+// fold-operator can be '>' or '>>'.
+template bool greater_than() { return (N > ...); }
+template int right_shift() { return (N >> ...); }
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -270,7 +270,7 @@
   return Level > prec::Unknown && Level != prec::Conditional;
 }
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }
 
 /// \brief Parse a binary expression that starts with \p LHS and has a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37925: Allow specifying sanitizers in blacklists

2017-09-19 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich updated this revision to Diff 115849.
vlad.tsyrklevich added a comment.

- Refactor to make compile() not virtual again
- Refactor a confusing use of ASanMask into individual uses of 
SanitizerKind::{Address,KernelAddress}
- 'Sections' -> 'Section names'


https://reviews.llvm.org/D37925

Files:
  docs/ControlFlowIntegrity.rst
  docs/SanitizerSpecialCaseList.rst
  include/clang/Basic/SanitizerBlacklist.h
  include/clang/Basic/SanitizerSpecialCaseList.h
  lib/AST/Decl.cpp
  lib/Basic/CMakeLists.txt
  lib/Basic/SanitizerBlacklist.cpp
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/XRayLists.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGen/Inputs/sanitizer-special-case-list.sanitized.txt
  test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized1.txt
  test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized2.txt
  test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized3.txt
  test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized4.txt
  test/CodeGen/sanitizer-special-case-list.c
  test/CodeGenCXX/cfi-blacklist.cpp

Index: test/CodeGenCXX/cfi-blacklist.cpp
===
--- test/CodeGenCXX/cfi-blacklist.cpp
+++ test/CodeGenCXX/cfi-blacklist.cpp
@@ -1,6 +1,18 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s
-// RUN: echo "type:std::*" > %t.txt
-// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
+
+// Check that blacklisting cfi and cfi-vcall work correctly
+// RUN: echo "[cfi-vcall]" > %t.vcall.txt
+// RUN: echo "type:std::*" >> %t.vcall.txt
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.vcall.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
+//
+// RUN: echo "[cfi]" > %t.cfi.txt
+// RUN: echo "type:std::*" >> %t.cfi.txt
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.cfi.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOSTD %s
+
+// Check that blacklisting non-vcall modes does not affect vcalls
+// RUN: echo "[cfi-icall|cfi-nvcall|cfi-cast-strict|cfi-derived-cast|cfi-unrelated-cast]" > %t.other.txt
+// RUN: echo "type:std::*" >> %t.other.txt
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fvisibility hidden -fms-extensions -fsanitize=cfi-vcall -fsanitize-blacklist=%t.other.txt -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOBL %s
 
 struct S1 {
   virtual void f();
Index: test/CodeGen/sanitizer-special-case-list.c
===
--- /dev/null
+++ test/CodeGen/sanitizer-special-case-list.c
@@ -0,0 +1,26 @@
+// Verify that blacklist sections correctly select sanitizers to apply blacklist entries to.
+//
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized1.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized2.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized3.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.unsanitized4.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=UNSANITIZED
+//
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow,cfi-icall -fsanitize-blacklist=%S/Inputs/sanitizer-special-case-list.sanitized.txt -emit-llvm %s -o - | FileCheck %s --check-prefix=SANITIZED
+
+unsigned i;
+
+// SANITIZED: @overflow
+// UNSANITIZED: @overflow
+unsigned overflow() {
+  // SANITIZED: call {{.*}}void @__ubsan
+  // UNSANITIZED-NOT: call {{.*}}void @__ubsan
+  return i * 37;
+}
+
+// SANITIZED: @cfi
+// UNSANITIZED: @cfi
+void cfi(void (*fp)()) {
+  // SANITIZED: llvm.type.test
+  // UNSANITIZED-NOT: llvm.type.test
+  fp();
+}
Index: test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized4.txt
===
--- /dev/null
+++ test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized4.txt
@@ -0,0 +1,4 @@
+[c*]
+fun:*cfi*
+[u*]
+fun:*overflow*
Index: test/CodeGen/Inputs/sanitizer-special-case-list.unsanitized3.txt
==

[PATCH] D37804: [OpenCL] Handle address space conversion while setting type alignment

2017-09-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:957
 
-return Builder.CreateBitCast(Addr, ConvertType(E->getType()));
+return Builder.CreatePointerBitCastOrAddrSpaceCast(
+Addr, ConvertType(E->getType()));

Anastasia wrote:
> yaxunl wrote:
> > Better assert that only CK_AddressSpaceConversion allows different addr 
> > spaces in target type.
> It seems for consistency then we would have to assert for both different ASes 
> in non-`addrspacecast` case and equivalent ASes in the other case.
> 
> As condition becomes complicated then, I could as well split into either 
> creating `bitcast` or `addrspacecast` explicitly at a cost of one simple 
> check in the return statement but that would be in release build too. 
Sam, do you prefer an assert that will check both things or a runtime check 
that would make sure to build the right IR node here?


https://reviews.llvm.org/D37804



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


[PATCH] D37925: Allow specifying sanitizers in blacklists

2017-09-19 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich marked an inline comment as done.
vlad.tsyrklevich added inline comments.



Comment at: docs/SanitizerSpecialCaseList.rst:57
+
+Sections are regular expressions written in square brackets that denote which
+sanitizer the following entries apply to. For example, ``[address]`` specifies

eugenis wrote:
> Section names are regular expressions
> 
> also, not really a regular expression, more like a wildcard; I don't know 
> what's the right term for this
They are not just wildcards in the sense that `cfi-vcall|cfi-icall` is more 
than just wildcard matching. I could add a line that explains how `*` behaves 
here as well? (similarly to what we have in the entries paragraph)



Comment at: lib/CodeGen/CGDeclCXX.cpp:322
+  if (getLangOpts().Sanitize.hasOneOf(ASanMask))
+if (!isInSanitizerBlacklist(ASanMask, Fn, Loc))
   Fn->addFnAttr(llvm::Attribute::SanitizeAddress);

eugenis wrote:
> vlad.tsyrklevich wrote:
> > This use of ASanMask could also confound address & kernel-address as 
> > @eugenis pointed out.
> It would be more readable to split it in two if-s, IMHO: one for asan, one 
> for kernel-asan.
> 
Agreed.


https://reviews.llvm.org/D37925



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


[PATCH] D38041: [libc++] Separate locale tests that are XFAIL on linux-gnu from others

2017-09-19 Thread Tim Shen via Phabricator via cfe-commits
timshen created this revision.
Herald added a subscriber: sanjoy.
Herald added a reviewer: EricWF.

The tests don't pass on linux-gnu. Move them out, so that the others
don't have to be XFAIL.


https://reviews.llvm.org/D38041

Files:
  libcxx/test/std/re/re.alg/re.alg.match/basic.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.match/basic_locale.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.match/ecma.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.match/ecma_locale.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.match/extended.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.match/extended_locale.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/awk.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/awk_locale.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/ecma.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/ecma_locale.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/extended.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/extended_locale.pass.cpp

Index: libcxx/test/std/re/re.alg/re.alg.search/extended_locale.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/extended_locale.pass.cpp
@@ -0,0 +1,98 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: locale.cs_CZ.ISO8859-2
+
+// 
+
+// template 
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+//  match_results& m,
+//  const basic_regex& e,
+//  regex_constants::match_flag_type flags = regex_constants::match_default);
+
+// TODO: investigation needed
+// XFAIL: linux-gnu
+
+#include 
+#include 
+#include "test_macros.h"
+#include "test_iterators.h"
+
+#include "platform_support.h" // locale name macros
+
+int main()
+{
+std::locale::global(std::locale(LOCALE_cs_CZ_ISO8859_2));
+{
+std::cmatch m;
+const char s[] = "m";
+assert(std::regex_search(s, m, std::regex("[a[=M=]z]",
+ std::regex_constants::extended)));
+assert(m.size() == 1);
+assert(!m.prefix().matched);
+assert(m.prefix().first == s);
+assert(m.prefix().second == m[0].first);
+assert(!m.suffix().matched);
+assert(m.suffix().first == m[0].second);
+assert(m.suffix().second == m[0].second);
+assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s));
+assert(m.position(0) == 0);
+assert(m.str(0) == s);
+}
+{
+std::cmatch m;
+const char s[] = "Ch";
+assert(std::regex_search(s, m, std::regex("[a[.ch.]z]",
+   std::regex_constants::extended | std::regex_constants::icase)));
+assert(m.size() == 1);
+assert(!m.prefix().matched);
+assert(m.prefix().first == s);
+assert(m.prefix().second == m[0].first);
+assert(!m.suffix().matched);
+assert(m.suffix().first == m[0].second);
+assert(m.suffix().second == m[0].second);
+assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s));
+assert(m.position(0) == 0);
+assert(m.str(0) == s);
+}
+{
+std::wcmatch m;
+const wchar_t s[] = L"m";
+assert(std::regex_search(s, m, std::wregex(L"[a[=M=]z]",
+ std::regex_constants::extended)));
+assert(m.size() == 1);
+assert(!m.prefix().matched);
+assert(m.prefix().first == s);
+assert(m.prefix().second == m[0].first);
+assert(!m.suffix().matched);
+assert(m.suffix().first == m[0].second);
+assert(m.suffix().second == m[0].second);
+assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s));
+assert(m.position(0) == 0);
+assert(m.str(0) == s);
+}
+{
+std::wcmatch m;
+const wchar_t s[] = L"Ch";
+assert(std::regex_search(s, m, std::wregex(L"[a[.ch.]z]",
+   std::regex_constants::extended | std::regex_constants::icase)));
+assert(m.size() == 1);
+assert(!m.prefix().matched);
+assert(m.prefix().first == s);
+assert(m.prefix().second == m[0].first);
+assert(!m.suffix().matched);
+assert(m.suffix().first == m[0].second);
+assert(m.suffix().second == m[0].second);
+assert(m.length(0) >= 0 && static_cast(m.length(0)) == std::char_traits::length(s));
+assert(m.position(0) == 0);
+assert(m.str(0) == s);
+}
+}
Index: libcxx/test/std/re/re.alg/re.alg.search/extended.pass.cpp
===

[PATCH] D37955: [libcxx] Fix invert negative bracket match.

2017-09-19 Thread Tim Shen via Phabricator via cfe-commits
timshen updated this revision to Diff 115851.
timshen added a comment.

Fixed.

Those tests were XFAILing on linux-gnu. I also created 
https://reviews.llvm.org/D38041 to XFAIL only on the failing ones.


https://reviews.llvm.org/D37955

Files:
  libcxx/include/regex
  libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp


Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -0,0 +1,29 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template 
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+//  match_results& m,
+//  const basic_regex& e,
+//  regex_constants::match_flag_type flags = 
regex_constants::match_default);
+
+#include 
+#include 
+#include "test_macros.h"
+
+// PR34310
+int main()
+{
+  assert(std::regex_search("HelloWorld", std::regex("[^\\W]")));
+  assert(std::regex_search("_", std::regex("[^\\W]")));
+  return 0;
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2409,17 +2409,28 @@
 goto __exit;
 }
 }
-if (!__neg_chars_.empty())
+// set of "__found" chars =
+//   union(complement(union(__neg_chars_, __neg_mask_)),
+// other cases...)
+//
+// __neg_chars_ and __neg_mask_'d better be handled together, as there
+// are no short circuit opportunities.
+//
+// In addition, when __neg_mask_/__neg_chars_ is empty, they should be
+// treated as all ones/all chars.
 {
-for (size_t __i = 0; __i < __neg_chars_.size(); ++__i)
-{
-if (__ch == __neg_chars_[__i])
-goto __is_neg_char;
-}
+  const bool __in_neg_mask = (__neg_mask_ == 0) ||
+  __traits_.isctype(__ch, __neg_mask_);
+  const bool __in_neg_chars =
+  __neg_chars_.empty() ||
+  std::find(__neg_chars_.begin(), __neg_chars_.end(), __ch) !=
+  __neg_chars_.end();
+  if (!(__in_neg_mask || __in_neg_chars))
+  {
 __found = true;
 goto __exit;
+  }
 }
-__is_neg_char:
 if (!__ranges_.empty())
 {
 string_type __s2 = __collate_ ?
@@ -2451,11 +2462,6 @@
 __found = true;
 goto __exit;
 }
-if (__neg_mask_ && !__traits_.isctype(__ch, __neg_mask_))
-{
-__found = true;
-goto __exit;
-}
 }
 else
 __found = __negate_;  // force reject


Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -0,0 +1,29 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template 
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+//  match_results& m,
+//  const basic_regex& e,
+//  regex_constants::match_flag_type flags = regex_constants::match_default);
+
+#include 
+#include 
+#include "test_macros.h"
+
+// PR34310
+int main()
+{
+  assert(std::regex_search("HelloWorld", std::regex("[^\\W]")));
+  assert(std::regex_search("_", std::regex("[^\\W]")));
+  return 0;
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2409,17 +2409,28 @@
 goto __exit;
 }
 }
-if (!__neg_chars_.empty())
+// set of "__found" chars =
+//   union(complement(union(__neg_chars_, __neg_mask_)),
+// other cases...)
+//
+// __neg_chars_ and __neg_mask_'d better be handled together, as there
+// are no short circuit opportunities.
+//
+// In addition, when __neg_mask_/__neg_chars_ is empty, they should be
+// treated as all ones/all chars.
 

[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl created this revision.

This patch fixes a regression introduced by r290392 
(https://reviews.llvm.org/D28047 — Make '-disable-llvm-optzns' an alias for 
'-disable-llvm-passes'.)

After r290392, CodeGenOpts.DisableLLVMOpts implicitly disables 
CodeGenOpts.VerifyModule, because the Verifier also happens to be implemented 
as an LLVM pass. This new behavior is undesirable because the Verifier is 
clearly not an optimization. One use-case where this is relevant is when using 
clang to compile bitcode; we don't want the compiler to crash on invalid input 
just because -disable-llvm-optzns is used.


https://reviews.llvm.org/D38042

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGen/verify-debuginfo.ll


Index: test/CodeGen/verify-debuginfo.ll
===
--- /dev/null
+++ test/CodeGen/verify-debuginfo.ll
@@ -0,0 +1,17 @@
+; REQUIRES: x86-registered-target
+; RUN: %clang_cc1 -triple i386-apple-darwin -disable-llvm-optzns -S %s -o - 
2>&1 \
+; RUN:   | FileCheck %s
+; CHECK: invalid global variable ref
+; CHECK: warning: ignoring invalid debug info in {{.*}}.ll
+
+@global = common global i32 0, align 4, !dbg !2
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!5, !6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: 
"adrian", emissionKind: FullDebug, globals: !{!3})
+!1 = !DIFile(filename: "broken.c", directory: "/")
+!2 = !DIGlobalVariableExpression(var: !3, expr: !DIExpression())
+!3 = !DIGlobalVariable(name: "g", scope: !0, file: !1, line: 1, type: !1, 
isLocal: false, isDefinition: true)
+!5 = !{i32 2, !"Dwarf Version", i32 4}
+!6 = !{i32 1, !"Debug Info Version", i32 3}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -461,8 +461,16 @@
   legacy::FunctionPassManager &FPM) {
   // Handle disabling of all LLVM passes, where we want to preserve the
   // internal module before any optimization.
-  if (CodeGenOpts.DisableLLVMPasses)
+  if (CodeGenOpts.DisableLLVMPasses) {
+if (CodeGenOpts.VerifyModule) {
+  ModulePassManager MPM;
+  MPM.addPass(VerifierPass(false));
+  ModuleAnalysisManager MAM;
+  MAM.registerPass([&] { return VerifierAnalysis(); });
+  MPM.run(*TheModule, MAM);
+}
 return;
+  }
 
   // Figure out TargetLibraryInfo.  This needs to be added to MPM and FPM
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)


Index: test/CodeGen/verify-debuginfo.ll
===
--- /dev/null
+++ test/CodeGen/verify-debuginfo.ll
@@ -0,0 +1,17 @@
+; REQUIRES: x86-registered-target
+; RUN: %clang_cc1 -triple i386-apple-darwin -disable-llvm-optzns -S %s -o - 2>&1 \
+; RUN:   | FileCheck %s
+; CHECK: invalid global variable ref
+; CHECK: warning: ignoring invalid debug info in {{.*}}.ll
+
+@global = common global i32 0, align 4, !dbg !2
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!5, !6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "adrian", emissionKind: FullDebug, globals: !{!3})
+!1 = !DIFile(filename: "broken.c", directory: "/")
+!2 = !DIGlobalVariableExpression(var: !3, expr: !DIExpression())
+!3 = !DIGlobalVariable(name: "g", scope: !0, file: !1, line: 1, type: !1, isLocal: false, isDefinition: true)
+!5 = !{i32 2, !"Dwarf Version", i32 4}
+!6 = !{i32 1, !"Debug Info Version", i32 3}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -461,8 +461,16 @@
   legacy::FunctionPassManager &FPM) {
   // Handle disabling of all LLVM passes, where we want to preserve the
   // internal module before any optimization.
-  if (CodeGenOpts.DisableLLVMPasses)
+  if (CodeGenOpts.DisableLLVMPasses) {
+if (CodeGenOpts.VerifyModule) {
+  ModulePassManager MPM;
+  MPM.addPass(VerifierPass(false));
+  ModuleAnalysisManager MAM;
+  MAM.registerPass([&] { return VerifierAnalysis(); });
+  MPM.run(*TheModule, MAM);
+}
 return;
+  }
 
   // Figure out TargetLibraryInfo.  This needs to be added to MPM and FPM
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37913: [OpenMP] Enable the existing nocudalib flag for OpenMP offloading toolchain.

2017-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:255-257
   CudaArch gpu_arch = StringToCudaArch(GPUArchName);
-  assert(gpu_arch != CudaArch::UNKNOWN &&
+  assert((gpu_arch != CudaArch::UNKNOWN ||
+  Args.hasArg(options::OPT_nocudalib)) &&

The purpose of the original assert was to catch a programming error and this 
change negates that purpose.
Perhaps I'm missing something. Could you elaborate on what's the motivation for 
this particular change?

I don't understand why it would be OK to end up with an unknown GPU 
architecture if -nocudalib is specified.
You still do want to pass *some* specific GPU arch to ptxas and that has 
nothing to do with whether you happen to have suitable libdevice.




Repository:
  rL LLVM

https://reviews.llvm.org/D37913



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


[libcxx] r313643 - Resubmit "Fix llvm-lit script generation in libcxx."

2017-09-19 Thread Zachary Turner via cfe-commits
Author: zturner
Date: Tue Sep 19 10:19:10 2017
New Revision: 313643

URL: http://llvm.org/viewvc/llvm-project?rev=313643&view=rev
Log:
Resubmit "Fix llvm-lit script generation in libcxx."

After speaking with the libcxx owners, they agreed that this is
a bug in the bot that needs to be fixed by the bot owners, and
the CMake changes are correct.

Modified:
libcxx/trunk/CMakeLists.txt
libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake
libcxx/trunk/test/CMakeLists.txt

Modified: libcxx/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=313643&r1=313642&r2=313643&view=diff
==
--- libcxx/trunk/CMakeLists.txt (original)
+++ libcxx/trunk/CMakeLists.txt Tue Sep 19 10:19:10 2017
@@ -653,6 +653,7 @@ endif()
 #
 # However, since some submission systems strip test/ subdirectories, check for
 # it before adding it.
+
 if(IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test")
   add_subdirectory(test)
 endif()
@@ -660,6 +661,15 @@ if (LIBCXX_INCLUDE_TESTS)
   add_subdirectory(lib/abi)
 endif()
 
+if (LIBCXX_STANDALONE_BUILD AND EXISTS "${LLVM_MAIN_SRC_DIR}/utils/llvm-lit")
+  # Make sure the llvm-lit script is generated into the bin directory, and do
+  # it after adding all tests, since the generated script will only work
+  # correctly discovered tests against test locations from the source tree that
+  # have already been discovered.
+  add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit
+   ${CMAKE_CURRENT_BINARY_DIR}/llvm-lit)
+endif()
+
 if (LIBCXX_INCLUDE_DOCS)
   add_subdirectory(docs)
 endif()

Modified: libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake?rev=313643&r1=313642&r2=313643&view=diff
==
--- libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake (original)
+++ libcxx/trunk/cmake/Modules/HandleOutOfTreeLLVM.cmake Tue Sep 19 10:19:10 
2017
@@ -106,6 +106,11 @@ macro(configure_out_of_tree_llvm)
 set(LLVM_ENABLE_SPHINX OFF)
   endif()
 
+  # In a standalone build, we don't have llvm to automatically generate the
+  # llvm-lit script for us.  So we need to provide an explicit directory that
+  # the configurator should write the script into.
+  set(LLVM_LIT_OUTPUT_DIR "${libcxx_BINARY_DIR}/bin")
+
   # Required LIT Configuration 
   # Define the default arguments to use with 'lit', and an option for the user
   # to override.

Modified: libcxx/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/CMakeLists.txt?rev=313643&r1=313642&r2=313643&view=diff
==
--- libcxx/trunk/test/CMakeLists.txt (original)
+++ libcxx/trunk/test/CMakeLists.txt Tue Sep 19 10:19:10 2017
@@ -49,10 +49,9 @@ set(LIBCXX_EXECUTOR "None" CACHE STRING
 
 set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not 
edit!")
 
-configure_file(
+configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
-  @ONLY)
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg)
 
 set(LIBCXX_TEST_DEPS "")
 


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


[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

2017-09-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

ping


https://reviews.llvm.org/D36915



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


[PATCH] D37912: [OpenMP] Bugfix: output file name drops the absolute path where full path is needed.

2017-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:441
 
-SmallString<256> Name = llvm::sys::path::filename(II.getFilename());
+SmallString<256> Name = StringRef(II.getFilename());
 llvm::sys::path::replace_extension(Name, "cubin");

I don't think explicit StringRef is needed here. `SmallString<256> 
Name(II.getFilename());` should do the job.


Repository:
  rL LLVM

https://reviews.llvm.org/D37912



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

But, the verifier itself will just "crash". It won't print a stack trace, but I 
don't see why that's much better? And this flag is supposed to be a developer 
option and not a user facing one, so I'm somewhat confused at what the intent 
is here...


https://reviews.llvm.org/D38042



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


[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Ping, would be really nice to get this going :)
Would unblock me for working on the second half of the fix for 
https://bugs.llvm.org/show_bug.cgi?id=34147


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



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


[PATCH] D38040: [OpenMP] Add an additional test for D34888

2017-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

LGTM in general.




Comment at: test/OpenMP/target_map_codegen.cpp:4845
+///==///
+// RUN: %clang_cc1 -DCK30 -std=c++11 -fopenmp -S -emit-llvm -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda %s -o - 2>&1 | FileCheck 
-check-prefix=CK30 %s
+

Please break the line to make it easier to read.


Repository:
  rL LLVM

https://reviews.llvm.org/D38040



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

In https://reviews.llvm.org/D38042#875303, @chandlerc wrote:

> But, the verifier itself will just "crash". It won't print a stack trace, but 
> I don't see why that's much better? And this flag is supposed to be a 
> developer option and not a user facing one, so I'm somewhat confused at what 
> the intent is here...


No, it will report_fatal_error() instead of crashing the compiler later on.
In any case, that is not my primary motivation: The intent is exactly what is 
illustrated by the testcase, stripping malformed debug info metadata produced 
by older, buggy versions of clang. The backstory to this is that historically 
the Verifier was very weak when it came to verifying debug info. To allow us to 
make the Verifier better (stricter), while still supporting importing of older 
.bc files produced by older compilers, we added a mechanism that strips all 
debug info metadata if the verification of the debug info failed, but the 
bitcode is otherwise correct.


https://reviews.llvm.org/D38042



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D38042#875328, @aprantl wrote:

> In https://reviews.llvm.org/D38042#875303, @chandlerc wrote:
>
> > But, the verifier itself will just "crash". It won't print a stack trace, 
> > but I don't see why that's much better? And this flag is supposed to be a 
> > developer option and not a user facing one, so I'm somewhat confused at 
> > what the intent is here...
>
>
> No, it will report_fatal_error() instead of crashing the compiler later on.
>  In any case, that is not my primary motivation: The intent is exactly what 
> is illustrated by the testcase, stripping malformed debug info metadata 
> produced by older, buggy versions of clang. The backstory to this is that 
> historically the Verifier was very weak when it came to verifying debug info. 
> To allow us to make the Verifier better (stricter), while still supporting 
> importing of older .bc files produced by older compilers, we added a 
> mechanism that strips all debug info metadata if the verification of the 
> debug info failed, but the bitcode is otherwise correct.


Ok, that use case makes way more sense. I'd replace the change description with 
some discussion of this use case.

My next question is -- why is this done by the verifier? It seems *really* bad 
that the verifier *changes the IR*. Don't get me wrong, what you're trying to 
do (strip malformed debug info) makes perfect sense. But I think that the thing 
which does that shouldn't be called a verifier. It might *use* the verifier of 
course.

Last but not least, I still suspect that this shouldn't be run here. If the 
user wants to disable LLVM passes *and emits LLVM IR*, they should get it 
unperturbed. The stripping of malformed debug info seems like it should happen 
later as part of the passes to emit code, and I'd actually expect the LLVM code 
generator to add the necessary pass rather than relying on every frontend 
remembering to do so?


https://reviews.llvm.org/D38042



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


r313653 - Fix ClangDiagnosticHandler::is*RemarkEnabled members

2017-09-19 Thread Adam Nemet via cfe-commits
Author: anemet
Date: Tue Sep 19 10:59:40 2017
New Revision: 313653

URL: http://llvm.org/viewvc/llvm-project?rev=313653&view=rev
Log:
Fix ClangDiagnosticHandler::is*RemarkEnabled members

Apparently these weren't really working. I added test coverage and fixed the
typo in the name and the parameter.

Added:
cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c
Modified:
cfe/trunk/lib/CodeGen/CodeGenAction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=313653&r1=313652&r2=313653&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Tue Sep 19 10:59:40 2017
@@ -53,19 +53,20 @@ namespace clang {
 : CodeGenOpts(CGOpts), BackendCon(BCon) {}
   
 bool handleDiagnostics(const DiagnosticInfo &DI) override;
-bool isAnalysisRemarkEnable(const std::string &PassName) {
+
+bool isAnalysisRemarkEnabled(StringRef PassName) const override {
   return (CodeGenOpts.OptimizationRemarkAnalysisPattern &&
   CodeGenOpts.OptimizationRemarkAnalysisPattern->match(PassName));
 }
-bool isMissedOptRemarkEnable(const std::string &PassName) {
+bool isMissedOptRemarkEnabled(StringRef PassName) const override {
   return (CodeGenOpts.OptimizationRemarkMissedPattern &&
   CodeGenOpts.OptimizationRemarkMissedPattern->match(PassName));
 }
-bool isPassedOptRemarkEnable(const std::string &PassName) {
+bool isPassedOptRemarkEnabled(StringRef PassName) const override {
   return (CodeGenOpts.OptimizationRemarkPattern &&
   CodeGenOpts.OptimizationRemarkPattern->match(PassName));
 }
-  
+
   private:
 const CodeGenOptions &CodeGenOpts;
 BackendConsumer *BackendCon;

Added: cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c?rev=313653&view=auto
==
--- cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c (added)
+++ cfe/trunk/test/Frontend/optimization-remark-extra-analysis.c Tue Sep 19 
10:59:40 2017
@@ -0,0 +1,11 @@
+// Test that the is*RemarkEnabled overrides are working properly.  This remark
+// requiring extra analysis is only conditionally enabled.
+
+// RUN: %clang_cc1 %s -Rpass-missed=gvn -O2 -emit-llvm-only -verify
+
+int foo(int *x, int *y) {
+  int a = *x;
+  *y = 2;
+  // expected-remark@+1 {{load of type i32 not eliminated}}
+  return a + *x;
+}


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


[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-09-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

The overall idea makes sense to me. I'd like you to join the effort with Peter 
who during his work on loop widening came up with a matcher-based procedure for 
finding out if a variable is changed anywhere; it currently lives in 
`LoopUnrolling.cpp` and we need only once implementation of that.


Repository:
  rL LLVM

https://reviews.llvm.org/D37897



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


[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8593
 
-  bool Match = true;
+  // is this a tautological comparison? if yes, than contains the always-result
+  llvm::Optional Result;

Comments should be complete sentences with capitalization and punctuation (here 
and elsewhere in the patch).

Also, I can't really understand what this comment is saying.



Comment at: lib/Sema/SemaChecking.cpp:8596
+  Expr *Value; // which one is the value, and not a constant?
+  const char *cmp; // the comparison used, as string
 

Should be `Cmp` per coding standards.


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

In https://reviews.llvm.org/D38042#875334, @chandlerc wrote:

> In https://reviews.llvm.org/D38042#875328, @aprantl wrote:
>
> > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote:
> >
> > > But, the verifier itself will just "crash". It won't print a stack trace, 
> > > but I don't see why that's much better? And this flag is supposed to be a 
> > > developer option and not a user facing one, so I'm somewhat confused at 
> > > what the intent is here...
> >
> >
> > No, it will report_fatal_error() instead of crashing the compiler later on.
> >  In any case, that is not my primary motivation: The intent is exactly what 
> > is illustrated by the testcase, stripping malformed debug info metadata 
> > produced by older, buggy versions of clang. The backstory to this is that 
> > historically the Verifier was very weak when it came to verifying debug 
> > info. To allow us to make the Verifier better (stricter), while still 
> > supporting importing of older .bc files produced by older compilers, we 
> > added a mechanism that strips all debug info metadata if the verification 
> > of the debug info failed, but the bitcode is otherwise correct.
>
>
> Ok, that use case makes way more sense. I'd replace the change description 
> with some discussion of this use case.
>
> My next question is -- why is this done by the verifier? It seems *really* 
> bad that the verifier *changes the IR*. Don't get me wrong, what you're 
> trying to do (strip malformed debug info) makes perfect sense. But I think 
> that the thing which does that shouldn't be called a verifier. It might *use* 
> the verifier of course.


That was a purely pragmatic decision: Most (but not all) LLVM-based tools are 
running the Verifier as an LLVM pass so adding the stripping into the pass was 
the least invasive way of implementing this feature. If we are truly bothered 
by this, I think what could work is to separate out a second 
StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to run 
immediately after it. I don't see this adding much value though, and we would 
have to modify all tools to schedule the new pass explicitly. Do you think that 
would be worth pursuing?

> Last but not least, I still suspect that this shouldn't be run here. If the 
> user wants to disable LLVM passes *and emits LLVM IR*, they should get it 
> unperturbed. The stripping of malformed debug info seems like it should 
> happen later as part of the passes to emit code, and I'd actually expect the 
> LLVM code generator to add the necessary pass rather than relying on every 
> frontend remembering to do so?

The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not LLVM 
passes.
Also, I believe the Verifier is special. The Verifier protects LLVM from 
crashing and as a user I think I would prefer a Verifier error over clang 
crashing while emitting bitcode. Because of auto-upgrades users already don't 
get the IR unperturbed, and I view stripping of broken debug info as a (in all 
fairness: very brutal :-) auto-upgrade.


https://reviews.llvm.org/D38042



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D38042#875412, @aprantl wrote:

> In https://reviews.llvm.org/D38042#875334, @chandlerc wrote:
>
> > In https://reviews.llvm.org/D38042#875328, @aprantl wrote:
> >
> > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote:
> > >
> > > > But, the verifier itself will just "crash". It won't print a stack 
> > > > trace, but I don't see why that's much better? And this flag is 
> > > > supposed to be a developer option and not a user facing one, so I'm 
> > > > somewhat confused at what the intent is here...
> > >
> > >
> > > No, it will report_fatal_error() instead of crashing the compiler later 
> > > on.
> > >  In any case, that is not my primary motivation: The intent is exactly 
> > > what is illustrated by the testcase, stripping malformed debug info 
> > > metadata produced by older, buggy versions of clang. The backstory to 
> > > this is that historically the Verifier was very weak when it came to 
> > > verifying debug info. To allow us to make the Verifier better (stricter), 
> > > while still supporting importing of older .bc files produced by older 
> > > compilers, we added a mechanism that strips all debug info metadata if 
> > > the verification of the debug info failed, but the bitcode is otherwise 
> > > correct.
> >
> >
> > Ok, that use case makes way more sense. I'd replace the change description 
> > with some discussion of this use case.
> >
> > My next question is -- why is this done by the verifier? It seems *really* 
> > bad that the verifier *changes the IR*. Don't get me wrong, what you're 
> > trying to do (strip malformed debug info) makes perfect sense. But I think 
> > that the thing which does that shouldn't be called a verifier. It might 
> > *use* the verifier of course.
>
>
> That was a purely pragmatic decision: Most (but not all) LLVM-based tools are 
> running the Verifier as an LLVM pass so adding the stripping into the pass 
> was the least invasive way of implementing this feature. If we are truly 
> bothered by this, I think what could work is to separate out a second 
> StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to 
> run immediately after it. I don't see this adding much value though, and we 
> would have to modify all tools to schedule the new pass explicitly. Do you 
> think that would be worth pursuing?


Absolutely. I think the verifier should never, under any circumstances, mutate 
the IR. Think about it, with the current design if a pass corrupts the debug 
info the verifier may "hide" this by stripping it out rather than allowing us 
to find it.

> 
> 
>> Last but not least, I still suspect that this shouldn't be run here. If the 
>> user wants to disable LLVM passes *and emits LLVM IR*, they should get it 
>> unperturbed. The stripping of malformed debug info seems like it should 
>> happen later as part of the passes to emit code, and I'd actually expect the 
>> LLVM code generator to add the necessary pass rather than relying on every 
>> frontend remembering to do so?
> 
> The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not 
> LLVM passes.

(sorry for the off-list duplication, but it belongs here anyways)

I disagree. `-disable-llvm-optzns` is a developer flag, and was almost an alias 
for `-disable-llvm-passes`. After discussion on the list we made it an actual 
legacy and deprecated alias for `-disable-llvm-passes` because the latter was 
more clear, understandable, and correct. We had cases where the passes run by 
`-disable-llvm-optzns` were actually problematic and we wanted to debug them 
but were unable to do so.

> Also, I believe the Verifier is special. The Verifier protects LLVM from 
> crashing and as a user I think I would prefer a Verifier error over clang 
> crashing while emitting bitcode.

I think this distinction is not a meaningful one ultimately. And the verifier 
should *never* be a functional requirement because it should have no effect. 
I'm happy for us to verify when we read input, but even then it should not 
mutate the IR.

> Because of auto-upgrades users already don't get the IR unperturbed, and I 
> view stripping of broken debug info as a (in all fairness: very brutal :-) 
> auto-upgrade.

But auto-upgrade happens on *read*. If you want to add the debug info stripping 
to auto-upgrade, that's a reasonable discussion to have, and no change here 
will be required. There might be concerns about that on the LLVM side, I don't 
know. But the verifier (as well as running it here) does not seem like the 
right solution.


https://reviews.llvm.org/D38042



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


[PATCH] D38048: [clangd] Add textDocument/signatureHelp

2017-09-19 Thread Raoul Wols via Phabricator via cfe-commits
rwols created this revision.

Makes clangd respond to a client's "textDocument/signatureHelp" request by
presenting function/method overloads.
Still need to add tests.
Also, there is duplicate code in clangd::codeComplete and clangd::signatureHelp
now, maybe refactor this to a common function?


https://reviews.llvm.org/D38048

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h

Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -46,6 +46,8 @@
 JSONOutput &Out) = 0;
   virtual void onCompletion(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput &Out) = 0;
+  virtual void onSignatureHelp(TextDocumentPositionParams Params, StringRef ID,
+   JSONOutput &Out) = 0;
   virtual void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput &Out) = 0;
 };
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -186,6 +186,23 @@
   ProtocolCallbacks &Callbacks;
 };
 
+struct SignatureHelpHandler : Handler {
+  SignatureHelpHandler(JSONOutput &Output, ProtocolCallbacks &Callbacks)
+  : Handler(Output), Callbacks(Callbacks) {}
+
+  void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override {
+auto TDPP = TextDocumentPositionParams::parse(Params);
+if (!TDPP) {
+  Output.log("Failed to decode TextDocumentPositionParams!\n");
+  return;
+}
+Callbacks.onSignatureHelp(*TDPP, ID, Output);
+  }
+
+private:
+  ProtocolCallbacks &Callbacks;
+};
+
 struct GotoDefinitionHandler : Handler {
   GotoDefinitionHandler(JSONOutput &Output, ProtocolCallbacks &Callbacks)
   : Handler(Output), Callbacks(Callbacks) {}
@@ -238,6 +255,9 @@
   "textDocument/completion",
   llvm::make_unique(Out, Callbacks));
   Dispatcher.registerHandler(
+  "textDocument/signatureHelp",
+  llvm::make_unique(Out, Callbacks));
+  Dispatcher.registerHandler(
   "textDocument/definition",
   llvm::make_unique(Out, Callbacks));
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -400,6 +400,71 @@
   static std::string unparse(const CompletionItem &P);
 };
 
+/// Represents a parameter of a callable-signature.
+///
+/// A parameter can have a label and a doc-comment.
+struct ParameterInformation {
+
+  /// The label of this parameter. Will be shown in the UI.
+  std::string label;
+
+  /// The human-readable doc-comment of this parameter. Will be shown in the UI
+  /// but can be omitted.
+  std::string documentation;
+
+  static std::string unparse(const ParameterInformation &);
+};
+
+/// Represents the signature of something callable.
+///
+/// A signature can have a label, like a function-name, a doc-comment, and a set
+/// of parameters.
+struct SignatureInformation {
+
+  /// The label of this signature. Will be shown in the UI.
+  std::string label;
+
+  /// The human-readable doc-comment of this signature. Will be shown in the UI
+  /// but can be omitted.
+  std::string documentation;
+
+  /// The parameters of this signature.
+  std::vector parameters;
+
+  static std::string unparse(const SignatureInformation &);
+};
+
+/// Signature help represents the signature of something callable.
+///
+/// There can be multiple signature but only one active and only one active
+/// parameter.
+struct SignatureHelp {
+
+  /// One or more signatures.
+  std::vector signatures;
+
+  /// The active signature.
+  ///
+  /// If omitted or the value lies outside the range of `signatures` the value
+  /// defaults to zero or is ignored if `signatures.length === 0`. Whenever
+  /// possible implementors should make an active decision about the active
+  /// signature and shouldn't rely on a default value. In future version of the
+  /// protocol this property might become mandantory to better express this.
+  int activeSignature = 0;
+
+  /// The active parameter of the active signature.
+  ///
+  /// If omitted or the value lies outside the range of
+  /// `signatures[activeSignature].parameters` defaults to 0 if the active
+  /// signature has parameters. If the active signature has no parameters it is
+  /// ignored. In future version of the protocol this property might become
+  /// mandantory to better express the active parameter if the active signature
+  /// does have any.
+  int activeParameter = 0;
+
+  static std::string unparse(const SignatureHelp &);
+};
+
 } // namespace clangd
 } // namespace clang
 
Index: clangd/Protocol.cpp
==

[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

In https://reviews.llvm.org/D38042#875412, @aprantl wrote:

> In https://reviews.llvm.org/D38042#875334, @chandlerc wrote:
>
> > In https://reviews.llvm.org/D38042#875328, @aprantl wrote:
> >
> > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote:
> > >
> > > > But, the verifier itself will just "crash". It won't print a stack 
> > > > trace, but I don't see why that's much better? And this flag is 
> > > > supposed to be a developer option and not a user facing one, so I'm 
> > > > somewhat confused at what the intent is here...
> > >
> > >
> > > No, it will report_fatal_error() instead of crashing the compiler later 
> > > on.
> > >  In any case, that is not my primary motivation: The intent is exactly 
> > > what is illustrated by the testcase, stripping malformed debug info 
> > > metadata produced by older, buggy versions of clang. The backstory to 
> > > this is that historically the Verifier was very weak when it came to 
> > > verifying debug info. To allow us to make the Verifier better (stricter), 
> > > while still supporting importing of older .bc files produced by older 
> > > compilers, we added a mechanism that strips all debug info metadata if 
> > > the verification of the debug info failed, but the bitcode is otherwise 
> > > correct.
> >
> >
> > Ok, that use case makes way more sense. I'd replace the change description 
> > with some discussion of this use case.
> >
> > My next question is -- why is this done by the verifier? It seems *really* 
> > bad that the verifier *changes the IR*. Don't get me wrong, what you're 
> > trying to do (strip malformed debug info) makes perfect sense. But I think 
> > that the thing which does that shouldn't be called a verifier. It might 
> > *use* the verifier of course.
>
>
> That was a purely pragmatic decision: Most (but not all) LLVM-based tools are 
> running the Verifier as an LLVM pass so adding the stripping into the pass 
> was the least invasive way of implementing this feature. If we are truly 
> bothered by this, I think what could work is to separate out a second 
> StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to 
> run immediately after it. I don't see this adding much value though, and we 
> would have to modify all tools to schedule the new pass explicitly. Do you 
> think that would be worth pursuing?
>
> > Last but not least, I still suspect that this shouldn't be run here. If the 
> > user wants to disable LLVM passes *and emits LLVM IR*, they should get it 
> > unperturbed. The stripping of malformed debug info seems like it should 
> > happen later as part of the passes to emit code, and I'd actually expect 
> > the LLVM code generator to add the necessary pass rather than relying on 
> > every frontend remembering to do so?
>
> The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not 
> LLVM passes.


It was just pointed out that `-disable-llvm-optzns` is now a deprecated alias 
for `-disable-llvm-passes`, so I see that this argument makes less sense today.

Do you think that `-disable-llvm-passes` should imply `-disable-llvm-verifier`?
Is it obvious to users the the Verifier is a pass?
If yes, how about adding a positive version of `-disable-llvm-verifier`?

> Also, I believe the Verifier is special. The Verifier protects LLVM from 
> crashing and as a user I think I would prefer a Verifier error over clang 
> crashing while emitting bitcode. Because of auto-upgrades users already don't 
> get the IR unperturbed, and I view stripping of broken debug info as a (in 
> all fairness: very brutal :-) auto-upgrade.


https://reviews.llvm.org/D38042



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


[PATCH] D37903: Fix assume-filename handling in clang-format.el

2017-09-19 Thread Micah Werbitt via Phabricator via cfe-commits
werbitt updated this revision to Diff 115875.
werbitt edited the summary of this revision.
werbitt added a comment.

Rename assume-file to assume-file-name and update documentation


https://reviews.llvm.org/D37903

Files:
  tools/clang-format/clang-format.el


Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,20 +119,22 @@
   (byte-to-position (1+ byte)
 
 ;;;###autoload
-(defun clang-format-region (start end &optional style assume-file)
-  "Use clang-format to format the code between START and END according to 
STYLE.
-If called interactively uses the region or the current statement if there
-is no active region.  If no style is given uses `clang-format-style'."
+(defun clang-format-region (start end &optional style assume-file-name)
+  "Use clang-format to format the code between START and END according to STYLE
+using ASSUME-FILE-NAME to locate a style config file. If called interactively
+uses the region or the current statement if there is no active region. If no
+style is given uses `clang-format-style'. If no assume-file-name is given uses
+`buffer-file-name'."
   (interactive
(if (use-region-p)
(list (region-beginning) (region-end))
  (list (point) (point
 
   (unless style
 (setq style clang-format-style))
 
-  (unless assume-file
-(setq assume-file buffer-file-name))
+  (unless assume-file-name
+(setq assume-file-name buffer-file-name))
 
   (let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
 'utf-8-unix))
@@ -151,7 +153,12 @@
  nil nil clang-format-executable
  nil `(,temp-buffer ,temp-file) nil
  `("-output-replacements-xml"
-   ,@(and assume-file (list "-assume-filename" 
assume-file))
+   ;; Gaurd against a nil assume-file-name.
+   ;; If -assume-filename is given a blank string
+   ;; it will crash as per the following bug report
+   ;; https://bugs.llvm.org/show_bug.cgi?id=34667
+   ,@(and assume-file-name
+  (list "-assume-filename" 
assume-file-name))
"-style" ,style
"-offset" ,(number-to-string file-start)
"-length" ,(number-to-string (- file-end 
file-start))
@@ -183,10 +190,12 @@
   (when (buffer-name temp-buffer) (kill-buffer temp-buffer)
 
 ;;;###autoload
-(defun clang-format-buffer (&optional style assume-file)
-  "Use clang-format to format the current buffer according to STYLE."
+(defun clang-format-buffer (&optional style assume-file-name)
+  "Use clang-format to format the current buffer according to STYLE using
+ASSUME-FILE-NAME to locate a style config file. If no style is given uses
+`clang-format-style'. If no assume-file-name is given uses `buffer-file-name'."
   (interactive)
-  (clang-format-region (point-min) (point-max) style assume-file))
+  (clang-format-region (point-min) (point-max) style assume-file-name))
 
 ;;;###autoload
 (defalias 'clang-format 'clang-format-region)


Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,20 +119,22 @@
   (byte-to-position (1+ byte)
 
 ;;;###autoload
-(defun clang-format-region (start end &optional style assume-file)
-  "Use clang-format to format the code between START and END according to STYLE.
-If called interactively uses the region or the current statement if there
-is no active region.  If no style is given uses `clang-format-style'."
+(defun clang-format-region (start end &optional style assume-file-name)
+  "Use clang-format to format the code between START and END according to STYLE
+using ASSUME-FILE-NAME to locate a style config file. If called interactively
+uses the region or the current statement if there is no active region. If no
+style is given uses `clang-format-style'. If no assume-file-name is given uses
+`buffer-file-name'."
   (interactive
(if (use-region-p)
(list (region-beginning) (region-end))
  (list (point) (point
 
   (unless style
 (setq style clang-format-style))
 
-  (unless assume-file
-(setq assume-file buffer-file-name))
+  (unless assume-file-name
+(setq assume-file-name buffer-file-name))
 
   (let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
 'utf-8-unix))
@@ -151,7 +153,12 @@
  nil nil clang-format-executable
  nil `(,temp-buffer ,temp

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 115877.
lebedev.ri added a comment.

1. Correctly added `TautologicalUnsignedEnumZeroCompare` to 
`TautologicalCompare` group (sigh)
2. Hopefully fixed spelling of the comments and added a bit better comments
3. Fixed variable names to comply with the coding style.


Repository:
  rL LLVM

https://reviews.llvm.org/D37629

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.c

Index: test/Sema/tautological-unsigned-enum-zero-compare.c
===
--- /dev/null
+++ test/Sema/tautological-unsigned-enum-zero-compare.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+
+int main() {
+  enum A { A_foo, A_bar };
+  enum A a;
+
+#ifdef TEST
+  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+return 0;
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+return 0;
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+return 0;
+#else
+  // expected-no-diagnostics
+  if (a < 0)
+return 0;
+  if (a >= 0)
+return 0;
+  if (0 <= a)
+return 0;
+  if (0 > a)
+return 0;
+  if (a < 0U)
+return 0;
+  if (a >= 0U)
+return 0;
+  if (0U <= a)
+return 0;
+  if (0U > a)
+return 0;
+#endif
+
+  return 1;
+}
Index: test/Sema/compare.c
===
--- test/Sema/compare.c
+++ test/Sema/compare.c
@@ -308,8 +308,59 @@
 int rdar8511238() {
   enum A { A_foo, A_bar };
   enum A a;
+
+  if (a == 0)
+  return 0;
+  if (a != 0)
+  return 0;
   if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
+  return 0;
+  if (a <= 0)
+  return 0;
+  if (a > 0)
+  return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  return 0;
+
+  if (0 == a)
+  return 0;
+  if (0 != a)
+  return 0;
+  if (0 < a)
+  return 0;
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  return 0;
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  return 0;
+  if (0 >= a)
+  return 0;
+
+  if (a == 0U)
+  return 0;
+  if (a != 0U)
+  return 0;
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  return 0;
+  if (a <= 0U)
+  return 0;
+  if (a > 0U)
+  return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  return 0;
+
+  if (0U == a)
+  return 0;
+  if (0U != a)
+  return 0;
+  if (0U < a)
+  return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  return 0;
+  if (0U >= a)
+  return 0;
+
   return 20;
 }
 
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8583,35 +8583,89 @@
 
   // bool values are handled by DiagnoseOutOfRangeComparison().
 
-  BinaryOperatorKind op = E->getOpcode();
+  BinaryOperatorKind Op = E->getOpcode();
   if (E->isValueDependent())
 return false;
 
   Expr *LHS = E->getLHS();
   Expr *RHS = E->getRHS();
 
-  bool Match = true;
+  // Result is used for two purposes. If the Result is empty, then we decided
+  // that given comparison is non-tautological, and no warning should emitted.
+  // Alternatively, if the Result is not empty, then it contains the result of
+  // the comparison. Since the comparison is tautological, it always evaluates
+  // to the same result - be it either true or false.
+  llvm::Optional Result;
 
-  if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
-  << "< 0" << "false" << HasEnumType(LHS)
-  << LHS->getSour

[PATCH] D37413: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions

2017-09-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D37413



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


[PATCH] D37466: D37461: fixups for existing InlineAsm tests + adding new ones

2017-09-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: test/CodeGen/ms-inline-asm.cpp:37-38
-  int lvar = 10;
-  __asm mov eax, offset Foo::ptr
-  __asm mov eax, offset Foo::Bar::ptr
-// CHECK-LABEL: define void @_Z2t2v()

These don't seem tested anywhere now


Repository:
  rL LLVM

https://reviews.llvm.org/D37466



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

In https://reviews.llvm.org/D38042#875418, @chandlerc wrote:

> In https://reviews.llvm.org/D38042#875412, @aprantl wrote:
>
> > In https://reviews.llvm.org/D38042#875334, @chandlerc wrote:
> >
> > > In https://reviews.llvm.org/D38042#875328, @aprantl wrote:
> > >
> > > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote:
> > > >
> > > > > But, the verifier itself will just "crash". It won't print a stack 
> > > > > trace, but I don't see why that's much better? And this flag is 
> > > > > supposed to be a developer option and not a user facing one, so I'm 
> > > > > somewhat confused at what the intent is here...
> > > >
> > > >
> > > > No, it will report_fatal_error() instead of crashing the compiler later 
> > > > on.
> > > >  In any case, that is not my primary motivation: The intent is exactly 
> > > > what is illustrated by the testcase, stripping malformed debug info 
> > > > metadata produced by older, buggy versions of clang. The backstory to 
> > > > this is that historically the Verifier was very weak when it came to 
> > > > verifying debug info. To allow us to make the Verifier better 
> > > > (stricter), while still supporting importing of older .bc files 
> > > > produced by older compilers, we added a mechanism that strips all debug 
> > > > info metadata if the verification of the debug info failed, but the 
> > > > bitcode is otherwise correct.
> > >
> > >
> > > Ok, that use case makes way more sense. I'd replace the change 
> > > description with some discussion of this use case.
> > >
> > > My next question is -- why is this done by the verifier? It seems 
> > > *really* bad that the verifier *changes the IR*. Don't get me wrong, what 
> > > you're trying to do (strip malformed debug info) makes perfect sense. But 
> > > I think that the thing which does that shouldn't be called a verifier. It 
> > > might *use* the verifier of course.
> >
> >
> > That was a purely pragmatic decision: Most (but not all) LLVM-based tools 
> > are running the Verifier as an LLVM pass so adding the stripping into the 
> > pass was the least invasive way of implementing this feature. If we are 
> > truly bothered by this, I think what could work is to separate out a second 
> > StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to 
> > run immediately after it. I don't see this adding much value though, and we 
> > would have to modify all tools to schedule the new pass explicitly. Do you 
> > think that would be worth pursuing?
>
>
> Absolutely. I think the verifier should never, under any circumstances, 
> mutate the IR. Think about it, with the current design if a pass corrupts the 
> debug info the verifier may "hide" this by stripping it out rather than 
> allowing us to find it.


Okay, I think I agree. Before I venture off implementing this, do you think 
that separating out a StripBrokenDebugInfoPass that depends on the Verifier is 
right way forward?

>> 
>> 
>>> Last but not least, I still suspect that this shouldn't be run here. If the 
>>> user wants to disable LLVM passes *and emits LLVM IR*, they should get it 
>>> unperturbed. The stripping of malformed debug info seems like it should 
>>> happen later as part of the passes to emit code, and I'd actually expect 
>>> the LLVM code generator to add the necessary pass rather than relying on 
>>> every frontend remembering to do so?
>> 
>> The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not 
>> LLVM passes.
> 
> (sorry for the off-list duplication, but it belongs here anyways)
> 
> I disagree. `-disable-llvm-optzns` is a developer flag, and was almost an 
> alias for `-disable-llvm-passes`. After discussion on the list we made it an 
> actual legacy and deprecated alias for `-disable-llvm-passes` because the 
> latter was more clear, understandable, and correct. We had cases where the 
> passes run by `-disable-llvm-optzns` were actually problematic and we wanted 
> to debug them but were unable to do so.
> 
>> Also, I believe the Verifier is special. The Verifier protects LLVM from 
>> crashing and as a user I think I would prefer a Verifier error over clang 
>> crashing while emitting bitcode.
> 
> I think this distinction is not a meaningful one ultimately. And the verifier 
> should *never* be a functional requirement because it should have no effect. 
> I'm happy for us to verify when we read input, but even then it should not 
> mutate the IR.
> 
>> Because of auto-upgrades users already don't get the IR unperturbed, and I 
>> view stripping of broken debug info as a (in all fairness: very brutal :-) 
>> auto-upgrade.
> 
> But auto-upgrade happens on *read*. If you want to add the debug info 
> stripping to auto-upgrade, that's a reasonable discussion to have, and no 
> change here will be required. There might be concerns about that on the LLVM 
> side, I don't know. But the verifier (as well as running it here) does not 
> seem like the right solution.

Would 

[PATCH] D37544: [ubsan] Skip alignment checks which are folded away

2017-09-19 Thread Vedant Kumar via Phabricator via cfe-commits
vsk updated this revision to Diff 115879.
vsk added a comment.

- Use a better test case. This one was lifted from an actual example in 
X86CallingConv.h:86.


https://reviews.llvm.org/D37544

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/ubsan-suppress-checks.cpp


Index: test/CodeGenCXX/ubsan-suppress-checks.cpp
===
--- test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -17,6 +17,16 @@
   // CHECK: ret void
 }
 
+// CHECK-LABEL: define void @_Z31use_us16_aligned_array_elementsv
+void use_us16_aligned_array_elements() {
+  static const unsigned short Arr[] = {0, 1, 2};
+  auto use_array = [](const unsigned short(&X)[3]) -> void {};
+  use_array(Arr);
+
+  // CHECK-NOT: and i64 {{.*}}, !nosanitize
+  // CHECK: ret void
+}
+
 struct A {
   int foo;
 
@@ -229,4 +239,5 @@
   d->load_member_3();
 
   load_non_null_pointers();
+  use_us16_aligned_array_elements();
 }
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -618,6 +618,7 @@
   auto PtrToAlloca =
   dyn_cast(Ptr->stripPointerCastsNoFollowAliases());
 
+  llvm::Value *True = llvm::ConstantInt::getTrue(getLLVMContext());
   llvm::Value *IsNonNull = nullptr;
   bool IsGuaranteedNonNull =
   SkippedChecks.has(SanitizerKind::Null) || PtrToAlloca;
@@ -629,8 +630,7 @@
 
 // The IR builder can constant-fold the null check if the pointer points to
 // a constant.
-IsGuaranteedNonNull =
-IsNonNull == llvm::ConstantInt::getTrue(getLLVMContext());
+IsGuaranteedNonNull = IsNonNull == True;
 
 // Skip the null check if the pointer is known to be non-null.
 if (!IsGuaranteedNonNull) {
@@ -684,7 +684,8 @@
   PtrAsInt, llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));
   llvm::Value *Aligned =
   Builder.CreateICmpEQ(Align, llvm::ConstantInt::get(IntPtrTy, 0));
-  Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment));
+  if (Aligned != True)
+Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment));
 }
   }
 


Index: test/CodeGenCXX/ubsan-suppress-checks.cpp
===
--- test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -17,6 +17,16 @@
   // CHECK: ret void
 }
 
+// CHECK-LABEL: define void @_Z31use_us16_aligned_array_elementsv
+void use_us16_aligned_array_elements() {
+  static const unsigned short Arr[] = {0, 1, 2};
+  auto use_array = [](const unsigned short(&X)[3]) -> void {};
+  use_array(Arr);
+
+  // CHECK-NOT: and i64 {{.*}}, !nosanitize
+  // CHECK: ret void
+}
+
 struct A {
   int foo;
 
@@ -229,4 +239,5 @@
   d->load_member_3();
 
   load_non_null_pointers();
+  use_us16_aligned_array_elements();
 }
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -618,6 +618,7 @@
   auto PtrToAlloca =
   dyn_cast(Ptr->stripPointerCastsNoFollowAliases());
 
+  llvm::Value *True = llvm::ConstantInt::getTrue(getLLVMContext());
   llvm::Value *IsNonNull = nullptr;
   bool IsGuaranteedNonNull =
   SkippedChecks.has(SanitizerKind::Null) || PtrToAlloca;
@@ -629,8 +630,7 @@
 
 // The IR builder can constant-fold the null check if the pointer points to
 // a constant.
-IsGuaranteedNonNull =
-IsNonNull == llvm::ConstantInt::getTrue(getLLVMContext());
+IsGuaranteedNonNull = IsNonNull == True;
 
 // Skip the null check if the pointer is known to be non-null.
 if (!IsGuaranteedNonNull) {
@@ -684,7 +684,8 @@
   PtrAsInt, llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));
   llvm::Value *Aligned =
   Builder.CreateICmpEQ(Align, llvm::ConstantInt::get(IntPtrTy, 0));
-  Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment));
+  if (Aligned != True)
+Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment));
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values

2017-09-19 Thread Rachel Craik via Phabricator via cfe-commits
rcraik created this revision.

When the value specified for //n// in ordered(//n//) is larger than the number 
of loops a segmentation fault can occur in one of two ways when attempting to 
print out a diagnostic for an associated depend(sink : //vec//):

1. The iteration vector //vec// contains less than //n// items
2. The iteration vector //vec// contains a variable that is not a loop control 
variable

This patch addresses both of these issues.


https://reviews.llvm.org/D38049

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/ordered_messages.cpp


Index: test/OpenMP/ordered_messages.cpp
===
--- test/OpenMP/ordered_messages.cpp
+++ test/OpenMP/ordered_messages.cpp
@@ -270,5 +270,13 @@
 }
   }
 
+#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' 
clause}}
+  for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops 
after '#pragma omp for', but found only 1}}
+#pragma omp ordered depend(sink : i)
+int j;
+#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop 
iteration variable}}
+foo();
+  }
+
   return foo(); // expected-note {{in instantiation of function template 
specialization 'foo' requested here}}
 }
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -10510,9 +10510,14 @@
 if (!CurContext->isDependentContext() &&
 DSAStack->getParentOrderedRegionParam() &&
 DepCounter != DSAStack->isParentLoopControlVariable(D).first) {
-  Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
-  << DSAStack->getParentLoopControlVariable(
- DepCounter.getZExtValue());
+  ValueDecl* VD = DSAStack->getParentLoopControlVariable(
+  DepCounter.getZExtValue());
+  if (VD) {
+Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
+<< 1 << VD;
+  } else {
+ Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 
0;
+  }
   continue;
 }
 OpsOffs.push_back({RHS, OOK});
@@ -10545,8 +10550,9 @@
 
 if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink &&
 TotalDepCount > VarList.size() &&
-DSAStack->getParentOrderedRegionParam()) {
-  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration)
+DSAStack->getParentOrderedRegionParam() &&
+DSAStack->getParentLoopControlVariable(VarList.size() + 1)) {
+  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1
   << DSAStack->getParentLoopControlVariable(VarList.size() + 1);
 }
 if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink &&
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8869,7 +8869,7 @@
 def err_omp_depend_clause_thread_simd : Error<
   "'depend' clauses cannot be mixed with '%0' clause">;
 def err_omp_depend_sink_expected_loop_iteration : Error<
-  "expected %0 loop iteration variable">;
+  "expected%select{| %1}0 loop iteration variable">;
 def err_omp_depend_sink_unexpected_expr : Error<
   "unexpected expression: number of expressions is larger than the number of 
associated loops">;
 def err_omp_depend_sink_expected_plus_minus : Error<


Index: test/OpenMP/ordered_messages.cpp
===
--- test/OpenMP/ordered_messages.cpp
+++ test/OpenMP/ordered_messages.cpp
@@ -270,5 +270,13 @@
 }
   }
 
+#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' clause}}
+  for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops after '#pragma omp for', but found only 1}}
+#pragma omp ordered depend(sink : i)
+int j;
+#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop iteration variable}}
+foo();
+  }
+
   return foo(); // expected-note {{in instantiation of function template specialization 'foo' requested here}}
 }
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -10510,9 +10510,14 @@
 if (!CurContext->isDependentContext() &&
 DSAStack->getParentOrderedRegionParam() &&
 DepCounter != DSAStack->isParentLoopControlVariable(D).first) {
-  Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
-  << DSAStack->getParentLoopControlVariable(
- DepCounter.getZExtValue());
+  ValueDecl* VD = DSAStack->getParentLoopControlVariable(
+  DepCounter.getZExtValue());
+  if (VD) {
+Diag(ELoc, diag::err_

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

2017-09-19 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 115881.
Rakete added a project: clang.
Rakete added a comment.

Used better diagnostic id.


https://reviews.llvm.org/D36357

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  lib/Parse/ParseExprCXX.cpp
  test/Parser/cxx0x-lambda-expressions.cpp
  test/SemaCXX/new-delete-0x.cpp


Index: test/SemaCXX/new-delete-0x.cpp
===
--- test/SemaCXX/new-delete-0x.cpp
+++ test/SemaCXX/new-delete-0x.cpp
@@ -34,6 +34,5 @@
 void bad_deletes()
 {
   // 'delete []' is always array delete, per [expr.delete]p1.
-  // FIXME: Give a better diagnostic.
-  delete []{ return (int*)0; }(); // expected-error {{expected expression}}
+  delete []{ return (int*)0; }(); // expected-error{{missing parentheses 
around lambda expression}}
 }
Index: test/Parser/cxx0x-lambda-expressions.cpp
===
--- test/Parser/cxx0x-lambda-expressions.cpp
+++ test/Parser/cxx0x-lambda-expressions.cpp
@@ -53,7 +53,7 @@
   void delete_lambda(int *p) {
 delete [] p;
 delete [] (int*) { new int }; // ok, compound-literal, not lambda
-delete [] { return new int; } (); // expected-error{{expected expression}}
+delete [] { return new int; } (); // expected-error{{missing parentheses 
around lambda expression}}
 delete [&] { return new int; } (); // ok, lambda
   }
 
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2890,15 +2890,39 @@
 //   [Footnote: A lambda expression with a lambda-introducer that consists
 //  of empty square brackets can follow the delete keyword if
 //  the lambda expression is enclosed in parentheses.]
-// FIXME: Produce a better diagnostic if the '[]' is unambiguously a
-//lambda-introducer.
-ArrayDelete = true;
-BalancedDelimiterTracker T(*this, tok::l_square);
+TentativeParsingAction TPA(*this);
 
-T.consumeOpen();
-T.consumeClose();
-if (T.getCloseLocation().isInvalid())
-  return ExprError();
+// Basic lookahead to check if we have a lambda expression. If we
+// encounter two braces with a semicolon, we can be pretty sure
+// that this is a lambda, not say a compound literal. 
+if (!SkipUntil(tok::l_brace, SkipUntilFlags::StopAtSemi) ||
+(NextToken().isNot(tok::r_brace) && !SkipUntil(tok::semi)) ||
+!SkipUntil(tok::r_brace, SkipUntilFlags::StopAtSemi)) {
+  TPA.Revert();
+  ArrayDelete = true;
+  BalancedDelimiterTracker T(*this, tok::l_square);
+
+  T.consumeOpen();
+  T.consumeClose();
+  if (T.getCloseLocation().isInvalid())
+return ExprError();
+} else {
+  TPA.Revert();
+
+  // Warn if the non-capturing lambda isn't surrounded by parenthesis
+  // to disambiguate it from 'delete[]'.
+  ExprResult Lambda = TryParseLambdaExpression();
+  if (!Lambda.isInvalid()) {
+SourceLocation StartLoc = Lambda.get()->getLocStart();
+Diag(StartLoc, diag::err_lambda_after_delete)
+  << SourceRange(StartLoc, Lambda.get()->getLocEnd());
+
+// Evaluate any postfix expressions used on the lambda.
+Lambda = ParsePostfixExpressionSuffix(Lambda);
+return Actions.ActOnCXXDelete(Start, UseGlobal, /*ArrayForm=*/false,
+  Lambda.get());
+  }
+}
   }
 
   ExprResult Operand(ParseCastExpression(false));
Index: include/clang/Basic/DiagnosticParseKinds.td
===
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -99,6 +99,8 @@
   InGroup, DefaultIgnore;
 def ext_alignof_expr : ExtWarn<
   "%0 applied to an expression is a GNU extension">, 
InGroup;
+def err_lambda_after_delete : Error<
+  "missing parentheses around lambda expression">;
 
 def warn_microsoft_dependent_exists : Warning<
   "dependent %select{__if_not_exists|__if_exists}0 declarations are ignored">, 


Index: test/SemaCXX/new-delete-0x.cpp
===
--- test/SemaCXX/new-delete-0x.cpp
+++ test/SemaCXX/new-delete-0x.cpp
@@ -34,6 +34,5 @@
 void bad_deletes()
 {
   // 'delete []' is always array delete, per [expr.delete]p1.
-  // FIXME: Give a better diagnostic.
-  delete []{ return (int*)0; }(); // expected-error {{expected expression}}
+  delete []{ return (int*)0; }(); // expected-error{{missing parentheses around lambda expression}}
 }
Index: test/Parser/cxx0x-lambda-expressions.cpp
===
--- test/Parser/cxx0x-lambda-expressions.cpp
+++ test/Parser/cxx0x-lambda-expressions.cpp
@@ -53,7 +53,7 @@
   void delete_lambda(int *p) {
 delete [] p;
 delete [] (int*) { new int }; // ok, compound-literal, not

[PATCH] D37955: [libcxx] Fix invert negative bracket match.

2017-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.

All the tests pass now (on Mac OS) Thanks!


https://reviews.llvm.org/D37955



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


[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:8605
 
-  return Match;
+  const char *Cmp; // Simplified, pretty-printed comparison string
+  // Similar to E->getOpcodeStr(), but with extra 0 on either LHS or RHS

I'd drop this comment entirely.



Comment at: lib/Sema/SemaChecking.cpp:8608
+
+  if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
+Result = false;

Instead of doing all of this complex logic, why not structure the code like the 
original and use '?:' to determine which diagnostic id to pass in?
```
if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
  S.Diag(E->getOperatorLoc(), HasEnumType(LHS) ? 
diag::warn_lunsigned_enum_always_true_comparison :
diag::warn_lunsigned_always_true_comparison)
<< "< 0" << "false"
<< LHS->getSourceRange() << RHS->getSourceRange();
```


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



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


[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values

2017-09-19 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


https://reviews.llvm.org/D38049



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


r313666 - Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

2017-09-19 Thread Andrew Kaylor via cfe-commits
Author: akaylor
Date: Tue Sep 19 13:26:40 2017
New Revision: 313666

URL: http://llvm.org/viewvc/llvm-project?rev=313666&view=rev
Log:
Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

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


Added:
cfe/trunk/test/CodeGen/nullptr-arithmetic.c
cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/pointer-addition.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=313666&r1=313665&r2=313666&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Sep 19 13:26:40 2017
@@ -3126,6 +3126,12 @@ public:
 return isShiftAssignOp(getOpcode());
   }
 
+  // Return true if a binary operator using the specified opcode and operands
+  // would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized
+  // integer to a pointer.
+  static bool isNullPointerArithmeticExtension(ASTContext &Ctx, Opcode Opc,
+   Expr *LHS, Expr *RHS);
+
   static bool classof(const Stmt *S) {
 return S->getStmtClass() >= firstBinaryOperatorConstant &&
S->getStmtClass() <= lastBinaryOperatorConstant;

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313666&r1=313665&r2=313666&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 13:26:40 2017
@@ -338,6 +338,7 @@ def NonPODVarargs : DiagGroup<"non-pod-v
 def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>;
 def : DiagGroup<"nonportable-cfstrings">;
 def NonVirtualDtor : DiagGroup<"non-virtual-dtor">;
+def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">;
 def : DiagGroup<"effc++", [NonVirtualDtor]>;
 def OveralignedType : DiagGroup<"over-aligned">;
 def AlignedAllocationUnavailable : DiagGroup<"aligned-allocation-unavailable">;
@@ -706,7 +707,8 @@ def Extra : DiagGroup<"extra", [
 SemiBeforeMethodBody,
 MissingMethodReturnType,
 SignCompare,
-UnusedParameter
+UnusedParameter,
+NullPointerArithmetic
   ]>;
 
 def Most : DiagGroup<"most", [

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313666&r1=313665&r2=313666&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 13:26:40 
2017
@@ -5501,6 +5501,12 @@ def err_offsetof_field_of_virtual_base :
 def warn_sub_ptr_zero_size_types : Warning<
   "subtraction of pointers to type %0 of zero size has undefined behavior">,
   InGroup;
+def warn_pointer_arith_null_ptr : Warning<
+  "performing pointer arithmetic on a null pointer has undefined 
behavior%select{| if the offset is nonzero}0">,
+  InGroup, DefaultIgnore;
+def warn_gnu_null_ptr_arith : Warning<
+  "arithmetic on a null pointer treated as a cast from integer to pointer is a 
GNU extension">,
+  InGroup, DefaultIgnore;
 
 def warn_floatingpoint_eq : Warning<
   "comparing floating point with == or != is unsafe">,

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=313666&r1=313665&r2=313666&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Sep 19 13:26:40 2017
@@ -1829,6 +1829,45 @@ OverloadedOperatorKind BinaryOperator::g
   return OverOps[Opc];
 }
 
+bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
+  Opcode Opc,
+  Expr *LHS, Expr *RHS) {
+  if (Opc != BO_Add)
+return false;
+
+  // Check that we have one pointer and one integer operand.
+  Expr *PExp;
+  Expr *IExp;
+  if (LHS->getType()->isPointerType()) {
+if (!RHS->getType()->isIntegerType())
+  return false;
+PExp = LHS;
+IExp = RHS;
+  } else if (RHS->getType()->isPointerType()) {
+if (!LHS->getType()->isIntegerType())
+  return false;
+PExp = RHS;
+IExp = LHS;
+  } else {
+return false;
+  }
+
+  // Check that the pointer is a nullptr.
+  if (!PExp->IgnoreParenCasts()
+  ->i

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

2017-09-19 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313666: Teach clang to tolerate the 'p = nullptr + n' idiom 
used by glibc (authored by akaylor).

Changed prior to commit:
  https://reviews.llvm.org/D37042?vs=115262&id=115889#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37042

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/CodeGen/nullptr-arithmetic.c
  cfe/trunk/test/Sema/pointer-addition.c
  cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp

Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -2678,6 +2678,30 @@
   unsigned width = cast(index->getType())->getBitWidth();
   auto &DL = CGF.CGM.getDataLayout();
   auto PtrTy = cast(pointer->getType());
+
+  // Some versions of glibc and gcc use idioms (particularly in their malloc
+  // routines) that add a pointer-sized integer (known to be a pointer value)
+  // to a null pointer in order to cast the value back to an integer or as
+  // part of a pointer alignment algorithm.  This is undefined behavior, but
+  // we'd like to be able to compile programs that use it.
+  //
+  // Normally, we'd generate a GEP with a null-pointer base here in response
+  // to that code, but it's also UB to dereference a pointer created that
+  // way.  Instead (as an acknowledged hack to tolerate the idiom) we will
+  // generate a direct cast of the integer value to a pointer.
+  //
+  // The idiom (p = nullptr + N) is not met if any of the following are true:
+  //
+  //   The operation is subtraction.
+  //   The index is not pointer-sized.
+  //   The pointer type is not byte-sized.
+  //
+  if (BinaryOperator::isNullPointerArithmeticExtension(CGF.getContext(),
+   op.Opcode,
+   expr->getLHS(), 
+   expr->getRHS()))
+return CGF.Builder.CreateIntToPtr(index, pointer->getType());
+
   if (width != DL.getTypeSizeInBits(PtrTy)) {
 // Zero-extend or sign-extend the pointer value according to
 // whether the index is signed or not.
Index: cfe/trunk/lib/AST/Expr.cpp
===
--- cfe/trunk/lib/AST/Expr.cpp
+++ cfe/trunk/lib/AST/Expr.cpp
@@ -1829,6 +1829,45 @@
   return OverOps[Opc];
 }
 
+bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
+  Opcode Opc,
+  Expr *LHS, Expr *RHS) {
+  if (Opc != BO_Add)
+return false;
+
+  // Check that we have one pointer and one integer operand.
+  Expr *PExp;
+  Expr *IExp;
+  if (LHS->getType()->isPointerType()) {
+if (!RHS->getType()->isIntegerType())
+  return false;
+PExp = LHS;
+IExp = RHS;
+  } else if (RHS->getType()->isPointerType()) {
+if (!LHS->getType()->isIntegerType())
+  return false;
+PExp = RHS;
+IExp = LHS;
+  } else {
+return false;
+  }
+
+  // Check that the pointer is a nullptr.
+  if (!PExp->IgnoreParenCasts()
+  ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
+return false;
+
+  // Check that the pointee type is char-sized.
+  const PointerType *PTy = PExp->getType()->getAs();
+  if (!PTy || !PTy->getPointeeType()->isCharType())
+return false;
+
+  // Check that the integer type is pointer-sized.
+  if (Ctx.getTypeSize(IExp->getType()) != Ctx.getTypeSize(PExp->getType()))
+return false;
+
+  return true;
+}
 InitListExpr::InitListExpr(const ASTContext &C, SourceLocation lbraceloc,
ArrayRef initExprs, SourceLocation rbraceloc)
   : Expr(InitListExprClass, QualType(), VK_RValue, OK_Ordinary, false, false,
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -8572,6 +8572,21 @@
 << 0 /* one pointer */ << Pointer->getSourceRange();
 }
 
+/// \brief Diagnose invalid arithmetic on a null pointer.
+///
+/// If \p IsGNUIdiom is true, the operation is using the 'p = (i8*)nullptr + n'
+/// idiom, which we recognize as a GNU extension.
+///
+static void diagnoseArithmeticOnNullPointer(Sema &S, SourceLocation Loc,
+Expr *Pointer, bool IsGNUIdiom) {
+  if (IsGNUIdiom)
+S.Diag(Loc, diag::warn_gnu_null_ptr_arith)
+  << Pointer->getSourceRange();
+  else
+S.Diag(Loc, diag::warn_pointer_arith_null_ptr)
+  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+}
+
 /// \brief Di

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 115890.
lebedev.ri added a comment.

Simplify `CheckTautologicalComparisonWithZero()` as per review notes.
Less LOC.


Repository:
  rL LLVM

https://reviews.llvm.org/D37629

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/compare.c
  test/Sema/tautological-unsigned-enum-zero-compare.c

Index: test/Sema/tautological-unsigned-enum-zero-compare.c
===
--- /dev/null
+++ test/Sema/tautological-unsigned-enum-zero-compare.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+
+int main() {
+  enum A { A_foo, A_bar };
+  enum A a;
+
+#ifdef TEST
+  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+return 0;
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+return 0;
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+return 0;
+#else
+  // expected-no-diagnostics
+  if (a < 0)
+return 0;
+  if (a >= 0)
+return 0;
+  if (0 <= a)
+return 0;
+  if (0 > a)
+return 0;
+  if (a < 0U)
+return 0;
+  if (a >= 0U)
+return 0;
+  if (0U <= a)
+return 0;
+  if (0U > a)
+return 0;
+#endif
+
+  return 1;
+}
Index: test/Sema/compare.c
===
--- test/Sema/compare.c
+++ test/Sema/compare.c
@@ -308,8 +308,59 @@
 int rdar8511238() {
   enum A { A_foo, A_bar };
   enum A a;
+
+  if (a == 0)
+  return 0;
+  if (a != 0)
+  return 0;
   if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
+  return 0;
+  if (a <= 0)
+  return 0;
+  if (a > 0)
+  return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  return 0;
+
+  if (0 == a)
+  return 0;
+  if (0 != a)
+  return 0;
+  if (0 < a)
+  return 0;
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  return 0;
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  return 0;
+  if (0 >= a)
+  return 0;
+
+  if (a == 0U)
+  return 0;
+  if (a != 0U)
+  return 0;
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  return 0;
+  if (a <= 0U)
+  return 0;
+  if (a > 0U)
+  return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  return 0;
+
+  if (0U == a)
+  return 0;
+  if (0U != a)
+  return 0;
+  if (0U < a)
+  return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  return 0;
+  if (0U >= a)
+  return 0;
+
   return 20;
 }
 
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8583,31 +8583,39 @@
 
   // bool values are handled by DiagnoseOutOfRangeComparison().
 
-  BinaryOperatorKind op = E->getOpcode();
+  BinaryOperatorKind Op = E->getOpcode();
   if (E->isValueDependent())
 return false;
 
   Expr *LHS = E->getLHS();
   Expr *RHS = E->getRHS();
 
   bool Match = true;
 
-  if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
-  << "< 0" << "false" << HasEnumType(LHS)
-  << LHS->getSourceRange() << RHS->getSourceRange();
-  } else if (op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
-  << ">= 0" << "true" << HasEnumType(LHS)
-  << LHS->getSourceRange() << RHS->getSourceRange();
-  } else if (op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
-S.Diag(E->getOperatorLoc(), diag::warn_runsigned_always_true_comparison)
-  << "0 >" << "false" << HasEnumType(RHS)
-  << LHS->getSourceRange() << RHS->get

[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions

2017-09-19 Thread Weiming Zhao via Phabricator via cfe-commits
weimingz updated this revision to Diff 115891.
weimingz added a comment.

Moved the inclusion from stdexcept.cpp into refstring.h

For exception.cpp and new.cpp, I think it's better to keep the inclusion there. 
Those files include  new_handler_fallback.ipp, exception_fallback.ipp, which 
need the header file. Including it in those .ipp requires relative path as 
"../../include/atomic_support.h" and I tried to avoid that.


https://reviews.llvm.org/D35235

Files:
  src/exception.cpp
  src/include/atomic_support.h
  src/include/refstring.h
  src/locale.cpp
  src/new.cpp
  src/support/runtime/exception_fallback.ipp
  src/support/runtime/new_handler_fallback.ipp

Index: src/support/runtime/new_handler_fallback.ipp
===
--- src/support/runtime/new_handler_fallback.ipp
+++ src/support/runtime/new_handler_fallback.ipp
@@ -15,13 +15,13 @@
 new_handler
 set_new_handler(new_handler handler) _NOEXCEPT
 {
-return __sync_lock_test_and_set(&__new_handler, handler);
+return __libcpp_atomic_exchange(&__new_handler, handler);
 }
 
 new_handler
 get_new_handler() _NOEXCEPT
 {
-return __sync_fetch_and_add(&__new_handler, nullptr);
+return __libcpp_atomic_load(&__new_handler);
 }
 
 } // namespace std
Index: src/support/runtime/exception_fallback.ipp
===
--- src/support/runtime/exception_fallback.ipp
+++ src/support/runtime/exception_fallback.ipp
@@ -20,13 +20,13 @@
 unexpected_handler
 set_unexpected(unexpected_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__unexpected_handler, func);
+  return __libcpp_atomic_exchange(&__unexpected_handler, func);
 }
 
 unexpected_handler
 get_unexpected() _NOEXCEPT
 {
-  return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0);
+  return __libcpp_atomic_load(&__unexpected_handler);
 
 }
 
@@ -41,14 +41,13 @@
 terminate_handler
 set_terminate(terminate_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__terminate_handler, func);
+  return __libcpp_atomic_exchange(&__terminate_handler, func);
 }
 
 terminate_handler
 get_terminate() _NOEXCEPT
 {
-  return __sync_fetch_and_add(&__terminate_handler, (terminate_handler)0);
-
+  return __libcpp_atomic_load(&__terminate_handler);
 }
 
 #ifndef __EMSCRIPTEN__ // We provide this in JS
Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -12,6 +12,7 @@
 #include 
 
 #include "new"
+#include "include/atomic_support.h"
 
 #if defined(_LIBCPP_ABI_MICROSOFT)
 // nothing todo
Index: src/locale.cpp
===
--- src/locale.cpp
+++ src/locale.cpp
@@ -36,6 +36,7 @@
 #endif
 #include 
 #include 
+#include "include/atomic_support.h"
 #include "__undef_macros"
 
 // On Linux, wint_t and wchar_t have different signed-ness, and this causes
@@ -667,7 +668,7 @@
 void
 locale::id::__init()
 {
-__id_ = __sync_add_and_fetch(&__next_id, 1);
+__id_ = __libcpp_atomic_add(&__next_id, 1);
 }
 
 // template <> class collate_byname
Index: src/include/refstring.h
===
--- src/include/refstring.h
+++ src/include/refstring.h
@@ -18,6 +18,7 @@
 #include 
 #include 
 #endif
+#include "atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -83,19 +84,19 @@
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 }
 
 inline
 __libcpp_refstring& __libcpp_refstring::operator=(__libcpp_refstring const& s) _NOEXCEPT {
 bool adjust_old_count = __uses_refcount();
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 if (adjust_old_count)
 {
-if (__sync_add_and_fetch(&old_rep->count, count_t(-1)) < 0)
+if (__libcpp_atomic_add(&old_rep->count, count_t(-1)) < 0)
 {
 ::operator delete(old_rep);
 }
@@ -107,7 +108,7 @@
 __libcpp_refstring::~__libcpp_refstring() {
 if (__uses_refcount()) {
 _Rep_base* rep = rep_from_data(__imp_);
-if (__sync_add_and_fetch(&rep->count, count_t(-1)) < 0) {
+if (__libcpp_atomic_add(&rep->count, count_t(-1)) < 0) {
 ::operator delete(rep);
 }
 }
Index: src/include/atomic_support.h
===
--- src/include/atomic_support.h
+++ src/include/atomic_support.h
@@ -16,6 +16,7 @@
 #if defined(__clang__) && __has_builtin(__atomic_load_n) \
&& __has_builtin(__atomic_store_n)\
&& __has_builtin(__atomic_add_fetch)  \
+   && __

[PATCH] D37881: [Sema] Prevent InstantiateClass from checking unrelated exception specs.

2017-09-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D37881#871556, @vsapsai wrote:

> I don't like name `SavePendingDelayedStateRAII` as too vague. Hope you'll be 
> able to suggest something better or I'll have better ideas later.


I like everything about this patch other than the name of the class :)

I think we need to clearly identify what state we're saving here and why. It's 
the state that is pending from a class that we're currently in the middle of 
parsing, so how about something like `SavePendingParsedClassStateRAII`?


https://reviews.llvm.org/D37881



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


[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



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


r313675 - [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values

2017-09-19 Thread Rachel Craik via cfe-commits
Author: rcraik
Date: Tue Sep 19 14:04:23 2017
New Revision: 313675

URL: http://llvm.org/viewvc/llvm-project?rev=313675&view=rev
Log:
[OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values

When the value specified for n in ordered(n) is larger than the number of loops 
a segmentation fault can occur in one of two ways when attempting to print out 
a diagnostic for an associated depend(sink : vec):
1) The iteration vector vec contains less than n items
2) The iteration vector vec contains a variable that is not a loop control 
variable
This patch addresses both of these issues.

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/ordered_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313675&r1=313674&r2=313675&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 14:04:23 
2017
@@ -8875,7 +8875,7 @@ def note_omp_critical_no_hint : Note<
 def err_omp_depend_clause_thread_simd : Error<
   "'depend' clauses cannot be mixed with '%0' clause">;
 def err_omp_depend_sink_expected_loop_iteration : Error<
-  "expected %0 loop iteration variable">;
+  "expected%select{| %1}0 loop iteration variable">;
 def err_omp_depend_sink_unexpected_expr : Error<
   "unexpected expression: number of expressions is larger than the number of 
associated loops">;
 def err_omp_depend_sink_expected_plus_minus : Error<

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=313675&r1=313674&r2=313675&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Sep 19 14:04:23 2017
@@ -10510,9 +10510,14 @@ Sema::ActOnOpenMPDependClause(OpenMPDepe
 if (!CurContext->isDependentContext() &&
 DSAStack->getParentOrderedRegionParam() &&
 DepCounter != DSAStack->isParentLoopControlVariable(D).first) {
-  Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
-  << DSAStack->getParentLoopControlVariable(
- DepCounter.getZExtValue());
+  ValueDecl* VD = DSAStack->getParentLoopControlVariable(
+  DepCounter.getZExtValue());
+  if (VD) {
+Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
+<< 1 << VD;
+  } else {
+ Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 
0;
+  }
   continue;
 }
 OpsOffs.push_back({RHS, OOK});
@@ -10545,8 +10550,9 @@ Sema::ActOnOpenMPDependClause(OpenMPDepe
 
 if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink &&
 TotalDepCount > VarList.size() &&
-DSAStack->getParentOrderedRegionParam()) {
-  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration)
+DSAStack->getParentOrderedRegionParam() &&
+DSAStack->getParentLoopControlVariable(VarList.size() + 1)) {
+  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1
   << DSAStack->getParentLoopControlVariable(VarList.size() + 1);
 }
 if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink &&

Modified: cfe/trunk/test/OpenMP/ordered_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/ordered_messages.cpp?rev=313675&r1=313674&r2=313675&view=diff
==
--- cfe/trunk/test/OpenMP/ordered_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/ordered_messages.cpp Tue Sep 19 14:04:23 2017
@@ -270,5 +270,13 @@ int k;
 }
   }
 
+#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' 
clause}}
+  for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops 
after '#pragma omp for', but found only 1}}
+#pragma omp ordered depend(sink : i)
+int j;
+#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop 
iteration variable}}
+foo();
+  }
+
   return foo(); // expected-note {{in instantiation of function template 
specialization 'foo' requested here}}
 }


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


[PATCH] D38049: [OpenMP] fix seg-faults printing diagnostics with invalid ordered(n) values

2017-09-19 Thread Rachel Craik via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313675: [OpenMP] fix seg-faults printing diagnostics with 
invalid ordered(n) values (authored by rcraik).

Changed prior to commit:
  https://reviews.llvm.org/D38049?vs=115876&id=115901#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38049

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaOpenMP.cpp
  cfe/trunk/test/OpenMP/ordered_messages.cpp


Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp
@@ -10510,9 +10510,14 @@
 if (!CurContext->isDependentContext() &&
 DSAStack->getParentOrderedRegionParam() &&
 DepCounter != DSAStack->isParentLoopControlVariable(D).first) {
-  Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
-  << DSAStack->getParentLoopControlVariable(
- DepCounter.getZExtValue());
+  ValueDecl* VD = DSAStack->getParentLoopControlVariable(
+  DepCounter.getZExtValue());
+  if (VD) {
+Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
+<< 1 << VD;
+  } else {
+ Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 
0;
+  }
   continue;
 }
 OpsOffs.push_back({RHS, OOK});
@@ -10545,8 +10550,9 @@
 
 if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink &&
 TotalDepCount > VarList.size() &&
-DSAStack->getParentOrderedRegionParam()) {
-  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration)
+DSAStack->getParentOrderedRegionParam() &&
+DSAStack->getParentLoopControlVariable(VarList.size() + 1)) {
+  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration) << 1
   << DSAStack->getParentLoopControlVariable(VarList.size() + 1);
 }
 if (DepKind != OMPC_DEPEND_source && DepKind != OMPC_DEPEND_sink &&
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8875,7 +8875,7 @@
 def err_omp_depend_clause_thread_simd : Error<
   "'depend' clauses cannot be mixed with '%0' clause">;
 def err_omp_depend_sink_expected_loop_iteration : Error<
-  "expected %0 loop iteration variable">;
+  "expected%select{| %1}0 loop iteration variable">;
 def err_omp_depend_sink_unexpected_expr : Error<
   "unexpected expression: number of expressions is larger than the number of 
associated loops">;
 def err_omp_depend_sink_expected_plus_minus : Error<
Index: cfe/trunk/test/OpenMP/ordered_messages.cpp
===
--- cfe/trunk/test/OpenMP/ordered_messages.cpp
+++ cfe/trunk/test/OpenMP/ordered_messages.cpp
@@ -270,5 +270,13 @@
 }
   }
 
+#pragma omp for ordered(2) // expected-note {{as specified in 'ordered' 
clause}}
+  for (int i = 0; i < 10; ++i) { // expected-error {{expected 2 for loops 
after '#pragma omp for', but found only 1}}
+#pragma omp ordered depend(sink : i)
+int j;
+#pragma omp ordered depend(sink : i, j) // expected-error {{expected loop 
iteration variable}}
+foo();
+  }
+
   return foo(); // expected-note {{in instantiation of function template 
specialization 'foo' requested here}}
 }


Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp
@@ -10510,9 +10510,14 @@
 if (!CurContext->isDependentContext() &&
 DSAStack->getParentOrderedRegionParam() &&
 DepCounter != DSAStack->isParentLoopControlVariable(D).first) {
-  Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
-  << DSAStack->getParentLoopControlVariable(
- DepCounter.getZExtValue());
+  ValueDecl* VD = DSAStack->getParentLoopControlVariable(
+  DepCounter.getZExtValue());
+  if (VD) {
+Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration)
+<< 1 << VD;
+  } else {
+ Diag(ELoc, diag::err_omp_depend_sink_expected_loop_iteration) << 0;
+  }
   continue;
 }
 OpsOffs.push_back({RHS, OOK});
@@ -10545,8 +10550,9 @@
 
 if (!CurContext->isDependentContext() && DepKind == OMPC_DEPEND_sink &&
 TotalDepCount > VarList.size() &&
-DSAStack->getParentOrderedRegionParam()) {
-  Diag(EndLoc, diag::err_omp_depend_sink_expected_loop_iteration)
+DSAStack->getParentOrderedRegionParam() &&
+DSAStack->getParentLoopControlVariable(VarList.size() + 1)) {
+  Diag(EndLoc, diag::err_omp_depend_sink

r313677 - [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Tue Sep 19 14:11:35 2017
New Revision: 313677

URL: http://llvm.org/viewvc/llvm-project?rev=313677&view=rev
Log:
[Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

Summary:
As requested by Sam McCall:
> Enums (not new I guess). Typical case: if (enum < 0 || enum > MAX)
> The warning strongly suggests that the enum < 0 check has no effect
> (for enums with nonnegative ranges).
> Clang doesn't seem to optimize such checks out though, and they seem
> likely to catch bugs in some cases. Yes, only if there's UB elsewhere,
> but I assume not optimizing out these checks indicates a deliberate
> decision to stay somewhat compatible with a technically-incorrect
> mental model.
> If this is the case, should we move these to a
> -Wtautological-compare-enum subcategory?

Reviewers: rjmccall, rsmith, aaron.ballman, sammccall, bkramer, djasper

Reviewed By: aaron.ballman

Subscribers: jroelofs, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/compare.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313677&r1=313676&r2=313677&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 14:11:35 2017
@@ -429,12 +429,14 @@ def StringPlusInt : DiagGroup<"string-pl
 def StringPlusChar : DiagGroup<"string-plus-char">;
 def StrncatSize : DiagGroup<"strncat-size">;
 def TautologicalUnsignedZeroCompare : 
DiagGroup<"tautological-unsigned-zero-compare">;
+def TautologicalUnsignedEnumZeroCompare : 
DiagGroup<"tautological-unsigned-enum-zero-compare">;
 def TautologicalOutOfRangeCompare : 
DiagGroup<"tautological-constant-out-of-range-compare">;
 def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
 def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
 def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
 def TautologicalCompare : DiagGroup<"tautological-compare",
 [TautologicalUnsignedZeroCompare,
+ TautologicalUnsignedEnumZeroCompare,
  TautologicalOutOfRangeCompare,
  TautologicalPointerCompare,
  TautologicalOverlapCompare,

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313677&r1=313676&r2=313677&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 14:11:35 
2017
@@ -5906,19 +5906,26 @@ def note_typecheck_assign_const : Note<
   "member function %q1 is declared const here|"
   "%select{|nested }1data member %2 declared const here}0">;
 
+def warn_lunsigned_always_true_comparison : Warning<
+  "comparison of unsigned expression %0 is always %select{false|true}1">,
+  InGroup;
+def warn_runsigned_always_true_comparison : Warning<
+  "comparison of %0 unsigned expression is always %select{false|true}1">,
+  InGroup;
+def warn_lunsigned_enum_always_true_comparison : Warning<
+  "comparison of unsigned enum expression %0 is always %select{false|true}1">,
+  InGroup;
+def warn_runsigned_enum_always_true_comparison : Warning<
+  "comparison of %0 unsigned enum expression is always %select{false|true}1">,
+  InGroup;
+
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of different signs: %0 and %1">,
   InGroup, DefaultIgnore;
-def warn_lunsigned_always_true_comparison : Warning<
-  "comparison of unsigned%select{| enum}2 expression %0 is always %1">,
-  InGroup;
 def warn_out_of_range_compare : Warning<
   "comparison of %select{constant %0|true|false}1 with " 
   "%select{expression of type %2|boolean expression}3 is always "
   "%select{false|true}4">, InGroup;
-def warn_runsigned_always_true_comparison : Warning<
-  "comparison of %0 unsigned%select{| enum}2 expression is always %1">,
-  InGroup;
 def warn_comparison_of_mixed_enum_types : Warning<
   "comparison of two values with different enumeration types"
   "%diff{ ($ and $)|}0,1">,

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=313677&r1=313676&r2=313677&view=diff
==
--- cfe/tru

Re: r312750 - [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-19 Thread Roman Lebedev via cfe-commits
On Fri, Sep 8, 2017 at 7:10 PM, Roman Lebedev  wrote:
> On Fri, Sep 8, 2017 at 3:26 PM, Roman Lebedev  wrote:
>> On Fri, Sep 8, 2017 at 2:48 PM, Sam McCall  wrote:
>> Hi.
>>
>>> Nice fix!
>> Thank you!
>>
>>> It catches a lot of new cases on our codebase, all technically
>>> correct so far.
>>>
>>> A couple of issues though:
>>> A) Rollout - until we've completely cleaned up, we need to disable
>>> -Wtautological-compare entirely, which is a valuable check. I imagine anyone
>>> else using -Werror is in the same boat.
>>> What do you think about putting the new warnings behind a subcategory? (e.g.
>>> -Wtautological-compare-unsigned-zero, implied by -Wtautological-compare)
>>> It's an ugly artifact of the history here, but allows this fix to be rolled
>>> out in a controlled way.
>> https://reviews.llvm.org/D37620
> And landed.
>
>>> B) Enums (not new I guess). Typical case: if (enum < 0 || enum > MAX)
>>> The warning strongly suggests that the enum < 0 check has no effect (for
>>> enums with nonnegative ranges).
>>> Clang doesn't seem to optimize such checks out though, and they seem likely
>>> to catch bugs in some cases. Yes, only if there's UB elsewhere, but I assume
>>> not optimizing out these checks indicates a deliberate decision to stay
>>> somewhat compatible with a technically-incorrect mental model.
>>> If this is the case, should we move these to a -Wtautological-compare-enum
>>> subcategory?
>> (Did not look at this yet)
> https://reviews.llvm.org/D37629 i hope that is what you meant.
And landed, too.

Next will try to work on the second part of the fix for
https://bugs.llvm.org/show_bug.cgi?id=34147

>> Roman.
> Roman.
Roman.

>>> On Fri, Sep 8, 2017 at 12:14 AM, Roman Lebedev via cfe-commits
>>>  wrote:

 Author: lebedevri
 Date: Thu Sep  7 15:14:25 2017
 New Revision: 312750

 URL: http://llvm.org/viewvc/llvm-project?rev=312750&view=rev
 Log:
 [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

 Summary:
 This is a first half(?) of a fix for the following bug:
 https://bugs.llvm.org/show_bug.cgi?id=34147 (gcc -Wtype-limits)

 GCC's -Wtype-limits does warn on comparison of unsigned value
 with signed zero (as in, with 0), but clang only warns if the
 zero is unsigned (i.e. 0U).

 Also, be careful not to double-warn, or falsely warn on
 comparison of signed/fp variable and signed 0.

 Yes, all these testcases are needed.

 Testing: $ ninja check-clang-sema check-clang-semacxx
 Also, no new warnings for clang stage-2 build.

 Reviewers: rjmccall, rsmith, aaron.ballman

 Reviewed By: rjmccall

 Subscribers: cfe-commits

 Tags: #clang

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

 Modified:
 cfe/trunk/docs/ReleaseNotes.rst
 cfe/trunk/lib/Sema/SemaChecking.cpp
 cfe/trunk/test/Sema/compare.c
 cfe/trunk/test/Sema/outof-range-constant-compare.c
 cfe/trunk/test/SemaCXX/compare.cpp

 Modified: cfe/trunk/docs/ReleaseNotes.rst
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=312750&r1=312749&r2=312750&view=diff

 ==
 --- cfe/trunk/docs/ReleaseNotes.rst (original)
 +++ cfe/trunk/docs/ReleaseNotes.rst Thu Sep  7 15:14:25 2017
 @@ -71,6 +71,13 @@ Improvements to Clang's diagnostics
errors/warnings, as the system frameworks might add a method with the
 same
selector which could make the message send to ``id`` ambiguous.

 +- ``-Wtautological-compare`` now warns when comparing an unsigned integer
 and 0
 +  regardless of whether the constant is signed or unsigned."
 +
 +- ``-Wtautological-compare`` now warns about comparing a signed integer
 and 0
 +  when the signed integer is coerced to an unsigned type for the
 comparison.
 +  ``-Wsign-compare`` was adjusted not to warn in this case.
 +
  Non-comprehensive list of changes in this release
  -


 Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=312750&r1=312749&r2=312750&view=diff

 ==
 --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
 +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Sep  7 15:14:25 2017
 @@ -8567,32 +8567,51 @@ bool HasEnumType(Expr *E) {
return E->getType()->isEnumeralType();
  }

 -void CheckTrivialUnsignedComparison(Sema &S, BinaryOperator *E) {
 +bool isNonBooleanUnsignedValue(Expr *E) {
 +  // We are checking that the expression is not known to have boolean
 value,
 +  // is an integer type; and is either unsigned after implicit casts,
 +

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313677: [Sema] Move some stuff into 
-Wtautological-unsigned-enum-zero-compare (authored by lebedevri).

Changed prior to commit:
  https://reviews.llvm.org/D37629?vs=115890&id=115903#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37629

Files:
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/compare.c
  cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c

Index: cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
===
--- cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
+++ cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+
+int main() {
+  enum A { A_foo, A_bar };
+  enum A a;
+
+#ifdef TEST
+  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+return 0;
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+return 0;
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+return 0;
+#else
+  // expected-no-diagnostics
+  if (a < 0)
+return 0;
+  if (a >= 0)
+return 0;
+  if (0 <= a)
+return 0;
+  if (0 > a)
+return 0;
+  if (a < 0U)
+return 0;
+  if (a >= 0U)
+return 0;
+  if (0U <= a)
+return 0;
+  if (0U > a)
+return 0;
+#endif
+
+  return 1;
+}
Index: cfe/trunk/test/Sema/compare.c
===
--- cfe/trunk/test/Sema/compare.c
+++ cfe/trunk/test/Sema/compare.c
@@ -308,8 +308,59 @@
 int rdar8511238() {
   enum A { A_foo, A_bar };
   enum A a;
+
+  if (a == 0)
+  return 0;
+  if (a != 0)
+  return 0;
   if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
+  return 0;
+  if (a <= 0)
+  return 0;
+  if (a > 0)
+  return 0;
+  if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  return 0;
+
+  if (0 == a)
+  return 0;
+  if (0 != a)
+  return 0;
+  if (0 < a)
+  return 0;
+  if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  return 0;
+  if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  return 0;
+  if (0 >= a)
+  return 0;
+
+  if (a == 0U)
+  return 0;
+  if (a != 0U)
+  return 0;
+  if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  return 0;
+  if (a <= 0U)
+  return 0;
+  if (a > 0U)
+  return 0;
+  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  return 0;
+
+  if (0U == a)
+  return 0;
+  if (0U != a)
+  return 0;
+  if (0U < a)
+  return 0;
+  if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  return 0;
+  if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  return 0;
+  if (0U >= a)
+  return 0;
+
   return 20;
 }
 
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -8583,31 +8583,39 @@
 
   // bool values are handled by DiagnoseOutOfRangeComparison().
 
-  BinaryOperatorKind op = E->getOpcode();
+  BinaryOperatorKind Op = E->getOpcode();
   if (E->isValueDependent())
 return false;
 
   Expr *LHS = E->getLHS();
   Expr *RHS = E->getRHS();
 
   bool Match = true;
 
-  if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
-  << "< 0" << "false" << HasEnumType(LHS)
-  << LHS->getSourceRange() << RHS->getSourceRange();
-  } else if (op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
-S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
-  << ">= 0" << "true" << HasEnumType(LHS)
-  

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

2017-09-19 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor added a comment.

This is breaking buildbots for 32-bit targets because the offset in 'nullptr + 
int8_t_N' is being implicitly cast to an int.  That makes the sizeof(offset) == 
sizeof(ptr) check turn out differently than my tests were assuming.

I can get the buildbots green quickly by taking out parts of the tests, but I 
just talked to Erich Keane about this and we think the right way to fix this 
long term is to stop checking for sizeof(offset) == sizeof(ptr) in the code 
that identifies the idiom, since that check is of dubious value and would be 
difficult to always get to behave the way I intended.


Repository:
  rL LLVM

https://reviews.llvm.org/D37042



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


[PATCH] D36150: [clangd] LSP extension to switch between source/header file

2017-09-19 Thread William Enright via Phabricator via cfe-commits
Nebiroth added a comment.

In https://reviews.llvm.org/D36150#867971, @ilya-biryukov wrote:

> Overall looks good, but still needs at least a few tests.


I have a test for this commit that uses included source and header files, would 
that be okay to do or should I generate files dynamically? If so, how can I 
accomplish this?


https://reviews.llvm.org/D36150



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


r313683 - Revert "[Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare"

2017-09-19 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Tue Sep 19 14:40:41 2017
New Revision: 313683

URL: http://llvm.org/viewvc/llvm-project?rev=313683&view=rev
Log:
Revert "[Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare"

This reverts commit r313677.

Buildbots fail with assertion failure
Failing Tests (7):
Clang :: Analysis/null-deref-ps.c
Clang :: CodeGen/enum.c
Clang :: Sema/compare.c
Clang :: Sema/outof-range-constant-compare.c
Clang :: Sema/tautological-unsigned-enum-zero-compare.c
Clang :: Sema/tautological-unsigned-zero-compare.c
Clang :: SemaCXX/compare.cpp

Removed:
cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/compare.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313683&r1=313682&r2=313683&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 14:40:41 2017
@@ -429,14 +429,12 @@ def StringPlusInt : DiagGroup<"string-pl
 def StringPlusChar : DiagGroup<"string-plus-char">;
 def StrncatSize : DiagGroup<"strncat-size">;
 def TautologicalUnsignedZeroCompare : 
DiagGroup<"tautological-unsigned-zero-compare">;
-def TautologicalUnsignedEnumZeroCompare : 
DiagGroup<"tautological-unsigned-enum-zero-compare">;
 def TautologicalOutOfRangeCompare : 
DiagGroup<"tautological-constant-out-of-range-compare">;
 def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
 def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
 def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
 def TautologicalCompare : DiagGroup<"tautological-compare",
 [TautologicalUnsignedZeroCompare,
- TautologicalUnsignedEnumZeroCompare,
  TautologicalOutOfRangeCompare,
  TautologicalPointerCompare,
  TautologicalOverlapCompare,

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313683&r1=313682&r2=313683&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 14:40:41 
2017
@@ -5906,26 +5906,19 @@ def note_typecheck_assign_const : Note<
   "member function %q1 is declared const here|"
   "%select{|nested }1data member %2 declared const here}0">;
 
-def warn_lunsigned_always_true_comparison : Warning<
-  "comparison of unsigned expression %0 is always %select{false|true}1">,
-  InGroup;
-def warn_runsigned_always_true_comparison : Warning<
-  "comparison of %0 unsigned expression is always %select{false|true}1">,
-  InGroup;
-def warn_lunsigned_enum_always_true_comparison : Warning<
-  "comparison of unsigned enum expression %0 is always %select{false|true}1">,
-  InGroup;
-def warn_runsigned_enum_always_true_comparison : Warning<
-  "comparison of %0 unsigned enum expression is always %select{false|true}1">,
-  InGroup;
-
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of different signs: %0 and %1">,
   InGroup, DefaultIgnore;
+def warn_lunsigned_always_true_comparison : Warning<
+  "comparison of unsigned%select{| enum}2 expression %0 is always %1">,
+  InGroup;
 def warn_out_of_range_compare : Warning<
   "comparison of %select{constant %0|true|false}1 with " 
   "%select{expression of type %2|boolean expression}3 is always "
   "%select{false|true}4">, InGroup;
+def warn_runsigned_always_true_comparison : Warning<
+  "comparison of %0 unsigned%select{| enum}2 expression is always %1">,
+  InGroup;
 def warn_comparison_of_mixed_enum_types : Warning<
   "comparison of two values with different enumeration types"
   "%diff{ ($ and $)|}0,1">,

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=313683&r1=313682&r2=313683&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Sep 19 14:40:41 2017
@@ -8583,7 +8583,7 @@ bool CheckTautologicalComparisonWithZero
 
   // bool values are handled by DiagnoseOutOfRangeComparison().
 
-  BinaryOperatorKind Op = E->getOpcode();
+  BinaryOperatorKind op = E->getOpcode();
   if (E->isValueDependent())
 return false;
 
@@ -8592,30 +8592,22 @@ bool CheckTautologicalCom

r313684 - Fix 32-bit buildbots by removing tests that are dependent on pointer-size comparisons.

2017-09-19 Thread Andrew Kaylor via cfe-commits
Author: akaylor
Date: Tue Sep 19 14:43:01 2017
New Revision: 313684

URL: http://llvm.org/viewvc/llvm-project?rev=313684&view=rev
Log:
Fix 32-bit buildbots by removing tests that are dependent on pointer-size 
comparisons.

The recently behavior in the code that these tests were meant to be checking 
will be ammended as soon as a suitable change can be properly reviewed.


Modified:
cfe/trunk/test/CodeGen/nullptr-arithmetic.c
cfe/trunk/test/Sema/pointer-addition.c
cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp

Modified: cfe/trunk/test/CodeGen/nullptr-arithmetic.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/nullptr-arithmetic.c?rev=313684&r1=313683&r2=313684&view=diff
==
--- cfe/trunk/test/CodeGen/nullptr-arithmetic.c (original)
+++ cfe/trunk/test/CodeGen/nullptr-arithmetic.c Tue Sep 19 14:43:01 2017
@@ -17,26 +17,18 @@ int8_t *test1(intptr_t n) {
 // CHECK: inttoptr
 // CHECK-NOT: getelementptr
 
-// This doesn't meet the idiom because the offset type isn't pointer-sized.
-int8_t *test2(int8_t n) {
-  return NULLPTRI8 + n;
-}
-// CHECK-LABEL: test2
-// CHECK: getelementptr
-// CHECK-NOT: inttoptr
-
 // This doesn't meet the idiom because the element type is larger than a byte.
-int16_t *test3(intptr_t n) {
+int16_t *test2(intptr_t n) {
   return (int16_t*)0 + n;
 }
-// CHECK-LABEL: test3
+// CHECK-LABEL: test2
 // CHECK: getelementptr
 // CHECK-NOT: inttoptr
 
 // This doesn't meet the idiom because the offset is subtracted.
-int8_t* test4(intptr_t n) {
+int8_t* test3(intptr_t n) {
   return NULLPTRI8 - n;
 }
-// CHECK-LABEL: test4
+// CHECK-LABEL: test3
 // CHECK: getelementptr
 // CHECK-NOT: inttoptr

Modified: cfe/trunk/test/Sema/pointer-addition.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pointer-addition.c?rev=313684&r1=313683&r2=313684&view=diff
==
--- cfe/trunk/test/Sema/pointer-addition.c (original)
+++ cfe/trunk/test/Sema/pointer-addition.c Tue Sep 19 14:43:01 2017
@@ -27,6 +27,4 @@ void a(S* b, void* c) {
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
-  unsigned char j = (unsigned char)b;
-  f = (char*)0 + j; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
 }

Modified: cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp?rev=313684&r1=313683&r2=313684&view=diff
==
--- cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp (original)
+++ cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Tue Sep 19 14:43:01 2017
@@ -2,7 +2,7 @@
 
 #include 
 
-void f(intptr_t offset, int8_t b) {
+void f(intptr_t offset) {
   // A zero offset from a nullptr is OK.
   char *f = (char*)nullptr + 0;
   int *g = (int*)0 + 0;
@@ -13,7 +13,6 @@ void f(intptr_t offset, int8_t b) {
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - offset; // expected-warning {{performing pointer arithmetic 
on a null pointer has undefined behavior if the offset is nonzero}}
   g = (int*)0 + offset; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior if the offset is nonzero}}
-  f = (char*)0 + b; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior if the offset is nonzero}}
 }
 
 // Value-dependent pointer arithmetic should not produce a nullptr warning.


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


[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri reopened this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Reverted due to buildbot failures.
Need to investigate.


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



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


[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions

2017-09-19 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF accepted this revision.
EricWF added inline comments.



Comment at: src/include/atomic_support.h:153
+_ValueType old;
+old = *__target;
+*__target = __value;

Initialize `old` on the same line it's declared on.


https://reviews.llvm.org/D35235



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


[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

2017-09-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I agree, it doesn't add much value.

Either way, though, please make sure you address the buildbot failures quickly.


Repository:
  rL LLVM

https://reviews.llvm.org/D37042



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


[PATCH] D31140: [LLVMbugs] [Bug 18710] Only generate .ARM.exidx and .ARM.extab when needed in EHABI

2017-09-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I would rather not make ARM baremetal do something different from every other 
target...

On Linux, for most targets, we don't add the uwtable attribute by default; 
without the uwtable attribute, non-ARM backends (e.g. aarch64) only emit tables 
for functions which might unwind.  Following this precedent is probably the 
most straightforward.

MachO::IsUnwindTablesDefault does something similar to what you're doing here 
(except it doesn't depend on CCCIsCXX).  But Apple platforms have to deal with 
ObjC exception handling, so I'm not sure that's the best precedent to follow.


https://reviews.llvm.org/D31140



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


[PATCH] D37954: Expand absolute system header paths when using -MD depfiles

2017-09-19 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

I think for depfile generation, we generally try to be gcc-compatible (I can 
link to prior changes in this spirit), so I think this seems like a good thing 
to do to me. gcc only does this for system headers, yes?

Does gcc support the non-negated spelling of this flag 
(-f-canonical-system-headers) too? If so, we should probably support that too 
for completeness, even if specifying it explicitly won't change behavior.


https://reviews.llvm.org/D37954



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


r313693 - Add override for ClangDiagnosticHandler::isAnyRemarkEnabled()

2017-09-19 Thread Adam Nemet via cfe-commits
Author: anemet
Date: Tue Sep 19 16:00:59 2017
New Revision: 313693

URL: http://llvm.org/viewvc/llvm-project?rev=313693&view=rev
Log:
Add override for ClangDiagnosticHandler::isAnyRemarkEnabled()

This is used by the new closure-based variant of
OptimizationRemarkEmitter::emit().

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

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=313693&r1=313692&r2=313693&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Tue Sep 19 16:00:59 2017
@@ -67,6 +67,12 @@ namespace clang {
   CodeGenOpts.OptimizationRemarkPattern->match(PassName));
 }
 
+bool isAnyRemarkEnabled() const override {
+  return (CodeGenOpts.OptimizationRemarkAnalysisPattern ||
+  CodeGenOpts.OptimizationRemarkMissedPattern ||
+  CodeGenOpts.OptimizationRemarkPattern);
+}
+
   private:
 const CodeGenOptions &CodeGenOpts;
 BackendConsumer *BackendCon;


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


[PATCH] D32520: Support __fp16 vectors

2017-09-19 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1042
+}
+
+assert(SrcElementTy->isFloatingPointTy() &&

What happens if the SrcElementTy is float and DstElementTy isn't? Seems like it 
will hit the assertion below.



Comment at: lib/Sema/SemaExpr.cpp:7511
+  // If this is a compound assignment, allow converting the RHS to the type
+  // of the LHS.
+  if (IsCompAssign && isVector(LHSType, Context.HalfTy)) {

Since it seems that you're always doing the same conversion (the only variable 
input here is the number of elements), can you update the comment to mention 
the exact conversion?



Comment at: lib/Sema/SemaExpr.cpp:8072
+/// Convert E, which is a vector, to a vector that has a different element
+/// type.
+static ExprResult convertVector(Expr *E, QualType ElementType, Sema &S) {

What about `Convert vector E to a vector with the same number of elements but 
different element type`?



Comment at: lib/Sema/SemaExpr.cpp:11316
+// This helper function promotes a binary operator's operands (which are of a
+// half vector type) to a vector of floats and then truncates the result to
+// a vector of either half or short.

`which are of a half vector type` -> should be there an assertion below to make 
sure?



Comment at: lib/Sema/SemaExpr.cpp:11329
+  QualType BinOpResTy = RHS.get()->getType();
+  if (isVector(ResultTy, Context.ShortTy))
+BinOpResTy = S.GetSignedVectorType(BinOpResTy);

Can you add a comment explaining that a) this conversion from float -> int and 
b) it's needed in case the original binop is a comparison? 



Comment at: lib/Sema/SemaExpr.cpp:11537
+  // Some of the binary operations require promoting operands of half vector
+  // and truncating the result. For now, we do this only when HalfArgsAndReturn
+  // is set (that is, when the target is arm or arm64).

What about `of half vector and truncating the result` to `of half vector to 
float and truncating the result back to half`



Comment at: lib/Sema/SemaExpr.cpp:11568
   
-  if (CompResultTy.isNull())
+  if (CompResultTy.isNull()) {
+if (ConvertHalfVec)

Please add a comment here explaining that this path only happens when it's a 
compound assignment.



Comment at: lib/Sema/SemaExpr.cpp:11978
+// truncating the result. For now, we do this only when HalfArgsAndReturns
+// is set (that is, when the target is arm or arm64).
+ConvertHalfVec =

This same logic is used elsewhere in the patch, perhaps factor it out into a 
static function?


https://reviews.llvm.org/D32520



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


[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

2017-09-19 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D38042#875441, @aprantl wrote:

> In https://reviews.llvm.org/D38042#875418, @chandlerc wrote:
>
> > Absolutely. I think the verifier should never, under any circumstances, 
> > mutate the IR. Think about it, with the current design if a pass corrupts 
> > the debug info the verifier may "hide" this by stripping it out rather than 
> > allowing us to find it.
>
>
> Okay, I think I agree. Before I venture off implementing this, do you think 
> that separating out a StripBrokenDebugInfoPass that depends on the Verifier 
> is right way forward?


I don't know what you mean by "depends on" and I'm not sure I'm the right 
person to say what the exact best design is... But I'd throw something together 
and start that review on llvm-commits where we can get folks more familiar w/ 
these details involved to figure it out. It at least seems to be in the right 
direction.

My only concern is that passes are supposed to be able to rely on the verifier 
passing, and this wouldn't... Not sure how to handle that case.

> 
> 
>> But auto-upgrade happens on *read*. If you want to add the debug info 
>> stripping to auto-upgrade, that's a reasonable discussion to have, and no 
>> change here will be required. There might be concerns about that on the LLVM 
>> side, I don't know. But the verifier (as well as running it here) does not 
>> seem like the right solution.
> 
> Would splitting the VerifierPass into a VerifierPass and a 
> StripBrokenDebugInfoPass *together* with adding a -enable-llvm-verifier (an 
> explicit opposite of -disable-llvm-verifier) work for you?

If you want a way to use the `clang` binary to strip broken debug info from a 
bitcode input, I would add a flag that does exactly this, and leave the 
verifier as an independent component. I don't know whether such a flag makes 
sense or not (Richard or other more deep in Clang-land would have a better feel 
than I would).

But I think that whether the verifier is enabled or not should be orthogonal 
from any debug info stripping. Stripping the debug info might impact whether 
something verifies, but the flags should be completely independent.

However, if the debug info stripping ends up as part of auto upgrade, all of 
this becomes much simpler to think about.


https://reviews.llvm.org/D38042



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


[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions

2017-09-19 Thread Weiming Zhao via Phabricator via cfe-commits
weimingz updated this revision to Diff 115918.
weimingz added a comment.

minor change


https://reviews.llvm.org/D35235

Files:
  src/exception.cpp
  src/include/atomic_support.h
  src/include/refstring.h
  src/locale.cpp
  src/new.cpp
  src/support/runtime/exception_fallback.ipp
  src/support/runtime/new_handler_fallback.ipp

Index: src/support/runtime/new_handler_fallback.ipp
===
--- src/support/runtime/new_handler_fallback.ipp
+++ src/support/runtime/new_handler_fallback.ipp
@@ -15,13 +15,13 @@
 new_handler
 set_new_handler(new_handler handler) _NOEXCEPT
 {
-return __sync_lock_test_and_set(&__new_handler, handler);
+return __libcpp_atomic_exchange(&__new_handler, handler);
 }
 
 new_handler
 get_new_handler() _NOEXCEPT
 {
-return __sync_fetch_and_add(&__new_handler, nullptr);
+return __libcpp_atomic_load(&__new_handler);
 }
 
 } // namespace std
Index: src/support/runtime/exception_fallback.ipp
===
--- src/support/runtime/exception_fallback.ipp
+++ src/support/runtime/exception_fallback.ipp
@@ -20,13 +20,13 @@
 unexpected_handler
 set_unexpected(unexpected_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__unexpected_handler, func);
+  return __libcpp_atomic_exchange(&__unexpected_handler, func);
 }
 
 unexpected_handler
 get_unexpected() _NOEXCEPT
 {
-  return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0);
+  return __libcpp_atomic_load(&__unexpected_handler);
 
 }
 
@@ -41,14 +41,13 @@
 terminate_handler
 set_terminate(terminate_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__terminate_handler, func);
+  return __libcpp_atomic_exchange(&__terminate_handler, func);
 }
 
 terminate_handler
 get_terminate() _NOEXCEPT
 {
-  return __sync_fetch_and_add(&__terminate_handler, (terminate_handler)0);
-
+  return __libcpp_atomic_load(&__terminate_handler);
 }
 
 #ifndef __EMSCRIPTEN__ // We provide this in JS
Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -12,6 +12,7 @@
 #include 
 
 #include "new"
+#include "include/atomic_support.h"
 
 #if defined(_LIBCPP_ABI_MICROSOFT)
 // nothing todo
Index: src/locale.cpp
===
--- src/locale.cpp
+++ src/locale.cpp
@@ -36,6 +36,7 @@
 #endif
 #include 
 #include 
+#include "include/atomic_support.h"
 #include "__undef_macros"
 
 // On Linux, wint_t and wchar_t have different signed-ness, and this causes
@@ -667,7 +668,7 @@
 void
 locale::id::__init()
 {
-__id_ = __sync_add_and_fetch(&__next_id, 1);
+__id_ = __libcpp_atomic_add(&__next_id, 1);
 }
 
 // template <> class collate_byname
Index: src/include/refstring.h
===
--- src/include/refstring.h
+++ src/include/refstring.h
@@ -18,6 +18,7 @@
 #include 
 #include 
 #endif
+#include "atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -83,19 +84,19 @@
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 }
 
 inline
 __libcpp_refstring& __libcpp_refstring::operator=(__libcpp_refstring const& s) _NOEXCEPT {
 bool adjust_old_count = __uses_refcount();
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 if (adjust_old_count)
 {
-if (__sync_add_and_fetch(&old_rep->count, count_t(-1)) < 0)
+if (__libcpp_atomic_add(&old_rep->count, count_t(-1)) < 0)
 {
 ::operator delete(old_rep);
 }
@@ -107,7 +108,7 @@
 __libcpp_refstring::~__libcpp_refstring() {
 if (__uses_refcount()) {
 _Rep_base* rep = rep_from_data(__imp_);
-if (__sync_add_and_fetch(&rep->count, count_t(-1)) < 0) {
+if (__libcpp_atomic_add(&rep->count, count_t(-1)) < 0) {
 ::operator delete(rep);
 }
 }
Index: src/include/atomic_support.h
===
--- src/include/atomic_support.h
+++ src/include/atomic_support.h
@@ -16,6 +16,7 @@
 #if defined(__clang__) && __has_builtin(__atomic_load_n) \
&& __has_builtin(__atomic_store_n)\
&& __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_exchange_n) \
&& __has_builtin(__atomic_compare_exchange_n) \
&& defined(__ATOMIC_RELAXED)  \
&& defined(__ATOMIC_CONSUME)  \
@@ -82,6 +83,14 @@
 return __atomic_add_fetch(__val, __a, __order);
 }
 
+te

[PATCH] D38059: Rename list::base to list::__base.

2017-09-19 Thread David L. Jones via Phabricator via cfe-commits
dlj created this revision.
Herald added a subscriber: sanjoy.

Even though the base typedef is private, it still participates in lookup. For 
example:

  void base() {}
  struct X : private std::list {
X() { base(); }
  };


https://reviews.llvm.org/D38059

Files:
  include/list

Index: include/list
===
--- include/list
+++ include/list
@@ -805,30 +805,30 @@
 class _LIBCPP_TEMPLATE_VIS list
 : private __list_imp<_Tp, _Alloc>
 {
-typedef __list_imp<_Tp, _Alloc> base;
-typedef typename base::__node  __node;
-typedef typename base::__node_allocator__node_allocator;
-typedef typename base::__node_pointer  __node_pointer;
-typedef typename base::__node_alloc_traits __node_alloc_traits;
-typedef typename base::__node_base __node_base;
-typedef typename base::__node_base_pointer __node_base_pointer;
-typedef typename base::__link_pointer __link_pointer;
+typedef __list_imp<_Tp, _Alloc> __base;
+typedef typename __base::__node  __node;
+typedef typename __base::__node_allocator__node_allocator;
+typedef typename __base::__node_pointer  __node_pointer;
+typedef typename __base::__node_alloc_traits __node_alloc_traits;
+typedef typename __base::__node_base __node_base;
+typedef typename __base::__node_base_pointer __node_base_pointer;
+typedef typename __base::__link_pointer  __link_pointer;
 
 public:
 typedef _Tp  value_type;
 typedef _Alloc   allocator_type;
 static_assert((is_same::value),
   "Invalid allocator::value_type");
 typedef value_type&  reference;
 typedef const value_type&const_reference;
-typedef typename base::pointer   pointer;
-typedef typename base::const_pointer const_pointer;
-typedef typename base::size_type size_type;
-typedef typename base::difference_type   difference_type;
-typedef typename base::iterator  iterator;
-typedef typename base::const_iteratorconst_iterator;
-typedef _VSTD::reverse_iterator reverse_iterator;
-typedef _VSTD::reverse_iterator   const_reverse_iterator;
+typedef typename __base::pointer pointer;
+typedef typename __base::const_pointer   const_pointer;
+typedef typename __base::size_type   size_type;
+typedef typename __base::difference_type difference_type;
+typedef typename __base::iteratoriterator;
+typedef typename __base::const_iterator  const_iterator;
+typedef _VSTD::reverse_iteratorreverse_iterator;
+typedef _VSTD::reverse_iterator  const_reverse_iterator;
 
 _LIBCPP_INLINE_VISIBILITY
 list()
@@ -839,7 +839,7 @@
 #endif
 }
 _LIBCPP_INLINE_VISIBILITY
-explicit list(const allocator_type& __a) : base(__a)
+explicit list(const allocator_type& __a) : __base(__a)
 {
 #if _LIBCPP_DEBUG_LEVEL >= 2
 __get_db()->__insert_c(this);
@@ -895,29 +895,29 @@
 allocator_type get_allocator() const _NOEXCEPT;
 
 _LIBCPP_INLINE_VISIBILITY
-size_type size() const _NOEXCEPT {return base::__sz();}
+size_type size() const _NOEXCEPT {return __base::__sz();}
 _LIBCPP_INLINE_VISIBILITY
-bool empty() const _NOEXCEPT {return base::empty();}
+bool empty() const _NOEXCEPT {return __base::empty();}
 _LIBCPP_INLINE_VISIBILITY
 size_type max_size() const _NOEXCEPT
 {
 return std::min(
-base::__node_alloc_max_size(),
+__base::__node_alloc_max_size(),
 numeric_limits::max());
 }
 
 _LIBCPP_INLINE_VISIBILITY
-  iterator begin() _NOEXCEPT{return base::begin();}
+  iterator begin() _NOEXCEPT{return __base::begin();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator begin()  const _NOEXCEPT {return base::begin();}
+const_iterator begin()  const _NOEXCEPT {return __base::begin();}
 _LIBCPP_INLINE_VISIBILITY
-  iterator end() _NOEXCEPT  {return base::end();}
+  iterator end() _NOEXCEPT  {return __base::end();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator end()const _NOEXCEPT {return base::end();}
+const_iterator end()const _NOEXCEPT {return __base::end();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator cbegin() const _NOEXCEPT {return base::begin();}
+const_iterator cbegin() const _NOEXCEPT {return __base::begin();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator cend()   const _NOEXCEPT {return base::end();}
+const_iterator cend()   const _NOEXCEPT {return __base::end();}
 
 _LIBCPP_INLINE_VISIBILITY
   reverse_iterator rbegin() _NOEXCEPT
@@ 

[libcxx] r313694 - [libc++] Replace __sync_* functions with __libcpp_atomic_* functions

2017-09-19 Thread Weiming Zhao via cfe-commits
Author: weimingz
Date: Tue Sep 19 16:18:03 2017
New Revision: 313694

URL: http://llvm.org/viewvc/llvm-project?rev=313694&view=rev
Log:
[libc++] Replace __sync_* functions with __libcpp_atomic_* functions

Summary:
This patch replaces __sync_* with __libcpp_atomic_* and adds a wrapper
function for __atomic_exchange to support _LIBCPP_HAS_NO_THREADS.

Reviewers: EricWF, jroelofs, mclow.lists, compnerd

Reviewed By: EricWF, compnerd

Subscribers: compnerd, efriedma, cfe-commits, joerg, llvm-commits

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

Modified:
libcxx/trunk/src/exception.cpp
libcxx/trunk/src/include/atomic_support.h
libcxx/trunk/src/include/refstring.h
libcxx/trunk/src/locale.cpp
libcxx/trunk/src/new.cpp
libcxx/trunk/src/support/runtime/exception_fallback.ipp
libcxx/trunk/src/support/runtime/new_handler_fallback.ipp

Modified: libcxx/trunk/src/exception.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/exception.cpp?rev=313694&r1=313693&r2=313694&view=diff
==
--- libcxx/trunk/src/exception.cpp (original)
+++ libcxx/trunk/src/exception.cpp Tue Sep 19 16:18:03 2017
@@ -31,6 +31,7 @@
 #include "support/runtime/exception_glibcxx.ipp"
 #include "support/runtime/exception_pointer_glibcxx.ipp"
 #else
+#include "include/atomic_support.h"
 #include "support/runtime/exception_fallback.ipp"
 #include "support/runtime/exception_pointer_unimplemented.ipp"
 #endif

Modified: libcxx/trunk/src/include/atomic_support.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/include/atomic_support.h?rev=313694&r1=313693&r2=313694&view=diff
==
--- libcxx/trunk/src/include/atomic_support.h (original)
+++ libcxx/trunk/src/include/atomic_support.h Tue Sep 19 16:18:03 2017
@@ -16,6 +16,7 @@
 #if defined(__clang__) && __has_builtin(__atomic_load_n) \
&& __has_builtin(__atomic_store_n)\
&& __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_exchange_n) \
&& __has_builtin(__atomic_compare_exchange_n) \
&& defined(__ATOMIC_RELAXED)  \
&& defined(__ATOMIC_CONSUME)  \
@@ -84,6 +85,14 @@ _ValueType __libcpp_atomic_add(_ValueTyp
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
+_ValueType __libcpp_atomic_exchange(_ValueType* __target,
+_ValueType __value, int __order = _AO_Seq)
+{
+return __atomic_exchange_n(__target, __value, __order);
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY
 bool __libcpp_atomic_compare_exchange(_ValueType* __val,
 _ValueType* __expected, _ValueType __after,
 int __success_order = _AO_Seq,
@@ -136,6 +145,16 @@ _ValueType __libcpp_atomic_add(_ValueTyp
 }
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
+_ValueType __libcpp_atomic_exchange(_ValueType* __target,
+_ValueType __value, int __order = _AO_Seq)
+{
+_ValueType old = *__target;
+*__target = __value;
+return old;
+}
+
+template 
 inline _LIBCPP_INLINE_VISIBILITY
 bool __libcpp_atomic_compare_exchange(_ValueType* __val,
 _ValueType* __expected, _ValueType __after,

Modified: libcxx/trunk/src/include/refstring.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/include/refstring.h?rev=313694&r1=313693&r2=313694&view=diff
==
--- libcxx/trunk/src/include/refstring.h (original)
+++ libcxx/trunk/src/include/refstring.h Tue Sep 19 16:18:03 2017
@@ -18,6 +18,7 @@
 #include 
 #include 
 #endif
+#include "atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -83,7 +84,7 @@ __libcpp_refstring::__libcpp_refstring(c
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 }
 
 inline
@@ -92,10 +93,10 @@ __libcpp_refstring& __libcpp_refstring::
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 if (adjust_old_count)
 {
-if (__sync_add_and_fetch(&old_rep->count, count_t(-1)) < 0)
+if (__libcpp_atomic_add(&old_rep->count, count_t(-1)) < 0)
 {
 ::operator delete(old_rep);
 }
@@ -107,7 +108,7 @@ inline
 __libcpp_refstring::~__libcpp_refstring() {
 if (__uses_refcount()) {
 _Rep_base* rep = rep_from_data(__imp_);
-if (__sync_add_and_fetch(&rep->count, count_t(-1)) < 0) {
+if (__libcpp_atomic_add(&rep->count, count_t(-1)) < 0) {
 ::operator delete(rep);
 }

[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions

2017-09-19 Thread Weiming Zhao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313694: [libc++] Replace __sync_* functions with 
__libcpp_atomic_* functions (authored by weimingz).

Changed prior to commit:
  https://reviews.llvm.org/D35235?vs=115918&id=115922#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35235

Files:
  libcxx/trunk/src/exception.cpp
  libcxx/trunk/src/include/atomic_support.h
  libcxx/trunk/src/include/refstring.h
  libcxx/trunk/src/locale.cpp
  libcxx/trunk/src/new.cpp
  libcxx/trunk/src/support/runtime/exception_fallback.ipp
  libcxx/trunk/src/support/runtime/new_handler_fallback.ipp

Index: libcxx/trunk/src/support/runtime/new_handler_fallback.ipp
===
--- libcxx/trunk/src/support/runtime/new_handler_fallback.ipp
+++ libcxx/trunk/src/support/runtime/new_handler_fallback.ipp
@@ -15,13 +15,13 @@
 new_handler
 set_new_handler(new_handler handler) _NOEXCEPT
 {
-return __sync_lock_test_and_set(&__new_handler, handler);
+return __libcpp_atomic_exchange(&__new_handler, handler);
 }
 
 new_handler
 get_new_handler() _NOEXCEPT
 {
-return __sync_fetch_and_add(&__new_handler, nullptr);
+return __libcpp_atomic_load(&__new_handler);
 }
 
 } // namespace std
Index: libcxx/trunk/src/support/runtime/exception_fallback.ipp
===
--- libcxx/trunk/src/support/runtime/exception_fallback.ipp
+++ libcxx/trunk/src/support/runtime/exception_fallback.ipp
@@ -20,13 +20,13 @@
 unexpected_handler
 set_unexpected(unexpected_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__unexpected_handler, func);
+  return __libcpp_atomic_exchange(&__unexpected_handler, func);
 }
 
 unexpected_handler
 get_unexpected() _NOEXCEPT
 {
-  return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0);
+  return __libcpp_atomic_load(&__unexpected_handler);
 
 }
 
@@ -41,14 +41,13 @@
 terminate_handler
 set_terminate(terminate_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__terminate_handler, func);
+  return __libcpp_atomic_exchange(&__terminate_handler, func);
 }
 
 terminate_handler
 get_terminate() _NOEXCEPT
 {
-  return __sync_fetch_and_add(&__terminate_handler, (terminate_handler)0);
-
+  return __libcpp_atomic_load(&__terminate_handler);
 }
 
 #ifndef __EMSCRIPTEN__ // We provide this in JS
Index: libcxx/trunk/src/include/atomic_support.h
===
--- libcxx/trunk/src/include/atomic_support.h
+++ libcxx/trunk/src/include/atomic_support.h
@@ -16,6 +16,7 @@
 #if defined(__clang__) && __has_builtin(__atomic_load_n) \
&& __has_builtin(__atomic_store_n)\
&& __has_builtin(__atomic_add_fetch)  \
+   && __has_builtin(__atomic_exchange_n) \
&& __has_builtin(__atomic_compare_exchange_n) \
&& defined(__ATOMIC_RELAXED)  \
&& defined(__ATOMIC_CONSUME)  \
@@ -84,6 +85,14 @@
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
+_ValueType __libcpp_atomic_exchange(_ValueType* __target,
+_ValueType __value, int __order = _AO_Seq)
+{
+return __atomic_exchange_n(__target, __value, __order);
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY
 bool __libcpp_atomic_compare_exchange(_ValueType* __val,
 _ValueType* __expected, _ValueType __after,
 int __success_order = _AO_Seq,
@@ -137,6 +146,16 @@
 
 template 
 inline _LIBCPP_INLINE_VISIBILITY
+_ValueType __libcpp_atomic_exchange(_ValueType* __target,
+_ValueType __value, int __order = _AO_Seq)
+{
+_ValueType old = *__target;
+*__target = __value;
+return old;
+}
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY
 bool __libcpp_atomic_compare_exchange(_ValueType* __val,
 _ValueType* __expected, _ValueType __after,
 int = 0, int = 0)
Index: libcxx/trunk/src/include/refstring.h
===
--- libcxx/trunk/src/include/refstring.h
+++ libcxx/trunk/src/include/refstring.h
@@ -18,6 +18,7 @@
 #include 
 #include 
 #endif
+#include "atomic_support.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -83,19 +84,19 @@
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 }
 
 inline
 __libcpp_refstring& __libcpp_refstring::operator=(__libcpp_refstring const& s) _NOEXCEPT {
 bool adjust_old_count = __uses_refcount();
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__sync_add_and_fetch(&rep_from_data(__imp_)->count, 1);
+__libcpp_atomic_add(&rep_from_data(__imp_)->count, 1);
 if (adjust_old_count)

  1   2   >