date:20201204

[PATCH] D92573: [clang] Add a C++17 deduction guide testcase.

2020-12-04 Thread Haojian Wu via Phabricator via cfe-commits

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

address comment, simplify the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92573

Files:
  clang/test/PCH/cxx17-deduction-guide-decl.cpp


Index: clang/test/PCH/cxx17-deduction-guide-decl.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx17-deduction-guide-decl.cpp
@@ -0,0 +1,24 @@
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -std=c++17  -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -std=c++17 -o - %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace RP47219 {
+typedef int MyInt;
+template 
+class Some {
+ public:
+  explicit Some(T, MyInt) {}
+};
+
+struct Foo {};
+void ParseNatural() {
+  Some(Foo(), 1);
+}
+}
+
+#else
+
+#endif


Index: clang/test/PCH/cxx17-deduction-guide-decl.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx17-deduction-guide-decl.cpp
@@ -0,0 +1,24 @@
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -std=c++17  -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -std=c++17 -o - %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace RP47219 {
+typedef int MyInt;
+template 
+class Some {
+ public:
+  explicit Some(T, MyInt) {}
+};
+
+struct Foo {};
+void ParseNatural() {
+  Some(Foo(), 1);
+}
+}
+
+#else
+
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92573: [clang] Add a C++17 deduction guide testcase.

2020-12-04 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang/test/PCH/cxx17-deduction-guide-decl.cpp:16
+
+// Class template argument deduction
+template 

adamcz wrote:
> You can drop these 3 lines, they're not needed for the crash repro. Also, 
> c-tor doesn't have to be explicit. Probably best to keep the code as short as 
> possible.
yeah, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92573

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

[clang] 5b9fc44 - [clang] Add a C++17 deduction guide testcase.

2020-12-04 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-12-04T09:02:50+01:00
New Revision: 5b9fc44d8128717ef2f219b061c018abb85c717f

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

LOG: [clang] Add a C++17 deduction guide testcase.

>From https://bugs.llvm.org/show_bug.cgi?id=47219.

It was crashing before the commit 1e14588d0f68.

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

Added: 
clang/test/PCH/cxx17-deduction-guide-decl.cpp

Modified: 


Removed: 




diff  --git a/clang/test/PCH/cxx17-deduction-guide-decl.cpp 
b/clang/test/PCH/cxx17-deduction-guide-decl.cpp
new file mode 100644
index ..93ab82c02403
--- /dev/null
+++ b/clang/test/PCH/cxx17-deduction-guide-decl.cpp
@@ -0,0 +1,24 @@
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -std=c++17  -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -std=c++17 -o - %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace RP47219 {
+typedef int MyInt;
+template 
+class Some {
+ public:
+  explicit Some(T, MyInt) {}
+};
+
+struct Foo {};
+void ParseNatural() {
+  Some(Foo(), 1);
+}
+}
+
+#else
+
+#endif



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

[PATCH] D92573: [clang] Add a C++17 deduction guide testcase.

2020-12-04 Thread Haojian Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b9fc44d8128: [clang] Add a C++17 deduction guide testcase. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92573

Files:
  clang/test/PCH/cxx17-deduction-guide-decl.cpp


Index: clang/test/PCH/cxx17-deduction-guide-decl.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx17-deduction-guide-decl.cpp
@@ -0,0 +1,24 @@
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -std=c++17  -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -std=c++17 -o - %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace RP47219 {
+typedef int MyInt;
+template 
+class Some {
+ public:
+  explicit Some(T, MyInt) {}
+};
+
+struct Foo {};
+void ParseNatural() {
+  Some(Foo(), 1);
+}
+}
+
+#else
+
+#endif


Index: clang/test/PCH/cxx17-deduction-guide-decl.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx17-deduction-guide-decl.cpp
@@ -0,0 +1,24 @@
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -std=c++17  -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -std=c++17 -o - %s
+
+#ifndef HEADER
+#define HEADER
+
+namespace RP47219 {
+typedef int MyInt;
+template 
+class Some {
+ public:
+  explicit Some(T, MyInt) {}
+};
+
+struct Foo {};
+void ParseNatural() {
+  Some(Foo(), 1);
+}
+}
+
+#else
+
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

2020-12-04 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

There is no such thing as an object "teleporting" in C++.  Objects are always 
observed in memory with a specific address.  When an argument is passed in 
registers, its address can be observed to be different on both sides, and that 
is not permitted; there must be some operation that creates the object at the 
new address and destroys it at the old.  That operation is a destructive move 
(which I certainly did not originate as a term for this), or what your proposal 
calls a relocation (which may be a more palatable term for the C++ committee).

As I think I've mentioned before, the only conceptual difference between an 
*accepted* `trivial_abi` attribute and your proposed attribute is that your 
attribute intentionally restricts when destructive moves can be performed so 
that adopting it doesn't constitute an ABI break.  Your trait should certainly 
consider a `trivial_abi` type to be trivially relocatable.

I do think that understanding how representation choices restrict types and 
their operations is important to determining the right design here.  I do 
understand that you would prefer a more unconditional, directive-like approach, 
but that is not the direction we have taken with `trivial_abi`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92361

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

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2020-12-04 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, Szelethus, steakhal, martong, 
ASDenysPetrov.
Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit adds a very first version of this feature.
It is off by default and has to be turned on by checking the
corresponding box.  For this reason, HTML reports still keep
control notes (aka grey bubbles).

Further on, we plan on attaching arrows to events and having all arrows
not related to a currently selected event barely visible.  This will
help with reports where control flow goes back and forth (eg in loops).
Right now, it can get pretty crammed with all the arrows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,11 +10,11 @@
 //
 //===--===//
 
-#include "clang/Analysis/IssueHash.h"
-#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Analysis/IssueHash.h"
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -26,6 +26,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -88,6 +89,10 @@
  const PathDiagnosticMacroPiece& P,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter &R, FileID BugFileID,
+   const PathDiagnosticControlFlowPiece &P,
+   unsigned Number);
+
   void HandlePiece(Rewriter &R, FileID BugFileID, const PathDiagnosticPiece &P,
const std::vector &PopUpRanges, unsigned num,
unsigned max);
@@ -112,14 +117,22 @@
   // Rewrite the file specified by FID with HTML formatting.
   void RewriteFile(Rewriter &R, const PathPieces& path, FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter &R, FileID BugFileID, unsigned NumberOfArrows);
+
   /// \return Javascript for displaying shortcuts help;
   StringRef showHelpJavascript();
 
   /// \return Javascript for navigating the HTML report using j/k keys.
   StringRef generateKeyboardNavigationJavascript();
 
+  /// \return Javascript for drawing control-flow arrows.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
   std::string showRelevantLinesJavascript(
 const PathDiagnostic &D, const PathPieces &path);
@@ -130,6 +143,17 @@
 llvm::raw_string_ostream &os);
 };
 
+bool isArrowPiece(const PathDiagnosticPiece &P) {
+  return isa(P) && P.getString().empty();
+}
+
+unsigned getPathSizeWithoutArrows(const PathPieces &Path) {
+  unsigned TotalPieces = Path.size();
+  unsigned TotalArrowPieces = llvm::count_if(
+  Path, [](const PathDiagnosticPieceRef &P) { return isArrowPiece(*P); });
+  return TotalPieces - TotalArrowPieces;
+}
+
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
@@ -434,7 +458,7 @@
   if (event.key == "S") {
 var checked = document.getElement

[PATCH] D92381: [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 309474.
sammccall marked 6 inline comments as done.
sammccall added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92381

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h

Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -81,13 +81,19 @@
   llvm::Optional getProjectInfo(PathRef File) const override;
 
 private:
-  /// Caches compilation databases loaded from directories.
-  struct CachedCDB {
-std::string Path; // Not case-folded.
-std::unique_ptr CDB = nullptr;
-bool SentBroadcast = false;
-  };
-  CachedCDB &getCDBInDirLocked(PathRef File) const;
+  class DirectoryCache;
+  // If there's an explicit CompileCommandsDir, cache of the CDB found there.
+  mutable std::unique_ptr OnlyDirCache;
+
+  // Keyed by possibly-case-folded directory path.
+  // We can hand out pointers as they're stable and entries are never removed.
+  // Empty if CompileCommandsDir is given (OnlyDirCache is used instead).
+  mutable llvm::StringMap DirCaches;
+  // DirCaches access must be locked (unlike OnlyDirCache, which is threadsafe).
+  mutable std::mutex DirCachesMutex;
+
+  std::vector
+  getDirectoryCaches(llvm::ArrayRef Dirs) const;
 
   struct CDBLookupRequest {
 PathRef FileName;
@@ -95,21 +101,13 @@
 bool ShouldBroadcast = false;
   };
   struct CDBLookupResult {
-tooling::CompilationDatabase *CDB = nullptr;
+std::shared_ptr CDB;
 ProjectInfo PI;
   };
   llvm::Optional lookupCDB(CDBLookupRequest Request) const;
 
   // Performs broadcast on governed files.
   void broadcastCDB(CDBLookupResult Res) const;
-
-  mutable std::mutex Mutex;
-  // Keyed by possibly-case-folded directory path.
-  mutable llvm::StringMap CompilationDatabases;
-
-  /// Used for command argument pointing to folder where compile_commands.json
-  /// is located.
-  llvm::Optional CompileCommandsDir;
 };
 
 /// Extracts system include search path from drivers matching QueryDriverGlobs
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -16,11 +16,13 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include 
 #include 
 #include 
 #include 
@@ -58,10 +60,112 @@
   return Cmd;
 }
 
+// Loads and caches the CDB from a single directory.
+//
+// This class is threadsafe, which is to say we have independent locks for each
+// directory we're searching for a CDB.
+// Loading is deferred until first access.
+//
+// The DirectoryBasedCDB keeps a map from path => DirectoryCache.
+// Typical usage is to:
+//  - 1) determine all the paths that might be searched
+//  - 2) acquire the map lock and get-or-create all the DirectoryCache entries
+//  - 3) release the map lock and query the caches as desired
+//
+// FIXME: this should revalidate the cache sometimes
+// FIXME: IO should go through a VFS
+class DirectoryBasedGlobalCompilationDatabase::DirectoryCache {
+  // Absolute canonical path that we're the cache for. (Not case-folded).
+  std::string Path;
+
+  // True if we've looked for a CDB here and found none.
+  // (This makes it possible for get() to return without taking a lock)
+  // FIXME: this should have an expiry time instead of lasting forever.
+  std::atomic FinalizedNoCDB = {false};
+
+  // Guards following cache state.
+  std::mutex Mu;
+  // Has cache been filled from disk? FIXME: this should be an expiry time.
+  bool CachePopulated = false;
+  // Whether a new CDB has been loaded but not broadcast yet.
+  bool NeedsBroadcast = false;
+  // Last loaded CDB, meaningful if CachePopulated is set.
+  // shared_ptr so we can overwrite this when callers are still using the CDB.
+  std::shared_ptr CDB;
+
+public:
+  DirectoryCache(llvm::StringRef Path) : Path(Path) {
+assert(llvm::sys::path::is_absolute(Path));
+  }
+
+  // Get the CDB associated with this directory.
+  // ShouldBroadcast:
+  //  - as input, signals whether the caller is willing to broadcast a
+  //newly-discovered CDB. (e.g. to trigger background indexing)
+  //  - as output, signals whether the caller should do so.
+  // (If a new CDB is discovered and ShouldBroadcast is false, we mark the
+  // CDB as needing broadcast, and broadcast it next time we can).
+  std::shared_p

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2020-12-04 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko added a comment.

Here is how it looks for that test file:
F14534708: report-6ea17d.html 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92639

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

[PATCH] D92381: [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:91
+  // Whether a new CDB has been loaded but not broadcast yet.
+  bool Dirty = false;
+  // Last loaded CDB, meaningful if CachePopulated is set.

kadircet wrote:
> maybe rename this to `DidBroadcast` if we are not planning to add extra 
> meaning to `Dirty` in the near future?
Renamed to `NeedsBroadcast`.

(`DidBroadcast` isn't quite right once we can reload databases and thus need to 
broadcast again)



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:119
+if (CachePopulated) {
+  ShouldBroadcast = Dirty;
+  Dirty = false;

kadircet wrote:
> what if caller is not willing to broadcast?
Good catch, thanks.

Changed the handling of ShouldBroadcast to always flow via NeedsBroadcast, 
which is set by load().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92381

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

[PATCH] D92640: Add ability to load a FixedCompilationDatabase from a buffer.

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: usaxena95.
Herald added a subscriber: kadircet.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

Previously, loading one from a file meant allowing the library to do the IO.
Clangd would prefer to do such IO itself (e.g. to allow caching).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92640

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


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -541,6 +541,27 @@
   EXPECT_EQ(0ul, Database.getAllCompileCommands().size());
 }
 
+TEST(FixedCompilationDatabase, FromBuffer) {
+  const char *Data = R"(
+
+ -DFOO=BAR
+
+--baz
+
+  )";
+  std::string ErrorMsg;
+  auto CDB = 
FixedCompilationDatabase::loadFromBuffer("/path/compile_flags.txt",
+  Data, ErrorMsg);
+
+  std::vector Result = CDB->getCompileCommands("/foo/bar.cc");
+  ASSERT_EQ(1ul, Result.size());
+  EXPECT_EQ("/path", Result.front().Directory);
+  EXPECT_EQ("/foo/bar.cc", Result.front().Filename);
+  EXPECT_THAT(
+  Result.front().CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO=BAR", "--baz", 
"/foo/bar.cc"));
+}
+
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
   std::string ErrorMsg;
Index: clang/lib/Tooling/CompilationDatabase.cpp
===
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -348,9 +348,17 @@
 ErrorMsg = "Error while opening fixed database: " + Result.message();
 return nullptr;
   }
+  return loadFromBuffer(Path, (*File)->getBuffer(), ErrorMsg);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromBuffer(StringRef Path, StringRef Data,
+ std::string &ErrorMsg) {
+  ErrorMsg.clear();
   std::vector Args;
-  for (llvm::StringRef Line :
-   llvm::make_range(llvm::line_iterator(**File), llvm::line_iterator())) {
+  StringRef Line;
+  while (!Data.empty()) {
+std::tie(Line, Data) = Data.split('\n');
 // Stray whitespace is almost certainly unintended.
 Line = Line.trim();
 if (!Line.empty())
Index: clang/include/clang/Tooling/CompilationDatabase.h
===
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -184,11 +184,15 @@
   int &Argc, const char *const *Argv, std::string &ErrorMsg,
   Twine Directory = ".");
 
-  /// Reads flags from the given file, one-per line.
+  /// Reads flags from the given file, one-per-line.
   /// Returns nullptr and sets ErrorMessage if we can't read the file.
   static std::unique_ptr
   loadFromFile(StringRef Path, std::string &ErrorMsg);
 
+  /// Reads flags from the given buffer, one-per-line.
+  static std::unique_ptr
+  loadFromBuffer(StringRef Path, StringRef Data, std::string &ErrorMsg);
+
   /// Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -541,6 +541,27 @@
   EXPECT_EQ(0ul, Database.getAllCompileCommands().size());
 }
 
+TEST(FixedCompilationDatabase, FromBuffer) {
+  const char *Data = R"(
+
+ -DFOO=BAR
+
+--baz
+
+  )";
+  std::string ErrorMsg;
+  auto CDB = FixedCompilationDatabase::loadFromBuffer("/path/compile_flags.txt",
+  Data, ErrorMsg);
+
+  std::vector Result = CDB->getCompileCommands("/foo/bar.cc");
+  ASSERT_EQ(1ul, Result.size());
+  EXPECT_EQ("/path", Result.front().Directory);
+  EXPECT_EQ("/foo/bar.cc", Result.front().Filename);
+  EXPECT_THAT(
+  Result.front().CommandLine,
+  ElementsAre(EndsWith("clang-tool"), "-DFOO=BAR", "--baz", "/foo/bar.cc"));
+}
+
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
   std::string ErrorMsg;
Index: clang/lib/Tooling/CompilationDatabase.cpp
===
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -348,9 +348,17 @@
 ErrorMsg = "Error while opening fixed database: " + Result.message();
 return nullptr;
   }
+  return loadFromBuffer(Path, (*File)->getBuffer(), ErrorMsg);
+}
+
+std::un

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

2020-12-04 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

It might be worth to extend StmtComparer 

 as well with this Expr. I mean, probably two selections exprs should be 
considered equal if their assoc exprs are equal too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

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

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

2020-12-04 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

In D92600#2433263 , @martong wrote:

> It might be worth to extend StmtComparer 
> 
>  as well with this Expr. I mean, probably two selections exprs should be 
> considered equal if their assoc exprs are equal too...

@teemperor , WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

In D79773#2432088 , @miscco wrote:

> As someone who has extensively worked with concepts I cannot stress how much 
> this would improve my live

and I'd like to think that this will help even in its current form, but it 
needs someone to accept it first before it can land,

This has been my problem with clang-format , I do try and give some of my time 
to keeping up with the new and outstanding reviews coming in, but my own 
progress on clang-format or the fixing of bugs has slowed up because of a lack 
of people to review, we just need a couple more people who are willing to come 
once or twice a week and just keep things rolling along. but someone has to 
"watch the watcher" too.

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

https://reviews.llvm.org/D79773

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

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

2020-12-04 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

Having the comparison code would be nice. The custom types in the associated 
types aren't compared by the generic code, so an overload like this in the 
`StmtComparer` class should be enough:

  bool IsStmtEquivalent(const GenericSelectionExpr *E1, const 
GenericSelectionExpr *E2) {
for (auto Pair : zip_longest(E1->getAssocTypeSourceInfos(),
 E2->getAssocTypeSourceInfos())) {
  Optional Child1 = std::get<0>(Pair);
  Optional Child2 = std::get<1>(Pair);
  // Different number of associated types.
  if (!Child1 || !Child2)
return false;
  
  if (!IsStructurallyEquivalent(Context, (*Child1)->getType(),
(*Child2)->getType()))
return false;
}
return true;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92600

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

[PATCH] D92245: -fstack-clash-protection: Return an actual error when used on unsupported OS

2020-12-04 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

In D92245#2425817 , @emaste wrote:

>> How do things go wrong on Darwin? I was under the impression that this was 
>> implemented in LLVM as strictly inline code, no runtime support required.
>
> That is my impression as well (although it seems that an earlier version 
> might have emitted calls to a stack probe routine?)

The original security advisory doesn't mention Darwin, but there's nothing 
specific to Darwin in the stack clash protection implementation, so I'm fine 
with allowing it for Darwin too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92245

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

[PATCH] D92642: [clangd] Fix an assertion violation in rename.

2020-12-04 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang.

NamedDecl::getName() asserts the name must be an identifier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92642

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) f^oo; // special name.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) f^oo; // special name.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c17fdca - [clang] [Headers] Use the corresponding _aligned_free or __mingw_aligned_free in _mm_free

2020-12-04 Thread Martin Storsjö via cfe-commits


Author: Martin Storsjö
Date: 2020-12-04T11:34:12+02:00
New Revision: c17fdca1883ddee94c6b7e055428d4445ab13e42

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

LOG: [clang] [Headers] Use the corresponding _aligned_free or 
__mingw_aligned_free in _mm_free

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

Added: 


Modified: 
clang/lib/Headers/mm_malloc.h

Removed: 




diff  --git a/clang/lib/Headers/mm_malloc.h b/clang/lib/Headers/mm_malloc.h
index 0ea32517aea8..933dbaacade5 100644
--- a/clang/lib/Headers/mm_malloc.h
+++ b/clang/lib/Headers/mm_malloc.h
@@ -54,7 +54,13 @@ _mm_malloc(size_t __size, size_t __align)
 static __inline__ void __attribute__((__always_inline__, __nodebug__))
 _mm_free(void *__p)
 {
+#if defined(__MINGW32__)
+  __mingw_aligned_free(__p);
+#elif defined(_WIN32)
+  _aligned_free(__p);
+#else
   free(__p);
+#endif
 }
 #endif
 



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

[PATCH] D92570: [clang] [Headers] Use the corresponding _aligned_free or __mingw_aligned_free in _mm_free

2020-12-04 Thread Martin Storsjö via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc17fdca1883d: [clang] [Headers] Use the corresponding 
_aligned_free or __mingw_aligned_free… (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92570

Files:
  clang/lib/Headers/mm_malloc.h


Index: clang/lib/Headers/mm_malloc.h
===
--- clang/lib/Headers/mm_malloc.h
+++ clang/lib/Headers/mm_malloc.h
@@ -54,7 +54,13 @@
 static __inline__ void __attribute__((__always_inline__, __nodebug__))
 _mm_free(void *__p)
 {
+#if defined(__MINGW32__)
+  __mingw_aligned_free(__p);
+#elif defined(_WIN32)
+  _aligned_free(__p);
+#else
   free(__p);
+#endif
 }
 #endif
 


Index: clang/lib/Headers/mm_malloc.h
===
--- clang/lib/Headers/mm_malloc.h
+++ clang/lib/Headers/mm_malloc.h
@@ -54,7 +54,13 @@
 static __inline__ void __attribute__((__always_inline__, __nodebug__))
 _mm_free(void *__p)
 {
+#if defined(__MINGW32__)
+  __mingw_aligned_free(__p);
+#elif defined(_WIN32)
+  _aligned_free(__p);
+#else
   free(__p);
+#endif
 }
 #endif
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2020-12-04 Thread Ella Ma via Phabricator via cfe-commits

OikawaKirie added a comment.

It is really a good idea!

The operations that would not leave an event in the report are now clearly 
printed.

But there are three arrows that confuse me in the example report: the 
assignment `x = 0` (x -> 0 -> x), the function call `dereference(x)` (x -> 
dereference), and the return statement `return *x` (int -> *x). I know the 
arrow is based on the evaluation order of the engine. But from the view of a 
user, I think these arrows are confusing to some extent.

For the first two, I think it would be better to point just the statement 
(maybe a `CFGElement`) without inner arrows (x -> 0 -> x and x -> dereference), 
or point to the location of the operator itself rather than the BeginLoc (e.g. 
x -> 0 -> =). For the third one, an arrow from the function name to the first 
`CFGElement` looks good to me. And an arrow from the returned expr to the 
return type or to a special mark (e.g. ⬅️) can also be added, together with 
function calls (an arrow from the callstmt to a special mark, e.g. ➡️).

By the way, what do you think about adding arrows for data flows of specific 
symbolic values in the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92639

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

[PATCH] D91925: [clangd][NFC] Small tweak to combined provider

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 309486.
njames93 added a comment.

Removed const qualifier on Providers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91925

Files:
  clang-tools-extra/clangd/ConfigProvider.cpp


Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -142,7 +142,7 @@
 
 std::unique_ptr
 Provider::combine(std::vector Providers) {
-  struct CombinedProvider : Provider {
+  class CombinedProvider : public Provider {
 std::vector Providers;
 
 std::vector
@@ -154,14 +154,13 @@
   }
   return Result;
 }
+
+  public:
+CombinedProvider(std::vector Providers)
+: Providers(std::move(Providers)) {}
   };
-  auto Result = std::make_unique();
-  Result->Providers = std::move(Providers);
-  // FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
-  //   The constructor that is supposed to allow for Derived to Base
-  //   conversion does not work. Remove this if we drop support for such
-  //   configurations.
-  return std::unique_ptr(Result.release());
+
+  return std::make_unique(std::move(Providers));
 }
 
 Config Provider::getConfig(const Params &P, DiagnosticCallback DC) const {


Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -142,7 +142,7 @@
 
 std::unique_ptr
 Provider::combine(std::vector Providers) {
-  struct CombinedProvider : Provider {
+  class CombinedProvider : public Provider {
 std::vector Providers;
 
 std::vector
@@ -154,14 +154,13 @@
   }
   return Result;
 }
+
+  public:
+CombinedProvider(std::vector Providers)
+: Providers(std::move(Providers)) {}
   };
-  auto Result = std::make_unique();
-  Result->Providers = std::move(Providers);
-  // FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
-  //   The constructor that is supposed to allow for Derived to Base
-  //   conversion does not work. Remove this if we drop support for such
-  //   configurations.
-  return std::unique_ptr(Result.release());
+
+  return std::make_unique(std::move(Providers));
 }
 
 Config Provider::getConfig(const Params &P, DiagnosticCallback DC) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

Its close




Comment at: clang/lib/Format/Format.cpp:751
 /*AfterFunction=*/false,
 /*AfterNamespace=*/false,
 /*AfterObjCDeclaration=*/false,

I think you may need to rebase, this code looks to be from before a relatively 
recent change



Comment at: clang/lib/Format/Format.cpp:893
  /*BeforeLambdaBody=*/false,
+ /*BeforeStructInitialization=*/false,
  /*BeforeWhile=*/false,

I believe there are 3 places where BraceWrapping is initialized you seem to 
only be doing 2 of them



Comment at: clang/unittests/Format/FormatTest.cpp:5063
+   "a = 1,\n"
+   "b = 2,\n"
+   "};",

could you add an additional test without the trailing `,`


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

https://reviews.llvm.org/D91949

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

[PATCH] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 309489.
njames93 added a comment.

Made this just focus on the destruction asserts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

Files:
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  llvm/include/llvm/ADT/IntrusiveRefCntPtr.h


Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -70,11 +70,23 @@
 template  class RefCountedBase {
   mutable unsigned RefCount = 0;
 
-public:
+protected:
   RefCountedBase() = default;
   RefCountedBase(const RefCountedBase &) {}
   RefCountedBase &operator=(const RefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~RefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~RefCountedBase() = default;
+#endif
+
+public:
   void Retain() const { ++RefCount; }
 
   void Release() const {
@@ -94,6 +106,17 @@
   ThreadSafeRefCountedBase &
   operator=(const ThreadSafeRefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~ThreadSafeRefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~ThreadSafeRefCountedBase() = default;
+#endif
+
 public:
   void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
 
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -191,9 +191,20 @@
   IntrusiveRefCntPtr InnerMatcher;
 };
 
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This 
prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+  static void call(void *Ptr) {
+static_cast(Ptr)->Release();
+  }
+};
+
 } // namespace
 
-static llvm::ManagedStatic TrueMatcherInstance;
+static llvm::ManagedStatic,
+   DynMatcherInterfaceDeleter>
+TrueMatcherInstance;
 
 bool ASTMatchFinder::isTraversalIgnoringImplicitNodes() const {
   return getASTContext().getParentMapContext().getTraversalKind() ==


Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -70,11 +70,23 @@
 template  class RefCountedBase {
   mutable unsigned RefCount = 0;
 
-public:
+protected:
   RefCountedBase() = default;
   RefCountedBase(const RefCountedBase &) {}
   RefCountedBase &operator=(const RefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~RefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~RefCountedBase() = default;
+#endif
+
+public:
   void Retain() const { ++RefCount; }
 
   void Release() const {
@@ -94,6 +106,17 @@
   ThreadSafeRefCountedBase &
   operator=(const ThreadSafeRefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~ThreadSafeRefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~ThreadSafeRefCountedBase() = default;
+#endif
+
 public:
   void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
 
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -191,9 +191,20 @@
   IntrusiveRefCntPtr InnerMatcher;
 };
 
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+  static void call(void *Ptr) {
+static_cast(Ptr)->Release();
+  }
+};
+
 } // namespace
 
-static llvm::ManagedStatic TrueMatcherInstance;
+static llvm::ManagedStatic,
+   DynMatcherInterfaceDeleter>
+TrueMatcherInstance;
 
 bool ASTMatchFinder::isTraversalIgnoringImplicitNodes() const {
   return getASTContext().getParentMapContext().getTraversalKind() ==
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92480: [llvm] Add asserts in (ThreadSafe)?RefCountedBase destructors

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:190-196
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This 
prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+  static void call(void *Ptr) {
+static_cast(Ptr)->Release();
+  }
+};

dblaikie wrote:
> I think probably the right solution is to have TrueMatcherImpl's dtor Release 
> the same way its ctor Retains. Symmetry is nice/helps avoid splitting this 
> quirk into two different places.
That method is a nasty can of worms. If TrueMatcherImpl dtor calls `Release`, 
that `Release` call would realise the RefCount is now zero. That in turn would 
then call delete on `TrueMatcherImpl` resulting in a double-free.
I think symmetry is possibly the right way to go though, just in using a custom 
creator that calls Retain itself, remove that responsibility from 
TrueMatcherImpl. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2020-12-04 Thread Anastasiia Lukianenko via Phabricator via cfe-commits

anastasiia_lukianenko added a comment.

For now without my patch current behavior is the following:
Your examples listed below:

  asm("AA"
  : "DEF"
  : "GHI");
  
  asm volatile(
  "AAA"
  : "DEF"
  : "GHI");
  
  asm("AAA" : "DEF" : "GHI");
  
  asm volatile("A" : "DEF" : "GHI");
  
  asm volatile("A" : "DEF"(dst) : "GHI"(src));
  asm volatile("A" : "DEF"(dst));
  
  asm volatile("A" : [Foo] "DEF"(dst) : [Foo] "GHI"(src));
  asm volatile("A" : % [Foo] "DEF"(dst) : % [Foo] "GHI"(src));

Formatted with clang-format in this way:

  asm("AA"
  : "DEF"
  : "GHI");
  
  asm volatile(
  "AAA"
  : "DEF"
  : "GHI");
  
  asm("AAA" : "DEF" : "GHI");
  
  asm volatile("A" : "DEF" : "GHI");
  
  asm volatile("A" : "DEF"(dst) : "GHI"(src));
  asm volatile("A" : "DEF"(dst));
  
  asm volatile("A" : [Foo] "DEF"(dst) : [Foo] "GHI"(src));
  asm volatile("A" : % [Foo] "DEF"(dst) : % [Foo] "GHI"(src));

So that's why my patch is breaking only long strings. If this is a bug, I can 
try to fix it. Then I update my patch so the configuration will be as 
@MyDeveloperDay expected.


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

https://reviews.llvm.org/D91950

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

[PATCH] D92646: [Tooling] JSONCompilationDatabase::loadFromBuffer retains the buffer, copy it.

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: usaxena95.
Herald added a subscriber: kadircet.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

This function doesn't seem to be used in-tree outside tests.
However clangd wants to use it soon, and having the CDB be self-contained seems
reasonable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92646

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


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -172,13 +172,15 @@
 }
 
 static CompileCommand findCompileArgsInJsonDatabase(StringRef FileName,
-StringRef JSONDatabase,
+std::string JSONDatabase,
 std::string &ErrorMessage) 
{
   std::unique_ptr Database(
   JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage,
   JSONCommandLineSyntax::Gnu));
   if (!Database)
 return CompileCommand();
+  // Overwrite the string to verify we're not reading from it later.
+  JSONDatabase.assign(JSONDatabase.size(), '*');
   std::vector Commands = 
Database->getCompileCommands(FileName);
   EXPECT_LE(Commands.size(), 1u);
   if (Commands.empty())
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -217,7 +217,7 @@
 std::string &ErrorMessage,
 JSONCommandLineSyntax Syntax) {
   std::unique_ptr DatabaseBuffer(
-  llvm::MemoryBuffer::getMemBuffer(DatabaseString));
+  llvm::MemoryBuffer::getMemBufferCopy(DatabaseString));
   std::unique_ptr Database(
   new JSONCompilationDatabase(std::move(DatabaseBuffer), Syntax));
   if (!Database->parse(ErrorMessage))


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -172,13 +172,15 @@
 }
 
 static CompileCommand findCompileArgsInJsonDatabase(StringRef FileName,
-StringRef JSONDatabase,
+std::string JSONDatabase,
 std::string &ErrorMessage) {
   std::unique_ptr Database(
   JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage,
   JSONCommandLineSyntax::Gnu));
   if (!Database)
 return CompileCommand();
+  // Overwrite the string to verify we're not reading from it later.
+  JSONDatabase.assign(JSONDatabase.size(), '*');
   std::vector Commands = Database->getCompileCommands(FileName);
   EXPECT_LE(Commands.size(), 1u);
   if (Commands.empty())
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -217,7 +217,7 @@
 std::string &ErrorMessage,
 JSONCommandLineSyntax Syntax) {
   std::unique_ptr DatabaseBuffer(
-  llvm::MemoryBuffer::getMemBuffer(DatabaseString));
+  llvm::MemoryBuffer::getMemBufferCopy(DatabaseString));
   std::unique_ptr Database(
   new JSONCompilationDatabase(std::move(DatabaseBuffer), Syntax));
   if (!Database->parse(ErrorMessage))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92642: [clangd] Fix an assertion violation in rename.

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:949
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}

This case seems a little *too* special: you might consider `-(int) foo:(int)x;` 
or so to make it clearer why this isn't an ordinary name.

I'd also make the comment a bit more specific: "Token is an identifier, but 
declaration name isn't a simple identifier".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92642

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

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2020-12-04 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko added a comment.

In D92639#2433303 , @OikawaKirie wrote:

> It is really a good idea!

Thanks 😊

> The operations that would not leave an event in the report are now clearly 
> printed.
>
> But there are three arrows that confuse me in the example report: the 
> assignment `x = 0` (x -> 0 -> x), the function call `dereference(x)` (x -> 
> dereference), and the return statement `return *x` (int -> *x). I know the 
> arrow is based on the evaluation order of the engine. But from the view of a 
> user, I think these arrows are confusing to some extent.
>
> For the first two, I think it would be better to point just the statement 
> (maybe a `CFGElement`) without inner arrows (x -> 0 -> x and x -> 
> dereference), or point to the location of the operator itself rather than the 
> BeginLoc (e.g. x -> 0 -> =). For the third one, an arrow from the function 
> name to the first `CFGElement` looks good to me. And an arrow from the 
> returned expr to the return type or to a special mark (e.g. ⬅️) can also be 
> added, together with function calls (an arrow from the callstmt to a special 
> mark, e.g. ➡️).

Sorry, for the confusion.  I did not come up with the idea of arrows and 
neither did I chose which tokens are connected by arrows.  It is an existing 
feature, and it existed for quite a long time.  The analyzer can produce 
`plist` files that contain all these locations, `plist` files are further used 
by Xcode to draw arrows.  You can see a very old example on our website: 
https://clang-analyzer.llvm.org
Essentially I took this existing information and used it in the HTML report 
generation as well.  The biggest chunk of this commit is the algorithm for 
drawing SVG curves.

> By the way, what do you think about adding arrows for data flows of specific 
> symbolic values in the future?

I'm open for many ideas in the ways how we can improve our HTML reports!  I 
thought of a popup when you hover over an arrow showing values/constraints for 
symbols actively involved in the report.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92639

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

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2020-12-04 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko updated this revision to Diff 309497.
vsavchenko added a comment.

Fix incorrect comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,11 +10,11 @@
 //
 //===--===//
 
-#include "clang/Analysis/IssueHash.h"
-#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Analysis/IssueHash.h"
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -26,6 +26,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -88,6 +89,10 @@
  const PathDiagnosticMacroPiece& P,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter &R, FileID BugFileID,
+   const PathDiagnosticControlFlowPiece &P,
+   unsigned Number);
+
   void HandlePiece(Rewriter &R, FileID BugFileID, const PathDiagnosticPiece &P,
const std::vector &PopUpRanges, unsigned num,
unsigned max);
@@ -112,14 +117,22 @@
   // Rewrite the file specified by FID with HTML formatting.
   void RewriteFile(Rewriter &R, const PathPieces& path, FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter &R, FileID BugFileID, unsigned NumberOfArrows);
+
   /// \return Javascript for displaying shortcuts help;
   StringRef showHelpJavascript();
 
   /// \return Javascript for navigating the HTML report using j/k keys.
   StringRef generateKeyboardNavigationJavascript();
 
+  /// \return Javascript for drawing control-flow arrows.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
   std::string showRelevantLinesJavascript(
 const PathDiagnostic &D, const PathPieces &path);
@@ -130,6 +143,17 @@
 llvm::raw_string_ostream &os);
 };
 
+bool isArrowPiece(const PathDiagnosticPiece &P) {
+  return isa(P) && P.getString().empty();
+}
+
+unsigned getPathSizeWithoutArrows(const PathPieces &Path) {
+  unsigned TotalPieces = Path.size();
+  unsigned TotalArrowPieces = llvm::count_if(
+  Path, [](const PathDiagnosticPieceRef &P) { return isArrowPiece(*P); });
+  return TotalPieces - TotalArrowPieces;
+}
+
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
@@ -434,7 +458,7 @@
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;
 filterCounterexample(!checked);
-document.getElementsByName("showCounterexample")[0].checked = !checked;
+document.getElementsByName("showCounterexample")[0].click();
   } else {
 return;
   }
@@ -454,6 +478,11 @@
 
Show only relevant lines
 
+
+
+   Show control flow arrows
+
 
 )<<<";
 
@@ -482,6 +511,9 @@
   R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
  generateKeyboardNavigationJavascript());
 
+  R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
+ generateArrowDrawingJavascript());
+
   // Checkbox and javascript for filtering the o

[PATCH] D92157: [clangd] Add language metrics for recovery AST usage.

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:41
+const char *getLanguage(const clang::LangOptions &Lang) {
+  if (Lang.C99 || Lang.C11 || Lang.C17 || Lang.C2x)
+return "C";

Testing for specific C versions seems a bit weird to me - what if we're in C89? 
I'm not sure what the intention is - if the idea is to exclude extensions like 
openmp I'm not sure this actually does that.
And by checking it before ObjC I think we're misclassifying `clang -std=c99 
foo.m`

I'd suggest checking (optionally objc++), objc, c++, and calling everything 
else C. If you really want to avoid particular dialects, probably best to name 
them specifically.



Comment at: clang-tools-extra/clangd/Selection.cpp:65
   RecoveryType.record(RE->isTypeDependent() ? 0 : 1);
+  RecoveryAvailable.record(1, getLanguage(AST.getLangOpts()));
   return;

I'm not clear what this is trying to measure - why isn't this the same metric 
as SelectionUsedRecovery (just adding a field to that one?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92157

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

[PATCH] D92647: Refactor codes and tests to make it work in PCH builds

2020-12-04 Thread Yafei Liu via Phabricator via cfe-commits

psionic12 created this revision.
psionic12 added reviewers: aaron.ballman, john.brawn.
psionic12 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The former codes won't work for cases which PCH involves 
in a build workflow, this patch fixes this. Besides, add 
some tips on how to deal with PCH when traversing nodes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92647

Files:
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/Frontend/plugin-call-super-pch.cpp

Index: clang/test/Frontend/plugin-call-super-pch.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super-pch.cpp
@@ -0,0 +1,19 @@
+// RUN: split-file %s %t
+// RUN: %clang -cc1 -verify=verify-pch -x c++-header %t/call_super_test.h -emit-pch -o %t/call_super_test.h.pch -load %llvmshlibdir/CallSuperAttr%pluginext
+// RUN: %clang -cc1 -verify=verify-source -fsyntax-only -include-pch %t/call_super_test.h.pch %t/call_super_test.cpp  -load %llvmshlibdir/CallSuperAttr%pluginext
+//--- call_super_test.h
+struct Foo {
+  [[clang::call_super]]
+  virtual void test() {}
+  [[clang::call_super]]
+  virtual void test2() final {} // verify-pch-warning {{'call_super' attribute marked on a final method}}
+};
+
+struct Bar : public Foo {
+  void test() override final;
+};
+
+//--- call_super_test.cpp
+void Bar::test() { // verify-source-warning {{virtual function 'Foo::test' is marked as 'call_super' but this overriding method does not call the base version}}
+// verify-source-note@call_super_test.h:7 {{function marked 'call_super' here}}
+}
\ No newline at end of file
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -12,15 +12,10 @@
 // This example shows that attribute plugins combined with ``PluginASTAction``
 // in Clang can do some of the same things which Java Annotations do.
 //
-// Unlike the other attribute plugin examples, this one does not attach an
-// attribute AST node to the declaration AST node. Instead, it keeps a separate
-// list of attributed declarations, which may be faster than using
-// ``Decl::getAttr()`` in some cases. The disadvantage of this approach is
-// that the attribute is not part of the AST, which means that dumping the AST
-// will lose the attribute information, pretty printing the AST won't write the
-// attribute back out to source, and AST matchers will not be able to match
-// against the attribute on the declaration.
-//
+// Notice the part of traversing decls, it only focuses on nodes which belong to
+// the current parsing file, all other nodes are not loaded, this is especially
+// useful to improve the performance when using PCH, or the upcoming module
+// feature in C++20.
 //===--===//
 
 #include "clang/AST/ASTContext.h"
@@ -34,11 +29,14 @@
 using namespace clang;
 
 namespace {
-// Cached methods which are marked as 'call_super'.
-llvm::SmallPtrSet MarkedMethods;
+constexpr char AnnotateStr[] = "call_super";
 bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
-  // Uses this way to avoid add an annotation attr to the AST.
-  return MarkedMethods.contains(D);
+  for (auto It = D->specific_attr_begin();
+   It != D->specific_attr_end(); It++) {
+if (AnnotateStr == It->getAnnotation())
+  return true;
+  }
+  return false;
 }
 
 class MethodUsageVisitor : public RecursiveASTVisitor {
@@ -78,6 +76,12 @@
 DiagnosticsEngine::Note, "function marked 'call_super' here");
   }
   bool VisitCXXMethodDecl(CXXMethodDecl *MethodDecl) {
+if (isMarkedAsCallSuper(MethodDecl) &&
+MethodDecl->isFirstDecl() /* avoid warning multiple times */
+) {
+  lateDiagAppertainsToDecl(MethodDecl);
+}
+
 if (MethodDecl->isThisDeclarationADefinition() && MethodDecl->hasBody()) {
   // First find out which overridden methods are marked as 'call_super'
   llvm::SmallPtrSet OverriddenMarkedMethods;
@@ -106,27 +110,11 @@
   DiagnosticsEngine &Diags;
   unsigned WarningSuperNotCalled;
   unsigned NotePreviousCallSuperDeclaration;
-};
-
-class CallSuperConsumer : public ASTConsumer {
-public:
-  void HandleTranslationUnit(ASTContext &Context) override {
-auto &Diags = Context.getDiagnostics();
-for (const auto *Method : MarkedMethods) {
-  lateDiagAppertainsToDecl(Diags, Method);
-}
-
-CallSuperVisitor Visitor(Context.getDiagnostics());
-Visitor.TraverseDecl(Context.getTranslationUnitDecl());
-  }
-
-private:
   // This function does checks which cannot be done in `diagAppertainsToDecl()`,
   // typical example is checking Attributes (such as `FinalAttr`), on the time
   // when `diagAppertainsToDecl()` is called, `FinalAttr` is not added i

[PATCH] D92642: [clangd] Fix an assertion violation in rename.

2020-12-04 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 309506.
hokein marked an inline comment as done.
hokein added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92642

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) fo^o:(int)x; // Token is an identifier, but declaration name 
isn't a simple identifier.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) fo^o:(int)x; // Token is an identifier, but declaration name isn't a simple identifier.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 445289a - [clangd] Fix an assertion violation in rename.

2020-12-04 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-12-04T12:23:26+01:00
New Revision: 445289aa63e1b82b9eea6497fb2d0443813a9d4e

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

LOG: [clangd] Fix an assertion violation in rename.

NamedDecl::getName() asserts the name must be an identifier.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 0af8a98427c7..8ed5811c88b2 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@ llvm::Expected rename(const RenameInputs 
&RInputs) {
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp 
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 306909892509..0aa87c61baeb 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@ TEST(RenameTest, Renameable) {
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) fo^o:(int)x; // Token is an identifier, but declaration name 
isn't a simple identifier.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);



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

[PATCH] D92642: [clangd] Fix an assertion violation in rename.

2020-12-04 Thread Haojian Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG445289aa63e1: [clangd] Fix an assertion violation in rename. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92642

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) fo^o:(int)x; // Token is an identifier, but declaration name 
isn't a simple identifier.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -946,6 +946,13 @@
)cpp",
"not a supported kind", !HeaderFile, Index},
 
+  {R"cpp(// disallow rename on non-normal identifiers.
+ @interface Foo {}
+ -(int) fo^o:(int)x; // Token is an identifier, but declaration name isn't a simple identifier.
+ @end
+   )cpp",
+   "not a supported kind", HeaderFile, Index},
+
   {R"cpp(
  void foo(int);
  void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -637,7 +637,10 @@
   if (DeclsUnderCursor.size() > 1)
 return makeError(ReasonToReject::AmbiguousSymbol);
   const auto &RenameDecl = **DeclsUnderCursor.begin();
-  if (RenameDecl.getName() == RInputs.NewName)
+  const auto *ID = RenameDecl.getIdentifier();
+  if (!ID)
+return makeError(ReasonToReject::UnsupportedSymbol);
+  if (ID->getName() == RInputs.NewName)
 return makeError(ReasonToReject::SameName);
   auto Invalid = checkName(RenameDecl, RInputs.NewName);
   if (Invalid)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-12-04 Thread Paul C. Anagnostopoulos via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG415fab6f67b4: [TableGen] Eliminate the 'code' type 
(authored by Paul-C-Anagnostopoulos).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

Files:
  clang/utils/TableGen/ClangOptionDocEmitter.cpp
  llvm/docs/TableGen/BackEnds.rst
  llvm/docs/TableGen/BackGuide.rst
  llvm/docs/TableGen/ProgRef.rst
  llvm/include/llvm/TableGen/Error.h
  llvm/include/llvm/TableGen/Record.h
  llvm/include/llvm/TableGen/SearchableTable.td
  llvm/lib/TableGen/Error.cpp
  llvm/lib/TableGen/JSONBackend.cpp
  llvm/lib/TableGen/Record.cpp
  llvm/lib/TableGen/TGLexer.cpp
  llvm/lib/TableGen/TGLexer.h
  llvm/lib/TableGen/TGParser.cpp
  llvm/lib/Target/AMDGPU/MIMGInstructions.td
  llvm/test/TableGen/code.td
  llvm/test/TableGen/generic-tables.td
  llvm/test/TableGen/interleave.td
  llvm/test/TableGen/unterminated-code-block.td
  llvm/utils/TableGen/AsmWriterEmitter.cpp
  llvm/utils/TableGen/DFAEmitter.cpp
  llvm/utils/TableGen/GICombinerEmitter.cpp
  llvm/utils/TableGen/RISCVCompressInstEmitter.cpp
  llvm/utils/TableGen/SearchableTableEmitter.cpp
  mlir/include/mlir/TableGen/Operator.h
  mlir/lib/TableGen/Attribute.cpp
  mlir/lib/TableGen/Dialect.cpp
  mlir/lib/TableGen/Operator.cpp
  mlir/lib/TableGen/Pattern.cpp
  mlir/lib/TableGen/Type.cpp
  mlir/lib/TableGen/TypeDef.cpp
  mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Index: mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
===
--- mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -137,7 +137,7 @@
 static inline bool hasStringAttribute(const Record &record,
   StringRef fieldName) {
   auto valueInit = record.getValueInit(fieldName);
-  return isa(valueInit);
+  return isa(valueInit);
 }
 
 static std::string getArgumentName(const Operator &op, int index) {
@@ -1796,15 +1796,15 @@
 return;
 
   auto valueInit = def.getValueInit("printer");
-  CodeInit *codeInit = dyn_cast(valueInit);
-  if (!codeInit)
+  StringInit *stringInit = dyn_cast(valueInit);
+  if (!stringInit)
 return;
 
   auto *method =
   opClass.addMethodAndPrune("void", "print", "::mlir::OpAsmPrinter &", "p");
   FmtContext fctx;
   fctx.addSubst("cppClass", opClass.getClassName());
-  auto printer = codeInit->getValue().ltrim().rtrim(" \t\v\f\r");
+  auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r");
   method->body() << "  " << tgfmt(printer, &fctx);
 }
 
@@ -1816,8 +1816,8 @@
<< "return ::mlir::failure();\n";
 
   auto *valueInit = def.getValueInit("verifier");
-  CodeInit *codeInit = dyn_cast(valueInit);
-  bool hasCustomVerify = codeInit && !codeInit->getValue().empty();
+  StringInit *stringInit = dyn_cast(valueInit);
+  bool hasCustomVerify = stringInit && !stringInit->getValue().empty();
   populateSubstitutions(op, "this->getAttr", "this->getODSOperands",
 "this->getODSResults", verifyCtx);
 
@@ -1841,7 +1841,7 @@
   if (hasCustomVerify) {
 FmtContext fctx;
 fctx.addSubst("cppClass", opClass.getClassName());
-auto printer = codeInit->getValue().ltrim().rtrim(" \t\v\f\r");
+auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r");
 body << "  " << tgfmt(printer, &fctx);
   } else {
 body << "  return ::mlir::success();\n";
Index: mlir/lib/TableGen/TypeDef.cpp
===
--- mlir/lib/TableGen/TypeDef.cpp
+++ mlir/lib/TableGen/TypeDef.cpp
@@ -78,10 +78,10 @@
   return def->getValueAsOptionalString("mnemonic");
 }
 llvm::Optional TypeDef::getPrinterCode() const {
-  return def->getValueAsOptionalCode("printer");
+  return def->getValueAsOptionalString("printer");
 }
 llvm::Optional TypeDef::getParserCode() const {
-  return def->getValueAsOptionalCode("parser");
+  return def->getValueAsOptionalString("parser");
 }
 bool TypeDef::genAccessors() const {
   return def->getValueAsBit("genAccessors");
@@ -114,7 +114,7 @@
 llvm::RecordVal *code = typeParameter->getDef()->getValue("allocator");
 if (!code)
   return llvm::Optional();
-if (llvm::CodeInit *ci = dyn_cast(code->getValue()))
+if (llvm::StringInit *ci = dyn_cast(code->getValue()))
   return ci->getValue();
 if (isa(code->getValue()))
   return llvm::Optional();
Index: mlir/lib/TableGen/Type.cpp
===
--- mlir/lib/TableGen/Type.cpp
+++ mlir/lib/TableGen/Type.cpp
@@ -46,7 +46,7 @@
   if (!builderCall || !builderCall->getValue())
 return llvm::None;
   return TypeSwitch>(builderCall->getValue())
-  .Case([&](auto *init) {
+  .Case([&](auto *init) {
 StringRef value = init->getValue();
 return value.empty() ? Optional() : value;
   })
Index: mlir/lib/TableGen/Pattern.cpp
=

[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-12-04 Thread Sean Silva via Phabricator via cfe-commits

silvas added a comment.

Wow, awesome work!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a minor testing request.




Comment at: clang/lib/AST/ASTTypeTraits.cpp:138
+return ASTNodeKind(NKI_##A##Attr);
+#include "clang/Basic/AttrList.inc"
+  }

ymandel wrote:
> aaron.ballman wrote:
> > Oye, this brings up an interesting point. Plugin-based attributes currently 
> > cannot create their own semantic attribute, but will often instead reuse an 
> > existing semantic attribute like `annotate`. This means code like 
> > `[[clang::plugin_attr]] int x;` may or may not be possible to match. 
> > Further, some builtin attributes have no semantic attribute associated with 
> > them whatsoever: 
> > https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/Attr.td#L2740
> > 
> > I think the `switch` statement logic here is correct in these weird cases 
> > and we won't hit the `llvm_unreachable`. For attributes with no AST 
> > representation, there's no `Attr` object that could be passed in the first 
> > place. Unknown attributes similarly won't get here because there's no way 
> > to get an AST node for them. Plugin-based attributes are still going to be 
> > similarly surprising, but... I don't know that we can solve that here given 
> > there's no way to create a plugin-based semantic attribute yet.
> > 
> > Pining @ymandel to raise awareness of these sorts of issues that stencil 
> > may run into. For the AST matchers, I think it's reasonable for us to say 
> > "if there's no AST node, we can't match on it", but IIRC, stencil was 
> > looking to stay a bit closer to the user's source code rather than be 
> > strongly tied to the AST.
> @aaron.ballman Thanks for pinging me. However, `Stencil` is limited to AST 
> nodes, for better or worse. They make it somewhat easier to _generate_ source 
> plainly, but they are fundamentaly an abstraction over the AST.  I think that 
> the only way we'll get beyond the AST is Syntax Trees.
> 
> Still, nice to see this patch! I've been meaning to do the same for 
> LambaCapture for a while and this will be a handy guide to what needs to be 
> changed.
That's good to know, @ymandel, thank you!



Comment at: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1887
+  // On windows, some nodes have an implicit visibility attribute.
+  EXPECT_TRUE(
+  notMatches("struct F{}; int x(int *);", attr(unless(isImplicit();

aaron.ballman wrote:
> Can you add an expects false test for an unknown attribute and another one 
> for an attribute with no AST node associated with it?
This comment may have been missed during the discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

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

[PATCH] D92480: [llvm] Add asserts in (ThreadSafe)?RefCountedBase destructors

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 309512.
njames93 added a comment.

Made ManagedStatic code transparent to users for RefCounted objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

Files:
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
  llvm/include/llvm/Support/ManagedStatic.h

Index: llvm/include/llvm/Support/ManagedStatic.h
===
--- llvm/include/llvm/Support/ManagedStatic.h
+++ llvm/include/llvm/Support/ManagedStatic.h
@@ -15,21 +15,79 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 
+namespace detail {
+// Helper struct to detect classes that has Retain and Release member functions
+// that are const with no params.
+template  struct HasRetainRelease {
+private:
+  template 
+  static constexpr decltype(std::declval().Retain(),
+std::declval().Release(), bool())
+  test(int) {
+return true;
+  }
+
+  template  static constexpr bool test(...) { return false; }
+
+public:
+  static constexpr bool Value = test(int());
+};
+
+template 
+std::enable_if_t::Value, void *> createObject() {
+  return new T();
+}
+
+template 
+std::enable_if_t::Value, void *> createObject() {
+  T *Ptr = new T();
+  Ptr->Retain();
+  return Ptr;
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  delete Ptr;
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  Ptr->Release();
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  delete[] Ptr;
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  for (auto I = Ptr, E = I + N; I != E; ++I)
+I->Release();
+}
+} // namespace detail
+
 /// object_creator - Helper method for ManagedStatic.
-template  struct object_creator {
-  static void *call() { return new C(); }
+
+template  struct object_creator {
+  static void *call() { return detail::createObject(); }
 };
 
 /// object_deleter - Helper method for ManagedStatic.
-///
 template  struct object_deleter {
-  static void call(void *Ptr) { delete (T *)Ptr; }
+  static void call(void *Ptr) {
+return detail::deleteObject(static_cast(Ptr));
+  }
 };
+
 template  struct object_deleter {
-  static void call(void *Ptr) { delete[](T *)Ptr; }
+  static void call(void *Ptr) {
+return detail::deleteObject(static_cast(Ptr));
+  }
 };
 
 // ManagedStatic must be initialized to zero, and it must *not* have a dynamic
Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -70,11 +70,23 @@
 template  class RefCountedBase {
   mutable unsigned RefCount = 0;
 
-public:
+protected:
   RefCountedBase() = default;
   RefCountedBase(const RefCountedBase &) {}
   RefCountedBase &operator=(const RefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~RefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~RefCountedBase() = default;
+#endif
+
+public:
   void Retain() const { ++RefCount; }
 
   void Release() const {
@@ -94,6 +106,17 @@
   ThreadSafeRefCountedBase &
   operator=(const ThreadSafeRefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~ThreadSafeRefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~ThreadSafeRefCountedBase() = default;
+#endif
+
 public:
   void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
 
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -156,9 +156,7 @@
 /// of cache hits.
 class TrueMatcherImpl : public DynMatcherInterface {
 public:
-  TrueMatcherImpl() {
-Retain(); // Reference count will never become zero.
-  }
+  TrueMatcherImpl() = default;
 
   bool dynMatches(const DynTypedNode &, ASTMatchFinder *,
   BoundNodesTreeBuilder *) const override {
@@ -191,6 +189,9 @@
   IntrusiveRefCntPtr InnerMatcher;
 };
 
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+
 } // namespace
 
 static llvm::ManagedStatic TrueMatcherInstance;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92650: [RISCV] Define preprocessor definitions for 'V' extension.

2020-12-04 Thread Hsiangkai Wang via Phabricator via cfe-commits

HsiangKai created this revision.
HsiangKai added reviewers: craig.topper, asb, luismarques, evandro.
Herald added subscribers: frasercrmck, NickHung, apazos, sameer.abuasal, 
pzheng, s.egerton, lenary, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar.
HsiangKai requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92650

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -79,6 +79,14 @@
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
 // CHECK-DOUBLE-NOT: __riscv_float_abi_single
 
+// RUN: %clang -target riscv32-unknown-linux-gnu 
-menable-experimental-extensions \
+// RUN:   -march=rv32iv0p9 -x c -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu 
-menable-experimental-extensions \
+// RUN:   -march=rv64iv0p9 -x c -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
+// CHECK-V-EXT: __riscv_vector 1
+//
 // RUN: %clang -target riscv32-unknown-linux-gnu 
-menable-experimental-extensions -march=rv32izfh0p1 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZFH-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu 
-menable-experimental-extensions -march=rv64izfh0p1 -x c -E -dM %s \
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -31,12 +31,13 @@
   bool HasD;
   bool HasC;
   bool HasB;
+  bool HasV;
   bool HasZfh;
 
 public:
   RISCVTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false), HasD(false),
-HasC(false), HasB(false), HasZfh(false) {
+HasC(false), HasB(false), HasV(false), HasZfh(false) {
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -136,6 +136,9 @@
   if (HasB)
 Builder.defineMacro("__riscv_bitmanip");
 
+  if (HasV)
+Builder.defineMacro("__riscv_vector");
+
   if (HasZfh)
 Builder.defineMacro("__riscv_zfh");
 }
@@ -153,6 +156,7 @@
   .Case("d", HasD)
   .Case("c", HasC)
   .Case("experimental-b", HasB)
+  .Case("experimental-v", HasV)
   .Case("experimental-zfh", HasZfh)
   .Default(false);
 }
@@ -173,6 +177,8 @@
   HasC = true;
 else if (Feature == "+experimental-b")
   HasB = true;
+else if (Feature == "+experimental-v")
+  HasV = true;
 else if (Feature == "+experimental-zfh")
   HasZfh = true;
   }


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -79,6 +79,14 @@
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
 // CHECK-DOUBLE-NOT: __riscv_float_abi_single
 
+// RUN: %clang -target riscv32-unknown-linux-gnu -menable-experimental-extensions \
+// RUN:   -march=rv32iv0p9 -x c -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -menable-experimental-extensions \
+// RUN:   -march=rv64iv0p9 -x c -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-V-EXT %s
+// CHECK-V-EXT: __riscv_vector 1
+//
 // RUN: %clang -target riscv32-unknown-linux-gnu -menable-experimental-extensions -march=rv32izfh0p1 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZFH-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -menable-experimental-extensions -march=rv64izfh0p1 -x c -E -dM %s \
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -31,12 +31,13 @@
   bool HasD;
   bool HasC;
   bool HasB;
+  bool HasV;
   bool HasZfh;
 
 public:
   RISCVTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false), HasD(false),
-HasC(false), HasB(false), HasZfh(false) {
+HasC(false), HasB(false), HasV(false), HasZfh(false) {
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/RISCV.c

[PATCH] D92650: [RISCV] Define preprocessor definitions for 'V' extension.

2020-12-04 Thread Sam Elliott via Phabricator via cfe-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92650

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

[PATCH] D92384: [AST][NFC] Silence GCC warning about broken strict aliasing rules

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92384

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

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: Eugene.Zelenko, aaron.ballman.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Using bools instead of integers better conveys the expected value of the option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92652

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-string-compare.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-definitions-in-headers.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-bind.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-pass-by-value.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-bool-literals.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-default.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-delete.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-noexcept.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-use-transparent-functors.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-move-const-arg.rst
  clang-tools-extra/docs/clang-tidy/checks/portability-simd-intrinsics.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-declaration.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst

Index: clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -52,5 +52,5 @@
 
 .. option:: IgnoreMacros
 
-   If this option is set to non-zero (default is `1`), the check will not warn
+   If this option is set to `true` (default is `true`), the check will not warn
about literal suffixes inside macros.
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simpli

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 309519.
njames93 added a comment.

Whoops missed one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92652

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-widening-cast.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-string-compare.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-definitions-in-headers.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-bind.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-pass-by-value.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-bool-literals.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-default-member-init.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-default.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-equals-delete.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-noexcept.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-use-transparent-functors.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-move-const-arg.rst
  clang-tools-extra/docs/clang-tidy/checks/portability-simd-intrinsics.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-declaration.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst

Index: clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -52,5 +52,5 @@
 
 .. option:: IgnoreMacros
 
-   If this option is set to non-zero (default is `1`), the check will not warn
+   If this option is set to `true` (default is `true`), the check will not warn
about literal suffixes inside macros.
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -77,10 +77,10 @@
 
 .. option:: ChainedConditionalReturn
 
-   If non-zero, conditional boolean return statements at the end of an
-   ``if/else if`` chain will

[clang] 0519e1d - [HIP] Fix bug in driver about wavefront size

2020-12-04 Thread Yaxun Liu via cfe-commits


Author: Yaxun (Sam) Liu
Date: 2020-12-04T08:36:52-05:00
New Revision: 0519e1ddb3885d070f054ca30a7487f915f6f795

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

LOG: [HIP] Fix bug in driver about wavefront size

The static variable causes it only initialized once and take
the same value for different GPU archs, whereas they
may be different for different GPU archs, e.g. when
there are both gfx900 and gfx1010.

Removing static fixes that.

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

Added: 
clang/test/Driver/hip-wavefront-size.hip

Modified: 
clang/lib/Driver/ToolChains/AMDGPU.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 5df7236f0223..1220594281ec 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -499,7 +499,7 @@ llvm::DenormalMode 
AMDGPUToolChain::getDefaultDenormalModeForType(
 bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList &DriverArgs,
llvm::AMDGPU::GPUKind Kind) {
   const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
-  static bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
+  bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
 
   return !HasWave32 || DriverArgs.hasFlag(
 options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);

diff  --git a/clang/test/Driver/hip-wavefront-size.hip 
b/clang/test/Driver/hip-wavefront-size.hip
new file mode 100644
index ..dd7ca16ae2d3
--- /dev/null
+++ b/clang/test/Driver/hip-wavefront-size.hip
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver,amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE64
+// WAVE64: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} 
"-target-cpu" "gfx900"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE32
+// WAVE32: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} 
"-target-cpu" "gfx1010"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=BOTH
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} 
"-target-cpu" "gfx900"
+// BOTH-DAG: "-mlink-builtin-bitcode" 
"{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"



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

[PATCH] D92628: [HIP] Fix bug in driver about wavefront size

2020-12-04 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0519e1ddb388: [HIP] Fix bug in driver about wavefront size 
(authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92628

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/hip-wavefront-size.hip


Index: clang/test/Driver/hip-wavefront-size.hip
===
--- /dev/null
+++ clang/test/Driver/hip-wavefront-size.hip
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver,amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE64
+// WAVE64: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} 
"-target-cpu" "gfx900"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE32
+// WAVE32: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} 
"-target-cpu" "gfx1010"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=BOTH
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} 
"-target-cpu" "gfx900"
+// BOTH-DAG: "-mlink-builtin-bitcode" 
"{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -499,7 +499,7 @@
 bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList &DriverArgs,
llvm::AMDGPU::GPUKind Kind) {
   const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
-  static bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
+  bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
 
   return !HasWave32 || DriverArgs.hasFlag(
 options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);


Index: clang/test/Driver/hip-wavefront-size.hip
===
--- /dev/null
+++ clang/test/Driver/hip-wavefront-size.hip
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver,amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE64
+// WAVE64: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} "-target-cpu" "gfx900"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=WAVE32
+// WAVE32: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx1010 \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
+// RUN:   2>&1 | FileCheck %s --check-prefixes=BOTH
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} "-target-cpu" "gfx900"
+// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -499,7 +499,7 @@
 bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList &DriverArgs,
llvm::AMDGPU::GPUKind Kind) {
   const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
-  static bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
+  bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
 
   return !HasWave32 || DriverArgs.hasFlag(
 options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:276
+  // Never ignore these.
+} else if (!Context.isCheckEnabled(Error.DiagnosticName) &&
+   Error.DiagLevel != ClangTidyError::Error) {

aaron.ballman wrote:
> Perhaps this is a bad idea, but would it make sense to have 
> `isCheckEnabled()` report `true` for `clang-tidy-config`? It's not really a 
> check, so that's a bit odd, but it seems like anywhere we're testing this 
> property we wouldn't want to ignore config issues?
I didn't personally think that was a good way to go. This is the only real 
place its used where you would get isCheckEnabled called with 
`clang-tidy-config`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91885

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

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 309521.
njames93 added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91885

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -166,6 +166,10 @@
   ClangTidyOptions Options;
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
+  ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
+   &DiagConsumer, false);
+  Context.setDiagnosticsEngine(&DE);
   TestCheck TestCheck(&Context);
   CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError,
   "option not found 'test.Opt'");
@@ -191,6 +195,10 @@
 
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
+  ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
+   &DiagConsumer, false);
+  Context.setDiagnosticsEngine(&DE);
   TestCheck TestCheck(&Context);
 
 #define CHECK_ERROR_INT(Name, Expected)\
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -10,7 +10,9 @@
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context) {
+diag("DiagWithNoLoc");
+  }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override {
 Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
   }
@@ -26,9 +28,10 @@
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", &Errors);
-  EXPECT_EQ(2ul, Errors.size());
-  EXPECT_EQ("type specifier", Errors[0].Message.Message);
-  EXPECT_EQ("variable", Errors[1].Message.Message);
+  EXPECT_EQ(3ul, Errors.size());
+  EXPECT_EQ("DiagWithNoLoc", Errors[0].Message.Message);
+  EXPECT_EQ("type specifier", Errors[1].Message.Message);
+  EXPECT_EQ("variable", Errors[2].Message.Message);
 }
 
 } // namespace test
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
@@ -5,11 +5,11 @@
 // RUN:   {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
 // RUN:   {key: readability-identifier-naming.StructCase, value: CAMEL}, \
 // RUN:   {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
-// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
 
-// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
-// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
+// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'? [clang-tidy-config]
+// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.Clas

[PATCH] D92480: [llvm] Add asserts in (ThreadSafe)?RefCountedBase destructors

2020-12-04 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 309524.
njames93 added a comment.

Remove unnecessary comment added in ASTMatchersInternal.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92480

Files:
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
  llvm/include/llvm/Support/ManagedStatic.h

Index: llvm/include/llvm/Support/ManagedStatic.h
===
--- llvm/include/llvm/Support/ManagedStatic.h
+++ llvm/include/llvm/Support/ManagedStatic.h
@@ -15,21 +15,79 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 
+namespace detail {
+// Helper struct to detect classes that has Retain and Release member functions
+// that are const with no params.
+template  struct HasRetainRelease {
+private:
+  template 
+  static constexpr decltype(std::declval().Retain(),
+std::declval().Release(), bool())
+  test(int) {
+return true;
+  }
+
+  template  static constexpr bool test(...) { return false; }
+
+public:
+  static constexpr bool Value = test(int());
+};
+
+template 
+std::enable_if_t::Value, void *> createObject() {
+  return new T();
+}
+
+template 
+std::enable_if_t::Value, void *> createObject() {
+  T *Ptr = new T();
+  Ptr->Retain();
+  return Ptr;
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  delete Ptr;
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  Ptr->Release();
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  delete[] Ptr;
+}
+
+template 
+std::enable_if_t::Value> deleteObject(T *Ptr) {
+  for (auto I = Ptr, E = I + N; I != E; ++I)
+I->Release();
+}
+} // namespace detail
+
 /// object_creator - Helper method for ManagedStatic.
-template  struct object_creator {
-  static void *call() { return new C(); }
+
+template  struct object_creator {
+  static void *call() { return detail::createObject(); }
 };
 
 /// object_deleter - Helper method for ManagedStatic.
-///
 template  struct object_deleter {
-  static void call(void *Ptr) { delete (T *)Ptr; }
+  static void call(void *Ptr) {
+return detail::deleteObject(static_cast(Ptr));
+  }
 };
+
 template  struct object_deleter {
-  static void call(void *Ptr) { delete[](T *)Ptr; }
+  static void call(void *Ptr) {
+return detail::deleteObject(static_cast(Ptr));
+  }
 };
 
 // ManagedStatic must be initialized to zero, and it must *not* have a dynamic
Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -70,11 +70,23 @@
 template  class RefCountedBase {
   mutable unsigned RefCount = 0;
 
-public:
+protected:
   RefCountedBase() = default;
   RefCountedBase(const RefCountedBase &) {}
   RefCountedBase &operator=(const RefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~RefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~RefCountedBase() = default;
+#endif
+
+public:
   void Retain() const { ++RefCount; }
 
   void Release() const {
@@ -94,6 +106,17 @@
   ThreadSafeRefCountedBase &
   operator=(const ThreadSafeRefCountedBase &) = delete;
 
+#ifndef NDEBUG
+  ~ThreadSafeRefCountedBase() {
+assert(RefCount == 0 &&
+   "Destruction occured when there are still references to this.");
+  }
+#else
+  // Default the destructor in release builds, A trivial destructor may enable
+  // better codegen.
+  ~ThreadSafeRefCountedBase() = default;
+#endif
+
 public:
   void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
 
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -156,9 +156,7 @@
 /// of cache hits.
 class TrueMatcherImpl : public DynMatcherInterface {
 public:
-  TrueMatcherImpl() {
-Retain(); // Reference count will never become zero.
-  }
+  TrueMatcherImpl() = default;
 
   bool dynMatches(const DynTypedNode &, ASTMatchFinder *,
   BoundNodesTreeBuilder *) const override {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread Mitchell via Phabricator via cfe-commits

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.

I am in favor of landing this and iterating.


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

https://reviews.llvm.org/D79773

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius accepted this revision.
curdeius added a comment.

LGTM.


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

https://reviews.llvm.org/D79773

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

[clang] 507bbc4 - [AST][NFC] Silence GCC warning about broken strict aliasing rules

2020-12-04 Thread Thomas Preud'homme via cfe-commits


Author: Thomas Preud'homme
Date: 2020-12-04T14:34:51Z
New Revision: 507bbc45bba90fab3f1a2b42e94ae4fbebdd6498

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

LOG: [AST][NFC] Silence GCC warning about broken strict aliasing rules

The deserialize() method would trigger the following warning on GCC <7:

   warning: dereferencing type-punned pointer will break
   strict-aliasing rules [-Wstrict-aliasing]

   ParamIdx P(*reinterpret_cast(&S));
  ^

&S was previously reinterpret_casted from a ParamIdx into a SerialType,
it is therefore safe to cast back into a ParamIdx. Similar to what was
done in D50608, we replace it with two static_cast via void * which
silences the warning and presumably makes GCC understand that no
strict-aliasing violation is happening.

No functional change intended.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/AST/Attr.h

Removed: 




diff  --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 545dc992c5ed..8d9fb8f2bf27 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -259,7 +259,10 @@ class ParamIdx {
 
   /// Construct from a result from \c serialize.
   static ParamIdx deserialize(SerialType S) {
-ParamIdx P(*reinterpret_cast(&S));
+// Using this two-step static_cast via void * instead of reinterpret_cast
+// silences a -Wstrict-aliasing false positive from GCC7 and earlier.
+void *ParamIdxPtr = static_cast(&S);
+ParamIdx P(*static_cast(ParamIdxPtr));
 assert((!P.IsValid || P.Idx >= 1) && "valid Idx must be one-origin");
 return P;
   }



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

[PATCH] D92384: [AST][NFC] Silence GCC warning about broken strict aliasing rules

2020-12-04 Thread Thomas Preud'homme via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG507bbc45bba9: [AST][NFC] Silence GCC warning about broken 
strict aliasing rules (authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92384

Files:
  clang/include/clang/AST/Attr.h


Index: clang/include/clang/AST/Attr.h
===
--- clang/include/clang/AST/Attr.h
+++ clang/include/clang/AST/Attr.h
@@ -259,7 +259,10 @@
 
   /// Construct from a result from \c serialize.
   static ParamIdx deserialize(SerialType S) {
-ParamIdx P(*reinterpret_cast(&S));
+// Using this two-step static_cast via void * instead of reinterpret_cast
+// silences a -Wstrict-aliasing false positive from GCC7 and earlier.
+void *ParamIdxPtr = static_cast(&S);
+ParamIdx P(*static_cast(ParamIdxPtr));
 assert((!P.IsValid || P.Idx >= 1) && "valid Idx must be one-origin");
 return P;
   }


Index: clang/include/clang/AST/Attr.h
===
--- clang/include/clang/AST/Attr.h
+++ clang/include/clang/AST/Attr.h
@@ -259,7 +259,10 @@
 
   /// Construct from a result from \c serialize.
   static ParamIdx deserialize(SerialType S) {
-ParamIdx P(*reinterpret_cast(&S));
+// Using this two-step static_cast via void * instead of reinterpret_cast
+// silences a -Wstrict-aliasing false positive from GCC7 and earlier.
+void *ParamIdxPtr = static_cast(&S);
+ParamIdx P(*static_cast(ParamIdxPtr));
 assert((!P.IsValid || P.Idx >= 1) && "valid Idx must be one-origin");
 return P;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92269: [TableGen] Eliminate the 'code' type

2020-12-04 Thread Paul C. Anagnostopoulos via Phabricator via cfe-commits

Paul-C-Anagnostopoulos added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92269

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

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

2020-12-04 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D90157#2361773 , @ASDenysPetrov 
wrote:

> Who can confirm if this is correct or somewhere it needs fixes? Here is a 
> generated result of `evalCast` from the origin branch(before the patch):
>
>   void foo(int* x,  // &SymRegion{reg_$0}
>int** y, // &SymRegion{reg_$1}
>int***z) // &SymRegion{reg_$2}
>   {
>(void*)x;// &SymRegion{reg_$0}
>(void*)y;// &SymRegion{reg_$1}
>(void*)z;// &SymRegion{reg_$2}
>(void**)x;   // &Element{SymRegion{reg_$0},0 S64b,void *}
>(void**)y;   // &SymRegion{reg_$1}
>(void**)z;   // &SymRegion{reg_$2}
>(void***)x;  // &Element{SymRegion{reg_$0},0 S64b,void **}
>(void***)y;  // &Element{SymRegion{reg_$1},0 S64b,void **}
>(void***)z;  // &SymRegion{reg_$2}
>(void)x; // &Element{SymRegion{reg_$0},0 S64b,void ***}
>(void)y; // &Element{SymRegion{reg_$1},0 S64b,void ***}
>(void)z; // &Element{SymRegion{reg_$2},0 S64b,void ***}
>   }

AFAIK, Element regions represent the reinterpret casts, and static-casts 
doesn't require anything to do as the loading from the region would be cast to 
the appropriate type.
So these dumps seem to be correct.

BTW, this dispatching logic should be done with an SValVisitor IMO. That way 
you could make it even more cleaner. @ASDenysPetrov

> @steakhal
>
>> By the same token, I think you should run the test-suite using the Z3 
>> refutation and/or Z3 constraint solver to check if this introduces any 
>> regression.
>
> Never worked with Z3 before. I'll try.



---

I've run the baseline and your patch as well on 15 projects, both with and 
without Z3 refutation enabled.

The reports for these 13 projects did not change at all:

  Bitcoin_v0.20.1
  Curl_curl-7_66_0
  Memchached_1.6.8
  OpenSSL_openssl-3.0.0-alpha7
  PostgreSQL_REL_13_0
  Redis_5.0.6
  SQLite_version-3.33.0
  TinyXML2_8.0.0
  Vim_v8.2.1920
  Xerces_v3.2.3
  libWebM_libwebm-1.0.0.27
  protobuf_v3.13.0
  tmux_3.0

However, for `twin`, it introduced a new unique report:

  server/socketalien.h @ Line 64Access out-of-bound array element 
(buffer overflow) alpha.security.ArrayBound

And for the `FFMPEG`, 2 unique reports were lost without Z3 refutation:

  vf_edgedetect.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  mpc8.cAccess out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound

3 unique reports were lost with Z3 refutation:

  utvideodec.c  7th function call argument is an uninitialized value
core.CallAndMessage
  utvideodec.c  The left operand of '-' is a garbage value  
core.UndefinedBinaryOperatorResult
  xan.c Arguments must not be overlapping buffers   
alpha.unix.cstring.BufferOverlap

And introduced 35 unique new ones without Z3 refutation, only 34 with 
refutation:

  lpc.c Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound   (Z3 refutation filters this)
  on2avc.c  Memory copy function accesses out-of-bound array element
alpha.unix.cstring.OutOfBounds
  bytestream.h  Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  ws-snd1.c Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bound array element (buffer overflow) 
alpha.security.ArrayBound
  h264qpel_template.c   Access out-of-bo

[PATCH] D92655: [OPENMP]Fix PR48387: disable warning messages caused by internal conversions.

2020-12-04 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev created this revision.
ABataev added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.
Herald added a project: clang.

Compiler needs to convert some of the loop iteration
variables/conditions to different types for better codegen and it may
lead to spurious warning messages about implicit signed/unsigned
conversions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92655

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


Index: clang/test/OpenMP/for_ast_print.cpp
===
--- clang/test/OpenMP/for_ast_print.cpp
+++ clang/test/OpenMP/for_ast_print.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s -Wsign-conversion | 
FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify 
%s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s -Wsign-conversion | 
FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only 
-verify %s -ast-print | FileCheck %s
 // expected-no-diagnostics
@@ -223,9 +223,9 @@
 // CHECK: static int a;
 #pragma omp for schedule(guided, argc) reduction(+:argv[0][:1]) 
order(concurrent)
   // CHECK-NEXT: #pragma omp for schedule(guided, argc) reduction(+: 
argv[0][:1]) order(concurrent)
-  for (int i = 0; i < 2; ++i)
+  for (int i = argc; i < c; ++i)
 a = 2;
-// CHECK-NEXT: for (int i = 0; i < 2; ++i)
+// CHECK-NEXT: for (int i = argc; i < c; ++i)
 // CHECK-NEXT: a = 2;
 #pragma omp parallel
 #pragma omp for private(argc, b), firstprivate(argv, c), lastprivate(d, f) 
collapse(3) schedule(auto) ordered nowait linear(g:-1) reduction(task, +:e)
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4196,6 +4196,7 @@
   if (!WithInit)
 CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C));
   S.CurContext->addHiddenDecl(CED);
+  Sema::TentativeAnalysisScope Trap(S);
   S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false);
   return CED;
 }
@@ -7580,6 +7581,7 @@
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);
 
+  Sema::TentativeAnalysisScope Trap(SemaRef);
   Diff = SemaRef.ActOnFinishFullExpr(Diff.get(), /*DiscardedValue=*/false);
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);


Index: clang/test/OpenMP/for_ast_print.cpp
===
--- clang/test/OpenMP/for_ast_print.cpp
+++ clang/test/OpenMP/for_ast_print.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s -Wsign-conversion | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s -Wsign-conversion | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 // expected-no-diagnostics
@@ -223,9 +223,9 @@
 // CHECK: static int a;
 #pragma omp for schedule(guided, argc) reduction(+:argv[0][:1]) order(concurrent)
   // CHECK-NEXT: #pragma omp for schedule(guided, argc) reduction(+: argv[0][:1]) order(concurrent)
-  for (int i = 0; i < 2; ++i)
+  for (int i = argc; i < c; ++i)
 a = 2;
-// CHECK-NEXT: for (int i = 0; i < 2; ++i)
+// CHECK-NEXT: for (int i = argc; i < c; ++i)
 // CHECK-NEXT: a = 2;
 #pragma omp parallel
 #pragma omp for private(argc, b), firstprivate(argv, c), lastprivate(d, f) collapse(3) schedule(auto) ordered nowait linear(g:-1) reduction(task, +:e)
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4196,6 +4196,7 @@
   if (!WithInit)
 CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C));
   S.CurContext->addHiddenDecl(CED);
+  Sema::TentativeAnalysisScope Trap(S);
   S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false);
   return CED;
 }
@@ -7580,6 +7581,7 @@
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);
 
+  Sema::TentativeAnalysisScope Trap(SemaRef);
   Diff = SemaRef.ActOnFinishFullExpr(Diff.get(), /*DiscardedValue=*/false);
   if (!Diff.isUsable())
 return std::make_pair

[clang] 090dd64 - [Sema] Fold VLAs to constant arrays in a few more contexts

2020-12-04 Thread Erik Pilkington via cfe-commits


Author: Erik Pilkington
Date: 2020-12-04T10:03:23-05:00
New Revision: 090dd647d98dc50a56a42fbba0f3f11b10a3a187

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

LOG: [Sema] Fold VLAs to constant arrays in a few more contexts

552c6c2 removed support for promoting VLAs to constant arrays when the bounds
isn't an ICE, since this can result in miscompiling a conforming program that
assumes that the array is a VLA. Promoting VLAs for fields is still supported,
since clang doesn't support VLAs in fields, so no conforming program could have
a field VLA.

This change is really disruptive, so this commit carves out two more cases
where we promote VLAs which can't miscompile a conforming program:

 - When the VLA appears in an ivar -- this seems like a corollary to the field 
thing
 - When the VLA has an initializer -- VLAs can't have an initializer

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

Added: 
clang/test/SemaObjC/variable-size-ivar.m

Modified: 
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/CXX/basic/basic.types/p10.cpp
clang/test/Sema/decl-in-prototype.c
clang/test/Sema/vla.c
clang/test/SemaCXX/vla.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/DeclSpec.h 
b/clang/include/clang/Sema/DeclSpec.h
index afcbbaa5cfa7..fb30f95f5914 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1844,6 +1844,9 @@ class Declarator {
   /// Indicates whether the InlineParams / InlineBindings storage has been 
used.
   unsigned InlineStorageUsed : 1;
 
+  /// Indicates whether this declarator has an initializer.
+  unsigned HasInitializer : 1;
+
   /// Attrs - Attributes.
   ParsedAttributes Attrs;
 
@@ -1892,8 +1895,8 @@ class Declarator {
FunctionDefinitionKind::Declaration)),
 Redeclaration(false), Extension(false), ObjCIvar(false),
 ObjCWeakProperty(false), InlineStorageUsed(false),
-Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr),
-TrailingRequiresClause(nullptr),
+HasInitializer(false), Attrs(ds.getAttributePool().getFactory()),
+AsmLabel(nullptr), TrailingRequiresClause(nullptr),
 InventedTemplateParameterList(nullptr) {}
 
   ~Declarator() {
@@ -1976,6 +1979,7 @@ class Declarator {
 Attrs.clear();
 AsmLabel = nullptr;
 InlineStorageUsed = false;
+HasInitializer = false;
 ObjCIvar = false;
 ObjCWeakProperty = false;
 CommaLoc = SourceLocation();
@@ -2574,6 +2578,9 @@ class Declarator {
 return (FunctionDefinitionKind)FunctionDefinition;
   }
 
+  void setHasInitializer(bool Val = true) { HasInitializer = Val; }
+  bool hasInitializer() const { return HasInitializer; }
+
   /// Returns true if this declares a real member and not a friend.
   bool isFirstDeclarationOfMember() {
 return getContext() == DeclaratorContext::Member &&

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 277a67f3513f..c3c56ddccb6b 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2192,7 +2192,22 @@ Decl 
*Parser::ParseDeclarationAfterDeclaratorAndAttributes(
 }
   };
 
-  // Inform the current actions module that we just parsed this declarator.
+  enum class InitKind { Uninitialized, Equal, CXXDirect, CXXBraced };
+  InitKind TheInitKind;
+  // If a '==' or '+=' is found, suggest a fixit to '='.
+  if (isTokenEqualOrEqualTypo())
+TheInitKind = InitKind::Equal;
+  else if (Tok.is(tok::l_paren))
+TheInitKind = InitKind::CXXDirect;
+  else if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace) &&
+   (!CurParsedObjCImpl || !D.isFunctionDeclarator()))
+TheInitKind = InitKind::CXXBraced;
+  else
+TheInitKind = InitKind::Uninitialized;
+  if (TheInitKind != InitKind::Uninitialized)
+D.setHasInitializer();
+
+  // Inform Sema that we just parsed this declarator.
   Decl *ThisDecl = nullptr;
   Decl *OuterDecl = nullptr;
   switch (TemplateInfo.Kind) {
@@ -2254,9 +2269,9 @@ Decl 
*Parser::ParseDeclarationAfterDeclaratorAndAttributes(
 }
   }
 
+  switch (TheInitKind) {
   // Parse declarator '=' initializer.
-  // If a '==' or '+=' is found, suggest a fixit to '='.
-  if (isTokenEqualOrEqualTypo()) {
+  case InitKind::Equal: {
 SourceLocation EqualLoc = ConsumeToken();
 
 if (Tok.is(tok::kw_delete)) {
@@ -2311,7 +2326,9 @@ Decl 
*Parser::ParseDeclarationAfterDeclaratorAndAttributes(
 Actions.AddInitializerToDecl(ThisDecl, Init.get(),
  /*DirectInit=*/false);
 }
-  } else if (Tok.is(tok::l_paren)) {
+break;
+  }
+  case InitKind::CXXDirect: {
 // Parse C++ direct initializer: '(' ex

[PATCH] D90871: [Sema] Fold VLAs to constant arrays in a few more contexts

2020-12-04 Thread Erik Pilkington via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
erik.pilkington marked 3 inline comments as done.
Closed by commit rG090dd647d98d: [Sema] Fold VLAs to constant arrays in a few 
more contexts (authored by erik.pilkington).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D90871?vs=305506&id=309532#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90871

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/basic/basic.types/p10.cpp
  clang/test/Sema/decl-in-prototype.c
  clang/test/Sema/vla.c
  clang/test/SemaCXX/vla.cpp
  clang/test/SemaObjC/variable-size-ivar.m

Index: clang/test/SemaObjC/variable-size-ivar.m
===
--- /dev/null
+++ clang/test/SemaObjC/variable-size-ivar.m
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+const int ksize = 42;
+int size = 42;
+
+@interface X
+{
+  int arr1[ksize]; // expected-warning{{variable length array folded to constant array}}
+  int arr2[size]; // expected-error{{instance variables must have a constant size}}
+  int arr3[ksize-43]; // expected-error{{array size is negative}}
+}
+@end
Index: clang/test/SemaCXX/vla.cpp
===
--- clang/test/SemaCXX/vla.cpp
+++ clang/test/SemaCXX/vla.cpp
@@ -20,3 +20,9 @@
 
 void pr23151(int (&)[*]) { // expected-error {{variable length array must be bound in function definition}}
 }
+
+void test_fold() {
+  char a1[(unsigned long)(int *)0+1]{}; // expected-warning{{variable length array folded to constant array as an extension}}
+  char a2[(unsigned long)(int *)0+1] = {}; // expected-warning{{variable length array folded to constant array as an extension}}
+  char a3[(unsigned long)(int *)0+1];
+}
Index: clang/test/Sema/vla.c
===
--- clang/test/Sema/vla.c
+++ clang/test/Sema/vla.c
@@ -100,3 +100,33 @@
 typedef struct {
   char c[pr44406_a]; // expected-warning {{folded to constant array as an extension}}
 } pr44406_s;
+
+void test_fold_to_constant_array() {
+  const int ksize = 4;
+
+  goto jump_over_a1; // expected-error{{cannot jump from this goto statement to its label}}
+  char a1[ksize]; // expected-note{{variable length array}}
+ jump_over_a1:;
+
+  goto jump_over_a2;
+  char a2[ksize] = "foo"; // expected-warning{{variable length array folded to constant array as an extension}}
+ jump_over_a2:;
+
+  goto jump_over_a3;
+  char a3[ksize] = {}; // expected-warning {{variable length array folded to constant array as an extension}} expected-warning{{use of GNU empty initializer}}
+ jump_over_a3:;
+
+  goto jump_over_a4; // expected-error{{cannot jump from this goto statement to its label}}
+  char a4[ksize][2]; // expected-note{{variable length array}}
+ jump_over_a4:;
+
+  char a5[ksize][2] = {}; // expected-warning {{variable length array folded to constant array as an extension}} expected-warning{{use of GNU empty initializer}}
+
+  int a6[ksize] = {1,2,3,4}; // expected-warning{{variable length array folded to constant array as an extension}}
+
+  // expected-warning@+1{{variable length array folded to constant array as an extension}}
+  int a7[ksize] __attribute__((annotate("foo"))) = {1,2,3,4};
+
+  // expected-warning@+1{{variable length array folded to constant array as an extension}}
+  char a8[2][ksize] = {{1,2,3,4},{4,3,2,1}};
+}
Index: clang/test/Sema/decl-in-prototype.c
===
--- clang/test/Sema/decl-in-prototype.c
+++ clang/test/Sema/decl-in-prototype.c
@@ -49,7 +49,7 @@
 // function.
 enum { BB = 0 };
 void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
-  SA(1, AA == 5); // expected-error {{variable-sized object may not be initialized}}
+  SA(1, AA == 5); // expected-warning{{variable length array folded to constant array as an extension}}
   SA(2, BB == 0);
 }
 
Index: clang/test/CXX/basic/basic.types/p10.cpp
===
--- clang/test/CXX/basic/basic.types/p10.cpp
+++ clang/test/CXX/basic/basic.types/p10.cpp
@@ -140,8 +140,7 @@
   int a[n]; // expected-error {{variable of non-literal type 'int [n]' cannot be defined in a constexpr function}}
 }
 // expected-warning@+1 {{variable length array folded to constant array as an extension}}
-constexpr long Overflow[ // expected-error {{constexpr variable cannot have non-literal type 'long const[(1 << 30) << 2]'}}
-(1 << 30) << 2]{};   // expected-warning {{requires 34 bits to represent}}
+constexpr long Overflow[(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}}
 
 namespace inherited_ctor {
   struct A { constexpr A(int); };
Index: clang/lib/Sema/SemaDecl.cpp
==

[PATCH] D92655: [OPENMP]Fix PR48387: disable warning messages caused by internal conversions.

2020-12-04 Thread Johannes Doerfert via Phabricator via cfe-commits

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92655

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

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

2020-12-04 Thread Eugene Zelenko via Phabricator via cfe-commits

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

Looks OK for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92652

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

[PATCH] D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10

2020-12-04 Thread Albion Fung via Phabricator via cfe-commits

Conanap updated this revision to Diff 309534.
Conanap marked 4 inline comments as done.
Conanap added a comment.

Removed braces for single lines


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

https://reviews.llvm.org/D90173

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
  llvm/test/CodeGen/PowerPC/p10-splatImm32.ll

Index: llvm/test/CodeGen/PowerPC/p10-splatImm32.ll
===
--- llvm/test/CodeGen/PowerPC/p10-splatImm32.ll
+++ llvm/test/CodeGen/PowerPC/p10-splatImm32.ll
@@ -118,3 +118,25 @@
   %vecins1 = shufflevector <4 x i32> , <4 x i32> %a, <4 x i32> 
   ret <4 x i32> %vecins1
 }
+
+define dso_local <2 x double> @test_xxsplti32dx_8() {
+; CHECK-LABEL: test_xxsplti32dx_8
+; CHECK-LE: xxlxor vs34, vs34, vs34
+; CHECK-LE: xxsplti32dx vs34, 1, 1082660167
+; CHECK-BE: xxlxor vs34, vs34, vs34
+; CHECK-BE: xxsplti32dx vs34, 0, 1082660167
+; CHECK: blr
+entry:
+  ret <2 x double> 
+}
+
+define dso_local <8 x i16> @test_xxsplti32dx_9() {
+; CHECK-LABEL: test_xxsplti32dx_9
+; CHECK-LE: xxlxor vs34, vs34, vs34
+; CHECK-LE: xxsplti32dx vs34, 1, 23855277
+; CHECK-BE: xxlxor vs34, vs34, vs34
+; CHECK-BE: xxsplti32dx vs34, 0, 19070977
+; CHECK: blr
+entry:
+  ret <8 x i16> 
+}
Index: llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
===
--- llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
+++ llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
@@ -1,114 +1,216 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O2 \
-; RUN: -ppc-asm-full-reg-names -mcpu=pwr10 < %s | FileCheck %s
+; RUN: -ppc-asm-full-reg-names -mcpu=pwr10 < %s | FileCheck %s --check-prefixes=CHECK-LE
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -O2 \
 ; RUN: -ppc-asm-full-reg-names -mcpu=pwr10 < %s | FileCheck %s \
-; RUN: --check-prefix=CHECK-NOPCREL
+; RUN: --check-prefixes=CHECK-NOPCREL-BE
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O2 \
 ; RUN: -mattr=-pcrelative-memops -ppc-asm-full-reg-names -mcpu=pwr10 < %s | \
-; RUN: FileCheck %s --check-prefix=CHECK-NOPCREL
+; RUN: FileCheck %s --check-prefixes=CHECK-NOPCREL-LE
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O2 \
 ; RUN: -mattr=-prefix-instrs -ppc-asm-full-reg-names -mcpu=pwr10 < %s | \
-; RUN: FileCheck %s --check-prefix=CHECK-NOPCREL
+; RUN: FileCheck %s --check-prefixes=CHECK-NOPREFIX
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -O2 \
 ; RUN: -ppc-asm-full-reg-names -target-abi=elfv2 -mcpu=pwr10 < %s | \
-; RUN: FileCheck %s
+; RUN: FileCheck %s --check-prefixes=CHECK-BE
 
 define dso_local <2 x double> @testDoubleToDoubleFail() local_unnamed_addr {
-; CHECK-LABEL: testDoubleToDoubleFail:
-; CHECK:   # %bb.0: # %entry
-; CHECK-NEXT:plxv vs34, .LCPI0_0@PCREL(0), 1
-; CHECK-NEXT:blr
-;
-; CHECK-NOPCREL-LABEL: testDoubleToDoubleFail:
-; CHECK-NOPCREL:   # %bb.0: # %entry
-; CHECK-NOPCREL-NEXT:addis r3, r2, .LCPI0_0@toc@ha
-; CHECK-NOPCREL-NEXT:addi r3, r3, .LCPI0_0@toc@l
-; CHECK-NOPCREL-NEXT:lxvx vs34, 0, r3
-; CHECK-NOPCREL-NEXT:blr
-
+; CHECK-LE-LABEL: testDoubleToDoubleFail:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:xxlxor vs34, vs34, vs34
+; CHECK-LE-NEXT:xxsplti32dx vs34, 1, 1081435463
+; CHECK-LE-NEXT:blr
+;
+; CHECK-NOPCREL-BE-LABEL: testDoubleToDoubleFail:
+; CHECK-NOPCREL-BE:   # %bb.0: # %entry
+; CHECK-NOPCREL-BE-NEXT:xxlxor vs34, vs34, vs34
+; CHECK-NOPCREL-BE-NEXT:xxsplti32dx vs34, 0, 1081435463
+; CHECK-NOPCREL-BE-NEXT:blr
+;
+; CHECK-NOPCREL-LE-LABEL: testDoubleToDoubleFail:
+; CHECK-NOPCREL-LE:   # %bb.0: # %entry
+; CHECK-NOPCREL-LE-NEXT:xxlxor vs34, vs34, vs34
+; CHECK-NOPCREL-LE-NEXT:xxsplti32dx vs34, 1, 1081435463
+; CHECK-NOPCREL-LE-NEXT:blr
+;
+; CHECK-NOPREFIX-LABEL: testDoubleToDoubleFail:
+; CHECK-NOPREFIX:   # %bb.0: # %entry
+; CHECK-NOPREFIX-NEXT:addis r3, r2, .LCPI0_0@toc@ha
+; CHECK-NOPREFIX-NEXT:addi r3, r3, .LCPI0_0@toc@l
+; CHECK-NOPREFIX-NEXT:lxvx vs34, 0, r3
+; CHECK-NOPREFIX-NEXT:blr
+;
+; CHECK-BE-LABEL: testDoubleToDoubleFail:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:xxlxor vs34, vs34, vs34
+; CHECK-BE-NEXT:xxsplti32dx vs34, 0, 1081435463
+; CHECK-BE-NEXT:blr
 entry:
   ret <2 x double> 
 }
 
 define dso_local <2 x double> @testFloatDenormToDouble() local_unnamed_addr {
-; CHECK-LABEL: testFloatDenormToDouble:
-; CHECK:   # %bb.0: # %entry
-; CHECK-NEXT:plxv vs34, .LCPI1_0@PCREL(0), 1
-; CHECK-NEXT:blr
-;
-; CHECK-NOPCREL-LABEL: testFloatDenormToDouble:
-; CHECK-NOPCREL:   # %bb.0: #

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

2020-12-04 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru added a comment.

Thanks for doing that :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92652

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

[PATCH] D92658: [libTooling] Add `formatNode` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel created this revision.
ymandel added a reviewer: tdl-g.
ymandel requested review of this revision.
Herald added a project: clang.

This new stencil combinator is intended for use in diagnostics and the like.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92658

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -55,14 +55,14 @@
 // `StatementCode` may contain other statements not described by `Matcher`.
 static llvm::Optional matchStmt(StringRef StatementCode,
StatementMatcher Matcher) {
-  auto AstUnit = tooling::buildASTFromCode(wrapSnippet(StatementCode));
+  auto AstUnit = tooling::buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
+   {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
 ADD_FAILURE() << "AST construction failed";
 return llvm::None;
   }
   ASTContext &Context = AstUnit->getASTContext();
-  auto Matches = ast_matchers::match(
-  traverse(ast_type_traits::TK_AsIs, wrapMatcher(Matcher)), Context);
+  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();
@@ -365,6 +365,50 @@
   EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("field"));
 }
 
+TEST_F(StencilTest, FormatNodeStmt) {
+  std::string Snippet = "int *x; if (x == nullptr) (void)*x;";
+  std::string Expected = "if (x == nullptr)\n(void)*x;\n";
+  auto StmtMatch = matchStmt(Snippet, ifStmt().bind("id"));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(formatNode("id")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, FormatNodeExpr) {
+  StringRef Id = "id";
+  testExpr(Id, "int *x; (*x + 1);", formatNode(Id), "(*x + 1)");
+}
+
+TEST_F(StencilTest, FormatNodeType) {
+  std::string Snippet = "int *x; x;";
+  std::string Expected = "int *";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(formatNode("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, FormatNodeSugaredType) {
+  std::string Snippet = "using Ty = int; Ty *x; x;";
+  std::string Expected = "Ty *";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(formatNode("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, FormatNodeDecl) {
+  std::string Snippet = "int *x; x;";
+  std::string Expected = "int *x";
+  auto StmtMatch =
+  matchStmt(Snippet, declStmt(hasSingleDecl(decl().bind("decl";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(formatNode("decl")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
 TEST_F(StencilTest, RunOp) {
   StringRef Id = "id";
   auto SimpleFn = [Id](const MatchResult &R) {
@@ -448,6 +492,12 @@
   EXPECT_EQ(S->toString(), Expected);
 }
 
+TEST(StencilToStringTest, FormatNodeOp) {
+  auto S = formatNode("Id");
+  StringRef Expected = R"repr(formatNode("Id"))repr";
+  EXPECT_EQ(S->toString(), Expected);
+}
+
 TEST(StencilToStringTest, DebugPrintNodeOp) {
   auto S = dPrint("Id");
   StringRef Expected = R"repr(dPrint("Id"))repr";
Index: clang/lib/Tooling/Transformer/Stencil.cpp
===
--- clang/lib/Tooling/Transformer/Stencil.cpp
+++ clang/lib/Tooling/Transformer/Stencil.cpp
@@ -63,6 +63,7 @@
   MaybeDeref,
   AddressOf,
   MaybeAddressOf,
+  Format,
 };
 
 // Generic container for stencil operations with a (single) node-id argument.
@@ -133,6 +134,9 @@
   case UnaryNodeOperator::MaybeAddressOf:
 OpName = "maybeAddressOf";
 break;
+  case UnaryNodeOperator::Format:
+OpName = "formatNode";
+break;
   }
   return (OpName + "(\"" + Data.Id + "\")").str();
 }
@@ -174,11 +178,11 @@
   return Error::success();
 }
 
-Error evalData(const DebugPrintNodeData &Data,
-   const MatchFinder::MatchResult &Match, std::string *Result) {
+static Error printNode(StringRef Id, const MatchFinder::MatchResult &Match,
+   std::string *Result) {
   std::string Output;
   llvm::raw_string_ostream Os(Output);
-  auto NodeOrErr = getNode(Match.Nodes, Data.Id);
+  auto NodeOrErr = getNode(Match.Nodes, Id);
   if (auto Err = NodeOrErr.takeError())
 return Err;
   NodeOrErr->print(Os, PrintingPolicy(Match.Context->getLangOpts

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

2020-12-04 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington updated this revision to Diff 309537.
erik.pilkington marked 5 inline comments as done.
erik.pilkington added a comment.

Address review comments.


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

https://reviews.llvm.org/D92495

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/Sema.h
  clang/lib/APINotes/APINotesYAMLCompiler.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Index/nullability.c
  clang/test/SemaObjC/nullability.m
  clang/test/SemaObjC/nullable-result.m
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -1315,6 +1315,8 @@
 return CXTypeNullability_NonNull;
   case NullabilityKind::Nullable:
 return CXTypeNullability_Nullable;
+  case NullabilityKind::NullableResult:
+return CXTypeNullability_NullableResult;
   case NullabilityKind::Unspecified:
 return CXTypeNullability_Unspecified;
 }
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -1539,10 +1539,20 @@
 
   const char *nullability = 0;
   switch (N) {
-case CXTypeNullability_NonNull: nullability = "nonnull"; break;
-case CXTypeNullability_Nullable: nullability = "nullable"; break;
-case CXTypeNullability_Unspecified: nullability = "unspecified"; break;
-case CXTypeNullability_Invalid: break;
+  case CXTypeNullability_NonNull:
+nullability = "nonnull";
+break;
+  case CXTypeNullability_Nullable:
+nullability = "nullable";
+break;
+  case CXTypeNullability_NullableResult:
+nullability = "nullable_result";
+break;
+  case CXTypeNullability_Unspecified:
+nullability = "unspecified";
+break;
+  case CXTypeNullability_Invalid:
+break;
   }
 
   if (nullability) {
Index: clang/test/SemaObjC/nullable-result.m
===
--- /dev/null
+++ clang/test/SemaObjC/nullable-result.m
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -Wnullable-to-nonnull-conversion %s
+// RUN: %clang_cc1 -xobjective-c++ -verify -fsyntax-only -fblocks -Wnullable-to-nonnull-conversion %s
+
+@class X;
+@class NSError;
+
+@interface SomeClass
+-(void)async_get:(void (^)(R *_Nullable_result rptr, NSError *err))completionHandler;
+@end
+
+void call(SomeClass *sc) {
+  [sc async_get:^(R *_Nullable_result rptr, NSError *err) {}];
+  [sc async_get:^(R *_Nullable rptr, NSError *err) {}];
+}
+
+void test_conversion() {
+  X *_Nullable_result nr;
+  X *_Nonnull l = nr; // expected-warning{{implicit conversion from nullable pointer 'X * _Nullable_result' to non-nullable pointer type 'X * _Nonnull'}}
+
+  (void)^(X * _Nullable_result p) {
+X *_Nonnull l = p; // expected-warning{{implicit conversion from nullable pointer 'X * _Nullable_result' to non-nullable pointer type 'X * _Nonnull'}}
+  };
+}
+
+void test_dup() {
+  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate nullability specifier _Nullable_result}}
+  id _Nullable _Nullable_result b; // expected-error{{nullability specifier _Nullable_result conflicts with existing specifier '_Nullable'}}
+  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier '_Nonnull' conflicts with existing specifier _Nullable_result}}
+}
Index: clang/test/SemaObjC/nullability.m
===
--- clang/test/SemaObjC/nullability.m
+++ clang/test/SemaObjC/nullability.m
@@ -116,11 +116,13 @@
 - (nonnull id)returnsNonNull;
 - (nullable id)returnsNullable;
 - (null_unspecified id)returnsNullUnspecified;
+- (_Nullable_result id)returnsNullableResult;
 @end
 
 void test_receiver_merge(NSMergeReceiver *none,
  _Nonnull NSMergeReceiver *nonnull,
  _Nullable NSMergeReceiver *nullable,
+ _Nullable_result NSMergeReceiver *nullable_result,
  _Null_unspecified NSMergeReceiver *null_unspecified) {
   int *ptr;
 
@@ -129,6 +131,12 @@
   ptr = [nullable returnsNonNull]; // expected-warning{{'id _Nullable'}}
   ptr = [nullable returnsNone]; // expected-warning{{'id _Nullable'}}

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

2020-12-04 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington added inline comments.



Comment at: clang/lib/Basic/IdentifierTable.cpp:718-719
+  case NullabilityKind::NullableResult:
+assert(!isContextSensitive &&
+   "_Nullable_result isn't supported as context-sensitive keyword");
+return "_Nullable_result";

aaron.ballman wrote:
> Can you explain why it differs from `_Nullable` in this case?
Sure, _Nullable can be written like `nullable` in an `@property` or method 
return/param type i.e.:

```
@property(nullable) id x;
```

`_Nullable_result` isn't really useful in these contexts (you should always use 
`nullable` there), so I didn't add parsing support for it. 



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1566
   unsigned receiverNullabilityIdx = 0;
-  if (auto nullability = ReceiverType->getNullability(Context))
+  if (auto nullability = ReceiverType->getNullability(Context)) {
+if (*nullability == NullabilityKind::NullableResult)

aaron.ballman wrote:
> Should that be `auto *`?
Its an `Optional`, so I guess it should really be written out. 
New patch drops the auto.


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

https://reviews.llvm.org/D92495

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

[clang] 2502f89 - [OPENMP]Fix PR48387: disable warning messages caused by internal conversions.

2020-12-04 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2020-12-04T07:44:36-08:00
New Revision: 2502f899543151cf3d35c0fa0eef4ba681ad4e77

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

LOG: [OPENMP]Fix PR48387: disable warning messages caused by internal 
conversions.

Compiler needs to convert some of the loop iteration
variables/conditions to different types for better codegen and it may
lead to spurious warning messages about implicit signed/unsigned
conversions.

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/for_ast_print.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index dece57bb060a..932017d638f5 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -4196,6 +4196,7 @@ static OMPCapturedExprDecl *buildCaptureDecl(Sema &S, 
IdentifierInfo *Id,
   if (!WithInit)
 CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C));
   S.CurContext->addHiddenDecl(CED);
+  Sema::TentativeAnalysisScope Trap(S);
   S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false);
   return CED;
 }
@@ -7580,6 +7581,7 @@ std::pair 
OpenMPIterationSpaceChecker::buildMinMaxValues(
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);
 
+  Sema::TentativeAnalysisScope Trap(SemaRef);
   Diff = SemaRef.ActOnFinishFullExpr(Diff.get(), /*DiscardedValue=*/false);
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);

diff  --git a/clang/test/OpenMP/for_ast_print.cpp 
b/clang/test/OpenMP/for_ast_print.cpp
index 15187becf2e8..1cc68c4ad87e 100644
--- a/clang/test/OpenMP/for_ast_print.cpp
+++ b/clang/test/OpenMP/for_ast_print.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s -Wsign-conversion | 
FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify 
%s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s -Wsign-conversion | 
FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only 
-verify %s -ast-print | FileCheck %s
 // expected-no-diagnostics
@@ -223,9 +223,9 @@ int main(int argc, char **argv) {
 // CHECK: static int a;
 #pragma omp for schedule(guided, argc) reduction(+:argv[0][:1]) 
order(concurrent)
   // CHECK-NEXT: #pragma omp for schedule(guided, argc) reduction(+: 
argv[0][:1]) order(concurrent)
-  for (int i = 0; i < 2; ++i)
+  for (int i = argc; i < c; ++i)
 a = 2;
-// CHECK-NEXT: for (int i = 0; i < 2; ++i)
+// CHECK-NEXT: for (int i = argc; i < c; ++i)
 // CHECK-NEXT: a = 2;
 #pragma omp parallel
 #pragma omp for private(argc, b), firstprivate(argv, c), lastprivate(d, f) 
collapse(3) schedule(auto) ordered nowait linear(g:-1) reduction(task, +:e)



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

[PATCH] D92655: [OPENMP]Fix PR48387: disable warning messages caused by internal conversions.

2020-12-04 Thread Alexey Bataev via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2502f8995431: [OPENMP]Fix PR48387: disable warning messages 
caused by internal conversions. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92655

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


Index: clang/test/OpenMP/for_ast_print.cpp
===
--- clang/test/OpenMP/for_ast_print.cpp
+++ clang/test/OpenMP/for_ast_print.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s -Wsign-conversion | 
FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify 
%s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s -Wsign-conversion | 
FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only 
-verify %s -ast-print | FileCheck %s
 // expected-no-diagnostics
@@ -223,9 +223,9 @@
 // CHECK: static int a;
 #pragma omp for schedule(guided, argc) reduction(+:argv[0][:1]) 
order(concurrent)
   // CHECK-NEXT: #pragma omp for schedule(guided, argc) reduction(+: 
argv[0][:1]) order(concurrent)
-  for (int i = 0; i < 2; ++i)
+  for (int i = argc; i < c; ++i)
 a = 2;
-// CHECK-NEXT: for (int i = 0; i < 2; ++i)
+// CHECK-NEXT: for (int i = argc; i < c; ++i)
 // CHECK-NEXT: a = 2;
 #pragma omp parallel
 #pragma omp for private(argc, b), firstprivate(argv, c), lastprivate(d, f) 
collapse(3) schedule(auto) ordered nowait linear(g:-1) reduction(task, +:e)
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4196,6 +4196,7 @@
   if (!WithInit)
 CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C));
   S.CurContext->addHiddenDecl(CED);
+  Sema::TentativeAnalysisScope Trap(S);
   S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false);
   return CED;
 }
@@ -7580,6 +7581,7 @@
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);
 
+  Sema::TentativeAnalysisScope Trap(SemaRef);
   Diff = SemaRef.ActOnFinishFullExpr(Diff.get(), /*DiscardedValue=*/false);
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);


Index: clang/test/OpenMP/for_ast_print.cpp
===
--- clang/test/OpenMP/for_ast_print.cpp
+++ clang/test/OpenMP/for_ast_print.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s -Wsign-conversion | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s -Wsign-conversion | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 // expected-no-diagnostics
@@ -223,9 +223,9 @@
 // CHECK: static int a;
 #pragma omp for schedule(guided, argc) reduction(+:argv[0][:1]) order(concurrent)
   // CHECK-NEXT: #pragma omp for schedule(guided, argc) reduction(+: argv[0][:1]) order(concurrent)
-  for (int i = 0; i < 2; ++i)
+  for (int i = argc; i < c; ++i)
 a = 2;
-// CHECK-NEXT: for (int i = 0; i < 2; ++i)
+// CHECK-NEXT: for (int i = argc; i < c; ++i)
 // CHECK-NEXT: a = 2;
 #pragma omp parallel
 #pragma omp for private(argc, b), firstprivate(argv, c), lastprivate(d, f) collapse(3) schedule(auto) ordered nowait linear(g:-1) reduction(task, +:e)
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -4196,6 +4196,7 @@
   if (!WithInit)
 CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C));
   S.CurContext->addHiddenDecl(CED);
+  Sema::TentativeAnalysisScope Trap(S);
   S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false);
   return CED;
 }
@@ -7580,6 +7581,7 @@
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);
 
+  Sema::TentativeAnalysisScope Trap(SemaRef);
   Diff = SemaRef.ActOnFinishFullExpr(Diff.get(), /*DiscardedValue=*/false);
   if (!Diff.isUsable())
 return std::make_pair(nullptr, nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/

[PATCH] D90871: [Sema] Fold VLAs to constant arrays in a few more contexts

2020-12-04 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Looks like this breaks tests on Windows: http://45.33.8.238/win/29181/step_7.txt

PTAL, and revert while you investigate if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90871

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

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

2020-12-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added a comment.

In D92361#2433190 , @rjmccall wrote:

> There is no such thing as an object "teleporting" in C++.  Objects are always 
> observed in memory with a specific address.  When an argument is passed in 
> registers, its address can be observed to be different on both sides, and 
> that is not permitted; there must be some operation that creates the object 
> at the new address and destroys it at the old.

That's where you're wrong (about C++). You might be right about C or 
Objective-C, I don't know. In C++, that "teleporting" happens //without// any 
call to a special member — there is no move happening, and no destroy 
happening. You can actually observe this: https://godbolt.org/z/zojooc The 
object is simply "bitwise-teleported" from one place to another. Standard C++17 
says that this is a "guaranteed-copy-elision" context; there is indeed only one 
C++ "object" here. It just happens to blit around in memory //beyond// what the 
C++ code is doing to it.

> That operation is a destructive move (which I certainly did not originate as 
> a term for this), or what your proposal calls a relocation (which may be a 
> more palatable term for the C++ committee).

D50119  "relocation" is different; it's a way 
for the programmer to warrant an invariant about the behavior of the 
//C++-language-level// operations which you called "copy" and "destroy," and 
say that these operations can be //replaced// by the compiler as long as 
they're replaced in matching pairs. (A move and a destroy always end up 
"teleporting" the object from one place to another as a side effect; 
`[[trivially_relocatable]]` warrants that that pair of operations has no 
//other// side-effects worth preserving.) But D50119 
 "relocation" doesn't permit the compiler to 
introduce new "teleports" in places where the programmer hasn't written any 
operations at all. https://godbolt.org/z/zojooc is an example of code where the 
programmer hasn't written any operations at all — we have just one object — and 
yet, the compiler teleports it from place to place.

> Your trait should certainly consider a `trivial_abi` type to be trivially 
> relocatable.

The two attributes are essentially orthogonal, from the compiler's POV. One 
says it's okay to replace [move+destroy] with [teleport] but you must not 
introduce any unrequested teleports; the other says it's okay to introduce 
unrequested teleports but you must fully execute all moves and destroys. 
https://godbolt.org/z/EKbznG  (Of course the other major difference in practice 
is that `[[trivially_relocatable]]` is all libc++ magic that manually replaces 
[move+destroy] pairs only in //library// code. So my use of libc++ `std::swap` 
in that demo is deliberate; you wouldn't see the [move+destroy] pairs getting 
removed by the compiler if you had written out swap's 
move-assign-assign-destroy dance by hand.)

---

However, I see there's yet //another// way to think about it, which is highly 
relevant to this PR! In P0593 "implicit-lifetime objects" 

conceptually spring into existence via the implementation secretly calling one 
of their constructors at an unspecified point; but this call is unobservable 
because the compiler magically selects a //trivial// constructor to call. Types 
with no trivial constructors can't be implicit-lifetime (IIUC). You could apply 
the same trick here — you could say that when a `[[trivial_abi]]` object 
"bitwise-teleports" from one place to another, it's not //actually// 
teleporting — the compiler is secretly inserting calls to [move+destroy] and 
then replacing the pair with a teleport. But this trick works only if the type 
is in fact movable and/or copyable! If the type //has// no move or copy 
operations, then you can't use this trick to explain what's going on.

Here's an example using Clang trunk `[[trivial_abi]]`: 
https://godbolt.org/z/jox9Er
`U0` is trivial-abi, movable, non-copyable. `U1` is trivial-abi, copyable, 
non-movable. `U2` has a `U0` data member, so it's movable and non-copyable, and 
it inherits trivial-abi. `U3` has a `U1` data member, so it's copyable and 
non-movable, and //it// inherits trivial-abi. But `U4` has both `U0` and `U1` 
members; it wants to inherit trivial-abi, but it's neither movable nor 
copyable. If we're using the trick of saying that "the compiler is not 
inserting bitwise-teleports, it's inserting C++-language-level operations and 
then replacing them with teleports," then we can't use that trick here, because 
`U4` //has// no language-level operations to secretly insert — in the same way 
that a type with no constructors can't be implicit-lifetime (IIUC).

So I think Zoe's PR here is basically forcing us to choose a model: is 
`[[trivial_abi]]` giving the compiler permission to insert unrequested 
bitwise-teleports //di

[PATCH] D92231: [OpenCL] Implement extended subgroups fully in headers

2020-12-04 Thread Piotr Fusik via Phabricator via cfe-commits

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



Comment at: clang/lib/Headers/opencl-c-base.h:17-23
+#define cl_khr_subgroup_extended_types
+#define cl_khr_subgroup_non_uniform_vote
+#define cl_khr_subgroup_ballot
+#define cl_khr_subgroup_non_uniform_arithmetic
+#define cl_khr_subgroup_shuffle
+#define cl_khr_subgroup_shuffle_relative
+#define cl_khr_subgroup_clustered_reduce

These are currently defined as "1": https://godbolt.org/z/MnoWeo
Is the change to blank intentional?
This should be tested.


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

https://reviews.llvm.org/D92231

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

[PATCH] D92640: Add ability to load a FixedCompilationDatabase from a buffer.

2020-12-04 Thread Utkarsh Saxena via Phabricator via cfe-commits

usaxena95 added inline comments.



Comment at: clang/include/clang/Tooling/CompilationDatabase.h:194
+  static std::unique_ptr
+  loadFromBuffer(StringRef Path, StringRef Data, std::string &ErrorMsg);
+

`Path` is not meaningful for load from buffer. May be change name to 
`Directory` and call loadFromBuffer(**llvm::sys::path::parent_path(Path)**, 
(*File)->getBuffer(), ErrorMsg); instead.



Comment at: clang/include/clang/Tooling/CompilationDatabase.h:194
+  static std::unique_ptr
+  loadFromBuffer(StringRef Path, StringRef Data, std::string &ErrorMsg);
+

usaxena95 wrote:
> `Path` is not meaningful for load from buffer. May be change name to 
> `Directory` and call loadFromBuffer(**llvm::sys::path::parent_path(Path)**, 
> (*File)->getBuffer(), ErrorMsg); instead.
Do you intend to use it apart from testing ?



Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:553
+  std::string ErrorMsg;
+  auto CDB = 
FixedCompilationDatabase::loadFromBuffer("/path/compile_flags.txt",
+  Data, ErrorMsg);

nit: change to something like `/path/to/cdb/dir`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92640

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

[clang] 4fa0dbd - Fix a test failing on windows

2020-12-04 Thread Erik Pilkington via cfe-commits


Author: Erik Pilkington
Date: 2020-12-04T11:20:17-05:00
New Revision: 4fa0dbd6885bc4a48ceecdbb0fca9638792762cf

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

LOG: Fix a test failing on windows

Added: 


Modified: 
clang/test/SemaCXX/vla.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/vla.cpp b/clang/test/SemaCXX/vla.cpp
index f1e75f5dce9a..f7cae9e370f4 100644
--- a/clang/test/SemaCXX/vla.cpp
+++ b/clang/test/SemaCXX/vla.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -verify %s
 
 // PR11925
 int n;



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

[PATCH] D90871: [Sema] Fold VLAs to constant arrays in a few more contexts

2020-12-04 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington added a comment.

In D90871#2433766 , @thakis wrote:

> Looks like this breaks tests on Windows: 
> http://45.33.8.238/win/29181/step_7.txt
>
> PTAL, and revert while you investigate if it takes a while to fix.

Thanks for letting me know, should be fixed by: 
4fa0dbd6885bc4a48ceecdbb0fca9638792762cf 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90871

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

[PATCH] D92658: [libTooling] Add `formatNode` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

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

renamed the combinator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92658

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -55,14 +55,14 @@
 // `StatementCode` may contain other statements not described by `Matcher`.
 static llvm::Optional matchStmt(StringRef StatementCode,
StatementMatcher Matcher) {
-  auto AstUnit = tooling::buildASTFromCode(wrapSnippet(StatementCode));
+  auto AstUnit = tooling::buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
+   {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
 ADD_FAILURE() << "AST construction failed";
 return llvm::None;
   }
   ASTContext &Context = AstUnit->getASTContext();
-  auto Matches = ast_matchers::match(
-  traverse(ast_type_traits::TK_AsIs, wrapMatcher(Matcher)), Context);
+  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();
@@ -365,6 +365,50 @@
   EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("field"));
 }
 
+TEST_F(StencilTest, NodeAsStringStmt) {
+  std::string Snippet = "int *x; if (x == nullptr) (void)*x;";
+  std::string Expected = "if (x == nullptr)\n(void)*x;\n";
+  auto StmtMatch = matchStmt(Snippet, ifStmt().bind("id"));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(nodeAsString("id")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, NodeAsStringExpr) {
+  StringRef Id = "id";
+  testExpr(Id, "int *x; (*x + 1);", nodeAsString(Id), "(*x + 1)");
+}
+
+TEST_F(StencilTest, NodeAsStringType) {
+  std::string Snippet = "int *x; x;";
+  std::string Expected = "int *";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(nodeAsString("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, NodeAsStringSugaredType) {
+  std::string Snippet = "using Ty = int; Ty *x; x;";
+  std::string Expected = "Ty *";
+  auto StmtMatch =
+  matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(nodeAsString("type")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
+TEST_F(StencilTest, NodeAsStringDecl) {
+  std::string Snippet = "int *x; x;";
+  std::string Expected = "int *x";
+  auto StmtMatch =
+  matchStmt(Snippet, declStmt(hasSingleDecl(decl().bind("decl";
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(nodeAsString("decl")->eval(StmtMatch->Result),
+   HasValue(std::string(Expected)));
+}
+
 TEST_F(StencilTest, RunOp) {
   StringRef Id = "id";
   auto SimpleFn = [Id](const MatchResult &R) {
@@ -448,6 +492,12 @@
   EXPECT_EQ(S->toString(), Expected);
 }
 
+TEST(StencilToStringTest, NodeAsStringOp) {
+  auto S = nodeAsString("Id");
+  StringRef Expected = R"repr(nodeAsString("Id"))repr";
+  EXPECT_EQ(S->toString(), Expected);
+}
+
 TEST(StencilToStringTest, DebugPrintNodeOp) {
   auto S = dPrint("Id");
   StringRef Expected = R"repr(dPrint("Id"))repr";
Index: clang/lib/Tooling/Transformer/Stencil.cpp
===
--- clang/lib/Tooling/Transformer/Stencil.cpp
+++ clang/lib/Tooling/Transformer/Stencil.cpp
@@ -63,6 +63,7 @@
   MaybeDeref,
   AddressOf,
   MaybeAddressOf,
+  AsString,
 };
 
 // Generic container for stencil operations with a (single) node-id argument.
@@ -133,6 +134,9 @@
   case UnaryNodeOperator::MaybeAddressOf:
 OpName = "maybeAddressOf";
 break;
+  case UnaryNodeOperator::AsString:
+OpName = "nodeAsString";
+break;
   }
   return (OpName + "(\"" + Data.Id + "\")").str();
 }
@@ -174,11 +178,11 @@
   return Error::success();
 }
 
-Error evalData(const DebugPrintNodeData &Data,
-   const MatchFinder::MatchResult &Match, std::string *Result) {
+static Error printNode(StringRef Id, const MatchFinder::MatchResult &Match,
+   std::string *Result) {
   std::string Output;
   llvm::raw_string_ostream Os(Output);
-  auto NodeOrErr = getNode(Match.Nodes, Data.Id);
+  auto NodeOrErr = getNode(Match.Nodes, Id);
   if (auto Err = NodeOrErr.takeError())
 return Err;
   NodeOrErr->print(Os, PrintingPolicy(Match.Context->getLangOpts()));
@@ -186,8 +190,18

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM! Thank you for the new functionality! You may want to wait a bit before 
landing to see if @njames93 has any remaining comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D92661: [RFC] Fix TLS and Coroutine

2020-12-04 Thread Xun Li via Phabricator via cfe-commits

lxfind created this revision.
Herald added subscribers: hoy, modimo, wenlei, steven_wu, modocache, hiraditya, 
mgorny.
lxfind requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1, jdoerfert.
Herald added projects: clang, LLVM.

This patch is to address https://bugs.llvm.org/show_bug.cgi?id=47835.
A relevant discussion regarding pthread_self and TLS can be found here: 
http://lists.llvm.org/pipermail/llvm-dev/2020-November/146766.html.

A coroutine may suspend and resume on a different thread, and hence the address 
of a thread_local variable may change after coroutine suspension.
In the existing design, getting the address of a TLS variable is through a 
direct reference, like @tls_variable. Such kind of value can be
arbitrarily moved around/replaced in the IR within the same function. This will 
lead to incorrect caching of TLS variable address in coroutines across 
suspension points.
To fix it, we have to turn the TLS address access into an intrinsics call, so 
that it will not be simply CSE-ed.
After CoroSplit, we no longer have coroutines, and hence can safely lower the 
TLS intrinsics back into references.

Note:
The current placement of the LowerThreadLocalIntrinsicPass may not be ideal. I 
am not quite sure how to organize it. Suggestions welcome!
Testing isn't sufficient, and there may also be failing tests. I will add/fix 
more tests if this patch is along the right direction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92661

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c
  clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
  clang/test/CodeGenCoroutines/coro-tls.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Scalar.h
  llvm/include/llvm/Transforms/Scalar/LowerThreadLocalIntrinsic.h
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Scalar/CMakeLists.txt
  llvm/lib/Transforms/Scalar/LowerThreadLocalIntrinsic.cpp
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-O0-defaults.ll
  llvm/test/Other/new-pm-defaults.ll

Index: llvm/test/Other/new-pm-defaults.ll
===
--- llvm/test/Other/new-pm-defaults.ll
+++ llvm/test/Other/new-pm-defaults.ll
@@ -209,6 +209,7 @@
 ; CHECK-EP-CGSCC-LATE-NEXT: Running pass: NoOpCGSCCPass
 ; CHECK-O-NEXT: Finished CGSCC pass manager run.
 ; CHECK-O-NEXT: Finished llvm::Module pass manager run.
+; CHECK-O-NEXT: Running pass: LowerThreadLocalIntrinsicPass
 ; CHECK-O-NEXT: Running pass: GlobalOptPass
 ; CHECK-O-NEXT: Running pass: GlobalDCEPass
 ; CHECK-DEFAULT-NEXT: Running pass: EliminateAvailableExternallyPass
Index: llvm/test/Other/new-pm-O0-defaults.ll
===
--- llvm/test/Other/new-pm-O0-defaults.ll
+++ llvm/test/Other/new-pm-O0-defaults.ll
@@ -32,6 +32,7 @@
 ; CHECK-DEFAULT-NEXT: Running analysis: ProfileSummaryAnalysis
 ; CHECK-MATRIX-NEXT: Running pass: LowerMatrixIntrinsicsPass
 ; CHECK-MATRIX-NEXT: Running analysis: TargetIRAnalysis
+; CHECK-DEFAULT-NEXT: Running pass: LowerThreadLocalIntrinsicPass
 ; CHECK-PRE-LINK-NEXT: Running pass: CanonicalizeAliasesPass
 ; CHECK-PRE-LINK-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-THINLTO-NEXT: Running pass: Annotation2MetadataPass
Index: llvm/test/Other/new-pass-manager.ll
===
--- llvm/test/Other/new-pass-manager.ll
+++ llvm/test/Other/new-pass-manager.ll
@@ -366,6 +366,7 @@
 ; CHECK-EXT-NEXT: Starting llvm::Function pass manager run.
 ; CHECK-EXT-NEXT: Running pass: {{.*}}Bye
 ; CHECK-EXT-NEXT: Finished llvm::Function pass manager run.
+; CHECK-O0-NEXT: Running pass: LowerThreadLocalIntrinsicPass
 ; CHECK-O0-NEXT: Finished llvm::Module pass manager run
 
 ; RUN: opt -disable-output -disable-verify -debug-pass-manager \
Index: llvm/lib/Transforms/Scalar/LowerThreadLocalIntrinsic.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Scalar/LowerThreadLocalIntrinsic.cpp
@@ -0,0 +1,76 @@
+//===- LowerThreadLocalIntrinsic.cpp - Lower the threadlocal intrinsic
+//---===//
+//
+// 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
+//
+//===--===//
+//
+// This pass lowers the llvm.threadlocal intrinsic to a direct reference to the
+// thread local variable.
+//
+//===--===//
+
+#include "llvm/Transforms/Scalar/LowerThreadLocalIntri

[PATCH] D92662: [Clang][Coroutine] Drop const attribute on pthread_self when coroutine is enabled

2020-12-04 Thread Xun Li via Phabricator via cfe-commits

lxfind created this revision.
Herald added subscribers: hoy, modimo, wenlei, modocache.
lxfind requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is to address https://bugs.llvm.org/show_bug.cgi?id=47833
A relevant discussion can also be found in 
http://lists.llvm.org/pipermail/llvm-dev/2020-November/146766.html

pthread_self() from glibc is defined with "__attribute__
((__const__))". The const attribute tells the compiler that it does
not read nor write any global state and hence always return the same
result. Hence in the following code:

auto x1 = pthread_self();
...
auto x2 = pthread_self();

the second call to pthread_self() can be optimized out. This has been
correct until coroutines. With coroutines, we can have code like this:

auto x1 = pthread_self();
co_await ...
auto x2 = pthread_self();

Now because of the co_await, the function can suspend and resume in a
different thread, in which case the second call to pthread_self()
should return a different result than the first one. Unfortunately
LLVM will still optimize out the second call in the case of
coroutines.

To fix the issue, this patch drops the readnone attribute from the pthread_self 
function in Clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92662

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCoroutines/coro-pthread_self.cpp


Index: clang/test/CodeGenCoroutines/coro-pthread_self.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-pthread_self.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang -fcoroutines-ts -std=c++14 -O3 -emit-llvm -S %s -o - | 
FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+typedef void *pthread_t;
+pthread_t pthread_self(void) __attribute__((__const__));
+
+struct awaitable {
+  bool await_ready() { return false; }
+  void await_suspend(coro::coroutine_handle<> h);
+  void await_resume() {}
+};
+awaitable switch_to_new_thread();
+
+struct task {
+  struct promise_type {
+task get_return_object() { return {}; }
+coro::suspend_never initial_suspend() { return {}; }
+coro::suspend_never final_suspend() noexcept { return {}; }
+void return_void() {}
+void unhandled_exception() {}
+  };
+};
+
+void check(pthread_t p1, pthread_t p2);
+
+task resuming_on_new_thread() {
+  auto pthread1 = pthread_self();
+  co_await switch_to_new_thread();
+  auto pthread2 = pthread_self();
+  check(pthread1, pthread2);
+}
+
+void non_coroutine() {
+  auto pthread1 = pthread_self();
+  check(pthread1, pthread1);
+  auto pthread2 = pthread_self();
+  check(pthread1, pthread2);
+}
+
+// CHECK-LABEL: define void @_Z13non_coroutinev()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:%call = tail call i8* @_Z12pthread_selfv()
+// CHECK-NEXT:tail call void @_Z5checkPvS_(i8* %call, i8* %call)
+// CHECK-NEXT:%call1 = tail call i8* @_Z12pthread_selfv()
+// CHECK-NEXT:tail call void @_Z5checkPvS_(i8* %call, i8* %call1)
+// CHECK-NEXT:ret void
+// CHECK-NEXT:  }
+
+// CHECK-LABEL: define internal fastcc void @_Z22resuming_on_new_threadv.resume
+// CHECK: %[[CALL:.+]] = invoke i8* @_Z12pthread_selfv()
+// CHECK-NEXT:to label %[[CONT:.+]] unwind label %{{.+}}
+// CHECK:  [[CONT]]:
+// CHECK-NEXT:%[[RELOAD_ADDR:.+]] = getelementptr inbounds 
%_Z22resuming_on_new_threadv.Frame, %_Z22resuming_on_new_threadv.Frame* 
%FramePtr, i64 0, i32 {{.+}}
+// CHECK-NEXT:%[[RELOAD:.+]] = load i8*, i8** %[[RELOAD_ADDR]], align 8
+// CHECK-NEXT:invoke void @_Z5checkPvS_(i8* %[[RELOAD]], i8* %[[CALL]])
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14938,6 +14938,12 @@
   IdentifierInfo *Name = FD->getIdentifier();
   if (!Name)
 return;
+
+  if (getLangOpts().Coroutines && Name->isStr("pthread_self") &&
+  FD->hasAttr()) {
+FD->dropAttr();
+  }
+
   if ((!getLangOpts().CPlusPlus &&
FD->getDeclContext()->isTranslationUnit()) ||
   (isa(FD->getDeclContext()) &&


Index: clang/test/CodeGenCoroutines/coro-pthread_self.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-pthread_self.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang -fcoroutines-ts -std=c++14 -O3 -emit-llvm -S %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+typedef void *pthread_t;
+pthread_t pthread_self(void) __attribute__((__const__));
+
+struct awaitable {
+  bool await_ready() { return false; }
+  void await_suspend(coro::coroutine_handle<> h);
+  void await_resume() {}
+};
+awaitable switch_to_new_thread();
+
+struct task {
+  struct promise_type {
+task get_return_object() { return {}; }
+coro::suspend_never initial_suspend() { return {}; }
+cor

[PATCH] D92108: Fix inconsistent availability attribute message string literal check.

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

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

I think this is a good change but would point out that the diagnostic message 
is pretty confusing as-is. `u8"a"` is a string literal, so saying that a string 
literal is expected doesn't make sense. We may want to consider either 
supporting string literals more generally or changing the diagnostic. The 
mixing of adjacent string literals with prefixes is something I don't think we 
need/want to support (esp as both C and C++ are making efforts to make such 
code ill-formed anyway), but I don't see why we should reject 
`message=u8"whatever"` (or any of the other prefixes).

However, that's an orthogonal issue and this is good incremental progress, so 
LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92108

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

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM aside from a few minor NFC changes and a documentation request. Thank you 
for this!




Comment at: clang/docs/LanguageExtensions.rst:623
+C++11 Attributes on using-declarations
+
+

Wrong number of `=` here -- should extend them to the end of the heading.



Comment at: clang/docs/LanguageExtensions.rst:630
+
+  [[clang::using_if_exists]] using foo::bar;
+

Can you also add `using foo::bar [[clang::using_if_exists]];` to show that we 
support the syntax in two locations?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2419
 static void handleAvailabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (isa(D) || isa(D) ||
+  isa(D)) {

`isa(D)`



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6952
 }
+  } else if (isa(D) ||
+ isa(D) ||

Same suggestion here.


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

https://reviews.llvm.org/D91630

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

[PATCH] D92176: Don't use sysroot/include when sysroot is empty.

2020-12-04 Thread Hafiz Abid Qadeer via Phabricator via cfe-commits

abidh added a comment.

@jroelofs Do you have any comments on this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92176

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

[PATCH] D92663: [clangd] Add hot-reload of compile_commands.json and compile_flags.txt

2020-12-04 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, jfb, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

When querying the CDB, we stat the underlying file to check it hasn't changed.
We don't do this every time, but only if we didn't check within 5 seconds.

This behavior only exists for compile_commands.json and compile_flags.txt.
The CDB plugin system doesn't expose enough information to handle others.

Slight behavior change: we now only look for `build/compile_commands.json`
rather than trying every CDB strategy under `build` subdirectories.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92663

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/CompilationDatabase.cpp
  clang/lib/Tooling/JSONCompilationDatabase.cpp

Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -217,7 +217,7 @@
 std::string &ErrorMessage,
 JSONCommandLineSyntax Syntax) {
   std::unique_ptr DatabaseBuffer(
-  llvm::MemoryBuffer::getMemBuffer(DatabaseString));
+  llvm::MemoryBuffer::getMemBufferCopy(DatabaseString));
   std::unique_ptr Database(
   new JSONCompilationDatabase(std::move(DatabaseBuffer), Syntax));
   if (!Database->parse(ErrorMessage))
Index: clang/lib/Tooling/CompilationDatabase.cpp
===
--- clang/lib/Tooling/CompilationDatabase.cpp
+++ clang/lib/Tooling/CompilationDatabase.cpp
@@ -348,9 +348,17 @@
 ErrorMsg = "Error while opening fixed database: " + Result.message();
 return nullptr;
   }
+  return loadFromBuffer(Path, (*File)->getBuffer(), ErrorMsg);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromBuffer(StringRef Path, StringRef Data,
+ std::string &ErrorMsg) {
+  ErrorMsg.clear();
   std::vector Args;
-  for (llvm::StringRef Line :
-   llvm::make_range(llvm::line_iterator(**File), llvm::line_iterator())) {
+  StringRef Line;
+  while (!Data.empty()) {
+std::tie(Line, Data) = Data.split('\n');
 // Stray whitespace is almost certainly unintended.
 Line = Line.trim();
 if (!Line.empty())
Index: clang/include/clang/Tooling/CompilationDatabase.h
===
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -184,11 +184,15 @@
   int &Argc, const char *const *Argv, std::string &ErrorMsg,
   Twine Directory = ".");
 
-  /// Reads flags from the given file, one-per line.
+  /// Reads flags from the given file, one-per-line.
   /// Returns nullptr and sets ErrorMessage if we can't read the file.
   static std::unique_ptr
   loadFromFile(StringRef Path, std::string &ErrorMsg);
 
+  /// Reads flags from the given buffer, one-per-line.
+  static std::unique_ptr
+  loadFromBuffer(StringRef Path, StringRef Data, std::string &ErrorMsg);
+
   /// Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -11,8 +11,10 @@
 #include "Matchers.h"
 #include "TestFS.h"
 #include "support/Path.h"
+#include "support/ThreadsafeFS.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -23,6 +25,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -40,7 +43,8 @@
 using ::testing::UnorderedElementsAre;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
-  DirectoryBasedGlobalCompilationDatabase DB(None);
+  MockFS TFS;
+  DirectoryBasedGlobalCompilationDatabase DB(TFS);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
   EXPECT_THAT(Cmd.CommandLine, ElementsAre("clang", testPath("foo/bar.cc")));
@@ -166,6 +170,7 @@
 }
 
 // Allows placement of files for tests

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

2020-12-04 Thread Doug Gregor via Phabricator via cfe-commits

doug.gregor added inline comments.



Comment at: clang/test/SemaObjC/nullable-result.m:6
+@class NSError;
+
+@interface SomeClass

Can you add a `__has_attribute` check for `_Nullable_result`?


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

https://reviews.llvm.org/D92495

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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

@JohelEGP can you let me know if you plan to remove the "requested changes" you 
asked for back in July?

I cannot commit with the revision until that is removed


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

https://reviews.llvm.org/D79773

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

[PATCH] D92176: Don't use sysroot/include when sysroot is empty.

2020-12-04 Thread Jon Roelofs via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92176

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

[PATCH] D92661: [RFC] Fix TLS and Coroutine

2020-12-04 Thread Hongtao Yu via Phabricator via cfe-commits

hoy added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1309
+// Intrinsic to obtain the address of a thread_local variable.
+def int_threadlocal : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>;
+

With the intrinsic, can TLS variable reference in the same coroutine or regular 
routine be DCE-ed anymore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661

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

[PATCH] D92354: [clang] add a new `swift_attr` attribute

2020-12-04 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman updated this revision to Diff 309553.
arphaman marked 2 inline comments as done.
arphaman added a comment.

Updated for review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92354

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/attr-swift_attr.m
  clang/test/SemaObjC/validate-attr-swift_attr.m

Index: clang/test/SemaObjC/validate-attr-swift_attr.m
===
--- /dev/null
+++ clang/test/SemaObjC/validate-attr-swift_attr.m
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+// expected-error@+1 {{'swift_attr' attribute takes one argument}}
+__attribute__((swift_attr))
+@interface I
+@end
+
+// expected-error@+1 {{'swift_attr' attribute requires a string}}
+__attribute__((swift_attr(1)))
+@interface J
+@end
Index: clang/test/AST/attr-swift_attr.m
===
--- /dev/null
+++ clang/test/AST/attr-swift_attr.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
+
+__attribute__((swift_attr("@actor")))
+@interface View
+@end
+
+// CHECK: InterfaceDecl {{.*}} View
+// CHECK-NEXT: SwiftAttrAttr {{.*}} "@actor"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5607,6 +5607,16 @@
   D->addAttr(::new (S.Context) ObjCPreciseLifetimeAttr(S.Context, AL));
 }
 
+static void handleSwiftAttrAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // Make sure that there is a string literal as the annotation's single
+  // argument.
+  StringRef Str;
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
+return;
+
+  D->addAttr(::new (S.Context) SwiftAttrAttr(S.Context, AL, Str));
+}
+
 static void handleSwiftBridge(Sema &S, Decl *D, const ParsedAttr &AL) {
   // Make sure that there is a string literal as the annotation's single
   // argument.
@@ -7941,6 +7951,9 @@
 break;
 
   // Swift attributes.
+  case ParsedAttr::AT_SwiftAttr:
+handleSwiftAttrAttr(S, D, AL);
+break;
   case ParsedAttr::AT_SwiftBridge:
 handleSwiftBridge(S, D, AL);
 break;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3628,6 +3628,19 @@
   }];
 }
 
+def SwiftAttrDocs : Documentation {
+  let Category = SwiftDocs;
+  let Heading = "swift_attr";
+  let Content = [{
+The ``swift_attr`` provides a Swift-specific annotation for the declaration
+to which the attribute appertains to. It can be used on any declaration
+in Clang. This kind of annotation is ignored by Clang as it doesn't have any
+semantic meaning in languages supported by Clang. The Swift compiler can
+interpret these annotations according to its own rules when importing C or
+Objective-C declarations.
+}];
+}
+
 def SwiftBridgeDocs : Documentation {
   let Category = SwiftDocs;
   let Heading = "swift_bridge";
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2149,6 +2149,12 @@
   let ASTNode = 0;
 }
 
+def SwiftAttr : InheritableAttr {
+  let Spellings = [GNU<"swift_attr">];
+  let Args = [StringArgument<"Attribute">];
+  let Documentation = [SwiftAttrDocs];
+}
+
 def SwiftBridge : InheritableAttr {
   let Spellings = [GNU<"swift_bridge">];
   let Args = [StringArgument<"SwiftType">];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92354: [clang] add a new `swift_attr` attribute

2020-12-04 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added a comment.

Thanks for taking a look.

In D92354#2428604 , @erik.pilkington 
wrote:

> Do we need to add APINotes support for this?

Not at the moment, maybe in the future




Comment at: clang/include/clang/Basic/Attr.td:2153
+def SwiftAttr : InheritableAttr {
+  let Spellings = [Clang<"swift_attr">];
+  let Args = [StringArgument<"Attribute">];

aaron.ballman wrote:
> The other swift attributes use a GNU spelling and don't expose any C++ 
> spelling. Should this follow suit (or should the other attributes be updated)?
You're right, that was my mistake. Changed it to `GNU`.



Comment at: clang/include/clang/Basic/Attr.td:2155
+  let Args = [StringArgument<"Attribute">];
+  let Documentation = [SwiftAttrDocs];
+}

erik.pilkington wrote:
> Should we limit this to appear on certain subjects? Presumably the swift 
> importer is only going to look for this in certain places, so I think it 
> makes sense to call out places where it'll be ignored in clang. WDYT?
I think we want to leave it as a flexible attribute that can be applied to any 
declaration for now, and let the Swift importer decide on which declarations it 
wants to support them.



Comment at: clang/include/clang/Basic/AttrDocs.td:3635
+  let Content = [{
+The ``swift_attr`` provides a Swift-specific annotation for the declaration
+to which the attribute appertains to. This kind of annotation is ignored by

aaron.ballman wrote:
> What declarations does this attribute appertain to?
Mentioned that it can be applied to any declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92354

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

[PATCH] D90188: Add support for attribute 'using_if_exists'

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM aside from a minor nit, thank you!




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11681
+  if (isa(Target) !=
+  (NonTag && isa(NonTag))) {
+if (!NonTag && !Tag)

You can use `isa_and_nonnull<>()` instead of testing for null yourself.


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

https://reviews.llvm.org/D90188

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

[PATCH] D92661: [RFC] Fix TLS and Coroutine

2020-12-04 Thread Hongtao Yu via Phabricator via cfe-commits

hoy added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1309
+// Intrinsic to obtain the address of a thread_local variable.
+def int_threadlocal : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>;
+

hoy wrote:
> With the intrinsic, can TLS variable reference in the same coroutine or 
> regular routine be DCE-ed anymore?
Sorry, I meant CSE-ed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661

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

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

2020-12-04 Thread Anastasiia Lukianenko via Phabricator via cfe-commits

anastasiia_lukianenko added inline comments.



Comment at: clang/lib/Format/Format.cpp:893
  /*BeforeLambdaBody=*/false,
+ /*BeforeStructInitialization=*/false,
  /*BeforeWhile=*/false,

MyDeveloperDay wrote:
> I believe there are 3 places where BraceWrapping is initialized you seem to 
> only be doing 2 of them
Sorry, I didn't find one more place. Can you please say where it is?



Comment at: clang/unittests/Format/FormatTest.cpp:5063
+   "a = 1,\n"
+   "b = 2,\n"
+   "};",

MyDeveloperDay wrote:
> could you add an additional test without the trailing `,`
Yes, I can add the test, but before I want to discuss the expected result.
Now with my patch and BeforeStructInitialization = false the behavior is the 
following:

```
struct new_struct struct_name = {a = 1};
struct new_struct struct_name = {a = 1, b = 2};
```
And with BeforeStructInitialization = true:

```
struct new_struct struct_name =
{a = 1};
struct new_struct struct_name =
{a = 1, b = 2};
```
Is it okay?


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

https://reviews.llvm.org/D91949

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

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D92633#2433108 , @tmsriram wrote:

> You said : "The name -mpie-copy-relocations is misleading [1] and does not 
> capture the idea that this option can actually apply to all of 
> -fno-pic,-fpie, ..."
>
> Could you please clarify why this option needs to apply to -fno-pic?  Here is 
> what I tried with trunk clang:

If the user wants to guarantee no copy relocations in -fno-pic code, they can 
theoretically apply -fno-direct-access-external-data to use a GOT indirection.
This is not implemented, though.

> extern int var;
> int get() { return var; }
>
> $ clang -S foo.c -o -
> 
>   movlvar, %eax
>   popq%rbp
> ...
>
> With -fno-pic,  this will never need to use -mpie-copy-relocations at all, so 
> this case is not relevant right?  Did I miss anything?

-fno-pic code can only be used with -no-pie links (position-dependent 
executables) If var is not defined in the linked executable, it will have a 
copy relocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92633

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

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

2020-12-04 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from the testing requests.




Comment at: clang/lib/Basic/IdentifierTable.cpp:718-719
+  case NullabilityKind::NullableResult:
+assert(!isContextSensitive &&
+   "_Nullable_result isn't supported as context-sensitive keyword");
+return "_Nullable_result";

erik.pilkington wrote:
> aaron.ballman wrote:
> > Can you explain why it differs from `_Nullable` in this case?
> Sure, _Nullable can be written like `nullable` in an `@property` or method 
> return/param type i.e.:
> 
> ```
> @property(nullable) id x;
> ```
> 
> `_Nullable_result` isn't really useful in these contexts (you should always 
> use `nullable` there), so I didn't add parsing support for it. 
Ah, thank you for the explanation!



Comment at: clang/test/SemaObjC/nullability.m:119
 - (null_unspecified id)returnsNullUnspecified;
+- (_Nullable_result id)returnsNullableResult;
 @end

Can you add a test case that shows `- (nullable_result 
id)returnsNullableResultErr;` doesn't work (but also doesn't do something bad, 
like assert)?


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

https://reviews.llvm.org/D92495

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

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits

tmsriram added a comment.

In D92633#2433979 , @MaskRay wrote:

> In D92633#2433108 , @tmsriram wrote:
>
>> You said : "The name -mpie-copy-relocations is misleading [1] and does not 
>> capture the idea that this option can actually apply to all of 
>> -fno-pic,-fpie, ..."
>>
>> Could you please clarify why this option needs to apply to -fno-pic?  Here 
>> is what I tried with trunk clang:
>
> If the user wants to guarantee no copy relocations in -fno-pic code, they can 
> theoretically apply -fno-direct-access-external-data to use a GOT indirection.
> This is not implemented, though.
>
>> extern int var;
>> int get() { return var; }
>>
>> $ clang -S foo.c -o -
>> 
>>  movlvar, %eax
>>  popq%rbp
>> ...
>>
>> With -fno-pic,  this will never need to use -mpie-copy-relocations at all, 
>> so this case is not relevant right?  Did I miss anything?
>
> -fno-pic code can only be used with -no-pie links (position-dependent 
> executables) If var is not defined in the linked executable, it will have a 
> copy relocation.

Thanks for explaining.  I know that by default (i.e. no-pic and no-pie), copy 
relocations will be used for external data accesses.  So, you are saying that 
you are adding a mechanism to disable copy relocations for the no-pic/no-pie 
case too. Is there a need for this, purely a question.  I know that copy 
relocations are frowned upon so maybe there was a feature request. If so, 
citing that would make it more clear. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92633

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

[clang] 840e651 - [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread via cfe-commits


Author: mydeveloperday
Date: 2020-12-04T17:45:50Z
New Revision: 840e651dc6d7fe652667eb8b4d04ef4daf4769df

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

LOG: [clang-format] Improve clang-formats handling of concepts

This is a starting point to improve the handling of concepts in clang-format. 
There is currently no real formatting of concepts and this can lead to some odd 
formatting, e.g.

Reviewed By: mitchell-stellar, miscco, curdeius

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

Added: 


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

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 6c5556a94391..d6d437a0a05e 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1382,6 +1382,18 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+**BreakBeforeConceptDeclarations** (``bool``)
+  If ``true``, concept will be placed on a new line.
+
+  .. code-block:: c++
+
+true:
+ template
+ concept ...
+
+false:
+ template concept ...
+
 **BreakBeforeTernaryOperators** (``bool``)
   If ``true``, ternary operators will be placed after line breaks.
 
@@ -1901,6 +1913,25 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+**IndentRequires** (``bool``)
+  Indent the requires clause in a template
+
+  .. code-block:: c++
+
+ true:
+ template 
+   requires Iterator
+ void sort(It begin, It end) {
+   //
+ }
+
+ false:
+ template 
+ requires Iterator
+ void sort(It begin, It end) {
+   //
+ }
+
 **IndentWidth** (``unsigned``)
   The number of columns to use for indentation.
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d62c62dad3d2..d90d059e0659 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -271,6 +271,14 @@ clang-format
 };
 
 
+- Experimental Support in clang-format for concepts has been improved, to 
+  aid this the follow options have been added
+
+- Option ``IndentRequires`` has been added to indent the ``requires`` keyword
+  in templates.
+- Option ``BreakBeforeConceptDeclarations`` has been added to aid the 
formatting of concepts.
+
+
 libclang
 
 

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index dab4cbbbdfe1..6e304630fbdc 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1160,6 +1160,17 @@ struct FormatStyle {
   /// \endcode
   BraceWrappingFlags BraceWrapping;
 
+  /// If ``true``, concept will be placed on a new line.
+  /// \code
+  ///   true:
+  ///template
+  ///concept ...
+  ///
+  ///   false:
+  ///template concept ...
+  /// \endcode
+  bool BreakBeforeConceptDeclarations;
+
   /// If ``true``, ternary operators will be placed after line breaks.
   /// \code
   ///true:
@@ -1590,6 +1601,24 @@ struct FormatStyle {
   /// IndentExternBlockStyle is the type of indenting of extern blocks.
   IndentExternBlockStyle IndentExternBlock;
 
+  /// Indent the requires clause in a template
+  /// \code
+  ///true:
+  ///template 
+  ///  requires Iterator
+  ///void sort(It begin, It end) {
+  ///  //
+  ///}
+  ///
+  ///false:
+  ///template 
+  ///requires Iterator
+  ///void sort(It begin, It end) {
+  ///  //
+  ///}
+  /// \endcode
+  bool IndentRequires;
+
   /// The number of columns to use for indentation.
   /// \code
   ///IndentWidth: 3
@@ -2435,6 +2464,7 @@ struct FormatStyle {
BinPackParameters == R.BinPackParameters &&
BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
BreakBeforeBraces == R.BreakBeforeBraces &&
+   BreakBeforeConceptDeclarations == R.BreakBeforeConceptDeclarations 
&&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
BreakConstructorInitializers == R.BreakConstructorInitializers &&
CompactNamespaces == R.CompactNamespaces &&
@@ -2466,7 +2496,8 @@ struct FormatStyle {
IndentGotoLabels == R.IndentGotoLabels &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentExternBlock == R.IndentExternBlock &&
-   IndentWidth == R.IndentWidth && Language == R.Language &&
+   IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth 
&&
+

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG840e651dc6d7: [clang-format] Improve clang-formats handling 
of concepts (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79773

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14104,6 +14104,7 @@
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
@@ -14115,6 +14116,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -17288,6 +17290,277 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "

[PATCH] D92666: [clang-format] [NFC] keep clang-format tests clang-format clean

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, sammccall.
MyDeveloperDay added a project: clang-format.
MyDeveloperDay requested review of this revision.
Herald added a project: clang.

I use several of the clang-format clean directories as a test suite, this one 
had got slightly out of wack in a prior commit


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92666

Files:
  clang/unittests/Format/MacroExpanderTest.cpp


Index: clang/unittests/Format/MacroExpanderTest.cpp
===
--- clang/unittests/Format/MacroExpanderTest.cpp
+++ clang/unittests/Format/MacroExpanderTest.cpp
@@ -65,6 +65,7 @@
   << Context << " in " << text(Tokens) << " at " << File << ":" << 
Line;
 }
   }
+
 protected:
   llvm::SpecificBumpPtrAllocator Allocator;
   std::vector> Buffers;


Index: clang/unittests/Format/MacroExpanderTest.cpp
===
--- clang/unittests/Format/MacroExpanderTest.cpp
+++ clang/unittests/Format/MacroExpanderTest.cpp
@@ -65,6 +65,7 @@
   << Context << " in " << text(Tokens) << " at " << File << ":" << Line;
 }
   }
+
 protected:
   llvm::SpecificBumpPtrAllocator Allocator;
   std::vector> Buffers;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

2020-12-04 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5940
+void Sema::ActOnFinishedFunctionDefinitionInOpenMPAssumeScope(Decl *D) {
+  FunctionDecl *FD = nullptr;
+  if (auto *UTemplDecl = dyn_cast(D))

You can also check if the decl is invalid here



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5945-5946
+FD = cast(D);
+  if (!FD)
+return;
+

Looks like `FD==nullptr` is impossible here. Or 
`UTemplDecl->getTemplatedDecl()` can return `nullptr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

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

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

2020-12-04 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies updated this revision to Diff 309565.
ffrankies added a comment.

Implemented changes requested by @aaron.ballman

- using `hasAnyName()` instead of multiple `hasName()` calls in the matcher
- switched to a combination of `hasAttr<>()` and `getAttr<>()` to remove need 
for casting and looping over all attributes (did not use `specific_attrs<>()` 
because there should only be one `ReqdWorkGroupSizeAttr` per function)
- re-phrased diagnostic messages
- added all 4 ID functions to the documentation
- removed the update to BUILD.gn


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

https://reviews.llvm.org/D72241

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp
  clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-single-work-item-barrier.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/altera-single-work-item-barrier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera-single-work-item-barrier.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-single-work-item-barrier.cpp
@@ -0,0 +1,300 @@
+// RUN: %check_clang_tidy -check-suffix=OLDCLOLDAOC %s altera-single-work-item-barrier %t -- -header-filter=.* "--" -cl-std=CL1.2 -c --include opencl-c.h -DOLDCLOLDAOC
+// RUN: %check_clang_tidy -check-suffix=NEWCLOLDAOC %s altera-single-work-item-barrier %t -- -header-filter=.* "--" -cl-std=CL2.0 -c --include opencl-c.h -DNEWCLOLDAOC
+// RUN: %check_clang_tidy -check-suffix=OLDCLNEWAOC %s altera-single-work-item-barrier %t -- -config='{CheckOptions: [{key: altera-single-work-item-barrier.AOCVersion, value: 1701}]}' -header-filter=.* "--" -cl-std=CL1.2 -c --include opencl-c.h -DOLDCLNEWAOC
+// RUN: %check_clang_tidy -check-suffix=NEWCLNEWAOC %s altera-single-work-item-barrier %t -- -config='{CheckOptions: [{key: altera-single-work-item-barrier.AOCVersion, value: 1701}]}' -header-filter=.* "--" -cl-std=CL2.0 -c --include opencl-c.h -DNEWCLNEWAOC
+
+#ifdef OLDCLOLDAOC  // OpenCL 1.2 Altera Offline Compiler < 17.1
+void __kernel error_barrier_no_id(__global int * foo, int size) {
+  // CHECK-MESSAGES-OLDCLOLDAOC: :[[@LINE-1]]:15: warning: kernel function 'error_barrier_no_id' does not call 'get_global_id' or 'get_local_id' and will be treated as a single work-item [altera-single-work-item-barrier]
+  for (int j = 0; j < 256; j++) {
+	for (int i = 256; i < size; i+= 256) {
+  foo[j] += foo[j+i];
+}
+  }
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  // CHECK-MESSAGES-OLDCLOLDAOC: :[[@LINE-1]]:3: note: barrier call is in a single work-item and may error out 
+  for (int i = 1; i < 256; i++) {
+	foo[0] += foo[i];
+  }
+}
+
+void __kernel success_barrier_global_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_global_id(0);
+}
+
+void __kernel success_barrier_local_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_local_id(0);
+}
+
+void __kernel success_barrier_both_ids(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int gid = get_global_id(0);
+  int lid = get_local_id(0);
+}
+
+void success_nokernel_barrier_no_id(__global int * foo, int size) {
+  for (int j = 0; j < 256; j++) {
+	for (int i = 256; i < size; i+= 256) {
+  foo[j] += foo[j+i];
+}
+  }
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  for (int i = 1; i < 256; i++) {
+	foo[0] += foo[i];
+  }
+}
+
+void success_nokernel_barrier_global_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_global_id(0);
+}
+
+void success_nokernel_barrier_local_id(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_local_id(0);
+}
+
+void success_nokernel_barrier_both_ids(__global int * foo, int size) {
+  barrier(CLK_GLOBAL_MEM_FENCE);
+  int gid = get_global_id(0);
+  int lid = get_local_id(0);
+}
+#endif
+
+#ifdef NEWCLOLDAOC  // OpenCL 2.0 Altera Offline Compiler < 17.1
+void __kernel error_barrier_no_id(__global int * foo, int size) {
+  // CHECK-MESSAGES-NEWCLOLDAOC: :[[@LINE-1]]:15: warning: kernel function 'error_barrier_no_id' does not call 'get_global_id' or 'get_local_id' and will be treated as a single work-item [altera-single-work-item-barrier]
+  for (int j = 0; j < 256; j++) {
+	for (int i = 256; i < size; i+= 256) {
+  foo[j] += foo[j+i];
+}
+  }
+  work_group_barrier(CLK_GLOBAL_MEM_FENCE);
+  // CHECK-MESSAGES-NEWCLOLDAOC: :[[@LINE-1]]:3: note: barrier call is in a single work-item and may error out 
+  for (int i = 1; i < 256; i++) {
+	foo[0] += foo[i];
+  }
+}
+
+void __kernel success_barrier_global_id(__global int * foo, int size) {
+  work_group_barrier(CLK_GLOBAL_MEM_FENCE);
+  int tid = get_gl

[PATCH] D92661: [RFC] Fix TLS and Coroutine

2020-12-04 Thread Xun Li via Phabricator via cfe-commits

lxfind added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1309
+// Intrinsic to obtain the address of a thread_local variable.
+def int_threadlocal : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>;
+

hoy wrote:
> hoy wrote:
> > With the intrinsic, can TLS variable reference in the same coroutine or 
> > regular routine be DCE-ed anymore?
> Sorry, I meant CSE-ed.
Since the intrinsics does not have readnone attribute, it won't be CSE-ed 
before CoroSplit.
However after CoroSplit, it will be lowered back to the direct reference of the 
TLS, and will be CSE-ed by latter passes.
I can add a test function to demonstrate that too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661

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

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

2020-12-04 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington updated this revision to Diff 309569.
erik.pilkington marked 3 inline comments as done.
erik.pilkington added a comment.

Address review comments.


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

https://reviews.llvm.org/D92495

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/Sema.h
  clang/lib/APINotes/APINotesYAMLCompiler.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Index/nullability.c
  clang/test/SemaObjC/nullability.m
  clang/test/SemaObjC/nullable-result.m
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -1315,6 +1315,8 @@
 return CXTypeNullability_NonNull;
   case NullabilityKind::Nullable:
 return CXTypeNullability_Nullable;
+  case NullabilityKind::NullableResult:
+return CXTypeNullability_NullableResult;
   case NullabilityKind::Unspecified:
 return CXTypeNullability_Unspecified;
 }
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -1539,10 +1539,20 @@
 
   const char *nullability = 0;
   switch (N) {
-case CXTypeNullability_NonNull: nullability = "nonnull"; break;
-case CXTypeNullability_Nullable: nullability = "nullable"; break;
-case CXTypeNullability_Unspecified: nullability = "unspecified"; break;
-case CXTypeNullability_Invalid: break;
+  case CXTypeNullability_NonNull:
+nullability = "nonnull";
+break;
+  case CXTypeNullability_Nullable:
+nullability = "nullable";
+break;
+  case CXTypeNullability_NullableResult:
+nullability = "nullable_result";
+break;
+  case CXTypeNullability_Unspecified:
+nullability = "unspecified";
+break;
+  case CXTypeNullability_Invalid:
+break;
   }
 
   if (nullability) {
Index: clang/test/SemaObjC/nullable-result.m
===
--- /dev/null
+++ clang/test/SemaObjC/nullable-result.m
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -Wnullable-to-nonnull-conversion %s
+// RUN: %clang_cc1 -xobjective-c++ -verify -fsyntax-only -fblocks -Wnullable-to-nonnull-conversion %s
+
+@class X;
+@class NSError;
+
+_Static_assert(__has_feature(nullability_nullable_result), "");
+
+@interface SomeClass
+-(void)async_get:(void (^)(X *_Nullable_result rptr, NSError *err))completionHandler;
+@end
+
+void call(SomeClass *sc) {
+  [sc async_get:^(X *_Nullable_result rptr, NSError *err) {}];
+  [sc async_get:^(X *_Nullable rptr, NSError *err) {}];
+}
+
+void test_conversion() {
+  X *_Nullable_result nr;
+  X *_Nonnull l = nr; // expected-warning{{implicit conversion from nullable pointer 'X * _Nullable_result' to non-nullable pointer type 'X * _Nonnull'}}
+
+  (void)^(X * _Nullable_result p) {
+X *_Nonnull l = p; // expected-warning{{implicit conversion from nullable pointer 'X * _Nullable_result' to non-nullable pointer type 'X * _Nonnull'}}
+  };
+}
+
+void test_dup() {
+  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate nullability specifier _Nullable_result}}
+  id _Nullable _Nullable_result b; // expected-error{{nullability specifier _Nullable_result conflicts with existing specifier '_Nullable'}}
+  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier '_Nonnull' conflicts with existing specifier _Nullable_result}}
+}
+
+@interface NoContextSensitive
+-(nullable_result id)m; // expected-error {{expected a type}}
+@property(assign, nullable_result) id p; // expected-error{{unknown property attribute 'nullable_result'}}
+@end
Index: clang/test/SemaObjC/nullability.m
===
--- clang/test/SemaObjC/nullability.m
+++ clang/test/SemaObjC/nullability.m
@@ -116,11 +116,13 @@
 - (nonnull id)returnsNonNull;
 - (nullable id)returnsNullable;
 - (null_unspecified id)returnsNullUnspecified;
+- (_Nullable_result id)returnsNullableResult;
 @end
 
 void test_receiver_merge(NSMergeReceiver *none,
  _Nonnull NSMergeReceiver *nonnull,
  _Nullable NSMergeReceiver *nullable,
+

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread Johel Ernesto Guerrero Peña via Phabricator via cfe-commits

JohelEGP added a comment.

I was a bit late, but thanks for everything!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79773

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

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

2020-12-04 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington added inline comments.



Comment at: clang/test/SemaObjC/nullable-result.m:6
+@class NSError;
+
+@interface SomeClass

doug.gregor wrote:
> Can you add a `__has_attribute` check for `_Nullable_result`?
Hmm, it looks like __has_attribute doesn't support keyword attributes. I'll add 
a `__has_feature`.


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

https://reviews.llvm.org/D92495

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

1 2 3 >

1 - 100 of 247 matches

Mail list logo