[PATCH] D60183: Fix typos in tests. NFC.

[Xing via Phabricator via cfe-commits]

Higuoxing created this revision.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, kubamracek.
Herald added projects: clang, Sanitizers, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60183

Files:
  clang/test/Analysis/analyzer-list-configs.c
  compiler-rt/test/tsan/race_on_heap.cc


Index: compiler-rt/test/tsan/race_on_heap.cc
===
--- compiler-rt/test/tsan/race_on_heap.cc
+++ compiler-rt/test/tsan/race_on_heap.cc
@@ -39,7 +39,7 @@
 // CHECK: WARNING: ThreadSanitizer: data race
 // ...
 // CHECK: Location is heap block of size 99 at [[ADDR]] allocated by thread T1:
-// CHCEK: #0 malloc
+// CHECK: #0 malloc
 // CHECK: #{{1|2}} alloc
 // CHECK: #{{2|3}} AllocThread
 // ...
Index: clang/test/Analysis/analyzer-list-configs.c
===
--- clang/test/Analysis/analyzer-list-configs.c
+++ clang/test/Analysis/analyzer-list-configs.c
@@ -3,7 +3,7 @@
 //
 // CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 

 //
-// CHCEK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, 
-analyzer-config OPTION2=VALUE, ...
+// CHECK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, 
-analyzer-config OPTION2=VALUE, ...
 //
 // CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config 
-Xclang
 //


Index: compiler-rt/test/tsan/race_on_heap.cc
===
--- compiler-rt/test/tsan/race_on_heap.cc
+++ compiler-rt/test/tsan/race_on_heap.cc
@@ -39,7 +39,7 @@
 // CHECK: WARNING: ThreadSanitizer: data race
 // ...
 // CHECK: Location is heap block of size 99 at [[ADDR]] allocated by thread T1:
-// CHCEK: #0 malloc
+// CHECK: #0 malloc
 // CHECK: #{{1|2}} alloc
 // CHECK: #{{2|3}} AllocThread
 // ...
Index: clang/test/Analysis/analyzer-list-configs.c
===
--- clang/test/Analysis/analyzer-list-configs.c
+++ clang/test/Analysis/analyzer-list-configs.c
@@ -3,7 +3,7 @@
 //
 // CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 
 //
-// CHCEK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, -analyzer-config OPTION2=VALUE, ...
+// CHECK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, -analyzer-config OPTION2=VALUE, ...
 //
 // CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r357561 - [clangd] Return clangd::TextEdit in ClangdServer::rename. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Apr  3 00:18:43 2019
New Revision: 357561

URL: http://llvm.org/viewvc/llvm-project?rev=357561&view=rev
Log:
[clangd] Return clangd::TextEdit in ClangdServer::rename. NFC

Summary:
Instead of tooling::Replacement. To avoid the need to have contents of
the file at the caller site. This also aligns better with other methods
in ClangdServer, majority of those already return LSP-specific data
types.

Reviewers: hokein, ioeric, sammccall

Reviewed By: sammccall

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=357561&r1=357560&r2=357561&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Apr  3 00:18:43 2019
@@ -541,19 +541,13 @@ void ClangdLSPServer::onRename(const Ren
   Server->rename(
   File, Params.position, Params.newName,
   Bind(
-  [File, Code, Params](
-  decltype(Reply) Reply,
-  llvm::Expected> Replacements) {
-if (!Replacements)
-  return Reply(Replacements.takeError());
+  [File, Code, Params](decltype(Reply) Reply,
+   llvm::Expected> Edits) {
+if (!Edits)
+  return Reply(Edits.takeError());
 
-// Turn the replacements into the format specified by the Language
-// Server Protocol. Fuse them into one big JSON array.
-std::vector Edits;
-for (const auto &R : *Replacements)
-  Edits.push_back(replacementToEdit(*Code, R));
 WorkspaceEdit WE;
-WE.changes = {{Params.textDocument.uri.uri(), Edits}};
+WE.changes = {{Params.textDocument.uri.uri(), *Edits}};
 Reply(WE);
   },
   std::move(Reply)));

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=357561&r1=357560&r2=357561&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Apr  3 00:18:43 2019
@@ -280,9 +280,9 @@ ClangdServer::formatOnType(llvm::StringR
 }
 
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
-  Callback> CB) {
+  Callback> CB) {
   auto Action = [Pos](Path File, std::string NewName,
-  Callback> CB,
+  Callback> CB,
   llvm::Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
@@ -306,7 +306,7 @@ void ClangdServer::rename(PathRef File,
 if (!ResultCollector.Result.getValue())
   return CB(ResultCollector.Result->takeError());
 
-std::vector Replacements;
+std::vector Replacements;
 for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
   tooling::Replacements ChangeReps = Change.getReplacements();
   for (const auto &Rep : ChangeReps) {
@@ -320,7 +320,8 @@ void ClangdServer::rename(PathRef File,
 //   * rename globally in project
 //   * rename in open files
 if (Rep.getFilePath() == File)
-  Replacements.push_back(Rep);
+  Replacements.push_back(
+  replacementToEdit(InpAST->Inputs.Contents, Rep));
   }
 }
 return CB(std::move(Replacements));

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=357561&r1=357560&r2=357561&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Apr  3 00:18:43 2019
@@ -217,7 +217,7 @@ public:
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   void rename(PathRef File, Position Pos, llvm::StringRef NewName,
-  Callback> CB);
+  Callback> CB);
 
   struct TweakRef {
 std::string ID;/// ID to pass for applyTweak.

Modified: clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp?rev=357561&r1=357560&r2=357561&view=diff
=

[PATCH] D60179: [clangd] Return clangd::TextEdit in ClangdServer::rename. NFC

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357561: [clangd] Return clangd::TextEdit in 
ClangdServer::rename. NFC (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60179?vs=193435&id=193440#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60179

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
  clang-tools-extra/trunk/unittests/clangd/SyncAPI.h

Index: clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
@@ -98,10 +98,10 @@
   return std::move(*Result);
 }
 
-llvm::Expected>
-runRename(ClangdServer &Server, PathRef File, Position Pos,
-  llvm::StringRef NewName) {
-  llvm::Optional>> Result;
+llvm::Expected> runRename(ClangdServer &Server,
+PathRef File, Position Pos,
+llvm::StringRef NewName) {
+  llvm::Optional>> Result;
   Server.rename(File, Pos, NewName, capture(Result));
   return std::move(*Result);
 }
Index: clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
===
--- clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
+++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
@@ -38,7 +38,7 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected>
+llvm::Expected>
 runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -217,7 +217,7 @@
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   void rename(PathRef File, Position Pos, llvm::StringRef NewName,
-  Callback> CB);
+  Callback> CB);
 
   struct TweakRef {
 std::string ID;/// ID to pass for applyTweak.
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -541,19 +541,13 @@
   Server->rename(
   File, Params.position, Params.newName,
   Bind(
-  [File, Code, Params](
-  decltype(Reply) Reply,
-  llvm::Expected> Replacements) {
-if (!Replacements)
-  return Reply(Replacements.takeError());
-
-// Turn the replacements into the format specified by the Language
-// Server Protocol. Fuse them into one big JSON array.
-std::vector Edits;
-for (const auto &R : *Replacements)
-  Edits.push_back(replacementToEdit(*Code, R));
+  [File, Code, Params](decltype(Reply) Reply,
+   llvm::Expected> Edits) {
+if (!Edits)
+  return Reply(Edits.takeError());
+
 WorkspaceEdit WE;
-WE.changes = {{Params.textDocument.uri.uri(), Edits}};
+WE.changes = {{Params.textDocument.uri.uri(), *Edits}};
 Reply(WE);
   },
   std::move(Reply)));
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -280,9 +280,9 @@
 }
 
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
-  Callback> CB) {
+  Callback> CB) {
   auto Action = [Pos](Path File, std::string NewName,
-  Callback> CB,
+  Callback> CB,
   llvm::Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
@@ -306,7 +306,7 @@
 if (!ResultCollector.Result.getValue())
   return CB(ResultCollector.Result->takeError());
 
-std::vector Replacements;
+std::vector Replacements;
 for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
   tooling::Replacements ChangeReps = Change.getReplacements();
   for (const auto &Rep : ChangeReps) {
@@ -320,7 +320,8 @@
 //   * rename globally in project
 //   * rename in open files
 if (Rep.getFilePath() == File)

r357562 - [libclang][test] Suppress annoying 'LIBCLANG TOOLING ERROR' output

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Apr  3 00:25:04 2019
New Revision: 357562

URL: http://llvm.org/viewvc/llvm-project?rev=357562&view=rev
Log:
[libclang][test] Suppress annoying 'LIBCLANG TOOLING ERROR' output

check-all invokes check-clang-python which prints the annoying message:

LIBCLANG TOOLING ERROR: fixed-compilation-database: Error while opening fixed 
database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or 
directory

Let's fix it now with os.dup os.dup2 trick.

Modified:
cfe/trunk/bindings/python/tests/cindex/test_cdb.py

Modified: cfe/trunk/bindings/python/tests/cindex/test_cdb.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_cdb.py?rev=357562&r1=357561&r2=357562&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_cdb.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_cdb.py Wed Apr  3 00:25:04 2019
@@ -23,8 +23,17 @@ class TestCDB(unittest.TestCase):
 def test_create_fail(self):
 """Check we fail loading a database with an assertion"""
 path = os.path.dirname(__file__)
+
+# clang_CompilationDatabase_fromDirectory calls fprintf(stderr, ...)
+# Suppress its output.
+stderr = os.dup(2)
+with open(os.devnull, 'wb') as null:
+os.dup2(null.fileno(), 2)
 with self.assertRaises(CompilationDatabaseError) as cm:
 cdb = CompilationDatabase.fromDirectory(path)
+os.dup2(stderr, 2)
+os.close(stderr)
+
 e = cm.exception
 self.assertEqual(e.cdb_error,
 CompilationDatabaseError.ERROR_CANNOTLOADDATABASE)


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

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina updated this revision to Diff 193441.
MarinaKalashina added a comment.

added a link to CLion


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

https://reviews.llvm.org/D60115

Files:
  docs/ClangFormat.rst


Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -165,6 +165,19 @@
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into `CLion `_ as an alternative code formatter. It is disabled by default and
+can be turned on in Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 


Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -165,6 +165,19 @@
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into `CLion `_ as an alternative code formatter. It is disabled by default and
+can be turned on in Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Sylvestre Ledru via Phabricator via cfe-commits]

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

works for me, thanks


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

https://reviews.llvm.org/D60115



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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

LGTM with the new changes. Specifying the `clang::Expr` type explicitly when 
calling `change` looks a bit less clear than the original approach, but also 
looks pretty clear to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

@alexfh, do you still have any objections?


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

https://reviews.llvm.org/D59449



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

[PATCH] D60116: [clang-format] Regroup #includes into blocks for Google style

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 193446.
ioeric added a comment.

- Improved test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60116

Files:
  lib/Format/Format.cpp
  unittests/Format/CleanupTest.cpp
  unittests/Format/SortIncludesTest.cpp


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -262,9 +262,13 @@
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
+"#include \n"
 "#include \n"
-"#include \n",
-sort("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
@@ -272,9 +276,15 @@
   FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include \n"
 "#include \n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
 "#include \"a.h\"\n"
 "#include \"c.h\"\n",
-sort("#include \n"
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -420,8 +420,10 @@
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -784,6 +784,7 @@
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -262,9 +262,13 @@
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
+"#include \n"
 "#include \n"
-"#include \n",
-sort("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
@@ -272,9 +276,15 @@
   FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include \n"
 "#include \n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
 "#include \"a.h\"\n"
 "#include \"c.h\"\n",
-sort("#include \n"
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -420,8 +420,10 @@
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -784,6 +784,7 @@
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}

[PATCH] D60186: Support CLANG_ENABLE_DEFAULT_PIE like gcc --enable-default-pie

[Jiang Yi via Phabricator via cfe-commits]

jiangyi created this revision.
Herald added subscribers: cfe-commits, mstorsjo, aheejin, jgravelle-google, 
sbc100, mgorny, dschuff, emaste.
Herald added a project: clang.

GCC 6 added the --enable-defaut-pie flag, which would provide better security 
with ASLR enabled.

--enable-defaut-pie is enabled by default in many linux distros such as Alpine, 
Arch, Debian, Fedora, Gentoo, OpenSUSE, Ubuntu...

Add a cmake option CLANG_ENABLE_DEFAULT_PIE to support the same functionality 
for clang.

A pre-processor macro is placed at clang/Driver/ToolChain.h to conditionally 
make isPIEDefault() a non-virtual function. By doing so,  future subclasses of 
clang::driver::ToolChain will not accidentally override isPIEDefault() while 
not taking into account of CLANG_ENABLE_DEFAULT_PIE.


Repository:
  rC Clang

https://reviews.llvm.org/D60186

Files:
  CMakeLists.txt
  include/clang/Config/config.h.cmake
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/BareMetal.h
  lib/Driver/ToolChains/CloudABI.cpp
  lib/Driver/ToolChains/CloudABI.h
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CrossWindows.cpp
  lib/Driver/ToolChains/CrossWindows.h
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/FreeBSD.h
  lib/Driver/ToolChains/Fuchsia.cpp
  lib/Driver/ToolChains/Fuchsia.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Gnu.h
  lib/Driver/ToolChains/HIP.h
  lib/Driver/ToolChains/Haiku.h
  lib/Driver/ToolChains/Hurd.cpp
  lib/Driver/ToolChains/Linux.cpp
  lib/Driver/ToolChains/Linux.h
  lib/Driver/ToolChains/MSP430.h
  lib/Driver/ToolChains/MSVC.cpp
  lib/Driver/ToolChains/MSVC.h
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/MinGW.h
  lib/Driver/ToolChains/OpenBSD.h
  lib/Driver/ToolChains/Solaris.cpp
  lib/Driver/ToolChains/TCE.cpp
  lib/Driver/ToolChains/TCE.h
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  lib/Driver/ToolChains/XCore.cpp
  lib/Driver/ToolChains/XCore.h
  lib/Frontend/CompilerInvocation.cpp

Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -25,7 +25,6 @@
 #include "clang/Basic/Version.h"
 #include "clang/Basic/Visibility.h"
 #include "clang/Basic/XRayInstr.h"
-#include "clang/Config/config.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
Index: lib/Driver/ToolChains/XCore.h
===
--- lib/Driver/ToolChains/XCore.h
+++ lib/Driver/ToolChains/XCore.h
@@ -58,7 +58,9 @@
 
 public:
   bool isPICDefault() const override;
-  bool isPIEDefault() const override;
+#if !CLANG_ENABLE_DEFAULT_PIE
+  bool isPIEDefault() const override { return false; }
+#endif
   bool isPICDefaultForced() const override;
   bool SupportsProfiling() const override;
   bool hasBlocksRuntime() const override;
Index: lib/Driver/ToolChains/XCore.cpp
===
--- lib/Driver/ToolChains/XCore.cpp
+++ lib/Driver/ToolChains/XCore.cpp
@@ -100,8 +100,6 @@
 
 bool XCoreToolChain::isPICDefault() const { return false; }
 
-bool XCoreToolChain::isPIEDefault() const { return false; }
-
 bool XCoreToolChain::isPICDefaultForced() const { return false; }
 
 bool XCoreToolChain::SupportsProfiling() const { return false; }
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -45,7 +45,9 @@
   bool IsObjCNonFragileABIDefault() const override;
   bool UseObjCMixedDispatch() const override;
   bool isPICDefault() const override;
-  bool isPIEDefault() const override;
+#if !CLANG_ENABLE_DEFAULT_PIE
+  bool isPIEDefault() const override { return false; }
+#endif
   bool isPICDefaultForced() const override;
   bool IsIntegratedAssemblerDefault() const override;
   bool hasBlocksRuntime() const override;
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -127,8 +127,6 @@
 
 bool WebAssembly::isPICDefault() const { return false; }
 
-bool WebAssembly::isPIEDefault() const { return false; }
-
 bool WebAssembly::isPICDefaultForced() const { return false; }
 
 bool WebAssembly::IsIntegratedAssemblerDefault() const { return true; }
Index: lib/Driver/ToolChains/TCE.h
===
--- lib/Driver/ToolChains/TCE.h
+++ lib/Driver/ToolChains/TCE.h
@@ -27,7 +27,9 @@
 
   bool IsMathErrnoDefault() const override;
   bool isPICDefault() const override;
- 

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Marina Kalashina via Phabricator via cfe-commits]

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

@sylvestre.ledru Thank you! Could you please explain how I should proceed with 
the accepted revision now (without commit access, do I need to ask someone to 
make the commit for me)?


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

https://reviews.llvm.org/D60115



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

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Sure, i will do it for you


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

https://reviews.llvm.org/D60115



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

r357566 - Adding 'CLion integration' to clang-format doc

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Wed Apr  3 01:56:30 2019
New Revision: 357566

URL: http://llvm.org/viewvc/llvm-project?rev=357566&view=rev
Log:
Adding 'CLion integration' to clang-format doc

Summary:
This commit adds a chapter 'CLion integration' to ClangFormat.rst. 
The official announcement of clang-format support in CLion 2019.1: 
https://blog.jetbrains.com/clion/2019/03/clion-2019-1-embedded-dev-clangformat-memory-view/

Patch by Marina Kalashina 

Reviewers: djasper, sylvestre.ledru, krasimir

Reviewed By: sylvestre.ledru

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/ClangFormat.rst

Modified: cfe/trunk/docs/ClangFormat.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormat.rst?rev=357566&r1=357565&r2=357566&view=diff
==
--- cfe/trunk/docs/ClangFormat.rst (original)
+++ cfe/trunk/docs/ClangFormat.rst Wed Apr  3 01:56:30 2019
@@ -165,6 +165,19 @@ menu item by renaming the script, and ca
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into `CLion `_ as an alternative code formatter. It is disabled by default and
+can be turned on in Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 


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

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357566: Adding 'CLion integration' to clang-format 
doc (authored by sylvestre, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D60115

Files:
  docs/ClangFormat.rst


Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -165,6 +165,19 @@
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into `CLion `_ as an alternative code formatter. It is disabled by default and
+can be turned on in Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 


Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -165,6 +165,19 @@
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into `CLion `_ as an alternative code formatter. It is disabled by default and
+can be turned on in Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina added a comment.

@sylvestre.ledru Thanks a lot for your help


Repository:
  rC Clang

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

https://reviews.llvm.org/D60115



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

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

You are welcome, thanks for your contribution!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60115



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

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193449.
DennisL marked 13 inline comments as done.
DennisL added a comment.

Updated patch to address reviewer feedback


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new has insufficient target size [misc-placement-new-target-size]
+  delete ptr;
+}
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,37 @@
+//===--- PlacementNewTargetSizeCheck.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_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into
+/// allocated space where the pointer to that allocated space is of a type
+/// smaller than the constructed object and the pointer is implicitly cast to
+/// void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,79 @@
+//===--- PlacementNewTargetSizeCheck.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
+//
+//===

r357567 - [clang-format] Regroup #includes into blocks for Google style

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Apr  3 02:25:16 2019
New Revision: 357567

URL: http://llvm.org/viewvc/llvm-project?rev=357567&view=rev
Log:
[clang-format] Regroup #includes into blocks for Google style

Summary:
Regrouping #includes in blocks separated by blank lines when sorting C++ 
#include
headers was implemented recently, and it has been preferred in Google's C++ 
style guide:
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

Reviewers: sammccall, klimek

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/CleanupTest.cpp
cfe/trunk/unittests/Format/SortIncludesTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=357567&r1=357566&r2=357567&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Apr  3 02:25:16 2019
@@ -784,6 +784,7 @@ FormatStyle getGoogleStyle(FormatStyle::
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;

Modified: cfe/trunk/unittests/Format/CleanupTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/CleanupTest.cpp?rev=357567&r1=357566&r2=357567&view=diff
==
--- cfe/trunk/unittests/Format/CleanupTest.cpp (original)
+++ cfe/trunk/unittests/Format/CleanupTest.cpp Wed Apr  3 02:25:16 2019
@@ -420,8 +420,10 @@ TEST_F(CleanUpReplacementsTest, InsertMu
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"

Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=357567&r1=357566&r2=357567&view=diff
==
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Wed Apr  3 02:25:16 2019
@@ -262,9 +262,13 @@ TEST_F(SortIncludesTest, CommentsAlwaysS
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
+"#include \n"
 "#include \n"
-"#include \n",
-sort("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
@@ -272,9 +276,15 @@ TEST_F(SortIncludesTest, HandlesAngledIn
   FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include \n"
 "#include \n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
 "#include \"a.h\"\n"
 "#include \"c.h\"\n",
-sort("#include \n"
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));


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

[PATCH] D60116: [clang-format] Regroup #includes into blocks for Google style

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357567: [clang-format] Regroup #includes into blocks for 
Google style (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60116?vs=193446&id=193452#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60116

Files:
  lib/Format/Format.cpp
  unittests/Format/CleanupTest.cpp
  unittests/Format/SortIncludesTest.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -784,6 +784,7 @@
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -420,8 +420,10 @@
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"
Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -262,9 +262,13 @@
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
+"#include \n"
 "#include \n"
-"#include \n",
-sort("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
@@ -272,9 +276,15 @@
   FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include \n"
 "#include \n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
 "#include \"a.h\"\n"
 "#include \"c.h\"\n",
-sort("#include \n"
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -784,6 +784,7 @@
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -420,8 +420,10 @@
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"
Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -262,9 +262,13 @@
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
+"#include \n"
 "#include \n"
-"#include \n",
-sort("#include \n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
  "#include \n"
  "#include \"c.h\"\n"
  "#include \"a.h\"\n"));
@@ -272,

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193454.
DennisL added a comment.

Remove debug output


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new has insufficient target size [misc-placement-new-target-size]
+  delete ptr;
+}
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,37 @@
+//===--- PlacementNewTargetSizeCheck.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_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into
+/// allocated space where the pointer to that allocated space is of a type
+/// smaller than the constructed object and the pointer is implicitly cast to
+/// void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,78 @@
+//===--- PlacementNewTargetSizeCheck.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 "PlacementNewTargetSizeCheck.h"
+#incl

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

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

As discussed offline, the current approach only works for checks provide a 
single fix, providing such API is somehow misleading.

Instead, we'd emit the check fix and the fix description via diagnostic::Note, 
rather than attaching to the main diagnostic of the check:

- match the data model of the checks provide different semantic fixes depending 
on the context;
- align with the way how existing clang diagnostics emit alternative fixes (via 
diagnostic::Note);
- open a door to write clang-tidy checks that provide alternative fixes; we 
don't have these checks at the moment, but some clang diagnostics like 
`clang-diagnostic-parentheses` do (and our current implementation just 
aggregates all the fixes together, which is not correct);

It would require some changes in clang-tidy check side:

Before this patch:

  void MyCheck::check(...) {
 ...
 diag(loc, "my check warning") <<  FixtItHint::CreateReplacement(...);
  }

After this patch:

  void MyCheck::check(...) {
 ...
 diag(loc, "my check warning"); // Emit a check warning
 // We might want to introduce an utility method like `diagFix` to save 
some verbosed words.
 diag(loc, "fix description", DiagnosticIDs::Note) << 
FixtItHint::CreateReplacement(...); // Emit a diagnostic note and a fix
  }

An example of unused-using-decls check (clang-tidy command line output)

- before:

  1 warning generated.
  /tmp/test.cpp:8:12: warning: using decl 'Foo' is unused 
[misc-unused-using-decls]
  using foo::Foo;
  ~~~^~~~

- after:

  1 warning generated.
  /tmp/test.cpp:8:12: warning: using decl 'Foo' is unused 
[misc-unused-using-decls]
  using foo::Foo;
 ^
  /tmp/test.cpp:8:12: note: remove the using
  using foo::Foo;
  ~~~^~~~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D55049: Changed every use of ASTImporter::Import to Import_New

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping @shafik @a_sidorin


Repository:
  rC Clang

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

https://reviews.llvm.org/D55049



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

[PATCH] D59985: Re-fix invalid address space generation for clk_event_t arguments of enqueue_kernel builtin function

[Alexey Sotkin via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGBuiltin.cpp:3711
+  EventList = EventList->getType()->isIntegerTy()
+  ? Builder.CreateIntToPtr(EventList, EventPtrTy)
+  : Builder.CreatePointerCast(EventList, EventPtrTy);

Anastasia wrote:
> It seems we are not testing the casts?
Do you mean that when we run LIT tests, this code is not executed? If so, in 
the modified test below, literal zeros are making clang to execute 
CreateIntToPtr call indeed.
Or, do you mean that we need some extra check(to make sure the cast will be 
successful for example) in the source code itself ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59985



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

[PATCH] D60130: gn build: Add build files for clangd xpc framework code

[Mirko Bonadei via Phabricator via cfe-commits]

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

Really nice work and thanks for the thread on gn-dev about create_bundle!

LGTM!


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

https://reviews.llvm.org/D60130



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

[PATCH] D60193: [OpenCL] Added addrspace_cast operator

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: rjmccall, ebevhan.
Herald added a subscriber: yaxunl.

This change adds extra cast operator `addrspace_cast` described earlier in this 
RFC:
http://lists.llvm.org/pipermail/cfe-dev/2018-December/060546.html

This operator is intended for casting between pointers to objects in different 
address spaces and follows similar logic as `const_cast` in C++.

Example:

  int* gen = ...;
  global int* gl = addrspace_cast(gen);

Note that functionality of casting pointers to different address spaces has 
been previously removed from all other cast operators and can only be currently 
done using C style casts.

This commit only enables this change for OpenCL, but if agreed it can be 
changed to apply to C++ too (it needs to be enabled as a valid keyword). 
However, for C++ it might need to be added as a Clang only extension?


https://reviews.llvm.org/D60193

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/StmtNodes.td
  include/clang/Basic/TokenKinds.def
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CodeGenOpenCLCXX/addrspace_cast.cl
  test/SemaOpenCLCXX/addrspace_cast.cl
  test/SemaOpenCLCXX/addrspace_cast_ast_dump.cl

Index: test/SemaOpenCLCXX/addrspace_cast_ast_dump.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/addrspace_cast_ast_dump.cl
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -ast-dump | FileCheck %s
+
+// expected-no-diagnostics
+
+//CHECK:`-FunctionDecl {{.*}} bar 'void (__global int *)'
+//CHECK:  |-ParmVarDecl {{.*}} used gl '__global int *'
+//CHECK:  `-VarDecl {{.*}} gen '__generic int *' cinit
+//CHECK:`-CXXAddrspaceCastExpr {{.*}} '__generic int *' addrspace_cast<__generic int *> 
+//CHECK:  `-DeclRefExpr {{.*}} '__global int *' lvalue ParmVar {{.*}} 'gl' '__global int *'
+
+void bar(global int *gl) {
+  int *gen = addrspace_cast(gl);
+}
Index: test/SemaOpenCLCXX/addrspace_cast.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/addrspace_cast.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+
+void foo(global int *gl, const global int *gl_const, global int &gl_ref) {
+  //FIXME: Diagnostics can be improved to be more specific in some cases.
+  float *gen_fl = addrspace_cast(gl); //expected-error{{addrspace_cast from '__global int *' to '__generic float *' is not allowed}}
+
+  int i = addrspace_cast(gl); //expected-error{{addrspace_cast from '__global int *' to 'int' is not allowed}}
+
+  int *gen = addrspace_cast(*gl); //expected-error{{addrspace_cast from '__global int' to '__generic int *' is not allowed}}
+
+  local int *loc = addrspace_cast(gl); //expected-error{{addrspace_cast from '__global int *' to '__local int *' converts between mismatching address spaces}}
+
+  int *gen2 = addrspace_cast(gl_const); //expected-error{{addrspace_cast from 'const __global int *' to '__generic int *' is not allowed}}
+
+  //FIXME: Do we expect this behavior? This will get cast successfully as reinterpret_cast.
+  int &gen_ref = addrspace_cast(gl_ref); //expected-error{{addrspace_cast from '__global int' to '__generic int &' is not allowed}}
+}
Index: test/CodeGenOpenCLCXX/addrspace_cast.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace_cast.cl
@@ -0,0 +1,7 @@
+//RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+//CHECK-LABEL: define spir_func void @_Z3barPU3AS1i
+void bar(global int *gl) {
+  //CHECK: addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
+  int *gen = addrspace_cast(gl);
+}
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -1411,6 +1411,11 @@
   Code = serialization::EXPR_CXX_CONST_CAST;
 }
 
+void ASTStmtWriter::VisitCXXAddrspaceCastExpr(CXXAddrspaceCastExpr *E) {
+  VisitCXXNamedCastExpr(E);
+  Code = serialization::EXPR_CXX_ADDRSPACE_CAST;
+}
+
 void ASTStmtWriter::VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr *E) {
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getLParenLoc());
Index: lib/Serialization/ASTWriter.c

[PATCH] D59985: Re-fix invalid address space generation for clk_event_t arguments of enqueue_kernel builtin function

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:3711
+  EventList = EventList->getType()->isIntegerTy()
+  ? Builder.CreateIntToPtr(EventList, EventPtrTy)
+  : Builder.CreatePointerCast(EventList, EventPtrTy);

AlexeySotkin wrote:
> Anastasia wrote:
> > It seems we are not testing the casts?
> Do you mean that when we run LIT tests, this code is not executed? If so, in 
> the modified test below, literal zeros are making clang to execute 
> CreateIntToPtr call indeed.
> Or, do you mean that we need some extra check(to make sure the cast will be 
> successful for example) in the source code itself ?
I mean since you are generating extra IR nodes we should check in the tests 
that they appear correctly. I don't see these casts checked in the tests 
currently.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59985



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

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: ilya-biryukov, klimek.
Herald added subscribers: cfe-commits, kadircet, ioeric.
Herald added a project: clang.

Use cases:

- a tool that dumps the heuristic used for each header in a project can be used 
to evaluate changes to the heuristic
- we want to expose this information to users in clangd as it affects 
accuracy/reliability of editor features
- express interpolation tests more directly


Repository:
  rC Clang

https://reviews.llvm.org/D60194

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -673,6 +673,27 @@
 return llvm::join(Results[0].CommandLine, " ");
   }
 
+  // Parse the file whose command was used out of the Heuristic string.
+  std::string getProxy(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+StringRef Proxy = Results.front().Heuristic;
+if (!Proxy.consume_front("inferred from "))
+  return "";
+// We have a proxy file, convert back to a unix relative path.
+// This is a bit messy, but we do need to test these strings somehow...
+llvm::SmallString<32> TempDir;
+llvm::sys::path::system_temp_directory(false, TempDir);
+Proxy.consume_front(TempDir);
+Proxy.consume_front(llvm::sys::path::get_separator());
+llvm::SmallString<32> Result = Proxy;
+llvm::sys::path::native(Result, llvm::sys::path::Style::posix);
+return Result.str();
+  }
+
   MemCDB::EntryMap Entries;
 };
 
@@ -682,18 +703,16 @@
   add("an/other/foo.cpp");
 
   // great: dir and name both match (prefix or full, case insensitive)
-  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
-  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getProxy("dir/f.cpp"), "dir/foo.cpp");
+  EXPECT_EQ(getProxy("dir/FOO.cpp"), "dir/foo.cpp");
   // no name match. prefer matching dir, break ties by alpha
-  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  EXPECT_EQ(getProxy("dir/a.cpp"), "dir/bar.cpp");
   // an exact name match beats one segment of directory match
-  EXPECT_EQ(getCommand("some/other/bar.h"),
-"clang -D dir/bar.cpp -x c++-header");
+  EXPECT_EQ(getProxy("some/other/bar.h"), "dir/bar.cpp");
   // two segments of directory match beat a prefix name match
-  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  EXPECT_EQ(getProxy("an/other/b.cpp"), "an/other/foo.cpp");
   // if nothing matches at all, we still get the closest alpha match
-  EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
-"clang -D an/other/foo.cpp");
+  EXPECT_EQ(getProxy("below/some/obscure/path.cpp"), "an/other/foo.cpp");
 }
 
 TEST_F(InterpolateTest, Language) {
@@ -727,7 +746,7 @@
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
   // Case mismatches are completely ignored, so we choose the name match.
-  EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D 
FOO/BAR/BAZ/SHOUT.cc");
+  EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
 TEST_F(InterpolateTest, Aliasing) {
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -226,6 +226,7 @@
   LangStandard::getLangStandardForKind(Std).getName()).str());
 }
 Result.CommandLine.push_back(Filename);
+Result.Heuristic = "inferred from " + Cmd.Filename;
 return Result;
   }
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,9 +59,15 @@
   /// The output file associated with the command.
   std::string Output;
 
+  /// If this compile command was guessed rather than read from an 
authoritative
+  /// source, a short human-readable explanation.
+  /// e.g. "inferred from foo/bar.h".
+  std::string Heuristic;
+
   friend bool operator==(const CompileCommand &LHS, const CompileCommand &RHS) 
{
 return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
-   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output &&
+   LHS.Heuristic == RHS.Heuristic;
   }
 
   friend bool operator!=(const CompileCommand &LHS, const CompileCommand &RHS) 
{


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193471.
DennisL added a comment.

Simplify logic


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new has insufficient target size [misc-placement-new-target-size]
+  delete ptr;
+}
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,37 @@
+//===--- PlacementNewTargetSizeCheck.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_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into
+/// allocated space where the pointer to that allocated space is of a type
+/// smaller than the constructed object and the pointer is implicitly cast to
+/// void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,78 @@
+//===--- PlacementNewTargetSizeCheck.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 "PlacementNewTargetSizeCheck.h"
+#include "

[PATCH] D60193: [OpenCL] Added addrspace_cast operator

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/TokenKinds.def:564
+// OpenCL addrspace_cast operator
+KEYWORD(addrspace_cast  , KEYOPENCLCXX)
+

If I mark it as CXX keyword this will make it available in C++ too because the 
rest of the patch is not specific to OpenCL but perhaps we also need to emit a 
warning that this is an extension?


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

https://reviews.llvm.org/D60193



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),
+  StringRef("[super init]"));

stephanemoore wrote:
> aaron.ballman wrote:
> > stephanemoore wrote:
> > > stephanemoore wrote:
> > > > aaron.ballman wrote:
> > > > > This could be dangerous if the `[super self]` construct is in a 
> > > > > macro, couldn't it? e.g.,
> > > > > ```
> > > > > #define DERP self
> > > > > 
> > > > > [super DERP];
> > > > > ```
> > > > Good point. Let me add some test cases and make sure this is handled 
> > > > properly.
> > > Added some test cases where `[super self]` is expanded from macros.
> > You missed the test case I was worried about -- where the macro is mixed 
> > into the expression. I don't think we want to try to add a fix-it in that 
> > case.
> Added a test case though at the moment it generates a fixit.
> 
> Before I investigate modifying the check to avoid generating a fixit in this 
> case, I think it would be helpful for me to understand your concerns in that 
> scenario better. Is your concern that a proper fix might involve fixing the 
> macro itself rather than the message expression?
> 
> ```
> #if FLAG
> #define INVOCATION self
> #else
> #define INVOCATION init
> #endif
> 
> - (instancetype)init {
>   return [super INVOCATION];
> }
> ```
> Before I investigate modifying the check to avoid generating a fixit in this 
> case, I think it would be helpful for me to understand your concerns in that 
> scenario better. Is your concern that a proper fix might involve fixing the 
> macro itself rather than the message expression?

Essentially, yes. Macros can get arbitrarily complex and so our rule of thumb 
is to not apply fix-its when the code being fixed is within a macro. Another 
example that can be tricky is adding another layer of macros:
```
#define FOO super
#define BAR self
#define BAZ FOO BAR

- (instancetype)init {
  return [BAZ];
}
```



Comment at: clang-tools-extra/test/clang-tidy/objc-super-self.m:41
+  INITIALIZER_IMPL();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: suspicious invocation of 'self' in 
initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+}

stephanemoore wrote:
> aaron.ballman wrote:
> > Are you missing a `CHECK-FIXES` here?
> > 
> > Personally, I don't think we should try to generate a fixit for this case, 
> > so I would expect a CHECK-FIXES that ensures this doesn't get modified.
> No fix is currently generated for this case which is why there is no 
> `CHECK-FIXES`. I also agree that no fix should be generated for this case.
> 
> I must confess that I have yet to fully understand why the fix for this case 
> is discarded (though I am grateful for the behavior). Let me dig around to 
> try to better understand why no fixit is generated for this case and assess 
> adding a condition for emitting the fixit.
Our typical way to check that a fix is not applied is to use `// CHECK-FIXES: 
` to test that the fix was not applied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[clang-tools-extra] r357574 - gn build: Add build files for clangd xpc framework code

[Nico Weber via cfe-commits]

Author: nico
Date: Wed Apr  3 05:33:19 2019
New Revision: 357574

URL: http://llvm.org/viewvc/llvm-project?rev=357574&view=rev
Log:
gn build: Add build files for clangd xpc framework code

This is a bit of a larger change since this is the first (and as far as
I can tell only) place where the LLVM build produces macOS framework
bundles.

GN has some built-in support for this, so use that.
`gn help create_bundle` has a terse description (but it's a bit
outdated: `deps` must be `public_deps` and the conditionals in the
example in the help aren't quite right on non-iOS).

We need a new 'copy_bundle_data' tool, and since we copy the clangd.xpc
bundle as bundle_data into ClangdXPC.framework it needs to be able to
handle directories in addition to files.

GN also insists we have a compile_xcassets tool even though it's not
used. I just made that run `false`.

Despite GN's support for bundles, we still need to manually create the
expected symlink structure in the .framework bundle. Since this code
never runs on Windows, it's safe to create the symlinks before the
symlink targets exist, so we can just make the bundle depend on the
steps that create the symlinks. For this to work, change the symlink
script to create the symlink's containing directory if it doesn't yet
exist.

I locally verified that CMake and GN build create the same bundle
structure. (I noticed that both builds set LC_ID_DYLIB to the pre-copy
libClangdXPCLib.dylib name, but that seems to not cause any issues and
it happens in the CMake build too.)

(Also add an error message to clangd-xpc-test-client for when loading
the dylib fails – this was useful while locally debugging this.)

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

Modified:
clang-tools-extra/trunk/clangd/xpc/framework/CMakeLists.txt
clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp

Modified: clang-tools-extra/trunk/clangd/xpc/framework/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/xpc/framework/CMakeLists.txt?rev=357574&r1=357573&r2=357574&view=diff
==
--- clang-tools-extra/trunk/clangd/xpc/framework/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/xpc/framework/CMakeLists.txt Wed Apr  3 
05:33:19 2019
@@ -1,6 +1,7 @@
 
 set(SOURCES
-ClangdXPC.cpp)
+  ClangdXPC.cpp
+)
 add_clang_library(ClangdXPCLib SHARED
   ${SOURCES}
   DEPENDS

Modified: clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp?rev=357574&r1=357573&r2=357574&view=diff
==
--- clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp Wed 
Apr  3 05:33:19 2019
@@ -49,8 +49,10 @@ int main(int argc, char *argv[]) {
   // Open the ClangdXPC dylib in the framework.
   std::string LibPath = getLibraryPath();
   void *dlHandle = dlopen(LibPath.c_str(), RTLD_LOCAL | RTLD_FIRST);
-  if (!dlHandle)
+  if (!dlHandle) {
+llvm::errs() << "Failed to load framework from \'" << LibPath << "\'\n";
 return 1;
+  }
 
   // Lookup the XPC service bundle name, and launch it.
   clangd_xpc_get_bundle_identifier_t clangd_xpc_get_bundle_identifier =


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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18
+
+AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
+} // namespace ast_matchers

hintonda wrote:
> @aaron.ballman:  This matcher seems genuinely useful.  What do you think 
> about moving it to ASTMatchers.h? 
I think that adding something like this might be a good idea. We don't have any 
notion of source locations in the AST matching syntax currently, and I'm not 
certain whether that's a good thing or not here. I'm mildly uncomfortable that 
this matcher operates on an `Expr` but then internally uses a source location 
from that expression and I wonder if we would rather introduce source location 
matching. For instance, what if the user cares about the difference between 
`getExprLoc()` and `getBeginLoc()` for some reason?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D60130: gn build: Add build files for clangd xpc framework code

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE357574: gn build: Add build files for clangd xpc framework 
code (authored by nico, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D60130?vs=193313&id=193476#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60130

Files:
  clangd/xpc/framework/CMakeLists.txt
  clangd/xpc/test-client/ClangdXPCTestClient.cpp


Index: clangd/xpc/framework/CMakeLists.txt
===
--- clangd/xpc/framework/CMakeLists.txt
+++ clangd/xpc/framework/CMakeLists.txt
@@ -1,6 +1,7 @@
 
 set(SOURCES
-ClangdXPC.cpp)
+  ClangdXPC.cpp
+)
 add_clang_library(ClangdXPCLib SHARED
   ${SOURCES}
   DEPENDS
Index: clangd/xpc/test-client/ClangdXPCTestClient.cpp
===
--- clangd/xpc/test-client/ClangdXPCTestClient.cpp
+++ clangd/xpc/test-client/ClangdXPCTestClient.cpp
@@ -49,8 +49,10 @@
   // Open the ClangdXPC dylib in the framework.
   std::string LibPath = getLibraryPath();
   void *dlHandle = dlopen(LibPath.c_str(), RTLD_LOCAL | RTLD_FIRST);
-  if (!dlHandle)
+  if (!dlHandle) {
+llvm::errs() << "Failed to load framework from \'" << LibPath << "\'\n";
 return 1;
+  }
 
   // Lookup the XPC service bundle name, and launch it.
   clangd_xpc_get_bundle_identifier_t clangd_xpc_get_bundle_identifier =


Index: clangd/xpc/framework/CMakeLists.txt
===
--- clangd/xpc/framework/CMakeLists.txt
+++ clangd/xpc/framework/CMakeLists.txt
@@ -1,6 +1,7 @@
 
 set(SOURCES
-ClangdXPC.cpp)
+  ClangdXPC.cpp
+)
 add_clang_library(ClangdXPCLib SHARED
   ${SOURCES}
   DEPENDS
Index: clangd/xpc/test-client/ClangdXPCTestClient.cpp
===
--- clangd/xpc/test-client/ClangdXPCTestClient.cpp
+++ clangd/xpc/test-client/ClangdXPCTestClient.cpp
@@ -49,8 +49,10 @@
   // Open the ClangdXPC dylib in the framework.
   std::string LibPath = getLibraryPath();
   void *dlHandle = dlopen(LibPath.c_str(), RTLD_LOCAL | RTLD_FIRST);
-  if (!dlHandle)
+  if (!dlHandle) {
+llvm::errs() << "Failed to load framework from \'" << LibPath << "\'\n";
 return 1;
+  }
 
   // Lookup the XPC service bundle name, and launch it.
   clangd_xpc_get_bundle_identifier_t clangd_xpc_get_bundle_identifier =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

[Anastasia Stulova via Phabricator via cfe-commits]

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

Improved comment about initializers in __local addr space.


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

https://reviews.llvm.org/D59646

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  // FIXME: In OpenCL C we don't accept initializers for local
+  // address space variables. User defined initialization could
+  // make sense, but would it mean that all work items need to
+  // execute it? Potentially disallowing any initialization would
+  // make things easier and assingments can be used to set specific
+  // values. This rules should make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an object in local address space.
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C 
addrspace(4)*))
 
-
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*
 // EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], 
%class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* 
@_ZZ11test__localvE1c to %class.C addrspace(4)*))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11645,7 +11645,11 @@
   setFunctionHasBranchProtectedScope();
   }
 }
-
+// In OpenCL, we can't initialize objects in the __local address space,
+// even implicitly, so don't synthesize an implicit initializer.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;
 // C++03 [dcl.init]p9:
 //   If no initializer is specified for an object, and the
 //   object is of (possibly cv-qualified) non-POD class type (or


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  // FIXME: In OpenCL C we don't accept initializers for local
+  // address space variables. User defined initialization could
+  // make sense, but would it mean that all work items need to
+  // execute it? Potentially disallowing any initialization would
+  // make things easier and assingments can be used to set specific
+  // values. This rules should make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initial

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145
+
+diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null")
+<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),

hintonda wrote:
> aaron.ballman wrote:
> > hintonda wrote:
> > > aaron.ballman wrote:
> > > > This diagnostic doesn't tell the user what they've done wrong with the 
> > > > code or why this is a better choice.
> > > Yes, but I'm not yet sure what it should say.  Was sorta hoping for a 
> > > suggestion.  
> > Do you have any evidence that this situation happens in practice? I kind of 
> > feel like this entire branch could be eliminated from this patch unless it 
> > actually catches problems that happen.
> Yes, here are a few from clang/lib -- let me know if you think it's worth it 
> or not to keep this:
> 
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 305293
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaTemplate.cpp
>   Message: method 'getAsTemplateDecl' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaTemplate.cpp
> Length: 93
> Offset: 305293
> ReplacementText: 
> dyn_cast_or_null(Name.getAsTemplateDecl())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 153442
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/ASTContext.cpp
>   Message: method 'getAsTemplateDecl' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/ASTContext.cpp
> Length: 92
> Offset: 153442
> ReplacementText: 
> dyn_cast_or_null(Template.getAsTemplateDecl())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 97556
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/Expr.cpp
>   Message: method 'getMethodDecl' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/Expr.cpp
> Length: 68
> Offset: 97556
> ReplacementText: dyn_cast_or_null(MCE->getMethodDecl())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 301950
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaInit.cpp
>   Message: method 'get' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaInit.cpp
> Length: 49
> Offset: 301950
> ReplacementText: dyn_cast_or_null(CurInit.get())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 14335
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
>   Message: method 'operator bool' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
> Length: 57
> Offset: 14335
> ReplacementText: dyn_cast_or_null(B->getTerminator())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 15997
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
>   Message: method 'operator bool' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
> Length: 55
> Offset: 15997
> ReplacementText: dyn_cast_or_null(B.getTerminator())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 9492
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
>   Message: method 'sexpr' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
> Length: 39
> Offset: 9492
> ReplacementText: dyn_cast_or_null(sexpr())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 9572
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
>   Message: method 'sexpr' is called twice and could be expensive
>   Replacements:
>   - FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
> Length: 38
> Offset: 9572
> ReplacementText: dyn_cast_or_null(sexpr())
> - DiagnosticName: llvm-avoid-cast-in-conditional
>   FileOffset: 9492
>   FilePath: 
> /Users/dhinton/projects/llvm_project/monorepo/llvm-

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:293
   bit Negated = negated;
+  string CustomCode = customCode;
 }

I think the type here should be `code` instead of `string` since the user is 
passing in code snippets, no?



Comment at: lib/Sema/SemaDeclObjC.cpp:4129-4130
+
+if (IntfDecl->hasAttr()) {
+  if (!IntfDecl->hasAttr())
+Diag(IntfDecl->getLocation(), 
diag::err_class_stub_subclassing_mismatch);

Combine these `if` statements?


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

https://reviews.llvm.org/D59628



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

r357576 - [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Wed Apr  3 06:05:19 2019
New Revision: 357576

URL: http://llvm.org/viewvc/llvm-project?rev=357576&view=rev
Log:
[LibTooling] Add Transformer, a library for source-to-source transformations.

Summary: Adds a basic version of Transformer, a library supporting the concise 
specification of clang-based source-to-source transformations.  A full 
discussion of the end goal can be found on the cfe-dev list with subject "[RFC] 
Easier source-to-source transformations with clang tooling".

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: mgorny, jfb, jdoerfert, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp
Modified:
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Added: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=357576&view=auto
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (added)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Wed Apr  3 
06:05:19 2019
@@ -0,0 +1,210 @@
+//===--- Transformer.h - Clang source-rewriting library -*- 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
+//
+//===--===//
+///
+///  \file
+///  Defines a library supporting the concise specification of clang-based
+///  source-to-source transformations.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_TRANSFORMER_H_
+#define LLVM_CLANG_TOOLING_REFACTOR_TRANSFORMER_H_
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+/// Determines the part of the AST node to replace.  We support this to work
+/// around the fact that the AST does not differentiate various syntactic
+/// elements into their own nodes, so users can specify them relative to a 
node,
+/// instead.
+enum class NodePart {
+  /// The node itself.
+  Node,
+  /// Given a \c MemberExpr, selects the member's token.
+  Member,
+  /// Given a \c NamedDecl or \c CxxCtorInitializer, selects that token of the
+  /// relevant name, not including qualifiers.
+  Name,
+};
+
+using TextGenerator =
+std::function;
+
+/// Description of a source-code transformation.
+//
+// A *rewrite rule* describes a transformation of source code. It has the
+// following components:
+//
+// * Matcher: the pattern term, expressed as clang matchers (with Transformer
+//   extensions).
+//
+// * Target: the source code impacted by the rule. This identifies an AST node,
+//   or part thereof (\c TargetPart), whose source range indicates the extent 
of
+//   the replacement applied by the replacement term.  By default, the extent 
is
+//   the node matched by the pattern term (\c NodePart::Node). Target's are
+//   typed (\c TargetKind), which guides the determination of the node extent
+//   and might, in the future, statically constrain the set of eligible
+//   NodeParts for a given node.
+//
+// * Replacement: a function that produces a replacement string for the target,
+//   based on the match result.
+//
+// * Explanation: explanation of the rewrite.  This will be displayed to the
+//   user, where possible (for example, in clang-tidy fix descriptions).
+//
+// Rules have an additional, implicit, component: the parameters. These are
+// portions of the pattern which are left unspecified, yet named so that we can
+// reference them in the replacement term.  The structure of parameters can be
+// partially or even fully specified, in which case they serve just to identify
+// matched nodes for later reference rather than abstract over portions of the
+// AST.  However, in all cases, we refer to named portions of the pattern as
+// parameters.
+//
+// RewriteRule is constructed in a "fluent" style, by creating a builder and
+// chaining setters of individual components.
+// \code
+//   RewriteRule MyRule = buildRule(functionDecl(...)).replaceWith(...);
+// \endcode
+//
+// The \c Transformer class should then be used to apply the rewrite rule and
+// obtain the corresponding replacements.
+struct RewriteRule {
+  // `Matcher` describes the cont

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357576: [LibTooling] Add Transformer, a library for 
source-to-source transformations. (authored by ymandel, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59376?vs=193306&id=193480#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59376

Files:
  include/clang/Tooling/Refactoring/Transformer.h
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/Transformer.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/TransformerTest.cpp

Index: lib/Tooling/Refactoring/Transformer.cpp
===
--- lib/Tooling/Refactoring/Transformer.cpp
+++ lib/Tooling/Refactoring/Transformer.cpp
@@ -0,0 +1,204 @@
+//===--- Transformer.cpp - Transformer library implementation ---*- 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
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Transformer.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace tooling;
+
+using ast_matchers::MatchFinder;
+using ast_type_traits::ASTNodeKind;
+using ast_type_traits::DynTypedNode;
+using llvm::Error;
+using llvm::Expected;
+using llvm::Optional;
+using llvm::StringError;
+using llvm::StringRef;
+using llvm::Twine;
+
+using MatchResult = MatchFinder::MatchResult;
+
+// Did the text at this location originate in a macro definition (aka. body)?
+// For example,
+//
+//   #define NESTED(x) x
+//   #define MACRO(y) { int y  = NESTED(3); }
+//   if (true) MACRO(foo)
+//
+// The if statement expands to
+//
+//   if (true) { int foo = 3; }
+//   ^ ^
+//   Loc1  Loc2
+//
+// For SourceManager SM, SM.isMacroArgExpansion(Loc1) and
+// SM.isMacroArgExpansion(Loc2) are both true, but isOriginMacroBody(sm, Loc1)
+// is false, because "foo" originated in the source file (as an argument to a
+// macro), whereas isOriginMacroBody(SM, Loc2) is true, because "3" originated
+// in the definition of MACRO.
+static bool isOriginMacroBody(const clang::SourceManager &SM,
+  clang::SourceLocation Loc) {
+  while (Loc.isMacroID()) {
+if (SM.isMacroBodyExpansion(Loc))
+  return true;
+// Otherwise, it must be in an argument, so we continue searching up the
+// invocation stack. getImmediateMacroCallerLoc() gives the location of the
+// argument text, inside the call text.
+Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+  return false;
+}
+
+static llvm::Error invalidArgumentError(Twine Message) {
+  return llvm::make_error(llvm::errc::invalid_argument, Message);
+}
+
+static llvm::Error typeError(StringRef Id, const ASTNodeKind &Kind,
+ Twine Message) {
+  return invalidArgumentError(
+  Message + " (node id=" + Id + " kind=" + Kind.asStringRef() + ")");
+}
+
+static llvm::Error missingPropertyError(StringRef Id, Twine Description,
+StringRef Property) {
+  return invalidArgumentError(Description + " requires property '" + Property +
+  "' (node id=" + Id + ")");
+}
+
+static Expected
+getTargetRange(StringRef Target, const DynTypedNode &Node, ASTNodeKind Kind,
+   NodePart TargetPart, ASTContext &Context) {
+  switch (TargetPart) {
+  case NodePart::Node: {
+// For non-expression statements, associate any trailing semicolon with the
+// statement text.  However, if the target was intended as an expression (as
+// indicated by its kind) then we do not associate any trailing semicolon
+// with it.  We only associate the exact expression text.
+if (Node.get() != nullptr) {
+  auto ExprKind = ASTNodeKind::getFromNodeKind();
+  if (!ExprKind.isBaseOf(Kind))
+return fixit::getExtendedRange(Node, tok::TokenKind::semi, Context);
+}
+return CharSourceRange::getTokenRange(Node.getSourceRange());
+  }
+  case NodePart::Member:
+if (auto *M = Node.get())
+  return CharSourceRange::getTokenRange(
+  M->getMemberNameInfo().getSourceRange());
+return typeError(Target, Node.getNodeKind(),
+

[PATCH] D59985: Re-fix invalid address space generation for clk_event_t arguments of enqueue_kernel builtin function

[Alexey Sotkin via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGBuiltin.cpp:3711
+  EventList = EventList->getType()->isIntegerTy()
+  ? Builder.CreateIntToPtr(EventList, EventPtrTy)
+  : Builder.CreatePointerCast(EventList, EventPtrTy);

Anastasia wrote:
> AlexeySotkin wrote:
> > Anastasia wrote:
> > > It seems we are not testing the casts?
> > Do you mean that when we run LIT tests, this code is not executed? If so, 
> > in the modified test below, literal zeros are making clang to execute 
> > CreateIntToPtr call indeed.
> > Or, do you mean that we need some extra check(to make sure the cast will be 
> > successful for example) in the source code itself ?
> I mean since you are generating extra IR nodes we should check in the tests 
> that they appear correctly. I don't see these casts checked in the tests 
> currently.
Since we are casting null constants they are folded to null values, like this 
`%opencl.clk_event_t{{.*}}* addrspace(4)* null`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59985



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

[PATCH] D59985: Re-fix invalid address space generation for clk_event_t arguments of enqueue_kernel builtin function

[Alexey Sotkin via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGBuiltin.cpp:3711
+  EventList = EventList->getType()->isIntegerTy()
+  ? Builder.CreateIntToPtr(EventList, EventPtrTy)
+  : Builder.CreatePointerCast(EventList, EventPtrTy);

AlexeySotkin wrote:
> Anastasia wrote:
> > AlexeySotkin wrote:
> > > Anastasia wrote:
> > > > It seems we are not testing the casts?
> > > Do you mean that when we run LIT tests, this code is not executed? If so, 
> > > in the modified test below, literal zeros are making clang to execute 
> > > CreateIntToPtr call indeed.
> > > Or, do you mean that we need some extra check(to make sure the cast will 
> > > be successful for example) in the source code itself ?
> > I mean since you are generating extra IR nodes we should check in the tests 
> > that they appear correctly. I don't see these casts checked in the tests 
> > currently.
> Since we are casting null constants they are folded to null values, like this 
> `%opencl.clk_event_t{{.*}}* addrspace(4)* null`.
I think `0` is the only possible integral literal, which can be given as the 
events arguments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59985



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

[PATCH] D60183: Fix typos in tests. NFC.

[Xing via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357577: Fix typos in tests. NFC. (authored by Higuoxing, 
committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D60183?vs=193439&id=193482#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60183

Files:
  cfe/trunk/test/Analysis/analyzer-list-configs.c
  compiler-rt/trunk/test/tsan/race_on_heap.cc


Index: compiler-rt/trunk/test/tsan/race_on_heap.cc
===
--- compiler-rt/trunk/test/tsan/race_on_heap.cc
+++ compiler-rt/trunk/test/tsan/race_on_heap.cc
@@ -39,7 +39,7 @@
 // CHECK: WARNING: ThreadSanitizer: data race
 // ...
 // CHECK: Location is heap block of size 99 at [[ADDR]] allocated by thread T1:
-// CHCEK: #0 malloc
+// CHECK: #0 malloc
 // CHECK: #{{1|2}} alloc
 // CHECK: #{{2|3}} AllocThread
 // ...
Index: cfe/trunk/test/Analysis/analyzer-list-configs.c
===
--- cfe/trunk/test/Analysis/analyzer-list-configs.c
+++ cfe/trunk/test/Analysis/analyzer-list-configs.c
@@ -3,7 +3,7 @@
 //
 // CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 

 //
-// CHCEK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, 
-analyzer-config OPTION2=VALUE, ...
+// CHECK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, 
-analyzer-config OPTION2=VALUE, ...
 //
 // CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config 
-Xclang
 //


Index: compiler-rt/trunk/test/tsan/race_on_heap.cc
===
--- compiler-rt/trunk/test/tsan/race_on_heap.cc
+++ compiler-rt/trunk/test/tsan/race_on_heap.cc
@@ -39,7 +39,7 @@
 // CHECK: WARNING: ThreadSanitizer: data race
 // ...
 // CHECK: Location is heap block of size 99 at [[ADDR]] allocated by thread T1:
-// CHCEK: #0 malloc
+// CHECK: #0 malloc
 // CHECK: #{{1|2}} alloc
 // CHECK: #{{2|3}} AllocThread
 // ...
Index: cfe/trunk/test/Analysis/analyzer-list-configs.c
===
--- cfe/trunk/test/Analysis/analyzer-list-configs.c
+++ cfe/trunk/test/Analysis/analyzer-list-configs.c
@@ -3,7 +3,7 @@
 //
 // CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 
 //
-// CHCEK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, -analyzer-config OPTION2=VALUE, ...
+// CHECK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, -analyzer-config OPTION2=VALUE, ...
 //
 // CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60197: [clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh created this revision.
alexfh added reviewers: sammccall, hokein.
Herald added subscribers: kadircet, arphaman, jkorous, xazax.hun.
Herald added a project: clang.

All in-tree clang-tidy checks have been migrated to the new
ClangTidyCheck::registerPPCallbacks method. Time to drop the old one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60197

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -42,7 +42,6 @@
 Preprocessor *PP = &Compiler.getPreprocessor();
 for (auto &Check : Checks) {
   Check->registerMatchers(&Finder);
-  Check->registerPPCallbacks(Compiler);
   Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
 }
 return Finder.newASTConsumer();
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -299,7 +299,6 @@
 for (const auto &Check : CTChecks) {
   // FIXME: the PP callbacks skip the entire preamble.
   // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(*Clang);
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
   Check->registerMatchers(&CTFinder);
 }
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -53,9 +53,6 @@
   /// constructor using the Options.get() methods below.
   ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
 
-  /// DEPRECATED: Use the other overload.
-  virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
-
   /// \brief Override this to register ``PPCallbacks`` in the preprocessor.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -393,7 +393,6 @@
 
   for (auto &Check : Checks) {
 Check->registerMatchers(&*Finder);
-Check->registerPPCallbacks(Compiler);
 Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -42,7 +42,6 @@
 Preprocessor *PP = &Compiler.getPreprocessor();
 for (auto &Check : Checks) {
   Check->registerMatchers(&Finder);
-  Check->registerPPCallbacks(Compiler);
   Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
 }
 return Finder.newASTConsumer();
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -299,7 +299,6 @@
 for (const auto &Check : CTChecks) {
   // FIXME: the PP callbacks skip the entire preamble.
   // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(*Clang);
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
   Check->registerMatchers(&CTFinder);
 }
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -53,9 +53,6 @@
   /// constructor using the Options.get() methods below.
   ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
 
-  /// DEPRECATED: Use the other overload.
-  virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
-
   /// \brief Override this to register ``PPCallbacks`` in the preprocessor.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -393,7 +393,6 @@
 
   for (auto &Check : Checks) {
 Check->registerMatchers(&*Finder);
-Check->registerPPCallbacks(Compiler);
 Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
___
cfe-commits mailing lis

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Alexander Kornienko via Phabricator via cfe-commits]

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

The change looks fine, but I don't understand the description of this revision. 
Could you clarify which checkers you're talking about and which bug you observe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Can you verify that the add_new_check.py script works fine with this new module?
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

r357577 - Fix typos in tests. NFC.

[Xing GUO via cfe-commits]

Author: higuoxing
Date: Wed Apr  3 06:15:09 2019
New Revision: 357577

URL: http://llvm.org/viewvc/llvm-project?rev=357577&view=rev
Log:
Fix typos in tests. NFC.

Reviewers: Higuoxing

Reviewed By: Higuoxing

Subscribers: kubamracek, cfe-commits, #sanitizers, llvm-commits

Tags: #clang, #sanitizers, #llvm

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

Modified:
cfe/trunk/test/Analysis/analyzer-list-configs.c

Modified: cfe/trunk/test/Analysis/analyzer-list-configs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-list-configs.c?rev=357577&r1=357576&r2=357577&view=diff
==
--- cfe/trunk/test/Analysis/analyzer-list-configs.c (original)
+++ cfe/trunk/test/Analysis/analyzer-list-configs.c Wed Apr  3 06:15:09 2019
@@ -3,7 +3,7 @@
 //
 // CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 

 //
-// CHCEK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, 
-analyzer-config OPTION2=VALUE, ...
+// CHECK:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, 
-analyzer-config OPTION2=VALUE, ...
 //
 // CHECK:  clang [CLANG_OPTIONS] -Xclang -analyzer-config 
-Xclang
 //


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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

I do not like that we're removing the namespace qualifier here. I would prefer 
to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` if there is a namespace 
clash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

r357579 - Fixing a typo; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Apr  3 06:37:56 2019
New Revision: 357579

URL: http://llvm.org/viewvc/llvm-project?rev=357579&view=rev
Log:
Fixing a typo; NFC.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=357579&r1=357578&r2=357579&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Apr  3 06:37:56 2019
@@ -2773,7 +2773,7 @@ Decl has pointer identity in the AST.
 Given
   __attribute__((device)) void f() { ... }
 decl(hasAttr(clang::attr::CUDADevice)) matches the function declaration of
-f. If the matcher is use from clang-query, attr::Kind parameter should be
+f. If the matcher is used from clang-query, attr::Kind parameter should be
 passed as a quoted string. e.g., hasAttr("attr::CUDADevice").
 
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=357579&r1=357578&r2=357579&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Apr  3 06:37:56 2019
@@ -6187,7 +6187,7 @@ AST_MATCHER_P(CaseStmt, hasCaseConstant,
 ///   __attribute__((device)) void f() { ... }
 /// \endcode
 /// decl(hasAttr(clang::attr::CUDADevice)) matches the function declaration of
-/// f. If the matcher is use from clang-query, attr::Kind parameter should be
+/// f. If the matcher is used from clang-query, attr::Kind parameter should be
 /// passed as a quoted string. e.g., hasAttr("attr::CUDADevice").
 AST_MATCHER_P(Decl, hasAttr, attr::Kind, AttrKind) {
   for (const auto *Attr : Node.attrs()) {


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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

[John McCall via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D59646



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

[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

[John McCall via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D59988



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

[clang-tools-extra] r357582 - [clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Apr  3 07:03:43 2019
New Revision: 357582

URL: http://llvm.org/viewvc/llvm-project?rev=357582&view=rev
Log:
[clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method

Summary:
All in-tree clang-tidy checks have been migrated to the new
ClangTidyCheck::registerPPCallbacks method. Time to drop the old one.

Reviewers: sammccall, hokein

Reviewed By: hokein

Subscribers: xazax.hun, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=357582&r1=357581&r2=357582&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Wed Apr  3 07:03:43 2019
@@ -393,7 +393,6 @@ ClangTidyASTConsumerFactory::CreateASTCo
 
   for (auto &Check : Checks) {
 Check->registerMatchers(&*Finder);
-Check->registerPPCallbacks(Compiler);
 Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h?rev=357582&r1=357581&r2=357582&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h Wed Apr  3 07:03:43 2019
@@ -53,9 +53,6 @@ public:
   /// constructor using the Options.get() methods below.
   ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
 
-  /// DEPRECATED: Use the other overload.
-  virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
-
   /// \brief Override this to register ``PPCallbacks`` in the preprocessor.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=357582&r1=357581&r2=357582&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Apr  3 07:03:43 2019
@@ -299,7 +299,6 @@ ParsedAST::build(std::unique_ptrregisterPPCallbacks(*Clang);
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
   Check->registerMatchers(&CTFinder);
 }

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=357582&r1=357581&r2=357582&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Wed Apr  3 
07:03:43 2019
@@ -42,7 +42,6 @@ private:
 Preprocessor *PP = &Compiler.getPreprocessor();
 for (auto &Check : Checks) {
   Check->registerMatchers(&Finder);
-  Check->registerPPCallbacks(Compiler);
   Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
 }
 return Finder.newASTConsumer();


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

[PATCH] D60199: [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
krasimir added a reviewer: ioeric.

Currently clang-format would always emit a replacement for multi-block #include
sections if `IBS_Regroup`, even if the sections are correct:

  % cat ~/test.h
  #include 
  
  #include "b.h"
  % bin/clang-format --output-replacements-xml -style=google ~/test.h
  
  
  #include 

#include 
"b.h"
  
  %

This change makes clang-format not emit replacements in this case.
The logic is similar to the one implemented for Java in r354452.


Repository:
  rC Clang

https://reviews.llvm.org/D60199

Files:
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp

Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -8,6 +8,7 @@
 
 #include "FormatTestUtils.h"
 #include "clang/Format/Format.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Debug.h"
 #include "gtest/gtest.h"
 
@@ -24,9 +25,13 @@
   }
 
   std::string sort(StringRef Code, std::vector Ranges,
+   llvm::Optional ExpectedNumRanges = llvm::None,
StringRef FileName = "input.cc") {
 auto Replaces = sortIncludes(FmtStyle, Code, Ranges, FileName);
 Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+if (ExpectedNumRanges) {
+  EXPECT_EQ(*ExpectedNumRanges, Replaces.size());
+}
 auto Sorted = applyAllReplacements(Code, Replaces);
 EXPECT_TRUE(static_cast(Sorted));
 auto Result = applyAllReplacements(
@@ -35,8 +40,10 @@
 return *Result;
   }
 
-  std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
-return sort(Code, GetCodeRange(Code), FileName);
+  std::string sort(StringRef Code,
+   llvm::Optional ExpectedNumRanges = llvm::None,
+   StringRef FileName = "input.cpp") {
+return sort(Code, GetCodeRange(Code), ExpectedNumRanges, FileName);
   }
 
   unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {
@@ -321,6 +328,7 @@
 sort("#include \"llvm/a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "a.cc"));
   EXPECT_EQ("#include \"llvm/a.h\"\n"
 "#include \"b.h\"\n"
@@ -328,6 +336,7 @@
 sort("#include \"llvm/a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "a_test.cc"));
   EXPECT_EQ("#include \"llvm/input.h\"\n"
 "#include \"b.h\"\n"
@@ -335,6 +344,7 @@
 sort("#include \"llvm/input.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "input.mm"));
 
   // Don't allow prefixes.
@@ -344,6 +354,7 @@
 sort("#include \"llvm/not_a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "a.cc"));
 
   // Don't do this for _main and other suffixes.
@@ -353,6 +364,7 @@
 sort("#include \"llvm/a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "a_main.cc"));
 
   // Don't do this in headers.
@@ -362,6 +374,7 @@
 sort("#include \"llvm/a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "a.h"));
 
   // Only do this in the first #include block.
@@ -375,6 +388,7 @@
  "#include \"llvm/a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"b.h\"\n",
+ 1,
  "a.cc"));
 
   // Only recognize the first #include with a matching basename as main include.
@@ -386,6 +400,7 @@
  "#include \"a.h\"\n"
  "#include \"c.h\"\n"
  "#include \"llvm/a.h\"\n",
+ 1,
  "a.cc"));
 }
 
@@ -400,6 +415,7 @@
  "\n"
  "#include \"a.h\"\n"
  "#include \"c.h\"\n",
+ 1,
  "c.cc"));
 }
 
@@ -415,6 +431,7 @@
  "\n"
  "#include \"a.h\"\n"
  "#include \"c.h\"\n",
+ 1,
  "a.cc"));
 }
 
@@ -438,6 +455,7 @@
  "#include \"gmock/gmock.h\"\n"
  "#include \"llvm/X.h\"\n"
  "#include \"LLVM/z.h\"\n",
+ 1,
  "a_TEST.cc"));
 }
 
@@ -449,6 +467,7 @@
 sort("#include \"c_main.h\"\n"
  "#include \"a_other.h\"\n"
  "#include \"important_os_header.h\"\n",
+ 1,
  "c_main.cc"));
 
   // check stable when re-run
@@ -458,6 +477,7 @@
 sort("#include \"important_os_header.h\"\n"
  

[PATCH] D60197: [clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE357582: [clang-tidy] Remove the old 
ClangTidyCheck::registerPPCallbacks method (authored by alexfh, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60197?vs=193481&id=193486#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60197

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyCheck.h
  clangd/ClangdUnit.cpp
  unittests/clang-tidy/ClangTidyTest.h


Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -42,7 +42,6 @@
 Preprocessor *PP = &Compiler.getPreprocessor();
 for (auto &Check : Checks) {
   Check->registerMatchers(&Finder);
-  Check->registerPPCallbacks(Compiler);
   Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
 }
 return Finder.newASTConsumer();
Index: clang-tidy/ClangTidyCheck.h
===
--- clang-tidy/ClangTidyCheck.h
+++ clang-tidy/ClangTidyCheck.h
@@ -53,9 +53,6 @@
   /// constructor using the Options.get() methods below.
   ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
 
-  /// DEPRECATED: Use the other overload.
-  virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
-
   /// \brief Override this to register ``PPCallbacks`` in the preprocessor.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -393,7 +393,6 @@
 
   for (auto &Check : Checks) {
 Check->registerMatchers(&*Finder);
-Check->registerPPCallbacks(Compiler);
 Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -299,7 +299,6 @@
 for (const auto &Check : CTChecks) {
   // FIXME: the PP callbacks skip the entire preamble.
   // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(*Clang);
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
   Check->registerMatchers(&CTFinder);
 }


Index: unittests/clang-tidy/ClangTidyTest.h
===
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -42,7 +42,6 @@
 Preprocessor *PP = &Compiler.getPreprocessor();
 for (auto &Check : Checks) {
   Check->registerMatchers(&Finder);
-  Check->registerPPCallbacks(Compiler);
   Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
 }
 return Finder.newASTConsumer();
Index: clang-tidy/ClangTidyCheck.h
===
--- clang-tidy/ClangTidyCheck.h
+++ clang-tidy/ClangTidyCheck.h
@@ -53,9 +53,6 @@
   /// constructor using the Options.get() methods below.
   ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
 
-  /// DEPRECATED: Use the other overload.
-  virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
-
   /// \brief Override this to register ``PPCallbacks`` in the preprocessor.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -393,7 +393,6 @@
 
   for (auto &Check : Checks) {
 Check->registerMatchers(&*Finder);
-Check->registerPPCallbacks(Compiler);
 Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -299,7 +299,6 @@
 for (const auto &Check : CTChecks) {
   // FIXME: the PP callbacks skip the entire preamble.
   // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(*Clang);
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
   Check->registerMatchers(&CTFinder);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/misc-placement-new-target-size.rst:5
+==
+
+

Eugene.Zelenko wrote:
> Unnecessary empty line.
Somehow empty line is still there.


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

https://reviews.llvm.org/D60139



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

[PATCH] D33841: [clang-tidy] redundant keyword check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8
+
+`extern` is redundant in function declarations
+

koldaniel wrote:
> alexfh wrote:
> > xazax.hun wrote:
> > > alexfh wrote:
> > > > Could you explain, why you think `extern` is redundant in function 
> > > > declarations?
> > > Just to be clear here, do you think there is a case where extern is not 
> > > redundant or you just want the documentation to be extended? 
> > Sorry for being unclear. I would expect a more in-depth explanation of why 
> > the keyword is redundant with references to the appropriate sections of the 
> > standard or some other authoritative source.
> https://en.cppreference.com/w/cpp/language/language_linkage
> 
> The default language linkage is C++, so without any additional parameters it 
> is redundant (**extern "C++"** can also be redundant, but it depends on the 
> context). In C context (**extern "C"**) the situation is the same, **extern** 
> keyword is redundant for function declarations 
> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf - 6.2.2.5) 
This sort of a description would be helpful in the documentation of the check. 
Users are not likely to search for clarifications in code review comments on 
LLVM Phabricator ;)


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

https://reviews.llvm.org/D33841



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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added a comment.

Thank you, I submitted a clang-formatted fix for the if statement.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60141



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

[PATCH] D60201: Make clangd-fuzzer use the normal add_llvm_fuzzer() machinery

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: bkramer.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov, mgorny.

This allows building it even if no fuzzer is enabled. (Sadly, it only builds on 
Linux at the moment.)


https://reviews.llvm.org/D60201

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/fuzzer/CMakeLists.txt
  clang-tools-extra/clangd/fuzzer/ClangdFuzzer.cpp
  clang-tools-extra/clangd/fuzzer/DummyClangdMain.cpp
  clang-tools-extra/clangd/fuzzer/clangd-fuzzer.cpp


Index: clang-tools-extra/clangd/fuzzer/DummyClangdMain.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/fuzzer/DummyClangdMain.cpp
@@ -0,0 +1,18 @@
+//=== DummyClangdMain.cpp - Entry point to sanity check the fuzzer 
===//
+//
+// 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
+//
+//===--===//
+//
+// Implementation of main so we can build and test without linking libFuzzer.
+//
+//===--===//
+
+#include "llvm/FuzzMutate/FuzzerCLI.h"
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+int main(int argc, char *argv[]) {
+  return llvm::runFuzzerOnInputs(argc, argv, LLVMFuzzerTestOneInput);
+}
Index: clang-tools-extra/clangd/fuzzer/CMakeLists.txt
===
--- clang-tools-extra/clangd/fuzzer/CMakeLists.txt
+++ clang-tools-extra/clangd/fuzzer/CMakeLists.txt
@@ -2,13 +2,10 @@
 
 set(LLVM_LINK_COMPONENTS support)
 
-if(LLVM_USE_SANITIZE_COVERAGE)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
-endif()
-
-add_clang_executable(clangd-fuzzer
-  EXCLUDE_FROM_ALL
-  ClangdFuzzer.cpp
+# This fuzzer runs on oss-fuzz, so keep it around even if it looks 
unreferenced.
+add_llvm_fuzzer(clangd-fuzzer
+  clangd-fuzzer.cpp
+  DUMMY_MAIN DummyClangdMain.cpp
   )
 
 target_link_libraries(clangd-fuzzer
@@ -20,5 +17,4 @@
   clangSema
   clangTooling
   clangToolingCore
-  ${LLVM_LIB_FUZZING_ENGINE}
   )
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -116,7 +116,8 @@
   )
 
 add_subdirectory(refactor/tweaks)
-if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
+if (LINUX)
+  # FIXME: Make fuzzer not use linux-specific APIs, build it everywhere.
   add_subdirectory(fuzzer)
 endif()
 add_subdirectory(tool)


Index: clang-tools-extra/clangd/fuzzer/DummyClangdMain.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/fuzzer/DummyClangdMain.cpp
@@ -0,0 +1,18 @@
+//=== DummyClangdMain.cpp - Entry point to sanity check the fuzzer ===//
+//
+// 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
+//
+//===--===//
+//
+// Implementation of main so we can build and test without linking libFuzzer.
+//
+//===--===//
+
+#include "llvm/FuzzMutate/FuzzerCLI.h"
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+int main(int argc, char *argv[]) {
+  return llvm::runFuzzerOnInputs(argc, argv, LLVMFuzzerTestOneInput);
+}
Index: clang-tools-extra/clangd/fuzzer/CMakeLists.txt
===
--- clang-tools-extra/clangd/fuzzer/CMakeLists.txt
+++ clang-tools-extra/clangd/fuzzer/CMakeLists.txt
@@ -2,13 +2,10 @@
 
 set(LLVM_LINK_COMPONENTS support)
 
-if(LLVM_USE_SANITIZE_COVERAGE)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
-endif()
-
-add_clang_executable(clangd-fuzzer
-  EXCLUDE_FROM_ALL
-  ClangdFuzzer.cpp
+# This fuzzer runs on oss-fuzz, so keep it around even if it looks unreferenced.
+add_llvm_fuzzer(clangd-fuzzer
+  clangd-fuzzer.cpp
+  DUMMY_MAIN DummyClangdMain.cpp
   )
 
 target_link_libraries(clangd-fuzzer
@@ -20,5 +17,4 @@
   clangSema
   clangTooling
   clangToolingCore
-  ${LLVM_LIB_FUZZING_ENGINE}
   )
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -116,7 +116,8 @@
   )
 
 add_subdirectory(refactor/tweaks)
-if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
+if (LINUX)
+  # FIXME: Make fuzzer not use linux-specific APIs, build it everywhere.
   add_subdirectory(fuzze

r357585 - Add a new attribute documentation category for declarations.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Apr  3 07:26:32 2019
New Revision: 357585

URL: http://llvm.org/viewvc/llvm-project?rev=357585&view=rev
Log:
Add a new attribute documentation category for declarations.

This moves documentation for some attributes into new categories that are 
hopefully a bit more clear. In general, "Type" documentation should be for 
attributes that appertain to types while "Declaration" documentation should be 
for attributes that appertain to declarations other than functions or variables.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=357585&r1=357584&r2=357585&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Apr  3 07:26:32 2019
@@ -16,6 +16,8 @@ def DocCatFunction : DocumentationCatego
 def DocCatVariable : DocumentationCategory<"Variable Attributes">;
 def DocCatType : DocumentationCategory<"Type Attributes">;
 def DocCatStmt : DocumentationCategory<"Statement Attributes">;
+def DocCatDecl : DocumentationCategory<"Declaration Attributes">;
+
 // Attributes listed under the Undocumented category do not generate any public
 // documentation. Ideally, this category should be used for internal-only
 // attributes which contain no spellings.

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=357585&r1=357584&r2=357585&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Apr  3 07:26:32 2019
@@ -1057,7 +1057,7 @@ implementation of an override in a subcl
 }
 
 def ObjCRuntimeNameDocs : Documentation {
-let Category = DocCatFunction;
+let Category = DocCatDecl;
 let Content = [{
 By default, the Objective-C interface or protocol identifier is used
 in the metadata name for that object. The `objc_runtime_name`
@@ -1078,14 +1078,14 @@ can only be placed before an @protocol o
 }
 
 def ObjCRuntimeVisibleDocs : Documentation {
-let Category = DocCatFunction;
+let Category = DocCatDecl;
 let Content = [{
 This attribute specifies that the Objective-C class to which it applies is 
visible to the Objective-C runtime but not to the linker. Classes annotated 
with this attribute cannot be subclassed and cannot have categories defined for 
them.
 }];
 }
 
 def ObjCBoxableDocs : Documentation {
-let Category = DocCatFunction;
+let Category = DocCatDecl;
 let Content = [{
 Structs and unions marked with the ``objc_boxable`` attribute can be used
 with the Objective-C boxed expression syntax, ``@(...)``.
@@ -1305,7 +1305,7 @@ Also see the documentation for `@availab
 }
 
 def ExternalSourceSymbolDocs : Documentation {
-  let Category = DocCatFunction;
+  let Category = DocCatDecl;
   let Content = [{
 The ``external_source_symbol`` attribute specifies that a declaration 
originates
 from an external source and describes the nature of that source.
@@ -2449,7 +2449,7 @@ behavior of the program is undefined.
 }
 
 def FlagEnumDocs : Documentation {
-  let Category = DocCatType;
+  let Category = DocCatDecl;
   let Content = [{
 This attribute can be added to an enumerator to signal to the compiler that it
 is intended to be used as a flag type. This will cause the compiler to assume
@@ -2459,7 +2459,7 @@ manipulating bits of the enumerator when
 }
 
 def EnumExtensibilityDocs : Documentation {
-  let Category = DocCatType;
+  let Category = DocCatDecl;
   let Content = [{
 Attribute ``enum_extensibility`` is used to distinguish between enum 
definitions
 that are extensible and those that are not. The attribute can take either
@@ -2508,7 +2508,7 @@ standard and instructs clang to be more
 }
 
 def EmptyBasesDocs : Documentation {
-  let Category = DocCatType;
+  let Category = DocCatDecl;
   let Content = [{
 The empty_bases attribute permits the compiler to utilize the
 empty-base-optimization more frequently.
@@ -2518,7 +2518,7 @@ It is only supported when using the Micr
 }
 
 def LayoutVersionDocs : Documentation {
-  let Category = DocCatType;
+  let Category = DocCatDecl;
   let Content = [{
 The layout_version attribute requests that the compiler utilize the class
 layout rules of a particular compiler version.
@@ -2544,7 +2544,7 @@ changes.
 }
 
 def TrivialABIDocs : Documentation {
-  let Category = DocCatVariable;
+  let Category = DocCatDecl;
   let Content = [{
 The ``trivial_abi`` attribute can be applied to a C++ class, struct, or union.
 It instructs the compiler to pass and return the type using the C ABI for the
@@ -2586,7 +2586,7 @@ Attribute ``trivial_abi`` has no effect
 }
 
 def MSInheritanceDocs : Doc

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{

aaron.ballman wrote:
> slavapestov wrote:
> > aaron.ballman wrote:
> > > This seems like the wrong category -- the attribute doesn't apply to 
> > > functions.
> > Would DocCatType make more sense? Would you like me to change 
> > ObjCRuntimeVisible and a handful of other miscategorized attributes too?
> Oye, it looks like we have a lot of inconsistent categories currently. It's 
> not really a type attribute, it's a declaration attribute, but we don't have 
> such a notion yet. Go ahead and leave this as `DocCatType` for now; I'll see 
> about cleaning this up in a follow-up.
I added `DocCatDecl` in r357585, which I think would be the correct category to 
use here.


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

https://reviews.llvm.org/D59628



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

r357589 - Whitespace and formatting changes; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Apr  3 07:40:00 2019
New Revision: 357589

URL: http://llvm.org/viewvc/llvm-project?rev=357589&view=rev
Log:
Whitespace and formatting changes; NFC.

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=357589&r1=357588&r2=357589&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Apr  3 07:40:00 2019
@@ -921,8 +921,6 @@ and Objective-C methods.
   }];
 }
 
-
-
 def NoDebugDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{
@@ -1080,7 +1078,10 @@ can only be placed before an @protocol o
 def ObjCRuntimeVisibleDocs : Documentation {
 let Category = DocCatDecl;
 let Content = [{
-This attribute specifies that the Objective-C class to which it applies is 
visible to the Objective-C runtime but not to the linker. Classes annotated 
with this attribute cannot be subclassed and cannot have categories defined for 
them.
+This attribute specifies that the Objective-C class to which it applies is
+visible to the Objective-C runtime but not to the linker. Classes annotated
+with this attribute cannot be subclassed and cannot have categories defined for
+them.
 }];
 }
 
@@ -2774,8 +2775,6 @@ def PipelineHintDocs : Documentation {
   }];
 }
 
-
-
 def OpenCLUnrollHintDocs : Documentation {
   let Category = DocCatStmt;
   let Content = [{
@@ -3345,7 +3344,7 @@ jumps from i386 arch code).
   }];
 }
 
-def AnyX86NoCfCheckDocs : Documentation{
+def AnyX86NoCfCheckDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Jump Oriented Programming attacks rely on tampering with addresses used by


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

[PATCH] D60199: [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: unittests/Format/SortIncludesTest.cpp:28
   std::string sort(StringRef Code, std::vector Ranges,
+   llvm::Optional ExpectedNumRanges = llvm::None,
StringRef FileName = "input.cc") {

As most cases would pass `1`, maybe it would make sense to make this a class 
member that defaults to `1`? It can be explicitly set in specific tests when 
needed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60199



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

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Got here trying to understand how D60126  
works.

It seems there's two fairly independent changes here:

- the one described, allow actions to run without a preamble
- defer the blocking getCompileCommand() call until we're building the preamble

Certainly they interact to make zero-latency code completion work, but is it 
possible to pull the latter out of this patch?
There are other approaches to the second part (e.g. giving TUScheduler a 
reference to the global CDB) that might be cleaner. I don't think it needs to 
block D60126 .




Comment at: clangd/TUScheduler.h:204
+   Callback Action,
+   bool AllowFallback = false);
 

I think this isn't orthogonal to `PreambleConsistency`.
When would we use AllowFallback = true but PreambleConsistency = Consistent?

Two possible options:
 - adding a new `StaleOrAbsent` option to PreambleConsistency
 - changing `Stale` to these new semantics, as codeComplete is the only caller
The problem with the latter is we can't put it behind a flag.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59811



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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193496.
ymandel added a comment.

Sever dependency on NodeId and some general cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,204 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// 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 "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil_generators::node;
+using tooling::stencil_generators::snode;
+using tooling::stencil_generators::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext &Context = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil &Stencil,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " 

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D59449#1449664 , @zinovy.nis wrote:

> > Why not just use clang-tidy-diff.py? The clang-tidy-diff.py script has a 
> > distinct and somewhat self-documenting name and a very specific purpose 
> > (find clang-tidy regressions in a patch), while run-clang-tidy.py is more 
> > focused on running over larger bodies of code with a purpose of analyzing 
> > or cleaning up. Is there any benefit in having all functionality in 
> > run-clang-tidy.py?
>
> Both scripts have almost the same implementation, almost the same syntax (at 
> least after -j and -timeout we introduced). So why not merge them.


There are multiple (mostly user-facing) reasons to prefer separate scripts. 
Simplicity, focus, and internal consistency come to mind first. The 
clang-tidy-diff being separate and serving a single purpose makes it easier to 
find, understand, and use. If we merge the two scripts together, will it be 
easy to understand the different use-cases? Will all options be compatible? 
(E.g. -header-filter doesn't make much sense for the --diff mode, since the 
diff defines a different filter)


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

https://reviews.llvm.org/D59449



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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193498.
ymandel added a comment.

Remove noisy default-defined constructors/operators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,204 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// 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 "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil_generators::node;
+using tooling::stencil_generators::snode;
+using tooling::stencil_generators::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext &Context = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil &Stencil,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " <

[PATCH] D60199: [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 193500.
krasimir added a comment.

- Address review comments


Repository:
  rC Clang

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

https://reviews.llvm.org/D60199

Files:
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp

Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -8,6 +8,7 @@
 
 #include "FormatTestUtils.h"
 #include "clang/Format/Format.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Debug.h"
 #include "gtest/gtest.h"
 
@@ -24,9 +25,11 @@
   }
 
   std::string sort(StringRef Code, std::vector Ranges,
-   StringRef FileName = "input.cc") {
+   StringRef FileName = "input.cc",
+   unsigned ExpectedNumRanges = 1) {
 auto Replaces = sortIncludes(FmtStyle, Code, Ranges, FileName);
 Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+EXPECT_EQ(ExpectedNumRanges, Replaces.size());
 auto Sorted = applyAllReplacements(Code, Replaces);
 EXPECT_TRUE(static_cast(Sorted));
 auto Result = applyAllReplacements(
@@ -35,8 +38,10 @@
 return *Result;
   }
 
-  std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
-return sort(Code, GetCodeRange(Code), FileName);
+  std::string sort(StringRef Code,
+   StringRef FileName = "input.cpp",
+   unsigned ExpectedNumRanges = 1) {
+return sort(Code, GetCodeRange(Code), FileName, ExpectedNumRanges);
   }
 
   unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {
@@ -151,7 +156,7 @@
  "#include \n"
  "#include \n"
  "#include \n"
- "/* clang-format onwards */\n"));
+ "/* clang-format onwards */\n", "input.h", 2));
 }
 
 TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {
@@ -161,7 +166,8 @@
 "#include \"b.h\"\n",
 sort("#include \"a.h\"\n"
  "#include \"c.h\"\n"
- "#include \"b.h\"\n"));
+ "#include \"b.h\"\n",
+ "input.h", 0));
 }
 
 TEST_F(SortIncludesTest, MixIncludeAndImport) {
@@ -214,7 +220,7 @@
 sort("#include \"a.h\"\n"
  "#include \"c.h\"\n"
  "\n"
- "#include \"b.h\"\n"));
+ "#include \"b.h\"\n", "input.h", 0));
 }
 
 TEST_F(SortIncludesTest, SortsAllBlocksWhenMerging) {
@@ -458,7 +464,7 @@
 sort("#include \"important_os_header.h\"\n"
  "#include \"c_main.h\"\n"
  "#include \"a_other.h\"\n",
- "c_main.cc"));
+ "c_main.cc", 0));
 }
 
 TEST_F(SortIncludesTest, PriorityGroupsAreSeparatedWhenRegroupping) {
@@ -486,7 +492,7 @@
  "#include \"c_main.h\"\n"
  "\n"
  "#include \"a_other.h\"\n",
- "c_main.cc"));
+ "c_main.cc", 0));
 }
 
 TEST_F(SortIncludesTest, CalculatesCorrectCursorPosition) {
@@ -634,7 +640,17 @@
 sort(""));
+ "-->", "input.h", 0));
+}
+
+TEST_F(SortIncludesTest, DoNotOutputReplacementsForSortedBlocksWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = R"(
+#include "b.h"
+
+#include 
+)";
+  EXPECT_EQ(Code, sort(Code, "input.h", 0));
 }
 
 } // end namespace
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1753,6 +1753,7 @@
 static void sortCppIncludes(const FormatStyle &Style,
 const SmallVectorImpl &Includes,
 ArrayRef Ranges, StringRef FileName,
+StringRef Code,
 tooling::Replacements &Replaces, unsigned *Cursor) {
   unsigned IncludesBeginOffset = Includes.front().Offset;
   unsigned IncludesEndOffset =
@@ -1788,6 +1789,10 @@
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
+  // In case Style.IncldueStyle.IncludeBlocks != IBS_Preserve, this check is not
+  // enough as additional newlines might be added or removed across #include
+  // blocks. This we handle below by generating the updated #imclude blocks and
+  // comparing it to the original.
   if (Indices.size() == Includes.size() &&
   std::is_sorted(Indices.begin(), Indices.end()) &&
   Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
@@ -1808,6 +1813,11 @@
 CurrentCategory = Includes[Index].Category;
   }
 
+  // If the #includes are out of order, we generate a single replacement fixing
+  // the entire range of blocks. Otherwise, no replacement is generated.
+  if (result == Code.substr(IncludesBeginOffset, Include

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60151#1453099 , @alexfh wrote:

> The change looks fine, but I don't understand the description of this 
> revision. Could you clarify which checkers you're talking about and which bug 
> you observe?


Sorry for not being clearer in my original description.

The error can be triggered by opening up a new `llvm` namespace under 
`clang::tidy` prior to referencing `llvm::SmallSet`, which would happened if a 
new llvm checker that gets included in `LLVMTidyModule.cpp` before 
`HeaderGuardCheck.h` -- `HeaderGuardCheck.h` includes `HeaderFileExternsions.h` 
prior to opening the new `llvm` namespace.  Here's the error message you'd get:

  
llvm-project/clang-tools-extra/clang-tidy/llvm/../utils/../utils/HeaderFileExtensionsUtils.h:24:9:
 error: no template named 'SmallSet' in namespace 'clang::tidy::llvm'; did you 
mean '::llvm::SmallSet'?
  typedef llvm::SmallSet HeaderFileExtensionsSet;
  ^~
  ::llvm::SmallSet
  llvm-project/llvm/include/llvm/ADT/SmallSet.h:134:7: note: '::llvm::SmallSet' 
declared here
  class SmallSet {
^
  
llvm-project/clang-tools-extra/clang-tidy/llvm/../utils/../utils/HeaderFileExtensionsUtils.h:24:30:
 error: no member named 'StringRef' in namespace 'clang::tidy::llvm'
  typedef llvm::SmallSet HeaderFileExtensionsSet;
 ~~^
  2 errors generated.

Here's the one line contrived change needed to demonstrate the error produced 
above, which is the same effect of including an `llvm` checker header before 
including this file:

  --- a/clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  +++ b/clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  @@ -16,6 +16,7 @@
  
   namespace clang {
   namespace tidy {
  +namespace llvm {}
   namespace utils {
  
   typedef llvm::SmallSet HeaderFileExtensionsSet;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D60203: Updating Chromium's Java import order

[Sam Maier via Phabricator via cfe-commits]

SamMaier created this revision.
SamMaier added a reviewer: thakis.
Herald added subscribers: cfe-commits, srhines.
Herald added a project: clang.

Adding in androidx as another import group.


Repository:
  rC Clang

https://reviews.llvm.org/D60203

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -893,9 +893,16 @@
 // See styleguide for import groups:
 // 
https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md#Import-Order
 ChromiumStyle.JavaImportGroups = {
-"android",  "com",  "dalvik",
-"junit","org",  "com.google.android.apps.chrome",
-"org.chromium", "java", "javax",
+"android",
+"androidx",
+"com",
+"dalvik",
+"junit",
+"org",
+"com.google.android.apps.chrome",
+"org.chromium",
+"java",
+"javax",
 };
 ChromiumStyle.SortIncludes = true;
   } else if (Language == FormatStyle::LK_JavaScript) {


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -893,9 +893,16 @@
 // See styleguide for import groups:
 // https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md#Import-Order
 ChromiumStyle.JavaImportGroups = {
-"android",  "com",  "dalvik",
-"junit","org",  "com.google.android.apps.chrome",
-"org.chromium", "java", "javax",
+"android",
+"androidx",
+"com",
+"dalvik",
+"junit",
+"org",
+"com.google.android.apps.chrome",
+"org.chromium",
+"java",
+"javax",
 };
 ChromiumStyle.SortIncludes = true;
   } else if (Language == FormatStyle::LK_JavaScript) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357599 - [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Wed Apr  3 08:16:04 2019
New Revision: 357599

URL: http://llvm.org/viewvc/llvm-project?rev=357599&view=rev
Log:
[clang-format] Do not emit replacements while regrouping if Cpp includes are OK

Summary:
Currently clang-format would always emit a replacement for multi-block #include
sections if `IBS_Regroup`, even if the sections are correct:
```
% cat ~/test.h
#include 

#include "b.h"
% bin/clang-format --output-replacements-xml -style=google ~/test.h


#include 

#include 
"b.h"

%
```

This change makes clang-format not emit replacements in this case.
The logic is similar to the one implemented for Java in r354452.

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/SortIncludesTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=357599&r1=357598&r2=357599&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Apr  3 08:16:04 2019
@@ -1753,6 +1753,7 @@ FindCursorIndex(const SmallVectorImpl &Includes,
 ArrayRef Ranges, StringRef 
FileName,
+StringRef Code,
 tooling::Replacements &Replaces, unsigned *Cursor) 
{
   unsigned IncludesBeginOffset = Includes.front().Offset;
   unsigned IncludesEndOffset =
@@ -1788,6 +1789,10 @@ static void sortCppIncludes(const Format
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
+  // In case Style.IncldueStyle.IncludeBlocks != IBS_Preserve, this check is 
not
+  // enough as additional newlines might be added or removed across #include
+  // blocks. This we handle below by generating the updated #imclude blocks and
+  // comparing it to the original.
   if (Indices.size() == Includes.size() &&
   std::is_sorted(Indices.begin(), Indices.end()) &&
   Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
@@ -1808,6 +1813,11 @@ static void sortCppIncludes(const Format
 CurrentCategory = Includes[Index].Category;
   }
 
+  // If the #includes are out of order, we generate a single replacement fixing
+  // the entire range of blocks. Otherwise, no replacement is generated.
+  if (result == Code.substr(IncludesBeginOffset, IncludesBlockSize))
+return;
+
   auto Err = Replaces.add(tooling::Replacement(
   FileName, Includes.front().Offset, IncludesBlockSize, result));
   // FIXME: better error handling. For now, just skip the replacement for the
@@ -1876,8 +1886,8 @@ tooling::Replacements sortCppIncludes(co
   MainIncludeFound = true;
 IncludesInBlock.push_back({IncludeName, Line, Prev, Category});
   } else if (!IncludesInBlock.empty() && !EmptyLineSkipped) {
-sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces,
-Cursor);
+sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
+Replaces, Cursor);
 IncludesInBlock.clear();
 FirstIncludeBlock = false;
   }
@@ -1887,8 +1897,10 @@ tooling::Replacements sortCppIncludes(co
   break;
 SearchFrom = Pos + 1;
   }
-  if (!IncludesInBlock.empty())
-sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces, 
Cursor);
+  if (!IncludesInBlock.empty()) {
+sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code, Replaces,
+Cursor);
+  }
   return Replaces;
 }
 

Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=357599&r1=357598&r2=357599&view=diff
==
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Wed Apr  3 08:16:04 2019
@@ -8,6 +8,7 @@
 
 #include "FormatTestUtils.h"
 #include "clang/Format/Format.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Debug.h"
 #include "gtest/gtest.h"
 
@@ -24,9 +25,11 @@ protected:
   }
 
   std::string sort(StringRef Code, std::vector Ranges,
-   StringRef FileName = "input.cc") {
+   StringRef FileName = "input.cc",
+   unsigned ExpectedNumRanges = 1) {
 auto Replaces = sortIncludes(FmtStyle, Code, Ranges, FileName);
 Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+EXPECT_EQ(ExpectedNumRanges, Replaces.size());
 auto Sorted = applyAllReplacements(Code, Replaces);
 EXPECT_TRUE(static_cast(Sorted));
 auto Result = applyAllReplacements(
@@ -35,8 +38,10 @@ protected:
 return *Result;
   }
 
-  std:

[PATCH] D60199: [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357599: [clang-format] Do not emit replacements while 
regrouping if Cpp includes are OK (authored by krasimir, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60199?vs=193500&id=193508#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60199

Files:
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp

Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1753,6 +1753,7 @@
 static void sortCppIncludes(const FormatStyle &Style,
 const SmallVectorImpl &Includes,
 ArrayRef Ranges, StringRef FileName,
+StringRef Code,
 tooling::Replacements &Replaces, unsigned *Cursor) {
   unsigned IncludesBeginOffset = Includes.front().Offset;
   unsigned IncludesEndOffset =
@@ -1788,6 +1789,10 @@
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
+  // In case Style.IncldueStyle.IncludeBlocks != IBS_Preserve, this check is not
+  // enough as additional newlines might be added or removed across #include
+  // blocks. This we handle below by generating the updated #imclude blocks and
+  // comparing it to the original.
   if (Indices.size() == Includes.size() &&
   std::is_sorted(Indices.begin(), Indices.end()) &&
   Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
@@ -1808,6 +1813,11 @@
 CurrentCategory = Includes[Index].Category;
   }
 
+  // If the #includes are out of order, we generate a single replacement fixing
+  // the entire range of blocks. Otherwise, no replacement is generated.
+  if (result == Code.substr(IncludesBeginOffset, IncludesBlockSize))
+return;
+
   auto Err = Replaces.add(tooling::Replacement(
   FileName, Includes.front().Offset, IncludesBlockSize, result));
   // FIXME: better error handling. For now, just skip the replacement for the
@@ -1876,8 +1886,8 @@
   MainIncludeFound = true;
 IncludesInBlock.push_back({IncludeName, Line, Prev, Category});
   } else if (!IncludesInBlock.empty() && !EmptyLineSkipped) {
-sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces,
-Cursor);
+sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
+Replaces, Cursor);
 IncludesInBlock.clear();
 FirstIncludeBlock = false;
   }
@@ -1887,8 +1897,10 @@
   break;
 SearchFrom = Pos + 1;
   }
-  if (!IncludesInBlock.empty())
-sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces, Cursor);
+  if (!IncludesInBlock.empty()) {
+sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code, Replaces,
+Cursor);
+  }
   return Replaces;
 }
 
Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -8,6 +8,7 @@
 
 #include "FormatTestUtils.h"
 #include "clang/Format/Format.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Debug.h"
 #include "gtest/gtest.h"
 
@@ -24,9 +25,11 @@
   }
 
   std::string sort(StringRef Code, std::vector Ranges,
-   StringRef FileName = "input.cc") {
+   StringRef FileName = "input.cc",
+   unsigned ExpectedNumRanges = 1) {
 auto Replaces = sortIncludes(FmtStyle, Code, Ranges, FileName);
 Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
+EXPECT_EQ(ExpectedNumRanges, Replaces.size());
 auto Sorted = applyAllReplacements(Code, Replaces);
 EXPECT_TRUE(static_cast(Sorted));
 auto Result = applyAllReplacements(
@@ -35,8 +38,10 @@
 return *Result;
   }
 
-  std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
-return sort(Code, GetCodeRange(Code), FileName);
+  std::string sort(StringRef Code,
+   StringRef FileName = "input.cpp",
+   unsigned ExpectedNumRanges = 1) {
+return sort(Code, GetCodeRange(Code), FileName, ExpectedNumRanges);
   }
 
   unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {
@@ -151,7 +156,7 @@
  "#include \n"
  "#include \n"
  "#include \n"
- "/* clang-format onwards */\n"));
+ "/* clang-format onwards */\n", "input.h", 2));
 }
 
 TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {
@@ -161,7 +166,8 @@
 "#include \"b.h\"\n",
 sort("#include \"a.h\"\n"
  "#include \"c.h\"\n"
- "#include \"b.h\"\n"));
+ "#include \"b.h\"\n",
+

[PATCH] D59985: Re-fix invalid address space generation for clk_event_t arguments of enqueue_kernel builtin function

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin added a comment.

Alternative way to fix it is to use `isNullPointerConstant` like we do in 
`SemaOpenCLBuiltinEnqueueKernel`. So in case we have a zero literal value we 
can emit `ConstantPointerNull` directly, without `EmitScalarExpr` .


Repository:
  rC Clang

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

https://reviews.llvm.org/D59985



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

[PATCH] D60203: Updating Chromium's Java import order

[Nico Weber via Phabricator via cfe-commits]

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

Thanks!

(Test?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60203



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 193510.
hokein added a comment.

Emit the check fix description via diagnostic::note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
  clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
  clang/include/clang/Tooling/Core/Diagnostic.h
  clang/include/clang/Tooling/DiagnosticsYaml.h
  clang/lib/Tooling/Core/Diagnostic.cpp
  clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Index: clang/unittests/Tooling/DiagnosticsYamlTest.cpp
===
--- clang/unittests/Tooling/DiagnosticsYamlTest.cpp
+++ clang/unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -20,11 +20,13 @@
 using clang::tooling::Diagnostic;
 
 static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset,
- const std::string &FilePath) {
+ const std::string &FilePath,
+ const StringMap &Fix) {
   DiagnosticMessage DiagMessage;
   DiagMessage.Message = Message;
   DiagMessage.FileOffset = FileOffset;
   DiagMessage.FilePath = FilePath;
+  DiagMessage.Fix = Fix;
   return DiagMessage;
 }
 
@@ -32,10 +34,52 @@
  const std::string &Message, int FileOffset,
  const std::string &FilePath,
  const StringMap &Fix) {
-  return Diagnostic(DiagnosticName, makeMessage(Message, FileOffset, FilePath),
-Fix, {}, Diagnostic::Warning, "path/to/build/directory");
+  return Diagnostic(DiagnosticName,
+makeMessage(Message, FileOffset, FilePath, Fix), {},
+Diagnostic::Warning, "path/to/build/directory");
 }
 
+static const char *YAMLContent =
+"---\n"
+"MainSourceFile:  'path/to/source.cpp'\n"
+"Diagnostics: \n"
+"  - DiagnosticName:  'diagnostic#1\'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #1'\n"
+"  FilePath:'path/to/source.cpp'\n"
+"  FileOffset:  55\n"
+"  Replacements:\n"
+"- FilePath:'path/to/source.cpp'\n"
+"  Offset:  100\n"
+"  Length:  12\n"
+"  ReplacementText: 'replacement #1'\n"
+"  - DiagnosticName:  'diagnostic#2'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #2'\n"
+"  FilePath:'path/to/header.h'\n"
+"  FileOffset:  60\n"
+"  Replacements:\n"
+"- FilePath:'path/to/header.h'\n"
+"  Offset:  62\n"
+"  Length:  2\n"
+"  ReplacementText: 'replacement #2'\n"
+"  - DiagnosticName:  'diagnostic#3'\n"
+"DiagnosticMessage: \n"
+"  Message: 'message #3'\n"
+"  FilePath:'path/to/source2.cpp'\n"
+"  FileOffset:  72\n"
+"  Replacements:[]\n"
+"Notes:   \n"
+"  - Message: Note1\n"
+"FilePath:'path/to/note1.cpp'\n"
+"FileOffset:  88\n"
+"Replacements:[]\n"
+"  - Message: Note2\n"
+"FilePath:'path/to/note2.cpp'\n"
+"FileOffset:  99\n"
+"Replacements:[]\n"
+"...\n";
+
 TEST(DiagnosticsYamlTest, serializesDiagnostics) {
   TranslationUnitDiagnostics TUD;
   TUD.MainSourceFile = "path/to/source.cpp";
@@ -55,9 +99,9 @@
   TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72,
"path/to/source2.cpp", {}));
   TUD.Diagnostics.back().Notes.push_back(
-  makeMessage("Note1", 88, "path/to/note1.cpp"));
+  makeMessage("Note1", 88, "path/to/note1.cpp", {}));
   TUD.Diagnostics.back().Notes.push_back(
-  makeMessage("Note2", 99, "path/to/note2.cpp"));
+  makeMessage("Note2", 99, "path/to/note2.cpp", {}));
 
   std::string YamlContent;
   raw_string_ostream YamlContentStream(YamlContent);
@@ -65,80 +109,12 @@
   yaml::Output YAML(YamlContentStream);
   YAML << TUD;
 
-  EXPECT_EQ("---\n"
-"MainSourceFile:  'path/to/source.cpp'\n"
-"Diagnostics: \n"
-"  - DiagnosticName:  'diagnostic#1\'\n"
-"Message: 'message #1'\n"
-"FileOffset:  55\n"
-"FilePath:'path/to/source.cpp'\n"
-"Replacements:\n"
-"  - 

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

aaron.ballman wrote:
> I do not like that we're removing the namespace qualifier here. I would 
> prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` if there is a 
> namespace clash.
Other than aesthetics, the reason I don't like the idea of fully scoping these 
types, at least without a comment, is that the error is triggered by some other 
code gets included first, and has nothing to do with this code -- there's 
nothing actually wrong with the original code.  So it could/would be confusing 
for a reader later on wondering why you needed to fully scope these types, and 
not others.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Running out of time today, the patch is not finished yet, but it should be good 
for another initial review/comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

hintonda wrote:
> aaron.ballman wrote:
> > I do not like that we're removing the namespace qualifier here. I would 
> > prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` if there is 
> > a namespace clash.
> Other than aesthetics, the reason I don't like the idea of fully scoping 
> these types, at least without a comment, is that the error is triggered by 
> some other code gets included first, and has nothing to do with this code -- 
> there's nothing actually wrong with the original code.  So it could/would be 
> confusing for a reader later on wondering why you needed to fully scope these 
> types, and not others.
I would argue that the original code is wrong to not use fully-qualified 
namespace specifiers. The issue is that we have two different namespaces named 
`llvm` and have gotten away with poor namespace hygiene by accident. Either we 
should rename the clang-tidy `llvm` namespace to something that does not 
conflict, or we should consistently use fully-qualified namespace specifiers 
when in clang-tidy and needing to refer to an `llvm` namespace explicitly.

I think this patch goes in the wrong direction by making it easier to limp 
along with poor namespace hygiene.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > I do not like that we're removing the namespace qualifier here. I would 
> > > prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` if there 
> > > is a namespace clash.
> > Other than aesthetics, the reason I don't like the idea of fully scoping 
> > these types, at least without a comment, is that the error is triggered by 
> > some other code gets included first, and has nothing to do with this code 
> > -- there's nothing actually wrong with the original code.  So it 
> > could/would be confusing for a reader later on wondering why you needed to 
> > fully scope these types, and not others.
> I would argue that the original code is wrong to not use fully-qualified 
> namespace specifiers. The issue is that we have two different namespaces 
> named `llvm` and have gotten away with poor namespace hygiene by accident. 
> Either we should rename the clang-tidy `llvm` namespace to something that 
> does not conflict, or we should consistently use fully-qualified namespace 
> specifiers when in clang-tidy and needing to refer to an `llvm` namespace 
> explicitly.
> 
> I think this patch goes in the wrong direction by making it easier to limp 
> along with poor namespace hygiene.
By fully qualified, do you mean appending the global namespace, `::` to 
everything?   I actually like using `llvm::`, but `::llvm::` is odd and needs 
explanation.

I'd be happy to abandon this change and instead rename the `clang::tidy::llvm` 
to `clang::tidy::something_else`, if that's what the community would prefer.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via cfe-commits]

rnk marked 2 inline comments as done.
rnk added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60018



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

[PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 193526.
rnk added a comment.

- final updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60018

Files:
  lld/COFF/PDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
  llvm/include/llvm/DebugInfo/CodeView/RecordSerialization.h
  llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h
  llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
  llvm/lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
  llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
  llvm/lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp
  llvm/lib/DebugInfo/CodeView/GlobalTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp
  llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
  llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
  llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
  llvm/lib/DebugInfo/CodeView/TypeTableCollection.cpp
  llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
  llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
  llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
  llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

Index: llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
===
--- llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
+++ llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
@@ -42,8 +42,6 @@
 }
 
 inline bool operator==(const CVType &R1, const CVType &R2) {
-  if (R1.Type != R2.Type)
-return false;
   if (R1.RecordData != R2.RecordData)
 return false;
   return true;
@@ -107,7 +105,7 @@
   GlobalState->Records.push_back(AR);
   GlobalState->Indices.push_back(Builder.writeLeafType(AR));
 
-  CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+  CVType Type(Builder.records().back());
   GlobalState->TypeVector.push_back(Type);
 
   GlobalState->AllOffsets.push_back(
@@ -369,11 +367,10 @@
   TypeIndex IndexOne = Builder.writeLeafType(Modifier);
 
   // set up a type stream that refers to the above two serialized records.
-  std::vector TypeArray;
-  TypeArray.push_back(
-  CVType(static_cast(Class.Kind), Builder.records()[0]));
-  TypeArray.push_back(
-  CVType(static_cast(Modifier.Kind), Builder.records()[1]));
+  std::vector TypeArray = {
+  {Builder.records()[0]},
+  {Builder.records()[1]},
+  };
   BinaryItemStream ItemStream(llvm::support::little);
   ItemStream.setItems(TypeArray);
   VarStreamArray TypeStream(ItemStream);
Index: llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -224,7 +224,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}",
fmt_align(Index, AlignStyle::Right, Width),
-   formatTypeLeafKind(Record.Type), Record.length());
+   formatTypeLeafKind(Record.kind()), Record.length());
   if (Hashes) {
 std::string H;
 if (Index.toArrayIndex() >= HashValues.size()) {
Index: llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
@@ -331,7 +331,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}]",
fmt_align(Offset, AlignStyle::Right, 6),
-   formatSymbolKind(Record.Type), Record.length());
+   formatSymbolKind(Record.kind()), Record.length());
   P.Indent();
   return Error::success();
 }
Index: llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -98,7 +98,7 @@
 
   CVType toCodeViewRecord(AppendingTypeTableBuilder &TS) const override {
 TS.writeLeafType(Record);
-return CVType(Kind, TS.records().back());
+return CVType(TS.records().back());
   }
 
   mutable T Record;
@@ -496,7 +496,7 @@
 Member.Member->writeTo(CRB);
   }
   TS.insertRecord(CRB);
-  return CVType(Kind, TS.records().back());
+  return CVType(TS.records().back());
 }
 
 void MappingTraits::mapping(IO &io, OneMethodRecord &Record) {
Index: llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
@@ -248,7 +248,7 @@
 uint8_t *Buffer = Allocator.Allocate(TotalLen);
 ::memcpy(Buffer, 

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Patch breaks the build with the shared libraries, for example, 
http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/9498. 
Seems to me, you need to add an extra dependency on clangTooling. 
clangToolingCore is not enough.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59376



___
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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Note that with:

1. https://reviews.llvm.org/D60208 ("[X86] Extend boolean arguments to

inline-asm according to getBooleanType")

2. https://reviews.llvm.org/D58260 ("[INLINER] allow inlining of

blockaddresses if sole uses are callbrs")

3. https://reviews.llvm.org/D56571 ("[RFC prototype] Implementation of

asm-goto support in clang")

I can compile a mainline x86 defconfig Linux kernel and boot it in QEMU.


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

Re: [PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via cfe-commits]

Thanks.  Do you have build command I can run before/after to verify my fix
before I submit?

On Wed, Apr 3, 2019 at 12:43 PM Alexey Bataev via Phabricator <
revi...@reviews.llvm.org> wrote:

> ABataev added a comment.
>
> Patch breaks the build with the shared libraries, for example,
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/9498.
> Seems to me, you need to add an extra dependency on clangTooling.
> clangToolingCore is not enough.
>
>
> Repository:
>   rC Clang
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D59376/new/
>
> https://reviews.llvm.org/D59376
>
>
>
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r357606 - [libunwind] Do not share an object library to create the static/shared libraries

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Apr  3 09:59:33 2019
New Revision: 357606

URL: http://llvm.org/viewvc/llvm-project?rev=357606&view=rev
Log:
[libunwind] Do not share an object library to create the static/shared libraries

This change is similar to r356150, with the same motivation.

Modified:
libunwind/trunk/src/CMakeLists.txt

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=357606&r1=357605&r2=357606&view=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Wed Apr  3 09:59:33 2019
@@ -105,50 +105,16 @@ set_property(SOURCE ${LIBUNWIND_CXX_SOUR
 set_property(SOURCE ${LIBUNWIND_C_SOURCES}
  APPEND_STRING PROPERTY COMPILE_FLAGS " ${LIBUNWIND_C_FLAGS}")
 
-macro(unwind_object_library name)
-  cmake_parse_arguments(ARGS "" "" "DEFINES;FLAGS" ${ARGN})
-
-# Add a object library that contains the compiled source files.
-  add_library(${name} OBJECT ${LIBUNWIND_SOURCES} ${LIBUNWIND_HEADERS})
-
-  if(ARGS_DEFINES)
-target_compile_definitions(${name} PRIVATE ${ARGS_DEFINES})
-  endif()
-
-  set_target_properties(${name}
-PROPERTIES
-  COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}"
-  POSITION_INDEPENDENT_CODE ON)
-
-  if(ARGS_FLAGS)
-target_compile_options(${name} PRIVATE ${ARGS_FLAGS})
-  endif()
-endmacro()
-
-if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
-  append_flags_if_supported(UNWIND_STATIC_OBJECTS_FLAGS -fvisibility=hidden)
-  append_flags_if_supported(UNWIND_STATIC_OBJECTS_FLAGS 
-fvisibility-global-new-delete-hidden)
-  unwind_object_library(unwind_static_objects
-DEFINES _LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS
-FLAGS ${UNWIND_STATIC_OBJECTS_FLAGS})
-  unwind_object_library(unwind_shared_objects)
-  set(unwind_static_sources $)
-  set(unwind_shared_sources $)
-else()
-  unwind_object_library(unwind_objects)
-  set(unwind_static_sources $)
-  set(unwind_shared_sources $)
-endif()
-
 # Build the shared library.
 if (LIBUNWIND_ENABLE_SHARED)
-  add_library(unwind_shared SHARED ${unwind_shared_sources})
+  add_library(unwind_shared SHARED ${LIBUNWIND_SOURCES} ${LIBUNWIND_HEADERS})
   if(COMMAND llvm_setup_rpath)
 llvm_setup_rpath(unwind_shared)
   endif()
   target_link_libraries(unwind_shared PRIVATE ${libraries})
   set_target_properties(unwind_shared
 PROPERTIES
+  COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}"
   LINK_FLAGS"${LIBUNWIND_LINK_FLAGS}"
   OUTPUT_NAME   "unwind"
   VERSION   "1.0"
@@ -161,12 +127,21 @@ endif()
 
 # Build the static library.
 if (LIBUNWIND_ENABLE_STATIC)
-  add_library(unwind_static STATIC ${unwind_static_sources})
+  add_library(unwind_static STATIC ${LIBUNWIND_SOURCES} ${LIBUNWIND_HEADERS})
   target_link_libraries(unwind_static PRIVATE ${libraries})
   set_target_properties(unwind_static
 PROPERTIES
+  COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}"
   LINK_FLAGS"${LIBUNWIND_LINK_FLAGS}"
   OUTPUT_NAME   "unwind")
+
+  if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
+append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
+append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS 
-fvisibility-global-new-delete-hidden)
+target_compile_options(unwind_static PRIVATE 
${UNWIND_STATIC_LIBRARY_FLAGS})
+target_compile_definitions(unwind_static PRIVATE 
_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+  endif()
+
   list(APPEND LIBUNWIND_BUILD_TARGETS "unwind_static")
   if (LIBUNWIND_INSTALL_STATIC_LIBRARY)
 list(APPEND LIBUNWIND_INSTALL_TARGETS "unwind_static")


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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

You need to provide -DBUILD_SHARED_LIBS=true to cmake. Unfortunately, there is 
a dependency from the libToolingRefactor to libTooling. I don't think it will 
be easy to fix this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59376



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

[PATCH] D60213: Add clangTooling dependency to fix the build.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Fixes https://reviews.llvm.org/D59371, which broke the build for shared 
libraries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60213

Files:
  clang/lib/Tooling/Refactoring/CMakeLists.txt


Index: clang/lib/Tooling/Refactoring/CMakeLists.txt
===
--- clang/lib/Tooling/Refactoring/CMakeLists.txt
+++ clang/lib/Tooling/Refactoring/CMakeLists.txt
@@ -23,4 +23,5 @@
   clangLex
   clangRewrite
   clangToolingCore
+  clangTooling
   )


Index: clang/lib/Tooling/Refactoring/CMakeLists.txt
===
--- clang/lib/Tooling/Refactoring/CMakeLists.txt
+++ clang/lib/Tooling/Refactoring/CMakeLists.txt
@@ -23,4 +23,5 @@
   clangLex
   clangRewrite
   clangToolingCore
+  clangTooling
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via cfe-commits]

https://reviews.llvm.org/D60213 for the record

On Wed, Apr 3, 2019 at 12:55 PM Yitzhak Mandelbaum 
wrote:

> Thanks.  Do you have build command I can run before/after to verify my fix
> before I submit?
>
> On Wed, Apr 3, 2019 at 12:43 PM Alexey Bataev via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> ABataev added a comment.
>>
>> Patch breaks the build with the shared libraries, for example,
>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/9498.
>> Seems to me, you need to add an extra dependency on clangTooling.
>> clangToolingCore is not enough.
>>
>>
>> Repository:
>>   rC Clang
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D59376/new/
>>
>> https://reviews.llvm.org/D59376
>>
>>
>>
>>


smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60213: Add clangTooling dependency to fix the build.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Nope, it won't work. There is a cyclic dependency, unfortunately. clangTooling 
depends on clangToolingRefactor already. You need to revert your commit and 
refactor it to remove the cyclic dependency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60213



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

This looks like a more promising direction. Thanks for the readiness to 
experiment with this.

See the comments inline.




Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:130
+
+  // If we have alternative fixes (attached via diagnostic::Notes), we just
+  // choose the first one to apply.

Could you leave a FIXME here to explore options around interactive fix 
selection?



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:132
+  // choose the first one to apply.
+  const llvm::StringMap *ErrorFix = nullptr;
+  if (!Error.Message.Fix.empty())

`ErrorFix` brings more questions than it answers. Maybe `SelectedFix`, 
`ChosenFix`, or just `Fix`?



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:133
+  const llvm::StringMap *ErrorFix = nullptr;
+  if (!Error.Message.Fix.empty())
+ErrorFix = &Error.Message.Fix;

nit: I'd add braces here, since the `else` branch has them.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:144
+
+  if (ApplyFixes && ErrorFix) {
+for (const auto &FileAndReplacements : *ErrorFix) {

The nesting level starts getting out of control here. I'd try to pull the loop 
into a function.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:187
   }
+  reportFix(Diag, Error.Message.Fix);
 }

Why `Error.Message.Fix`? Should this use `*SelectedFix` instead?



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:262
+  for (const auto &Repl : FileAndReplacements.second) {
+if (Repl.isApplicable()) {
+  SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();

nit: Early "continue" would make sense here.

  if (!Repl.isApplicable())
continue;
  ...



Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:46
+
+  /// Fixes for this diagnostic.
+  llvm::StringMap Fix;

Some information from the original comment was lost here:

  /// Fixes to apply, grouped by file path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

r357610 - Bug-40323: MS ABI adding template static member in the linker directive section to make sure init function can be called before main.

[Jennifer Yu via cfe-commits]

Author: jyu2
Date: Wed Apr  3 10:21:40 2019
New Revision: 357610

URL: http://llvm.org/viewvc/llvm-project?rev=357610&view=rev
Log:
Bug-40323: MS ABI adding template static member in the linker directive section 
to make sure init function can be called before main.

Added:
cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=357610&r1=357609&r2=357610&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Wed Apr  3 10:21:40 2019
@@ -481,6 +481,12 @@ CodeGenModule::EmitCXXGlobalVarDeclInitF
 // minor startup time optimization.  In the MS C++ ABI, there are no guard
 // variables, so this COMDAT key is required for correctness.
 AddGlobalCtor(Fn, 65535, COMDATKey);
+if (getTarget().getCXXABI().isMicrosoft()) {
+  // In The MS C++, MS add template static data member in the linker
+  // drective.
+  assert(COMDATKey);
+  addUsedGlobal(COMDATKey);
+}
   } else if (D->hasAttr()) {
 // SelectAny globals will be comdat-folded. Put the initializer into a
 // COMDAT group associated with the global, so the initializers get folded

Added: cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp?rev=357610&view=auto
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp Wed Apr  3 
10:21:40 2019
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 %s -triple=i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-pc-win32 -fms-extensions -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple=i686-pc-windows-msvc -fms-extensions -emit-llvm 
-o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-pc-windows-msvc -fms-extensions 
-emit-llvm -o - | FileCheck %s
+
+struct S {
+  S();
+  ~S();
+};
+
+template  struct __declspec(dllexport) ExportedTemplate {
+  static S s;
+};
+template  S ExportedTemplate::s;
+void useExportedTemplate(ExportedTemplate x) {
+  (void)x.s;
+}
+int f();
+namespace selectany_init {
+// MS don't put selectany static var in the linker directive, init routine
+// f() is not getting called if x is not referenced.
+int __declspec(selectany) x = f();
+}
+
+namespace explicit_template_instantiation {
+template  struct A { static  int x; };
+template  int A::x = f();
+template struct A;
+}
+
+namespace implicit_template_instantiation {
+template  struct A { static  int x; };
+template   int A::x = f();
+int g() { return A::x; }
+}
+
+
+template 
+struct X_ {
+  static T ioo;
+  static T init();
+};
+template  T X_::ioo = X_::init();
+template struct X_;
+
+template 
+struct X {
+  static T ioo;
+  static T init();
+};
+// template specialized static data don't need in llvm.used,
+// the static init routine get call from _GLOBAL__sub_I_ routines.
+template <> int X::ioo = X::init();
+template struct X;
+// CHECK: @llvm.global_ctors = appending global [6 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* @"??__Ex@selectany_init@@YAXXZ", 
i8* bitcast (i32* @"?x@selectany_init@@3HA" to i8*) }, { i32, void ()*, i8* } { 
i32 65535, void ()* 
@"??__E?x@?$A@H@explicit_template_instantiation@@2HA@@YAXXZ", i8* bitcast (i32* 
@"?x@?$A@H@explicit_template_instantiation@@2HA" to i8*) }, { i32, void ()*, 
i8* } { i32 65535, void ()* @"??__E?ioo@?$X_@H@@2HA@@YAXXZ", i8* bitcast (i32* 
@"?ioo@?$X_@H@@2HA" to i8*) }, { i32, void ()*, i8* } { i32 65535, void ()* 
@"??__E?s@?$ExportedTemplate@H@@2US@@A@@YAXXZ", i8* getelementptr inbounds 
(%struct.S, %struct.S* @"?s@?$ExportedTemplate@H@@2US@@A", i32 0, i32 0) }, { 
i32, void ()*, i8* } { i32 65535, void ()* 
@"??__E?x@?$A@H@implicit_template_instantiation@@2HA@@YAXXZ", i8* bitcast (i32* 
@"?x@?$A@H@implicit_template_instantiation@@2HA" to i8*) }, { i32, void ()*, 
i8* } { i32 65535, void ()* 
@_GLOBAL__sub_I_microsoft_abi_template_static_init.cpp, i8* null }]
+// CHECK: @llvm.used = appending global [4 x i8*] [i8* bitcast (i32* 
@"?x@?$A@H@explicit_template_instantiation@@2HA" to i8*), i8* bitcast (i32* 
@"?ioo@?$X_@H@@2HA" to i8*), i8* getelementptr inbounds (%struct.S, %struct.S* 
@"?s@?$ExportedTemplate@H@@2US@@A", i32 0, i32 0), i8* bitcast (i32* 
@"?x@?$A@H@implicit_template_instantiation@@2HA" to i8*)], section 
"llvm.metadata"
+


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

[PATCH] D60213: Add clangTooling dependency to fix the build.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D60213#1453516 , @ABataev wrote:

> Nope, it won't work. There is a cyclic dependency, unfortunately. 
> clangTooling depends on clangToolingRefactor already. You need to revert your 
> commit and refactor it to remove the cyclic dependency.


I see. I've never reverted before and I can't seem to find any instructions for 
doing so (only some offhand mentions). If there's a doc on this, can you please 
link me to it?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60213



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

[PATCH] D60213: Add clangTooling dependency to fix the build.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I can revert it for you, if you want


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60213



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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

hintonda wrote:
> aaron.ballman wrote:
> > hintonda wrote:
> > > aaron.ballman wrote:
> > > > I do not like that we're removing the namespace qualifier here. I would 
> > > > prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` if there 
> > > > is a namespace clash.
> > > Other than aesthetics, the reason I don't like the idea of fully scoping 
> > > these types, at least without a comment, is that the error is triggered 
> > > by some other code gets included first, and has nothing to do with this 
> > > code -- there's nothing actually wrong with the original code.  So it 
> > > could/would be confusing for a reader later on wondering why you needed 
> > > to fully scope these types, and not others.
> > I would argue that the original code is wrong to not use fully-qualified 
> > namespace specifiers. The issue is that we have two different namespaces 
> > named `llvm` and have gotten away with poor namespace hygiene by accident. 
> > Either we should rename the clang-tidy `llvm` namespace to something that 
> > does not conflict, or we should consistently use fully-qualified namespace 
> > specifiers when in clang-tidy and needing to refer to an `llvm` namespace 
> > explicitly.
> > 
> > I think this patch goes in the wrong direction by making it easier to limp 
> > along with poor namespace hygiene.
> By fully qualified, do you mean appending the global namespace, `::` to 
> everything?   I actually like using `llvm::`, but `::llvm::` is odd and needs 
> explanation.
> 
> I'd be happy to abandon this change and instead rename the 
> `clang::tidy::llvm` to `clang::tidy::something_else`, if that's what the 
> community would prefer.
> 
Aaron, you have  a very good point. We also have a more recent example of a 
good namespace hygiene in clang-tidy code: the `abseil` module is not called 
`absl` mainly to "avoid collisions with a well-known top-level namespace" 
(https://google.github.io/styleguide/cppguide.html#Namespace_Names).

If we can rename the llvm module to something reasonable ("llvm_project"?) 
without breaking the naming invariants (used by the add_new_check.py script, 
for example), it would be a much better solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

Re: [PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via cfe-commits]

I'll revert the change.

On Wed, Apr 3, 2019 at 1:00 PM Yitzhak Mandelbaum 
wrote:

> https://reviews.llvm.org/D60213 for the record
>
> On Wed, Apr 3, 2019 at 12:55 PM Yitzhak Mandelbaum 
> wrote:
>
>> Thanks.  Do you have build command I can run before/after to verify my
>> fix before I submit?
>>
>> On Wed, Apr 3, 2019 at 12:43 PM Alexey Bataev via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> ABataev added a comment.
>>>
>>> Patch breaks the build with the shared libraries, for example,
>>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/9498.
>>> Seems to me, you need to add an extra dependency on clangTooling.
>>> clangToolingCore is not enough.
>>>
>>>
>>> Repository:
>>>   rC Clang
>>>
>>> CHANGES SINCE LAST ACTION
>>>   https://reviews.llvm.org/D59376/new/
>>>
>>> https://reviews.llvm.org/D59376
>>>
>>>
>>>
>>>


smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357612 - Revert "[LibTooling] Add Transformer, a library for source-to-source transformations."

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Apr  3 10:34:04 2019
New Revision: 357612

URL: http://llvm.org/viewvc/llvm-project?rev=357612&view=rev
Log:
Revert "[LibTooling] Add Transformer, a library for source-to-source 
transformations."

This reverts commit r357576 to fix the problem with the cyclic
dependencies between libTooling and libToolingRefactor.

Removed:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp
Modified:
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Removed: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=357611&view=auto
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (removed)
@@ -1,210 +0,0 @@
-//===--- Transformer.h - Clang source-rewriting library -*- 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
-//
-//===--===//
-///
-///  \file
-///  Defines a library supporting the concise specification of clang-based
-///  source-to-source transformations.
-///
-//===--===//
-
-#ifndef LLVM_CLANG_TOOLING_REFACTOR_TRANSFORMER_H_
-#define LLVM_CLANG_TOOLING_REFACTOR_TRANSFORMER_H_
-
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/ASTMatchers/ASTMatchersInternal.h"
-#include "clang/Tooling/Refactoring/AtomicChange.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/Support/Error.h"
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-namespace clang {
-namespace tooling {
-/// Determines the part of the AST node to replace.  We support this to work
-/// around the fact that the AST does not differentiate various syntactic
-/// elements into their own nodes, so users can specify them relative to a 
node,
-/// instead.
-enum class NodePart {
-  /// The node itself.
-  Node,
-  /// Given a \c MemberExpr, selects the member's token.
-  Member,
-  /// Given a \c NamedDecl or \c CxxCtorInitializer, selects that token of the
-  /// relevant name, not including qualifiers.
-  Name,
-};
-
-using TextGenerator =
-std::function;
-
-/// Description of a source-code transformation.
-//
-// A *rewrite rule* describes a transformation of source code. It has the
-// following components:
-//
-// * Matcher: the pattern term, expressed as clang matchers (with Transformer
-//   extensions).
-//
-// * Target: the source code impacted by the rule. This identifies an AST node,
-//   or part thereof (\c TargetPart), whose source range indicates the extent 
of
-//   the replacement applied by the replacement term.  By default, the extent 
is
-//   the node matched by the pattern term (\c NodePart::Node). Target's are
-//   typed (\c TargetKind), which guides the determination of the node extent
-//   and might, in the future, statically constrain the set of eligible
-//   NodeParts for a given node.
-//
-// * Replacement: a function that produces a replacement string for the target,
-//   based on the match result.
-//
-// * Explanation: explanation of the rewrite.  This will be displayed to the
-//   user, where possible (for example, in clang-tidy fix descriptions).
-//
-// Rules have an additional, implicit, component: the parameters. These are
-// portions of the pattern which are left unspecified, yet named so that we can
-// reference them in the replacement term.  The structure of parameters can be
-// partially or even fully specified, in which case they serve just to identify
-// matched nodes for later reference rather than abstract over portions of the
-// AST.  However, in all cases, we refer to named portions of the pattern as
-// parameters.
-//
-// RewriteRule is constructed in a "fluent" style, by creating a builder and
-// chaining setters of individual components.
-// \code
-//   RewriteRule MyRule = buildRule(functionDecl(...)).replaceWith(...);
-// \endcode
-//
-// The \c Transformer class should then be used to apply the rewrite rule and
-// obtain the corresponding replacements.
-struct RewriteRule {
-  // `Matcher` describes the context of this rule. It should always be bound to
-  // at least `RootId`.  The builder class below takes care of this
-  // binding. Here, we bind it to a trivial Matcher to enable the default
-  // constructor, since DynTypedMatcher has no default constructor.
-  ast_matchers::internal::DynTypedMatcher Matcher = ast_matchers::stmt();
-  // The (bo

Re: [PATCH] D60213: Add clangTooling dependency to fix the build.

[Yitzhak Mandelbaum via cfe-commits]

That would be great, thank you.

On Wed, Apr 3, 2019 at 1:28 PM Alexey Bataev via Phabricator <
revi...@reviews.llvm.org> wrote:

> ABataev added a comment.
>
> I can revert it for you, if you want
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D60213/new/
>
> https://reviews.llvm.org/D60213
>
>
>
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60213: Add clangTooling dependency to fix the build.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Reverted in r357612


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60213



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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

alexfh wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > hintonda wrote:
> > > > aaron.ballman wrote:
> > > > > I do not like that we're removing the namespace qualifier here. I 
> > > > > would prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` 
> > > > > if there is a namespace clash.
> > > > Other than aesthetics, the reason I don't like the idea of fully 
> > > > scoping these types, at least without a comment, is that the error is 
> > > > triggered by some other code gets included first, and has nothing to do 
> > > > with this code -- there's nothing actually wrong with the original 
> > > > code.  So it could/would be confusing for a reader later on wondering 
> > > > why you needed to fully scope these types, and not others.
> > > I would argue that the original code is wrong to not use fully-qualified 
> > > namespace specifiers. The issue is that we have two different namespaces 
> > > named `llvm` and have gotten away with poor namespace hygiene by 
> > > accident. Either we should rename the clang-tidy `llvm` namespace to 
> > > something that does not conflict, or we should consistently use 
> > > fully-qualified namespace specifiers when in clang-tidy and needing to 
> > > refer to an `llvm` namespace explicitly.
> > > 
> > > I think this patch goes in the wrong direction by making it easier to 
> > > limp along with poor namespace hygiene.
> > By fully qualified, do you mean appending the global namespace, `::` to 
> > everything?   I actually like using `llvm::`, but `::llvm::` is odd and 
> > needs explanation.
> > 
> > I'd be happy to abandon this change and instead rename the 
> > `clang::tidy::llvm` to `clang::tidy::something_else`, if that's what the 
> > community would prefer.
> > 
> Aaron, you have  a very good point. We also have a more recent example of a 
> good namespace hygiene in clang-tidy code: the `abseil` module is not called 
> `absl` mainly to "avoid collisions with a well-known top-level namespace" 
> (https://google.github.io/styleguide/cppguide.html#Namespace_Names).
> 
> If we can rename the llvm module to something reasonable ("llvm_project"?) 
> without breaking the naming invariants (used by the add_new_check.py script, 
> for example), it would be a much better solution.
> By fully qualified, do you mean appending the global namespace, :: to 
> everything? I actually like using llvm::, but ::llvm:: is odd and needs 
> explanation.

I mean that within clang-tidy, anywhere we write `llvm::` today, we write 
`::llvm::` instead when we're talking about the global `llvm` namespace as 
opposed to the clang-tidy `llvm` namespace.

> I'd be happy to abandon this change and instead rename the clang::tidy::llvm 
> to clang::tidy::something_else, if that's what the community would prefer.

That's my personal preference. I'm fine with the suggestion from @alexfh of 
using `llvm_project` instead, but we could also go with `llvm_proj`, 
`llvm_code`, `llvm_tidy`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 

aaron.ballman wrote:
> alexfh wrote:
> > hintonda wrote:
> > > aaron.ballman wrote:
> > > > hintonda wrote:
> > > > > aaron.ballman wrote:
> > > > > > I do not like that we're removing the namespace qualifier here. I 
> > > > > > would prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 
> > > > > > 5>` if there is a namespace clash.
> > > > > Other than aesthetics, the reason I don't like the idea of fully 
> > > > > scoping these types, at least without a comment, is that the error is 
> > > > > triggered by some other code gets included first, and has nothing to 
> > > > > do with this code -- there's nothing actually wrong with the original 
> > > > > code.  So it could/would be confusing for a reader later on wondering 
> > > > > why you needed to fully scope these types, and not others.
> > > > I would argue that the original code is wrong to not use 
> > > > fully-qualified namespace specifiers. The issue is that we have two 
> > > > different namespaces named `llvm` and have gotten away with poor 
> > > > namespace hygiene by accident. Either we should rename the clang-tidy 
> > > > `llvm` namespace to something that does not conflict, or we should 
> > > > consistently use fully-qualified namespace specifiers when in 
> > > > clang-tidy and needing to refer to an `llvm` namespace explicitly.
> > > > 
> > > > I think this patch goes in the wrong direction by making it easier to 
> > > > limp along with poor namespace hygiene.
> > > By fully qualified, do you mean appending the global namespace, `::` to 
> > > everything?   I actually like using `llvm::`, but `::llvm::` is odd and 
> > > needs explanation.
> > > 
> > > I'd be happy to abandon this change and instead rename the 
> > > `clang::tidy::llvm` to `clang::tidy::something_else`, if that's what the 
> > > community would prefer.
> > > 
> > Aaron, you have  a very good point. We also have a more recent example of a 
> > good namespace hygiene in clang-tidy code: the `abseil` module is not 
> > called `absl` mainly to "avoid collisions with a well-known top-level 
> > namespace" 
> > (https://google.github.io/styleguide/cppguide.html#Namespace_Names).
> > 
> > If we can rename the llvm module to something reasonable ("llvm_project"?) 
> > without breaking the naming invariants (used by the add_new_check.py 
> > script, for example), it would be a much better solution.
> > By fully qualified, do you mean appending the global namespace, :: to 
> > everything? I actually like using llvm::, but ::llvm:: is odd and needs 
> > explanation.
> 
> I mean that within clang-tidy, anywhere we write `llvm::` today, we write 
> `::llvm::` instead when we're talking about the global `llvm` namespace as 
> opposed to the clang-tidy `llvm` namespace.
> 
> > I'd be happy to abandon this change and instead rename the 
> > clang::tidy::llvm to clang::tidy::something_else, if that's what the 
> > community would prefer.
> 
> That's my personal preference. I'm fine with the suggestion from @alexfh of 
> using `llvm_project` instead, but we could also go with `llvm_proj`, 
> `llvm_code`, `llvm_tidy`, etc.
Looks like a consensus.  I'll work up a patch and get back to you.  Thanks 
again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D58160: MS ABI: adding template static member in the linker directive section to make sure init function can be called before main.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 closed this revision.
jyu2 added a comment.

Closed by commit rG0b28b8b09be7 
: 
Bug-40323: MS ABI adding template static member in the linker directive 
section… (authored by jyu2).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58160



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

r357617 - [OPENMP]Add codegen for firstprivate vars with allocate clause.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Apr  3 10:57:06 2019
New Revision: 357617

URL: http://llvm.org/viewvc/llvm-project?rev=357617&view=rev
Log:
[OPENMP]Add codegen for firstprivate vars with allocate clause.

Added codegen/test for the firstprivatized variables with the allocate
clause.

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/parallel_firstprivate_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=357617&r1=357616&r2=357617&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Apr  3 10:57:06 2019
@@ -1457,7 +1457,13 @@ CodeGenFunction::EmitAutoVarAlloca(const
 
   Address address = Address::invalid();
   Address AllocaAddr = Address::invalid();
-  if (Ty->isConstantSizeType()) {
+  Address OpenMPLocalAddr =
+  getLangOpts().OpenMP
+  ? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
+  : Address::invalid();
+  if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
+address = OpenMPLocalAddr;
+  } else if (Ty->isConstantSizeType()) {
 bool NRVO = getLangOpts().ElideConstructors &&
   D.isNRVOVariable();
 
@@ -1500,14 +1506,7 @@ CodeGenFunction::EmitAutoVarAlloca(const
 // unless:
 // - it's an NRVO variable.
 // - we are compiling OpenMP and it's an OpenMP local variable.
-
-Address OpenMPLocalAddr =
-getLangOpts().OpenMP
-? CGM.getOpenMPRuntime().getAddressOfLocalVariable(*this, &D)
-: Address::invalid();
-if (getLangOpts().OpenMP && OpenMPLocalAddr.isValid()) {
-  address = OpenMPLocalAddr;
-} else if (NRVO) {
+if (NRVO) {
   // The named return value optimization: allocate this variable in the
   // return slot, so that we can elide the copy when returning this
   // variable (C++0x [class.copy]p34).

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=357617&r1=357616&r2=357617&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Apr  3 10:57:06 2019
@@ -9768,10 +9768,13 @@ Address CGOpenMPRuntime::getAddressOfLoc
   CharUnits Align = CGM.getContext().getDeclAlign(CVD);
   if (CVD->getType()->isVariablyModifiedType()) {
 Size = CGF.getTypeSize(CVD->getType());
-Align = CGM.getContext().getTypeAlignInChars(CVD->getType());
+// Align the size: ((size + align - 1) / align) * align
+Size = CGF.Builder.CreateNUWAdd(
+Size, CGM.getSize(Align - CharUnits::fromQuantity(1)));
+Size = CGF.Builder.CreateUDiv(Size, CGM.getSize(Align));
+Size = CGF.Builder.CreateNUWMul(Size, CGM.getSize(Align));
   } else {
 CharUnits Sz = CGM.getContext().getTypeSizeInChars(CVD->getType());
-Align = CGM.getContext().getDeclAlign(CVD);
 Size = CGM.getSize(Sz.alignTo(Align));
   }
   llvm::Value *ThreadID = getThreadID(CGF, CVD->getBeginLoc());

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=357617&r1=357616&r2=357617&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Wed Apr  3 10:57:06 2019
@@ -750,8 +750,10 @@ bool CodeGenFunction::EmitOMPFirstprivat
   bool ThisFirstprivateIsLastprivate =
   Lastprivates.count(OrigVD->getCanonicalDecl()) > 0;
   const FieldDecl *FD = CapturedStmtInfo->lookup(OrigVD);
+  const auto *VD = cast(cast(IInit)->getDecl());
   if (!MustEmitFirstprivateCopy && !ThisFirstprivateIsLastprivate && FD &&
-  !FD->getType()->isReferenceType()) {
+  !FD->getType()->isReferenceType() &&
+  (!VD || !VD->hasAttr())) {
 EmittedAsFirstprivate.insert(OrigVD->getCanonicalDecl());
 ++IRef;
 ++InitsRef;
@@ -760,7 +762,8 @@ bool CodeGenFunction::EmitOMPFirstprivat
   // Do not emit copy for firstprivate constant variables in target 
regions,
   // captured by reference.
   if (DeviceConstTarget && OrigVD->getType().isConstant(getContext()) &&
-  FD && FD->getType()->isReferenceType()) {
+  FD && FD->getType()->isReferenceType() &&
+  (!VD || !VD->hasAttr())) {
 (void)CGM.getOpenMPRuntime().registerTargetFirstprivateCopy(*this,
 OrigVD);
 ++IRef;
@@ -770,7 +773,6 @@ bool CodeGenFunction::EmitOMPFirstprivat
   FirstprivateIsLastprivate =
   FirstprivateIsLastprivate || ThisFirstprivateIsLastprivate;
 

[PATCH] D60220: [CUDA][Windows] Final fix for bug 38811 (Step 3 of 3)

[Evgeny Mankov via Phabricator via cfe-commits]

emankov created this revision.
emankov added a reviewer: tra.
emankov added a project: clang.
Herald added a subscriber: cfe-commits.

Last fix for the clang Bug 38811  
"Clang fails to compile with CUDA-9.x on Windows".

**[IMPORTANT]**
With that last fix, CUDA has just started being compiling by clang on Windows 
after nearly a year and two clang’s major releases (7 and 8).
As long as the last LLVM release, in which clang was compiling CUDA on Windows 
successfully, was 6.0.1, this fix and two previous have to be included into 
upcoming 7.1.0 and 8.0.1 releases.

[How to repro]

  clang++.exe -x cuda "c:\ProgramData\NVIDIA Corporation\CUDA 
Samples\v9.0\0_Simple\simplePrintf\simplePrintf.cu" -I"c:\ProgramData\NVIDIA 
Corporation\CUDA Samples\v9.0\common\inc" --cuda-gpu-arch=sm_50 
--cuda-path="C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v9.0" 
-L"c:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v9.0\lib\x64" 
-lcudart.lib  -v

[Output]

  In file included from 
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.5\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:327:
  C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:390:11: error: no matching 
function for call to '__isinfl'
return (__isinfl(a) != 0);
^~~~
  C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:2662:14: note: candidate 
function not viable: call to __host__ function from __device__ function
  __func__(int __isinfl(long double a))
   ^
  In file included from :1:
  In file included from 
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.5\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:327:
  C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:438:11: error: no matching 
function for call to '__isnanl'
return (__isnanl(a) != 0);
^~~~
  C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:2672:14: note: candidate 
function not viable: call to __host__ function from __device__ function
  __func__(int __isnanl(long double a))
   ^
  In file included from :1:
  In file included from 
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.5\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:327:
  C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:486:11: error: no matching 
function for call to '__finitel'
return (__finitel(a) != 0);
^
  C:\Program Files\NVIDIA GPU Computing 
Toolkit\CUDA\v9.0/include\crt/math_functions.hpp:2652:14: note: candidate 
function not viable: call to __host__ function from __device__ function
  __func__(int __finitel(long double a))
   ^
  3 errors generated when compiling for sm_50.

[Solution]
Add missing device functions' declarations and definitions.


Repository:
  rC Clang

https://reviews.llvm.org/D60220

Files:
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h


Index: clang/lib/Headers/__clang_cuda_math_forward_declares.h
===
--- clang/lib/Headers/__clang_cuda_math_forward_declares.h
+++ clang/lib/Headers/__clang_cuda_math_forward_declares.h
@@ -98,12 +98,14 @@
 __DEVICE__ float hypot(float, float);
 __DEVICE__ int ilogb(double);
 __DEVICE__ int ilogb(float);
+__DEVICE__ bool isfinite(long double);
 __DEVICE__ bool isfinite(double);
 __DEVICE__ bool isfinite(float);
 __DEVICE__ bool isgreater(double, double);
 __DEVICE__ bool isgreaterequal(double, double);
 __DEVICE__ bool isgreaterequal(float, float);
 __DEVICE__ bool isgreater(float, float);
+__DEVICE__ bool isinf(long double);
 __DEVICE__ bool isinf(double);
 __DEVICE__ bool isinf(float);
 __DEVICE__ bool isless(double, double);
@@ -112,6 +114,7 @@
 __DEVICE__ bool isless(float, float);
 __DEVICE__ bool islessgreater(double, double);
 __DEVICE__ bool islessgreater(float, float);
+__DEVICE__ bool isnan(long double);
 __DEVICE__ bool isnan(double);
 __DEVICE__ bool isnan(float);
 __DEVICE__ bool isnormal(double);
Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -237,6 +237,7 @@
 __DEVICE__ int __ffsll(long long __a) { return __nv_ffsll(__a); }
 __DEVICE__ int __finite(double __a) { return __nv_isfinited(__a); }
 __DEVICE__ int __finitef(float __a) { return __nv_finitef(__a); }
+__DEVICE__ int __finitel(long double __a) { return __finite((double)__a); }
 __DEVICE__ int __float2int_rd(float __a) { return __nv_float2int_rd(__a); }
 __DEVICE__ int __float2int_rn(float __a) { return __nv_float2int_rn(__a); }
 __DEVICE__ int __float2int_r