[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-19 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment.

Thanks for pointing that out, I'm out of office today, will look at describing 
the intention to fall through when I get back in on Monday.




Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+  case llvm::Triple::thumbeb:
+IsBigEndian = true;
+  case llvm::Triple::arm:

xbolva00 wrote:
> Gnu.cpp:241:17: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  IsBigEndian = true;
The fall through is intentional in this case.


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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


[PATCH] D53417: [Clang][PowerPC] Choose a better candidate as function call if there is a compatible vector conversion instead of ambious call error

2018-10-19 Thread Zixuan Wu via Phabricator via cfe-commits
wuzish created this revision.
wuzish added reviewers: hfinkel, nemanjai, Douglasgido, hubert.reinterpretcast, 
fpichet.

There are 2 function variations with altivec type parameter. When we call them 
with argument of generic gcc vector type we would prefer to choose the 
variation with implicit argument conversion of compatible vector type.

  typedef float __v4sf __attribute__((__vector_size__(16)));
  void f(vector float);
  void f(vector signed int);
  
  int main {
 __v4sf a;
 f(a);
  }

Here, we'd like to choose f(vector float) but not report an ambiguous call 
error.


Repository:
  rC Clang

https://reviews.llvm.org/D53417

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Sema/altivec-generic-overload.c

Index: clang/test/Sema/altivec-generic-overload.c
===
--- /dev/null
+++ clang/test/Sema/altivec-generic-overload.c
@@ -0,0 +1,101 @@
+// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only
+
+typedef signed char __v4sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v4uc __attribute__((__vector_size__(16)));
+typedef signed short __v4ss __attribute__((__vector_size__(16)));
+typedef unsigned short __v4us __attribute__((__vector_size__(16)));
+typedef signed int __v4si __attribute__((__vector_size__(16)));
+typedef unsigned int __v4ui __attribute__((__vector_size__(16)));
+typedef signed long long __v4sll __attribute__((__vector_size__(16)));
+typedef unsigned long long __v4ull __attribute__((__vector_size__(16)));
+typedef signed __int128 __v4slll __attribute__((__vector_size__(16)));
+typedef unsigned __int128 __v4ulll __attribute__((__vector_size__(16)));
+typedef float __v4f __attribute__((__vector_size__(16)));
+typedef double __v4d __attribute__((__vector_size__(16)));
+
+void __attribute__((__overloadable__)) convert1(vector signed char);
+void __attribute__((__overloadable__)) convert1(vector unsigned char);
+void __attribute__((__overloadable__)) convert1(vector signed short);
+void __attribute__((__overloadable__)) convert1(vector unsigned short);
+void __attribute__((__overloadable__)) convert1(vector signed int);
+void __attribute__((__overloadable__)) convert1(vector unsigned int);
+void __attribute__((__overloadable__)) convert1(vector signed long long);
+void __attribute__((__overloadable__)) convert1(vector unsigned long long);
+void __attribute__((__overloadable__)) convert1(vector signed __int128);
+void __attribute__((__overloadable__)) convert1(vector unsigned __int128);
+void __attribute__((__overloadable__)) convert1(vector float);
+void __attribute__((__overloadable__)) convert1(vector double);
+void __attribute__((__overloadable__)) convert1(vector bool int);
+void __attribute__((__overloadable__)) convert1(vector pixel short);
+void __attribute__((__overloadable__)) convert2(__v4sc);
+void __attribute__((__overloadable__)) convert2(__v4uc);
+void __attribute__((__overloadable__)) convert2(__v4ss);
+void __attribute__((__overloadable__)) convert2(__v4us);
+void __attribute__((__overloadable__)) convert2(__v4si);
+void __attribute__((__overloadable__)) convert2(__v4ui);
+void __attribute__((__overloadable__)) convert2(__v4sll);
+void __attribute__((__overloadable__)) convert2(__v4ull);
+void __attribute__((__overloadable__)) convert2(__v4slll);
+void __attribute__((__overloadable__)) convert2(__v4ulll);
+void __attribute__((__overloadable__)) convert2(__v4f);
+void __attribute__((__overloadable__)) convert2(__v4d);
+
+void test()
+{
+  __v4sc gv1;
+  __v4uc gv2;
+  __v4ss gv3;
+  __v4us gv4;
+  __v4si gv5;
+  __v4ui gv6;
+  __v4sll gv7;
+  __v4ull gv8;
+  __v4slll gv9;
+  __v4ulll gv10;
+  __v4f gv11;
+  __v4d  gv12;
+
+  vector signed char av1;
+  vector unsigned char av2;
+  vector signed short av3;
+  vector unsigned short av4;
+  vector signed int av5;
+  vector unsigned int av6;
+  vector signed long long av7;
+  vector unsigned long long av8;
+  vector signed __int128 av9;
+  vector unsigned __int128 av10;
+  vector float av11;
+  vector double av12;
+  vector bool int av13;
+  vector pixel short av14;
+
+  convert1(gv1);
+  convert1(gv2);
+  convert1(gv3);
+  convert1(gv4);
+  convert1(gv5);
+  convert1(gv6);
+  convert1(gv7);
+  convert1(gv8);
+  convert1(gv9);
+  convert1(gv10);
+  convert1(gv11);
+  convert1(gv12);
+
+  convert2(av1);
+  convert2(av2);
+  convert2(av3);
+  convert2(av4);
+  convert2(av5);
+  convert2(av6);
+  convert2(av7);
+  convert2(av8);
+  convert2(av9);
+  convert2(av10);
+  convert2(av11);
+  convert2(av12);
+  convert2(av13); // expected-error {{call to 'convert2' is ambiguous}}
+  convert2(av14); // expected-error {{call to 'convert2' is ambiguous}}
+
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3900,6 +3

[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension

2018-10-19 Thread Dmitry Sidorov via Phabricator via cfe-commits
sidorovd added a comment.

Ping


https://reviews.llvm.org/D51402



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


[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+  case llvm::Triple::thumbeb:
+IsBigEndian = true;
+  case llvm::Triple::arm:

peter.smith wrote:
> xbolva00 wrote:
> > Gnu.cpp:241:17: warning: this statement may fall through 
> > [-Wimplicit-fallthrough=]
> >  IsBigEndian = true;
> The fall through is intentional in this case.
Please mark it with LLVM_FALLTHROUGH then.


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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


[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambious call error

2018-10-19 Thread Zixuan Wu via Phabricator via cfe-commits
wuzish added a comment.

If anybody knows who are familiar with C/C++ function overload and code related 
to this issue, please feel free to add them as reviewers and subscribers.


Repository:
  rC Clang

https://reviews.llvm.org/D53417



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


[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 170152.
curdeius added a comment.

Fixed diff.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53372

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  test/clang-tidy/readability-else-after-return-if-constexpr.cpp


Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++17
+
+// Constexpr if is an exception to the rule, we cannot remove the else.
+void f() {
+  if (sizeof(int) > 4)
+return;
+  else
+return;
+  // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return'
+
+  if constexpr (sizeof(int) > 4)
+return;
+  else
+return;
+
+  if constexpr (sizeof(int) > 4)
+return;
+  else if constexpr (sizeof(long) > 4)
+return;
+  else
+return;
+}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -25,7 +25,8 @@
  expr(ignoringImplicit(cxxThrowExpr().bind("throw");
   Finder->addMatcher(
   compoundStmt(forEach(
-  ifStmt(hasThen(stmt(
+  ifStmt(unless(isConstexpr()),
+ hasThen(stmt(
  anyOf(ControlFlowInterruptorMatcher,
compoundStmt(has(ControlFlowInterruptorMatcher),
  hasElse(stmt().bind("else")))


Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++17
+
+// Constexpr if is an exception to the rule, we cannot remove the else.
+void f() {
+  if (sizeof(int) > 4)
+return;
+  else
+return;
+  // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return'
+
+  if constexpr (sizeof(int) > 4)
+return;
+  else
+return;
+
+  if constexpr (sizeof(int) > 4)
+return;
+  else if constexpr (sizeof(long) > 4)
+return;
+  else
+return;
+}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -25,7 +25,8 @@
  expr(ignoringImplicit(cxxThrowExpr().bind("throw");
   Finder->addMatcher(
   compoundStmt(forEach(
-  ifStmt(hasThen(stmt(
+  ifStmt(unless(isConstexpr()),
+ hasThen(stmt(
  anyOf(ControlFlowInterruptorMatcher,
compoundStmt(has(ControlFlowInterruptorMatcher),
  hasElse(stmt().bind("else")))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.

2018-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Thanks! @arphaman, any concerns about the change to extension format?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53391



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


[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.

2018-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 170153.
sammccall marked an inline comment as done.
sammccall added a comment.

Don't call URIForFile again, rebase.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53391

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/fixits-embed-in-diagnostic.test
  unittests/clangd/ClangdUnitTests.cpp

Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -199,11 +199,13 @@
 
   // Transform dianostics and check the results.
   std::vector>> LSPDiags;
-  toLSPDiags(D, [&](clangd::Diagnostic LSPDiag,
-llvm::ArrayRef Fixes) {
-LSPDiags.push_back({std::move(LSPDiag),
-std::vector(Fixes.begin(), Fixes.end())});
-  });
+  toLSPDiags(
+  D, URIForFile("/path/to/foo/bar/main.cpp"), ClangdDiagnosticOptions(),
+  [&](clangd::Diagnostic LSPDiag, llvm::ArrayRef Fixes) {
+LSPDiags.push_back(
+{std::move(LSPDiag),
+ std::vector(Fixes.begin(), Fixes.end())});
+  });
 
   EXPECT_THAT(
   LSPDiags,
Index: test/clangd/fixits-embed-in-diagnostic.test
===
--- test/clangd/fixits-embed-in-diagnostic.test
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -1,12 +1,12 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true}}},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"codeActionsInline":true}}},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
-#  CHECK:"method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:"params": {
-# CHECK-NEXT: "diagnostics": [
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"clangd_fixes": [
+# CHECK-NEXT:"codeActions": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"edit": {
 # CHECK-NEXT:  "changes": {
@@ -27,6 +27,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
+# CHECK-NEXT:"kind": "quickfix",
 # CHECK-NEXT:"title": "change 'union' to 'struct'"
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -323,9 +323,8 @@
   /// workspace.symbol.symbolKind.valueSet
   llvm::Optional WorkspaceSymbolKinds;
 
-  /// Whether the client accepts diagnostics with fixes attached using the
-  /// "clangd_fixes" extension.
-  /// textDocument.publishDiagnostics.clangdFixSupport
+  /// Whether the client accepts diagnostics with codeActions attached inline.
+  /// textDocument.publishDiagnostics.codeActionsInline.
   bool DiagnosticFixes = false;
 
   /// Whether the client accepts diagnostics with category attached to it
@@ -536,6 +535,7 @@
 };
 bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
 
+struct CodeAction;
 struct Diagnostic {
   /// The range at which the message applies.
   Range range;
@@ -560,7 +560,12 @@
   /// An LSP extension that's used to send the name of the category over to the
   /// client. The category typically describes the compilation stage during
   /// which the issue was produced, e.g. "Semantic Issue" or "Parse Issue".
-  std::string category;
+  llvm::Optional category;
+
+  /// Clangd extension: code actions related to this diagnostic.
+  /// Only with capability textDocument.publishDiagnostics.codeActionsInline.
+  /// (These actions can also be obtained using textDocument/codeAction).
+  llvm::Optional> codeActions;
 };
 llvm::json::Value toJSON(const Diagnostic &);
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -210,8 +210,8 @@
 if (auto *Diagnostics = TextDocument->getObject("publishDiagnostics")) {
   if (auto CategorySupport = Diagnostics->getBoolean("categorySupport"))
 R.DiagnosticCategory = *CategorySupport;
-  if (auto ClangdFixSupport = Diagnostics->getBoolean("clangdFixSupport"))
-R.DiagnosticFixes = *ClangdFixSupport;
+  if (auto CodeActions = Diagnostics->getBoolean("codeActionsInline"))
+R.DiagnosticFixes = *CodeActions;
 }
 if (auto *Completion = TextDocument->getObject("

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2018-10-19 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment.

Do you know the better way to accomplish my aim than adding an option to 
libFormat? For example making a dependent library which serves a little 
different purpose than libFormat itself or something simpler?


https://reviews.llvm.org/D53072



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


[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

2018-10-19 Thread Alexey Sotkin via Phabricator via cfe-commits
AlexeySotkin added a comment.

Ping


https://reviews.llvm.org/D51484



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


[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-19 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment.

ping? Can I go forward in this way?


https://reviews.llvm.org/D51340



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


[PATCH] D53400: [clangd] Remove the overflow log.

2018-10-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 170157.
hokein marked an inline comment as done.
hokein added a comment.

Log the whole location when overflow happens.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53400

Files:
  clangd/XRefs.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  unittests/clangd/IndexTests.cpp


Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -46,10 +46,15 @@
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -42,6 +42,10 @@
 void setColumn(uint32_t Column);
 uint32_t column() const { return Column; }
 
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
 Line = MaxLine;
 return;
   }
   Line = L;
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
 Column = MaxColumn;
 return;
   }
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,11 @@
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();
+  logIfOverflow(Loc);
   return LSPLoc;
 }
 


Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -46,10 +46,15 @@
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -42,6 +42,10 @@
 void setColumn(uint32_t Column);
 uint32_t column() const { return Column; }
 
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
 Line = MaxLine;
 return;
   }
   Line = L;
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
 Column = MaxColumn;
 return;
   }
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,11 @@
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@
   LSPLoc.rang

[PATCH] D53400: [clangd] Remove the overflow log.

2018-10-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clangd/XRefs.cpp:43
+  if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+  return Line;

sammccall wrote:
> Log message could use more context. And I think it'd be really useful to 
> print the whole location here.
> 
> bikeshed: you could add a method `bool Position::hasOverflow()`, and then 
> write below
> 
> ```
> if (LSPLoc.range.start.hasOverflow() || LSPLoc.range.end.hasOverflow())
>   log("Possible overflow in symbol location: {0}", LSPLoc);
> ```
> 
> Distinguishing between line/column overflow isn't important if you're 
> printing the full range anyway.
This is neat. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53400



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


[PATCH] D53400: [clangd] Remove the overflow log.

2018-10-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 170159.
hokein added a comment.

Minor cleanup.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53400

Files:
  clangd/XRefs.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  unittests/clangd/IndexTests.cpp


Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -46,10 +46,15 @@
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -43,6 +43,10 @@
 void setColumn(uint32_t Column);
 uint32_t column() const { return Column; }
 
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
 Line = MaxLine;
 return;
   }
   Line = L;
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
 Column = MaxColumn;
 return;
   }
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,11 @@
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation &Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();
+  logIfOverflow(Loc);
   return LSPLoc;
 }
 


Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -46,10 +46,15 @@
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -43,6 +43,10 @@
 void setColumn(uint32_t Column);
 uint32_t column() const { return Column; }
 
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
 Line = MaxLine;
 return;
   }
   Line = L;
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
 Column = MaxColumn;
 return;
   }
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,11 @@
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation &Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc

[clang-tools-extra] r344777 - [clangd] Remove the overflow log.

2018-10-19 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Oct 19 01:35:24 2018
New Revision: 344777

URL: http://llvm.org/viewvc/llvm-project?rev=344777&view=rev
Log:
[clangd] Remove the overflow log.

Summary:
LLVM codebase has generated files (all are build/Target/XXX/*.inc) that
exceed the MaxLine & MaxColumn. Printing these log would be noisy.

Reviewers: sammccall

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

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=344777&r1=344776&r2=344777&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Oct 19 01:35:24 2018
@@ -37,6 +37,11 @@ const Decl *getDefinition(const Decl *D)
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation &Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@ llvm::Optional toLSPLocation(c
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();
+  logIfOverflow(Loc);
   return LSPLoc;
 }
 

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=344777&r1=344776&r2=344777&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Fri Oct 19 01:35:24 2018
@@ -23,7 +23,6 @@ constexpr uint32_t SymbolLocation::Posit
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
 Line = MaxLine;
 return;
   }
@@ -31,7 +30,6 @@ void SymbolLocation::Position::setLine(u
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
 Column = MaxColumn;
 return;
   }

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=344777&r1=344776&r2=344777&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Fri Oct 19 01:35:24 2018
@@ -43,6 +43,10 @@ struct SymbolLocation {
 void setColumn(uint32_t Column);
 uint32_t column() const { return Column; }
 
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 

Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp?rev=344777&r1=344776&r2=344777&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Fri Oct 19 01:35:24 
2018
@@ -46,10 +46,15 @@ TEST(SymbolLocation, Position) {
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 


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


[PATCH] D53400: [clangd] Remove the overflow log.

2018-10-19 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344777: [clangd] Remove the overflow log. (authored by 
hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53400

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/clangd/index/Index.cpp
  clang-tools-extra/trunk/clangd/index/Index.h
  clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
@@ -46,10 +46,15 @@
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -37,6 +37,11 @@
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation &Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();
+  logIfOverflow(Loc);
   return LSPLoc;
 }
 
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -43,6 +43,10 @@
 void setColumn(uint32_t Column);
 uint32_t column() const { return Column; }
 
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -23,15 +23,13 @@
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
 Line = MaxLine;
 return;
   }
   Line = L;
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
 Column = MaxColumn;
 return;
   }


Index: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
@@ -46,10 +46,15 @@
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -37,6 +37,11 @@
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation &Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
@@ -61,6 +66,7 @@
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();
+  logIfOverflow(Loc);
   return LSPLoc;
 }
 
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -43,6 +43,10 @@
 void setColumn(uint32_t Column);
 uint32_t co

r344778 - [OpenCL] Remove unwanted signedness conversion from tests

2018-10-19 Thread Marco Antognini via cfe-commits
Author: mantognini
Date: Fri Oct 19 02:01:37 2018
New Revision: 344778

URL: http://llvm.org/viewvc/llvm-project?rev=344778&view=rev
Log:
[OpenCL] Remove unwanted signedness conversion from tests

The get_kernel_* functions used in cl20-device-side-enqueue.cl all return
unsigned integers. This patch avoids undesired implicit conversions on the
returned values.

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


Modified:
cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl?rev=344778&r1=344777&r2=344778&view=diff
==
--- cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl (original)
+++ cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl Fri Oct 19 02:01:37 
2018
@@ -212,7 +212,7 @@ kernel void work_group_size_tests() {
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
 
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
@@ -220,7 +220,7 @@ kernel void foo(global int *buf)
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // 
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', 
expected block argument type}}
 }
 
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
 {
   __private ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@ kernel void bar(global int *buf)
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : disable
 
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // 
expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' 
requires cl_khr_subgroups extension to be enabled}}
 }
 
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error 
{{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires 
cl_khr_subgroups extension to be enabled}}


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


[PATCH] D52873: Remove unwanted signedness conversion from tests

2018-10-19 Thread Marco Antognini via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344778: [OpenCL] Remove unwanted signedness conversion from 
tests (authored by mantognini, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D52873

Files:
  test/SemaOpenCL/cl20-device-side-enqueue.cl


Index: test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -212,15 +212,15 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
 
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // 
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', 
expected 'ndrange_t' argument type}}
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // 
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', 
expected block argument type}}
 }
 
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
 {
   __private ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : disable
 
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // 
expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' 
requires cl_khr_subgroups extension to be enabled}}
 }
 
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error 
{{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires 
cl_khr_subgroups extension to be enabled}}


Index: test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -212,15 +212,15 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
 
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}}
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}}
 }
 
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
 {
   __private ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : disable
 
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}}
 }
 
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52873: Remove unwanted signedness conversion from tests

2018-10-19 Thread Marco Antognini via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344778: [OpenCL] Remove unwanted signedness conversion from 
tests (authored by mantognini, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52873?vs=168256&id=170165#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52873

Files:
  cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl


Index: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -212,15 +212,15 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
 
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // 
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', 
expected 'ndrange_t' argument type}}
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // 
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', 
expected block argument type}}
 }
 
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
 {
   __private ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : disable
 
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // 
expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' 
requires cl_khr_subgroups extension to be enabled}}
 }
 
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error 
{{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires 
cl_khr_subgroups extension to be enabled}}


Index: cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ cfe/trunk/test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -212,15 +212,15 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
 
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}}
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}}
 }
 
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
 {
   __private ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : disable
 
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}}
 }
 
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
 {
   ndrange_t n;
   buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53369: [CodeComplete] Fix accessibility of protected members when accessing members implicitly.

2018-10-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

The change looks reasonable to me, I'd let @ilya-biryukov take a second look.


Repository:
  rC Clang

https://reviews.llvm.org/D53369



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


[PATCH] D51633: [ASTImporter] Added error handling for AST import.

2018-10-19 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added inline comments.



Comment at: lib/AST/ASTImporter.cpp:8197
 
-void ASTImporter::ImportDefinition(Decl *From) {
+Error ASTImporter::ImportDefinition_New(Decl *From) {
   Decl *To = Import(From);

a_sidorin wrote:
> ImportDefinitionOrError?
The intention was to make the `_New` name only temporary until only this new 
function is used therefore it is not a nice name (requires changes in LLDB). 
Then it should be renamed back to `ImportDefinition`. It is probably not needed 
to indicate in the name that the function can fail, it can be seen from the 
return value. (This will be the case with other `Import` functions later.)


Repository:
  rC Clang

https://reviews.llvm.org/D51633



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


[PATCH] D53397: [MinGW] Link to correct openmp library

2018-10-19 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

Please add a testcase in test/Driver/, it looks like fopenmp.c there might be a 
good target to add a few lines in.


Repository:
  rC Clang

https://reviews.llvm.org/D53397



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


[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

2018-10-19 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy updated this revision to Diff 170169.
donat.nagy added a comment.

Give a reference for the significand size values of the IEEE 754 floating point 
types; cleanup comments and formatting.


Repository:
  rC Clang

https://reviews.llvm.org/D52730

Files:
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  test/Analysis/conversion.c

Index: test/Analysis/conversion.c
===
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -137,6 +137,12 @@
   U8 = S + 10;
 }
 
+char dontwarn6(long long x) {
+  long long y = 42;
+  y += x;
+  return y == 42;
+}
+
 
 // C library functions, handled via apiModeling.StdCLibraryFunctions
 
@@ -154,7 +160,7 @@
 # define EOF (-1)
 char reply_string[8192];
 FILE *cin;
-extern int dostuff (void);
+extern int dostuff(void);
 int libraryFunction2() {
   int c, n;
   int dig;
@@ -179,3 +185,26 @@
   }
 }
 
+double floating_point(long long a, int b) {
+  if (a > 1LL << 55) {
+double r = a; // expected-warning {{Loss of precision}}
+return r;
+  } else if (b > 1 << 25) {
+float f = b; // expected-warning {{Loss of precision}}
+return f;
+  }
+  return 137;
+}
+
+double floating_point2() {
+  int a = 1 << 24;
+  long long b = 1LL << 53;
+  double d = a; // no-warning
+  double f = b; // no-warning
+  return d - f;
+}
+
+int floating_point_3(unsigned long long a) {
+  double b = a; // no-warning
+  return 42;
+}
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -14,8 +14,10 @@
 // of expressions. A warning is reported when:
 // * a negative value is implicitly converted to an unsigned value in an
 //   assignment, comparison or multiplication.
-// * assignment / initialization when source value is greater than the max
-//   value of target
+// * assignment / initialization when the source value is greater than the max
+//   value of the target integer type
+// * assignment / initialization when the source integer is above the range
+//   where the target floating point type can represent all integers
 //
 // Many compilers and tools have similar checks that are based on semantic
 // analysis. Those checks are sound but have poor precision. ConversionChecker
@@ -29,6 +31,8 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
+#include 
+
 using namespace clang;
 using namespace ento;
 
@@ -40,11 +44,9 @@
 private:
   mutable std::unique_ptr BT;
 
-  // Is there loss of precision
   bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType,
  CheckerContext &C) const;
 
-  // Is there loss of sign
   bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const;
 
   void reportBug(ExplodedNode *N, CheckerContext &C, const char Msg[]) const;
@@ -132,19 +134,72 @@
 
   QualType SubType = Cast->IgnoreParenImpCasts()->getType();
 
-  if (!DestType->isIntegerType() || !SubType->isIntegerType())
+  if (!DestType->isRealType() || !SubType->isIntegerType())
 return false;
 
-  if (C.getASTContext().getIntWidth(DestType) >=
-  C.getASTContext().getIntWidth(SubType))
+  const bool isInteger = DestType->isIntegerType();
+
+  const auto &AC = C.getASTContext();
+
+  // We will find the largest RepresentsUntilExp value such that the DestType
+  // can exactly represent all nonnegative integers below 2^RepresentsUntilExp.
+  unsigned RepresentsUntilExp;
+
+  if (isInteger) {
+RepresentsUntilExp = AC.getIntWidth(DestType);
+if (RepresentsUntilExp == 1) {
+  // This is just casting a number to bool, probably not a bug.
+  return false;
+}
+if (DestType->isSignedIntegerType())
+  RepresentsUntilExp--;
+  } else {
+unsigned FloatingSize = AC.getTypeSize(DestType);
+
+// We assume that we are dealing with IEEE 754 floating point types
+// (including e.g. the 80-bit long double). The constants in the following
+// code come from the standard, see e.g.: https://enwp.org/IEEE_754
+
+switch (FloatingSize) {
+  case 64:
+// double (and possibly long double on some systems)
+RepresentsUntilExp = 53;
+break;
+  case 32:
+// float
+RepresentsUntilExp = 24;
+break;
+  case 16:
+// _Float16
+RepresentsUntilExp = 12;
+break;
+  default:
+// A larger type, which can represent all integers below 2^64.
+return false;
+}
+  }
+
+  if (RepresentsUntilExp >= sizeof(unsigned long long) * CHAR_BIT) {
+// Avoid overflow in our later calculations.
 return false;
+  }
 
-  unsigned W = C.getASTContext().getIntWidth(DestType);
-  if (W == 1 || W >= 64U)
+  unsigned CorrectedSrcWidth = AC.getIntWidth(SubType);
+  if (SubType->isSignedIntegerType())
+Cor

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

2018-10-19 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy marked 2 inline comments as done.
donat.nagy added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164
+// double and possibly long double on some systems
+RepresentsUntilExp = 53; break;
+  case 32:

donat.nagy wrote:
> xazax.hun wrote:
> > A link to the source of these number would be useful. How are these 
> > calculated. Also,  as far as I know the current C++ standard does not 
> > require anything about how floating points are represented in an 
> > implementation. So it would be great to somehow refer to the representation 
> > used by clang rather than hardcoding these values. (Note that I am 
> > perfectly fine with warning for implementation defined behavior as the 
> > original version also warn for such in case of integers.) 
> I took these magic numbers from the IEEE 754 standard; I completely agree 
> that their introduction is far from being elegant.
> 
> Unfortunately it seems that referring to the representation used by clang 
> seems to be somewhat difficult, see e.g. this old [[ 
> https://stackoverflow.com/questions/13780931/how-do-i-get-llvm-types-from-clang
>  | stackoverflow answer ]].  In the Z3 solver a similar problem was solved by 
> defining a static function ([[ 
> https://clang.llvm.org/doxygen/Z3ConstraintManager_8cpp_source.html#l00269 | 
> getFloatSemantics() ]]) which uses a switch to translate the bit width of a 
> floating point type into an llvm::fltSemantics value (which contains the 
> precision value as a field).
> 
> I could imagine three solutions: 
> 
>   - reimplementing the logic getFloatSemantics,
>   - moving getFloatSemantics to some utility library and using it from there,
>   - keeping the current code, with comments describing my assumptions and 
> referencing the IEEE standard.
> 
> Which of these is the best?
> 
> Note: According to the documentation [[ 
> https://releases.llvm.org/2.5/docs/LangRef.html#t_floating | the floating 
> point types ]] supported by the LLVM IR are just float, double and some high 
> precision extension types (which are handled by the `default:` branch of my 
> code). Unfortunately, I do not know what happens to the [[ 
> https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
>  | `_Float16` ]] half-width float type.
I updated the patch with a comment describing my assumptions, but I will 
implement a different solution if that would be better.


Repository:
  rC Clang

https://reviews.llvm.org/D52730



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


[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

2018-10-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.

This would save us 8 bytes per ref, and buy us ~50MB in total for llvm
index (from ~350MB to ~300 MB).

The char pointer must be null-terminated, and llvm::StringSaver
guarantees it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53427

Files:
  clangd/CodeComplete.cpp
  clangd/FindSymbols.cpp
  clangd/Quality.cpp
  clangd/XRefs.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/YAMLSerialization.cpp
  clangd/index/dex/Dex.cpp
  clangd/index/dex/dexp/Dexp.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/DexTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SerializationTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,8 +53,8 @@
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
-MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
+MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.fileURI() == P; }
+MATCHER_P(DefURI, P, "") { return arg.Definition.fileURI() == P; }
 MATCHER_P(IncludeHeader, P, "") {
   return (arg.IncludeHeaders.size() == 1) &&
  (arg.IncludeHeaders.begin()->IncludeHeader == P);
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -110,7 +110,7 @@
   EXPECT_EQ(Sym1.Signature, "");
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
-  EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
+  EXPECT_EQ(Sym1.CanonicalDeclaration.fileURI(), "file:///path/foo.h");
   EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
   EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
@@ -121,7 +121,7 @@
   EXPECT_THAT(Sym2, QName("clang::Foo2"));
   EXPECT_EQ(Sym2.Signature, "-sig");
   EXPECT_EQ(Sym2.ReturnType, "");
-  EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
+  EXPECT_EQ(Sym2.CanonicalDeclaration.fileURI(), "file:///path/bar.h");
   EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
 
@@ -133,7 +133,7 @@
testing::SizeIs(1;
   auto Ref1 = ParsedYAML->Refs->begin()->second.front();
   EXPECT_EQ(Ref1.Kind, RefKind::Reference);
-  EXPECT_EQ(Ref1.Location.FileURI, "file:///path/foo.cc");
+  EXPECT_EQ(Ref1.Location.fileURI(), "file:///path/foo.cc");
 }
 
 std::vector YAMLFromSymbols(const SymbolSlab &Slab) {
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -36,7 +36,7 @@
  std::make_tuple(Range.start.line, Range.start.character,
  Range.end.line, Range.end.character);
 }
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+MATCHER_P(FileURI, F, "") { return arg.Location.fileURI() == F; }
 
 TEST(SymbolLocation, Position) {
   using Position = SymbolLocation::Position;
@@ -206,8 +206,8 @@
   Symbol L, R;
   L.ID = R.ID = SymbolID("hello");
   L.Name = R.Name = "Foo";   // same in both
-  L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs
-  R.CanonicalDeclaration.FileURI = "file:///right.h";
+  L.CanonicalDeclaration.setFileURI("file:///left.h"); // differs
+  R.CanonicalDeclaration.setFileURI("file:///right.h");
   L.References = 1;
   R.References = 2;
   L.Signature = "()";   // present in left only
@@ -218,7 +218,7 @@
 
   Symbol M = mergeSymbol(L, R);
   EXPECT_EQ(M.Name, "Foo");
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h");
+  EXPECT_EQ(M.CanonicalDeclaration.fileURI(), "file:///left.h");
   EXPECT_EQ(M.References, 3u);
   EXPECT_EQ(M.Signature, "()");
   EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}");
@@ -231,20 +231,20 @@
   Symbol L, R;
 
   L.ID = R.ID = SymbolID("hello");
-  L.CanonicalDeclaration.FileURI = "file:/left.h";
-  R.CanonicalDeclaration.FileURI = "file:/right.h";
+  L.CanonicalDeclaration.setFileURI("file:/left.h");
+  R.CanonicalDeclaration.setFileURI("file:/right.h");
   L.Name = "left";
   R.Name = "right";
 
   Symbol M = mergeSymbol(L, R);
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h");
-  EXPECT_EQ(M.Definition.Fil

[PATCH] D53428: Adjust cl::opt uses for opt_storage refactored

2018-10-19 Thread Yevgeny Rouban via Phabricator via cfe-commits
yrouban created this revision.
yrouban added reviewers: chandlerc, rsmith, reames, skatkov.
Herald added a subscriber: cfe-commits.

This patch fixes compilation errors that result from refactoring of LLVM 
cl::opt structures introduced by https://reviews.llvm.org/D53426.
Basically for class-based cl::opt to access to the instance of MyClass 
one of the overloadable operators should be used.

For example if we have an option defined as cl::opt MyStringOption;
Then the changes should look like the following:

Old: MyStringOption.c_str()
New: MyStringOption->c_str()

Old: cout << MyStringOption;
New: cout << *MyStringOption;


Repository:
  rC Clang

https://reviews.llvm.org/D53428

Files:
  lib/Tooling/Execution.cpp
  tools/arcmt-test/arcmt-test.cpp
  tools/clang-import-test/clang-import-test.cpp
  tools/clang-rename/ClangRename.cpp
  tools/diagtool/FindDiagnosticID.cpp
  unittests/Tooling/ExecutionTest.cpp


Index: unittests/Tooling/ExecutionTest.cpp
===
--- unittests/Tooling/ExecutionTest.cpp
+++ unittests/Tooling/ExecutionTest.cpp
@@ -158,7 +158,7 @@
   auto Executor = internal::createExecutorFromCommandLineArgsImpl(
   argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
-  EXPECT_EQ(BeforeReset, "set");
+  EXPECT_EQ(*BeforeReset, "set");
   BeforeReset.removeArgument();
 }
 
Index: tools/diagtool/FindDiagnosticID.cpp
===
--- tools/diagtool/FindDiagnosticID.cpp
+++ tools/diagtool/FindDiagnosticID.cpp
@@ -66,7 +66,7 @@
   return 0;
 }
 
-llvm::errs() << "error: invalid diagnostic '" << DiagnosticName << "'\n";
+llvm::errs() << "error: invalid diagnostic '" << *DiagnosticName << "'\n";
 return 1;
   }
   OS << Diag->DiagID << "\n";
Index: tools/clang-rename/ClangRename.cpp
===
--- tools/clang-rename/ClangRename.cpp
+++ tools/clang-rename/ClangRename.cpp
@@ -104,9 +104,9 @@
   if (!Input.empty()) {
 // Populate QualifiedNames and NewNames from a YAML file.
 ErrorOr> Buffer =
-llvm::MemoryBuffer::getFile(Input);
+llvm::MemoryBuffer::getFile(*Input);
 if (!Buffer) {
-  errs() << "clang-rename: failed to read " << Input << ": "
+  errs() << "clang-rename: failed to read " << *Input << ": "
  << Buffer.getError().message() << "\n";
   return 1;
 }
Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -174,7 +174,7 @@
 
   {
 using namespace driver::types;
-ID Id = lookupTypeForTypeSpecifier(Input.c_str());
+ID Id = lookupTypeForTypeSpecifier(Input->c_str());
 assert(Id != TY_INVALID);
 if (isCXX(Id)) {
   Inv->getLangOpts()->CPlusPlus = true;
Index: tools/arcmt-test/arcmt-test.cpp
===
--- tools/arcmt-test/arcmt-test.cpp
+++ tools/arcmt-test/arcmt-test.cpp
@@ -243,7 +243,7 @@
   if (RemappingsFile.empty())
 inputBuf = MemoryBuffer::getSTDIN();
   else
-inputBuf = MemoryBuffer::getFile(RemappingsFile);
+inputBuf = MemoryBuffer::getFile(*RemappingsFile);
   if (!inputBuf) {
 errs() << "error: could not read remappings input\n";
 return true;
Index: lib/Tooling/Execution.cpp
===
--- lib/Tooling/Execution.cpp
+++ lib/Tooling/Execution.cpp
@@ -82,7 +82,7 @@
 return std::move(*Executor);
   }
   return llvm::make_error(
-  llvm::Twine("Executor \"") + ExecutorName + "\" is not registered.",
+  llvm::Twine("Executor \"") + *ExecutorName + "\" is not registered.",
   llvm::inconvertibleErrorCode());
 }
 } // end namespace internal


Index: unittests/Tooling/ExecutionTest.cpp
===
--- unittests/Tooling/ExecutionTest.cpp
+++ unittests/Tooling/ExecutionTest.cpp
@@ -158,7 +158,7 @@
   auto Executor = internal::createExecutorFromCommandLineArgsImpl(
   argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
-  EXPECT_EQ(BeforeReset, "set");
+  EXPECT_EQ(*BeforeReset, "set");
   BeforeReset.removeArgument();
 }
 
Index: tools/diagtool/FindDiagnosticID.cpp
===
--- tools/diagtool/FindDiagnosticID.cpp
+++ tools/diagtool/FindDiagnosticID.cpp
@@ -66,7 +66,7 @@
   return 0;
 }
 
-llvm::errs() << "error: invalid diagnostic '" << DiagnosticName << "'\n";
+llvm::errs() << "error: invalid diagnostic '" << *DiagnosticName << "'\n";
 return 1;
   }
   OS << Diag->DiagID << "\n";
Index: tools/clang-rename/ClangRename.cpp
===
--- tools/clang-rename/ClangRename.cpp
+++ tools/clang-rename/ClangR

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

2018-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:182
+  //  - if there were multiple replies, only the first is sent
+  class ReplyOnce {
+struct State {

As discussed offline, we could potentially make this non-copyable to make 
harder to be duplicated and called more than once. And we could also get rid of 
the shared `State` in the functor. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53399



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


[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

2018-10-19 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment.

Excuse me for bring this up so late, but why do we want to make any such 
promises? As in: fundamentally, LLVM IR doesn't have any order property on the 
module level. I have yet so seen reasonable code where the order of functions 
matters for anything but performance. I've seen a few things that required 
`-funit-at-a-time`, most noticable GCC's CRT implementation. But those are all 
major hacks. So under what premise is it useful to have to even pretend to 
honor source code order?


Repository:
  rC Clang

https://reviews.llvm.org/D34796



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


[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

2018-10-19 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:69
+  // Macros are ignored.
+  if (Arg->getBeginLoc().isMacroID())
+return;

hwright wrote:
> JonasToth wrote:
> > maybe `assert` instead, as your comment above suggests that macros are 
> > already filtered out?
> This is a different check than above.
> 
> In the first case, we want to be sure we aren't replacing cases inside of a 
> macro, such as:
> ```
> #define SECONDS(x) absl::Seconds(x)
> SECONDS(1.0)
> ```
> 
> In this one, we want to avoid changing the argument if it is itself a macro:
> ```
> #define VAL 1.0
> absl::Seconds(VAL);
> ```
> 
> So it is a separate check, not just a re-assertion of the first one.
Ok, I misunderstood the code then :)



Comment at: test/clang-tidy/abseil-duration-factory-float.cpp:32
+void ConvertFloatTest() {
+  absl::Duration d;
+

hwright wrote:
> JonasToth wrote:
> > Could you also provide test cases with hexadecimal floating literals, which 
> > are C++17? The thousand-separators could be checked as well (dunno the 
> > correct term right now, but the `1'000'000` feature).
> > Please add test cases for negative literals, too.
> Added the hexadecimal floating literal tests.
> 
> The testing infrastructure wouldn't build the test source with `3'000` as a 
> literal argument.  (There's also an argument that by the time we get to the 
> AST, that distinction is gone anyway and this test isn't the place to check 
> comprehensive literal parsing.)
> 
> I've also added a negative literal test, to illustrate that we don't yet 
> handle that case (which is in practice pretty rare).  I'd rather add it in a 
> separate change.
I am happy with that. I agree that parsing should deal with it fine and your 
code seems to do it fine as well. My experience is, that sometimes there are 
still surprises :)


https://reviews.llvm.org/D53339



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


[PATCH] D53433: [clangd] *Prototype* auto-index stores symbols per-file instead of per-TU.

2018-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision.
ioeric added reviewers: sammccall, hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.

This allows us to deduplicate header symbols across TUs. File digests
are collects when collecting symbols/refs. And the index store deduplicates
file symbols based on the file digest.

WIP: This is still a prototype and apparently needs better names and more tests.
Looking for early feedback to make sure this is heading the right direction.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/IndexAction.cpp
  clangd/index/IndexAction.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/indexer/IndexerMain.cpp
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/SyncAPI.cpp
  unittests/clangd/SyncAPI.h

Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -52,6 +52,7 @@
 
 SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
 SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
+RefSlab getRefs(const SymbolIndex &Index, SymbolID ID);
 
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "SyncAPI.h"
+#include "index/Index.h"
 
 namespace clang {
 namespace clangd {
@@ -137,5 +138,14 @@
   return std::move(Builder).build();
 }
 
+RefSlab getRefs(const SymbolIndex &Index, SymbolID ID) {
+  RefsRequest Req;
+  Req.IDs = {ID};
+  RefSlab::Builder Slab;
+  Index.refs(Req, [&](const Ref &S) { Slab.insert(ID, S); });
+  return std::move(Slab).build();
+}
+
+
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -72,14 +72,6 @@
   return llvm::make_unique(std::move(Slab).build());
 }
 
-RefSlab getRefs(const SymbolIndex &I, SymbolID ID) {
-  RefsRequest Req;
-  Req.IDs = {ID};
-  RefSlab::Builder Slab;
-  I.refs(Req, [&](const Ref &S) { Slab.insert(ID, S); });
-  return std::move(Slab).build();
-}
-
 TEST(FileSymbolsTest, UpdateAndGet) {
   FileSymbols FS;
   EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""), IsEmpty());
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -1,36 +1,78 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/Background.h"
+#include "gmock/gmock-generated-matchers.h"
+#include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+using testing::_;
+using testing::AllOf;
+using testing::Not;
 using testing::UnorderedElementsAre;
 
 namespace clang {
 namespace clangd {
 
 MATCHER_P(Named, N, "") { return arg.Name == N; }
+MATCHER(Declared, "") { return !arg.CanonicalDeclaration.FileURI.empty(); }
+MATCHER(Defined, "") { return !arg.Definition.FileURI.empty(); }
+
+MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+testing::Matcher
+RefsAre(std::vector> Matchers) {
+  return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
+}
 
 TEST(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
   // Currently we store symbols for each TU, so we get both.
-  FS.Files[testPath("root/A.h")] = "void a_h(); void NAME(){}";
-  FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
-  FS.Files[testPath("root/B.cc")] = "#define NAME bar\n#include \"A.h\"";
+  FS.Files[testPath("root/A.h")] = R"(
+  void common();
+  void f_b();
+  #if CC == A
+class A_H {};
+  #elif CC == B
+class B_H {};
+  #else
+class _H {};
+  #endif
+  )";
+  FS.Files[testPath("root/A.cc")] =
+  "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/B.cc")] =
+  "#define CC B\n#include \"A.h\"\nvoid f_b() { (void)common; }";
   BackgroundIndex Idx(Context::empty(), "", FS);
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
   Cmd.Directory = testPath("root");
-  Cmd.CommandLine = {"clang++", "-DNAME=foo", testPath("root/A.cc")};
+  Cmd.CommandLine = {"clang++", "-DCC=A", testPath("root/A.cc")};
   Idx.enqueue(testPath("root"), Cmd);
-  Cmd.CommandLine.back() = Cmd.Filename = testPath("root/B.cc");
+
+  Idx.blockUntilIdleForTest();

[PATCH] D53434: Java annotation declaration being handled correctly

2018-10-19 Thread Sam Maier via Phabricator via cfe-commits
SamMaier created this revision.
Herald added a subscriber: cfe-commits.

Previously, Java annotation declarations (`@interface AnnotationName`) were 
being handled as ObjC interfaces. This caused the brace formatting to mess up, 
so that when you had a class with an interface defined in it, it would indent 
the final brace of the class.

It used to format this class like so:

  class A {
@interface B {}
}

But will now just skip the @interface and format it like so:

  class A {
@interface B {}
  }


Repository:
  rC Clang

https://reviews.llvm.org/D53434

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestJava.cpp


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -155,6 +155,15 @@
"  void doStuff(int theStuff);\n"
"  void doMoreStuff(int moreStuff);\n"
"}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {\n"
+   "int stuff;\n"
+   "void doMoreStuff(int moreStuff);\n"
+   "  }\n"
+   "}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {}\n"
+   "}");
 }
 
 TEST_F(FormatTestJava, AnonymousClasses) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1130,6 +1130,10 @@
 nextToken();
 parseBracedList();
 break;
+  } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
   }
   switch (FormatTok->Tok.getObjCKeywordID()) {
   case tok::objc_public:


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -155,6 +155,15 @@
"  void doStuff(int theStuff);\n"
"  void doMoreStuff(int moreStuff);\n"
"}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {\n"
+   "int stuff;\n"
+   "void doMoreStuff(int moreStuff);\n"
+   "  }\n"
+   "}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {}\n"
+   "}");
 }
 
 TEST_F(FormatTestJava, AnonymousClasses) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1130,6 +1130,10 @@
 nextToken();
 parseBracedList();
 break;
+  } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
   }
   switch (FormatTok->Tok.getObjCKeywordID()) {
   case tok::objc_public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

2018-10-19 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3908
+   QualType ToType) {
+assert(FromType->isVectorType() && "FromType should be a vector type");
+assert(ToType->isVectorType() && "ToType should be a vector type");

These `assert`s would be redundant if `castAs` is used instead of `getAs`.



Comment at: clang/lib/Sema/SemaOverload.cpp:3911
+
+if (FromType->getAs()->getVectorKind() ==
+VectorType::AltiVecVector ||

Use `castAs` here and below.



Comment at: clang/lib/Sema/SemaOverload.cpp:3930
+  // }
+  // Here, we'd like to choose f(vector float) but not
+  // report an ambiguous call error

s/but/and/;


Repository:
  rC Clang

https://reviews.llvm.org/D53417



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


[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-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:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53372



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


r344784 - [ASTImporter] Removed uneeded default case label.

2018-10-19 Thread Balazs Keri via cfe-commits
Author: balazske
Date: Fri Oct 19 08:16:51 2018
New Revision: 344784

URL: http://llvm.org/viewvc/llvm-project?rev=344784&view=rev
Log:
[ASTImporter] Removed uneeded default case label.

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=344784&r1=344783&r2=344784&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Oct 19 08:16:51 2018
@@ -89,9 +89,9 @@ namespace clang {
   return "UnsupportedConstruct";
 case Unknown:
   return "Unknown error";
-default:
-  llvm_unreachable("Invalid error code.");
 }
+llvm_unreachable("Invalid error code.");
+return "Invalid error code.";
   }
 
   void ImportError::log(raw_ostream &OS) const {


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


[PATCH] D53433: [clangd] *Prototype* auto-index stores symbols per-file instead of per-TU.

2018-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clangd/index/FileIndex.cpp:167
+}
+// FIXME: aggregate symbol reference count based on references.
+for (const auto &Sym : FileSyms.getValue()->Refs) {

This is a good FIXME to carry over to FileSymbols::buildIndex(). It may (or may 
not) make sense to control this behavior with the same param that controls 
merge behavior, the logic being that it's also a quality/accuracy tradeoff.



Comment at: clangd/index/FileIndex.h:49
+/// rename the existing FileSymbols to something else e.g. TUSymbols?
+class SymbolsGroupedByFiles {
+public:

`FileSymbols` isn't actually that opinionated about the data it manages:
 - the keys ("filenames") just identify shards so we can tell whether a new 
shard is an addition or replaces an existing one.
 - the values (tu symbols/refs) are merged using pretty generic logic, we don't 
look at the details.

I think we should be able to use `FileSymbols` mostly unmodified, and store 
digests in parallel, probably in `BackgroundIndexer`. Is there a strong reason 
to store "main file" digests separately to header digests?

There are a couple of weak points:
 - The merging makes subtle assumptions: for symbols it picks one copy 
arbitrarily, assuming there are many copies (merging would be expensive) and 
they're mostly interchangeable duplicates. We could add a constructor or 
`buildIndex()` param to FileSymbols to control this, similar to the IndexType 
param. (For refs it assumes no duplicates and simply concatenates, which is 
also fine for our purposes).
 - `FileSymbols` locks internally to be threadsafe while keeping critical 
sections small. To keep digests in sync with FileSymbols contents we probably 
have to lock externally with a second mutex. This is a little ugly but not a 
real problem I think.



Comment at: clangd/index/IndexAction.h:31
+std::function RefsCallback,
+std::function FileDigestsCallback);
 

these parallel callbacks that always get called together are a bit ridiculous 
(my fault).
Can you add a FIXME to replace them with a single callback that passes a struct?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433



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


[clang-tools-extra] r344785 - [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-19 Thread Marek Kurdej via cfe-commits
Author: mkurdej
Date: Fri Oct 19 08:26:17 2018
New Revision: 344785

URL: http://llvm.org/viewvc/llvm-project?rev=344785&view=rev
Log:
[clang-tidy] Resolve readability-else-after-return false positive for constexpr 
if.

Summary:
It fixes the false positive when using constexpr if and where else cannot be 
removed:

Example:
```
  if constexpr (sizeof(int) > 4)
// ...
return /* ... */;
  else // This else cannot be removed.
// ...
return /* ... */;
```

Reviewers: alexfh, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: lebedev.ri, xazax.hun, cfe-commits

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

Added:

clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp?rev=344785&r1=344784&r2=344785&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Fri 
Oct 19 08:26:17 2018
@@ -1,57 +1,58 @@
-//===--- ElseAfterReturnCheck.cpp - 
clang-tidy-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "ElseAfterReturnCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Tooling/FixIt.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace readability {
-
-void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ControlFlowInterruptorMatcher =
-  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"),
- expr(ignoringImplicit(cxxThrowExpr().bind("throw");
-  Finder->addMatcher(
-  compoundStmt(forEach(
-  ifStmt(hasThen(stmt(
- anyOf(ControlFlowInterruptorMatcher,
-   compoundStmt(has(ControlFlowInterruptorMatcher),
- hasElse(stmt().bind("else")))
-  .bind("if"))),
-  this);
-}
-
-void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *If = Result.Nodes.getNodeAs("if");
-  SourceLocation ElseLoc = If->getElseLoc();
-  std::string ControlFlowInterruptor;
-  for (const auto *BindingName : {"return", "continue", "break", "throw"})
-if (Result.Nodes.getNodeAs(BindingName))
-  ControlFlowInterruptor = BindingName;
-
-  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
-   << ControlFlowInterruptor;
-  Diag << tooling::fixit::createRemoval(ElseLoc);
-
-  // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
-  // FIXME: Change clang-format to correctly un-indent the code.
-  if (const auto *CS = Result.Nodes.getNodeAs("else"))
-Diag << tooling::fixit::createRemoval(CS->getLBracLoc())
- << tooling::fixit::createRemoval(CS->getRBracLoc());
-}
-
-} // namespace readability
-} // namespace tidy
-} // namespace clang
+//===--- ElseAfterReturnCheck.cpp - 
clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ElseAfterReturnCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
+  Finder->addMatcher(
+  compoundStmt(forEach(
+  ifStmt(unless(isConstexpr()),
+ hasThen(stmt(
+ anyOf(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInterruptorMatcher),
+ hasElse(stmt().bind("else")))
+  .bind("if"))),
+  this);
+}
+
+void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *If = Result.Nodes.getNodeAs("if");
+  SourceLocation ElseLoc = I

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-19 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344785: [clang-tidy] Resolve readability-else-after-return 
false positive for constexpr… (authored by mkurdej, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53372?vs=170152&id=170201#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53372

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  test/clang-tidy/readability-else-after-return-if-constexpr.cpp

Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -1,57 +1,58 @@
-//===--- ElseAfterReturnCheck.cpp - clang-tidy-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "ElseAfterReturnCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Tooling/FixIt.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace readability {
-
-void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ControlFlowInterruptorMatcher =
-  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"),
- expr(ignoringImplicit(cxxThrowExpr().bind("throw");
-  Finder->addMatcher(
-  compoundStmt(forEach(
-  ifStmt(hasThen(stmt(
- anyOf(ControlFlowInterruptorMatcher,
-   compoundStmt(has(ControlFlowInterruptorMatcher),
- hasElse(stmt().bind("else")))
-  .bind("if"))),
-  this);
-}
-
-void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *If = Result.Nodes.getNodeAs("if");
-  SourceLocation ElseLoc = If->getElseLoc();
-  std::string ControlFlowInterruptor;
-  for (const auto *BindingName : {"return", "continue", "break", "throw"})
-if (Result.Nodes.getNodeAs(BindingName))
-  ControlFlowInterruptor = BindingName;
-
-  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
-   << ControlFlowInterruptor;
-  Diag << tooling::fixit::createRemoval(ElseLoc);
-
-  // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
-  // FIXME: Change clang-format to correctly un-indent the code.
-  if (const auto *CS = Result.Nodes.getNodeAs("else"))
-Diag << tooling::fixit::createRemoval(CS->getLBracLoc())
- << tooling::fixit::createRemoval(CS->getRBracLoc());
-}
-
-} // namespace readability
-} // namespace tidy
-} // namespace clang
+//===--- ElseAfterReturnCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ElseAfterReturnCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ControlFlowInterruptorMatcher =
+  stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
+  Finder->addMatcher(
+  compoundStmt(forEach(
+  ifStmt(unless(isConstexpr()),
+ hasThen(stmt(
+ anyOf(ControlFlowInterruptorMatcher,
+   compoundStmt(has(ControlFlowInterruptorMatcher),
+ hasElse(stmt().bind("else")))
+  .bind("if"))),
+  this);
+}
+
+void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *If = Result.Nodes.getNodeAs("if");
+  SourceLocation ElseLoc = If->getElseLoc();
+  std::string ControlFlowInterruptor;
+  for (const auto *BindingName : {"return", "continue", "break", "throw"})
+if (Result.Nodes.getNodeAs(BindingName))
+  ControlFlowInterruptor = BindingName;
+
+  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
+   << ControlFlowInterruptor;
+  Diag << tooling::fixit::createRemoval(ElseLoc);
+
+  // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
+  // FIXME: Change clang-format to correctly un-indent the code.
+  if (const auto *CS = Result.Nodes.getNodeAs("else")

r344786 - [Hexagon] Remove support for V4

2018-10-19 Thread Krzysztof Parzyszek via cfe-commits
Author: kparzysz
Date: Fri Oct 19 08:36:45 2018
New Revision: 344786

URL: http://llvm.org/viewvc/llvm-project?rev=344786&view=rev
Log:
[Hexagon] Remove support for V4

Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets/Hexagon.cpp
cfe/trunk/test/Driver/hexagon-toolchain-elf.c
cfe/trunk/test/Misc/target-invalid-cpu-note.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=344786&r1=344785&r2=344786&view=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Fri Oct 19 08:36:45 2018
@@ -2146,7 +2146,7 @@ Link stack frames through backchain on S
 
 .. option:: -mconsole
 
-.. option:: -mcpu=, -mv4 (equivalent to -mcpu=hexagonv4), -mv5 
(equivalent to -mcpu=hexagonv5), -mv55 (equivalent to -mcpu=hexagonv55), -mv60 
(equivalent to -mcpu=hexagonv60), -mv62 (equivalent to -mcpu=hexagonv62), -mv65 
(equivalent to -mcpu=hexagonv65)
+.. option:: -mcpu=, -mv5 (equivalent to -mcpu=hexagonv5), -mv55 
(equivalent to -mcpu=hexagonv55), -mv60 (equivalent to -mcpu=hexagonv60), -mv62 
(equivalent to -mcpu=hexagonv62), -mv65 (equivalent to -mcpu=hexagonv65)
 
 .. option:: -mcrc, -mno-crc
 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=344786&r1=344785&r2=344786&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Oct 19 08:36:45 2018
@@ -2661,8 +2661,6 @@ def _ : Joined<["--"], "">, Flags<[Unsup
 // Hexagon feature flags.
 def mieee_rnd_near : Flag<["-"], "mieee-rnd-near">,
   Group;
-def mv4 : Flag<["-"], "mv4">, Group,
-  Alias, AliasArgs<["hexagonv4"]>;
 def mv5 : Flag<["-"], "mv5">, Group, Alias,
   AliasArgs<["hexagonv5"]>;
 def mv55 : Flag<["-"], "mv55">, Group,

Modified: cfe/trunk/lib/Basic/Targets/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Hexagon.cpp?rev=344786&r1=344785&r2=344786&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Hexagon.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/Hexagon.cpp Fri Oct 19 08:36:45 2018
@@ -25,14 +25,7 @@ void HexagonTargetInfo::getTargetDefines
   Builder.defineMacro("__qdsp6__", "1");
   Builder.defineMacro("__hexagon__", "1");
 
-  if (CPU == "hexagonv4") {
-Builder.defineMacro("__HEXAGON_V4__");
-Builder.defineMacro("__HEXAGON_ARCH__", "4");
-if (Opts.HexagonQdsp6Compat) {
-  Builder.defineMacro("__QDSP6_V4__");
-  Builder.defineMacro("__QDSP6_ARCH__", "4");
-}
-  } else if (CPU == "hexagonv5") {
+  if (CPU == "hexagonv5") {
 Builder.defineMacro("__HEXAGON_V5__");
 Builder.defineMacro("__HEXAGON_ARCH__", "5");
 if (Opts.HexagonQdsp6Compat) {
@@ -150,9 +143,9 @@ struct CPUSuffix {
 };
 
 static constexpr CPUSuffix Suffixes[] = {
-{{"hexagonv4"}, {"4"}},   {{"hexagonv5"}, {"5"}},
-{{"hexagonv55"}, {"55"}}, {{"hexagonv60"}, {"60"}},
-{{"hexagonv62"}, {"62"}}, {{"hexagonv65"}, {"65"}},
+{{"hexagonv5"},  {"5"}},  {{"hexagonv55"}, {"55"}},
+{{"hexagonv60"}, {"60"}}, {{"hexagonv62"}, {"62"}},
+{{"hexagonv65"}, {"65"}},
 };
 
 const char *HexagonTargetInfo::getHexagonCPUSuffix(StringRef Name) {

Modified: cfe/trunk/test/Driver/hexagon-toolchain-elf.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-toolchain-elf.c?rev=344786&r1=344785&r2=344786&view=diff
==
--- cfe/trunk/test/Driver/hexagon-toolchain-elf.c (original)
+++ cfe/trunk/test/Driver/hexagon-toolchain-elf.c Fri Oct 19 08:36:45 2018
@@ -59,14 +59,6 @@
 // 
-
 // RUN: %clang -### -target hexagon-unknown-elf \
 // RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
-// RUN:   -mcpu=hexagonv4 \
-// RUN:   %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK020 %s
-// CHECK020: "-cc1" {{.*}} "-target-cpu" "hexagonv4"
-// CHECK020: 
{{hexagon-link|ld}}{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v4/crt0
-
-// RUN: %clang -### -target hexagon-unknown-elf \
-// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
 // RUN:   -mcpu=hexagonv5 \
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK021 %s

Modified: cfe/trunk/test/Misc/target-invalid-cpu-note.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/target-invalid-cpu-note.c?rev=344786&r1=344785&r2=344786&view=diff
==
--- cfe/trunk/test/Misc/target-invalid-cpu-n

[PATCH] D53404: [clangd] Set workspace root when initializing ClangdServer, disallow mutation.

2018-10-19 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344787: [clangd] Set workspace root when initializing 
ClangdServer, disallow mutation. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53404?vs=170098&id=170204#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53404

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/FindSymbolsTests.cpp

Index: unittests/clangd/FindSymbolsTests.cpp
===
--- unittests/clangd/FindSymbolsTests.cpp
+++ unittests/clangd/FindSymbolsTests.cpp
@@ -42,6 +42,7 @@
 
 ClangdServer::Options optsForTests() {
   auto ServerOpts = ClangdServer::optsForTest();
+  ServerOpts.WorkspaceRoot = testRoot();
   ServerOpts.BuildDynamicSymbolIndex = true;
   ServerOpts.URISchemes = {"unittest", "file"};
   return ServerOpts;
@@ -53,7 +54,6 @@
   : Server(CDB, FSProvider, DiagConsumer, optsForTests()) {
 // Make sure the test root directory is created.
 FSProvider.Files[testPath("unused")] = "";
-Server.setRootPath(testRoot());
   }
 
 protected:
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -86,6 +86,11 @@
 /// If set, use this index to augment code completion results.
 SymbolIndex *StaticIndex = nullptr;
 
+/// Clangd's workspace root. Relevant for "workspace" operations not bound
+/// to a particular file.
+/// FIXME: If not set, should use the current working directory.
+llvm::Optional WorkspaceRoot;
+
 /// The resource directory is used to find internal headers, overriding
 /// defaults and -resource-dir compiler flag).
 /// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
@@ -116,9 +121,6 @@
const FileSystemProvider &FSProvider,
DiagnosticsConsumer &DiagConsumer, const Options &Opts);
 
-  /// Set the root path of the workspace.
-  void setRootPath(PathRef RootPath);
-
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
   /// separate thread. When the parsing is complete, DiagConsumer passed in
@@ -255,8 +257,7 @@
   CachedCompletionFuzzyFindRequestByFile;
   mutable std::mutex CachedCompletionFuzzyFindRequestMutex;
 
-  // If set, this represents the workspace path.
-  llvm::Optional RootPath;
+  llvm::Optional WorkspaceRoot;
   std::shared_ptr PCHs;
   /// Used to serialize diagnostic callbacks.
   /// FIXME(ibiryukov): get rid of an extra map and put all version counters
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -109,6 +109,7 @@
  ? new FileIndex(Opts.URISchemes,
  Opts.HeavyweightDynamicSymbolIndex)
  : nullptr),
+  WorkspaceRoot(Opts.WorkspaceRoot),
   PCHs(std::make_shared()),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
@@ -131,18 +132,6 @@
 Index = nullptr;
 }
 
-void ClangdServer::setRootPath(PathRef RootPath) {
-  auto FS = FSProvider.getFileSystem();
-  auto Status = FS->status(RootPath);
-  if (!Status)
-elog("Failed to get status for RootPath {0}: {1}", RootPath,
- Status.getError().message());
-  else if (Status->isDirectory())
-this->RootPath = RootPath;
-  else
-elog("The provided RootPath {0} is not a directory.", RootPath);
-}
-
 void ClangdServer::addDocument(PathRef File, StringRef Contents,
WantDiagnostics WantDiags) {
   DocVersion Version = ++InternalVersion[File];
@@ -495,7 +484,7 @@
 void ClangdServer::workspaceSymbols(
 StringRef Query, int Limit, Callback> CB) {
   CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
- RootPath ? *RootPath : ""));
+ WorkspaceRoot.getValueOr("")));
 }
 
 void ClangdServer::documentSymbols(
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -261,6 +261,10 @@
 
 void ClangdLSPServer::onInitialize(const InitializeParams &Params,
Callback Reply) {
+  if (Params.rootUri && *Params.rootUri)
+ClangdServerOpts.WorkspaceRoot = Params.rootUri->file();
+  else if (Params.rootPath && !Params.rootPath->empty())
+ClangdServerOpts.WorkspaceRoot = *Params.rootPath;
   if (Server)
 return Reply(make_error("server already initialized",
   ErrorCode::InvalidRequest));
@@ -277,11 +281,6 

[clang-tools-extra] r344787 - [clangd] Set workspace root when initializing ClangdServer, disallow mutation.

2018-10-19 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Fri Oct 19 08:42:23 2018
New Revision: 344787

URL: http://llvm.org/viewvc/llvm-project?rev=344787&view=rev
Log:
[clangd] Set workspace root when initializing ClangdServer, disallow mutation.

Summary:
Rename instance variable to WorkspaceRoot to match what we call it internally.

Add fixme to set it automatically. Don't do it yet, clients have assumptions
that the constructor won't access the FS.

Don't second-guess the provided root.

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344787&r1=344786&r2=344787&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Oct 19 08:42:23 2018
@@ -261,6 +261,10 @@ void ClangdLSPServer::reply(llvm::json::
 
 void ClangdLSPServer::onInitialize(const InitializeParams &Params,
Callback Reply) {
+  if (Params.rootUri && *Params.rootUri)
+ClangdServerOpts.WorkspaceRoot = Params.rootUri->file();
+  else if (Params.rootPath && !Params.rootPath->empty())
+ClangdServerOpts.WorkspaceRoot = *Params.rootPath;
   if (Server)
 return Reply(make_error("server already initialized",
   ErrorCode::InvalidRequest));
@@ -277,11 +281,6 @@ void ClangdLSPServer::onInitialize(const
 applyConfiguration(Opts.ParamsChange);
   }
 
-  if (Params.rootUri && *Params.rootUri)
-Server->setRootPath(Params.rootUri->file());
-  else if (Params.rootPath && !Params.rootPath->empty())
-Server->setRootPath(*Params.rootPath);
-
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=344787&r1=344786&r2=344787&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Oct 19 08:42:23 2018
@@ -109,6 +109,7 @@ ClangdServer::ClangdServer(const GlobalC
  ? new FileIndex(Opts.URISchemes,
  Opts.HeavyweightDynamicSymbolIndex)
  : nullptr),
+  WorkspaceRoot(Opts.WorkspaceRoot),
   PCHs(std::make_shared()),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
@@ -131,18 +132,6 @@ ClangdServer::ClangdServer(const GlobalC
 Index = nullptr;
 }
 
-void ClangdServer::setRootPath(PathRef RootPath) {
-  auto FS = FSProvider.getFileSystem();
-  auto Status = FS->status(RootPath);
-  if (!Status)
-elog("Failed to get status for RootPath {0}: {1}", RootPath,
- Status.getError().message());
-  else if (Status->isDirectory())
-this->RootPath = RootPath;
-  else
-elog("The provided RootPath {0} is not a directory.", RootPath);
-}
-
 void ClangdServer::addDocument(PathRef File, StringRef Contents,
WantDiagnostics WantDiags) {
   DocVersion Version = ++InternalVersion[File];
@@ -495,7 +484,7 @@ void ClangdServer::onFileEvent(const Did
 void ClangdServer::workspaceSymbols(
 StringRef Query, int Limit, Callback> CB) {
   CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
- RootPath ? *RootPath : ""));
+ WorkspaceRoot.getValueOr("")));
 }
 
 void ClangdServer::documentSymbols(

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=344787&r1=344786&r2=344787&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Oct 19 08:42:23 2018
@@ -86,6 +86,11 @@ public:
 /// If set, use this index to augment code completion results.
 SymbolIndex *StaticIndex = nullptr;
 
+/// Clangd's workspace root. Relevant for "workspace" operations not bound
+/// to a particular file.
+/// FIXME: If not set, should use the current working directory.
+llvm::Optional Wo

[PATCH] D53434: Java annotation declaration being handled correctly

2018-10-19 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

Thanks for the fix! Do you happen to know what had regressed this?


Repository:
  rC Clang

https://reviews.llvm.org/D53434



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


[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

2018-10-19 Thread Taewook Oh via Phabricator via cfe-commits
twoh added a comment.

@joerg Sorry but I'm not sure if I understand your question. This doesn't 
pretend to honor source code order, but makes linker to place "hot" functions 
under .text.hot section (There's no guarantee of ordering between functions 
inside .hot.text section) while "cold" functions under .text.unlikely section. 
This is purely for performance.


Repository:
  rC Clang

https://reviews.llvm.org/D34796



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


[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

2018-10-19 Thread Taewook Oh via Phabricator via cfe-commits
twoh updated this revision to Diff 170205.
twoh added a comment.

Remove conflict line.


Repository:
  rC Clang

https://reviews.llvm.org/D34796

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/Inputs/freorder-functions.prof
  test/CodeGen/freorder-functions.c
  test/Driver/function-sections.c

Index: test/Driver/function-sections.c
===
--- test/Driver/function-sections.c
+++ test/Driver/function-sections.c
@@ -6,6 +6,8 @@
 // CHECK-NODS-NOT: -fdata-sections
 // CHECK-US-NOT: -fno-unique-section-names
 // CHECK-NOUS: -fno-unique-section-names
+// CHECK-RF-NOT: -fno-reorder-functions
+// CHECK-NORF: -fno-reorder-functions
 
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1   \
 // RUN: -target i386-unknown-linux \
@@ -72,3 +74,13 @@
 // RUN: -target i386-unknown-linux \
 // RUN: -fno-unique-section-names \
 // RUN:   | FileCheck --check-prefix=CHECK-NOUS %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\
+// RUN: -target i386-unknown-linux \
+// RUN: -freorder-functions \
+// RUN:   | FileCheck --check-prefix=CHECK-RF %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\
+// RUN: -target i386-unknown-linux \
+// RUN: -fno-reorder-functions \
+// RUN:   | FileCheck --check-prefix=CHECK-NORF %s
Index: test/CodeGen/freorder-functions.c
===
--- /dev/null
+++ test/CodeGen/freorder-functions.c
@@ -0,0 +1,22 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -fno-reorder-functions -o - < %s | FileCheck --check-prefix=CHECK-NOPREFIX %s
+
+// opt tool option precedes driver option.
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -fno-reorder-functions -mllvm -profile-guided-section-prefix=true -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -freorder-functions -mllvm -profile-guided-section-prefix=false -o - < %s | FileCheck --check-prefix=CHECK-NOPREFIX %s
+
+void hot_func() {
+  return;
+}
+
+void cold_func() {
+  hot_func();
+  return;
+}
+
+// CHECK: .section .text.hot.hot_func,"ax",@progbits
+// CHECK: .section .text.unlikely.cold_func,"ax",@progbits
+// CHECK-NOPREFIX: .section .text.hot_func,"ax",@progbits
+// CHECK-NOPREFIX: .section .text.cold_func,"ax",@progbits
Index: test/CodeGen/Inputs/freorder-functions.prof
===
--- /dev/null
+++ test/CodeGen/Inputs/freorder-functions.prof
@@ -0,0 +1,5 @@
+hot_func:1000:0
+ 1: 0
+cold_func:0:0
+ 1: 1
+ 2: 1
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -756,6 +756,8 @@
   Args.hasFlag(OPT_fstack_size_section, OPT_fno_stack_size_section, false);
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
  OPT_fno_unique_section_names, true);
+  Opts.ReorderFunctions =
+  Args.hasFlag(OPT_freorder_functions, OPT_fno_reorder_functions, true);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3891,6 +3891,10 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
+  if (!Args.hasFlag(options::OPT_freorder_functions,
+options::OPT_fno_reorder_functions, true))
+CmdArgs.push_back("-fno-reorder-functions");
+
   if (auto *A = Args.getLastArg(
   options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
@@ -3929,6 +3933,82 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_working_directory);
 
+  bool ARCMTEnabled = false;
+  if (!Args.hasArg(options::OPT_fno_objc_arc, options::OPT_fobjc_arc)) {
+if (const Arg *A = Args.getLastArg(options::OPT_ccc_arcmt_check,
+   options::OPT_ccc_arcmt_modify,
+   options::OPT_ccc_arcmt_migrate)) {
+  ARCMTEnabled = true;
+  switch (A->getOption().getID()) {
+  default:
+llvm_unreachable("missed a case");
+   

[PATCH] D53439: [clangd] Remove caching of compilation database commands.

2018-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added reviewers: ioeric, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.

The CDB implementations used in open-source code are fast, and our private
slow CDB will soon do the relevant caching itself.

Simplifying the GlobalCDB layer in clangd is important to get auto-index
implemented at the right layer.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53439

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h

Index: clangd/GlobalCompilationDatabase.h
===
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -86,33 +86,6 @@
   llvm::Optional CompileCommandsDir;
 };
 
-/// A wrapper around GlobalCompilationDatabase that caches the compile commands.
-/// Note that only results of getCompileCommand are cached.
-class CachingCompilationDb : public GlobalCompilationDatabase {
-public:
-  explicit CachingCompilationDb(const GlobalCompilationDatabase &InnerCDB);
-
-  /// Gets compile command for \p File from cache or CDB if it's not in the
-  /// cache.
-  llvm::Optional
-  getCompileCommand(PathRef File) const override;
-
-  /// Forwards to the inner CDB. Results of this function are not cached.
-  tooling::CompileCommand getFallbackCommand(PathRef File) const override;
-
-  /// Removes an entry for \p File if it's present in the cache.
-  void invalidate(PathRef File);
-
-  /// Removes all cached compile commands.
-  void clear();
-
-private:
-  const GlobalCompilationDatabase &InnerCDB;
-  mutable std::mutex Mut;
-  mutable llvm::StringMap>
-  Cached; /* GUARDED_BY(Mut) */
-};
-
 /// Gets compile args from an in-memory mapping based on a filepath. Typically
 /// used by clients who provide the compile commands themselves.
 class InMemoryCompilationDb : public GlobalCompilationDatabase {
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -117,39 +117,6 @@
   return nullptr;
 }
 
-CachingCompilationDb::CachingCompilationDb(
-const GlobalCompilationDatabase &InnerCDB)
-: InnerCDB(InnerCDB) {}
-
-llvm::Optional
-CachingCompilationDb::getCompileCommand(PathRef File) const {
-  std::unique_lock Lock(Mut);
-  auto It = Cached.find(File);
-  if (It != Cached.end())
-return It->second;
-
-  Lock.unlock();
-  llvm::Optional Command =
-  InnerCDB.getCompileCommand(File);
-  Lock.lock();
-  return Cached.try_emplace(File, std::move(Command)).first->getValue();
-}
-
-tooling::CompileCommand
-CachingCompilationDb::getFallbackCommand(PathRef File) const {
-  return InnerCDB.getFallbackCommand(File);
-}
-
-void CachingCompilationDb::invalidate(PathRef File) {
-  std::unique_lock Lock(Mut);
-  Cached.erase(File);
-}
-
-void CachingCompilationDb::clear() {
-  std::unique_lock Lock(Mut);
-  Cached.clear();
-}
-
 llvm::Optional
 InMemoryCompilationDb::getCompileCommand(PathRef File) const {
   std::lock_guard Lock(Mutex);
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -135,21 +135,17 @@
 
 /// Returns a CDB that should be used to get compile commands for the
 /// current instance of ClangdLSPServer.
-GlobalCompilationDatabase &getCDB();
+GlobalCompilationDatabase &getCDB() { return *CDB; }
 
   private:
 CompilationDB(std::unique_ptr CDB,
-  std::unique_ptr CachingCDB,
   bool IsDirectoryBased)
-: CDB(std::move(CDB)), CachingCDB(std::move(CachingCDB)),
-  IsDirectoryBased(IsDirectoryBased) {}
+: CDB(std::move(CDB)), IsDirectoryBased(IsDirectoryBased) {}
 
 // if IsDirectoryBased is true, an instance of InMemoryCDB.
 // If IsDirectoryBased is false, an instance of DirectoryBasedCDB.
 // unique_ptr CDB;
 std::unique_ptr CDB;
-// Non-null only for directory-based CDB
-std::unique_ptr CachingCDB;
 bool IsDirectoryBased;
   };
 
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -787,25 +787,22 @@
 }
 
 ClangdLSPServer::CompilationDB ClangdLSPServer::CompilationDB::makeInMemory() {
-  return CompilationDB(llvm::make_unique(), nullptr,
+  return CompilationDB(llvm::make_unique(),
/*IsDirectoryBased=*/false);
 }
 
 ClangdLSPServer::CompilationDB
 ClangdLSPServer::CompilationDB::makeDirectoryBased(
 llvm::Optional CompileCommandsDir) {
   auto CDB = llvm::make_unique(
   std::move(CompileCommandsDir));
-  auto CachingCDB = llvm::make_unique(*CDB);
-  return CompilationDB(std::move(CDB), std::move(CachingCDB),
+  return CompilationDB(std::

[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

2018-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Nice savings!

My initial thought is that it seems weird to only do it for this one string - 
the readers look different, the serialization code has different logic etc for 
the different flavors of strings.
The struct certainly looks nicer to my eyes.

This may be the right tradeoff, but I'd like to play with/think about other 
options for API here. One is to define a class like `NullTerminatedStringRef` 
(with a better name) that wraps a char* only, but otherwise behaves as much 
like stringref as we can manage. Happy for you to try some of these out or to 
try them myself.

I think this is worth a bit of thought before committing because as shown in 
this patch, these APIs have quite a lot of uses!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53427



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


r344789 - Java annotation declaration being handled correctly

2018-10-19 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Fri Oct 19 09:19:52 2018
New Revision: 344789

URL: http://llvm.org/viewvc/llvm-project?rev=344789&view=rev
Log:
Java annotation declaration being handled correctly

Previously, Java annotation declarations (@interface AnnotationName) were being
handled as ObjC interfaces. This caused the brace formatting to mess up, so
that when you had a class with an interface defined in it, it would indent the
final brace of the class.

It used to format this class like so:

  class A {
@interface B {}
}

But will now just skip the @interface and format it like so:

  class A {
@interface B {}
  }

Patch by Sam Maier!

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

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestJava.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=344789&r1=344788&r2=344789&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Fri Oct 19 09:19:52 2018
@@ -1130,6 +1130,10 @@ void UnwrappedLineParser::parseStructura
 nextToken();
 parseBracedList();
 break;
+  } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
   }
   switch (FormatTok->Tok.getObjCKeywordID()) {
   case tok::objc_public:

Modified: cfe/trunk/unittests/Format/FormatTestJava.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJava.cpp?rev=344789&r1=344788&r2=344789&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJava.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp Fri Oct 19 09:19:52 2018
@@ -155,6 +155,15 @@ TEST_F(FormatTestJava, ClassDeclarations
"  void doStuff(int theStuff);\n"
"  void doMoreStuff(int moreStuff);\n"
"}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {\n"
+   "int stuff;\n"
+   "void doMoreStuff(int moreStuff);\n"
+   "  }\n"
+   "}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {}\n"
+   "}");
 }
 
 TEST_F(FormatTestJava, AnonymousClasses) {


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


[PATCH] D53434: Java annotation declaration being handled correctly

2018-10-19 Thread Hans Wennborg via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344789: Java annotation declaration being handled correctly 
(authored by hans, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53434?vs=170197&id=170210#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53434

Files:
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTestJava.cpp


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1130,6 +1130,10 @@
 nextToken();
 parseBracedList();
 break;
+  } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
   }
   switch (FormatTok->Tok.getObjCKeywordID()) {
   case tok::objc_public:
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -155,6 +155,15 @@
"  void doStuff(int theStuff);\n"
"  void doMoreStuff(int moreStuff);\n"
"}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {\n"
+   "int stuff;\n"
+   "void doMoreStuff(int moreStuff);\n"
+   "  }\n"
+   "}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {}\n"
+   "}");
 }
 
 TEST_F(FormatTestJava, AnonymousClasses) {


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1130,6 +1130,10 @@
 nextToken();
 parseBracedList();
 break;
+  } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
   }
   switch (FormatTok->Tok.getObjCKeywordID()) {
   case tok::objc_public:
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -155,6 +155,15 @@
"  void doStuff(int theStuff);\n"
"  void doMoreStuff(int moreStuff);\n"
"}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {\n"
+   "int stuff;\n"
+   "void doMoreStuff(int moreStuff);\n"
+   "  }\n"
+   "}");
+  verifyFormat("class A {\n"
+   "  public @interface SomeInterface {}\n"
+   "}");
 }
 
 TEST_F(FormatTestJava, AnonymousClasses) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

A few more comments, mostly marking places of unintentional changes that we 
need to revert.
Hope it's not going past the point where the number of comments are not useful.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8616
   "use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
-def warn_opencl_generic_address_space_arg : Warning<
-  "passing non-generic address space pointer to %0"

This looks unintentional. Should we revert?



Comment at: lib/Sema/SemaChecking.cpp:852
 
-  if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
-S.Diag(Call->getArg(0)->getBeginLoc(),

aaron.ballman wrote:
> kadircet wrote:
> > aaron.ballman wrote:
> > > I'm not certain I understand why this code has been removed?
> > It shouldn't have been, tried rebasing but it didn't go away. I think it 
> > was deleted at some point by a different change and somehow ended up 
> > showing in here as well. (Tried to revert, got an error stating 
> > warn_opencl_generic_address_space_arg doesn't exist)
> That's truly strange. I bring it up because these sort of changes make it 
> harder for reviewers to test changes locally by applying the patch themselves.
+1, we definitely need to revert this, happy to help figure out why this 
happened.



Comment at: lib/Sema/SemaTemplateDeduction.cpp:4348
   TLB.reserve(TL.getFullDataSize());
-  return TransformType(TLB, TL);
+  QualType Result = TransformType(TLB, TL);
+  return TLB.getTypeSourceInfo(getSema().Context, Result);

Maybe remove the intermediate variable?



Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:354
   bool VisitTypeLoc(TypeLoc Loc) {
+if(Loc.getTypeLocClass() == TypeLoc::Auto)
+  return true;

Why do we need this change? Are we fixing some particular regressions?
A comment would be useful.



Comment at: test/SemaOpenCL/to_addr_builtin.cl:2
 // RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
 

Unrelated change.



Comment at: test/SemaOpenCL/to_addr_builtin.cl:48
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes 
address space of pointer}}
-  // expected-warning@-5{{passing non-generic address space pointer to 
to_global may cause dynamic conversion affecting performance}}
 #endif

Unrelated change



Comment at: test/SemaOpenCL/to_addr_builtin.cl:122
   // expected-warning@-4{{incompatible pointer types initializing '__global 
char *' with an expression of type '__global int *'}}
-  // expected-warning@-5{{passing non-generic address space pointer to 
to_global may cause dynamic conversion affecting performance}}
 #endif

Another unrelated change


Repository:
  rC Clang

https://reviews.llvm.org/D52301



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


[PATCH] D53441: [ms] Prevent explicit constructor name lookup if scope is missing

2018-10-19 Thread Will Wilson via Phabricator via cfe-commits
lantictac created this revision.
lantictac added a reviewer: rsmith.
lantictac added a project: clang.

MicrosoftExt allows explicit constructor calls. Prevent lookup of constructor 
name unless the name has explicit scope.
This avoids a compile-time crash due to confusing a member access for a 
constructor name.

Test case included. All tests pass.


Repository:
  rC Clang

https://reviews.llvm.org/D53441

Files:
  lib/Parse/ParseExpr.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp


Index: test/SemaCXX/MicrosoftCompatibility.cpp
===
--- test/SemaCXX/MicrosoftCompatibility.cpp
+++ test/SemaCXX/MicrosoftCompatibility.cpp
@@ -302,3 +302,23 @@
   void *a2 = &function_prototype; // expected-warning {{implicit conversion 
between pointer-to-function and pointer-to-object is a Microsoft extension}}
   void *a3 = function_ptr;// expected-warning {{implicit conversion 
between pointer-to-function and pointer-to-object is a Microsoft extension}}
 }
+
+namespace member_lookup {
+
+template
+struct ConfuseLookup {
+  T* m_val;
+  struct m_val {
+static size_t ms_test;
+  };
+};
+
+// Microsoft mode allows explicit constructor calls
+// This could confuse name lookup in cases such as this
+template
+size_t ConfuseLookup::m_val::ms_test
+  = size_t(&(char&)(reinterpret_cast*>(0)->m_val));
+
+void instantiate() { ConfuseLookup::m_val::ms_test = 1; }
+}
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1809,7 +1809,8 @@
 /*EnteringContext=*/false,
 /*AllowDestructorName=*/true,
 /*AllowConstructorName=*/
-  getLangOpts().MicrosoftExt,
+getLangOpts().MicrosoftExt &&
+SS.isNotEmpty(),
 /*AllowDeductionGuide=*/false,
 ObjectType, &TemplateKWLoc, Name)) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);


Index: test/SemaCXX/MicrosoftCompatibility.cpp
===
--- test/SemaCXX/MicrosoftCompatibility.cpp
+++ test/SemaCXX/MicrosoftCompatibility.cpp
@@ -302,3 +302,23 @@
   void *a2 = &function_prototype; // expected-warning {{implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension}}
   void *a3 = function_ptr;// expected-warning {{implicit conversion between pointer-to-function and pointer-to-object is a Microsoft extension}}
 }
+
+namespace member_lookup {
+
+template
+struct ConfuseLookup {
+  T* m_val;
+  struct m_val {
+static size_t ms_test;
+  };
+};
+
+// Microsoft mode allows explicit constructor calls
+// This could confuse name lookup in cases such as this
+template
+size_t ConfuseLookup::m_val::ms_test
+  = size_t(&(char&)(reinterpret_cast*>(0)->m_val));
+
+void instantiate() { ConfuseLookup::m_val::ms_test = 1; }
+}
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1809,7 +1809,8 @@
 /*EnteringContext=*/false,
 /*AllowDestructorName=*/true,
 /*AllowConstructorName=*/
-  getLangOpts().MicrosoftExt,
+getLangOpts().MicrosoftExt &&
+SS.isNotEmpty(),
 /*AllowDeductionGuide=*/false,
 ObjectType, &TemplateKWLoc, Name)) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53369: [CodeComplete] Fix accessibility of protected members when accessing members implicitly.

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

LGTM, thanks!
Would be super-nice if didn't have to rewrite this in code completion


Repository:
  rC Clang

https://reviews.llvm.org/D53369



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


[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

2018-10-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 170215.
kadircet added a comment.

- Keep only relavant changes and rebase


Repository:
  rC Clang

https://reviews.llvm.org/D52301

Files:
  include/clang/AST/PrettyPrinter.h
  include/clang/Sema/Sema.h
  lib/AST/TypePrinter.cpp
  lib/Frontend/ASTConsumers.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp

Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -4340,19 +4340,20 @@
   return E;
 }
 
-QualType Apply(TypeLoc TL) {
+TypeSourceInfo* Apply(TypeLoc TL) {
   // Create some scratch storage for the transformed type locations.
   // FIXME: We're just going to throw this information away. Don't build it.
   TypeLocBuilder TLB;
   TLB.reserve(TL.getFullDataSize());
-  return TransformType(TLB, TL);
+  QualType Result = TransformType(TLB, TL);
+  return TLB.getTypeSourceInfo(getSema().Context, Result);
 }
   };
 
 } // namespace
 
 Sema::DeduceAutoResult
-Sema::DeduceAutoType(TypeSourceInfo *Type, Expr *&Init, QualType &Result,
+Sema::DeduceAutoType(TypeSourceInfo *Type, Expr *&Init, TypeSourceInfo *&Result,
  Optional DependentDeductionDepth) {
   return DeduceAutoType(Type->getTypeLoc(), Init, Result,
 DependentDeductionDepth);
@@ -4398,7 +4399,7 @@
 ///'auto' template parameters. The value specified is the template
 ///parameter depth at which we should perform 'auto' deduction.
 Sema::DeduceAutoResult
-Sema::DeduceAutoType(TypeLoc Type, Expr *&Init, QualType &Result,
+Sema::DeduceAutoType(TypeLoc Type, Expr *&Init, TypeSourceInfo *&Result,
  Optional DependentDeductionDepth) {
   if (Init->getType()->isNonOverloadPlaceholderType()) {
 ExprResult NonPlaceholder = CheckPlaceholderExpr(Init);
@@ -4410,7 +4411,8 @@
   if (!DependentDeductionDepth &&
   (Type.getType()->isDependentType() || Init->isTypeDependent())) {
 Result = SubstituteDeducedTypeTransform(*this, QualType()).Apply(Type);
-assert(!Result.isNull() && "substituting DependentTy can't fail");
+assert(Result && !Result->getType().isNull() &&
+   "substituting DependentTy can't fail");
 return DAR_Succeeded;
   }
 
@@ -4433,7 +4435,7 @@
   // FIXME: Support a non-canonical deduced type for 'auto'.
   Deduced = Context.getCanonicalType(Deduced);
   Result = SubstituteDeducedTypeTransform(*this, Deduced).Apply(Type);
-  if (Result.isNull())
+  if (!Result || Result->getType().isNull())
 return DAR_FailedAlreadyDiagnosed;
   return DAR_Succeeded;
 } else if (!getLangOpts().CPlusPlus) {
@@ -4456,10 +4458,10 @@
   FixedSizeTemplateParameterListStorage<1, false> TemplateParamsSt(
   Loc, Loc, TemplParamPtr, Loc, nullptr);
 
-  QualType FuncParam =
-  SubstituteDeducedTypeTransform(*this, TemplArg, /*UseTypeSugar*/false)
+  TypeSourceInfo *FuncParamTSI =
+  SubstituteDeducedTypeTransform(*this, TemplArg, /*UseTypeSugar*/ false)
   .Apply(Type);
-  assert(!FuncParam.isNull() &&
+  assert(!FuncParamTSI->getType().isNull() &&
  "substituting template parameter for 'auto' failed");
 
   // Deduce type of TemplParam in Func(Init)
@@ -4474,7 +4476,8 @@
  ArrayRef Ranges) -> DeduceAutoResult {
 if (Init->isTypeDependent()) {
   Result = SubstituteDeducedTypeTransform(*this, QualType()).Apply(Type);
-  assert(!Result.isNull() && "substituting DependentTy can't fail");
+  assert(Result && !Result->getType().isNull() &&
+ "substituting DependentTy can't fail");
   return DAR_Succeeded;
 }
 if (diagnoseAutoDeductionFailure(*this, TDK, Info, Ranges))
@@ -4514,8 +4517,9 @@
 }
 
 if (auto TDK = DeduceTemplateArgumentsFromCallArgument(
-*this, TemplateParamsSt.get(), 0, FuncParam, Init, Info, Deduced,
-OriginalCallArgs, /*Decomposed*/ false, /*ArgIdx*/ 0, /*TDF*/ 0))
+*this, TemplateParamsSt.get(), 0, FuncParamTSI->getType(), Init,
+Info, Deduced, OriginalCallArgs, /*Decomposed*/ false, /*ArgIdx*/ 0,
+/*TDF*/ 0))
   return DeductionFailed(TDK, {});
   }
 
@@ -4532,18 +4536,18 @@
   }
 
   Result = SubstituteDeducedTypeTransform(*this, DeducedType).Apply(Type);
-  if (Result.isNull())
+  if (!Result || Result->getType().isNull())
 return DAR_FailedAlreadyDiagnosed;
 
   // Check that the deduced argument type is compatible with the original
   // argument type per C++ [temp.deduct.call]p4.
-  QualType DeducedA = InitList ? Deduced[0].getAsType() : Result;
+  QualType DeducedA = InitList ? Deduced[0].getAsType() : Result->getType();
   for (const OriginalCallArg &OriginalArg : OriginalCallArgs) {
 assert((bool

[PATCH] D52654: [OpenCL][NFC] Unify ZeroToOCL* cast types

2018-10-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks!


https://reviews.llvm.org/D52654



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


[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.

2018-10-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea created this revision.
gtbercea added reviewers: ABataev, caomhin.
Herald added subscribers: cfe-commits, guansong, jholewinski.

This patch enables the choosing of the default schedule for parallel for loops 
even in non-SPMD cases.


Repository:
  rC Clang

https://reviews.llvm.org/D53443

Files:
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CGStmtOpenMP.cpp


Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -2313,7 +2313,7 @@
   } else {
 // Default behaviour for schedule clause.
 CGM.getOpenMPRuntime().getDefaultScheduleAndChunk(
-*this, S, ScheduleKind.Schedule, Chunk);
+*this, S, ScheduleKind, Chunk);
   }
   const unsigned IVSize = getContext().getTypeSize(IVExpr->getType());
   const bool IVSigned = 
IVExpr->getType()->hasSignedIntegerRepresentation();
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -347,7 +347,7 @@
 
   /// Choose a default value for the schedule clause.
   void getDefaultScheduleAndChunk(CodeGenFunction &CGF,
-  const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind,
+  const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind,
   llvm::Value *&Chunk) const override;
 
 private:
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4238,16 +4238,17 @@
 Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF),
 CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0),
 S.getIterationVariable()->getType(), S.getBeginLoc());
+return;
   }
+  CGOpenMPRuntime::getDefaultDistScheduleAndChunk(
+  CGF, S, ScheduleKind, Chunk);
 }
 
 void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk(
 CodeGenFunction &CGF, const OMPLoopDirective &S,
-OpenMPScheduleClauseKind &ScheduleKind,
+OpenMPScheduleTy &ScheduleKind,
 llvm::Value *&Chunk) const {
-  if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) {
-ScheduleKind = OMPC_SCHEDULE_static;
-Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
-S.getIterationVariable()->getType()), 1);
-  }
+  ScheduleKind.Schedule = OMPC_SCHEDULE_static;
+  Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
+  S.getIterationVariable()->getType()), 1);
 }
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -1505,7 +1505,7 @@
   /// Choose default schedule type and chunk value for the
   /// schedule clause.
   virtual void getDefaultScheduleAndChunk(CodeGenFunction &CGF,
-  const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind,
+  const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind,
   llvm::Value *&Chunk) const {}
 
   /// Emits call of the outlined function with the provided arguments,


Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -2313,7 +2313,7 @@
   } else {
 // Default behaviour for schedule clause.
 CGM.getOpenMPRuntime().getDefaultScheduleAndChunk(
-*this, S, ScheduleKind.Schedule, Chunk);
+*this, S, ScheduleKind, Chunk);
   }
   const unsigned IVSize = getContext().getTypeSize(IVExpr->getType());
   const bool IVSigned = IVExpr->getType()->hasSignedIntegerRepresentation();
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -347,7 +347,7 @@
 
   /// Choose a default value for the schedule clause.
   void getDefaultScheduleAndChunk(CodeGenFunction &CGF,
-  const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind,
+  const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind,
   llvm::Value *&Chunk) const override;
 
 private:
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4238,16 +4238,17 @@
 Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF),
 CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0),
 S.getIterationVariable()->getType(), S.getBeginLoc());
+return;
   }
+  CGOpenMPRuntime::getDefaultDistScheduleAndChunk(
+  CGF, S, ScheduleKind, Chunk);
 }
 
 void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk(
 CodeGenFunction &CGF, con

[PATCH] D53200: [OpenCL] Fix serialization of OpenCLExtensionDecls

2018-10-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision.
Anastasia added inline comments.
This revision is now accepted and ready to land.



Comment at: test/Headers/opencl-pragma-extension-begin.cl:1
+// RUN: rm -rf %t
+// RUN: mkdir -p %t

I think the tests in this folder are for standard includes only but you are 
testing custom include file here.

Could this be integrated into test/SemaOpenCL/extension-begin.cl? Or if else 
you could just move to that folder (it might be better to append module to the 
name in that case).



Comment at: test/Headers/opencl-pragma-extension-begin.cl:11
+
+void __kernel test(__global int *data) {
+  *data = 10;

May be it makes sense to test the added extension?


https://reviews.llvm.org/D53200



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


r344795 - [Driver] Reland: Default Android toolchains to libc++.

2018-10-19 Thread Dan Albert via cfe-commits
Author: danalbert
Date: Fri Oct 19 11:06:02 2018
New Revision: 344795

URL: http://llvm.org/viewvc/llvm-project?rev=344795&view=rev
Log:
[Driver] Reland: Default Android toolchains to libc++.

The sanitizer builder that was broken by this should now be fixed.

Original review was https://reviews.llvm.org/D53109

Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.h
cfe/trunk/test/Driver/android-ndk-standalone.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=344795&r1=344794&r2=344795&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Fri Oct 19 11:06:02 2018
@@ -443,6 +443,12 @@ Linux::Linux(const Driver &D, const llvm
   addPathIfExists(D, SysRoot + "/usr/lib", Paths);
 }
 
+ToolChain::CXXStdlibType Linux::GetDefaultCXXStdlibType() const {
+  if (getTriple().isAndroid())
+return ToolChain::CST_Libcxx;
+  return ToolChain::CST_Libstdcxx;
+}
+
 bool Linux::HasNativeLLVMSupport() const { return true; }
 
 Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); }

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.h?rev=344795&r1=344794&r2=344795&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.h Fri Oct 19 11:06:02 2018
@@ -37,6 +37,7 @@ public:
   llvm::opt::ArgStringList &CC1Args) const override;
   void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
   bool isPIEDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;

Modified: cfe/trunk/test/Driver/android-ndk-standalone.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-ndk-standalone.cpp?rev=344795&r1=344794&r2=344795&view=diff
==
--- cfe/trunk/test/Driver/android-ndk-standalone.cpp (original)
+++ cfe/trunk/test/Driver/android-ndk-standalone.cpp Fri Oct 19 11:06:02 2018
@@ -2,21 +2,13 @@
 // toolchain.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -target arm-linux-androideabi21 \
 // RUN: -B%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN:   | FileCheck  %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/backward"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/v1"
 // CHECK: "-internal-isystem" "{{.*}}/sysroot/usr/local/include"
 // CHECK: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
 // CHECK: "-internal-externc-isystem" 
"{{.*}}/sysroot/usr/include/arm-linux-androideabi"
@@ -49,21 +41,47 @@
 // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target armv7a-none-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
+// RUN:   | FileCheck --check-prefix=CHECK-STDCXX %s
+// CHECK-STDCXX: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-STDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9"
+// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
+// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
+// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
+// CHECK-STDCXX: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-10-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

@echristo

> As far as the standard text here, IMO it was just there in case people didn't 
> have an objcopy around or don't want to split it. I'm not sure why we would 
> want the ability.

I think others have mentioned - but with distributed build it might be easier 
to keep it as a single file because your build system already knows how to ship 
around those files.

> That said, if we do I'd rather have it as dwarf5 without split-dwarf as an 
> option rather than a -gsingle-file-split-dwarf option.

Any suggestions on the name?


https://reviews.llvm.org/D52296



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


[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension

2018-10-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D51402



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


[PATCH] D53334: [Frontend] Show diagnostics on prebuilt module configuration mismatch too

2018-10-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Don't know enough about the code to have an opinion on the fix - but in any 
case this would need a test case, if possible


Repository:
  rC Clang

https://reviews.llvm.org/D53334



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


[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.

2018-10-19 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.h:350
   void getDefaultScheduleAndChunk(CodeGenFunction &CGF,
-  const OMPLoopDirective &S, OpenMPScheduleClauseKind &ScheduleKind,
+  const OMPLoopDirective &S, OpenMPScheduleTy &ScheduleKind,
   llvm::Value *&Chunk) const override;

Why do you need to change the type of the parameter?


Repository:
  rC Clang

https://reviews.llvm.org/D53443



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


[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

2018-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In https://reviews.llvm.org/D52301#1267163, @kadircet wrote:

> So in the middle of the patch we were just trying to figure out whether we 
> are going to break a big assumption that lots of different components relied 
> on but just didn't had any tests for. Maybe @rsmith would have more context 
> about the behavior?


We used to update the `auto` type in both the semantic type of the variable and 
in the type-as-written. I changed that in r180808; it was a while back, and I 
didn't do a good job explaining in the commit message, but I think the only 
reason was consistency. For the case of a deduced return type in a function, we 
really want to leave the type in the `TypeSourceInfo` alone, because we need to 
track the "declared return type" for redeclaration matching:

  auto f() { return 0; }
  int f(); // error
  auto f(); // OK

As we need to sometimes track the "declared type" of the entity as distinct 
from the semantic type, and as the `TypeSourceInfo` is intended to track the 
type as written, it made sense to consistently not update the `auto` type in 
the type source information (so the TSI has the declared type, and getType() 
returns the semantic type).

That said... I'm definitely sympathetic to the problem. When using an AST 
matcher, for instance, we want to simultaneously think about matching certain 
syntactic source constructs (eg, match a `TypeLoc`) and then ask about the 
semantic properties of the type (for which you want to know what the deduced 
type was for an auto type). If we update the `TypeSourceInfo` for variables but 
not for functions, is that going to deliver the value you're looking for here, 
or should we be looking for some way to deal with cases where we want the TSI 
and type to differ and still want to be able to work out correspondences 
between them?


Repository:
  rC Clang

https://reviews.llvm.org/D52301



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


[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

2018-10-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: lib/Headers/opencl-c.h:16197
+#ifdef cl_intel_device_side_avc_motion_estimation
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+

Anastasia wrote:
> Should we be using:
>   #pragma OPENCL EXTENSION my_ext : begin
> then the user can get correct diagnostics that the functions can only be used 
> once the extension is enabled. 
Would it be better to use `begin`/`end` instead of `enable`/`disable`?



Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:26
+ char4 c4, event_t e, struct st ss) {
+  intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer 
for mce types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with 
an expression of incompatible type 'int'}}

AlexeySachkov wrote:
> Anastasia wrote:
> > AlexeySachkov wrote:
> > > Anastasia wrote:
> > > > Would it make sense to add a check for non-zero constant?
> > > > 
> > > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type 
> > > > from the same type? Any other restrictions on assignment (i.e. w 
> > > > integer literals) and operations over these types?
> > > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type 
> > > > from the same type?
> > > 
> > > Yes, such assignment is allowed.
> > > 
> > > > Any other restrictions on assignment (i.e. w integer literals)
> > > 
> > > All of these types can only be initialized using call to a special 
> > > built-ins or using predefined macros like 
> > > `CLK_AVC_REF_RESULT_INITIALIZE_INTEL`. Any other assignment should lead 
> > > to an error. 
> > > 
> > > I found that I'm able to assign variable of type 
> > > `intel_sub_group_avc_imc_payload_t` to variable of type 
> > > `intel_sub_group_avc_mce_payload_t`, so I will update the patch when I 
> > > implement such diagnostic message.
> > > 
> > > > and operations over these types?
> > > Variables of these types can only be used as return values or arguments 
> > > for built-in functions described in the specification. All other 
> > > operations are restricted
> > W/o the spec change it's really difficult to review properly. So are you 
> > testing 2 groups of types:
> > 1. Init by zero in `bar`?
> > 2. Init by builtin function in `foo`?
> > 
> I would like to test:
> 1. foo: init by literal or variable, negative tests
> 2. far: init by other type or assignment between different types, negative 
> tests
> 3. bar: init by special initializers from spec, positive tests
Ok, I see. Considering that the restirctions are not in the spec yet. I feel 
adding a comment describing initialization/assignment/operation restrictions 
for different types would be helpful here.



Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:16
+
+#define CLK_AVC_IME_RESULT_INITIALIZE_INTEL 0x0
+#define CLK_AVC_REF_RESULT_INITIALIZE_INTEL 0x0

Is there any point to have all these defines if they are all mapped to 0? I 
think using constant literals are easier for testing.


https://reviews.llvm.org/D51484



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


r344800 - Add basic test that we perform lifetime extension in the expected

2018-10-19 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri Oct 19 12:01:31 2018
New Revision: 344800

URL: http://llvm.org/viewvc/llvm-project?rev=344800&view=rev
Log:
Add basic test that we perform lifetime extension in the expected
situations.

Added:
cfe/trunk/test/CXX/special/class.temporary/p6.cpp

Added: cfe/trunk/test/CXX/special/class.temporary/p6.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.temporary/p6.cpp?rev=344800&view=auto
==
--- cfe/trunk/test/CXX/special/class.temporary/p6.cpp (added)
+++ cfe/trunk/test/CXX/special/class.temporary/p6.cpp Fri Oct 19 12:01:31 2018
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -std=c++17 %s -emit-llvm -o - | FileCheck %s 
--implicit-check-not='call{{.*}}dtor'
+
+void then();
+
+struct dtor {
+  ~dtor();
+};
+
+dtor ctor();
+
+// [lifetime extension occurs if the object was obtained by]
+//  -- a temporary materialization conversion
+// CHECK-LABEL: ref_binding
+void ref_binding() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = ctor();
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+
+//  -- ( expression )
+// CHECK-LABEL: parens
+void parens() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = ctor();
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+
+//  -- subscripting of an array
+// CHECK-LABEL: array_subscript_1
+void array_subscript_1() {
+  using T = dtor[1];
+  // CHECK: call {{.*}}ctor
+  auto &&x = T{ctor()}[0];
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: array_subscript_2
+void array_subscript_2() {
+  using T = dtor[1];
+  // CHECK: call {{.*}}ctor
+  auto &&x = ((dtor*)T{ctor()})[0];
+  // CHECK: call {{.*}}dtor
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: }
+}
+
+struct with_member { dtor d; ~with_member(); };
+struct with_ref_member { dtor &&d; ~with_ref_member(); };
+
+//  -- a class member access using the . operator [...]
+// CHECK-LABEL: member_access_1
+void member_access_1() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = with_member{ctor()}.d;
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}with_member
+  // CHECK: }
+}
+// CHECK-LABEL: member_access_2
+void member_access_2() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = with_ref_member{ctor()}.d;
+  // CHECK: call {{.*}}with_ref_member
+  // CHECK: call {{.*}}dtor
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: }
+}
+// CHECK-LABEL: member_access_3
+void member_access_3() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = (&(const with_member&)with_member{ctor()})->d;
+  // CHECK: call {{.*}}with_member
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: }
+}
+
+//  -- a pointer-to-member operation using the .* operator [...]
+// CHECK-LABEL: member_ptr_access_1
+void member_ptr_access_1() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = with_member{ctor()}.*&with_member::d;
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}with_member
+  // CHECK: }
+}
+// CHECK-LABEL: member_ptr_access_2
+void member_ptr_access_2() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = (&(const with_member&)with_member{ctor()})->*&with_member::d;
+  // CHECK: call {{.*}}with_member
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: }
+}
+
+//  -- a [named] cast [...]
+// CHECK-LABEL: static_cast
+void test_static_cast() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = static_cast(ctor());
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: const_cast
+void test_const_cast() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = const_cast(ctor());
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: reinterpret_cast
+void test_reinterpret_cast() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = reinterpret_cast(static_cast(ctor()));
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: dynamic_cast
+void test_dynamic_cast() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = dynamic_cast(ctor());
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+
+//  -- [explicit cast notation is defined in terms of the above]
+// CHECK-LABEL: c_style_cast
+void c_style_cast() {
+  // CHECK: call {{.*}}ctor
+  auto &&x = (dtor&&)ctor();
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+// CHECK-LABEL: function_style_cast
+void function_style_cast() {
+  // CHECK: call {{.*}}ctor
+  using R = dtor&&;
+  auto &&x = R(ctor());
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: }
+}
+
+//  -- a conditional operator
+// CHECK-LABEL: conditional
+void conditional(bool b) {
+  // CHECK: call {{.*}}ctor
+  // CHECK: call {{.*}}ctor
+  auto &&x = b ? (dtor&&)ctor() : (dtor&&)ctor();
+  // CHECK: call {{.*}}then
+  then();
+  // CHECK: call {{.*}}dtor
+  // CHECK: call {{.*}}dto

r344801 - PR24164, PR39336: init-captures are not distinct full-expressions.

2018-10-19 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri Oct 19 12:01:34 2018
New Revision: 344801

URL: http://llvm.org/viewvc/llvm-project?rev=344801&view=rev
Log:
PR24164, PR39336: init-captures are not distinct full-expressions.

Rather, they are subexpressions of the enclosing lambda-expression, and
any temporaries in them are destroyed at the end of that
full-expression, or when the corresponding lambda-expression is
destroyed if they are lifetime-extended.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/test/CXX/special/class.temporary/p6.cpp
cfe/trunk/test/CodeGenCXX/cxx1y-init-captures.cpp
cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=344801&r1=344800&r2=344801&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Oct 19 12:01:34 2018
@@ -5315,8 +5315,7 @@ public:
   }
   ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
  bool DiscardedValue = false,
- bool IsConstexpr = false,
- bool IsLambdaInitCaptureInitializer = false);
+ bool IsConstexpr = false);
   StmtResult ActOnFinishFullStmt(Stmt *Stmt);
 
   // Marks SS invalid if it represents an incomplete type.

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=344801&r1=344800&r2=344801&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Fri Oct 19 12:01:34 2018
@@ -3255,11 +3255,8 @@ bool Expr::HasSideEffects(const ASTConte
 
   case LambdaExprClass: {
 const LambdaExpr *LE = cast(this);
-for (LambdaExpr::capture_iterator I = LE->capture_begin(),
-  E = LE->capture_end(); I != E; ++I)
-  if (I->getCaptureKind() == LCK_ByCopy)
-// FIXME: Only has a side-effect if the variable is volatile or if
-// the copy would invoke a non-trivial copy constructor.
+for (Expr *E : LE->capture_inits())
+  if (E->HasSideEffects(Ctx, IncludePossibleEffects))
 return true;
 return false;
   }

Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=344801&r1=344800&r2=344801&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Fri Oct 19 12:01:34 2018
@@ -2252,7 +2252,6 @@ llvm::Value *CodeGenFunction::EmitDynami
 }
 
 void CodeGenFunction::EmitLambdaExpr(const LambdaExpr *E, AggValueSlot Slot) {
-  RunCleanupsScope Scope(*this);
   LValue SlotLV = MakeAddrLValue(Slot.getAddress(), E->getType());
 
   CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin();

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=344801&r1=344800&r2=344801&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Oct 19 12:01:34 2018
@@ -7764,41 +7764,24 @@ Sema::CorrectDelayedTyposInExpr(Expr *E,
 
 ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
  bool DiscardedValue,
- bool IsConstexpr,
- bool IsLambdaInitCaptureInitializer) {
+ bool IsConstexpr) {
   ExprResult FullExpr = FE;
 
   if (!FullExpr.get())
 return ExprError();
 
-  // If we are an init-expression in a lambdas init-capture, we should not
-  // diagnose an unexpanded pack now (will be diagnosed once lambda-expr
-  // containing full-expression is done).
-  // template void test(Ts ... t) {
-  //   test([&a(t)]() { <-- (t) is an init-expr that shouldn't be diagnosed 
now.
-  // return a;
-  //   }() ...);
-  // }
-  // FIXME: This is a hack. It would be better if we pushed the lambda scope
-  // when we parse the lambda introducer, and teach capturing (but not
-  // unexpanded pack detection) to walk over LambdaScopeInfos which don't have 
a
-  // corresponding class yet (that is, have LambdaScopeInfo either represent a
-  // lambda where we've entered the introducer but not the body, or represent a
-  // lambda where we've entered the body, depending on where the
-  // parser/instantiation has got to).
-  if (!IsLambdaInitCaptureIn

[PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records

2018-10-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 170236.
gtbercea added a comment.
Herald added subscribers: cfe-commits, jholewinski.

  Refactor.


Repository:
  rC Clang

https://reviews.llvm.org/D53141

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp


Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4238,16 +4238,17 @@
 Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF),
 CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0),
 S.getIterationVariable()->getType(), S.getBeginLoc());
+return;
   }
+  CGOpenMPRuntime::getDefaultDistScheduleAndChunk(
+  CGF, S, ScheduleKind, Chunk);
 }
 
 void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk(
 CodeGenFunction &CGF, const OMPLoopDirective &S,
 OpenMPScheduleClauseKind &ScheduleKind,
 llvm::Value *&Chunk) const {
-  if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) {
-ScheduleKind = OMPC_SCHEDULE_static;
-Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
-S.getIterationVariable()->getType()), 1);
-  }
+  ScheduleKind = OMPC_SCHEDULE_static;
+  Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
+  S.getIterationVariable()->getType()), 1);
 }


Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4238,16 +4238,17 @@
 Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF),
 CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0),
 S.getIterationVariable()->getType(), S.getBeginLoc());
+return;
   }
+  CGOpenMPRuntime::getDefaultDistScheduleAndChunk(
+  CGF, S, ScheduleKind, Chunk);
 }
 
 void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk(
 CodeGenFunction &CGF, const OMPLoopDirective &S,
 OpenMPScheduleClauseKind &ScheduleKind,
 llvm::Value *&Chunk) const {
-  if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) {
-ScheduleKind = OMPC_SCHEDULE_static;
-Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
-S.getIterationVariable()->getType()), 1);
-  }
+  ScheduleKind = OMPC_SCHEDULE_static;
+  Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
+  S.getIterationVariable()->getType()), 1);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.

2018-10-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 170237.
gtbercea added a comment.

  Refactor.


Repository:
  rC Clang

https://reviews.llvm.org/D53443

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp


Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4238,16 +4238,17 @@
 Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF),
 CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0),
 S.getIterationVariable()->getType(), S.getBeginLoc());
+return;
   }
+  CGOpenMPRuntime::getDefaultDistScheduleAndChunk(
+  CGF, S, ScheduleKind, Chunk);
 }
 
 void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk(
 CodeGenFunction &CGF, const OMPLoopDirective &S,
 OpenMPScheduleClauseKind &ScheduleKind,
 llvm::Value *&Chunk) const {
-  if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) {
-ScheduleKind = OMPC_SCHEDULE_static;
-Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
-S.getIterationVariable()->getType()), 1);
-  }
+  ScheduleKind = OMPC_SCHEDULE_static;
+  Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
+  S.getIterationVariable()->getType()), 1);
 }


Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4238,16 +4238,17 @@
 Chunk = CGF.EmitScalarConversion(getNVPTXNumThreads(CGF),
 CGF.getContext().getIntTypeForBitwidth(32, /*Signed=*/0),
 S.getIterationVariable()->getType(), S.getBeginLoc());
+return;
   }
+  CGOpenMPRuntime::getDefaultDistScheduleAndChunk(
+  CGF, S, ScheduleKind, Chunk);
 }
 
 void CGOpenMPRuntimeNVPTX::getDefaultScheduleAndChunk(
 CodeGenFunction &CGF, const OMPLoopDirective &S,
 OpenMPScheduleClauseKind &ScheduleKind,
 llvm::Value *&Chunk) const {
-  if (getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_SPMD) {
-ScheduleKind = OMPC_SCHEDULE_static;
-Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
-S.getIterationVariable()->getType()), 1);
-  }
+  ScheduleKind = OMPC_SCHEDULE_static;
+  Chunk = CGF.Builder.getIntN(CGF.getContext().getTypeSize(
+  S.getIterationVariable()->getType()), 1);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records

2018-10-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 170238.
gtbercea added a comment.

  Refactor.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D53141

Files:
  libomptarget/deviceRTLs/nvptx/src/data_sharing.cu
  libomptarget/deviceRTLs/nvptx/src/interface.h
  libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu
  libomptarget/deviceRTLs/nvptx/src/supporti.h

Index: libomptarget/deviceRTLs/nvptx/src/supporti.h
===
--- libomptarget/deviceRTLs/nvptx/src/supporti.h
+++ libomptarget/deviceRTLs/nvptx/src/supporti.h
@@ -188,7 +188,6 @@
 {
   void *ptr = malloc(size);
   PRINT(LD_MEM, "malloc data of size %zu for %s: 0x%llx\n", size, msg, P64(ptr));
-  ASSERT(LT_SAFETY, ptr, "failed to allocate %zu bytes for %s\n", size, msg);
   return ptr;
 }
 
Index: libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu
===
--- libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu
+++ libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.cu
@@ -40,8 +40,6 @@
 INLINE unsigned smid() {
   unsigned id;
   asm("mov.u32 %0, %%smid;" : "=r"(id));
-  ASSERT0(LT_FUSSY, nsmid() <= MAX_SM,
-  "Expected number of SMs is less than reported.");
   return id;
 }
 
@@ -156,7 +154,6 @@
   //
   omptarget_nvptx_TaskDescr *newTaskDescr =
   omptarget_nvptx_threadPrivateContext->Level1TaskDescr(threadId);
-  ASSERT0(LT_FUSSY, newTaskDescr, "expected a task descr");
   newTaskDescr->InitLevelOneTaskDescr(ThreadLimit,
   currTeamDescr.LevelZeroTaskDescr());
   newTaskDescr->ThreadLimit() = ThreadLimit;
Index: libomptarget/deviceRTLs/nvptx/src/interface.h
===
--- libomptarget/deviceRTLs/nvptx/src/interface.h
+++ libomptarget/deviceRTLs/nvptx/src/interface.h
@@ -478,6 +478,8 @@
 
 EXTERN void __kmpc_data_sharing_init_stack();
 EXTERN void __kmpc_data_sharing_init_stack_spmd();
+EXTERN void *__kmpc_data_sharing_coalesced_push_stack(size_t size,
+int16_t UseSharedMemory);
 EXTERN void *__kmpc_data_sharing_push_stack(size_t size, int16_t UseSharedMemory);
 EXTERN void __kmpc_data_sharing_pop_stack(void *a);
 EXTERN void __kmpc_begin_sharing_variables(void ***GlobalArgs, size_t nArgs);
Index: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu
===
--- libomptarget/deviceRTLs/nvptx/src/data_sharing.cu
+++ libomptarget/deviceRTLs/nvptx/src/data_sharing.cu
@@ -369,96 +369,109 @@
   __threadfence_block();
 }
 
-// Called at the time of the kernel initialization. This is used to initilize
-// the list of references to shared variables and to pre-allocate global storage
-// for holding the globalized variables.
-//
-// By default the globalized variables are stored in global memory. If the
-// UseSharedMemory is set to true, the runtime will attempt to use shared memory
-// as long as the size requested fits the pre-allocated size.
-EXTERN void* __kmpc_data_sharing_push_stack(size_t DataSize,
-int16_t UseSharedMemory) {
+INLINE void* data_sharing_push_stack_common(size_t PushSize) {
   if (isRuntimeUninitialized()) {
 ASSERT0(LT_FUSSY, isSPMDMode(),
 "Expected SPMD mode with uninitialized runtime.");
-return omptarget_nvptx_SimpleThreadPrivateContext::Allocate(DataSize);
+return omptarget_nvptx_SimpleThreadPrivateContext::Allocate(PushSize);
   }
 
+  // Only warp active master threads manage the stack.
+  bool IsWarpMaster = (getThreadId() % WARPSIZE) == 0;
+
   // Add worst-case padding to DataSize so that future stack allocations are
   // correctly aligned.
   const size_t Alignment = 8;
-  if (DataSize % Alignment != 0) {
-DataSize += (Alignment - DataSize % Alignment);
-  }
+  PushSize = (PushSize + (Alignment - 1)) / Alignment * Alignment;
 
   // Frame pointer must be visible to all workers in the same warp.
   unsigned WID = getWarpId();
+  // void * volatile FramePointer = 0;
   void *&FrameP = DataSharingState.FramePtr[WID];
 
-  // Only warp active master threads manage the stack.
-  if (getThreadId() % WARPSIZE == 0) {
-// SlotP will point to either the shared memory slot or an existing
-// global memory slot.
-__kmpc_data_sharing_slot *&SlotP = DataSharingState.SlotPtr[WID];
-void *&StackP = DataSharingState.StackPtr[WID];
-
-// Compute the total memory footprint of the requested data.
-// The master thread requires a stack only for itself. A worker
-// thread (which at this point is a warp master) will require
-// space for the variables of each thread in the warp,
-// i.e. one DataSize chunk per warp lane.
-// TODO: change WARPSIZE to the number of active threads in the warp.
-size_t PushSize = IsMasterThread() ? DataSize : WARPSIZE * DataSize;
+  do {
+if (IsWarpMaster) {
+  // SlotP will point to either the shared memory slot or an existing
+  // global memory slot

r344806 - Revert "[Driver] Reland: Default Android toolchains to libc++."

2018-10-19 Thread Dan Albert via cfe-commits
Author: danalbert
Date: Fri Oct 19 12:23:01 2018
New Revision: 344806

URL: http://llvm.org/viewvc/llvm-project?rev=344806&view=rev
Log:
Revert "[Driver] Reland: Default Android toolchains to libc++."

This reverts commit 84677d5009d613232d360fda27e6e41fb5cb6700.

Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.h
cfe/trunk/test/Driver/android-ndk-standalone.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=344806&r1=344805&r2=344806&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Fri Oct 19 12:23:01 2018
@@ -443,12 +443,6 @@ Linux::Linux(const Driver &D, const llvm
   addPathIfExists(D, SysRoot + "/usr/lib", Paths);
 }
 
-ToolChain::CXXStdlibType Linux::GetDefaultCXXStdlibType() const {
-  if (getTriple().isAndroid())
-return ToolChain::CST_Libcxx;
-  return ToolChain::CST_Libstdcxx;
-}
-
 bool Linux::HasNativeLLVMSupport() const { return true; }
 
 Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); }

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.h?rev=344806&r1=344805&r2=344806&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.h Fri Oct 19 12:23:01 2018
@@ -37,7 +37,6 @@ public:
   llvm::opt::ArgStringList &CC1Args) const override;
   void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
-  CXXStdlibType GetDefaultCXXStdlibType() const override;
   bool isPIEDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;

Modified: cfe/trunk/test/Driver/android-ndk-standalone.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-ndk-standalone.cpp?rev=344806&r1=344805&r2=344806&view=diff
==
--- cfe/trunk/test/Driver/android-ndk-standalone.cpp (original)
+++ cfe/trunk/test/Driver/android-ndk-standalone.cpp Fri Oct 19 12:23:01 2018
@@ -2,13 +2,21 @@
 // toolchain.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi21 \
+// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
 // RUN: -B%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN:   | FileCheck  %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/v1"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/backward"
 // CHECK: "-internal-isystem" "{{.*}}/sysroot/usr/local/include"
 // CHECK: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
 // CHECK: "-internal-externc-isystem" 
"{{.*}}/sysroot/usr/include/arm-linux-androideabi"
@@ -41,47 +49,21 @@
 // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_ndk_tree \
-// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
-// RUN:   | FileCheck --check-prefix=CHECK-STDCXX %s
-// CHECK-STDCXX: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-STDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9"
-// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
-// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
-// CHECK-STDCXX: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi"
-// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// C

[PATCH] D53448: [OpenMP][NVPTX] Use single loops when generating code for distribute parallel for

2018-10-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea created this revision.
gtbercea added reviewers: ABataev, caomhin.
Herald added subscribers: cfe-commits, guansong, jholewinski.

This patch adds a new code generation path for bound sharing directives 
containing distribute parallel for. The new code generation scheme applies to 
chunked schedules on distribute and parallel for directives. The scheme 
simplifies the code that is being generated by eliminating the need for an 
outer for loop over chunks for both distribute and parallel for directives. In 
the case of distribute it applies to any sized chunk while in the parallel for 
case it only applies when chunk size is 1.


Repository:
  rC Clang

https://reviews.llvm.org/D53448

Files:
  include/clang/AST/StmtOpenMP.h
  include/clang/Basic/OpenMPKinds.h
  lib/AST/StmtOpenMP.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/OpenMP/distribute_parallel_for_codegen.cpp
  test/OpenMP/distribute_parallel_for_simd_codegen.cpp

Index: test/OpenMP/distribute_parallel_for_simd_codegen.cpp
===
--- test/OpenMP/distribute_parallel_for_simd_codegen.cpp
+++ test/OpenMP/distribute_parallel_for_simd_codegen.cpp
@@ -406,18 +406,16 @@
   a[i] = b[i] + c[i];
   // LAMBDA: define{{.+}} void [[OMP_OUTLINED_3]](
   // LAMBDA-DAG: [[OMP_IV:%.omp.iv]] = alloca
+  // LAMBDA-DAG: [[OMP_CAPT_EXPR:%.capture_expr.1]] = alloca
   // LAMBDA-DAG: [[OMP_LB:%.omp.comb.lb]] = alloca
   // LAMBDA-DAG: [[OMP_UB:%.omp.comb.ub]] = alloca
   // LAMBDA-DAG: [[OMP_ST:%.omp.stride]] = alloca
 
-  // unlike the previous tests, in this one we have a outer and inner loop for 'distribute'
   // LAMBDA: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, i32 91,
-  // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER:.+]]
 
-  // LAMBDA: [[DIST_OUTER_LOOP_HEADER]]:
   // check EUB for distribute
   // LAMBDA-DAG: [[OMP_UB_VAL_1:%.+]] = load{{.+}} [[OMP_UB]],
-  // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}},
+  // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}} [[OMP_CAPT_EXPR]],
   // LAMBDA-DAG: [[CMP_UB_NUM_IT:%.+]] = icmp sgt {{.+}}  [[OMP_UB_VAL_1]], [[NUM_IT_1]]
   // LAMBDA: br {{.+}} [[CMP_UB_NUM_IT]], label %[[EUB_TRUE:.+]], label %[[EUB_FALSE:.+]]
   // LAMBDA-DAG: [[EUB_TRUE]]:
@@ -436,18 +434,9 @@
 
   // check exit condition
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
-  // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_UB]],
+  // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_CAPT_EXPR]],
   // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], [[OMP_UB_VAL_3]]
-  // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_OUTER_LOOP_BODY:.+]], label %[[DIST_OUTER_LOOP_END:.+]]
-
-  // LAMBDA: [[DIST_OUTER_LOOP_BODY]]:
-  // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER:.+]]
-
-  // LAMBDA: [[DIST_INNER_LOOP_HEADER]]:
-  // LAMBDA-DAG: [[OMP_IV_VAL_2:%.+]] = load {{.+}} [[OMP_IV]],
-  // LAMBDA-DAG: [[OMP_UB_VAL_4:%.+]] = load {{.+}} [[OMP_UB]],
-  // LAMBDA: [[CMP_IV_UB_2:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_2]], [[OMP_UB_VAL_4]]
-  // LAMBDA: br{{.+}} [[CMP_IV_UB_2]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]]
+  // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
   // LAMBDA: [[DIST_INNER_LOOP_BODY]]:
@@ -466,25 +455,39 @@
   // LAMBDA-DAG: [[OMP_ST_VAL_1:%.+]] = load {{.+}}, {{.+}}* [[OMP_ST]],
   // LAMBDA: [[OMP_IV_INC:%.+]] = add{{.+}} [[OMP_IV_VAL_3]], [[OMP_ST_VAL_1]]
   // LAMBDA: store{{.+}} [[OMP_IV_INC]], {{.+}}* [[OMP_IV]],
-  // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER]]
-
-  // LAMBDA: [[DIST_INNER_LOOP_END]]:
-  // LAMBDA: br label %[[DIST_OUTER_LOOP_INC:.+]]
-
-  // LAMBDA: [[DIST_OUTER_LOOP_INC]]:
-  // check NextLB and NextUB
   // LAMBDA-DAG: [[OMP_LB_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_LB]],
   // LAMBDA-DAG: [[OMP_ST_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]],
   // LAMBDA-DAG: [[OMP_LB_NEXT:%.+]] = add{{.+}} [[OMP_LB_VAL_2]], [[OMP_ST_VAL_2]]
   // LAMBDA: store{{.+}} [[OMP_LB_NEXT]], {{.+}}* [[OMP_LB]],
   // LAMBDA-DAG: [[OMP_UB_VAL_5:%.+]] = load{{.+}}, {{.+}} [[OMP_UB]],
   // LAMBDA-DAG: [[OMP_ST_VAL_3:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]],
   // LAMBDA-DAG: [[OMP_UB_NEXT:%.+]] = add{{.+}} [[OMP_UB_VAL_5]], [[OMP_ST_VAL_3]]
   // LAMBDA: store{{.+}} [[OMP_UB_NEXT]], {{.+}}* [[OMP_UB]],
-  // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER]]
 
-  // outer loop exit
-  // LAMBDA: [[DIST_OUTER_LOOP_END]]:
+  // Update UB
+  // LAMBDA-DAG: [[OMP_UB_VAL_6:%.+]] = load{{.+}}, {{.+

[PATCH] D53448: [OpenMP][NVPTX] Use single loops when generating code for distribute parallel for

2018-10-19 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 170241.
gtbercea added a comment.

  Rebase.


Repository:
  rC Clang

https://reviews.llvm.org/D53448

Files:
  include/clang/AST/StmtOpenMP.h
  include/clang/Basic/OpenMPKinds.h
  lib/AST/StmtOpenMP.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/OpenMP/distribute_parallel_for_codegen.cpp
  test/OpenMP/distribute_parallel_for_simd_codegen.cpp

Index: test/OpenMP/distribute_parallel_for_simd_codegen.cpp
===
--- test/OpenMP/distribute_parallel_for_simd_codegen.cpp
+++ test/OpenMP/distribute_parallel_for_simd_codegen.cpp
@@ -406,18 +406,16 @@
   a[i] = b[i] + c[i];
   // LAMBDA: define{{.+}} void [[OMP_OUTLINED_3]](
   // LAMBDA-DAG: [[OMP_IV:%.omp.iv]] = alloca
+  // LAMBDA-DAG: [[OMP_CAPT_EXPR:%.capture_expr.1]] = alloca
   // LAMBDA-DAG: [[OMP_LB:%.omp.comb.lb]] = alloca
   // LAMBDA-DAG: [[OMP_UB:%.omp.comb.ub]] = alloca
   // LAMBDA-DAG: [[OMP_ST:%.omp.stride]] = alloca
 
-  // unlike the previous tests, in this one we have a outer and inner loop for 'distribute'
   // LAMBDA: call void @__kmpc_for_static_init_4({{.+}}, {{.+}}, i32 91,
-  // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER:.+]]
 
-  // LAMBDA: [[DIST_OUTER_LOOP_HEADER]]:
   // check EUB for distribute
   // LAMBDA-DAG: [[OMP_UB_VAL_1:%.+]] = load{{.+}} [[OMP_UB]],
-  // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}},
+  // LAMBDA: [[NUM_IT_1:%.+]] = load{{.+}} [[OMP_CAPT_EXPR]],
   // LAMBDA-DAG: [[CMP_UB_NUM_IT:%.+]] = icmp sgt {{.+}}  [[OMP_UB_VAL_1]], [[NUM_IT_1]]
   // LAMBDA: br {{.+}} [[CMP_UB_NUM_IT]], label %[[EUB_TRUE:.+]], label %[[EUB_FALSE:.+]]
   // LAMBDA-DAG: [[EUB_TRUE]]:
@@ -436,18 +434,9 @@
 
   // check exit condition
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
-  // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_UB]],
+  // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}} [[OMP_CAPT_EXPR]],
   // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], [[OMP_UB_VAL_3]]
-  // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_OUTER_LOOP_BODY:.+]], label %[[DIST_OUTER_LOOP_END:.+]]
-
-  // LAMBDA: [[DIST_OUTER_LOOP_BODY]]:
-  // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER:.+]]
-
-  // LAMBDA: [[DIST_INNER_LOOP_HEADER]]:
-  // LAMBDA-DAG: [[OMP_IV_VAL_2:%.+]] = load {{.+}} [[OMP_IV]],
-  // LAMBDA-DAG: [[OMP_UB_VAL_4:%.+]] = load {{.+}} [[OMP_UB]],
-  // LAMBDA: [[CMP_IV_UB_2:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_2]], [[OMP_UB_VAL_4]]
-  // LAMBDA: br{{.+}} [[CMP_IV_UB_2]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]]
+  // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
   // LAMBDA: [[DIST_INNER_LOOP_BODY]]:
@@ -466,25 +455,39 @@
   // LAMBDA-DAG: [[OMP_ST_VAL_1:%.+]] = load {{.+}}, {{.+}}* [[OMP_ST]],
   // LAMBDA: [[OMP_IV_INC:%.+]] = add{{.+}} [[OMP_IV_VAL_3]], [[OMP_ST_VAL_1]]
   // LAMBDA: store{{.+}} [[OMP_IV_INC]], {{.+}}* [[OMP_IV]],
-  // LAMBDA: br label %[[DIST_INNER_LOOP_HEADER]]
-
-  // LAMBDA: [[DIST_INNER_LOOP_END]]:
-  // LAMBDA: br label %[[DIST_OUTER_LOOP_INC:.+]]
-
-  // LAMBDA: [[DIST_OUTER_LOOP_INC]]:
-  // check NextLB and NextUB
   // LAMBDA-DAG: [[OMP_LB_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_LB]],
   // LAMBDA-DAG: [[OMP_ST_VAL_2:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]],
   // LAMBDA-DAG: [[OMP_LB_NEXT:%.+]] = add{{.+}} [[OMP_LB_VAL_2]], [[OMP_ST_VAL_2]]
   // LAMBDA: store{{.+}} [[OMP_LB_NEXT]], {{.+}}* [[OMP_LB]],
   // LAMBDA-DAG: [[OMP_UB_VAL_5:%.+]] = load{{.+}}, {{.+}} [[OMP_UB]],
   // LAMBDA-DAG: [[OMP_ST_VAL_3:%.+]] = load{{.+}}, {{.+}} [[OMP_ST]],
   // LAMBDA-DAG: [[OMP_UB_NEXT:%.+]] = add{{.+}} [[OMP_UB_VAL_5]], [[OMP_ST_VAL_3]]
   // LAMBDA: store{{.+}} [[OMP_UB_NEXT]], {{.+}}* [[OMP_UB]],
-  // LAMBDA: br label %[[DIST_OUTER_LOOP_HEADER]]
 
-  // outer loop exit
-  // LAMBDA: [[DIST_OUTER_LOOP_END]]:
+  // Update UB
+  // LAMBDA-DAG: [[OMP_UB_VAL_6:%.+]] = load{{.+}}, {{.+}} [[OMP_UB]],
+  // LAMBDA: [[OMP_EXPR_VAL:%.+]] = load{{.+}}, {{.+}} [[OMP_CAPT_EXPR]],
+  // LAMBDA-DAG: [[CMP_UB_NUM_IT_1:%.+]] = icmp sgt {{.+}}[[OMP_UB_VAL_6]], [[OMP_EXPR_VAL]]
+  // LAMBDA: br {{.+}} [[CMP_UB_NUM_IT_1]], label %[[EUB_TRUE_1:.+]], label %[[EUB_FALSE_1:.+]]
+  // LAMBDA-DAG: [[EUB_TRUE_1]]:
+  // LAMBDA: [[NUM_IT_3:%.+]] = load{{.+}} [[OMP_CAPT_EXPR]],
+  // LAMBDA: br label %[[EUB_END_1:.+]]
+  // LAMBDA-DAG: [[EUB_FALSE_1]]:
+  // LAMBDA: [[OMP_UB_VAL3:%.+]] = 

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-10-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Code looks good to me, but I'd wait for @george.karpenkov because it was his 
idea.




Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:502
+
+  auto AssertM = callExpr(callee(functionDecl(hasName("assert";
+  auto GuardM =

In a lot of standard libraries `assert()` is implemented as a macro. You might 
want to catch the corresponding builtin or look at macro names (not sure if we 
have a matcher for the latter but it should be easy to add).


https://reviews.llvm.org/D51866



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


[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

2018-10-19 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/altivec-generic-overload.c:3
+
+typedef signed char __v4sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v4uc __attribute__((__vector_size__(16)));

`__v4sc` is suspicious. The most consistent naming I have seen is `v16i8`, 
`v16u8`, `v8i16`, ...
Do we need the double underscores?



Repository:
  rC Clang

https://reviews.llvm.org/D53417



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


[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-10-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:515
+  continue;
+const auto *FirstAccess = Accesses[0].getNodeAs("access");
+

I feel that it's a good practice to `assert(FirstAccess)` here (same for 
`FirstGuard` and generally after every `getNodeAs`). The assertion will hold 
because "access" is always bound to on every possible match and is always a 
member-expression, and it'd be great to check that after the matcher is changed.


https://reviews.llvm.org/D51866



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


[PATCH] D53406: [clangd] Provide excuses for bad code completions, based on diagnostics. C++ API only.

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Since we're showing the diagnostics in the editors anyway, how crucial do we 
think it is to actually add that to the procotol?
Having more concrete reasons for misbehaving completions sounds more useful, 
though, e.g. "cannot complete members of the incomplete type", etc.

Totally fine to have this in the C++ API for experiments, of course.

> - LSP provides no protocol mechanims, and editors don't have UI for this

Have we tried returning an error in addition to the results? How do the clients 
behave in that case?




Comment at: clangd/CodeComplete.cpp:674
 
+// Tries to come up with a convincing excuse for our bad completion results,
+// based on the diagnostics near the completion point.

It took some time to figure out what "excuse" means here. 
Maybe be more straightforward in naming/comments about what this class does? 
I.e. finds a last diagnostic on the same line.
It's private to this file, so it's easy to rename if we'll want to expand the 
heuristics.



Comment at: clangd/CodeComplete.cpp:686
+ for (StartOfLine = Offset;
+  StartOfLine > 0 && Code[StartOfLine - 1] != '\n'; --StartOfLine)
+   ;

Maybe use a getLineNumber helper from the SourceManager instead?
It needs a FileID, but it shouldn't be hard to get one.



Comment at: clangd/CodeComplete.cpp:1040
+  IncludeStructure *Includes = nullptr,
+  std::string *Excuse = nullptr) {
   trace::Span Tracer("Sema completion");

Maybe replace an out parameter with returning a struct of two members?
Is there any use for calling code completion without providing an "excuse"?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53406



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


[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

2018-10-19 Thread Miklos Vajna via Phabricator via cfe-commits
vmiklos created this revision.
vmiklos added reviewers: alexfh, sbenza.
Herald added subscribers: cfe-commits, xazax.hun.

And also enable it by default to be consistent with e.g. modernize-use-using.

  

This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53454

Files:
  clang-tidy/readability/RedundantSmartptrGetCheck.cpp
  clang-tidy/readability/RedundantSmartptrGetCheck.h
  docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
  test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
  test/clang-tidy/readability-redundant-smartptr-get.cpp

Index: test/clang-tidy/readability-redundant-smartptr-get.cpp
===
--- test/clang-tidy/readability-redundant-smartptr-get.cpp
+++ test/clang-tidy/readability-redundant-smartptr-get.cpp
@@ -168,6 +168,8 @@
   // CHECK-FIXES: if (NULL == x);
 }
 
+#define MACRO(p) p.get()
+
 void Negative() {
   struct NegPtr {
 int* get();
@@ -193,4 +195,7 @@
   bool bb = ip.get() == nullptr;
   bb = !ip.get();
   bb = ip.get() ? true : false;
+  std::unique_ptr x;
+  if (MACRO(x) == nullptr)
+;
 }
Index: test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-smartptr-get.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+namespace std {
+
+template 
+struct shared_ptr {
+  T &operator*() const;
+  T *operator->() const;
+  T *get() const;
+  explicit operator bool() const noexcept;
+};
+
+} // namespace std
+
+#define MACRO(p) p.get()
+
+void Positive() {
+  std::shared_ptr x;
+  if (MACRO(x) == nullptr)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant get() call on smart pointer
+};
Index: docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
===
--- docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
+++ docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
@@ -14,3 +14,8 @@
   *ptr->get()  ==>  **ptr
   if (ptr.get() == nullptr) ... => if (ptr == nullptr) ...
 
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about calls inside macros.
Index: clang-tidy/readability/RedundantSmartptrGetCheck.h
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -28,9 +28,14 @@
 class RedundantSmartptrGetCheck : public ClangTidyCheck {
 public:
   RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -91,6 +91,11 @@
 
 } // namespace
 
+void RedundantSmartptrGetCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
@@ -126,6 +131,9 @@
   bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr;
   bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr;
   const auto *GetCall = Result.Nodes.getNodeAs("redundant_get");
+  if (GetCall->getBeginLoc().isMacroID() && IgnoreMacros)
+return;
+
   const auto *Smartptr = Result.Nodes.getNodeAs("smart_pointer");
 
   if (IsPtrToPtr && IsMemberExpr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

2018-10-19 Thread Miklos Vajna via Phabricator via cfe-commits
vmiklos added a comment.

In practice cppunit's `CPPUNIT_TEST_SUITE_END` macro triggers this problem.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53454



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


[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

2018-10-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163
+
+switch (FloatingSize) {
+  case 64:

Continuing the float semantics discussion on the new revision - Did you 
consider `llvm::APFloat`? (http://llvm.org/doxygen/classllvm_1_1APFloat.html) 
This looks like something that you're trying to re-implement.


Repository:
  rC Clang

https://reviews.llvm.org/D52730



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


[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

2018-10-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a reviewer: aprantl.
echristo added a comment.

I think Adrian has looked at this more recently than I have. Adding him here.


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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


[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

2018-10-19 Thread Oleg Smolsky via Phabricator via cfe-commits
oleg.smolsky added a comment.

In https://reviews.llvm.org/D52676#1268806, @djasper wrote:

> In https://reviews.llvm.org/D52676#1268748, @oleg.smolsky wrote:
>
> > In https://reviews.llvm.org/D52676#1268706, @djasper wrote:
> >
> > > Ok, I think I agree with both of you to a certain extent, but I also 
> > > think this change as is, is not yet right.
> > >
> > > First, it does too much. The original very first example in this CL is 
> > > actually not produced by clang-format (or if it is, I don't know with 
> > > which flag combination). It is a case where the lambda is the last 
> > > parameter.
> >
> >
> > Right, I cheated and created that example by hand. My apologies for the 
> > confusion. I've just pasted real code that I pumped through `clang-format`. 
> > Please take a look at the updated summary.
> >
> > > Second, I agree that the original behavior is inconsistent in a way that 
> > > we have a special cases for when the lambda is the very first parameter, 
> > > but somehow seem be forgetting about that when it's not the first 
> > > parameter. I'd be ok with "fixing" that (it's not a clear-cut bug, but I 
> > > think the alternative behavior would be cleaner). However, I don't think 
> > > your patch is doing enough there. I think this should be irrespective of 
> > > bin-packing (it's related but not quite the same thing),
> >
> > Also there is a special case for multiple lambdas. It forces line breaks. 
> > That aside, for the single-lambda case, are you suggesting that it is 
> > always "pulled up", irrespective of its place? That would contradict the 
> > direction I am trying to take as I like `BinPackArguments: false` and so 
> > long lamba args go to their own lines. This looks very much in line with 
> > what bin packing is, but not exactly the same. Obviously, I can add a new 
> > option `favor line breaks around multi-line lambda`.
>
>
> I don't think I am. You are right, there is the special case for multi-lambda 
> functions and I think we should have almost the same for single-lambda 
> functions. So, I think I agree with you and am in favor of:
>
>   someFunction(
>   a,
>   [] {
> // ...
>   },
>   b);
>   
>
> And this is irrespective of BinPacking. I think this is always better and 
> more consistent with what we'd be doing if "a" was not there. The only caveat 
> is that I think with BinPacking true or false, we should keep the more 
> compact formatting if "b" isn't there and the lambda introducer fits entirely 
> on the first line:
>
>   someFunction(a, [] {
> // ...
>   });


OK, cool, I've generalized the patch and extended the test. This global thing 
also effects other existing test that deal with sub-blocks/lambdas.

>> Could you look at the updated summary (high level) and the new tests I added 
>> (low level) please? Every other test passes, so we have almost the entire 
>> spec. I can add a few cases where an existing snippet is reformatted with 
>> `BinPackArguments: false` too.
> 
> Not sure I am seeing new test cases and I think at least a few cases are 
> missing from the entire spec, e.g. the case above.
> 
> Also, try to reduce the test cases a bit more:
> 
> - They don't need the surrounding functions
> - You can force the lambda to be multi-line with a "//" comment
> - There is no reason to have the lambda be an argument to a member function, 
> a free-standing function works just as well
> 
>   This might seem nit-picky, but in my experience, the more we can reduce the 
> test cases, the easier to read and the less brittle they become.

Makes sense, reduced the extra levels.

>>> ...and it should also apply to multiline strings if 
>>> AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done 
>>> in the same patch, but we should have a plan of what we want the end result 
>>> to look like and should be willing to get there.

Oh, yeah, I will look at that next.


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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


[PATCH] D53456: [Sema] Do not show unused parameter warnings when body is skipped

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.
Herald added a subscriber: arphaman.

Without the function body, we cannot determine is parameter was used.


Repository:
  rC Clang

https://reviews.llvm.org/D53456

Files:
  lib/Sema/SemaDecl.cpp
  test/Index/skipped-bodies-unused.cpp


Index: test/Index/skipped-bodies-unused.cpp
===
--- /dev/null
+++ test/Index/skipped-bodies-unused.cpp
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source 
all %s -Wunused-parameter 2>&1 \
+// RUN: | FileCheck %s
+
+// No 'unused parameter' warnings should be shown when skipping the function 
bodies.
+inline int foo(int used, int unused) {
+used = 100;
+}
+// CHECK-NOT: warning: unused parameter
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13037,7 +13037,7 @@
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
+  if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
 DiagnoseUnusedParameters(FD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
  FD->getReturnType(), FD);
@@ -13132,7 +13132,8 @@
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  DiagnoseUnusedParameters(MD->parameters());
+  if (!MD->hasSkippedBody())
+DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 


Index: test/Index/skipped-bodies-unused.cpp
===
--- /dev/null
+++ test/Index/skipped-bodies-unused.cpp
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s -Wunused-parameter 2>&1 \
+// RUN: | FileCheck %s
+
+// No 'unused parameter' warnings should be shown when skipping the function bodies.
+inline int foo(int used, int unused) {
+used = 100;
+}
+// CHECK-NOT: warning: unused parameter
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13037,7 +13037,7 @@
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
+  if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
 DiagnoseUnusedParameters(FD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
  FD->getReturnType(), FD);
@@ -13132,7 +13132,8 @@
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  DiagnoseUnusedParameters(MD->parameters());
+  if (!MD->hasSkippedBody())
+DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

2018-10-19 Thread Oleg Smolsky via Phabricator via cfe-commits
oleg.smolsky updated this revision to Diff 170270.
oleg.smolsky added a comment.

Generalized the patch so that it deals with lambda args irrespective of bin 
packing. Added additional tests and patched existing ones.


Repository:
  rC Clang

https://reviews.llvm.org/D52676

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3178,25 +3178,27 @@
  "});");
   FormatStyle Style = getGoogleStyle();
   Style.ColumnLimit = 45;
-  verifyFormat("Debug(a,\n"
-   "  {\n"
-   "if () return;\n"
-   "  },\n"
-   "  a);",
+  verifyFormat("Debug(\n"
+   "a,\n"
+   "{\n"
+   "  if () return;\n"
+   "},\n"
+   "a);",
Style);
 
   verifyFormat("SomeFunction({MACRO({ return output; }), b});");
 
   verifyNoCrash("^{v^{a}}");
 }
 
 TEST_F(FormatTest, FormatNestedBlocksInMacros) {
-  EXPECT_EQ("#define MACRO() \\\n"
-"  Debug(aaa, /* force line break */ \\\n"
-"{   \\\n"
-"  int i;\\\n"
-"  int j;\\\n"
-"})",
+  EXPECT_EQ("#define MACRO()   \\\n"
+"  Debug(  \\\n"
+"  aaa, /* force line break */ \\\n"
+"  {   \\\n"
+"int i;\\\n"
+"int j;\\\n"
+"  })",
 format("#define   MACRO()   Debug(aaa,  /* force line break */ \\\n"
"  {  int   i;  int  j;   })",
getGoogleStyle()));
@@ -11596,9 +11598,10 @@
"  other(x.begin(), x.end(), [&](int, int) { return 1; });\n"
"}\n");
   verifyFormat("void f() {\n"
-   "  other(x.begin(), //\n"
-   "x.end(),   //\n"
-   "[&](int, int) { return 1; });\n"
+   "  other(\n"
+   "  x.begin(), //\n"
+   "  x.end(),   //\n"
+   "  [&](int, int) { return 1; });\n"
"}\n");
   verifyFormat("SomeFunction([]() { // A cool function...\n"
"  return 43;\n"
@@ -11651,9 +11654,9 @@
   verifyFormat("SomeFunction({[&] {\n"
"  // comment\n"
"}});");
-  verifyFormat("virtual (std::function  =\n"
-   " [&]() { return true; },\n"
-   " a a);");
+  verifyFormat("virtual (\n"
+   "std::function  = [&]() { return true; },\n"
+   "a a);");
 
   // Lambdas with return types.
   verifyFormat("int c = []() -> int { return 2; }();\n");
@@ -11680,17 +11683,74 @@
"  return 1; //\n"
"};");
 
-  // Multiple lambdas in the same parentheses change indentation rules.
+  // Multiple lambdas in the same parentheses change indentation rules. These
+  // lambdas are forced to start on new lines.
   verifyFormat("SomeFunction(\n"
"[]() {\n"
-   "  int i = 42;\n"
-   "  return i;\n"
+   "  //\n"
"},\n"
"[]() {\n"
-   "  int j = 43;\n"
-   "  return j;\n"
+   "  //\n"
"});");
 
+  // A lambda passed as arg0 is always pushed to the next line.
+  verifyFormat("SomeFunction(\n"
+   "[this] {\n"
+   "  //\n"
+   "},\n"
+   "1);\n");
+
+  // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
+  // case above.
+  {
+auto Style = getGoogleStyle();
+Style.BinPackArguments = false;
+verifyFormat("SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ "  //\n"
+ "},\n"
+ "b);\n",
+ Style);
+  }
+  {
+verifyFormat("SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ "  //\n"
+ "},\n"
+ "b);\n");
+  }
+
+  // A lambda with a very long line forces arg0 to be pushed out irrespective of
+  // the BinPackArguments value (as long as the code is wide enough).
+  verifyFormat("something->SomeFu

[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.

2018-10-19 Thread Neeraj K. Singh via Phabricator via cfe-commits
neerajksingh created this revision.
neerajksingh added reviewers: rnk, hans.

The clang-cl driver disables access to command line options outside of the
"Core" and "CLOption" sets of command line arguments. This filtering makes it
impossible to pass arguments that are interpreted by the clang driver and not
by either 'cc1' (the frontend) or the LLVM backend.

An example driver-level flag is the '-fno-slp-vectorize' flag, which is
processed by the driver in Clang::ConstructJob and used to set the cc1 flag
"-vectorize-slp". There is no negative cc1 flag or -mllvm flag, so it is not
currently possible to disable the SLP vectorizer from the clang-cl driver.

This change introduces the "/Xdriver:" argument that is available when the
driver mode is set to CL compatibility. This option works similarly to the
"-Xclang" option, except that the option values are processed by the clang
driver rather than by 'cc1'. An example usage is:

  clang-cl /Xdriver:-fno-slp-vectorize /O2 test.c

Another example shows how "/Xdriver:" can be used to pass a flag where there is
a conflict between a clang-cl compat option and an overlapping clang driver
option:

  clang-cl /MD /Xdriver:-MD /Xdriver:-MF /Xdriver:test_dep_file.dep test.c

In the previous example, the unprefixed /MD selects the DLL version of the msvc
CRT, while the prefixed -MD flag and the -MF flags are used to create a make
dependency file for included headers.

One note about flag ordering: the /Xdriver: flags are concatenated to the end of
the argument list, so in cases where the last flag wins, the /Xdriver: flags
will be chosen regardless of their order relative to other flags on the driver
command line.


Repository:
  rC Clang

https://reviews.llvm.org/D53457

Files:
  include/clang/Driver/CLCompatOptions.td
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/cl-options.c

Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,17 @@
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 
+// Accept clang options under the /Xdriver: flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOXDRIVER %s
+// NOXDRIVER: "--dependent-lib=libcmt"
+// NOXDRIVER-SAME: "-vectorize-slp"
+// NOXDRIVER-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /Xdriver:-fno-slp-vectorize /Xdriver:-MD /Xdriver:-MF /Xdriver:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=XDRIVER %s
+// XDRIVER: "--dependent-lib=msvcrt"
+// XDRIVER-SAME: "-dependency-file" "my_dependency_file.dep"
+// XDRIVER-NOT: "--dependent-lib=libcmt"
+// XDRIVER-NOT: "-vectorize-slp"
 
 void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
 }
 
 InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool ForceNonCLMode,
  bool &ContainsError) {
   llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
   ContainsError = false;
 
   unsigned IncludedFlagsBitmask;
   unsigned ExcludedFlagsBitmask;
   std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
-  getIncludeExcludeOptionFlagMasks();
+  getIncludeExcludeOptionFlagMasks(ForceNonCLMode);
 
   unsigned MissingArgIndex, MissingArgCount;
   InputArgList Args =
@@ -730,7 +731,7 @@
   ConfigFile = CfgFileName.str();
   bool ContainErrors;
   CfgOptions = llvm::make_unique(
-  ParseArgStrings(NewCfgArgs, ContainErrors));
+  ParseArgStrings(NewCfgArgs, false, ContainErrors));
   if (ContainErrors) {
 CfgOptions.reset();
 return true;
@@ -924,7 +925,7 @@
   // Arguments specified in command line.
   bool ContainsError;
   CLOptions = llvm::make_unique(
-  ParseArgStrings(ArgList.slice(1), ContainsError));
+  ParseArgStrings(ArgList.slice(1), false, ContainsError));
 
   // Try parsing configuration file.
   if (!ContainsError)
@@ -934,21 +935,45 @@
   // All arguments, from both config file and command line.
   InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
   : std::move(*CLOptions));
-  if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
-  if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+  auto appendOneArg = [&Args](const Arg *Opt, const Arg *BaseArg) {
   unsigned Index = Args.MakeIndex(Opt->getSpelling());
-  const Arg *BaseArg = &Opt->getBaseArg();
-  if (BaseArg == Opt)
-BaseArg = nullptr;
   Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
  Index, BaseArg);
   Copy->getValues() = Opt->getValues();
   if (Opt->isClaimed())
 Copy->claim();
   Args.append(Copy);
+  };
+
+  if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+   

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

2018-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:711
+
+  /// Evaluate as a constant expression, as per C++11-and-later constexpr
+  /// rules. Stop if we find that the expression is not a constant

jyknight wrote:
> rsmith wrote:
> > This is not the intent. The evaluator uses the rules of the current 
> > language mode. However, it doesn't enforce the C and C++98 syntactic 
> > restrictions on what can appear in a constant expression, because it turns 
> > out to be cleaner to check those separately rather than to interleave them 
> > with evaluation.
> > 
> > We should probably document this as evaluating following the evaluation 
> > (but not syntactic) constant expression rules of the current language mode.
> I'm not really sure what you mean to distinguish between C/C++98 "evaluation 
> constant expression rules" and the definition of a C/C++98 ICE?
> 
> Perhaps you mean that if evaluation succeeds, it will have produced a value 
> consistent with the rules of the current language, but that it may not 
> produce the diagnostics in all cases it ought to?
> 
> AFAICT, the evaluator is designed to emit diagnostics only for the C++11 (and 
> later) constant expression rules, as well as for inability to evaluate. The 
> diagnostic notes are usually dropped on the floor in C++98 and C modes, as 
> only "fold" is typically used there -- along with the separate CheckICE to 
> produce warnings.
> 
> I believe the only place the diagnostics were being actually emitted in 
> pre-c++11 modes is for the diagnose_if and enable_if attributes' "potential 
> constant expression" check. And there it really seems to me to be using the 
> c++11 evaluation semantics.
> 
> The existence of isCXX11ConstantExpr suggests an intent for the function to 
> provide the C++11 rules, as well, no?
> 
> 
> Perhaps you mean that if evaluation succeeds, it will have produced a value 
> consistent with the rules of the current language, but that it may not 
> produce the diagnostics in all cases it ought to?

The concern I have with this comment is that it describes a happenstance of 
implementation rather than the intent. The intent is certainly *not* that we 
will use the C++11 rules when calling this in C++98 mode, but it just so 
happens that there are no interesting differences there right now.

Looking at this another way, the fact that we only guarantee to produce "not a 
constant expression because" notes in C++11 onwards is unrelated to the 
evaluation mode. Rather, the reason for that is instead because all the 
validity / ICE checking in other language modes is based on the syntactic form 
of the expression (and is checked separately as a result).

As a concrete suggestion, how about something like:

"Evaluate as a constant expression. Stop if we find that the expression is not 
a constant expression. Note that this doesn't enforce the syntactic ICE 
requirements of C and C++98."



Comment at: clang/lib/AST/ExprConstant.cpp:952
+/// with diagnostics.
+bool keepEvaluatingAfterNote() {
   switch (EvalMode) {

jyknight wrote:
> rsmith wrote:
> > "Note" is beside the point here. We should be talking about non-constant 
> > expressions; that's what matters.
> Renamed to keepEvaluatingAfterNonConstexpr, updated comment.
Capitalize as "...NonConstExpr" instead, since this is just an abbreviation of 
the term "constant expression", not C++11's notion of "constexpr". (I'd also 
lengthen this to "...NonConstantExpr" or "...NonConstantExpression" to further 
clarify.)



Comment at: clang/lib/AST/ExprConstant.cpp:1440-1444
+  if (checkSubobject(Info, E, isa(D) ? CSK_Field : CSK_Base)) {
 Designator.addDeclUnchecked(D, Virtual);
+return true;
+  }
+  return Info.keepEvaluatingAfterNote();

jyknight wrote:
> rsmith wrote:
> > `checkSubobject` should return `false` if it produced a diagnostic that 
> > should cause evaluation to halt; this call to `keepEvaluatingAfterNote` 
> > should not be necessary. (And likewise below.)
> Are you sure?
> 
> Today, checkSubobject returns false if it sees something "invalid" -- and the 
> callers seem to depend on that -- but invalid subobject designators are still 
> allowed during evaluation in fold mode.
> 
> E.g., this is valid in C:
> `struct Foo { int x[]; }; struct Foo f; int *g = f.x;`
> 
> But, in C++, `constexpr int *g = f.x;` is an error, diagnosing 
> note_constexpr_unsupported_unsized_array.
> 
> All the LValue code, with its mutable "Result" state, and carrying of a 
> couple different "Invalid" bits off on the side, is really rather confusing!
What I mean is that you should change the code to make what I said be true, if 
necessary. It's the callee's responsibility to say whether we hit something we 
can't evaluate past. The caller should not be guessing that the only reason 
that `checkSubobject` can fail is due to a `C

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

2018-10-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

I have a vague recollection that this column info hack was added to 
disambiguate two types defined on the same line (which is something that 
happened more often than one would think because of macro expansion). Did  you 
do the git archeology to ensure that the original reason for doing it this way 
has been resolved? FWIW, I'm fine with doing this change for the Darwin 
platforms because column info is turned on by default, so this shouldn't affect 
us.


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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


[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

2018-10-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

Oh wait, this patch is just for dumping the ASTs? Can you elaborate why this 
makes it into a binary then?


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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


[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

2018-10-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

RTTI?


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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


[PATCH] D53347: [clangd] Simplify auto hover

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 170273.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Addressed review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest &Test : Tests) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  if (!AT->getDeducedType().isNull())
+DeducedType = AT->getDeducedType();
 }
 return true;
   }
@@ -640,12 +621,13 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = UnwrapReferenceOrPointer(D->getReturnType());
-const AutoType *AT = dyn_cast(T.getTypePtr());
+const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
-  DeducedType = T.getUnqualifiedType();
-} else { // auto in a trailing return type just points to a DecltypeType.
-  const DecltypeType *DT = dyn_cast(T.getTypePtr());
+  DeducedType = AT->getDeducedType();
+} else {
+  // auto in a trailing return type just points to a DecltypeType and
+  // getContainedAutoType does not unwrap it.
+  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
 }


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest &Test : Tests) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in

Re: [PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

2018-10-19 Thread David Blaikie via cfe-commits
On Fri, Oct 19, 2018 at 3:56 PM Adrian Prantl via Phabricator via
llvm-commits  wrote:

> aprantl added a comment.
>
> I have a vague recollection that this column info hack was added to
> disambiguate two types defined on the same line (which is something that
> happened more often than one would think because of macro expansion). Did
> you do the git archeology to ensure that the original reason for doing it
> this way has been resolved? FWIW, I'm fine with doing this change for the
> Darwin platforms because column info is turned on by default, so this
> shouldn't affect us.
>

It was (I think) duplicate calls (rather than types) - so that inlining
(which previously didn't produce a unique'd inlinedAt location (because we
didn't have the "unique" metadata feature)) didn't collapse the
instructions from the two inlined calls into one.


>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D38061
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C

2018-10-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.

(Looks fine with a suitably-adjusted comment.)


https://reviews.llvm.org/D52384



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


[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C

2018-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/AST/DeclBase.cpp:1704-1705
+
+  // In C, the redeclaration context for enumerators is the translation unit,
+  // so we skip through transparent contexts as well as struct/union contexts.
+  bool SkipRecords = getDeclKind() == Decl::Kind::Enum &&

Nit: "the redeclaration context for enumerators is the translation unit" is not 
entirely accurate. The point instead is that a record type is only the 
redeclaration context for the fields of that record, so if we arrive at that 
context after skipping anything else, we should skip the record as well. (The 
check for "Enum" here is a red herring in that regard, but it happens to be 
correct because enumerations are the only transparent context that can exist 
within a struct or union currently.)


https://reviews.llvm.org/D52384



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


[PATCH] D53347: [clangd] Simplify auto hover

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

In https://reviews.llvm.org/D53347#1267216, @kadircet wrote:

> LGTM, it bugs me that some part in the documentation says it doesn't go 
> through decltype(`This looks through declarators like pointer types, but not 
> through decltype or typedefs`) but since tests containing decltype didn't 
> break it looks OK to me. I would still wait for @hokein to have another look 
> as well.


The decltypes are handled separately in `VisitDecltypeTypeLoc` and typedefs are 
a decl, so they are handled even before the visitor runs. This is only about 
'auto'. So I think we're covered here.




Comment at: clangd/XRefs.cpp:592
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
-}
+auto AT = D->getType()->getContainedAutoType();
+if (AT && !AT->getDeducedType().isNull())

hokein wrote:
> Maybe just 
> 
> ```
> if (auto* DT = D->getType()->getContainedDeducedType())
>DeducedType = *DT;
> ```
> 
> ? 
It feels like explicitly unwrapping auto from the deduced type instead of 
relying the printer to do that is a bit more straightforward.

Simplified code a bit per suggestions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347



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


[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.

2018-10-19 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

I haven't started looking at the code yet.

I'm not completely convinced that we want this. So far we've used the strategy 
of promoting clang options that are also useful in clang-cl to core options, 
and if someone wants to use more clang than that, maybe clang-cl isn't the 
right driver for that use-case.

But I suppose an argument could be made for having an escape hatch from 
clang-cl if it doesn't add too much complexity to the code.

I'm not sure I'm a fan of calling it /Xdriver: though, because what does it 
mean - clang-cl is the driver, but the option refers to the clang driver. The 
natural name would of course be -Xclang but that already means something else.  
Maybe we could just call it /clang:


Repository:
  rC Clang

https://reviews.llvm.org/D53457



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


[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

2018-10-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl created this revision.
aprantl added a reviewer: vsk.

Function calls without a !dbg location inside a function that has a 
DISubprogram make it impossible to construct inline information and are 
rejected by the verifier. This patch ensures that sanitizer check function 
calls have a !dbg location, by carrying forward the location of the preceding 
instruction or by inserting an artificial location if necessary.

This fixes a crash when compiling the attached testcase with -Os.

rdar://problem/45311226


https://reviews.llvm.org/D53459

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/ubsan-check-debuglocs.cpp


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1 {{.*}}, !dbg
+
+class SourceLocation {
+public:
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool 
IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1 {{.*}}, !dbg
+
+class SourceLocation {
+public:
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53460: [X86] When checking the bits in cpu_features for function multiversioning dispatcher in the resolver, make sure all the required bits are set. Not just one of them

2018-10-19 Thread Craig Topper via Phabricator via cfe-commits
craig.topper created this revision.
craig.topper added reviewers: erichkeane, echristo.

The multiversioning code repurposed the code from __builtin_cpu_supports for 
checking if a single feature is enabled. That code essentially performed 
(_cpu_features & (1 << C)) != 0. But with the multiversioning path, the mask is 
no longer guaranteed to be a power of 2. So we return true anytime any one of 
the bits in the mask is set not just all of the bits.

The correct check is (_cpu_features & mask) == mask


https://reviews.llvm.org/D53460

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/attr-target-mv.c
  test/CodeGen/builtin-cpu-supports.c


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -14,7 +14,7 @@
 
   // CHECK: [[LOAD:%[^ ]+]] = load i32, i32* getelementptr inbounds ({ i32, 
i32, i32, [1 x i32] }, { i32, i32, i32, [1 x i32] }* @__cpu_model, i32 0, i32 
3, i32 0)
   // CHECK: [[AND:%[^ ]+]] = and i32 [[LOAD]], 256
-  // CHECK: = icmp ne i32 [[AND]], 0
+  // CHECK: = icmp eq i32 [[AND]], 256
 
   return 0;
 }
Index: test/CodeGen/attr-target-mv.c
===
--- test/CodeGen/attr-target-mv.c
+++ test/CodeGen/attr-target-mv.c
@@ -70,6 +70,22 @@
 // CHECK: ret void ()* @foo_decls.sse4.2
 // CHECK: ret void ()* @foo_decls
 
+// CHECK: define void @bar4()
+// CHECK: call void @foo_multi.ifunc()
+
+// CHECK: define void ()* @foo_multi.resolver() comdat
+// CHECK: and i32 %{{.*}}, 4352
+// CHECK: icmp eq i32 %{{.*}}, 4352
+// CHECK: ret void ()* @foo_multi.fma4_sse4.2
+// CHECK: icmp eq i32 %{{.*}}, 12
+// CHECK: and i32 %{{.*}}, 4352
+// CHECK: icmp eq i32 %{{.*}}, 4352
+// CHECK: ret void ()* @foo_multi.arch_ivybridge_fma4_sse4.2
+// CHECK: and i32 %{{.*}}, 768
+// CHECK: icmp eq i32 %{{.*}}, 768
+// CHECK: ret void ()* @foo_multi.avx_sse4.2
+// CHECK: ret void ()* @foo_multi
+
 // CHECK: declare i32 @foo.arch_sandybridge()
 
 // CHECK: define available_externally i32 @foo_inline.sse4.2()
@@ -88,4 +104,3 @@
 // CHECK: define available_externally void @foo_multi.avx_sse4.2()
 // CHECK: define available_externally void @foo_multi.fma4_sse4.2()
 // CHECK: define available_externally void 
@foo_multi.arch_ivybridge_fma4_sse4.2()
-
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9129,9 +9129,9 @@
   Builder.CreateAlignedLoad(CpuFeatures, CharUnits::fromQuantity(4));
 
   // Check the value of the bit corresponding to the feature requested.
-  Value *Bitset = Builder.CreateAnd(
-  Features, llvm::ConstantInt::get(Int32Ty, FeaturesMask));
-  return Builder.CreateICmpNE(Bitset, llvm::ConstantInt::get(Int32Ty, 0));
+  Value *Mask = Builder.getInt32(FeaturesMask);
+  Value *Bitset = Builder.CreateAnd(Features, Mask);
+  return Builder.CreateICmpEQ(Bitset, Mask);
 }
 
 Value *CodeGenFunction::EmitX86CpuInit() {


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -14,7 +14,7 @@
 
   // CHECK: [[LOAD:%[^ ]+]] = load i32, i32* getelementptr inbounds ({ i32, i32, i32, [1 x i32] }, { i32, i32, i32, [1 x i32] }* @__cpu_model, i32 0, i32 3, i32 0)
   // CHECK: [[AND:%[^ ]+]] = and i32 [[LOAD]], 256
-  // CHECK: = icmp ne i32 [[AND]], 0
+  // CHECK: = icmp eq i32 [[AND]], 256
 
   return 0;
 }
Index: test/CodeGen/attr-target-mv.c
===
--- test/CodeGen/attr-target-mv.c
+++ test/CodeGen/attr-target-mv.c
@@ -70,6 +70,22 @@
 // CHECK: ret void ()* @foo_decls.sse4.2
 // CHECK: ret void ()* @foo_decls
 
+// CHECK: define void @bar4()
+// CHECK: call void @foo_multi.ifunc()
+
+// CHECK: define void ()* @foo_multi.resolver() comdat
+// CHECK: and i32 %{{.*}}, 4352
+// CHECK: icmp eq i32 %{{.*}}, 4352
+// CHECK: ret void ()* @foo_multi.fma4_sse4.2
+// CHECK: icmp eq i32 %{{.*}}, 12
+// CHECK: and i32 %{{.*}}, 4352
+// CHECK: icmp eq i32 %{{.*}}, 4352
+// CHECK: ret void ()* @foo_multi.arch_ivybridge_fma4_sse4.2
+// CHECK: and i32 %{{.*}}, 768
+// CHECK: icmp eq i32 %{{.*}}, 768
+// CHECK: ret void ()* @foo_multi.avx_sse4.2
+// CHECK: ret void ()* @foo_multi
+
 // CHECK: declare i32 @foo.arch_sandybridge()
 
 // CHECK: define available_externally i32 @foo_inline.sse4.2()
@@ -88,4 +104,3 @@
 // CHECK: define available_externally void @foo_multi.avx_sse4.2()
 // CHECK: define available_externally void @foo_multi.fma4_sse4.2()
 // CHECK: define available_externally void @foo_multi.arch_ivybridge_fma4_sse4.2()
-
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9129,9 +9129,9 @@
   Builder.CreateAl

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

2018-10-19 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal closed this revision.
BillyONeal added a comment.

Committed r344820


https://reviews.llvm.org/D50549



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


[PATCH] D50551: [libcxx] [test] Add missing to several tests.

2018-10-19 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal closed this revision.
BillyONeal added a comment.

Committed r344821


https://reviews.llvm.org/D50551



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


[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-10-19 Thread Ben via Phabricator via cfe-commits
bobsayshilol updated this revision to Diff 170277.
bobsayshilol retitled this revision from "[CodeGen] Fix places where the return 
type of a FunctionDecl was being used in place of the function type" to "Fix 
places where the return type of a FunctionDecl was being used in place of the 
function type".
bobsayshilol added a comment.

Add missing part of previous change to patch. Now all unit tests pass.

I'm removing [CodeGen] from the title too since this change touches more than 
just that section now.


https://reviews.llvm.org/D53263

Files:
  lib/AST/Decl.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Frontend/Rewrite/RewriteModernObjC.cpp

Index: lib/Frontend/Rewrite/RewriteModernObjC.cpp
===
--- lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -3099,10 +3099,10 @@
  SmallVectorImpl &MsgExprs,
  ObjCMethodDecl *Method) {
   // Now do the "normal" pointer to function cast.
-  QualType castType = getSimpleFunctionType(returnType, ArgTypes,
+  QualType funcType = getSimpleFunctionType(returnType, ArgTypes,
 Method ? Method->isVariadic()
: false);
-  castType = Context->getPointerType(castType);
+  QualType castType = Context->getPointerType(funcType);
 
   // build type for containing the objc_msgSend_stret object.
   static unsigned stretCount=0;
@@ -3177,7 +3177,7 @@
   // AST for __Stretn(receiver, args).s;
   IdentifierInfo *ID = &Context->Idents.get(name);
   FunctionDecl *FD = FunctionDecl::Create(*Context, TUDecl, SourceLocation(),
-  SourceLocation(), ID, castType,
+  SourceLocation(), ID, funcType,
   nullptr, SC_Extern, false, false);
   DeclRefExpr *DRE = new (Context) DeclRefExpr(FD, false, castType, VK_RValue,
SourceLocation());
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2315,11 +2315,13 @@
 FTy, GlobalInitFnName, getTypes().arrangeNullaryFunction(),
 SourceLocation());
 ASTContext &Ctx = getContext();
+QualType ReturnTy = Ctx.VoidTy;
+QualType FunctionTy = Ctx.getFunctionType(ReturnTy, llvm::None, {});
 FunctionDecl *FD = FunctionDecl::Create(
 Ctx, Ctx.getTranslationUnitDecl(), SourceLocation(), SourceLocation(),
-&Ctx.Idents.get(GlobalInitFnName), Ctx.VoidTy, nullptr, SC_Static,
+&Ctx.Idents.get(GlobalInitFnName), FunctionTy, nullptr, SC_Static,
 false, false);
-CGF.StartFunction(GlobalDecl(FD), getContext().VoidTy, GlobalInitFn,
+CGF.StartFunction(GlobalDecl(FD), ReturnTy, GlobalInitFn,
   getTypes().arrangeNullaryFunction(), FunctionArgList(),
   SourceLocation(), SourceLocation());
 
Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -385,11 +385,11 @@
   FunctionDecl *DebugFunctionDecl = nullptr;
   if (!FO.UIntPtrCastRequired) {
 FunctionProtoType::ExtProtoInfo EPI;
+QualType FunctionTy = Ctx.getFunctionType(Ctx.VoidTy, llvm::None, EPI);
 DebugFunctionDecl = FunctionDecl::Create(
 Ctx, Ctx.getTranslationUnitDecl(), FO.S->getBeginLoc(),
-SourceLocation(), DeclarationName(), Ctx.VoidTy,
-Ctx.getTrivialTypeSourceInfo(
-Ctx.getFunctionType(Ctx.VoidTy, llvm::None, EPI)),
+SourceLocation(), DeclarationName(), FunctionTy,
+Ctx.getTrivialTypeSourceInfo(FunctionTy),
 SC_Static, /*isInlineSpecified=*/false, /*hasWrittenPrototype=*/false);
   }
   for (const FieldDecl *FD : RD->fields()) {
Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -3229,29 +3229,36 @@
   ASTContext &C = getContext();
   IdentifierInfo *II
 = &CGM.getContext().Idents.get("__assign_helper_atomic_property_");
+
+  QualType ReturnTy = C.VoidTy;
+  QualType DestTy = C.getPointerType(Ty);
+  QualType SrcTy = Ty;
+  SrcTy.addConst();
+  SrcTy = C.getPointerType(SrcTy);
+
+  SmallVector ArgTys;
+  ArgTys.push_back(DestTy);
+  ArgTys.push_back(SrcTy);
+  QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {});
+
   FunctionDecl *FD = FunctionDecl::Create(C,
   C.getTranslationUnitDecl(),
   SourceLocation(

[PATCH] D53329: Generate DIFile with main program if source is not available

2018-10-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

@aprantl - yeah, not sure I have any big feels about this (nor do I fully 
understand it)

@yonghong-song - could you explain this maybe in a bit more detail. What 
behavior does this fix provide? (compared to behavior of existing working cases 
that don't hit this bug, for example)


Repository:
  rC Clang

https://reviews.llvm.org/D53329



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


[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

2018-10-19 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:2871
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);

Why shouldn't this always be line 0? A call to a check handler is always 
auto-generated.



Comment at: test/CodeGenCXX/ubsan-check-debuglocs.cpp:2
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s

Are all three sanitizers needed here to reproduce the bug? Seems like a simpler 
test would be:

```
// RUN: ... -fsanitize=null ...

int deref(int *p) { return *p; }
// CHECK-LABEL: @deref
// CHECK: __ubsan_handle_type_mismatch_v1{{.*}} !dbg 
[[ubsan_handler_loc:![0-9]+]]
// CHECK: [[ubsan_handler_loc]] = !DILocation(line: 0
```


https://reviews.llvm.org/D53459



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


[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

2018-10-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:2871
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);

vsk wrote:
> Why shouldn't this always be line 0? A call to a check handler is always 
> auto-generated.
I was thinking that it might be nice to have the source location of the 
expression with the UB in the backtrace, too — not just in the error message. 
(Which is the current behavior by accident, since no location means carry over 
the previous instruction's location). But I'm fine with just setting line 0, 
too.



Comment at: test/CodeGenCXX/ubsan-check-debuglocs.cpp:2
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s

vsk wrote:
> Are all three sanitizers needed here to reproduce the bug? Seems like a 
> simpler test would be:
> 
> ```
> // RUN: ... -fsanitize=null ...
> 
> int deref(int *p) { return *p; }
> // CHECK-LABEL: @deref
> // CHECK: __ubsan_handle_type_mismatch_v1{{.*}} !dbg 
> [[ubsan_handler_loc:![0-9]+]]
> // CHECK: [[ubsan_handler_loc]] = !DILocation(line: 0
> ```
Probably not for the testcase; only to trigger the subsequent crash.


https://reviews.llvm.org/D53459



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


[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-10-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

My take on the out-of-alpha checklist:

- The checker should be evaluated on a large codebase in order to discover 
crashes and false positives specific to the checker. It should demonstrate the 
expected reasonably good results on it, with the stress on having as little 
false positives as possible.
  - The codebase should contain patterns that the checker checks. It should 
ideally also contain actual bugs, but it is fine if the checker was inspired by 
a one-of-a-kind devastating bug while there aren't many other instances of this 
bug around.

- Warning and note messages should be clear and easy to understand, even if a 
bit long.
  - Messages should start with a capital letter (unlike Clang warnings!) and 
should not end with `.`.
  - Articles are usually omitted, eg. `Dereference of a null pointer` -> 
`Dereference of null pointer`.

- Static Analyzer's checker API saves you from a lot of work, but it still 
forces you into a certain amount of boilerplate code when it comes to managing 
entities specific to your checker:
  - Almost all path-sensitive checkers need their own `BugReporterVisitor` in 
order to highlight interesting events along the path - or at least a call to 
`trackNullOrUndefValue`. //(need to wait for https://reviews.llvm.org/D52758)//
- For example, SimpleStreamChecker should highlight the event of opening 
the file when reporting a file descriptor leak.
  - If the checker tracks anything in the program state, it needs to implement 
the `checkDeadSymbols` callback to clean the state up.

- Unless there's a strong indication that the user is willing to opt-in to a 
stricter checking, obvious false positives that could be caused by the checker 
itself should be fixed. For example:
  - If a checker tracks state of mutable objects enumerated by symbols, 
escaping symbols into unfamiliar functions must invalidate the state - for 
example, through `checkPointerEscape`.
- For example, SimpleStreamChecker should drop its knowledge about an open 
file descriptor when it is passed into a function that may potentially close 
the descriptor. At the same time, it is fine to not escape closed descriptors.
  - If the checker is intentionally leaving a room for false positives, i.e., 
it checks for a coding standard that isn't critical to follow when it comes to 
formal correctness of the program, then there should be a cheap explicit way to 
suppress the warnings when the violation of the coding standard is intentional.
  - Fuzzy matching of API function or type names is generally dangerous when it 
may lead to false positives, fine otherwise.
- For example, a checker for LLVM `isa<>` API shouldn't match it as `isa*`, 
so that it didn't accidentally make assumptions on how `isalpha` works.
- It is perfectly fine to have some fuzziness for initial experiments in 
alpha - that's a great way to figure out which APIs you want to cover.

My take on the everyday checklist:

- Checkers are encouraged to actively participate in the analysis by sharing 
its knowledge about the program state with the rest of the analyzer, but they 
should not be disrupting the analysis unnecessarily:
  - If a checker splits program state, this must be based on knowledge that the 
newly appearing branches are definitely possible and worth exploring from the 
user's perspective. Otherwise the state split should be delayed until there's 
an indication that one of the paths is taken, or one of the paths needs to be 
dropped entirely.
- For example, it is fine to eagerly split paths while modeling 
`isalpha(x)` as long as `x` is constrained accordingly on each path. At the 
same time, it is not a good idea to split paths over the return value of 
`printf()` while modeling the call because nobody ever checks for errors in 
`printf`; at best, it'd just double the remaining analysis time.
- Caution is advised when using `CheckerContext::generateNonFatalErrorNode` 
because it generates an independent transition, much like `addTransition`. It 
is easy to accidentally split paths while using it.
  - Ideally, try to structure the code so that it was obvious that every 
`addTransition` or `generateNonFatalErrorNode` (or sequence of such if the 
split is intended) is immediately followed by return from the checker callback.
  - Multiple implementations of `evalCall` in different checkers should not 
conflict.
  - When implementing `evalAssume`, the checker should always return a non-null 
state for either the true assumption or the false assumption (or both).
  - Checkers shall not mutate values of expressions, i.e. use the 
`ProgramState::BindExpr` API, unless they are fully responsible for computing 
the value. Under no circumstances should they change non-`Unknown` values of 
expressions.
- Currently the only valid use case for this API in checkers is to model 
the return value in the `evalCall` callback.
- If expression values are incorrect, `ExprEngine` needs to be fixed 
in

  1   2   >