date:20190204

[PATCH] D57507: [clang] Add getCommentHandler to PreambleCallbacks

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 184998.
kadircet marked an inline comment as done.
kadircet added a comment.
Herald added a project: clang.

- Update comment


Repository:
  rC Clang

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

https://reviews.llvm.org/D57507

Files:
  include/clang/Frontend/PrecompiledPreamble.h
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -18,6 +18,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/ADT/StringExtras.h"
@@ -346,6 +347,8 @@
   Callbacks.createPPCallbacks();
   if (DelegatedPPCallbacks)
 Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
+  if (auto CommentHandler = Callbacks.getCommentHandler())
+Clang->getPreprocessor().addCommentHandler(CommentHandler);
 
   Act->Execute();
 
@@ -742,6 +745,7 @@
 std::unique_ptr PreambleCallbacks::createPPCallbacks() {
   return nullptr;
 }
+CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
 static llvm::ManagedStatic 
BuildPreambleErrCategory;
 
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -283,6 +283,8 @@
   /// Creates wrapper class for PPCallbacks so we can also process information
   /// about includes that are inside of a preamble
   virtual std::unique_ptr createPPCallbacks();
+  /// The returned CommentHandler will be added to the preprocessor if not 
null.
+  virtual CommentHandler *getCommentHandler();
 };
 
 enum class BuildPreambleError {


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -18,6 +18,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/ADT/StringExtras.h"
@@ -346,6 +347,8 @@
   Callbacks.createPPCallbacks();
   if (DelegatedPPCallbacks)
 Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
+  if (auto CommentHandler = Callbacks.getCommentHandler())
+Clang->getPreprocessor().addCommentHandler(CommentHandler);
 
   Act->Execute();
 
@@ -742,6 +745,7 @@
 std::unique_ptr PreambleCallbacks::createPPCallbacks() {
   return nullptr;
 }
+CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
 static llvm::ManagedStatic BuildPreambleErrCategory;
 
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -283,6 +283,8 @@
   /// Creates wrapper class for PPCallbacks so we can also process information
   /// about includes that are inside of a preamble
   virtual std::unique_ptr createPPCallbacks();
+  /// The returned CommentHandler will be added to the preprocessor if not null.
+  virtual CommentHandler *getCommentHandler();
 };
 
 enum class BuildPreambleError {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r353026 - [clangd] Update vscode dependencies

2019-02-04 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Feb  4 01:20:41 2019
New Revision: 353026

URL: http://llvm.org/viewvc/llvm-project?rev=353026&view=rev
Log:
[clangd] Update vscode dependencies

This allows us to use latest LSP v3.14.0 (for go-to-declaration feature).

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=353026&r1=353025&r2=353026&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Feb  
4 01:20:41 2019
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {
-"vscode": "^1.27.0"
+"vscode": "^1.30.0"
 },
 "categories": [
 "Programming Languages",
@@ -32,8 +32,8 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^5.1.0",
-"vscode-languageserver": "^5.1.0"
+"vscode-languageclient": "^5.2.0",
+"vscode-languageserver": "^5.2.0"
 },
 "devDependencies": {
 "typescript": "^2.0.3",


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

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

2019-02-04 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.
Herald added a project: clang.

Ping. I'm not quite sure who is the best suited to review this..


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112



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

[clang-tools-extra] r353027 - [clangd] Bump vscode-clangd v0.0.10

2019-02-04 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Feb  4 01:26:42 2019
New Revision: 353027

URL: http://llvm.org/viewvc/llvm-project?rev=353027&view=rev
Log:
[clangd] Bump vscode-clangd v0.0.10

CHANGELOG:
- cleanup filestatus caches when clangd crashes
- extension workwith LSP v3.14.0, support go-to-declaration feature

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=353027&r1=353026&r2=353027&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Feb  
4 01:26:42 2019
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.9",
+"version": "0.0.10",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {


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

r353030 - [clang] Add getCommentHandler to PreambleCallbacks

2019-02-04 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Mon Feb  4 01:42:33 2019
New Revision: 353030

URL: http://llvm.org/viewvc/llvm-project?rev=353030&view=rev
Log:
[clang] Add getCommentHandler to PreambleCallbacks

Summary:
Enables users to add comment handlers to preprocessor when building
preambles.

Reviewers: ilya-biryukov, ioeric

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp

Modified: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h?rev=353030&r1=353029&r2=353030&view=diff
==
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h (original)
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h Mon Feb  4 01:42:33 
2019
@@ -283,6 +283,8 @@ public:
   /// Creates wrapper class for PPCallbacks so we can also process information
   /// about includes that are inside of a preamble
   virtual std::unique_ptr createPPCallbacks();
+  /// The returned CommentHandler will be added to the preprocessor if not 
null.
+  virtual CommentHandler *getCommentHandler();
 };
 
 enum class BuildPreambleError {

Modified: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp?rev=353030&r1=353029&r2=353030&view=diff
==
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp (original)
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp Mon Feb  4 01:42:33 2019
@@ -18,6 +18,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/ADT/StringExtras.h"
@@ -346,6 +347,8 @@ llvm::ErrorOr Preco
   Callbacks.createPPCallbacks();
   if (DelegatedPPCallbacks)
 Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
+  if (auto CommentHandler = Callbacks.getCommentHandler())
+Clang->getPreprocessor().addCommentHandler(CommentHandler);
 
   Act->Execute();
 
@@ -742,6 +745,7 @@ void PreambleCallbacks::HandleTopLevelDe
 std::unique_ptr PreambleCallbacks::createPPCallbacks() {
   return nullptr;
 }
+CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
 static llvm::ManagedStatic 
BuildPreambleErrCategory;
 


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

[PATCH] D57507: [clang] Add getCommentHandler to PreambleCallbacks

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353030: [clang] Add getCommentHandler to PreambleCallbacks 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57507

Files:
  cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
  cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp


Index: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
===
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
@@ -283,6 +283,8 @@
   /// Creates wrapper class for PPCallbacks so we can also process information
   /// about includes that are inside of a preamble
   virtual std::unique_ptr createPPCallbacks();
+  /// The returned CommentHandler will be added to the preprocessor if not 
null.
+  virtual CommentHandler *getCommentHandler();
 };
 
 enum class BuildPreambleError {
Index: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
===
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
@@ -18,6 +18,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/ADT/StringExtras.h"
@@ -346,6 +347,8 @@
   Callbacks.createPPCallbacks();
   if (DelegatedPPCallbacks)
 Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
+  if (auto CommentHandler = Callbacks.getCommentHandler())
+Clang->getPreprocessor().addCommentHandler(CommentHandler);
 
   Act->Execute();
 
@@ -742,6 +745,7 @@
 std::unique_ptr PreambleCallbacks::createPPCallbacks() {
   return nullptr;
 }
+CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
 static llvm::ManagedStatic 
BuildPreambleErrCategory;
 


Index: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
===
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
@@ -283,6 +283,8 @@
   /// Creates wrapper class for PPCallbacks so we can also process information
   /// about includes that are inside of a preamble
   virtual std::unique_ptr createPPCallbacks();
+  /// The returned CommentHandler will be added to the preprocessor if not null.
+  virtual CommentHandler *getCommentHandler();
 };
 
 enum class BuildPreambleError {
Index: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
===
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
@@ -18,6 +18,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/ADT/StringExtras.h"
@@ -346,6 +347,8 @@
   Callbacks.createPPCallbacks();
   if (DelegatedPPCallbacks)
 Clang->getPreprocessor().addPPCallbacks(std::move(DelegatedPPCallbacks));
+  if (auto CommentHandler = Callbacks.getCommentHandler())
+Clang->getPreprocessor().addCommentHandler(CommentHandler);
 
   Act->Execute();
 
@@ -742,6 +745,7 @@
 std::unique_ptr PreambleCallbacks::createPPCallbacks() {
   return nullptr;
 }
+CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
 static llvm::ManagedStatic BuildPreambleErrCategory;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r352307 - Remove Expr sugar decorating the CXXUuidofExpr node.

2019-02-04 Thread Hans Wennborg via cfe-commits

Merged to 8.0 in r353031.

On Sun, Jan 27, 2019 at 8:23 AM Bill Wendling via cfe-commits
 wrote:
>
> Author: void
> Date: Sat Jan 26 23:24:03 2019
> New Revision: 352307
>
> URL: http://llvm.org/viewvc/llvm-project?rev=352307&view=rev
> Log:
> Remove Expr sugar decorating the CXXUuidofExpr node.
>
> Summary: Sugar, like ConstantExpr, causes an infinite expansion of the 
> template object.
>
> Reviewers: rsmith, aaron.ballman
>
> Reviewed By: aaron.ballman
>
> Subscribers: riccibruno, aaron.ballman, cfe-commits, tzik, rnk
>
> Differential Revision: https://reviews.llvm.org/D57114
>
> Added:
> cfe/trunk/test/SemaCXX/PR40395.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaTemplate.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=352307&r1=352306&r2=352307&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Sat Jan 26 23:24:03 2019
> @@ -6308,7 +6308,7 @@ ExprResult Sema::CheckTemplateArgument(N
>// -- a predefined __func__ variable
>if (auto *E = Value.getLValueBase().dyn_cast()) {
>  if (isa(E)) {
> -  Converted = TemplateArgument(ArgResult.get());
> +  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
>break;
>  }
>  Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
>
> Added: cfe/trunk/test/SemaCXX/PR40395.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR40395.cpp?rev=352307&view=auto
> ==
> --- cfe/trunk/test/SemaCXX/PR40395.cpp (added)
> +++ cfe/trunk/test/SemaCXX/PR40395.cpp Sat Jan 26 23:24:03 2019
> @@ -0,0 +1,16 @@
> +// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 
> -verify %s
> +// expected-no-diagnostics
> +
> +// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
> +// expand.
> +struct _GUID {};
> +struct __declspec(uuid("{----}")) B {};
> +
> +template 
> +struct A {
> +  virtual void baz() { A(); }
> +};
> +
> +void f() {
> +  A<&__uuidof(B)>();
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57604: [clang-format] Fix breaking of qualified operator

2019-02-04 Thread Krasimir Georgiev via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353033: [clang-format] Fix breaking of qualified operator 
(authored by krasimir, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57604?vs=184791&id=185009#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57604

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2249,6 +2249,9 @@
   return 500;
   }
 
+  if (Left.is(tok::coloncolon) ||
+  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto))
+return 500;
   if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName) ||
   Right.is(tok::kw_operator)) {
 if (Line.startsWith(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
@@ -2267,9 +2270,6 @@
 return 160;
   if (Left.is(TT_CastRParen))
 return 100;
-  if (Left.is(tok::coloncolon) ||
-  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto))
-return 500;
   if (Left.isOneOf(tok::kw_class, tok::kw_struct))
 return 5000;
   if (Left.is(tok::comment))
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4183,6 +4183,18 @@
Style);
 }
 
+TEST_F(FormatTest, DontBreakBeforeQualifiedOperator) {
+  // Regression test for https://bugs.llvm.org/show_bug.cgi?id=40516:
+  // Prefer keeping `::` followed by `operator` together.
+  EXPECT_EQ("const ::bbb &\n"
+"c::operator++() {\n"
+"  stuff();\n"
+"}",
+format("const ::bbb\n"
+   "&c::operator++() { stuff(); }",
+   getLLVMStyleWithColumns(40)));
+}
+
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
   verifyFormat("struct S {\n"


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2249,6 +2249,9 @@
   return 500;
   }
 
+  if (Left.is(tok::coloncolon) ||
+  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto))
+return 500;
   if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName) ||
   Right.is(tok::kw_operator)) {
 if (Line.startsWith(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
@@ -2267,9 +2270,6 @@
 return 160;
   if (Left.is(TT_CastRParen))
 return 100;
-  if (Left.is(tok::coloncolon) ||
-  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto))
-return 500;
   if (Left.isOneOf(tok::kw_class, tok::kw_struct))
 return 5000;
   if (Left.is(tok::comment))
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4183,6 +4183,18 @@
Style);
 }
 
+TEST_F(FormatTest, DontBreakBeforeQualifiedOperator) {
+  // Regression test for https://bugs.llvm.org/show_bug.cgi?id=40516:
+  // Prefer keeping `::` followed by `operator` together.
+  EXPECT_EQ("const ::bbb &\n"
+"c::operator++() {\n"
+"  stuff();\n"
+"}",
+format("const ::bbb\n"
+   "&c::operator++() { stuff(); }",
+   getLLVMStyleWithColumns(40)));
+}
+
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
   verifyFormat("struct S {\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353033 - [clang-format] Fix breaking of qualified operator

2019-02-04 Thread Krasimir Georgiev via cfe-commits

Author: krasimir
Date: Mon Feb  4 01:56:16 2019
New Revision: 353033

URL: http://llvm.org/viewvc/llvm-project?rev=353033&view=rev
Log:
[clang-format] Fix breaking of qualified operator

Summary:
From https://bugs.llvm.org/show_bug.cgi?id=40516
```
$ cat a.cpp
const NamespaceName::VeryLongClassName 
&NamespaceName::VeryLongClassName::myFunction() {
  // do stuff
}

const NamespaceName::VeryLongClassName 
&NamespaceName::VeryLongClassName::operator++() {
  // do stuff
}
$ ~/ll/build/opt/bin/clang-format -style=LLVM a.cpp
const NamespaceName::VeryLongClassName &
NamespaceName::VeryLongClassName::myFunction() {
  // do stuff
}

const NamespaceName::VeryLongClassName &NamespaceName::VeryLongClassName::
operator++() {
  // do stuff
}
```
What was happening is that the split penalty before `operator` was being set to
a smaller value by a prior if block. Moved checks around to fix this and added a
regression test.

Reviewers: djasper

Reviewed By: djasper

Tags: #clang

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=353033&r1=353032&r2=353033&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Feb  4 01:56:16 2019
@@ -2249,6 +2249,9 @@ unsigned TokenAnnotator::splitPenalty(co
   return 500;
   }
 
+  if (Left.is(tok::coloncolon) ||
+  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto))
+return 500;
   if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName) ||
   Right.is(tok::kw_operator)) {
 if (Line.startsWith(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
@@ -2267,9 +2270,6 @@ unsigned TokenAnnotator::splitPenalty(co
 return 160;
   if (Left.is(TT_CastRParen))
 return 100;
-  if (Left.is(tok::coloncolon) ||
-  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto))
-return 500;
   if (Left.isOneOf(tok::kw_class, tok::kw_struct))
 return 5000;
   if (Left.is(tok::comment))

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=353033&r1=353032&r2=353033&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Feb  4 01:56:16 2019
@@ -4183,6 +4183,18 @@ TEST_F(FormatTest, BreaksFunctionDeclara
Style);
 }
 
+TEST_F(FormatTest, DontBreakBeforeQualifiedOperator) {
+  // Regression test for https://bugs.llvm.org/show_bug.cgi?id=40516:
+  // Prefer keeping `::` followed by `operator` together.
+  EXPECT_EQ("const ::bbb &\n"
+"c::operator++() {\n"
+"  stuff();\n"
+"}",
+format("const ::bbb\n"
+   "&c::operator++() { stuff(); }",
+   getLLVMStyleWithColumns(40)));
+}
+
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
   verifyFormat("struct S {\n"


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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: JonasToth, alexfh, hokein, aaron.ballman, 
Eugene.Zelenko.
MyDeveloperDay added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

bugprone-argument-comment only supports identifying those comments which do not 
match the function parameter name

This revision add 3 options to adding missing argument comments to literals 
(granularity on type is added to control verbosity of fixit)

  CheckOptions:
- key: bugprone-argument-comment.AddCommentsToBoolLiterals
  value:   '1'
- key: bugprone-argument-comment.AddCommentsToFloatLiterals
  value:   '1'
- key: bugprone-argument-comment.AddCommentsToIntegerLiterals
  value:   '1'

After applying these options, literal arguments will be preceded with 
/*paramter_name=*/


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57674

Files:
  clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tidy/bugprone/ArgumentCommentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-argument-comment.rst
  test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: AddCommentsToBoolLiterals, value: 1},{key: AddCommentsToIntegerLiterals, value: 1},{key: AddCommentsToFloatLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+};
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+}
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument '_with_underscores_' [bugprone-argument-comment]
+  // CHECK-FIXES: f(/*_with_underscores_=*/false);
+
+  f(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument
+  // CHECK-FIXES: f(/*_with_underscores_=*/true);
+}
Index: docs/clang-tidy/checks/bugprone-argument-comment.rst
===
--- docs/clang-tidy/checks/bugprone-argument-comment.rst
+++ docs/clang-tidy/checks/bugprone-argument-comment.rst
@@ -27,3 +27,18 @@
When ze

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: docs/ReleaseNotes.rst:89
 
+- The :doc:`bugprone-argument-comment
+  ` now supports 

note to self, I will reorganize this alphabetically, on next revision


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57674



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

An example of this checker run over clang-tidy/modernize is given here D57675: 
[clang-tidy] DO NOT SUBMIT.. example diff of applying 
bugprone-argument-comments with AddCommentsToXXXLiterals options 



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57674



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

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

There guys set a high barrier of entry, you need to at least start by adding 
some unit tests (or someone will break your code in the future), if you think 
it was a bug it might be worth logging that in bugzilla too!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 185014.
MyDeveloperDay added a comment.

Fix release note alphabetic order
Add examples of Fixit into the documentation


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

https://reviews.llvm.org/D57674

Files:
  clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tidy/bugprone/ArgumentCommentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-argument-comment.rst
  test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: AddCommentsToBoolLiterals, value: 1},{key: AddCommentsToIntegerLiterals, value: 1},{key: AddCommentsToFloatLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+};
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+}
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument '_with_underscores_' [bugprone-argument-comment]
+  // CHECK-FIXES: f(/*_with_underscores_=*/false);
+
+  f(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument
+  // CHECK-FIXES: f(/*_with_underscores_=*/true);
+}
Index: docs/clang-tidy/checks/bugprone-argument-comment.rst
===
--- docs/clang-tidy/checks/bugprone-argument-comment.rst
+++ docs/clang-tidy/checks/bugprone-argument-comment.rst
@@ -27,3 +27,66 @@
When zero (default value), the check will ignore leading and trailing
underscores and case when comparing names -- otherwise they are taken into
account.
+
+.. option:: AddCommentsToBoolLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*parameter_name=*/`` right before the boolean literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(bool turn_key,bool press_button);
+
+  foo(true,false);
+
+After:
+
+.. code-block:: c++
+
+  void foo(bool turn_key,bool press_button);
+
+  foo(/*turn_key=*/true,/*press_button*/false);
+
+.. option:: AddCommentsToIntegerLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*parameter_name=*/`` right before the i

[PATCH] D57353: [clang-tidy] Add the abseil-duration-double-conversion check

2019-02-04 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

The code looks good, but I have a concern about the check name -- `double` 
seems a confusing word, see my comment.




Comment at: clang-tidy/abseil/DurationDoubleConversionCheck.cpp:27
+
+  for (const auto &s : Scales) {
+std::string DurationFactory = (llvm::Twine("::absl::") + s).str();

nit: `s` => `S`; might be inline the `Scales`?



Comment at: clang-tidy/abseil/DurationDoubleConversionCheck.cpp:51
+
+  diag(OuterCall->getBeginLoc(), "remove double conversion of absl::Duration")
+  << FixItHint::CreateReplacement(

The `double` word may cause confusion easily -- at the first glance, I thought 
that it means convert the `absl::Duration` to the `double` type, but it turned 
out not.. How about using `abseil-duration-necessary-conversion`?


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

https://reviews.llvm.org/D57353



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

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Eric Liu via Phabricator via cfe-commits

ioeric added inline comments.
Herald added a project: clang.



Comment at: clangd/ClangdUnit.cpp:100
+  : File(File), ParsedCallback(ParsedCallback),
+IWYUHandler(collectIWYUHeaderMaps(&CanonIncludes)) {}
 

Does this have to own the `IWYUHandler`? Could we create one when 
`getCommentHandler` is called?



Comment at: clangd/ClangdUnit.cpp:125
 
+  CommentHandler *getCommentHandler() override { return IWYUHandler.get(); }
+

This looks like a memory leak? `release()`?



Comment at: clangd/ClangdUnit.cpp:338
 
+  // Do we really care about IWYU pragmas inside the file rather than the
+  // preamble?

I think so? A header file (with IWYU pragma) can also be the main file.



Comment at: unittests/clangd/CodeCompleteTests.cpp:2287
 
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+  MockFSProvider FS;

nit: maybe put this close to test cases that involve preamble. e.g. 
IndexSuppressesPreambleCompletions



Comment at: unittests/clangd/CodeCompleteTests.cpp:2301
+  )cpp";
+  constexpr const char *FileContent(R"cpp(
+#include "foo_header.h"

nit: just use `std::string` for readability.



Comment at: unittests/clangd/FileIndexTests.cpp:207
+  M, "f",
+  "// IWYU pragma: private, include \nclass string 
{};");
 

Are we sure the comment will be included in the preamble if there is no 
#include block?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508



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

[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space

2019-02-04 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia updated this revision to Diff 185024.
Anastasia added a comment.

- Renamed variables and made other changes to improve readability.


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

https://reviews.llvm.org/D57524

Files:
  lib/Sema/SemaInit.cpp
  test/SemaOpenCLCXX/address-space-of-this.cl


Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic 
-verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4670,19 +4670,23 @@
 //   applied.
 // Postpone address space conversions to after the temporary 
materialization
 // conversion to allow creating temporaries in the alloca address space.
-auto AS1 = T1Quals.getAddressSpace();
-auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
-QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-if (T1Quals != T2Quals)
+auto T1QualsIgnoreAS = T1Quals;
+auto T2QualsIgnoreAS = T2Quals;
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+  T1QualsIgnoreAS.removeAddressSpace();
+  T2QualsIgnoreAS.removeAddressSpace();
+}
+QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1QualsIgnoreAS);
+if (T1QualsIgnoreAS != T2QualsIgnoreAS)
   Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
 Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
 ValueKind = isLValueRef ? VK_LValue : VK_XValue;
-if (AS1 != AS2) {
-  T1Quals.addAddressSpace(AS1);
-  QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-  Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+// Add addr space conversion if required.
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+  auto T4Quals = cv1T4.getQualifiers();
+  T4Quals.addAddressSpace(T1Quals.getAddressSpace());
+  QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals);
+  Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind);
 }
 
 //   In any case, the reference is bound to the resulting glvalue (or to


Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4670,19 +4670,23 @@
 //   applied.
 // Postpone address space conversions to after the temporary materialization
 // conversion to allow creating temporaries in the alloca address space.
-auto AS1 = T1Quals.getAddressSpace();
-auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
-QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-if (T1Quals != T2Quals)
+auto T1QualsIgnoreAS = T1Quals;
+auto T2QualsIgnoreAS = T2Quals;
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+  T1QualsIgnoreAS.removeAddressSpace();
+  T2QualsIgnoreAS.removeAddressSpace();
+}
+QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1QualsIgnoreAS);
+if (T1QualsIgnoreAS != T2QualsIgnoreAS)
   Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
 Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
 ValueKind = isLValueRef ? VK_LValue : VK_XValue;
-if (AS1 != AS2) {
-  T1Quals.addAddressSpace(AS1);
-  QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-  Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+// Add addr space conversion if required.
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+  auto T4Quals = cv1T4.getQualifiers();
+  T4Quals.addAddressSpace(T1Quals.getAddressSpace());
+  QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals);
+  Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind);
 }
 
 //   In any case, the reference is bound to the resulting glvalue (or to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clangd/ClangdUnit.cpp:100
+  : File(File), ParsedCallback(ParsedCallback),
+IWYUHandler(collectIWYUHeaderMaps(&CanonIncludes)) {}
 

ioeric wrote:
> Does this have to own the `IWYUHandler`? Could we create one when 
> `getCommentHandler` is called?
The class needs to own it, since preprocessor doesn't take the ownership. But 
we can move the creation to getcommenthandler.



Comment at: clangd/ClangdUnit.cpp:125
 
+  CommentHandler *getCommentHandler() override { return IWYUHandler.get(); }
+

ioeric wrote:
> This looks like a memory leak? `release()`?
Preprocessor doesn't take the ownership, so this seems to be correct ?



Comment at: clangd/ClangdUnit.cpp:338
 
+  // Do we really care about IWYU pragmas inside the file rather than the
+  // preamble?

ioeric wrote:
> I think so? A header file (with IWYU pragma) can also be the main file.
Also call `addSystemHeadersMapping` in case there is no preamble.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508



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

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 185030.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Address comments.
- Call addSystemHeaderMappings when we are building ast without preamble.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -59,13 +59,15 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
+AST.getCanonicalIncludes());
 }
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
   auto Idx = llvm::make_unique(/*UseDex=*/true);
-  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
+  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+  AST.getCanonicalIncludes());
   Idx->updateMain(Filename, AST);
   return std::move(Idx);
 }
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -12,6 +12,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -147,8 +148,8 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.updatePreamble(File.Filename, AST.getASTContext(),
-   AST.getPreprocessorPtr());
+  M.updatePreamble(File.Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+   AST.getCanonicalIncludes());
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
@@ -199,13 +200,16 @@
QName("X::f")));
 }
 
-TEST(FileIndexTest, NoIncludeCollected) {
+TEST(FileIndexTest, IncludeCollected) {
   FileIndex M;
-  update(M, "f", "class string {};");
+  update(
+  M, "f",
+  "// IWYU pragma: private, include \nclass string {};");
 
   auto Symbols = runFuzzyFind(M, "");
   EXPECT_THAT(Symbols, ElementsAre(_));
-  EXPECT_THAT(Symbols.begin()->IncludeHeaders, IsEmpty());
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {
@@ -270,10 +274,11 @@
   buildPreamble(
   FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
   std::make_shared(), /*StoreInMemory=*/true,
-  [&](ASTContext &Ctx, std::shared_ptr PP) {
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &CanonIncludes) {
 EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
 IndexUpdated = true;
-Index.updatePreamble(FooCpp, Ctx, std::move(PP));
+Index.updatePreamble(FooCpp, Ctx, std::move(PP), CanonIncludes);
   });
   ASSERT_TRUE(IndexUpdated);
 
@@ -358,7 +363,8 @@
   MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
   tooling::CompileCommand(), PI, std::make_shared(),
   /*StoreInMemory=*/true,
-  [&](ASTContext &Ctx, std::shared_ptr PP) {});
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &) {});
   // Build AST for main file with preamble.
   auto AST =
   ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData,
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -645,6 +645,36 @@
   EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
 }
 
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+
+  FS.Files[testPath("foo_header.h")] = R"cpp(
+struct Foo {
+   // Member doc
+   int foo();
+};
+  )cpp";
+  const std::string FileContent(R"cpp(
+#include "foo_header.h"
+int Foo::foo() {
+  return 42;
+}
+  )cpp");
+  Server.addDocument(testPath("foo_impl.cpp"), FileContent);
+  // Wait for the dynamic index being built.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(
+  complet

[PATCH] D57021: [clangd] Suggest adding missing includes for typos (like include-fixer).

2019-02-04 Thread Eric Liu via Phabricator via cfe-commits

ioeric updated this revision to Diff 185031.
ioeric marked 13 inline comments as done.
ioeric added a comment.
Herald added a project: clang.

- address review comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57021

Files:
  clangd/ClangdUnit.cpp
  clangd/IncludeFixer.cpp
  clangd/IncludeFixer.h
  clangd/SourceCode.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -30,6 +30,11 @@
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));
 }
 
+testing::Matcher WithFix(testing::Matcher FixMatcher1,
+   testing::Matcher FixMatcher2) {
+  return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
+}
+
 testing::Matcher WithNote(testing::Matcher NoteMatcher) {
   return Field(&Diag::Notes, ElementsAre(NoteMatcher));
 }
@@ -281,6 +286,26 @@
   Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty(;
 }
 
+struct SymbolWithHeader {
+  std::string QName;
+  std::string DeclaringFile;
+  std::string IncludeHeader;
+};
+
+std::unique_ptr
+buildIndexWithSymbol(llvm::ArrayRef Syms) {
+  SymbolSlab::Builder Slab;
+  for (const auto &S : Syms) {
+Symbol Sym = cls(S.QName);
+Sym.Flags |= Symbol::IndexedForCodeCompletion;
+Sym.CanonicalDeclaration.FileURI = S.DeclaringFile.c_str();
+Sym.Definition.FileURI = S.DeclaringFile.c_str();
+Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1);
+Slab.insert(Sym);
+  }
+  return MemIndex::build(std::move(Slab).build(), RefSlab());
+}
+
 TEST(IncludeFixerTest, IncompleteType) {
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
@@ -293,15 +318,8 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  Symbol Sym = cls("ns::X");
-  Sym.Flags |= Symbol::IndexedForCodeCompletion;
-  Sym.CanonicalDeclaration.FileURI = "unittest:///x.h";
-  Sym.Definition.FileURI = "unittest:///x.h";
-  Sym.IncludeHeaders.emplace_back("\"x.h\"", 1);
-
-  SymbolSlab::Builder Slab;
-  Slab.insert(Sym);
-  auto Index = MemIndex::build(std::move(Slab).build(), RefSlab());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""}});
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
@@ -346,6 +364,65 @@
"member access into incomplete type 'ns::X'")));
 }
 
+TEST(IncludeFixerTest, Typo) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+void foo() {
+  $unqualified[[X]] x;
+}
+}
+void bar() {
+  ns::$qualified[[X]] x; // ns:: is valid.
+  ::$global[[Global]] glob;
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"Global", "unittest:///global.h", "\"global.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("qualified"),
+ "no type named 'X' in namespace 'ns'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("global"),
+ "no type named 'Global' in the global namespace"),
+WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",
+"Add include \"global.h\" for symbol Global");
+}
+
+TEST(IncludeFixerTest, MultipleMatchedSymbols) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace na {
+namespace nb {
+void foo() {
+  $unqualified[[X]] x;
+}
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""},
+   SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  WithFix(Fix(Test.range("insert"), "#include \"a.h\"\n",
+  "Add include \"a.h\" for symbol na::X"),
+  Fix(Test.range("insert"), "#include \"b.h\"\n",
+  "Add include \"b.h\" for symbol na::nb::X");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -47,7 +47,7 @@
 /// T

[PATCH] D57021: [clangd] Suggest adding missing includes for typos (like include-fixer).

2019-02-04 Thread Eric Liu via Phabricator via cfe-commits

ioeric added inline comments.



Comment at: clangd/IncludeFixer.cpp:209
+  std::vector Scopes;
+  if (Typo.SS) {
+Scopes.push_back(*Typo.SS);

sammccall wrote:
> why do we do this here rather than compute the scopes/name already in 
> CorrectTypo()?
> 
> Passing around more of these AST objects and doing the computations later 
> seems a bit more fragile.
Because not all typos will result in diagnostics. Computing scopes could be 
expensive, so we would only want to do that when necessary. For example, sema 
can perform typo corrections for both `x` and `y` in `x::y`, while only one 
diagnostic (for either `x` or `y`) would be generated.



Comment at: clangd/IncludeFixer.cpp:233
+
+  SymbolSlab::Builder Matches;
+  Index.fuzzyFind(Req, [&](const Symbol &Sym) {

sammccall wrote:
> why put these into a slab and then build fixes later, rather than build the 
> fixes immediately?
good point.



Comment at: clangd/IncludeFixer.h:35
 public:
-  IncludeFixer(llvm::StringRef File, std::shared_ptr Inserter,
+  IncludeFixer(CompilerInstance &Compiler, llvm::StringRef File,
+   std::shared_ptr Inserter,

sammccall wrote:
> Having to inject the whole compiler into the include fixer doesn't seem 
> right. Apart from the huge scope, we're also going to register parts of the 
> include fixer with the compiler.
> 
> A few observations:
>  - all we actually need here is Sema and the SM (available through Sema)
>  - it's only the typo recorder that needs access to Sema
>  - the typo recorder gets fed the Sema instance 
> (`ExternalSemaSource::InitializeSema`)
>  - the fact that `fixTypo()` needs to access the typo recorder doesn't seem 
> necessary
> 
> So I'd suggest changing this function to return a *new* ExternalSemaSource 
> that stashes the last typo in this IncludeFixer object directly.
> 
> e.g.
> 
> ```
> llvm::IntrusiveRefCntPtr typoRecorder() {
>   return new TypoRecorder(LastTypo);
> }
> private:
>   Optional LastTypo;
>   class TypoRecorder : public ExternalSemaSource {
> Sema *S;
> Optional &Out;
> InitializeSema(Sema& S) { this->S = &S; }
> CorrectTypo(...) {
>   assert(S);
>   ...
>   Out = ...;
> }
>   }
>   ```
Great idea. Thanks!

I've done most of the suggested refactoring.

> the fact that fixTypo() needs to access the typo recorder doesn't seem 
> necessary.
Do you mean access to *Sema*? The `IncludeFixer` still needs sema because we 
don't want to run Lookup for each typo sema encounters. We only want to do that 
for typos that make it to diagnostics. See my response to the other comment 
about this for details.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57021



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

[PATCH] D57540: [C++17] Don't crash while diagnosing different access specifier of a deduction guide.

2019-02-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: test/SemaCXX/cxx1z-class-template-argument-deduction.cpp:360-361
+  struct Type {
+Typo(); // expected-error{{deduction guide must be declared in the same 
scope}}
+// expected-error@-1{{deduction guide declaration without trailing return 
type}}
+  };

Rakete wrote:
> aaron.ballman wrote:
> > These errors are pretty unfortunate -- they don't really help the user to 
> > understand what's gone wrong here. They're an improvement over the crash, 
> > but I think we should try to make the errors more useful if we can.
> > 
> > Why is `Typo()` being treated as a deduction guide? Perhaps we could look 
> > to see if there is a `->` before making that determination?
> > Perhaps we could look to see if there is a -> before making that 
> > determination?
> 
> Yes that would be possible. The diagnostic would change for the following 
> code:
> 
> ```
> template 
> struct Foo {};
> 
> Foo();// -> Foo; 
> // currently: deduction guide missing ->
> // after: C++ requires type specifier for every declaration
> ```
> 
> Is that acceptable? Or I guess I could restrict this to partial deduction 
> guides in classes.
I think the original diagnostic is better in that case. If you restrict to 
partial deduction guides, do we get all the good diagnostics?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57540



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

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

2019-02-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: test/clang-tidy/google-objc-function-naming.m:3
 
+#import 
+

stephanemoore wrote:
> It turns out importing  is problematic and breaks the build (though 
> everything built successfully for me locally 🤔). I believe that the import is 
> not strictly necessary and I can embed a function declaration for `printf` to 
> reproduce the implicit function declaration with the caveat that the check 
> will trigger on a `printf` declaration that is not from a system header. I 
> suppose it might be reasonable to suppress the check output on `printf`?
> 
> I have reverted this change for now and will follow up with appropriate fixes.
> It turns out importing  is problematic and breaks the build (though 
> everything built successfully for me locally 🤔).

Sorry about not catching that during review. I'm used to mentioning that you 
can't do `#include` but I didn't recall if the same was true for `#import`.

> I believe that the import is not strictly necessary and I can embed a 
> function declaration for printf to reproduce the implicit function 
> declaration with the caveat that the check will trigger on a printf 
> declaration that is not from a system header. I suppose it might be 
> reasonable to suppress the check output on printf?

You can use line markers to specify that the declaration is part of a system 
header with something like this:
```
# 1 "system_header.h" 3
int printf(const char *, ...);
# 1 "google-objc-function-naming.m" 1
```
(Note, you may need to pull a more decorated signature for `printf()` to be 
accurate.)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57207



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

[PATCH] D45978: dllexport const variables must have external linkage.

2019-02-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D45978#1382292 , @zahiraam wrote:

> In D45978#1379901 , @rnk wrote:
>
> > I'm still not sure this is the best place to make this change, but the 
> > functionality is important. There are still unaddressed comments (no need 
> > to check MSVCCompatibility, formatting), and I think once those are fixed 
> > we can land this.
>
>
> Since it needs to fail for this:
>
> __declspec(dllexport) const int j;
>
> and pass for this:
>
> __declspec(dllexport) int const x = 3;
>
> I am proposing to add this code in Sema::AddInitializerToDecl(Decl *RealDecl, 
> Expr *Init, bool DirectInit).


That seems like a reasonable place to try, to me.


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

https://reviews.llvm.org/D45978



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

[PATCH] D57659: [Sema] SequenceChecker: Add some comments + related small NFCs in preparation of the following patches

2019-02-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:11748
+/// The expressions corresponding to each usage kind.
+Expr *UsageExprs[UK_Count]{};
+

You can drop the `{}` here and below.



Comment at: lib/Sema/SemaChecking.cpp:11927
+  //
+  // A last thing we have to be careful is that some operation sequences
+  // modification as side effect as well (for example: || or ,). To account for

A last thing we have to be careful is that -> We also have to be careful that


Repository:
  rC Clang

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

https://reviews.llvm.org/D57659



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

[PATCH] D57661: [Sema] SequenceChecker: Add tests for references and members.

2019-02-04 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno updated this revision to Diff 185047.
riccibruno added a comment.

Added tests for nested/anonymous unions and local structs.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57661

Files:
  test/SemaCXX/warn-unsequenced.cpp

Index: test/SemaCXX/warn-unsequenced.cpp
===
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-unused %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-unused -Wno-uninitialized -Wunsequenced %s
 
 int f(int, int = 0);
 
@@ -21,62 +21,62 @@
   ++ ++a; // ok
   (a++, a++); // ok
   ++a + ++a; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  a++ + a++; // expected-warning {{multiple unsequenced modifications}}
+  a++ + a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
   (a++, a) = 0; // ok, increment is sequenced before value computation of LHS
   a = xs[++a]; // ok
-  a = xs[a++]; // expected-warning {{multiple unsequenced modifications}}
-  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access}}
+  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to 'a'}}
   a = (++a, ++a); // ok
   a = (a++, ++a); // ok
-  a = (a++, a++); // expected-warning {{multiple unsequenced modifications}}
+  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
   f(a, a); // ok
-  f(a = 0, a); // expected-warning {{unsequenced modification and access}}
-  f(a, a += 0); // expected-warning {{unsequenced modification and access}}
-  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications}}
+  f(a = 0, a); // expected-warning {{unsequenced modification and access to 'a'}}
+  f(a, a += 0); // expected-warning {{unsequenced modification and access to 'a'}}
+  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to 'a'}}
   a = f(++a); // ok
   a = f(a++); // ok
-  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications}}
+  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
 
   // Compound assignment "A OP= B" is equivalent to "A = A OP B" except that A
   // is evaluated only once.
   (++a, a) = 1; // ok
   (++a, a) += 1; // ok
   a = ++a; // ok
-  a += ++a; // expected-warning {{unsequenced modification and access}}
+  a += ++a; // expected-warning {{unsequenced modification and access to 'a'}}
 
   A agg1 = { a++, a++ }; // ok
-  A agg2 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}}
+  A agg2 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access to 'a'}}
 
-  S str1(a++, a++); // expected-warning {{multiple unsequenced modifications}}
+  S str1(a++, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
   S str2 = { a++, a++ }; // ok
-  S str3 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access}}
+  S str3 = { a++ + a, a++ }; // expected-warning {{unsequenced modification and access to 'a'}}
 
   struct Z { A a; S s; } z = { { ++a, ++a }, { ++a, ++a } }; // ok
   a = S { ++a, a++ }.n; // ok
   A { ++a, a++ }.x; // ok
-  a = A { ++a, a++ }.x; // expected-warning {{unsequenced modifications}}
-  A { ++a, a++ }.x + A { ++a, a++ }.y; // expected-warning {{unsequenced modifications}}
+  a = A { ++a, a++ }.x; // expected-warning {{unsequenced modifications to 'a'}}
+  A { ++a, a++ }.x + A { ++a, a++ }.y; // expected-warning {{unsequenced modifications to 'a'}}
 
   (xs[2] && (a = 0)) + a; // ok
   (0 && (a = 0)) + a; // ok
-  (1 && (a = 0)) + a; // expected-warning {{unsequenced modification and access}}
+  (1 && (a = 0)) + a; // expected-warning {{unsequenced modification and access to 'a'}}
 
   (xs[3] || (a = 0)) + a; // ok
-  (0 || (a = 0)) + a; // expected-warning {{unsequenced modification and access}}
+  (0 || (a = 0)) + a; // expected-warning {{unsequenced modification and access to 'a'}}
   (1 || (a = 0)) + a; // ok
 
   (xs[4] ? a : ++a) + a; // ok
-  (0 ? a : ++a) + a; // expected-warning {{unsequenced modification and access}}
+  (0 ? a : ++a) + a; // expected-warning {{unsequenced modification and access to 'a'}}
   (1 ? a : ++a) + a; // ok
-  (0 ? a : a++) + a; // expected-warning {{unsequenced modification and access}}
+  (0 ? a : a++) + a; // expected-warning {{unsequenced modification and access to 'a'}}
   (1 ? a : a++) + a; // ok
   (xs[5] ? ++a : ++a) + a; // FIXME: warn here
 
-  (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced modification and access}}
+  (++a, xs[6] ? ++a : 0) + a; // expected-warning {{unsequenced modification and access to 'a'}}
 
   // Here, the read of the fourth 'a' might happen before or after the write to

[PATCH] D57660: [Sema] SequenceChecker: Handle references and members

2019-02-04 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno updated this revision to Diff 185048.
riccibruno added a comment.

Added tests for nested/anonymous unions and local structs.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-unsequenced.cpp

Index: test/SemaCXX/warn-unsequenced.cpp
===
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -131,54 +131,53 @@
 
   // Operations with a single member of the implicit object.
   ++a = 0; // no-warning
-  a + ++a; // expected-warning {{unsequenced modification and access to 'a'}}
+  a + ++a; // expected-warning {{unsequenced modification and access to member 'a'}}
   a = ++a; // no-warning
-  a + a++; // expected-warning {{unsequenced modification and access to 'a'}}
-  a = a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a + a++; // expected-warning {{unsequenced modification and access to member 'a'}}
+  a = a++; // expected-warning {{multiple unsequenced modifications to member 'a'}}
   ++ ++a; // no-warning
   (a++, a++); // no-warning
-  ++a + ++a; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  a++ + a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  ++a + ++a; // expected-warning {{multiple unsequenced modifications to member 'a'}}
+  a++ + a++; // expected-warning {{multiple unsequenced modifications to member 'a'}}
   (a++, a) = 0; // no-warning
   a = xs[++a]; // no-warning
-  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to 'a'}}
+  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to member 'a'}}
+  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to member 'a'}}
   a = (++a, ++a); // no-warning
   a = (a++, ++a); // no-warning
-  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to member 'a'}}
   f(a, a); // no-warning
-  f(a = 0, a); // expected-warning {{unsequenced modification and access to 'a'}}
-  f(a, a += 0); // expected-warning {{unsequenced modification and access to 'a'}}
-  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  f(a = 0, a); // expected-warning {{unsequenced modification and access to member 'a'}}
+  f(a, a += 0); // expected-warning {{unsequenced modification and access to member 'a'}}
+  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to member 'a'}}
   a = f(++a); // no-warning
   a = f(a++); // no-warning
-  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to member 'a'}}
 
   // Operations with a single member of the other object 's'.
-  // TODO: For now this is completely unhandled.
   ++s.a = 0; // no-warning
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to }}
+  s.a + ++s.a; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
   s.a = ++s.a; // no-warning
-  s.a + s.a++; // no-warning TODO {{unsequenced modification and access to }}
-  s.a = s.a++; // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a + s.a++; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
+  s.a = s.a++; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   ++ ++s.a; // no-warning
   (s.a++, s.a++); // no-warning
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to }}
-  s.a++ + s.a++; // no-warning TODO {{multiple unsequenced modifications to }}
+  ++s.a + ++s.a; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a++ + s.a++; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   (s.a++, s.a) = 0; // no-warning
   s.a = xs[++s.a]; // no-warning
-  s.a = xs[s.a++]; // no-warning TODO {{multiple unsequenced modifications to }}
-  (s.a ? xs[0] : xs[1]) = ++s.a; // no-warning TODO {{unsequenced modification and access to }}
+  s.a = xs[s.a++]; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+  (s.a ? xs[0] : xs[1]) = ++s.a; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
   s.a = (++s.a, ++s.a); // no-warning
   s.a = (s.a++, ++s.a); // no-warning
-  s.a = (s.a++, s.a++); // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a = (s.a++, s.a++); // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   f(s.a, s.a); // no-warning
-  f(s.a = 0, s.a); // no-warning TODO {{unsequenced modification and ac

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Eric Liu via Phabricator via cfe-commits

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lgtm.

Note that this will enable include insertions for everyone (even without index 
switched on). Not sure if we should provide options.




Comment at: clangd/ClangdServer.cpp:16
 #include "Trace.h"
+#include "XRefs.h"
+#include "index/CanonicalIncludes.h"

nit: is this used?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508



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

[PATCH] D57659: [Sema] SequenceChecker: Add some comments + related small NFCs in preparation of the following patches

2019-02-04 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno marked an inline comment as done.
riccibruno added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:11748
+/// The expressions corresponding to each usage kind.
+Expr *UsageExprs[UK_Count]{};
+

aaron.ballman wrote:
> You can drop the `{}` here and below.
I don't think I can (although I can drop it for the array of 
`SequenceTree::Seq`). I want to value-initialize each expression in this array 
so that they are all null.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57659



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

2019-02-04 Thread Ronald Wampler via Phabricator via cfe-commits

rdwampler created this revision.
rdwampler added reviewers: djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This option `AllowShortLambdasOnASingleLine` similar to the other `AllowShort*` 
options, but applied to C++ lambdas.

I considered making this dependent on `AllowShortFunctionsOnASingleLine`, but 
could not think of how the breaking should be behave when the option is set to 
`Inline` and `InlineOnly`.

I could not find a specific coding style that requires short lambdas to 
//always// break on a new line, but I do think it's fairly common practice. 
Also, I note that the patch: https://reviews.llvm.org/D44609 attempts to add an 
option to allow formatting lambda using the Allman style braces. Here is a 
coding style that requires Allman style braces, but allows lambdas to be on 
single line if they are short. So, maybe this option would be required to 
handle this situation?

Addresses: https://bugs.llvm.org//show_bug.cgi?id=32151


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,30 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   " return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2835,7 +2835,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken &Tok) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
@@ -2962,6 +2962,10 @@
(Line.startsWith(t

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet marked an inline comment as done.
kadircet added a comment.

I don't think so, since the only reason this already wasn't enabled was because 
there was a fixme ? Also we want people to use -background-index by default in 
future so I think it should be OK


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508



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

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 185052.
kadircet added a comment.

- Delete unnecessary include


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -59,13 +59,15 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
+AST.getCanonicalIncludes());
 }
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
   auto Idx = llvm::make_unique(/*UseDex=*/true);
-  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
+  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+  AST.getCanonicalIncludes());
   Idx->updateMain(Filename, AST);
   return std::move(Idx);
 }
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -12,6 +12,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -147,8 +148,8 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.updatePreamble(File.Filename, AST.getASTContext(),
-   AST.getPreprocessorPtr());
+  M.updatePreamble(File.Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+   AST.getCanonicalIncludes());
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
@@ -199,13 +200,16 @@
QName("X::f")));
 }
 
-TEST(FileIndexTest, NoIncludeCollected) {
+TEST(FileIndexTest, IncludeCollected) {
   FileIndex M;
-  update(M, "f", "class string {};");
+  update(
+  M, "f",
+  "// IWYU pragma: private, include \nclass string {};");
 
   auto Symbols = runFuzzyFind(M, "");
   EXPECT_THAT(Symbols, ElementsAre(_));
-  EXPECT_THAT(Symbols.begin()->IncludeHeaders, IsEmpty());
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {
@@ -270,10 +274,11 @@
   buildPreamble(
   FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
   std::make_shared(), /*StoreInMemory=*/true,
-  [&](ASTContext &Ctx, std::shared_ptr PP) {
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &CanonIncludes) {
 EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
 IndexUpdated = true;
-Index.updatePreamble(FooCpp, Ctx, std::move(PP));
+Index.updatePreamble(FooCpp, Ctx, std::move(PP), CanonIncludes);
   });
   ASSERT_TRUE(IndexUpdated);
 
@@ -358,7 +363,8 @@
   MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
   tooling::CompileCommand(), PI, std::make_shared(),
   /*StoreInMemory=*/true,
-  [&](ASTContext &Ctx, std::shared_ptr PP) {});
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &) {});
   // Build AST for main file with preamble.
   auto AST =
   ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData,
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -645,6 +645,36 @@
   EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
 }
 
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+
+  FS.Files[testPath("foo_header.h")] = R"cpp(
+struct Foo {
+   // Member doc
+   int foo();
+};
+  )cpp";
+  const std::string FileContent(R"cpp(
+#include "foo_header.h"
+int Foo::foo() {
+  return 42;
+}
+  )cpp");
+  Server.addDocument(testPath("foo_impl.cpp"), FileContent);
+  // Wait for the dynamic index being built.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(
+  completions(Server, "Foo^ foo;").Completions,
+  ElementsAre(AllOf(Named("Foo"),
+HasIn

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Kelvin Li via Phabricator via cfe-commits

kkwli0 created this revision.
kkwli0 added a reviewer: ABataev.
Herald added a subscriber: guansong.

The compiler does not generate any error messages if there are more than one 
teams construct inside a target constructs.

  #pragma omp target
  {
#pragma omp teams
{  ...  }
  
#pragma omp teams
{ ... }
  }

After the fix, the error messages are issued.

  teams.c:4:9: error: target construct with nested teams region contains 
statements
outside of the teams construct
  #pragma omp target
  ^
  teams.c:14:11: note: nested teams construct here
#pragma omp teams
^
  teams.c:9:11: note: directive outside teams construct here
#pragma omp teams
^
  1 error generated.


https://reviews.llvm.org/D57690

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nesting_of_regions.cpp


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,10 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&
+ OMPTeamsFound)) {
+
   OMPTeamsFound = false;
   break;
 }


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,10 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&
+ OMPTeamsFound)) {
+

[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via Phabricator via cfe-commits

hwright updated this revision to Diff 185057.
hwright marked 4 inline comments as done.
hwright retitled this revision from "[clang-tidy] Add the 
abseil-duration-double-conversion check" to "[clang-tidy] Add the 
abseil-duration-unnecessary-conversion check".
hwright added a comment.

Renamed to `abseil-duration-unnecessary-conversion`


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

https://reviews.llvm.org/D57353

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-duration-unnecessary-conversion.cpp

Index: test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
@@ -0,0 +1,69 @@
+// RUN: %check_clang_tidy %s abseil-duration-unnecessary-conversion %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  absl::Duration d1, d2;
+
+  // Floating point
+  d2 = absl::Hours(absl::ToDoubleHours(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Minutes(absl::ToDoubleMinutes(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Seconds(absl::ToDoubleSeconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Microseconds(absl::ToDoubleMicroseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+
+  // Integer point
+  d2 = absl::Hours(absl::ToInt64Hours(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Minutes(absl::ToInt64Minutes(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Seconds(absl::ToInt64Seconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Milliseconds(absl::ToInt64Milliseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Microseconds(absl::ToInt64Microseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Nanoseconds(absl::ToInt64Nanoseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+
+  // As macro argument
+#define PLUS_FIVE_S(x) x + absl::Seconds(5)
+  d2 = PLUS_FIVE_S(absl::Seconds(absl::ToInt64Seconds(d1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: PLUS_FIVE_S(d1)
+#undef PLUS_FIVE_S
+
+  // Split by macro: should not change
+#define TOSECONDS(x) absl::Seconds(x)
+  d2 = TOSECONDS(absl::ToInt64Seconds(d1));
+#undef TOSECONDS
+
+  // Don't change something inside a macro definition
+#define VALUE(x) absl::Hours(absl::ToInt64Hours(x));
+  d2 = VALUE(d1);
+#undef VALUE
+
+  // These should not match
+  d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1));
+  d2 = absl::Seconds(4);
+  int i = absl::ToInt64Milliseconds(d1);
+}
Index: test/clang-tidy/Inputs/absl/time/time.h
===
--- test/clang-tidy/Inputs/absl/time/time.h
+++ test/clang-tidy/Inputs/absl/time/time.h
@@ -14,6 +14,8 @@
   Duration &operator/=(float r);
   Duration &operator/=(double r);
   template  Duration &operator/=(T r);
+
+  Duration &operator+(Duration d);
 };
 
 template  Duration operator*(Duration lhs, T rhs);
Index: docs/clang-tidy/checks/list.rst
=

[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via Phabricator via cfe-commits

hwright added a comment.

@hokein Thanks for the suggestion on the name, I was looking for something a 
little less confusing.


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

https://reviews.llvm.org/D57353



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

[clang-tools-extra] r353054 - [clangd] Enable include insertion for static index

2019-02-04 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Mon Feb  4 08:19:57 2019
New Revision: 353054

URL: http://llvm.org/viewvc/llvm-project?rev=353054&view=rev
Log:
[clangd] Enable include insertion for static index

Summary:
This enables include insertion by adding canonical includes into
preambledata.

Reviewers: ioeric, ilya-biryukov

Subscribers: javed.absar, MaskRay, jkorous, arphaman, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=353054&r1=353053&r2=353054&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Feb  4 08:19:57 2019
@@ -13,6 +13,7 @@
 #include "Headers.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
 #include "refactor/Tweak.h"
@@ -69,9 +70,10 @@ struct UpdateIndexCallbacks : public Par
   : FIndex(FIndex), DiagConsumer(DiagConsumer) {}
 
   void onPreambleAST(PathRef Path, ASTContext &Ctx,
- std::shared_ptr PP) override {
+ std::shared_ptr PP,
+ const CanonicalIncludes &CanonIncludes) override {
 if (FIndex)
-  FIndex->updatePreamble(Path, Ctx, std::move(PP));
+  FIndex->updatePreamble(Path, Ctx, std::move(PP), CanonIncludes);
   }
 
   void onMainAST(PathRef Path, ParsedAST &AST) override {

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=353054&r1=353053&r2=353054&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon Feb  4 08:19:57 2019
@@ -16,6 +16,7 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "index/CanonicalIncludes.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/LangOptions.h"
@@ -99,11 +100,16 @@ public:
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
+  CanonicalIncludes takeCanonicalIncludes() {
+addSystemHeadersMapping(&CanonIncludes);
+return std::move(CanonIncludes);
+  }
+
   void AfterExecute(CompilerInstance &CI) override {
 if (!ParsedCallback)
   return;
 trace::Span Tracer("Running PreambleCallback");
-ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr());
+ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr(), CanonIncludes);
   }
 
   void BeforeExecute(CompilerInstance &CI) override {
@@ -115,10 +121,17 @@ public:
 return collectIncludeStructureCallback(*SourceMgr, &Includes);
   }
 
+  CommentHandler *getCommentHandler() override {
+IWYUHandler = collectIWYUHeaderMaps(&CanonIncludes);
+return IWYUHandler.get();
+  }
+
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
   IncludeStructure Includes;
+  CanonicalIncludes CanonIncludes;
+  std::unique_ptr IWYUHandler = nullptr;
   SourceManager *SourceMgr = nullptr;
 };
 
@@ -324,6 +337,17 @@ ParsedAST::build(std::unique_ptrgetPreprocessor().addPPCallbacks(
   collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
 
+  // Copy over the includes from the preamble, then combine with the
+  // non-preamble includes below.
+  CanonicalIncludes CanonIncludes;
+  if (Preamble)
+CanonIncludes = Preamble->CanonIncludes;
+  else
+addSystemHeadersMapping(&CanonIncludes);
+  std::unique_ptr IWYUHandler =
+  collectIWYUHeaderMaps(&CanonIncludes);
+  Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
+
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
@@ -353,7 +377,7 @@ ParsedAST::build(std::unique_ptrDiags.begin(), 
Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
std::move(ParsedDecls), std::move(Diags),
-   std::move(Includes));
+   std::move(Includes), std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;
@@ -429,21 +453,28 @@ const IncludeStructure &ParsedAST::getIn
   return In

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353054: [clangd] Enable include insertion for static index 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57508

Files:
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/TUScheduler.cpp
  clang-tools-extra/trunk/clangd/TUScheduler.h
  clang-tools-extra/trunk/clangd/index/FileIndex.cpp
  clang-tools-extra/trunk/clangd/index/FileIndex.h
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
  clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
  clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -645,6 +645,36 @@
   EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
 }
 
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+
+  FS.Files[testPath("foo_header.h")] = R"cpp(
+struct Foo {
+   // Member doc
+   int foo();
+};
+  )cpp";
+  const std::string FileContent(R"cpp(
+#include "foo_header.h"
+int Foo::foo() {
+  return 42;
+}
+  )cpp");
+  Server.addDocument(testPath("foo_impl.cpp"), FileContent);
+  // Wait for the dynamic index being built.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(
+  completions(Server, "Foo^ foo;").Completions,
+  ElementsAre(AllOf(Named("Foo"),
+HasInclude('"' + testPath("foo_header.h") + '"'),
+InsertInclude(;
+}
+
 TEST(CompletionTest, DynamicIndexMultiFile) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
Index: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
@@ -59,13 +59,15 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
+AST.getCanonicalIncludes());
 }
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
   auto Idx = llvm::make_unique(/*UseDex=*/true);
-  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
+  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+  AST.getCanonicalIncludes());
   Idx->updateMain(Filename, AST);
   return std::move(Idx);
 }
Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -12,6 +12,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -147,8 +148,8 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.updatePreamble(File.Filename, AST.getASTContext(),
-   AST.getPreprocessorPtr());
+  M.updatePreamble(File.Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+   AST.getCanonicalIncludes());
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
@@ -199,13 +200,16 @@
QName("X::f")));
 }
 
-TEST(FileIndexTest, NoIncludeCollected) {
+TEST(FileIndexTest, IncludeCollected) {
   FileIndex M;
-  update(M, "f", "class string {};");
+  update(
+  M, "f",
+  "// IWYU pragma: private, include \nclass string {};");
 
   auto Symbols = runFuzzyFind(M, "");
   EXPECT_THAT(Symbols, ElementsAre(_));
-  EXPECT_THAT(Symbols.begin()->IncludeHeaders, IsEmpty());
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {
@@ -270,10 +274,11 @@
   buildPreamble(
   FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
   std::make_shared(), /*StoreInMemory=*/true,
-  [&](ASTContext &Ctx, std::shared_ptr PP) {
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:7070
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&

just `if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind() || 
OMPTeamsFound)`


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

https://reviews.llvm.org/D57690



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

[PATCH] D57464: Generalize method overloading on addr spaces to C++

2019-02-04 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: lib/Parse/ParseDeclCXX.cpp:2313
+}
+  }
 

ebevhan wrote:
> Anastasia wrote:
> > ebevhan wrote:
> > > Anastasia wrote:
> > > > Anastasia wrote:
> > > > > ebevhan wrote:
> > > > > > Is there a reason that the attributes are parsed here and not in 
> > > > > > `ParseFunctionDeclarator` like the rest of the ref-qualifiers (and 
> > > > > > the OpenCL++ ASes)?
> > > > > > 
> > > > > > That is possibly why the parser is getting confused with the 
> > > > > > trailing return.
> > > > > Good question! I have a feeling that this was done to unify parsing 
> > > > > of all CXX members, not just methods. For collecting the method 
> > > > > qualifiers it would certainly help making flow more coherent if this 
> > > > > was moved to `ParseFunctionDeclarator`. I will try to experiment with 
> > > > > this a bit more. What I found strange that the attributes here are 
> > > > > attached to the function type itself instead of its  qualifiers. May 
> > > > > be @rjmccall can shed some more light on the overall flow...
> > > > I looked at this a bit more and it seems that all the GNU attributes 
> > > > other than addr spaces belong to the function (not to be applied to 
> > > > `this`) for example 
> > > > https://blog.timac.org/2016/0716-constructor-and-destructor-attributes/.
> > > >  It seems correct to parse them here and apply to the function type. 
> > > > Although we could separate parsing of addr space attribute only and 
> > > > move into `ParseFunctionDeclarator`.  However this will only be needed 
> > > > for the function type not for the data members. So not sure whether it 
> > > > will make the flow any cleaner.
> > > > I looked at this a bit more and it seems that all the GNU attributes 
> > > > other than addr spaces belong to the function (not to be applied to 
> > > > this) 
> > > 
> > > Hm, I know what you mean. It's why Clang usually complains when you try 
> > > placing an AS attribute after a function declarator, because it's trying 
> > > to apply it to the function rather than the method qualifiers.
> > > 
> > > > However this will only be needed for the function type not for the data 
> > > > members. 
> > > 
> > > But we aren't really interested in data members, are we? Like struct 
> > > fields, non-static data members cannot be qualified with ASes (they 
> > > should 'inherit' the AS from the object type), and static ones should 
> > > probably just accept ASes as part of the member type itself.
> > > 
> > > These patches are to enable AS-qualifiers on methods, so it only stands 
> > > to reason that those ASes would be parsed along with the other method 
> > > qualifiers.
> > > 
> > > ParseFunctionDeclarator calls ParseTypeQualifierListOpt to get the 
> > > cv-qualifier-seq for the method qualifiers. Currently it's set to 
> > > `AR_NoAttributesParsed`. If we enable parsing attributes there, then the 
> > > qualifier parsing might 'eat' attributes that were possibly meant for the 
> > > function.
> > > 
> > > This is just a guess, but what I think you could do is include attributes 
> > > in the cv-qualifier parsing with `AR_GNUAttributesParsed`, look for any 
> > > AS attributes, take their AS and mark them invalid, then move the 
> > > attributes (somehow) from the qualifier-DeclSpec to the `FnAttrs` 
> > > function attribute list.
> > > 
> > > (The reason I'm concerned about where/how the qualifiers are parsed is 
> > > because this approach only works for custom ASes applied with the GNU 
> > > attribute directly. It won't work if custom address spaces are given with 
> > > a custom keyword qualifier, like in OpenCL. If the ASes are parsed into 
> > > the DeclSpec used for the other ref-qualifiers, then both of these cases 
> > > should 'just work'.)
> > > But we aren't really interested in data members, are we? Like struct 
> > > fields, non-static data members cannot be qualified with ASes (they 
> > > should 'inherit' the AS from the object type), and static ones should 
> > > probably just accept ASes as part of the member type itself.
> >  
> > Pointer data members can be qualified by and address space, but this should 
> > be parsed before.
> > 
> > 
> > > This is just a guess, but what I think you could do is include attributes 
> > > in the cv-qualifier parsing with AR_GNUAttributesParsed, look for any AS 
> > > attributes, take their AS and mark them invalid, then move the attributes 
> > > (somehow) from the qualifier-DeclSpec to the FnAttrs function attribute 
> > > list.
> > 
> > Ok, I will try that. Thanks for sharing your ideas!
> > 
> > Pointer data members can be qualified by and address space, but this should 
> > be parsed before.
> 
> Right, pointer-to-member is a thing. I always forget about those.
It seems unfortunately that moving parsing of GNU attributes into 
`ParseFunctionDeclarator` might not be a good viable alternative. One of the 
is

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

2019-02-04 Thread Zinovy Nis via Phabricator via cfe-commits

zinovy.nis updated this revision to Diff 185085.
zinovy.nis added a comment.

Fixed minor typos.


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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py

Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -25,9 +25,56 @@
 
 import argparse
 import json
+import multiprocessing
+import os
 import re
 import subprocess
 import sys
+import threading
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if not timeout is None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(' '.join(command) + '\n' + stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except:
+  with lock:
+sys.stderr.write('Failed: ' + str(sys.exc_info()[0]) + ' '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' + ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def run_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
 
 
 def main():
@@ -48,6 +95,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=0,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +135,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +153,65 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
+  max_task = args.j
+  if max_task == 0:
+  max_task = multiprocessing.cpu_count()
+  max_task = min(len(lines_by_file), max_task)
 
-  quote = "";
-  if sys.platform == 'win32':
-line_filter_json=re.sub(r'"', r'"""', line_filter_json)
-  else:
-quote = "'";
+  # Tasks for clang-tidy.
+  task_queue = queue.Queue(max_task)
+  # A lock for console output.
+  lock = threading.Lock()
 
-  # Run clang-tidy on files containing changes.
-  command = [args.clang_tidy_binary]
-  command.append('-line-filter=' + quote + line_filter_json + quote)
+  # Run a pool of clang-tidy workers.
+  run_workers(max_task, run_tidy, task_queue, lock, args.timeout)
+
+  # Run a watchdog.
+  quote = ""
+  if sys.platform != 'win32':
+quote = "'"
+
+  # Form the common args list.
+  common_clang_tidy_args = []
   if args.fix:
-command.append('-fix')
+common_clang_tidy_args.append('-fix')
   if args.export_fixes:
-command.append('-export-fixes=' + args.export_fixes)
+common_clang_tidy_args.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
-command.append('-checks=' + quote + args.checks + quote)
+common_clang_tidy_args.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
-command.append('-quiet')
+common_clang_tidy_args.append('-quiet')
   if args.build_path is not None:
-command.append('-p=%s' % args.build_path)
-  command.extend(lines_by_file.keys())
+common_clang_tidy_args.append('-p=%s' % args.build_path)
   for arg in args.extra_arg:
-  command.append('-extra-arg=%s' % arg)
+common_clang_tidy_args.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
-  command.append('-extra-arg-be

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

2019-02-04 Thread Zinovy Nis via Phabricator via cfe-commits

zinovy.nis updated this revision to Diff 185087.

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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py

Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -25,9 +25,56 @@
 
 import argparse
 import json
+import multiprocessing
+import os
 import re
 import subprocess
 import sys
+import threading
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if not timeout is None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(' '.join(command) + '\n' + stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except:
+  with lock:
+sys.stderr.write('Failed: ' + str(sys.exc_info()[0]) + ' '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' + ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def run_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
 
 
 def main():
@@ -48,6 +95,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=0,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +135,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +153,64 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
+  max_task = args.j
+  if max_task == 0:
+  max_task = multiprocessing.cpu_count()
+  max_task = min(len(lines_by_file), max_task)
 
-  quote = "";
-  if sys.platform == 'win32':
-line_filter_json=re.sub(r'"', r'"""', line_filter_json)
-  else:
-quote = "'";
+  # Tasks for clang-tidy.
+  task_queue = queue.Queue(max_task)
+  # A lock for console output.
+  lock = threading.Lock()
 
-  # Run clang-tidy on files containing changes.
-  command = [args.clang_tidy_binary]
-  command.append('-line-filter=' + quote + line_filter_json + quote)
+  # Run a pool of clang-tidy workers.
+  run_workers(max_task, run_tidy, task_queue, lock, args.timeout)
+
+  quote = ""
+  if sys.platform != 'win32':
+quote = "'"
+
+  # Form the common args list.
+  common_clang_tidy_args = []
   if args.fix:
-command.append('-fix')
+common_clang_tidy_args.append('-fix')
   if args.export_fixes:
-command.append('-export-fixes=' + args.export_fixes)
+common_clang_tidy_args.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
-command.append('-checks=' + quote + args.checks + quote)
+common_clang_tidy_args.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
-command.append('-quiet')
+common_clang_tidy_args.append('-quiet')
   if args.build_path is not None:
-command.append('-p=%s' % args.build_path)
-  command.extend(lines_by_file.keys())
+common_clang_tidy_args.append('-p=%s' % args.build_path)
   for arg in args.extra_arg:
-  command.append('-extra-arg=%s' % arg)
+common_clang_tidy_args.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
-  command.append('-extra-arg-before=%s' % arg)
-  command.extend(clang_tidy_args)
+common_clang_t

[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 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.

LGTM




Comment at: clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp:50
+
+  diag(OuterCall->getBeginLoc(), "remove double conversion of absl::Duration")
+  << FixItHint::CreateReplacement(

also avoid `double` word in the message. 



Comment at: docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst:17
+
+  // Suggestion - Remove double conversion
+  absl::Duration d2 = d1;

and here.



Comment at: docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst:25
+
+  // Suggestion - Remove double conversion
+  absl::Duration d2 = d1;

and here.


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

https://reviews.llvm.org/D57353



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

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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits

jyu2 commandeered this revision.
jyu2 edited reviewers, added: craig.topper; removed: jyu2.
jyu2 added inline comments.



Comment at: lib/Analysis/CFG.cpp:2126
+  if (cast(S)->isGCCAsmGoto())
+return VisitGCCAsmStmt(cast(S));
+  return VisitStmt(S, asc);

rsmith wrote:
> This is not an appropriate function name for a function that is only called 
> on `asm goto`. Given the convention here is to match the `Stmt` class 
> hierarchy with the `Visit` functions, the branch on `asm goto` should be in 
> the callee.
Changed.



Comment at: lib/CodeGen/CGStmt.cpp:1884
+template 
+void CodeGenFunction::UpdateCallInst(
+T &Result, bool HasSideEffect, bool ReadOnly, bool ReadNone,

efriedma wrote:
> rsmith wrote:
> > This is not a reasonable function name for a function that is specific to 
> > `AsmStmt`s. Please also make this a (static) non-member function, since it 
> > cannot be called outside this source file.
> Does this really need to be a template function?  I think all the functions 
> you're calling on "Result" are members of CallBase.
How about UpdateAsmCallInst?



Comment at: lib/CodeGen/CGStmt.cpp:1884
+template 
+void CodeGenFunction::UpdateCallInst(
+T &Result, bool HasSideEffect, bool ReadOnly, bool ReadNone,

jyu2 wrote:
> efriedma wrote:
> > rsmith wrote:
> > > This is not a reasonable function name for a function that is specific to 
> > > `AsmStmt`s. Please also make this a (static) non-member function, since 
> > > it cannot be called outside this source file.
> > Does this really need to be a template function?  I think all the functions 
> > you're calling on "Result" are members of CallBase.
> How about UpdateAsmCallInst?
Yes. I am using CallBase instead template.



Comment at: lib/CodeGen/CGStmt.cpp:2188
+ std::to_string(NumGccAsmGotoStmts));
+  NumGccAsmGotoStmts++;
+}

efriedma wrote:
> The NumGccAsmGotoStmts variable is pointless; if names are enabled, 
> createBasicBlock will automatically number all the blocks named "normal".
Okay changed.  Thanks.



Comment at: lib/CodeGen/CGStmt.cpp:2238-2245
+UpdateCallInst(*Result, HasSideEffect, ReadOnly,
+ ReadNone, S, ResultRegTypes, RegResults);
+EmitBlock(Fallthrough);
   } else {
-for (unsigned i = 0, e = ResultRegTypes.size(); i != e; ++i) {
-  llvm::Value *Tmp = Builder.CreateExtractValue(Result, i, "asmresult");
-  RegResults.push_back(Tmp);
-}
+llvm::CallInst *Result =
+Builder.CreateCall(IA, Args, getBundlesForFunclet(IA));
+UpdateCallInst(*Result, HasSideEffect, ReadOnly, ReadNone,

rsmith wrote:
> No need to explicitly specify the template argument here; it's deducible.
Thanks.  Changed.



Comment at: lib/Parse/ParseStmtAsm.cpp:833-837
+if (Tok.isNot(tok::identifier)) {
+  Diag(Tok, diag::err_expected) << tok::identifier;
+  SkipUntil(tok::r_paren, StopAtSemi);
+  return StmtError();
+}

rsmith wrote:
> This should be inside the loop below.
Yes.  Change.



Comment at: test/Parser/asm-goto.c:1
+// RUN: %clang_cc1 %s
+

efriedma wrote:
> What is this testing?
Just testing asm goto can parser those input operands and labels.  Not error 
checking.  

Error checking is in Sema/asm-goto


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

https://reviews.llvm.org/D56571



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

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

2019-02-04 Thread Nemanja Ivanovic via Phabricator via cfe-commits

nemanjai updated this revision to Diff 185088.
nemanjai added a comment.

As mentioned in a comment, the WASM tests weren't really meant to indicate that 
WASM supports the type. Removed the changes to the WASM target.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57577

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Headers/float16.c
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9166,20 +9166,6 @@
 // WEBASSEMBLY-NOT:#define __ELF__
 // WEBASSEMBLY-NEXT:#define __FINITE_MATH_ONLY__ 0
 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_DECIMAL_DIG__ 5
-// WEBASSEMBLY-NEXT:#define __FLT16_DENORM_MIN__ 5.9604644775390625e-8F16
-// WEBASSEMBLY-NEXT:#define __FLT16_DIG__ 3
-// WEBASSEMBLY-NEXT:#define __FLT16_EPSILON__ 9.765625e-4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_DENORM__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_INFINITY__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_QUIET_NAN__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_MANT_DIG__ 11
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_10_EXP__ 4
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_EXP__ 15
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX__ 6.5504e+4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_10_EXP__ (-13)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_EXP__ (-14)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN__ 6.103515625e-5F16
 // WEBASSEMBLY-NEXT:#define __FLT_DECIMAL_DIG__ 9
 // WEBASSEMBLY-NEXT:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // WEBASSEMBLY-NEXT:#define __FLT_DIG__ 6
Index: test/Headers/float16.c
===
--- test/Headers/float16.c
+++ test/Headers/float16.c
@@ -1,7 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -x c++ -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify -std=c89 \
+// RUN:   -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify \
+// RUN:   -std=c99 -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify -std=c11 \
+// RUN:   -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify \
+// RUN:   -std=c++11 -x c++ -ffreestanding %s
 // expected-no-diagnostics
 
 #define __STDC_WANT_IEC_60559_TYPES_EXT__
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -830,7 +830,8 @@
   DefineFmt("__UINTPTR", TI.getUIntPtrType(), TI, Builder);
   DefineTypeWidth("__UINTPTR_WIDTH__", TI.getUIntPtrType(), TI, Builder);
 
-  DefineFloatMacros(Builder, "FLT16", &TI.getHalfFormat(), "F16");
+  if (TI.hasFloat16Type())
+DefineFloatMacros(Builder, "FLT16", &TI.getHalfFormat(), "F16");
   DefineFloatMacros(Builder, "FLT", &TI.getFloatFormat(), "F");
   DefineFloatMacros(Builder, "DBL", &TI.getDoubleFormat(), "");
   DefineFloatMacros(Builder, "LDBL", &TI.getLongDoubleFormat(), "L");


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9166,20 +9166,6 @@
 // WEBASSEMBLY-NOT:#define __ELF__
 // WEBASSEMBLY-NEXT:#define __FINITE_MATH_ONLY__ 0
 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_DECIMAL_DIG__ 5
-// WEBASSEMBLY-NEXT:#define __FLT16_DENORM_MIN__ 5.9604644775390625e-8F16
-// WEBASSEMBLY-NEXT:#define __FLT16_DIG__ 3
-// WEBASSEMBLY-NEXT:#define __FLT16_EPSILON__ 9.765625e-4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_DENORM__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_INFINITY__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_QUIET_NAN__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_MANT_DIG__ 11
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_10_EXP__ 4
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_EXP__ 15
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX__ 6.5504e+4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_10_EXP__ (-13)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_EXP__ (-14)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN__ 6.103515625e-5F16
 // WEBASSEMBLY-NEXT:#define __FLT_DECIMAL_DIG__ 9
 // WEBASSEMBLY-NEXT:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // WEBASSEMBLY-NEXT:#define __FLT_DIG__ 6
Index: test/Headers/float16.c
===
--- test/Headers/float16.c
+++ test/Headers/float16.c
@@ -1,7 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -ffreestanding %s
-// RUN: %

[PATCH] D57581: Explicitly add language standard option to test cases that rely on the C++14 default

2019-02-04 Thread Nemanja Ivanovic via Phabricator via cfe-commits

nemanjai updated this revision to Diff 185093.
nemanjai added a comment.

Changed the option to standard C++ rather than GNU extensions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57581

Files:
  test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
  test/CodeCompletion/skip-auto-funcs.cpp
  test/CodeGenCXX/auto-var-init.cpp
  test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
  test/CodeGenCXX/new-overflow.cpp
  test/CodeGenCXX/new.cpp
  test/Lexer/cxx-features.cpp
  test/Lexer/half-literal.cpp
  test/Modules/friend-definition-2.cpp
  test/Modules/merge-lambdas.cpp
  test/SemaCXX/int-ptr-cast-SFINAE.cpp
  test/SemaTemplate/argument-dependent-lookup.cpp
  test/SemaTemplate/class-template-decl.cpp
  test/SemaTemplate/typo-dependent-name.cpp

Index: test/SemaTemplate/typo-dependent-name.cpp
===
--- test/SemaTemplate/typo-dependent-name.cpp
+++ test/SemaTemplate/typo-dependent-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 
 using nullptr_t = decltype(nullptr);
 
Index: test/SemaTemplate/class-template-decl.cpp
===
--- test/SemaTemplate/class-template-decl.cpp
+++ test/SemaTemplate/class-template-decl.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 
 template class A;
 
Index: test/SemaTemplate/argument-dependent-lookup.cpp
===
--- test/SemaTemplate/argument-dependent-lookup.cpp
+++ test/SemaTemplate/argument-dependent-lookup.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -verify %s
-// RUN: %clang_cc1 -verify %s -DHAVE_UNQUALIFIED_LOOKUP_RESULTS
+// RUN: %clang_cc1 -std=c++14 -verify %s
+// RUN: %clang_cc1 -std=c++14 -verify %s -DHAVE_UNQUALIFIED_LOOKUP_RESULTS
 // expected-no-diagnostics
 
 namespace address_of {
Index: test/SemaCXX/int-ptr-cast-SFINAE.cpp
===
--- test/SemaCXX/int-ptr-cast-SFINAE.cpp
+++ test/SemaCXX/int-ptr-cast-SFINAE.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17
 
 void foo(int* a, int *b) {
Index: test/Modules/merge-lambdas.cpp
===
--- test/Modules/merge-lambdas.cpp
+++ test/Modules/merge-lambdas.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fmodules -verify %s -emit-llvm-only
+// RUN: %clang_cc1 -std=c++14 -fmodules -verify %s -emit-llvm-only
 // expected-no-diagnostics
 
 #pragma clang module build A
Index: test/Modules/friend-definition-2.cpp
===
--- test/Modules/friend-definition-2.cpp
+++ test/Modules/friend-definition-2.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fmodules %s -verify
-// RUN: %clang_cc1 -fmodules %s -verify -triple i686-windows
+// RUN: %clang_cc1 -std=c++14 -fmodules %s -verify
+// RUN: %clang_cc1 -std=c++14 -fmodules %s -verify -triple i686-windows
 // expected-no-diagnostics
 #pragma clang module build A
 module A {}
Index: test/Lexer/half-literal.cpp
===
--- test/Lexer/half-literal.cpp
+++ test/Lexer/half-literal.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -triple aarch64-linux-gnu %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify -pedantic -triple aarch64-linux-gnu %s
 float a = 1.0h; // expected-error{{no matching literal operator for call to 'operator""h' with argument of type 'long double' or 'const char *', and no matching literal operator template}}
 float b = 1.0H; // expected-error{{invalid suffix 'H' on floating constant}}
 
Index: test/Lexer/cxx-features.cpp
===
--- test/Lexer/cxx-features.cpp
+++ test/Lexer/cxx-features.cpp
@@ -6,9 +6,9 @@
 //
 // RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fsized-deallocation -frelaxed-template-template-args -DRELAXED_TEMPLATE_TEMPLATE_ARGS=1 -verify %s
 // RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fsized-deallocation -fconcepts-ts -DCONCEPTS_TS=1 -verify %s
-// RUN: %clang_cc1 -fno-rtti -fno-threadsafe-statics -verify %s -DNO_EXCEPTIONS -DNO_RTTI -DNO_THREADSAFE_STATICS -fsized-deallocation
-// RUN: %clang_cc1 -fcoroutines-ts -DNO_EXCEPTIONS -DCOROUTINES -verify -fsized-deallocation %s
-// RUN: %clang_cc1 -fchar8_t -DNO_EXCEPTIONS -DCHAR8_T -verify -fsized-deallocation %s
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fno-threadsafe-statics -verify %s -DNO_EXCEPTIONS -DNO_RTTI -DNO_THREADSAFE_STATICS -fsized-deallocation
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -DNO_EXCEPTIONS -DCOROUTINES -verify -fsized-deallocation %s
+// RUN:

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a subscriber: TomTan.
efriedma added a comment.

Missing testcase changes.  (It should be possible to check that we aren't 
inserting incorrect truncation/extension operations in the IR.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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

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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits

jyu2 updated this revision to Diff 185094.
jyu2 added a comment.

1> Add code for scope checking
2> Using CastInterator
3> CFG change to remove duplicate Block list


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

https://reviews.llvm.org/D56571

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

Index: test/Sema/scope-check.c
===
--- test/Sema/scope-check.c
+++ test/Sema/scope-check.c
@@ -232,3 +232,18 @@
 
 // rdar://9024687
 int test16(int [sizeof &&z]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+
+//Asm goto:
+int test16(int n)
+{
+  // expected-error@+2 {{cannot jump from this goto statement to its label}}
+  // expected-error@+1 {{cannot jump from this goto statement to its label}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+loop:
+  return 0;
+}
Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,28 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+  // expected-error@+1 {{operand with 'l' modifier must refer to a label}}
+  asm ("jne %l0"::"r"(&&lab));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  // expected-error@+1 {{operand with 'l' modifier must refer to a label}}
+  asm goto ("jne %l0"::"r"(x)::lab);
+  asm goto ("jne %l0"lab);
+}
Index: test/Parser/asm.cpp
===
--- test/Parser/asm.cpp
+++ test/Parser/asm.cpp
@@ -7,3 +7,54 @@
 int foo5 asm (U"bar5"); // expected-error {{cannot use unicode string literal in 'asm'}}
 int foo6 asm ("bar6"_x); // expected-error {{string literal with user-defined suffix cannot be used here}}
 int foo6 asm ("" L"bar7"); // expected-error {{cannot use wide string literal in 'asm'}}
+
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm goto ("testl %0, %0; jne %l3;" :: "r"(cond)::label_true, loop)
+  // expected-error@+1 {{unknown symbolic operand name in inline

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

How do we actually want this to work for LTO?  Would it make sense to encode 
this on global variables somehow, to allow different thresholds for different 
source files?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits

jyu2 marked 12 inline comments as done.
jyu2 added inline comments.



Comment at: include/clang/AST/Stmt.h:1008
+ (*iterator_adaptor_base::I)->getStmtClass() <= lastExprConstant);
+  return *reinterpret_cast(iterator_adaptor_base::I);
 }

rsmith wrote:
> Ugh. This cast violates strict aliasing, and even in the absence of strict 
> aliasing won't work unless the `Stmt` base class is at offset 0 in `T`. The 
> preceding assert is also wrong, as it's asserting that `*I` is an `Expr`, not 
> that it's a `T`.
> 
> After r352925, you can use `CastIterator` instead; that should 
> substantially reduce the churn in this patch.
Hi Richard,
Thanks for explain for this. And thank you so much for proving CastIterator!!  
I change the code to use this.




Comment at: include/clang/AST/Stmt.h:2842
+
+  bool isGCCAsmGoto() const {
+return NumLabels > 0;

rsmith wrote:
> This is already inside a class `GCCAsmStmt`; the `GCC` here seems unnecessary.
Remove GCC



Comment at: include/clang/AST/Stmt.h:2860
+  StringRef getLabelName(unsigned i) const;
+  Expr *getExpr(unsigned i) const;
+  using labels_iterator = ExprIterator;

rsmith wrote:
> Please give this a better name; `getExpr` is pretty meaningless.
should I change to getOperandExpr?
Thanks.



Comment at: lib/Analysis/CFG.cpp:1474
+  appendScopeBegin(JT.block, VD, G);
+  addSuccessor(B, JT.block);
+}

jyu2 wrote:
> efriedma wrote:
> > Please don't copy-paste code.
> :-(  changed
I did this during in VisitGCCAsmStmt by not call addSuccessor when we need 
backpatch the the labels.



Comment at: lib/Analysis/CFG.cpp:1466
+  LabelMapTy::iterator LI = LabelMap.find(E->getLabel());
+  if (LI == LabelMap.end()) continue;
+  JT = LI->second;

rsmith wrote:
> If this happens for every iteration of the loop, `VD` is used uninitialized 
> after the loop.
I am totally wrong.  Changed



Comment at: lib/Parse/ParseStmtAsm.cpp:839
+if (Tok.isNot(tok::r_paren)) {
+  while (1) {
+LabelDecl *LD = Actions.LookupOrCreateLabel(Tok.getIdentifierInfo(),

rsmith wrote:
> `while (true)`
Yes.  Changed.
Thanks.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

2019-02-04 Thread Marco Elver via Phabricator via cfe-commits

melver added a comment.

In D56935#1382156 , @philip.pfaffe 
wrote:

> Landed it for you in r352972. Thanks!


Great, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D56935



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

[clang-tools-extra] r353079 - [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via cfe-commits

Author: hwright
Date: Mon Feb  4 11:28:20 2019
New Revision: 353079

URL: http://llvm.org/viewvc/llvm-project?rev=353079&view=rev
Log:
[clang-tidy] Add the abseil-duration-unnecessary-conversion check

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

Added:

clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp

clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst

clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=353079&r1=353078&r2=353079&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Mon Feb  4 
11:28:20 2019
@@ -16,6 +16,7 @@
 #include "DurationFactoryFloatCheck.h"
 #include "DurationFactoryScaleCheck.h"
 #include "DurationSubtractionCheck.h"
+#include "DurationUnnecessaryConversionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
 #include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
@@ -45,6 +46,8 @@ public:
 "abseil-duration-factory-scale");
 CheckFactories.registerCheck(
 "abseil-duration-subtraction");
+CheckFactories.registerCheck(
+"abseil-duration-unnecessary-conversion");
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=353079&r1=353078&r2=353079&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Mon Feb  4 
11:28:20 2019
@@ -10,6 +10,7 @@ add_clang_library(clangTidyAbseilModule
   DurationFactoryScaleCheck.cpp
   DurationRewriter.cpp
   DurationSubtractionCheck.cpp
+  DurationUnnecessaryConversionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp?rev=353079&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
 (added)
+++ 
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
 Mon Feb  4 11:28:20 2019
@@ -0,0 +1,58 @@
+//===--- DurationUnnecessaryConversionCheck.cpp - clang-tidy
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DurationUnnecessaryConversionCheck.h"
+#include "DurationRewriter.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 abseil {
+
+void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) 
{
+  for (const auto &Scale : {"Hours", "Minutes", "Seconds", "Milliseconds",
+"Microseconds", "Nanoseconds"}) {
+std::string DurationFactory = (llvm::Twine("::absl::") + Scale).str();
+std::string FloatConversion =
+(llvm::Twine("::absl::ToDouble") + Scale).str();
+std::string IntegerConversion =
+(llvm::Twine("::absl::ToInt64") + Scale).str();
+
+Finder->addMatcher(
+callExpr(
+callee(functionDecl(hasName(DurationFactory))),
+hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
+FloatConversion, IntegerConversion))),
+hasArgument(0, expr().bind("arg")
+.bind("call"),
+this);
+  }
+}
+
+void DurationUnnecessaryConversionCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *OuterCall

[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353079: [clang-tidy] Add the 
abseil-duration-unnecessary-conversion check (authored by hwright, committed by 
).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D57353?vs=185057&id=185104#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57353

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-duration-unnecessary-conversion.cpp

Index: clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
===
--- clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
+++ clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
@@ -0,0 +1,58 @@
+//===--- DurationUnnecessaryConversionCheck.cpp - clang-tidy
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DurationUnnecessaryConversionCheck.h"
+#include "DurationRewriter.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 abseil {
+
+void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
+  for (const auto &Scale : {"Hours", "Minutes", "Seconds", "Milliseconds",
+"Microseconds", "Nanoseconds"}) {
+std::string DurationFactory = (llvm::Twine("::absl::") + Scale).str();
+std::string FloatConversion =
+(llvm::Twine("::absl::ToDouble") + Scale).str();
+std::string IntegerConversion =
+(llvm::Twine("::absl::ToInt64") + Scale).str();
+
+Finder->addMatcher(
+callExpr(
+callee(functionDecl(hasName(DurationFactory))),
+hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
+FloatConversion, IntegerConversion))),
+hasArgument(0, expr().bind("arg")
+.bind("call"),
+this);
+  }
+}
+
+void DurationUnnecessaryConversionCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *OuterCall = Result.Nodes.getNodeAs("call");
+  const auto *Arg = Result.Nodes.getNodeAs("arg");
+
+  if (!isNotInMacro(Result, OuterCall))
+return;
+
+  diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions")
+  << FixItHint::CreateReplacement(
+ OuterCall->getSourceRange(),
+ tooling::fixit::getText(*Arg, *Result.Context));
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/CMakeLists.txt
===
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -10,6 +10,7 @@
   DurationFactoryScaleCheck.cpp
   DurationRewriter.cpp
   DurationSubtractionCheck.cpp
+  DurationUnnecessaryConversionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp
Index: clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
===
--- clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
+++ clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
@@ -0,0 +1,35 @@
+//===--- DurationUnnecessaryConversionCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds and fixes cases where ``absl::Duration`` values are being converted
+/// to numeric types and back again.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-unnecessary-conversion.html
+class DurationUnnecessaryConversionCheck : public ClangTidyCheck {
+public:
+  DurationUnnecessaryConversionCheck(StringRef Name, ClangTidy

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:82
 
+- The :doc:`bugprone-argument-comment
+  ` now supports 

Please move changes in existing checks after list of new checks.


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

https://reviews.llvm.org/D57674



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Can you drop the file mode changes that are in this review?




Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:292-293
+// If the argument comments are missing for literals add them.
+if (Comments.empty()) {
+  if (((isa(Args[I]) && AddCommentsToBoolLiterals) ||
+   (isa(Args[I]) && AddCommentsToIntegerLiterals) ||

These can be combined into a single `if` statement.



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:300-301
+diag(Args[I]->getBeginLoc(),
+ "argument comment missing for literal argument"
+ " %0")
+<< II

Why is this split into two lines?



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:304
+<< FixItHint::CreateInsertion(Args[I]->getBeginLoc(), ArgComment);
+continue;
+  }

This `continue` can be dropped without changing the semantics, correct?



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.h:44-46
+  const bool AddCommentsToBoolLiterals;
+  const bool AddCommentsToIntegerLiterals;
+  const bool AddCommentsToFloatLiterals;

Why not character or string literals?
What about `nullptr` literals or UDLs?



Comment at: docs/clang-tidy/checks/bugprone-argument-comment.rst:40
+
+  void foo(bool turn_key,bool press_button);
+

Format the code examples from our style guide as well (same below).


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

https://reviews.llvm.org/D57674



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

[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space

2019-02-04 Thread John McCall via Phabricator via cfe-commits

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay.


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

https://reviews.llvm.org/D57524



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

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Richard felt that this was an odd special case, and I was happy to use the old 
language-designer's dodge of banning something today so that we can decide to 
allow it tomorrow.  This isn't SFINAE-able.

...of course, if it's just a *warning* that this isn't allowed, that dodge 
doesn't quite work.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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

[libunwind] r353084 - [CMake] Support CMake variables for setting target, sysroot and toolchain

2019-02-04 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Mon Feb  4 12:02:26 2019
New Revision: 353084

URL: http://llvm.org/viewvc/llvm-project?rev=353084&view=rev
Log:
[CMake] Support CMake variables for setting target, sysroot and toolchain

CMake has a standard way of setting target triple, sysroot and external
toolchain through CMAKE__COMPILER_TARGET, CMAKE_SYSROOT and
CMAKE__COMPILER_EXTERNAL_TOOLCHAIN. These are turned into
corresponding --target=, --sysroot= and --gcc-toolchain= variables add
included appended to CMAKE__FLAGS.

libunwind, libc++abi, libc++ provides their own mechanism through
_TARGET_TRIPLE, _SYSROOT and _GCC_TOOLCHAIN
variables. These are also passed to lit via lit.site.cfg, and lit config
uses these to set the corresponding compiler flags when building tessts.

This means that there are two different ways of setting target, sysroot
and toolchain, but only one is properly supported in lit. This change
extends CMake build for libunwind, libc++abi and libc++ to also support
the CMake variables in addition to project specific ones in lit.

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

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/test/lit.site.cfg.in

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=353084&r1=353083&r2=353084&view=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Mon Feb  4 12:02:26 2019
@@ -225,12 +225,22 @@ macro(add_target_flags_if condition var)
 endmacro()
 
 add_target_flags_if(LIBUNWIND_BUILD_32_BITS "-m32")
-add_target_flags_if(LIBUNWIND_TARGET_TRIPLE
-  "--target=${LIBUNWIND_TARGET_TRIPLE}")
-add_target_flags_if(LIBUNWIND_GCC_TOOLCHAIN
-  "--gcc-toolchain=${LIBUNWIND_GCC_TOOLCHAIN}")
-add_target_flags_if(LIBUNWIND_SYSROOT
-  "--sysroot=${LIBUNWIND_SYSROOT}")
+
+if(LIBUNWIND_TARGET_TRIPLE)
+  add_target_flags("--target=${LIBUNWIND_TARGET_TRIPLE}")
+elseif(CMAKE_CXX_COMPILER_TARGET)
+  set(LIBUNWIND_TARGET_TRIPLE "${CMAKE_CXX_COMPILER_TARGET}")
+endif()
+if(LIBUNWIND_GCC_TOOLCHAIN)
+  add_target_flags("--gcc-toolchain=${LIBUNWIND_GCC_TOOLCHAIN}")
+elseif(CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN)
+  set(LIBUNWIND_GCC_TOOLCHAIN "${CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN}")
+endif()
+if(LIBUNWIND_SYSROOT)
+  add_target_flags("--sysroot=${LIBUNWIND_SYSROOT}")
+elseif(CMAKE_SYSROOT)
+  set(LIBUNWIND_SYSROOT "${CMAKE_SYSROOT}")
+endif()
 
 if (LIBUNWIND_TARGET_TRIPLE)
   set(TARGET_TRIPLE "${LIBUNWIND_TARGET_TRIPLE}")

Modified: libunwind/trunk/test/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/test/lit.site.cfg.in?rev=353084&r1=353083&r2=353084&view=diff
==
--- libunwind/trunk/test/lit.site.cfg.in (original)
+++ libunwind/trunk/test/lit.site.cfg.in Mon Feb  4 12:02:26 2019
@@ -20,7 +20,7 @@ config.enable_shared= "@LIBC
 config.enable_exceptions= "@LIBUNWIND_ENABLE_EXCEPTIONS@"
 config.host_triple  = "@LLVM_HOST_TRIPLE@"
 config.target_triple= "@TARGET_TRIPLE@"
-config.use_target   = len("@LIBUNWIND_TARGET_TRIPLE@") > 0
+config.use_target   = bool("@LIBUNWIND_TARGET_TRIPLE@")
 config.sysroot  = "@LIBUNWIND_SYSROOT@"
 config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@"
 config.cxx_ext_threads  = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@"


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

[PATCH] D57631: [COFF, ARM64] Add ARM64 support for MS intrinsic _fastfail

2019-02-04 Thread Tom Tan via Phabricator via cfe-commits

TomTan added a comment.

In D57631#1381806 , @efriedma wrote:

> LGTM


Thanks Eli. Could you please help commit this change when it is ready?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57631



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

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Tom Tan via Phabricator via cfe-commits

TomTan added a comment.

Thanks for finding out and fixing this. Seems there is also issue in expanding 
`_WriteStatusReg` in `CodeGenFunction::EmitAArch64BuiltinExpr`. The last 
argument for `_WriteStatusReg` is __zero extended__ to `__in64`, which is not 
expected (see below link).

https://github.com/llvm-mirror/clang/blob/8070ca12f87e66f76db528c107e9d291f4a91498/lib/CodeGen/CGBuiltin.cpp#L7100


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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

[PATCH] D40814: [libcxx] Use the correct variable name for target triple in lit

2019-02-04 Thread Petr Hosek via Phabricator via cfe-commits

phosek abandoned this revision.
phosek added a comment.
Herald added subscribers: libcxx-commits, ldionne.

This is no longer needed after D57670  landed.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D40814



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

[PATCH] D40815: [libcxxabi] Use the correct variable name for target triple in lit

2019-02-04 Thread Petr Hosek via Phabricator via cfe-commits

phosek abandoned this revision.
phosek added a comment.
Herald added subscribers: libcxx-commits, ldionne.

This is no longer needed after D57670  landed.


Repository:
  rCXXA libc++abi

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

https://reviews.llvm.org/D40815



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

[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

2019-02-04 Thread Petr Hosek via Phabricator via cfe-commits

phosek abandoned this revision.
phosek added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is no longer needed after D57670  landed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40816



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

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Kelvin Li via Phabricator via cfe-commits

kkwli0 marked 2 inline comments as done.
kkwli0 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:7070
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&

ABataev wrote:
> just `if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind() || 
> OMPTeamsFound)`
Yep, it simplifies the logic.


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

https://reviews.llvm.org/D57690



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

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Kelvin Li via Phabricator via cfe-commits

kkwli0 updated this revision to Diff 185125.
kkwli0 marked an inline comment as done.
kkwli0 added a comment.

Update based on review comment.


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

https://reviews.llvm.org/D57690

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nesting_of_regions.cpp


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,9 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+OMPTeamsFound) {
+
   OMPTeamsFound = false;
   break;
 }


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,9 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+OMPTeamsFound) {
+
   OMPTeamsFound = false;
   break;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


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

https://reviews.llvm.org/D57690



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

r353088 - Generalize pthread callback test case

2019-02-04 Thread Johannes Doerfert via cfe-commits

Author: jdoerfert
Date: Mon Feb  4 12:42:38 2019
New Revision: 353088

URL: http://llvm.org/viewvc/llvm-project?rev=353088&view=rev
Log:
Generalize pthread callback test case

Changes suggested by Eli Friedman 

Modified:
cfe/trunk/test/CodeGen/callback_pthread_create.c

Modified: cfe/trunk/test/CodeGen/callback_pthread_create.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/callback_pthread_create.c?rev=353088&r1=353087&r2=353088&view=diff
==
--- cfe/trunk/test/CodeGen/callback_pthread_create.c (original)
+++ cfe/trunk/test/CodeGen/callback_pthread_create.c Mon Feb  4 12:42:38 2019
@@ -1,14 +1,22 @@
-// RUN: %clang -O1 %s -S -c -emit-llvm -o - | FileCheck %s
-// RUN: %clang -O1 %s -S -c -emit-llvm -o - | opt -ipconstprop -S | FileCheck 
--check-prefix=IPCP %s
-
-// This is a linux only test for now due to the include.
-// UNSUPPORTED: !linux
+// RUN: %clang_cc1 -O1 %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -O1 %s -S -emit-llvm -o - | opt -ipconstprop -S | FileCheck 
--check-prefix=IPCP %s
 
 // CHECK: declare !callback ![[cid:[0-9]+]] {{.*}}i32 @pthread_create
 // CHECK: ![[cid]] = !{![[cidb:[0-9]+]]}
 // CHECK: ![[cidb]] = !{i64 2, i64 3, i1 false}
 
-#include 
+// Taken from test/Analysis/retain-release.m
+//{
+struct _opaque_pthread_t {};
+struct _opaque_pthread_attr_t {};
+typedef struct _opaque_pthread_t *__darwin_pthread_t;
+typedef struct _opaque_pthread_attr_t __darwin_pthread_attr_t;
+typedef __darwin_pthread_t pthread_t;
+typedef __darwin_pthread_attr_t pthread_attr_t;
+
+int pthread_create(pthread_t *, const pthread_attr_t *,
+   void *(*)(void *), void *);
+//}
 
 const int GlobalVar = 0;
 
@@ -26,8 +34,8 @@ static void *callee1(void *payload) {
 
 void foo() {
   pthread_t MyFirstThread;
-  pthread_create(&MyFirstThread, NULL, callee0, NULL);
+  pthread_create(&MyFirstThread, 0, callee0, 0);
 
   pthread_t MySecondThread;
-  pthread_create(&MySecondThread, NULL, callee1, (void *)&GlobalVar);
+  pthread_create(&MySecondThread, 0, callee1, (void *)&GlobalVar);
 }


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

Re: r351629 - Emit !callback metadata and introduce the callback attribute

2019-02-04 Thread Doerfert, Johannes via cfe-commits

Should be fixed in r353088 (git c3707cc5d91). Can you verify it now is as you 
expect it?

From: Eli Friedman 
Sent: Thursday, January 31, 2019 18:17
To: Doerfert, Johannes; Chandler Carruth; cfe-commits@lists.llvm.org
Cc: Raja Venkateswaran
Subject: RE: r351629 - Emit !callback metadata and introduce the callback 
attribute

(Comments inline.)

> -Original Message-
> From: cfe-commits  On Behalf Of
> Doerfert, Johannes Rudolf via cfe-commits
> Sent: Tuesday, January 22, 2019 9:29 AM
> To: Chandler Carruth 
> Cc: cfe-commits cfe 
> Subject: Re: r351629 - Emit !callback metadata and introduce the callback
> attribute
>
> > Another thing I notecide is that this code assumes the system has
> > `pthread.h` -- what about systems without it? I mean, you can disable the
> > test, but it seems bad to lose test coverage just because of that.
>
> So far, I disabled the test with a later addition which makes sure this
> test is only run under Linux. I'm unsure why we loose coverage because
> of that?

There isn't any way to safely disable the test without disabling it everywhere. 
 Specifically, the current version requires both that the host is Unix, and 
that the default target is the host.  Otherwise, the compiler might not be able 
to find and/or build pthread.h .

>
> > I would much prefer that you provide your own stub `pthread.h` in the
> > Inputs/... tree of the test suite and use that to test this in a portable
> > way.
>
> I do not completely follow but I'm open to improving the test. Basically
> I have to make sure the builtin recognition will trigger on the header
> file and the contained declaration. If we can somehow do this in a
> portable way I'm all for it. Is that how we test other builtin gnu extensions?

You don't need a header file; clang doesn't actually care where the builtin is 
declared.  You can just write "void pthread_create(void*,void*,void*(void*), 
void*);" or whatever directly in the C file.  It'll trigger a warning, but with 
your patch, there's no way to declare pthread_create without triggering a 
warning, so that hardly matters.

On a related note, code generation tests should use %clang_cc1.  Among other 
things, this avoids accidentally including headers from the host system.

-Eli

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

r353090 - [NewPM][MSan] Add Options Handling

2019-02-04 Thread Philip Pfaffe via cfe-commits

Author: pfaffe
Date: Mon Feb  4 13:02:49 2019
New Revision: 353090

URL: http://llvm.org/viewvc/llvm-project?rev=353090&view=rev
Log:
[NewPM][MSan] Add Options Handling

Summary: This patch enables passing options to msan via the passes pipeline, 
e.e., -passes=msan.

Reviewers: chandlerc, fedor.sergeev, leonardchan

Subscribers: hiraditya, bollu, llvm-commits

Tags: #llvm

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=353090&r1=353089&r2=353090&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Feb  4 13:02:49 2019
@@ -279,7 +279,8 @@ static void addGeneralOptsForMemorySanit
   const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, 
CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.
@@ -1035,7 +1036,7 @@ void EmitAssemblyHelper::EmitAssemblyWit
   if (LangOpts.Sanitize.has(SanitizerKind::Memory))
 PB.registerOptimizerLastEPCallback(
 [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });
   if (LangOpts.Sanitize.has(SanitizerKind::Thread))
 PB.registerOptimizerLastEPCallback(


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

[PATCH] D57640: [NewPM][MSan] Add Options Handling

2019-02-04 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353090: [NewPM][MSan] Add Options Handling (authored by 
pfaffe, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57640?vs=185133&id=185139#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57640

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -279,7 +279,8 @@
   const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, 
CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.
@@ -1035,7 +1036,7 @@
   if (LangOpts.Sanitize.has(SanitizerKind::Memory))
 PB.registerOptimizerLastEPCallback(
 [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });
   if (LangOpts.Sanitize.has(SanitizerKind::Thread))
 PB.registerOptimizerLastEPCallback(


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -279,7 +279,8 @@
   const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.
@@ -1035,7 +1036,7 @@
   if (LangOpts.Sanitize.has(SanitizerKind::Memory))
 PB.registerOptimizerLastEPCallback(
 [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) {
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });
   if (LangOpts.Sanitize.has(SanitizerKind::Thread))
 PB.registerOptimizerLastEPCallback(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

2019-02-04 Thread David Blaikie via Phabricator via cfe-commits

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Seems reasonable to me - but if you like you can wait for Richard who might 
have a more nuanced understanding of the code. (but I'm OK signing off on this 
if you are & Richard can provide any extra feedback post-commit)




Comment at: clang/lib/CodeGen/CGCall.cpp:3837
+// having pointee types).
+llvm::FunctionType *IRFuncTyFromInfo = 
getTypes().GetFunctionType(CallInfo);
+assert(IRFuncTy == IRFuncTyFromInfo);

This will be warned as unused in a release build.

Would this be hideous if it's just all one big assert?

  assert((CallInfo.isVariadic && CallInfo.getArgStruct) || IRFuncTy == 
getTypes().GetFunctionType(CallInfo));

(I think that's accurate?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57664



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

[clang-tools-extra] r353092 - [clang-tidy] Handle unions with existing default-member-init

2019-02-04 Thread Malcolm Parsons via cfe-commits

Author: malcolm.parsons
Date: Mon Feb  4 13:09:31 2019
New Revision: 353092

URL: http://llvm.org/viewvc/llvm-project?rev=353092&view=rev
Log:
[clang-tidy] Handle unions with existing default-member-init

Summary:
clang-tidy's modernize-use-default-member-init was crashing for unions
with an existing default member initializer.

Fixes PR40492

Reviewers: aaron.ballman, alexfh, JonasToth

Reviewed By: JonasToth

Subscribers: JonasToth, riccibruno, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp?rev=353092&r1=353091&r2=353092&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
Mon Feb  4 13:09:31 2019
@@ -273,7 +273,7 @@ void UseDefaultMemberInitCheck::checkDef
 
 void UseDefaultMemberInitCheck::checkExistingInit(
 const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
-  const FieldDecl *Field = Init->getMember();
+  const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
 return;

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp?rev=353092&r1=353091&r2=353092&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
Mon Feb  4 13:09:31 2019
@@ -382,6 +382,16 @@ struct ExistingString {
   const char *e4 = "bar";
 };
 
+struct UnionExisting {
+  UnionExisting() : e(5.0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: member initializer for 'e' is 
redundant
+  // CHECK-FIXES: UnionExisting()  {}
+  union {
+int i;
+double e = 5.0;
+  };
+};
+
 template 
 struct NegativeTemplateExisting {
   NegativeTemplateExisting(int) : t(0) {}


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

[PATCH] D57665: [clang-tidy] Handle unions with existing default-member-init

2019-02-04 Thread Malcolm Parsons via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353092: [clang-tidy] Handle unions with existing 
default-member-init (authored by malcolm.parsons, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57665?vs=184976&id=185141#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57665

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -273,7 +273,7 @@
 
 void UseDefaultMemberInitCheck::checkExistingInit(
 const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
-  const FieldDecl *Field = Init->getMember();
+  const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
 return;
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -382,6 +382,16 @@
   const char *e4 = "bar";
 };
 
+struct UnionExisting {
+  UnionExisting() : e(5.0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: member initializer for 'e' is 
redundant
+  // CHECK-FIXES: UnionExisting()  {}
+  union {
+int i;
+double e = 5.0;
+  };
+};
+
 template 
 struct NegativeTemplateExisting {
   NegativeTemplateExisting(int) : t(0) {}


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -273,7 +273,7 @@
 
 void UseDefaultMemberInitCheck::checkExistingInit(
 const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
-  const FieldDecl *Field = Init->getMember();
+  const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
 return;
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -382,6 +382,16 @@
   const char *e4 = "bar";
 };
 
+struct UnionExisting {
+  UnionExisting() : e(5.0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: member initializer for 'e' is redundant
+  // CHECK-FIXES: UnionExisting()  {}
+  union {
+int i;
+double e = 5.0;
+  };
+};
+
 template 
 struct NegativeTemplateExisting {
   NegativeTemplateExisting(int) : t(0) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

Yes, we should fix CodeGenFunction::EmitAArch64BuiltinExpr to eliminated the 
unnecessary calls to CreateZext/CreateTrunc.  (With this patch, they're no-ops, 
but better to clean up the code.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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

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

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added inline comments.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

This doesn't look right; I think we need to add it to IndirectJumps instead.  
This probably impacts a testcase like the following:

```
struct S { ~S(); };
int f() {
  {
S s;
asm goto(""BAR);
return 1;
  }
BAR:
  return 0;
}
```

(gcc currently accepts this and skips running the destructor, but I'm pretty 
sure that's a bug.)


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

https://reviews.llvm.org/D56571



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

[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

2019-02-04 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:1028-1030
+llvm::Constant *getPropertyFn = cast(
+CGM.getObjCRuntime().GetPropertyGetFunction().getCallee());
 if (!getPropertyFn) {

This code looks like it implies that the 'if' is never hit? (since cast doesn't 
propagate null (it asserts/fails/UB on null)) - should this be cast_or_null 
instead? Or "if(!CGM.getObjCRuntime().getPropertyGetFunction())" ?)

(there are a few similar instances later on)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57668



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 185148.
MyDeveloperDay edited the summary of this revision.
MyDeveloperDay added a comment.

Address review comments
Add support for additional StringLiterals,CharacterLiterals,UserDefinedLiterals 
and NullPtrs
Simplify the Options names from  "AddCommentsToXXX" to "CommentXXX"
Add extra unit tests to support additional supported literal types


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

https://reviews.llvm.org/D57674

Files:
  clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tidy/bugprone/ArgumentCommentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-argument-comment.rst
  test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+  void foo(const char *strabc);
+  void fooW(const wchar_t *wstrabc);
+  void fooPtr(A *ptrabc);
+  void foo(char chabc);
+};
+
+double operator"" _km(long double);
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+
+  a.foo("Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*strabc=*/"Hello World");
+  //
+  a.fooW(L"Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: argument comment missing for literal argument 'wstrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooW(/*wstrabc=*/L"Hello World");
+
+  a.fooPtr(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: argument comment missing for literal argument 'ptrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooPtr(/*ptrabc=*/nullptr);
+
+  a.foo(402.0_km);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
+
+  a.foo('A');
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument commen

[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Julian Lettner via Phabricator via cfe-commits

yln created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a follow up for https://reviews.llvm.org/D57278. The previous
revision should have also included Kernel ASan.

rdar://problem/40723397


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57711

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/ubsan-asan-noreturn.c
  llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll


Index: llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
@@ -35,6 +35,15 @@
 ; CHECK-NOT:call void @__asan_handle_no_return
 ; CHECK:call void @NoReturnFunc
 
+; Do *not* instrument functions without ASan
+define i32 @Call4() {
+  call void @NoReturnFunc() noreturn
+  unreachable
+}
+; CHECK-LABEL:  @Call4
+; CHECK-NOT:call void @__asan_handle_no_return
+; CHECK:call void @NoReturnFunc
+
 declare i32 @__gxx_personality_v0(...)
 
 define i64 @Invoke1() nounwind uwtable ssp sanitize_address personality i8* 
bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
Index: clang/test/CodeGen/ubsan-asan-noreturn.c
===
--- clang/test/CodeGen/ubsan-asan-noreturn.c
+++ clang/test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);


Index: llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
@@ -35,6 +35,15 @@
 ; CHECK-NOT:call void @__asan_handle_no_return
 ; CHECK:call void @NoReturnFunc
 
+; Do *not* instrument functions without ASan
+define i32 @Call4() {
+  call void @NoReturnFunc() noreturn
+  unreachable
+}
+; CHECK-LABEL:  @Call4
+; CHECK-NOT:call void @__asan_handle_no_return
+; CHECK:call void @NoReturnFunc
+
 declare i32 @__gxx_personality_v0(...)
 
 define i64 @Invoke1() nounwind uwtable ssp sanitize_address personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
Index: clang/test/CodeGen/ubsan-asan-noreturn.c
===
--- clang/test/CodeGen/ubsan-asan-noreturn.c
+++ clang/test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

2019-02-04 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

Fixes rdar://47713266

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D57712

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/infer-availability-from-init.m


Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- clang/test/SemaObjC/infer-availability-from-init.m
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }


Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- clang/test/SemaObjC/infer-availability-from-init.m
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

2019-02-04 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM, Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57712



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

[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Vedant Kumar via Phabricator via cfe-commits

vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

LGTM, thanks. It might make sense to add a predicate to SanitizerSet to make it 
easier to check when ASan is enabled, either pre-commit or as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57711



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

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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits

jyu2 marked 2 inline comments as done.
jyu2 added inline comments.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

efriedma wrote:
> This doesn't look right; I think we need to add it to IndirectJumps instead.  
> This probably impacts a testcase like the following:
> 
> ```
> struct S { ~S(); };
> int f() {
>   {
> S s;
> asm goto(""BAR);
> return 1;
>   }
> BAR:
>   return 0;
> }
> ```
> 
> (gcc currently accepts this and skips running the destructor, but I'm pretty 
> sure that's a bug.)
Hi Eli,
I see both g++ and clang++ with my change call ~S.  Am I missing something?  
Thanks.

1>clang++ j.cpp
1>./a.out
~S()
1>g++ j.cpp
1>./a.out
~S()

Here is the test case:
1>cat j.cpp
extern "C" int printf (const char *,...);
struct S { ~S() {printf("~S()\n");}; };
int f() {
  {
S s;
asm goto(""BAR);
return 1;
  }
BAR:
  return 0;
}

int main()
{
  f();
}



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

https://reviews.llvm.org/D56571



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

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

2019-02-04 Thread Russell McClellan via Phabricator via cfe-commits

russellmcc added a comment.

Bump!  Thanks for the consideration.


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

https://reviews.llvm.org/D40988



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

2019-02-04 Thread Russell McClellan via Phabricator via cfe-commits

russellmcc added a comment.

Bump!  Thanks for the consideration


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

https://reviews.llvm.org/D54881



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

[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Evgenii Stepanov via Phabricator via cfe-commits

eugenis accepted this revision.
eugenis added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57711



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

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

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: lib/AST/Stmt.cpp:457-460
   this->NumOutputs = NumOutputs;
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
+  this->NumLabels = NumLabels;

Please assert that you don't have both outputs and labels here. (If you did, 
you would assign them the same slots within `Exprs`.)

You also seem to be missing `Sema` enforcement of the rule that you cannot have 
both outputs and labels. (If you want to actually support that as an 
intentional extension to the GCC functionality, you should change the 
implementation of `GCCAsmStmt` to use different slots for outputs and labels, 
add some tests, and add a `-Wgcc-compat` warning for that case. Seems better to 
just add an error for it for now.)



Comment at: lib/Analysis/CFG.cpp:1445-1458
+  LabelMapTy::iterator LI = LabelMap.find(G->getLabel());;
+  // If there is no target for the goto, then we are looking at an
+  // incomplete AST.  Handle this by not registering a successor.
+  if (LI == LabelMap.end()) continue;
 
-JumpTarget JT = LI->second;
-prependAutomaticObjLifetimeWithTerminator(B, I->scopePosition,
-  JT.scopePosition);
-prependAutomaticObjDtorsWithTerminator(B, I->scopePosition,
-   JT.scopePosition);
-const VarDecl *VD = prependAutomaticObjScopeEndWithTerminator(
-B, I->scopePosition, JT.scopePosition);
-appendScopeBegin(JT.block, VD, G);
-addSuccessor(B, JT.block);
+  JumpTarget JT = LI->second;
+  prependAutomaticObjLifetimeWithTerminator(B, I->scopePosition,

Please factor out this duplicated code into a local lambda rather than 
copy-pasting it.



Comment at: lib/Analysis/CFG.cpp:1461
+if (auto *G = dyn_cast(B->getTerminator())) {
+  if (G->isAsmGoto()) {
+for (auto *E : G->labels()) {

This check is redundant; there will be no labels if it's not an `asm goto` so 
you can just perform the below loop unconditionally.



Comment at: lib/Analysis/CFG.cpp:3139-3141
+  // We will need to backpatch this block later.
+  BackpatchBlocks.push_back(JumpSource(Block, ScopePos));
+  return Block;

Do we not need the handling for the other labels if we hit this condition for 
one label?



Comment at: lib/Sema/SemaStmtAsm.cpp:656
+  // Check for duplicate asm operand name between input, output and label 
lists.
+  typedef std::pair  MyItemType;
+  SmallVector MyList;

No space after `pair`, please.



Comment at: lib/Sema/SemaStmtAsm.cpp:659-662
+  for (unsigned i = 0, e = NumOutputs + NumInputs + NumLabels; i != e; ++i)
+if (NS->getIdentifier(i) && !NS->getIdentifier(i)->getName().empty())
+  MyList.emplace_back(std::make_pair(NS->getIdentifier(i)->getName(),
+ NS->getOperandExpr(i)));

Looping over all three lists of expressions here is effectively hardcoding an 
implementation detail of `GCCAsmStmt` into this code, violating its 
encapsulation. Instead, please write three loops (iterating over the inputs, 
outputs, and labels), and remove `getOperandExpr` entirely.



Comment at: lib/Sema/SemaStmtAsm.cpp:660
+  for (unsigned i = 0, e = NumOutputs + NumInputs + NumLabels; i != e; ++i)
+if (NS->getIdentifier(i) && !NS->getIdentifier(i)->getName().empty())
+  MyList.emplace_back(std::make_pair(NS->getIdentifier(i)->getName(),

Checking whether the name is empty is redundant: we should never create an 
`IdentifierInfo` representing an empty string.


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

https://reviews.llvm.org/D56571



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

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-02-04 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

I'll do the honors. Thanks!


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

https://reviews.llvm.org/D54429



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

[PATCH] D57219: [Fixed Point Arithmetic] Fixed Point Comparisons

2019-02-04 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3444
   case BO_NE:
+return Builder.CreateICmpNE(FullLHS, FullRHS);
+  case BO_Mul:

leonardchan wrote:
> rjmccall wrote:
> > leonardchan wrote:
> > > rjmccall wrote:
> > > > Are padding bits guaranteed zero or unspecified?  Or are we just not 
> > > > really supporting padding bits all the way to IRGen at this time?
> > > I believe the consensus was leaving them unspecified, so operations that 
> > > can cause overflow into them would result in undefined behavior.
> > If I'm understanding you correctly, you're saying that (1) we are assuming 
> > that inputs are zero-padded and (2) we are taking advantage of the 
> > non-saturating-overflow-is-UB rule to avoid clearing the padding bits after 
> > arithmetic.  That's actually what I meant by "guaranteed zero", so we have 
> > our labels reversed, but at least we understand each other now.
> > 
> > (I suppose you're thinking of this as "unspecified" because non-saturating 
> > arithmetic can leave an unspecified value there.  I think of this as a 
> > "guarantee" because it's basically a representational invariant: it's a 
> > precondition for correctness that the bit is zero, and all operations 
> > guarantee that the bit will be zero in their well-defined cases (but 
> > overflow is not well-defined).  Just a difference in perspective, I guess.)
> > 
> > Is this written down somewhere?  Are there targets that use the opposite 
> > ABI rule that we might need to support someday?
> > 
> > At any rate, I think it's worth adding a short comment here explaining why 
> > it's okay to do a normal comparison despite the possibility of padding 
> > bits.  Along those lines, is there any value in communicating to the 
> > backend that padded-unsigned comparisons could reasonably be done with 
> > either a signed or unsigned comparison?  Are there interesting targets 
> > where one or the other is cheaper?
> Yes. We assume inputs are zero padded, and that non-saturating overflow is 
> undefined so we do not need to clear the padding after writing a new value. 
> Sorry for the misunderstanding. I see what you mean by guaranteed zero.
> 
> Overflow in general is controlled by the `FX_FRACT_OVERFLOW` and 
> `FX_ACCUM_OVERFLOW` pragmas, where if these are set to `DEFAULT`, operations 
> that can overflow with these types is undefined according to the standard 
> (4.1.3). In terms of padding bits, clause 6.2.6.3 mentions that the values of 
> padding bits are "unspecified". I imagine this means that we can assume the 
> value to be whatever we want it to, so we can assume that these values are a 
> guaranteed zero.
> 
> I believe @ebevhan requested this being added since his downstream 
> implementation used padding to match the scales of signed and unsigned types, 
> so he may be able to offer more information on targets with different ABIs. 
> We don't plan to use any special hardware, so we're following the "typical 
> desktop processor" layout that uses the whole underlying int and no padding 
> (mentioned in Annex 3).
> 
> In the same section, the standard also mentions types for other targets that 
> may have padding, so there could be some value in indicating to the backend 
> that for these particular targets, this part of the operand is padding, so 
> select an appropriate operation that performs a comparison on this size type. 
> But I don't know much about these processors and would just be guessing at 
> how they work.
Okay.  If we ever have a target that uses an ABI that (1) includes padding but 
(2) considers it to be "unspecified" in the sense of "it can validly be 
non-zero", we'll have to change this code, but I agree it's the right move to 
not preemptively generalize to cover that possibility.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57219



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

[PATCH] D57716: Let AMDGPU compile MSVC headers containing vectorcall

2019-02-04 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added a reviewer: rjmccall.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely, 
kzhuravl.

MSVC header files using vectorcall to differentiate overloaded functions, which
causes failure for AMDGPU target. Let AMDGPU target recognize vectorcall
calling convention so that these header files can be compiled.


https://reviews.llvm.org/D57716

Files:
  include/clang/Basic/Specifiers.h
  lib/AST/ItaniumMangle.cpp
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCUDA/amdgpu-windows-vectorcall.cu

Index: test/SemaCUDA/amdgpu-windows-vectorcall.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-windows-vectorcall.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+template
+ struct A
+ {
+ };
+
+template struct A<_Ret (__cdecl _Arg0::*)(_Types) > { }; 
+template struct A<_Ret (__vectorcall _Arg0::*)(_Types) > {};
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4568,7 +4568,8 @@
 CC = CC_Swift;
 break;
   case ParsedAttr::AT_VectorCall:
-CC = CC_X86VectorCall;
+CC = Context.getTargetInfo().getTriple().isAMDGPU()
+  ? CC_AMDGPUVectorCall : CC_X86VectorCall;
 break;
   case ParsedAttr::AT_AArch64VectorPcs:
 CC = CC_AArch64VectorCall;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -1113,6 +1113,7 @@
 static unsigned getDwarfCC(CallingConv CC) {
   switch (CC) {
   case CC_C:
+  case CC_AMDGPUVectorCall:
 // Avoid emitting DW_AT_calling_convention if the C convention was used.
 return 0;
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -58,6 +58,7 @@
   case CC_X86Pascal: return llvm::CallingConv::C;
   // TODO: Add support for __vectorcall to LLVM.
   case CC_X86VectorCall: return llvm::CallingConv::X86_VectorCall;
+  case CC_AMDGPUVectorCall: return llvm::CallingConv::C;
   case CC_AArch64VectorCall: return llvm::CallingConv::AArch64_VectorCall;
   case CC_SpirFunction: return llvm::CallingConv::SPIR_FUNC;
   case CC_OpenCLKernel: return CGM.getTargetCodeGenInfo().getOpenCLKernelCallingConv();
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -341,6 +341,7 @@
   return CCCR_Warning;
 case CC_C:
 case CC_OpenCLKernel:
+case CC_AMDGPUVectorCall:
   return CCCR_OK;
 }
   }
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -862,6 +862,7 @@
   OS << " __attribute__((thiscall))";
   break;
 case CC_X86VectorCall:
+case CC_AMDGPUVectorCall:
   OS << " __attribute__((vectorcall))";
   break;
 case CC_X86Pascal:
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2826,6 +2826,7 @@
   case CC_X86ThisCall: return "thiscall";
   case CC_X86Pascal: return "pascal";
   case CC_X86VectorCall: return "vectorcall";
+  case CC_AMDGPUVectorCall: return "amdgpu_vectorcall";
   case CC_Win64: return "ms_abi";
   case CC_X86_64SysV: return "sysv_abi";
   case CC_X86RegCall : return "regcall";
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2648,6 +2648,7 @@
 return "";
 
   case CC_X86VectorCall:
+  case CC_AMDGPUVectorCall:
   case CC_X86Pascal:
   case CC_X86RegCall:
   case CC_AAPCS:
Index: include/clang/Basic/Specifiers.h
===
--- include/clang/Basic/Specifiers.h
+++ include/clang/Basic/Specifiers.h
@@ -251,6 +251,7 @@
 CC_PreserveMost, // __attribute__((preserve_most))
 CC_PreserveAll,  // __attribute__((preserve_all))
 CC_AArch64VectorCall, // __attribute__((aarch64_vector_pcs))
+CC_AMDGPUVectorCall, // __attribute__((vectorcall))
   };
 
   /// Checks whether the given calling convention supports variadic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak updated this revision to Diff 185157.
ahatanak added a comment.

Add a note diagnostic to inform the user of the reason for not allowing 
annotating a class with `trivial_abi`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626

Files:
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  test/SemaObjCXX/attr-trivial-abi.mm

Index: test/SemaObjCXX/attr-trivial-abi.mm
===
--- test/SemaObjCXX/attr-trivial-abi.mm
+++ test/SemaObjCXX/attr-trivial-abi.mm
@@ -10,23 +10,23 @@
   int a;
 };
 
-struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}}
+struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}}
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{are polymorphic}}
   virtual void m();
 };
 
 struct S3_2 {
   virtual void m();
-} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}}
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{are polymorphic}}
 
 struct S4 {
   int a;
 };
 
-struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}}
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{have virtual bases}}
 };
 
 struct __attribute__((trivial_abi)) S9 : public S4 {
@@ -36,19 +36,19 @@
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}}
+struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}}
+struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}} expected-note {{have __weak fields under ARC}}
   __weak id a[2];
 };
 
-struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}}
+struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}} expected-note {{have fields of non-trivial class types}}
   S6 a;
 };
 
-struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}}
+struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}} expected-note {{have fields of non-trivial class types}}
   S6 a[2];
 };
 
@@ -66,7 +66,7 @@
 S10<__weak id> p2;
 
 template<>
-struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}}
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
@@ -90,8 +90,41 @@
 S16 s16;
 
 template
-struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}}
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
 S17 s17;
+
+namespace deletedCopyMoveConstructor {
+  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{"don't have non-deleted copy/move constructors}}
+CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{"don't have non-deleted copy/move constructors}}
+CopyMoveDeleted a;
+S18(const S18 &);
+S18(S18 &&);
+  };
+
+  struct __attribute__((trivial_abi)) CopyDeleted {
+CopyDeleted(const CopyDeleted &) = delete;
+CopyDeleted(CopyDeleted &&) = default;
+  };
+
+  struct __attribute__((trivial_abi)) MoveDeleted {
+MoveDeleted(const MoveDeleted &) = default;
+MoveDeleted(MoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{"don't have non-deleted copy/move constructors}}
+CopyDeleted a;
+MoveDeleted b;
+  };
+
+  // This is fine since the move constructor isn't deleted.
+  struct __attribute__((trivial_abi)) S20 {
+

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953
+  "have bases of non-trivial class types|have virtual bases|"
+  "have __weak fields under ARC|have fields of non-trivial class types}0">;
 

nit: "of non-trivial class types" should be "of non-trivial class type" in both 
places.

And I would write "are not move-constructible" rather than "don't have 
non-deleted copy/move constructors". Double negations aren't non-bad.

Actually I would rephrase this as `'trivial_abi' is disallowed on this class 
because it %select{is not move-constructible|is polymorphic|has a base of 
non-trivial class type|has a virtual base|has a __weak field|has a field of 
non-trivial class type}`, i.e., we're not just giving information about 
"classes" in general, we're talking about "this class" specifically. We could 
even name the class if we're feeling generous.



Comment at: lib/Sema/SemaDeclCXX.cpp:7886
+return false;
+  };
+

How confident are we that this logic is correct?  I ask because I need 
something similar for my own diagnostic in D50119. If this logic is rock-solid 
(no lurking corner-case bugs), I should copy it — and/or it should be moved 
into a helper member function on `CXXRecordDecl` so that other people can call 
it too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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

[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: test/CodeGen/ms-inline-asm.c:574
 // CHECK: fistp dword ptr $0
-// CHECK: "=*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* %{{.*}})
+// CHECK: "=*m,*m,~{fpsr},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* 
%{{.*}})
 }

Two fpsr? If we manually add that as an extra constraint, maybe we should 
remove that logic in this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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

[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Craig Topper via Phabricator via cfe-commits

craig.topper marked an inline comment as done.
craig.topper added inline comments.



Comment at: test/CodeGen/ms-inline-asm.c:574
 // CHECK: fistp dword ptr $0
-// CHECK: "=*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* %{{.*}})
+// CHECK: "=*m,*m,~{fpsr},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* 
%{{.*}})
 }

rnk wrote:
> Two fpsr? If we manually add that as an extra constraint, maybe we should 
> remove that logic in this change.
We slap "~{dirflag},~{fpsr},~{flags}" blindly onto both gcc and MS inline 
assembly. There are tests cases in this file with two ~{flags} as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

In D57626#1382391 , @Quuxplusone wrote:

> I admit that this `lock_guard` example is contrived and generally ill-advised 
> ,
>  but its ill-advisedness seems like a higher-level concern that shouldn't be 
> "enforced" by fiddling with the rules of [[trivial_abi]], so I hope that's 
> not what's going on here.

`[[trivial_abi]]` (at least right now) only affects whether user-provided 
special member functions are considered to be trivial for ABI purposes. A class 
whose copy and move constructor are both deleted is not passed in registers by 
the ABI; that has nothing to do with triviality, so it's unaffected by 
`[[trivial_abi]]` as currently specified. We could "fiddle with the rules of 
[[trivial_abi]]" to *make* that work (that's what the previous approach for 
this case did), but as you note, this is an ill-advised case, and fiddling with 
the rules to give it special behavior doesn't seem like worthwhile complexity. 
Our design intent was to produce a diagnostic if `[[trivial_abi]]` is specified 
on a non-template class that we can't actually pass in registers; this patch 
fixes a hole in our implementation of that design by adding a missing 
diagnostic.

Repository:
  rC Clang

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

https://reviews.llvm.org/D57626

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

[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:637-639
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string &C) {
+return C == "mxcsr";

In light of this comment about "clang always adding fpsr anyway" I'd rather 
keep the fpsr filter here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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

r353115 - [SemaObjC] Don't infer the availabilty of +new from -init if the receiver has Class type

2019-02-04 Thread Erik Pilkington via cfe-commits

Author: epilk
Date: Mon Feb  4 15:30:57 2019
New Revision: 353115

URL: http://llvm.org/viewvc/llvm-project?rev=353115&view=rev
Log:
[SemaObjC] Don't infer the availabilty of +new from -init if the receiver has 
Class type

rdar://47713266

Differential revision: https://reviews.llvm.org/D57712

Modified:
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/test/SemaObjC/infer-availability-from-init.m

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=353115&r1=353114&r2=353115&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Mon Feb  4 15:30:57 2019
@@ -2805,8 +2805,8 @@ ExprResult Sema::BuildInstanceMessage(Ex
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@ ExprResult Sema::BuildInstanceMessage(Ex
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }

Modified: cfe/trunk/test/SemaObjC/infer-availability-from-init.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/infer-availability-from-init.m?rev=353115&r1=353114&r2=353115&view=diff
==
--- cfe/trunk/test/SemaObjC/infer-availability-from-init.m (original)
+++ cfe/trunk/test/SemaObjC/infer-availability-from-init.m Mon Feb  4 15:30:57 
2019
@@ -56,3 +56,16 @@ void usenotmyobject() {
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end


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

[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

2019-02-04 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353115: [SemaObjC] Don't infer the availabilty of +new 
from -init if the receiver has… (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57712?vs=185150&id=185175#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57712

Files:
  lib/Sema/SemaExprObjC.cpp
  test/SemaObjC/infer-availability-from-init.m


Index: test/SemaObjC/infer-availability-from-init.m
===
--- test/SemaObjC/infer-availability-from-init.m
+++ test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }


Index: test/SemaObjC/infer-availability-from-init.m
===
--- test/SemaObjC/infer-availability-from-init.m
+++ test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56555: Add Attribute to define nonlazy objc classes

2019-02-04 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353116: [OBJC] Add attribute to mark Objective C class as 
non-lazy (authored by joseph_daniels, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56555?vs=184671&id=185177#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56555

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/lib/CodeGen/CGObjCMac.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
  cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
  cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m

Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -3699,6 +3699,19 @@
   }];
 }
 
+def ObjCNonLazyClassDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{
+This attribute can be added to an Objective-C ``@interface`` declaration to
+add the class to the list of non-lazily initialized classes. A non-lazy class
+will be initialized eagerly when the Objective-C runtime is loaded.  This is
+required for certain system classes which have instances allocated in
+non-standard ways, such as the classes for blocks and constant strings.  Adding
+this attribute is essentially equivalent to providing a trivial `+load` method 
+but avoids the (fairly small) load-time overheads associated with defining and
+calling such a method.
+  }];
+}
 
 def SelectAnyDocs : Documentation {
   let Category = DocCatType;
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -1758,6 +1758,13 @@
   let Documentation = [Undocumented];
 }
 
+def ObjCNonLazyClass : Attr {
+  let Spellings = [Clang<"objc_nonlazy_class">];
+  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let LangOpts = [ObjC];
+  let Documentation = [ObjCNonLazyClassDocs];
+}
+
 def ObjCSubclassingRestricted : InheritableAttr {
   let Spellings = [Clang<"objc_subclassing_restricted">];
   let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
Index: cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
===
--- cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
+++ cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
 // RUN: FileCheck %s
-// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [1 x {{.*}}] {{.*}}@"OBJC_CLASS_$_A"{{.*}}, section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
+// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [2 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
 // CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [1 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
 
 @interface A @end
@@ -30,3 +30,8 @@
 @interface C : A @end
 @implementation C
 @end
+
+__attribute__((objc_nonlazy_class))
+@interface D @end
+
+@implementation D @end
Index: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -100,6 +100,7 @@
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
+// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
===
--- cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
+++ cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -verify -Wno-objc-root-class  -fsyntax-only  %s
+
+__attribute__((objc_nonlazy_class))
+@interface A
+@end
+@implementation A
+@end
+
+__attribute__((objc_nonlazy_class)) int X; // expected-error {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
+
+__attribute__((objc_nonlazy_class()))
+@interface B
+@end
+@impl

r353120 - [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Julian Lettner via cfe-commits

Author: yln
Date: Mon Feb  4 15:37:50 2019
New Revision: 353120

URL: http://llvm.org/viewvc/llvm-project?rev=353120&view=rev
Log:
[Sanitizers] UBSan unreachable incompatible with Kernel ASan

Summary:
This is a follow up for https://reviews.llvm.org/D57278. The previous
revision should have also included Kernel ASan.

rdar://problem/40723397

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=353120&r1=353119&r2=353120&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Feb  4 15:37:50 2019
@@ -4403,7 +4403,8 @@ RValue CodeGenFunction::EmitCall(const C
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);

Modified: cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=353120&r1=353119&r2=353120&view=diff
==
--- cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c Mon Feb  4 15:37:50 2019
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 


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

[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Julian Lettner via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353120: [Sanitizers] UBSan unreachable incompatible with 
Kernel ASan (authored by yln, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57711?vs=185153&id=185178#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57711

Files:
  lib/CodeGen/CGCall.cpp
  test/CodeGen/ubsan-asan-noreturn.c


Index: test/CodeGen/ubsan-asan-noreturn.c
===
--- test/CodeGen/ubsan-asan-noreturn.c
+++ test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);


Index: test/CodeGen/ubsan-asan-noreturn.c
===
--- test/CodeGen/ubsan-asan-noreturn.c
+++ test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added inline comments.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

jyu2 wrote:
> efriedma wrote:
> > This doesn't look right; I think we need to add it to IndirectJumps 
> > instead.  This probably impacts a testcase like the following:
> > 
> > ```
> > struct S { ~S(); };
> > int f() {
> >   {
> > S s;
> > asm goto(""BAR);
> > return 1;
> >   }
> > BAR:
> >   return 0;
> > }
> > ```
> > 
> > (gcc currently accepts this and skips running the destructor, but I'm 
> > pretty sure that's a bug.)
> Hi Eli,
> I see both g++ and clang++ with my change call ~S.  Am I missing something?  
> Thanks.
> 
> 1>clang++ j.cpp
> 1>./a.out
> ~S()
> 1>g++ j.cpp
> 1>./a.out
> ~S()
> 
> Here is the test case:
> 1>cat j.cpp
> extern "C" int printf (const char *,...);
> struct S { ~S() {printf("~S()\n");}; };
> int f() {
>   {
> S s;
> asm goto(""BAR);
> return 1;
>   }
> BAR:
>   return 0;
> }
> 
> int main()
> {
>   f();
> }
> 
Oh, sorry, I wasn't paying attention to the actual contents of the asm.  If you 
modify the asm goto slightly, to `asm goto("jmp %0"BAR);`, you'll see that 
the destructor doesn't run, at least with gcc.  (This is different from the way 
`goto BAR;` works.)


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

https://reviews.llvm.org/D56571



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

1 2 >

1 - 100 of 126 matches

Mail list logo