[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D66765#1645726 , @Szelethus wrote:

> Please note that LLVM 9.0.0-final is due on the 28th of August.


There are still a lot of open bugs, so it will probably slip at least a little.




Comment at: clang/docs/ReleaseNotes.rst:237-238
+
+- New package: 'apiModeling.llvm' contains modeling checkers to improve the
+  accuracy of reports on LLVM's codebase.
+

NoQ wrote:
> NoQ wrote:
> > "On LLVM's **own** codebase" :)
> > 
> > @Charusso: I think we need to cherry-pick all of our latest patches to this 
> > checker to 9.0 because otherwise it'll be crashing with the 
> > `isValidBaseClass()` assertion.
> >>! In D66765#1645726, @Szelethus wrote:
> > Please note that LLVM 9.0.0-final is due on the 28th of August.
> 
> Mmm, most likely too late.
"all of our latest patches" sounds like a lot, so yeah probably too late.

If it's broken, maybe don't point it out in the release notes. Fixes can go in 
9.0.1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66765



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

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:269
+
+- New frontend flag: ``-analyzer-werror`` to turn analyzer warnings into 
errors.
 

Could you please also add:
- Numerous`` ASTImporter`` related fixes and improvements which increase the 
stability of CTU.
- CTU is enabled to inline virtual functions too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66765



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

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 217319.
ilya-biryukov added a comment.

- Add a comment to CreateFromArgs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66731

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -24,14 +24,9 @@
 using namespace clang;
 using namespace llvm::opt;
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object for
-/// a command line argument vector.
-///
-/// \return A CompilerInvocation, or 0 if none was built for the given
-/// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -95,11 +90,10 @@
 
   const ArgStringList &CCArgs = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(*CI,
- const_cast(CCArgs.data()),
- const_cast(CCArgs.data()) +
- CCArgs.size(),
- *Diags))
+  if (!CompilerInvocation::CreateFromArgs(
+  *CI, const_cast(CCArgs.data()),
+  const_cast(CCArgs.data()) + CCArgs.size(), *Diags) &&
+  !ShouldRecoverOnErorrs)
 return nullptr;
   return CI;
 }
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -213,13 +213,18 @@
 /// createInvocationFromCommandLine - Construct a compiler invocation object for
 /// a command line argument vector.
 ///
+/// \param ShouldRecoverOnErrors - whether we should attempt to return a
+/// non-null (and possibly incorrect) CompilerInvocation if any errors were
+/// encountered. When this flag is false, always return null on errors.
+///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
-IntrusiveRefCntPtr VFS = nullptr);
+IntrusiveRefCntPtr VFS = nullptr,
+bool ShouldRecoverOnErrors = false);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: clang/include/clang/Frontend/CompilerInvocation.h
===
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -147,6 +147,11 @@
   /// Create a compiler invocation from a list of input options.
   /// \returns true on success.
   ///
+  /// If any error was encountered while parsing the arguments, \returns false
+  /// and will attempts to recover and continue parsing command-line arguments.
+  /// The recovery is best-effort and the only guarantee is that \p Res will be
+  /// in one of the vaild-to-access (albeit arbitrary) states.
+  ///
   /// \param [out] Res - The resulting invocation.
   /// \param ArgBegin - The first element in the argument vector.
   /// \param ArgEnd - The last element in the argument vector.
Index: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
@@ -9,7 +9,9 @@
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
@@ -244,6 +246,20 @@
   EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
 }
 
+TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
+  // Unknown flags should not prevent a build of compiler invocation.
+  ParseInputs Inputs;
+  Inputs.FS = buildTestFS({{testPath("foo.cpp"), "void test() {}"}});
+  Inputs.CompileCommand.CommandLine = {"clang", "-fsome-unknown-flag",
+   testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+
+  // Unknown forwarded to -cc1 should not a failure either.
+  Inpu

[PATCH] D66787: GlobList: added a clear test for pattern priority

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gribozavr added a reviewer: ilya-biryukov.

The last glob that matches the string decides whether that string is
included or excluded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66787

Files:
  clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -31,7 +31,7 @@
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, Simple) {
+TEST(GlobList, OneSimplePattern) {
   GlobList Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
@@ -41,6 +41,40 @@
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
+TEST(GlobList, TwoSimplePatterns) {
+  GlobList Filter("aaa,bbb");
+
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_TRUE(Filter.contains("bbb"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aa"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains(""));
+}
+
+TEST(GlobList, PatternPriority) {
+  // The last glob that matches the string decides whether that string is
+  // included or excluded.
+  {
+GlobList Filter("a*,-aaa");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_FALSE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+  {
+GlobList Filter("-aaa,a*");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_TRUE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+}
+
 TEST(GlobList, WhitespacesAtBegin) {
   GlobList Filter("-*,   a.b.*");
 


Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -31,7 +31,7 @@
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, Simple) {
+TEST(GlobList, OneSimplePattern) {
   GlobList Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
@@ -41,6 +41,40 @@
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
+TEST(GlobList, TwoSimplePatterns) {
+  GlobList Filter("aaa,bbb");
+
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_TRUE(Filter.contains("bbb"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aa"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains(""));
+}
+
+TEST(GlobList, PatternPriority) {
+  // The last glob that matches the string decides whether that string is
+  // included or excluded.
+  {
+GlobList Filter("a*,-aaa");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_FALSE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+  {
+GlobList Filter("-aaa,a*");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_TRUE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+}
+
 TEST(GlobList, WhitespacesAtBegin) {
   GlobList Filter("-*,   a.b.*");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 217320.
ilya-biryukov added a comment.

- Fix english grammar in the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66731

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -24,14 +24,9 @@
 using namespace clang;
 using namespace llvm::opt;
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object for
-/// a command line argument vector.
-///
-/// \return A CompilerInvocation, or 0 if none was built for the given
-/// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -95,11 +90,10 @@
 
   const ArgStringList &CCArgs = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(*CI,
- const_cast(CCArgs.data()),
- const_cast(CCArgs.data()) +
- CCArgs.size(),
- *Diags))
+  if (!CompilerInvocation::CreateFromArgs(
+  *CI, const_cast(CCArgs.data()),
+  const_cast(CCArgs.data()) + CCArgs.size(), *Diags) &&
+  !ShouldRecoverOnErorrs)
 return nullptr;
   return CI;
 }
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -213,13 +213,18 @@
 /// createInvocationFromCommandLine - Construct a compiler invocation object for
 /// a command line argument vector.
 ///
+/// \param ShouldRecoverOnErrors - whether we should attempt to return a
+/// non-null (and possibly incorrect) CompilerInvocation if any errors were
+/// encountered. When this flag is false, always return null on errors.
+///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
-IntrusiveRefCntPtr VFS = nullptr);
+IntrusiveRefCntPtr VFS = nullptr,
+bool ShouldRecoverOnErrors = false);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: clang/include/clang/Frontend/CompilerInvocation.h
===
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -147,6 +147,11 @@
   /// Create a compiler invocation from a list of input options.
   /// \returns true on success.
   ///
+  /// If an error was encountered while parsing the arguments, \returns false
+  /// and attempts to recover and continue parsing the rest of the arguments.
+  /// The recovery is best-effort and only guarantees that \p Res will end up in
+  /// one of the vaild-to-access (albeit arbitrary) states.
+  ///
   /// \param [out] Res - The resulting invocation.
   /// \param ArgBegin - The first element in the argument vector.
   /// \param ArgEnd - The last element in the argument vector.
Index: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
@@ -9,7 +9,9 @@
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
@@ -244,6 +246,20 @@
   EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
 }
 
+TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
+  // Unknown flags should not prevent a build of compiler invocation.
+  ParseInputs Inputs;
+  Inputs.FS = buildTestFS({{testPath("foo.cpp"), "void test() {}"}});
+  Inputs.CompileCommand.CommandLine = {"clang", "-fsome-unknown-flag",
+   testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+
+  // Unknown forwarded to -cc1 should not a failure either.
+  Inputs

[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

[Dmitri Gribenko via Phabricator via cfe-commits]

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

I think it makes method implementations more obvious.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66788

Files:
  clang-tools-extra/clang-tidy/GlobList.cpp
  clang-tools-extra/clang-tidy/GlobList.h


Index: clang-tools-extra/clang-tidy/GlobList.h
===
--- clang-tools-extra/clang-tidy/GlobList.h
+++ clang-tools-extra/clang-tidy/GlobList.h
@@ -12,7 +12,7 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
-#include 
+#include 
 
 namespace clang {
 namespace tidy {
@@ -28,14 +28,15 @@
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) { return contains(S, false); }
+  bool contains(StringRef S);
 
 private:
-  bool contains(StringRef S, bool Contains);
 
-  bool Positive;
-  llvm::Regex Regex;
-  std::unique_ptr NextGlob;
+  struct GlobListItem {
+bool IsPositive;
+mutable llvm::Regex Regex;
+  };
+  std::vector Items;
 };
 
 } // end namespace tidy
Index: clang-tools-extra/clang-tidy/GlobList.cpp
===
--- clang-tools-extra/clang-tidy/GlobList.cpp
+++ clang-tools-extra/clang-tidy/GlobList.cpp
@@ -42,15 +42,20 @@
   return llvm::Regex(RegexText);
 }
 
-GlobList::GlobList(StringRef Globs)
-: Positive(!ConsumeNegativeIndicator(Globs)), Regex(ConsumeGlob(Globs)),
-  NextGlob(Globs.empty() ? nullptr : new GlobList(Globs)) {}
-
-bool GlobList::contains(StringRef S, bool Contains) {
-  if (Regex.match(S))
-Contains = Positive;
+GlobList::GlobList(StringRef Globs) {
+  do {
+GlobListItem Item;
+Item.IsPositive = !ConsumeNegativeIndicator(Globs);
+Item.Regex = ConsumeGlob(Globs);
+Items.push_back(std::move(Item));
+  } while (!Globs.empty());
+}
 
-  if (NextGlob)
-Contains = NextGlob->contains(S, Contains);
+bool GlobList::contains(StringRef S) {
+  bool Contains = false;
+  for (const GlobListItem &Item : Items) {
+if (Item.Regex.match(S))
+  Contains = Item.IsPositive;
+  }
   return Contains;
 }


Index: clang-tools-extra/clang-tidy/GlobList.h
===
--- clang-tools-extra/clang-tidy/GlobList.h
+++ clang-tools-extra/clang-tidy/GlobList.h
@@ -12,7 +12,7 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
-#include 
+#include 
 
 namespace clang {
 namespace tidy {
@@ -28,14 +28,15 @@
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) { return contains(S, false); }
+  bool contains(StringRef S);
 
 private:
-  bool contains(StringRef S, bool Contains);
 
-  bool Positive;
-  llvm::Regex Regex;
-  std::unique_ptr NextGlob;
+  struct GlobListItem {
+bool IsPositive;
+mutable llvm::Regex Regex;
+  };
+  std::vector Items;
 };
 
 } // end namespace tidy
Index: clang-tools-extra/clang-tidy/GlobList.cpp
===
--- clang-tools-extra/clang-tidy/GlobList.cpp
+++ clang-tools-extra/clang-tidy/GlobList.cpp
@@ -42,15 +42,20 @@
   return llvm::Regex(RegexText);
 }
 
-GlobList::GlobList(StringRef Globs)
-: Positive(!ConsumeNegativeIndicator(Globs)), Regex(ConsumeGlob(Globs)),
-  NextGlob(Globs.empty() ? nullptr : new GlobList(Globs)) {}
-
-bool GlobList::contains(StringRef S, bool Contains) {
-  if (Regex.match(S))
-Contains = Positive;
+GlobList::GlobList(StringRef Globs) {
+  do {
+GlobListItem Item;
+Item.IsPositive = !ConsumeNegativeIndicator(Globs);
+Item.Regex = ConsumeGlob(Globs);
+Items.push_back(std::move(Item));
+  } while (!Globs.empty());
+}
 
-  if (NextGlob)
-Contains = NextGlob->contains(S, Contains);
+bool GlobList::contains(StringRef S) {
+  bool Contains = false;
+  for (const GlobListItem &Item : Items) {
+if (Item.Regex.match(S))
+  Contains = Item.IsPositive;
+  }
   return Contains;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66787: GlobList: added a clear test for pattern priority

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov 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/D66787/new/

https://reviews.llvm.org/D66787



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

[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:46
+GlobList::GlobList(StringRef Globs) {
+  do {
+GlobListItem Item;

NIT: I suggest using `for (;!Globs.empty();) {}` to make the stop condition 
stand out more.
Not a big deal, though, feel free to keep as is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66788



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

[PATCH] D66590: [clangd] Fix toHalfOpenFileRange where start/end endpoints are in different files due to #include

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/SourceCode.cpp:284
+  return SM.getComposedLoc(IncludingFile, Offset);
+if (Buf[Offset] == '\n' || Offset == 0) // no hash, what's going on?
+  return SourceLocation();

SureYeaah wrote:
> nit: use this as a while condition?
> 
> ```
> while(Offset-- && Buf[Offset] != '\n')
> ```
Ooh, clever... a little too subtle for my taste though :-)
(In particular, allowing the variable to overflow but then ensuring we never 
read from it...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66590



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

[clang-tools-extra] r370028 - GlobList: added a clear test for pattern priority

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Tue Aug 27 01:42:42 2019
New Revision: 370028

URL: http://llvm.org/viewvc/llvm-project?rev=370028&view=rev
Log:
GlobList: added a clear test for pattern priority

Summary:
The last glob that matches the string decides whether that string is
included or excluded.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/GlobListTest.cpp

Modified: clang-tools-extra/trunk/unittests/clang-tidy/GlobListTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/GlobListTest.cpp?rev=370028&r1=370027&r2=370028&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/GlobListTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/GlobListTest.cpp Tue Aug 27 
01:42:42 2019
@@ -31,7 +31,7 @@ TEST(GlobList, Everything) {
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, Simple) {
+TEST(GlobList, OneSimplePattern) {
   GlobList Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
@@ -41,6 +41,40 @@ TEST(GlobList, Simple) {
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
+TEST(GlobList, TwoSimplePatterns) {
+  GlobList Filter("aaa,bbb");
+
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_TRUE(Filter.contains("bbb"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aa"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains(""));
+}
+
+TEST(GlobList, PatternPriority) {
+  // The last glob that matches the string decides whether that string is
+  // included or excluded.
+  {
+GlobList Filter("a*,-aaa");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_FALSE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+  {
+GlobList Filter("-aaa,a*");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_TRUE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+}
+
 TEST(GlobList, WhitespacesAtBegin) {
   GlobList Filter("-*,   a.b.*");
 


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

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:150
   ///
+  /// If an error was encountered while parsing the arguments, \returns false
+  /// and attempts to recover and continue parsing the rest of the arguments.

\returns false if an error...

(\returns is a paragraph command. It starts a new paragraph.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66731



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

[clang-tools-extra] r370029 - [clangd] Fix toHalfOpenFileRange where start/end endpoints are in different files due to #include

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Aug 27 01:44:06 2019
New Revision: 370029

URL: http://llvm.org/viewvc/llvm-project?rev=370029&view=rev
Log:
[clangd] Fix toHalfOpenFileRange where start/end endpoints are in different 
files due to #include

Summary: https://github.com/clangd/clangd/issues/129

Reviewers: SureYeaah

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=370029&r1=370028&r2=370029&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Aug 27 01:44:06 2019
@@ -264,6 +264,29 @@ bool halfOpenRangeTouches(const SourceMa
   return L == R.getEnd() || halfOpenRangeContains(Mgr, R, L);
 }
 
+SourceLocation includeHashLoc(FileID IncludedFile, const SourceManager &SM) {
+  assert(SM.getLocForEndOfFile(IncludedFile).isFileID());
+  FileID IncludingFile;
+  unsigned Offset;
+  std::tie(IncludingFile, Offset) =
+  SM.getDecomposedExpansionLoc(SM.getIncludeLoc(IncludedFile));
+  bool Invalid = false;
+  llvm::StringRef Buf = SM.getBufferData(IncludingFile, &Invalid);
+  if (Invalid)
+return SourceLocation();
+  // Now buf is "...\n#include \n..."
+  // and Offset points here:   ^
+  // Rewind to the preceding # on the line.
+  assert(Offset < Buf.size());
+  for (;; --Offset) {
+if (Buf[Offset] == '#')
+  return SM.getComposedLoc(IncludingFile, Offset);
+if (Buf[Offset] == '\n' || Offset == 0) // no hash, what's going on?
+  return SourceLocation();
+  }
+}
+
+
 static unsigned getTokenLengthAtLoc(SourceLocation Loc, const SourceManager 
&SM,
 const LangOptions &LangOpts) {
   Token TheTok;
@@ -308,50 +331,61 @@ static SourceRange toTokenRange(CharSour
 static SourceRange unionTokenRange(SourceRange R1, SourceRange R2,
const SourceManager &SM,
const LangOptions &LangOpts) {
-  SourceLocation E1 = getLocForTokenEnd(R1.getEnd(), SM, LangOpts);
-  SourceLocation E2 = getLocForTokenEnd(R2.getEnd(), SM, LangOpts);
-  return SourceRange(std::min(R1.getBegin(), R2.getBegin()),
- E1 < E2 ? R2.getEnd() : R1.getEnd());
-}
-
-// Check if two locations have the same file id.
-static bool inSameFile(SourceLocation Loc1, SourceLocation Loc2,
-   const SourceManager &SM) {
-  return SM.getFileID(Loc1) == SM.getFileID(Loc2);
-}
-
-// Find an expansion range (not necessarily immediate) the ends of which are in
-// the same file id.
-static SourceRange
-getExpansionTokenRangeInSameFile(SourceLocation Loc, const SourceManager &SM,
- const LangOptions &LangOpts) {
-  SourceRange ExpansionRange =
-  toTokenRange(SM.getImmediateExpansionRange(Loc), SM, LangOpts);
+  SourceLocation Begin =
+  SM.isBeforeInTranslationUnit(R1.getBegin(), R2.getBegin())
+  ? R1.getBegin()
+  : R2.getBegin();
+  SourceLocation End =
+  SM.isBeforeInTranslationUnit(getLocForTokenEnd(R1.getEnd(), SM, 
LangOpts),
+   getLocForTokenEnd(R2.getEnd(), SM, 
LangOpts))
+  ? R2.getEnd()
+  : R1.getEnd();
+  return SourceRange(Begin, End);
+}
+
+// Given a range whose endpoints may be in different expansions or files,
+// tries to find a range within a common file by following up the expansion and
+// include location in each.
+static SourceRange rangeInCommonFile(SourceRange R, const SourceManager &SM,
+ const LangOptions &LangOpts) {
   // Fast path for most common cases.
-  if (inSameFile(ExpansionRange.getBegin(), ExpansionRange.getEnd(), SM))
-return ExpansionRange;
+  if (SM.isWrittenInSameFile(R.getBegin(), R.getEnd()))
+return R;
   // Record the stack of expansion locations for the beginning, keyed by 
FileID.
   llvm::DenseMap BeginExpansions;
-  for (SourceLocation Begin = ExpansionRange.getBegin(); Begin.isValid();
+  for (SourceLocation Begin = R.getBegin(); Begin.isValid();
Begin = Begin.isFileID()
-   ? SourceLocation()
+   ? includeHashLoc(SM.getFileID(Begin), SM)
: SM.getImmediateExpansionRange(Begin).getBegin()) {
 BeginExpansions[SM.getFileID(Begin)] = Begin;
   }
   // Move up the stack of expansion locations for the end until we find the
   // location in BeginExpansions with that has the same file id.
-  for (SourceLocation End = ExpansionRange

[PATCH] D66787: GlobList: added a clear test for pattern priority

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd2315ce2101: GlobList: added a clear test for pattern 
priority (authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66787

Files:
  clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -31,7 +31,7 @@
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, Simple) {
+TEST(GlobList, OneSimplePattern) {
   GlobList Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
@@ -41,6 +41,40 @@
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
+TEST(GlobList, TwoSimplePatterns) {
+  GlobList Filter("aaa,bbb");
+
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_TRUE(Filter.contains("bbb"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aa"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains(""));
+}
+
+TEST(GlobList, PatternPriority) {
+  // The last glob that matches the string decides whether that string is
+  // included or excluded.
+  {
+GlobList Filter("a*,-aaa");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_FALSE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+  {
+GlobList Filter("-aaa,a*");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_TRUE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+}
+
 TEST(GlobList, WhitespacesAtBegin) {
   GlobList Filter("-*,   a.b.*");
 


Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -31,7 +31,7 @@
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, Simple) {
+TEST(GlobList, OneSimplePattern) {
   GlobList Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
@@ -41,6 +41,40 @@
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
+TEST(GlobList, TwoSimplePatterns) {
+  GlobList Filter("aaa,bbb");
+
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_TRUE(Filter.contains("bbb"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aa"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains(""));
+}
+
+TEST(GlobList, PatternPriority) {
+  // The last glob that matches the string decides whether that string is
+  // included or excluded.
+  {
+GlobList Filter("a*,-aaa");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_FALSE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+  {
+GlobList Filter("-aaa,a*");
+
+EXPECT_FALSE(Filter.contains(""));
+EXPECT_TRUE(Filter.contains("a"));
+EXPECT_TRUE(Filter.contains("aa"));
+EXPECT_TRUE(Filter.contains("aaa"));
+EXPECT_TRUE(Filter.contains(""));
+  }
+}
+
 TEST(GlobList, WhitespacesAtBegin) {
   GlobList Filter("-*,   a.b.*");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66590: [clangd] Fix toHalfOpenFileRange where start/end endpoints are in different files due to #include

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370029: [clangd] Fix toHalfOpenFileRange where start/end 
endpoints are in different… (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66590?vs=216609&id=217325#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66590

Files:
  clang-tools-extra/trunk/clangd/SourceCode.cpp
  clang-tools-extra/trunk/clangd/SourceCode.h
  clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp

Index: clang-tools-extra/trunk/clangd/SourceCode.h
===
--- clang-tools-extra/trunk/clangd/SourceCode.h
+++ clang-tools-extra/trunk/clangd/SourceCode.h
@@ -83,6 +83,11 @@
 /// the main file.
 bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM);
 
+/// Returns the #include location through which IncludedFIle was loaded.
+/// Where SM.getIncludeLoc() returns the location of the *filename*, which may
+/// be in a macro, includeHashLoc() returns the location of the #.
+SourceLocation includeHashLoc(FileID IncludedFile, const SourceManager &SM);
+
 /// Returns true if the token at Loc is spelled in the source code.
 /// This is not the case for:
 ///   * symbols formed via macro concatenation, the spelling location will
Index: clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
@@ -11,9 +11,11 @@
 #include "SourceCode.h"
 #include "TestTU.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -505,6 +507,53 @@
   CheckRange("f");
 }
 
+TEST(SourceCodeTests, HalfOpenFileRangePathologicalPreprocessor) {
+  const char *Case = R"cpp(
+#define MACRO while(1)
+void test() {
+[[#include "Expand.inc"
+br^eak]];
+}
+  )cpp";
+  Annotations Test(Case);
+  auto TU = TestTU::withCode(Test.code());
+  TU.AdditionalFiles["Expand.inc"] = "MACRO\n";
+  auto AST = TU.build();
+
+  const auto &Func = cast(findDecl(AST, "test"));
+  const auto &Body = cast(Func.getBody());
+  const auto &Loop = cast(*Body->child_begin());
+  llvm::Optional Range = toHalfOpenFileRange(
+  AST.getSourceManager(), AST.getASTContext().getLangOpts(),
+  Loop->getSourceRange());
+  ASSERT_TRUE(Range) << "Failed to get file range";
+  EXPECT_EQ(AST.getSourceManager().getFileOffset(Range->getBegin()),
+Test.llvm::Annotations::range().Begin);
+  EXPECT_EQ(AST.getSourceManager().getFileOffset(Range->getEnd()),
+Test.llvm::Annotations::range().End);
+}
+
+TEST(SourceCodeTests, IncludeHashLoc) {
+  const char *Case = R"cpp(
+$foo^#include "foo.inc"
+#define HEADER "bar.inc"
+  $bar^#  include HEADER
+  )cpp";
+  Annotations Test(Case);
+  auto TU = TestTU::withCode(Test.code());
+  TU.AdditionalFiles["foo.inc"] = "int foo;\n";
+  TU.AdditionalFiles["bar.inc"] = "int bar;\n";
+  auto AST = TU.build();
+  const auto& SM = AST.getSourceManager();
+
+  FileID Foo = SM.getFileID(findDecl(AST, "foo").getLocation());
+  EXPECT_EQ(SM.getFileOffset(includeHashLoc(Foo, SM)),
+Test.llvm::Annotations::point("foo"));
+  FileID Bar = SM.getFileID(findDecl(AST, "bar").getLocation());
+  EXPECT_EQ(SM.getFileOffset(includeHashLoc(Bar, SM)),
+Test.llvm::Annotations::point("foo"));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
@@ -346,6 +346,25 @@
   }
 }
 
+TEST(SelectionTest, PathologicalPreprocessor) {
+  const char *Case = R"cpp(
+#define MACRO while(1)
+void test() {
+#include "Expand.inc"
+br^eak;
+}
+  )cpp";
+  Annotations Test(Case);
+  auto TU = TestTU::withCode(Test.code());
+  TU.AdditionalFiles["Expand.inc"] = "MACRO\n";
+  auto AST = TU.build();
+  EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty());
+  auto T = makeSelectionTree(Case, AST);
+
+  EXPECT_EQ("BreakStmt", T.commonAncestor()->kind());
+  EXPECT_EQ("WhileStmt", T.commonAncestor()->Parent->kind());
+}
+
 TEST(SelectionTest, Implicit) {
   const char* Test = R"cpp(
 struct S { S(const char*); };
Index: clang-tools-extra/trunk/clangd/SourceCode.cpp
=

[PATCH] D66759: [clangd] Surface errors from command-line parsing

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Thanks! This looks really useful when we can't build an AST due to unknown 
compiler commands, but I am not sure about how useful it is to surface 
command-line parsing errors once we are able to build an AST.
Because people most likely won't care about these errors once clangd is working 
for them, and even get annoyed since most of the developers has little control 
over how their build system generates their compile
commands, therefore it will be really hard to get rid of those errors showing 
up in all the files they want to edit.

So I suggest only surfacing these if we are not able to create a compiler 
invocation or build an ast at all. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66759



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

[PATCH] D61758: [driver][xray] fix the macOS support checker by supporting -macos triple in addition to -darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: ributzka.

LGTM -- apologies for the delay, I hope this isn't too late.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61758



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

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Mostly LGTM, thanks! Just a few clarifying questions and suggestions

This API allows us to get a target declaration for a reference, but does not 
help with finding the source locations for those references. Do you plan to put 
this into the API? Have this as a separate function?
E.g. finding a source location of `field` for `DeclRefExpr` produced for 
`MyBase::field` seems to be the same amount of work (in general case) as 
finding the `Decl*` of `field`.




Comment at: clang-tools-extra/clangd/FindTarget.cpp:374
+  const char *Sep = "";
+  for (unsigned I = 0; I < RS.S.size(); ++I)
+if (RS.S.test(I)) {

NIT: maybe add braces around multi-line body of the for statement?



Comment at: clang-tools-extra/clangd/FindTarget.h:70
+llvm::SmallVector
+targetDecl(const ast_type_traits::DynTypedNode &, DeclRelationSet Mask);
+

Could we add convenience overloads for `Expr*`, `QualType`, `TypeLoc` and maybe 
other interesting cases?
Having to call `DynTypedNode::create` for each invocation of these two 
functions would feel awkward, saving some typing for the clients seems to be 
worth a little boilerplate.



Comment at: clang-tools-extra/clangd/FindTarget.h:86
+  /// This declaration is an alias that was referred to.
+  Alias,
+  /// This is the underlying declaration for an alias, decltype etc.

Could you provide examples for `Alias` and `Underlying` in the comments?
One with a template and one with a namespace alias



Comment at: clang-tools-extra/clangd/FindTarget.h:92
+  /// (e.g. a delegating constructor has a pure-lexical reference to the class)
+  PureLexical,
+};

Could you also provide an example here?
It this a delegating constructor in the constructor-init-list?



Comment at: clang-tools-extra/clangd/FindTarget.h:103
+  DeclRelationSet() = default;
+  DeclRelationSet(DeclRelation R) { S.set(static_cast(R)); 
}
+

Why not `unsigned`? What are the interesting corner cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751



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

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks, and sorry for the delay. Back in the office now, and I am addressing 
this:

> I think it would be slightly better to split off the change to disable 
> vectorization via llvm.loop.vectorize.enable=false instead of width=1.

Yep, I agree


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

https://reviews.llvm.org/D66290



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

[clang-tools-extra] r370030 - [clangd] Fix for r370029 test that got left in my client

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Aug 27 02:27:00 2019
New Revision: 370030

URL: http://llvm.org/viewvc/llvm-project?rev=370030&view=rev
Log:
[clangd] Fix for r370029 test that got left in my client

Modified:
clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp

Modified: clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp?rev=370030&r1=370029&r2=370030&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp Tue Aug 27 
02:27:00 2019
@@ -551,7 +551,7 @@ $foo^#include "foo.inc"
 Test.llvm::Annotations::point("foo"));
   FileID Bar = SM.getFileID(findDecl(AST, "bar").getLocation());
   EXPECT_EQ(SM.getFileOffset(includeHashLoc(Bar, SM)),
-Test.llvm::Annotations::point("foo"));
+Test.llvm::Annotations::point("bar"));
 }
 
 } // namespace


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

[PATCH] D66743: [clangd] Cleans up the semantic highlighting resources if clangd crashes.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:60
+  // Any disposables that should be cleaned up when clangd crashes.
+  private disposables: vscode.Disposable[] = [];
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {

nit: call it `subscriptions` to better align with vscode pattern.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:114
+  // restarts.
+  public crashDispose() {
+this.disposables.forEach((d) => d.dispose());

we just dispose some class members (leading the class to an intermediate 
state), I'd suggest that we dispose the whole class.

nit: please remove the "clangd crashes" bit in the API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66743



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via Phabricator via cfe-commits]

martong marked 4 inline comments as done.
martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3452
 << FoundField->getType();
-
-  return make_error(ImportError::NameConflict);
+  ConflictingDecls.push_back(FoundField);
 }

shafik wrote:
> I am a little concerned about this change. I am wondering if this was 
> previously handled differently as a band-aid for some other issues that only 
> showed up in rare cases when they iterated over all the results but not when 
> they errored out on the first. 
> 
> Could we make the changes to `VisitFieldDecl` a separate PR so it is easier 
> to revert later on if we do find this causes a regression we are not 
> currently covering?
Ok, I reverted these hunks. I am going to create a follow-up patch which will 
include these changes.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5615
+
+TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) {
+  Decl *ToTU = getToTuDecl(

shafik wrote:
> Since the "Liberal" strategy should be the current status quo, I think it 
> might make sense to have a first PR that just has the `*LiberalStrategy` 
> tests to verify that indeed this is the current behavior as we expect.
Ok, I removed the test suite `ConflictingDeclsWithConservativeStrategy`. I am 
going to create a subsequent patch which adds comprehensive testing for both 
strategies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692



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

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

[David CARLIER via Phabricator via cfe-commits]

devnexen added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:240
+
+- The Static Analyzer recieved a
+  :ref:`developer documentation `.

typo `recieved`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66765



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 217330.
martong marked 2 inline comments as done.
martong added a comment.

- Revert changes in VisitFieldDecl and in VisitIndirectFieldDecl
- Remove test suite ConflictingDeclsWithConservativeStrategy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/unittests/AST/ASTImporterFixtures.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/include/lldb/Symbol/ClangASTImporter.h

Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -252,7 +252,9 @@
 : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
  master.m_file_manager, true /*minimal*/),
   m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
-  m_master(master), m_source_ctx(source_ctx) {}
+  m_master(master), m_source_ctx(source_ctx) {
+setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+  }
 
 /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate
 /// and deattaches it at the end of the scope. Supports being used multiple
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -1879,7 +1879,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2432,6 +2432,62 @@
   EXPECT_EQ(ToD1, ToD2);
 }
 
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5127,15 +5183,6 @@
   }
 }
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest,
-DefaultTestValuesForRunOptions, );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
-::testing::Values(ArgVector()), );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
-::testing::Values(ArgVector()), );
-
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
@@ -5233,8 +5280,8 @@
   // prototype (inside 'friend') for it comes first in the AST and is not
   // linked to the definition.
   EXPECT_EQ(ImportedDef, ToClassDef);
-}  
-  
+}
+
 struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
   LLDBLookupTest() {
 Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
@@ -5362,10 +5409,205 @@
   EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
 }
 
+struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
+  ConflictingDeclsWithLiberalStrategy() {
+this->ODRHandling = ASTImporter::ODRHandlingType::Liberal;
+  }
+};
+
+// Check that a Decl has been successfully imported into a standalone redecl
+// chain.
+template 
+static void CheckImportedAsNew(llvm::Expected &Result, Decl *ToTU,
+   PatternTy Pattern) {
+ 

[PATCH] D66759: [clangd] Surface errors from command-line parsing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D66759#1646509 , @kadircet wrote:

> Thanks! This looks really useful when we can't build an AST due to unknown 
> compiler commands, but I am not sure about how useful it is to surface 
> command-line parsing errors once we are able to build an AST.
>  Because people most likely won't care about these errors once clangd is 
> working for them, and even get annoyed since most of the developers has 
> little control over how their build system generates their compile
>  commands, therefore it will be really hard to get rid of those errors 
> showing up in all the files they want to edit.


How common is this? Do we have any particular examples?

> So I suggest only surfacing these if we are not able to create a compiler 
> invocation or build an ast at all. WDYT?

My worry comes from the fact that I don't think we can guarantee that clangd is 
working for the users after we encountered those errors.
In particular, after D66731 , we will be 
getting a compiler invocation and building the AST in many more cases. However, 
the AST might be completely bogus, e.g. because we did not recognize actually 
important flags.
Surfacing this error to the user is useful, because that would explain why 
clangd is rusty, gives weird results, etc.

In turn, the users will report bugs and we'll be able to collect data on 
whether this is noisy or everyone is doing compile_commands.json wrong and we 
should do something about it or whether it's actually very rare in practice and 
turns out to be a useful indicator for the users.
Either result seems plausible.
I'm all in for hiding annoying errors, but I don't know what the failure modes 
are in that particular case and I'd be tempted to first see whether surfacing 
the errors would **actually** end up producing those annoying errors and what 
the errors are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66759



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

[PATCH] D66792: [ReleaseNotes] MemorySanitizer support of ASLR on FreeBSD

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: sylvestre.ledru, kcc.
devnexen created this object with visibility "All Users".
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D66792

Files:
  clang/docs/MemorySanitizer.rst


Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -204,6 +204,9 @@
   non-position-independent executables, and could fail on some Linux
   kernel versions with disabled ASLR. Refer to documentation for older versions
   for more details.
+* MemorySanitizer might be incompatible with position-independent executables
+  from FreeBSD 13 but there is a check done at runtime and throws a warning
+  in this case.
 
 Current Status
 ==


Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -204,6 +204,9 @@
   non-position-independent executables, and could fail on some Linux
   kernel versions with disabled ASLR. Refer to documentation for older versions
   for more details.
+* MemorySanitizer might be incompatible with position-independent executables
+  from FreeBSD 13 but there is a check done at runtime and throws a warning
+  in this case.
 
 Current Status
 ==
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r369749 - [Docs][OpenCL] Several corrections to C++ for OpenCL

[Anastasia Stulova via cfe-commits]

Hi Hans,


Can this be merged to the release, please?


Thank you,

Anastasia



From: cfe-commits  on behalf of Anastasia 
Stulova via cfe-commits 
Sent: 23 August 2019 12:43
To: cfe-commits@lists.llvm.org 
Subject: r369749 - [Docs][OpenCL] Several corrections to C++ for OpenCL

Author: stulova
Date: Fri Aug 23 04:43:49 2019
New Revision: 369749

URL: http://llvm.org/viewvc/llvm-project?rev=369749&view=rev
Log:
[Docs][OpenCL] Several corrections to C++ for OpenCL

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


Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=369749&r1=369748&r2=369749&view=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Fri Aug 23 04:43:49 2019
@@ -1640,8 +1640,8 @@ OpenCL Features
 C++ for OpenCL
 --

-This functionality is built on top of OpenCL C v2.0 and C++17. Regular C++
-features can be used in OpenCL kernel code. All functionality from OpenCL C
+This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
+regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
 is inherited. This section describes minor differences to OpenCL C and any
 limitations related to C++ support as well as interactions between OpenCL and
 C++ features that are not documented elsewhere.
@@ -1652,6 +1652,7 @@ Restrictions to C++17
 The following features are not supported:

 - Virtual functions
+- Exceptions
 - ``dynamic_cast`` operator
 - Non-placement ``new``/``delete`` operators
 - Standard C++ libraries. Currently there is no solution for alternative C++
@@ -1667,20 +1668,24 @@ Address space behavior
 Address spaces are part of the type qualifiers; many rules are just inherited
 from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
 extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally yet, Clang extends existing concept
+behavior in C++ is not documented formally, Clang extends the existing concept
 from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using sets and overlapping from
-Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
-implicit conversions are allowed from named to ``__generic`` but not vice versa
-(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
-rules are built on top of this behavior.
+conversion but the compatibility is determined using notation of sets and
+overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
+s3.1.3). For OpenCL it means that implicit conversions are allowed from
+a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
+v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
+address space does not overlap with any other and therefore no valid conversion
+between ``__constant`` and other address spaces exists. Most of the rules
+follow this logic.

 **Casts**

-C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
-permit implicit conversion to ``__generic``. However converting from named
-address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
-that conversions between ``__constant`` and any other is still disallowed.
+C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
+permit conversion to ``__generic`` implicitly. However converting from
+``__generic`` to named address spaces can only be done using 
``addrspace_cast``.
+Note that conversions between ``__constant`` and any other address space
+are disallowed.

 .. _opencl_cpp_addrsp_deduction:

@@ -1693,7 +1698,7 @@ Address spaces are not deduced for:
 - non-pointer/non-reference class members except for static data members that 
are
   deduced to ``__global`` address space.
 - non-pointer/non-reference alias declarations.
-- ``decltype`` expression.
+- ``decltype`` expressions.

 .. code-block:: c++

@@ -1722,7 +1727,7 @@ TODO: Add example for type alias and dec

 **References**

-References types can be qualified with an address space.
+Reference types can be qualified with an address space.

 .. code-block:: c++

@@ -1737,29 +1742,29 @@ rules from address space pointer convers
 **Default address space**

 All non-static member functions take an implicit object parameter ``this`` that
-is a pointer type. By default this pointer parameter is in ``__generic`` 
address
-space. All concrete objects passed as an argument to ``this`` parameter will be
-converted to ``__generic`` address space first if the conversion is valid.
-Therefore programs using objects in ``__constant`` address

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 217335.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Start a paragrah with \returns instead of putting it in the middle


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66731

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -24,14 +24,9 @@
 using namespace clang;
 using namespace llvm::opt;
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object for
-/// a command line argument vector.
-///
-/// \return A CompilerInvocation, or 0 if none was built for the given
-/// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -95,11 +90,10 @@
 
   const ArgStringList &CCArgs = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(*CI,
- const_cast(CCArgs.data()),
- const_cast(CCArgs.data()) +
- CCArgs.size(),
- *Diags))
+  if (!CompilerInvocation::CreateFromArgs(
+  *CI, const_cast(CCArgs.data()),
+  const_cast(CCArgs.data()) + CCArgs.size(), *Diags) &&
+  !ShouldRecoverOnErorrs)
 return nullptr;
   return CI;
 }
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -213,13 +213,18 @@
 /// createInvocationFromCommandLine - Construct a compiler invocation object for
 /// a command line argument vector.
 ///
+/// \param ShouldRecoverOnErrors - whether we should attempt to return a
+/// non-null (and possibly incorrect) CompilerInvocation if any errors were
+/// encountered. When this flag is false, always return null on errors.
+///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
-IntrusiveRefCntPtr VFS = nullptr);
+IntrusiveRefCntPtr VFS = nullptr,
+bool ShouldRecoverOnErrors = false);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: clang/include/clang/Frontend/CompilerInvocation.h
===
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -147,6 +147,11 @@
   /// Create a compiler invocation from a list of input options.
   /// \returns true on success.
   ///
+  /// \returns false if an error was encountered while parsing the arguments
+  /// and attempts to recover and continue parsing the rest of the arguments.
+  /// The recovery is best-effort and only guarantees that \p Res will end up in
+  /// one of the vaild-to-access (albeit arbitrary) states.
+  ///
   /// \param [out] Res - The resulting invocation.
   /// \param ArgBegin - The first element in the argument vector.
   /// \param ArgEnd - The last element in the argument vector.
Index: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
@@ -9,7 +9,9 @@
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
@@ -244,6 +246,20 @@
   EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
 }
 
+TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
+  // Unknown flags should not prevent a build of compiler invocation.
+  ParseInputs Inputs;
+  Inputs.FS = buildTestFS({{testPath("foo.cpp"), "void test() {}"}});
+  Inputs.CompileCommand.CommandLine = {"clang", "-fsome-unknown-flag",
+   testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nul

[PATCH] D66792: [ReleaseNotes] MemorySanitizer support of ASLR on FreeBSD

[Sylvestre Ledru via Phabricator via cfe-commits]

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

Looks good, thanks


Repository:
  rC Clang

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

https://reviews.llvm.org/D66792



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

Re: r369749 - [Docs][OpenCL] Several corrections to C++ for OpenCL

[Hans Wennborg via cfe-commits]

Yes, r370031.

Thanks,
Hans

On Tue, Aug 27, 2019 at 11:43 AM Anastasia Stulova
 wrote:
>
> Hi Hans,
>
>
> Can this be merged to the release, please?
>
>
> Thank you,
>
> Anastasia
>
>
>
> 
> From: cfe-commits  on behalf of Anastasia 
> Stulova via cfe-commits 
> Sent: 23 August 2019 12:43
> To: cfe-commits@lists.llvm.org 
> Subject: r369749 - [Docs][OpenCL] Several corrections to C++ for OpenCL
>
> Author: stulova
> Date: Fri Aug 23 04:43:49 2019
> New Revision: 369749
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369749&view=rev
> Log:
> [Docs][OpenCL] Several corrections to C++ for OpenCL
>
> Differential Revision:https://reviews.llvm.org/D64418
>
>
> Modified:
> cfe/trunk/docs/LanguageExtensions.rst
> cfe/trunk/docs/UsersManual.rst
>
> Modified: cfe/trunk/docs/LanguageExtensions.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=369749&r1=369748&r2=369749&view=diff
> ==
> --- cfe/trunk/docs/LanguageExtensions.rst (original)
> +++ cfe/trunk/docs/LanguageExtensions.rst Fri Aug 23 04:43:49 2019
> @@ -1640,8 +1640,8 @@ OpenCL Features
>  C++ for OpenCL
>  --
>
> -This functionality is built on top of OpenCL C v2.0 and C++17. Regular C++
> -features can be used in OpenCL kernel code. All functionality from OpenCL C
> +This functionality is built on top of OpenCL C v2.0 and C++17 enabling most 
> of
> +regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
>  is inherited. This section describes minor differences to OpenCL C and any
>  limitations related to C++ support as well as interactions between OpenCL and
>  C++ features that are not documented elsewhere.
> @@ -1652,6 +1652,7 @@ Restrictions to C++17
>  The following features are not supported:
>
>  - Virtual functions
> +- Exceptions
>  - ``dynamic_cast`` operator
>  - Non-placement ``new``/``delete`` operators
>  - Standard C++ libraries. Currently there is no solution for alternative C++
> @@ -1667,20 +1668,24 @@ Address space behavior
>  Address spaces are part of the type qualifiers; many rules are just inherited
>  from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
>  extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address 
> space
> -behavior in C++ is not documented formally yet, Clang extends existing 
> concept
> +behavior in C++ is not documented formally, Clang extends the existing 
> concept
>  from C and OpenCL. For example conversion rules are extended from 
> qualification
> -conversion but the compatibility is determined using sets and overlapping 
> from
> -Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
> -implicit conversions are allowed from named to ``__generic`` but not vice 
> versa
> -(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
> -rules are built on top of this behavior.
> +conversion but the compatibility is determined using notation of sets and
> +overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
> +s3.1.3). For OpenCL it means that implicit conversions are allowed from
> +a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
> +v2.0 6.5.5). Reverse conversion is only allowed explicitly. The 
> ``__constant``
> +address space does not overlap with any other and therefore no valid 
> conversion
> +between ``__constant`` and other address spaces exists. Most of the rules
> +follow this logic.
>
>  **Casts**
>
> -C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators 
> will
> -permit implicit conversion to ``__generic``. However converting from named
> -address spaces to ``__generic`` can only be done using ``addrspace_cast``. 
> Note
> -that conversions between ``__constant`` and any other is still disallowed.
> +C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
> +permit conversion to ``__generic`` implicitly. However converting from
> +``__generic`` to named address spaces can only be done using 
> ``addrspace_cast``.
> +Note that conversions between ``__constant`` and any other address space
> +are disallowed.
>
>  .. _opencl_cpp_addrsp_deduction:
>
> @@ -1693,7 +1698,7 @@ Address spaces are not deduced for:
>  - non-pointer/non-reference class members except for static data members 
> that are
>deduced to ``__global`` address space.
>  - non-pointer/non-reference alias declarations.
> -- ``decltype`` expression.
> +- ``decltype`` expressions.
>
>  .. code-block:: c++
>
> @@ -1722,7 +1727,7 @@ TODO: Add example for type alias and dec
>
>  **References**
>
> -References types can be qualified with an address space.
> +Reference types can be qualified with an address space.
>
>  .. code-block:: c++
>
> @@ -1737,29 +1742,29 @@ rules from address space pointer convers
>  **Default address space**
>
>  All non-static member functions take an implic

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177
   return;
+if (TP->isPointerType() || TP->isLValueReferenceType())
+  // When highlighting dependant template types the type can be a pointer 
or

jvikstrom wrote:
> ilya-biryukov wrote:
> > jvikstrom wrote:
> > > ilya-biryukov wrote:
> > > > jvikstrom wrote:
> > > > > ilya-biryukov wrote:
> > > > > > `RecursiveASTVisitor` also traverses the pointer and reference 
> > > > > > types, why does it not reach the inner `TemplateTypeParmType` in 
> > > > > > the cases you describe?
> > > > > The D in `using D = ...` `typedef ... D` does not have a TypeLoc (at 
> > > > > least not one that is visited). Therefore we use the 
> > > > > VisitTypedefNameDecl (line 121) to get the location of `D` to be able 
> > > > > to highlight it. And we just send the typeLocs typeptr to addType 
> > > > > (which is a Pointer for `using D = T*;`)...
> > > > > 
> > > > > But maybe we should get the underlying type before we call addType 
> > > > > with TypePtr? Just a while loop on line 123 basically (can we have 
> > > > > multiple PointerTypes nested in each other actually?)
> > > > > 
> > > > > Even if we keep it in addType the comment is actually wrong, because 
> > > > > it obviously works when for the actual "type occurrences" for `D` (so 
> > > > > will fix that no matter what). This recursion will just make us add 
> > > > > more duplicate tokens...
> > > > Could we investigate why `RecursiveASTVisitor` does not visit the 
> > > > `TypeLoc` of a corresponding decl?
> > > > Here's the code from `RecursiveASTVisitor.h` that should do the trick:
> > > > ```
> > > > DEF_TRAVERSE_DECL(TypeAliasDecl, {
> > > >   TRY_TO(TraverseTypeLoc(D->getTypeSourceInfo()->getTypeLoc()));
> > > >   // We shouldn't traverse D->getTypeForDecl(); it's a result of
> > > >   // declaring the type alias, not something that was written in the
> > > >   // source.
> > > > })
> > > > ```
> > > > 
> > > > If it doesn't, we are probably holding it wrong.
> > > There just doesn't seem to be a TypeLoc for the typedef'ed Decl.  We can 
> > > get the `T*` TypeLoc (with `D->getTypeSourceInfo()->getTypeLoc()`). But 
> > > there isn't one for `D`. Even the `D->getTypeForDecl` returns null.
> > > 
> > > And I have no idea where I'd even start debugging that. Or if it's even a 
> > > bug
> > > 
> > I may have misinterpreted the patch. Are we trying to add highlightings for 
> > the names of using aliases here? E.g. for the following range:
> > ```
> > template 
> > struct Foo {
> >   using [[D]] = T**;
> > };
> > ```
> > 
> > Why isn't this handled in `VisitNamedDecl`?
> > We don't seem to call this function for `TypedefNameDecl` at all and it 
> > actually weird. Is this because we attempt to highlight typedefs as their 
> > underlying types?
> So currently using aliases and typedefs are highlighted the same as the 
> underlying type (in most cases). One case where they aren't is when the 
> underlying type is a template parameter (which is what this patch is trying 
> to solve).
> 
> 
> > Why isn't this handled in VisitNamedDecl?
> 
> The Decl is actually visited in `VisitNamedDecl`, however as it is a 
> `TypeAliasDecl` which we do not have a check for in the addToken function it 
> will not get highlighted in that visit.
> 
> Actually, could add a check for `TypeAliasDecl` in `addToken` (should 
> probably be a check for `TypedefNameDecl` to cover both `using ...` and 
> `typedef ...`) and move the code from the `VisitTypedefNameDecl` to the 
> `addToken` function inside that check instead.
> 
> 
> 
> > We don't seem to call this function for TypedefNameDecl at all and it 
> > actually weird. Is this because we attempt to highlight typedefs as their 
> > underlying types?
> 
> 
> Don't understand what you mean. What function? 
> So currently using aliases and typedefs are highlighted the same as the 
> underlying type (in most cases). 
Thanks for clarifying this. This is where my confusion is coming from.
A few question to try understanding the approach taken (sorry if that's too 
detailed, I am probably missing the context here)
- What do we fallback to? From my reading of the code, we do not highlight them 
at all if the underlying type is not one of the predefined cases.
- Why are pointers and **l-value** references special? What about arrays, 
r-value references, function types, pack expansions, etc.?

> Don't understand what you mean. What function?
We don't call `VisitNamedDecl` from `VisitTypedefNameDecl`. I guess that's 
intentional if we try to highlight them as underlying types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66516



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 217337.
martong added a comment.

- Apply clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/unittests/AST/ASTImporterFixtures.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/include/lldb/Symbol/ClangASTImporter.h

Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -252,7 +252,9 @@
 : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
  master.m_file_manager, true /*minimal*/),
   m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
-  m_master(master), m_source_ctx(source_ctx) {}
+  m_master(master), m_source_ctx(source_ctx) {
+  setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+}
 
 /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate
 /// and deattaches it at the end of the scope. Supports being used multiple
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -1879,7 +1879,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2432,6 +2432,62 @@
   EXPECT_EQ(ToD1, ToD2);
 }
 
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5127,15 +5183,6 @@
   }
 }
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest,
-DefaultTestValuesForRunOptions, );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
-::testing::Values(ArgVector()), );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
-::testing::Values(ArgVector()), );
-
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
@@ -5233,8 +5280,8 @@
   // prototype (inside 'friend') for it comes first in the AST and is not
   // linked to the definition.
   EXPECT_EQ(ImportedDef, ToClassDef);
-}  
-  
+}
+
 struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
   LLDBLookupTest() {
 Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
@@ -5362,10 +5409,197 @@
   EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
 }
 
+struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
+  ConflictingDeclsWithLiberalStrategy() {
+this->ODRHandling = ASTImporter::ODRHandlingType::Liberal;
+  }
+};
+
+// Check that a Decl has been successfully imported into a standalone redecl
+// chain.
+template 
+static void CheckImportedAsNew(llvm::Expected &Result, Decl *ToTU,
+   PatternTy Pattern) {
+  ASSERT_TRUE(isSuccess(Result));
+  Decl *ImportedD = *Result;
+  ASSERT_TRUE(ImportedD);
+  auto *ToD = FirstDeclMatcher().match(ToTU, Pattern);
+  EXPECT_NE(

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:150
   ///
+  /// If an error was encountered while parsing the arguments, \returns false
+  /// and attempts to recover and continue parsing the rest of the arguments.

gribozavr wrote:
> \returns false if an error...
> 
> (\returns is a paragraph command. It starts a new paragraph.)
TIL something about doxygen :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66731



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

r370033 - [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 27 03:02:18 2019
New Revision: 370033

URL: http://llvm.org/viewvc/llvm-project?rev=370033&view=rev
Log:
[Driver] Add an option for createInvocationFromCommandLine to recover on errors

Summary:
Previously, it would always return nullptr on any error.
This change adds a parameter, controlling whether the function should
attempt to return a non-null result even if unknown arguments (or other
errors were encountered).

The new behavior is only used in clangd.

Considered an alternative of changing the return value instead of adding
a new parameter, but that would require updating all callsites. Settled
with the parameter to minimize the code changes.

Reviewers: gribozavr

Reviewed By: gribozavr

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

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/include/clang/Frontend/Utils.h
cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp

Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=370033&r1=370032&r2=370033&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h Tue Aug 27 03:02:18 
2019
@@ -147,6 +147,11 @@ public:
   /// Create a compiler invocation from a list of input options.
   /// \returns true on success.
   ///
+  /// \returns false if an error was encountered while parsing the arguments
+  /// and attempts to recover and continue parsing the rest of the arguments.
+  /// The recovery is best-effort and only guarantees that \p Res will end up 
in
+  /// one of the vaild-to-access (albeit arbitrary) states.
+  ///
   /// \param [out] Res - The resulting invocation.
   /// \param ArgBegin - The first element in the argument vector.
   /// \param ArgEnd - The last element in the argument vector.

Modified: cfe/trunk/include/clang/Frontend/Utils.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=370033&r1=370032&r2=370033&view=diff
==
--- cfe/trunk/include/clang/Frontend/Utils.h (original)
+++ cfe/trunk/include/clang/Frontend/Utils.h Tue Aug 27 03:02:18 2019
@@ -213,13 +213,18 @@ createChainedIncludesSource(CompilerInst
 /// createInvocationFromCommandLine - Construct a compiler invocation object 
for
 /// a command line argument vector.
 ///
+/// \param ShouldRecoverOnErrors - whether we should attempt to return a
+/// non-null (and possibly incorrect) CompilerInvocation if any errors were
+/// encountered. When this flag is false, always return null on errors.
+///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
-IntrusiveRefCntPtr VFS = nullptr);
+IntrusiveRefCntPtr VFS = nullptr,
+bool ShouldRecoverOnErrors = false);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.

Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=370033&r1=370032&r2=370033&view=diff
==
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp (original)
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Tue Aug 27 
03:02:18 2019
@@ -24,14 +24,9 @@
 using namespace clang;
 using namespace llvm::opt;
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object 
for
-/// a command line argument vector.
-///
-/// \return A CompilerInvocation, or 0 if none was built for the given
-/// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr 
Diags,
-IntrusiveRefCntPtr VFS) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) 
{
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -95,11 +90,10 @@ std::unique_ptr clan
 
   const ArgStringList &CCArgs = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(*CI,
- const_cast(CCArgs.data()),
- const_cast(CCArgs.data()) +
- CCArgs.size(),
- *Diags))
+  if (!CompilerInvocation::CreateFromArgs(
+  *CI, const_cast(CCArgs.data()),
+  

r370035 - [ReleaseNotes] MemorySanitizer support of ASLR on FreeBSD

[David Carlier via cfe-commits]

Author: devnexen
Date: Tue Aug 27 03:04:03 2019
New Revision: 370035

URL: http://llvm.org/viewvc/llvm-project?rev=370035&view=rev
Log:
[ReleaseNotes] MemorySanitizer support of ASLR on FreeBSD

Reviewers: sylvestre.ledru, kcc

Reviewed By: sylvestre.ledru

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

Modified:
cfe/trunk/docs/MemorySanitizer.rst

Modified: cfe/trunk/docs/MemorySanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/MemorySanitizer.rst?rev=370035&r1=370034&r2=370035&view=diff
==
--- cfe/trunk/docs/MemorySanitizer.rst (original)
+++ cfe/trunk/docs/MemorySanitizer.rst Tue Aug 27 03:04:03 2019
@@ -204,6 +204,9 @@ Limitations
   non-position-independent executables, and could fail on some Linux
   kernel versions with disabled ASLR. Refer to documentation for older versions
   for more details.
+* MemorySanitizer might be incompatible with position-independent executables
+  from FreeBSD 13 but there is a check done at runtime and throws a warning
+  in this case.
 
 Current Status
 ==


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

[clang-tools-extra] r370033 - [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 27 03:02:18 2019
New Revision: 370033

URL: http://llvm.org/viewvc/llvm-project?rev=370033&view=rev
Log:
[Driver] Add an option for createInvocationFromCommandLine to recover on errors

Summary:
Previously, it would always return nullptr on any error.
This change adds a parameter, controlling whether the function should
attempt to return a non-null result even if unknown arguments (or other
errors were encountered).

The new behavior is only used in clangd.

Considered an alternative of changing the return value instead of adding
a new parameter, but that would require updating all callsites. Settled
with the parameter to minimize the code changes.

Reviewers: gribozavr

Reviewed By: gribozavr

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/Compiler.cpp
clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

Modified: clang-tools-extra/trunk/clangd/Compiler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.cpp?rev=370033&r1=370032&r2=370033&view=diff
==
--- clang-tools-extra/trunk/clangd/Compiler.cpp (original)
+++ clang-tools-extra/trunk/clangd/Compiler.cpp Tue Aug 27 03:02:18 2019
@@ -59,7 +59,8 @@ buildCompilerInvocation(const ParseInput
   CompilerInstance::createDiagnostics(new DiagnosticOptions,
   &IgnoreDiagnostics, false);
   std::unique_ptr CI = createInvocationFromCommandLine(
-  ArgStrs, CommandLineDiagsEngine, Inputs.FS);
+  ArgStrs, CommandLineDiagsEngine, Inputs.FS,
+  /*ShouldRecoverOnErrors=*/true);
   if (!CI)
 return nullptr;
   // createInvocationFromCommandLine sets DisableFree.

Modified: clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp?rev=370033&r1=370032&r2=370033&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp Tue Aug 27 
03:02:18 2019
@@ -9,7 +9,9 @@
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
@@ -244,6 +246,20 @@ TEST(ClangdUnitTest, NoCrashOnTokensWith
   EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
 }
 
+TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
+  // Unknown flags should not prevent a build of compiler invocation.
+  ParseInputs Inputs;
+  Inputs.FS = buildTestFS({{testPath("foo.cpp"), "void test() {}"}});
+  Inputs.CompileCommand.CommandLine = {"clang", "-fsome-unknown-flag",
+   testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+
+  // Unknown forwarded to -cc1 should not a failure either.
+  Inputs.CompileCommand.CommandLine = {
+  "clang", "-Xclang", "-fsome-unknown-flag", testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I agree with Ilya's concerns on SourceLocations, even though most of the time 
user will have the source locations available in the dyntyped node, they might 
need some traversals to fetch the exact range especially in cases of nested 
name specifiers.
It would be nice to provide a referencing range in addition to the decls.




Comment at: clang-tools-extra/clangd/FindTarget.cpp:79
+  static const Decl *getTemplatePattern(const Decl *D) {
+if (const CXXRecordDecl *SD = dyn_cast(D)) {
+  return SD->getTemplateInstantiationPattern();

nit: s/SD/CRD/



Comment at: clang-tools-extra/clangd/FindTarget.h:97
+class DeclRelationSet {
+  using Set = std::bitset(DeclRelation::PureLexical) + 
1>;
+  Set S;

Could you rather put an end marker into the `DeclRelation` and make use of it 
in here? As people might forget to update this one if they add anything into 
DeclRelation.

(It seems rare for this enum to change though, so feel free to ignore if you 
believe it will stay the same).



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:49
+EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
+llvm::Annotations::Range R = A.llvm::Annotations::range();
+SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,

can't you just call `A.range()` ? :D
I hope it wasn't clangd that somehow code-completed this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751



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

[PATCH] D66770: Move EH spec mismatches under -fms-compatibility

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans 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/D66770/new/

https://reviews.llvm.org/D66770



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

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370033: [Driver] Add an option for 
createInvocationFromCommandLine to recover on errors (authored by ibiryukov, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66731?vs=217335&id=217344#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66731

Files:
  cfe/trunk/include/clang/Frontend/CompilerInvocation.h
  cfe/trunk/include/clang/Frontend/Utils.h
  cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang-tools-extra/trunk/clangd/Compiler.cpp
  clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

Index: clang-tools-extra/trunk/clangd/Compiler.cpp
===
--- clang-tools-extra/trunk/clangd/Compiler.cpp
+++ clang-tools-extra/trunk/clangd/Compiler.cpp
@@ -59,7 +59,8 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions,
   &IgnoreDiagnostics, false);
   std::unique_ptr CI = createInvocationFromCommandLine(
-  ArgStrs, CommandLineDiagsEngine, Inputs.FS);
+  ArgStrs, CommandLineDiagsEngine, Inputs.FS,
+  /*ShouldRecoverOnErrors=*/true);
   if (!CI)
 return nullptr;
   // createInvocationFromCommandLine sets DisableFree.
Index: clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
@@ -9,7 +9,9 @@
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
@@ -244,6 +246,20 @@
   EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
 }
 
+TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
+  // Unknown flags should not prevent a build of compiler invocation.
+  ParseInputs Inputs;
+  Inputs.FS = buildTestFS({{testPath("foo.cpp"), "void test() {}"}});
+  Inputs.CompileCommand.CommandLine = {"clang", "-fsome-unknown-flag",
+   testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+
+  // Unknown forwarded to -cc1 should not a failure either.
+  Inputs.CompileCommand.CommandLine = {
+  "clang", "-Xclang", "-fsome-unknown-flag", testPath("foo.cpp")};
+  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
===
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h
@@ -147,6 +147,11 @@
   /// Create a compiler invocation from a list of input options.
   /// \returns true on success.
   ///
+  /// \returns false if an error was encountered while parsing the arguments
+  /// and attempts to recover and continue parsing the rest of the arguments.
+  /// The recovery is best-effort and only guarantees that \p Res will end up in
+  /// one of the vaild-to-access (albeit arbitrary) states.
+  ///
   /// \param [out] Res - The resulting invocation.
   /// \param ArgBegin - The first element in the argument vector.
   /// \param ArgEnd - The last element in the argument vector.
Index: cfe/trunk/include/clang/Frontend/Utils.h
===
--- cfe/trunk/include/clang/Frontend/Utils.h
+++ cfe/trunk/include/clang/Frontend/Utils.h
@@ -213,13 +213,18 @@
 /// createInvocationFromCommandLine - Construct a compiler invocation object for
 /// a command line argument vector.
 ///
+/// \param ShouldRecoverOnErrors - whether we should attempt to return a
+/// non-null (and possibly incorrect) CompilerInvocation if any errors were
+/// encountered. When this flag is false, always return null on errors.
+///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
-IntrusiveRefCntPtr VFS = nullptr);
+IntrusiveRefCntPtr VFS = nullptr,
+bool ShouldRecoverOnErrors = false);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -24,14 +24,9 @@
 using names

[PATCH] D66792: [ReleaseNotes] MemorySanitizer support of ASLR on FreeBSD

[David CARLIER via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbccbd74c6255: [ReleaseNotes] MemorySanitizer support of ASLR 
on FreeBSD (authored by devnexen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66792

Files:
  clang/docs/MemorySanitizer.rst


Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -204,6 +204,9 @@
   non-position-independent executables, and could fail on some Linux
   kernel versions with disabled ASLR. Refer to documentation for older versions
   for more details.
+* MemorySanitizer might be incompatible with position-independent executables
+  from FreeBSD 13 but there is a check done at runtime and throws a warning
+  in this case.
 
 Current Status
 ==


Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -204,6 +204,9 @@
   non-position-independent executables, and could fail on some Linux
   kernel versions with disabled ASLR. Refer to documentation for older versions
   for more details.
+* MemorySanitizer might be incompatible with position-independent executables
+  from FreeBSD 13 but there is a check done at runtime and throws a warning
+  in this case.
 
 Current Status
 ==
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r369834 - PR42513: Enter the proper DeclContext before substituting into an

[Hans Wennborg via cfe-commits]

Merged to release_90 in r370038.

On Sat, Aug 24, 2019 at 4:28 AM Richard Smith via cfe-commits
 wrote:
>
> Author: rsmith
> Date: Fri Aug 23 19:30:00 2019
> New Revision: 369834
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369834&view=rev
> Log:
> PR42513: Enter the proper DeclContext before substituting into an
> default template argument expression.
>
> We already did this for type template parameters and template template
> parameters, but apparently forgot to do so for non-type template
> parameters. This causes the substituted default argument expression to
> be substituted in the proper context, and in particular to properly mark
> its subexpressions as odr-used.
>
> Modified:
> cfe/trunk/lib/Sema/SemaTemplate.cpp
> cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
> cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=369834&r1=369833&r2=369834&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri Aug 23 19:30:00 2019
> @@ -4693,6 +4693,7 @@ SubstDefaultTemplateArgument(Sema &SemaR
>for (unsigned i = 0, e = Param->getDepth(); i != e; ++i)
>  TemplateArgLists.addOuterTemplateArguments(None);
>
> +  Sema::ContextRAII SavedContext(SemaRef, Template->getDeclContext());
>EnterExpressionEvaluationContext ConstantEvaluated(
>SemaRef, Sema::ExpressionEvaluationContext::ConstantEvaluated);
>return SemaRef.SubstExpr(Param->getDefaultArgument(), TemplateArgLists);
>
> Modified: cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp?rev=369834&r1=369833&r2=369834&view=diff
> ==
> --- cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp 
> (original)
> +++ cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp Fri 
> Aug 23 19:30:00 2019
> @@ -268,13 +268,16 @@ namespace tuple_tests {
>// Don't get caught by surprise when X<...> doesn't even exist in the
>// selected specialization!
>namespace libcxx_2 {
> -template struct tuple { // expected-note {{candidate}}
> +template struct tuple {
>template struct X { static const bool value = false; };
> +  // Substitution into X::value succeeds but produces the
> +  // value-dependent expression
> +  //   tuple::X<>::value
> +  // FIXME: Is that the right behavior?
>template::value> tuple(U &&...u);
> -  // expected-note@-1 {{substitution failure [with T = <>, U =  int, int>]: cannot reference member of primary template because deduced class 
> template specialization 'tuple<>' is an explicit specialization}}
>  };
>  template <> class tuple<> {};
> -tuple a = {1, 2, 3}; // expected-error {{no viable constructor or 
> deduction guide}}
> +tuple a = {1, 2, 3}; // expected-error {{excess elements in struct 
> initializer}}
>}
>
>namespace libcxx_3 {
>
> Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp?rev=369834&r1=369833&r2=369834&view=diff
> ==
> --- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp (original)
> +++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx11.cpp Fri Aug 23 
> 19:30:00 2019
> @@ -48,3 +48,20 @@ void Useage() {
>  }
>  }
>
> +namespace PR42513 {
> +  template void f();
> +  constexpr int WidgetCtor(struct X1*);
> +
> +  struct X1 {
> +friend constexpr int WidgetCtor(X1*);
> +  };
> +  template
> +  struct StandardWidget {
> +friend constexpr int WidgetCtor(X1*) {
> +  return 0;
> +}
> +  };
> +  template struct StandardWidget;
> +
> +  void use() { f(); }
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r370039 - Refactor GlobList from an ad-hoc linked list to a vector

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Tue Aug 27 03:56:13 2019
New Revision: 370039

URL: http://llvm.org/viewvc/llvm-project?rev=370039&view=rev
Log:
Refactor GlobList from an ad-hoc linked list to a vector

Summary: I think it makes method implementations more obvious.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/GlobList.cpp
clang-tools-extra/trunk/clang-tidy/GlobList.h

Modified: clang-tools-extra/trunk/clang-tidy/GlobList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/GlobList.cpp?rev=370039&r1=370038&r2=370039&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/GlobList.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/GlobList.cpp Tue Aug 27 03:56:13 2019
@@ -42,15 +42,20 @@ static llvm::Regex ConsumeGlob(StringRef
   return llvm::Regex(RegexText);
 }
 
-GlobList::GlobList(StringRef Globs)
-: Positive(!ConsumeNegativeIndicator(Globs)), Regex(ConsumeGlob(Globs)),
-  NextGlob(Globs.empty() ? nullptr : new GlobList(Globs)) {}
-
-bool GlobList::contains(StringRef S, bool Contains) {
-  if (Regex.match(S))
-Contains = Positive;
+GlobList::GlobList(StringRef Globs) {
+  do {
+GlobListItem Item;
+Item.IsPositive = !ConsumeNegativeIndicator(Globs);
+Item.Regex = ConsumeGlob(Globs);
+Items.push_back(std::move(Item));
+  } while (!Globs.empty());
+}
 
-  if (NextGlob)
-Contains = NextGlob->contains(S, Contains);
+bool GlobList::contains(StringRef S) {
+  bool Contains = false;
+  for (const GlobListItem &Item : Items) {
+if (Item.Regex.match(S))
+  Contains = Item.IsPositive;
+  }
   return Contains;
 }

Modified: clang-tools-extra/trunk/clang-tidy/GlobList.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/GlobList.h?rev=370039&r1=370038&r2=370039&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/GlobList.h (original)
+++ clang-tools-extra/trunk/clang-tidy/GlobList.h Tue Aug 27 03:56:13 2019
@@ -12,30 +12,36 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
-#include 
+#include 
 
 namespace clang {
 namespace tidy {
 
-/// Read-only set of strings represented as a list of positive and
-/// negative globs. Positive globs add all matched strings to the set, negative
-/// globs remove them in the order of appearance in the list.
+/// Read-only set of strings represented as a list of positive and negative
+/// globs.
+///
+/// Positive globs add all matched strings to the set, negative globs remove
+/// them in the order of appearance in the list.
 class GlobList {
 public:
-  /// \p GlobList is a comma-separated list of globs (only '*'
-  /// metacharacter is supported) with optional '-' prefix to denote exclusion.
+  /// \p Globs is a comma-separated list of globs (only the '*' metacharacter 
is
+  /// supported) with an optional '-' prefix to denote exclusion.
+  ///
+  /// An empty \p Globs string is interpreted as one glob that matches an empty
+  /// string.
   GlobList(StringRef Globs);
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) { return contains(S, false); }
+  bool contains(StringRef S);
 
 private:
-  bool contains(StringRef S, bool Contains);
 
-  bool Positive;
-  llvm::Regex Regex;
-  std::unique_ptr NextGlob;
+  struct GlobListItem {
+bool IsPositive;
+mutable llvm::Regex Regex;
+  };
+  std::vector Items;
 };
 
 } // end namespace tidy


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

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D57450#1641308 , @jyknight wrote:

> In D57450#1641190 , @lenary wrote:
>
> > @jyknight I hear where you're coming from. I'll see what I can do about the 
> > psABI document.
> >
> > In that ticket, it's mentioned that the Darwin ABI explicitly says that 
> > non-power-of-two atomic types should be padded and realigned, but I cannot 
> > find any documentation explaining this. That would be useful, given 
> > presumably GCC does have to pad/align on Darwin.
>
>
> AFAIK, there is no such documentation, at least publicly. Possibly Apple has 
> some internally, but I suspect it more likely just some in-person 
> conversation or something.
>
> GCC is not really supported on Darwin, so I suspect it just gets it wrong.


To keep everyone updated, this has bubbled over into a thread on the GCC 
Mailing list: https://gcc.gnu.org/ml/gcc/2019-08/msg00191.html - Potentially 
this is a route to a resolution on non-RISC-V platforms (specifically x86).

> 
> 
>> Then the only outstanding question relates to zero-sized atomics, which GCC 
>> does not pad, but I think Clang has to pad to get the semantics correct, 
>> based on this comment: 
>> https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ASTContext.cpp#L2176
> 
> The semantics in GCC are that you can create such an object, but any attempt 
> to load or store it will result in a compile-time error. E.g., "error: 
> argument 1 of ‘__atomic_load’ must be a pointer to a nonzero size object". So 
> I don't think there's really an issue there.

Neat, ok.

I think the feeling with this ticket and the RISC-V backend is:

- We match GCC for power-of-two-sized atomics
- We don't match for non-power-of-two sized atomics (as on any other platform)

This feels like that should be enough for this patch to be merged, and in a 
future patch we can address the fall-out of ABIs changing across GCC and clang 
on more platforms than just RISC-V?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57450



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

[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:46
+GlobList::GlobList(StringRef Globs) {
+  do {
+GlobListItem Item;

ilya-biryukov wrote:
> NIT: I suggest using `for (;!Globs.empty();) {}` to make the stop condition 
> stand out more.
> Not a big deal, though, feel free to keep as is
It is actually important for the algorithm to use a do-while loop... The 
linked-list-based code always parsed at least one glob, even if the input 
string is empty. Therefore, new loop-based code should also parse at least one 
glob.

I added a doc comment:

```
+  /// An empty \p Globs string is interpreted as one glob that matches an empty
+  /// string.
   GlobList(StringRef Globs);
```

Please let me know if that is not sufficient. There are also tests for this 
behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66788



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

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: fhahn, Meinersbur, hsaito, Ayal.

This changes the behaviour of vectorize(disable). I.e., disabling vectorization
now means disabling vectorization, and not setting the vectorization width to 1.

This is a follow up of the discussion on the behaviour of different loop
pragmas, and that setting transformation options should imply enabling the
transformation, see also
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html.

Not only is this change in the behaviour of vectorize(disable) probably more 
sensible,
it also helps in implementing options implying transformations.


https://reviews.llvm.org/D66796

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop-predicate.cpp
  clang/test/CodeGenCXX/pragma-loop-safety.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp


Index: clang/test/CodeGenCXX/pragma-loop.cpp
===
--- clang/test/CodeGenCXX/pragma-loop.cpp
+++ clang/test/CodeGenCXX/pragma-loop.cpp
@@ -178,8 +178,8 @@
 // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
 
-// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]], 
![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_1:.*]]}
-// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]], 
![[DISTRIBUTE_DISABLE:.*]], ![[VECTORIZE_DISABLE:.*]]}
+// CHECK: ![[VECTORIZE_DISABLE]] = !{!"llvm.loop.vectorize.enable", i1 false}
 
 // CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[WIDTH_2:.*]], 
![[INTERLEAVE_2:.*]], ![[FOLLOWUP_VECTOR_6:.*]]}
 // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", 
![[AFTER_VECTOR_6:.*]]}
Index: clang/test/CodeGenCXX/pragma-loop-safety.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-safety.cpp
+++ clang/test/CodeGenCXX/pragma-loop-safety.cpp
@@ -53,6 +53,6 @@
 // CHECK: ![[INTERLEAVE_1]] = !{!"llvm.loop.interleave.count", i32 1}
 // CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
 // CHECK: ![[ACCESS_GROUP_8]] = distinct !{}
-// CHECK: ![[LOOP2_HINTS]] = distinct !{![[LOOP2_HINTS]], 
![[PARALLEL_ACCESSES_11:[0-9]+]], ![[UNROLL_DISABLE]], ![[WIDTH_1:[0-9]+]], 
![[INTENABLE_1]]}
+// CHECK: ![[LOOP2_HINTS]] = distinct !{![[LOOP2_HINTS]], 
![[PARALLEL_ACCESSES_11:[0-9]+]], ![[UNROLL_DISABLE]], 
![[VECTORIZE_DISABLE:[0-9]+]]}
 // CHECK: ![[PARALLEL_ACCESSES_11]] = !{!"llvm.loop.parallel_accesses", 
![[ACCESS_GROUP_8]]}
-// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK: ![[VECTORIZE_DISABLE]] = !{!"llvm.loop.vectorize.enable", i1 false}
Index: clang/test/CodeGenCXX/pragma-loop-predicate.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -71,6 +71,6 @@
 // CHECK-NEXT: ![[LOOP3]] = distinct !{![[LOOP3]], !5, !3}
 
 // CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !10}
-// CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.enable", i1 false}
 
 // CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !10}
Index: clang/lib/CodeGen/CGLoopInfo.cpp
===
--- clang/lib/CodeGen/CGLoopInfo.cpp
+++ clang/lib/CodeGen/CGLoopInfo.cpp
@@ -617,8 +617,7 @@
 case LoopHintAttr::Disable:
   switch (Option) {
   case LoopHintAttr::Vectorize:
-// Disable vectorization by specifying a width of 1.
-setVectorizeWidth(1);
+setVectorizeEnable(false);
 break;
   case LoopHintAttr::Interleave:
 // Disable interleaving by speciyfing a count of 1.


Index: clang/test/CodeGenCXX/pragma-loop.cpp
===
--- clang/test/CodeGenCXX/pragma-loop.cpp
+++ clang/test/CodeGenCXX/pragma-loop.cpp
@@ -178,8 +178,8 @@
 // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
 
-// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_1:.*]]}
-// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[VECTORIZE_DISABLE:.*]]}
+// CHECK: ![[VECTORIZE_DISABLE]] = !{!"llvm.loop.vectorize.enable", i1 false}
 
 // CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]], ![[FOLLOWUP_VECTOR_6:.*]]}
 // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_6:.*]]}
Index: clang/test/CodeGenCXX/pragma-loop-safety.cpp
===
--- clang/test/CodeGenCXX/

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

> I think it would be slightly better to split off the change to disable 
> vectorization via llvm.loop.vectorize.enable=false instead of width=1.

This is now D66796 .

I will now start stripping it out from this patch.


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

https://reviews.llvm.org/D66290



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

[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370039: Refactor GlobList from an ad-hoc linked list to a 
vector (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66788?vs=217322&id=217357#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66788

Files:
  clang-tools-extra/trunk/clang-tidy/GlobList.cpp
  clang-tools-extra/trunk/clang-tidy/GlobList.h


Index: clang-tools-extra/trunk/clang-tidy/GlobList.h
===
--- clang-tools-extra/trunk/clang-tidy/GlobList.h
+++ clang-tools-extra/trunk/clang-tidy/GlobList.h
@@ -12,30 +12,36 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
-#include 
+#include 
 
 namespace clang {
 namespace tidy {
 
-/// Read-only set of strings represented as a list of positive and
-/// negative globs. Positive globs add all matched strings to the set, negative
-/// globs remove them in the order of appearance in the list.
+/// Read-only set of strings represented as a list of positive and negative
+/// globs.
+///
+/// Positive globs add all matched strings to the set, negative globs remove
+/// them in the order of appearance in the list.
 class GlobList {
 public:
-  /// \p GlobList is a comma-separated list of globs (only '*'
-  /// metacharacter is supported) with optional '-' prefix to denote exclusion.
+  /// \p Globs is a comma-separated list of globs (only the '*' metacharacter 
is
+  /// supported) with an optional '-' prefix to denote exclusion.
+  ///
+  /// An empty \p Globs string is interpreted as one glob that matches an empty
+  /// string.
   GlobList(StringRef Globs);
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) { return contains(S, false); }
+  bool contains(StringRef S);
 
 private:
-  bool contains(StringRef S, bool Contains);
 
-  bool Positive;
-  llvm::Regex Regex;
-  std::unique_ptr NextGlob;
+  struct GlobListItem {
+bool IsPositive;
+mutable llvm::Regex Regex;
+  };
+  std::vector Items;
 };
 
 } // end namespace tidy
Index: clang-tools-extra/trunk/clang-tidy/GlobList.cpp
===
--- clang-tools-extra/trunk/clang-tidy/GlobList.cpp
+++ clang-tools-extra/trunk/clang-tidy/GlobList.cpp
@@ -42,15 +42,20 @@
   return llvm::Regex(RegexText);
 }
 
-GlobList::GlobList(StringRef Globs)
-: Positive(!ConsumeNegativeIndicator(Globs)), Regex(ConsumeGlob(Globs)),
-  NextGlob(Globs.empty() ? nullptr : new GlobList(Globs)) {}
-
-bool GlobList::contains(StringRef S, bool Contains) {
-  if (Regex.match(S))
-Contains = Positive;
+GlobList::GlobList(StringRef Globs) {
+  do {
+GlobListItem Item;
+Item.IsPositive = !ConsumeNegativeIndicator(Globs);
+Item.Regex = ConsumeGlob(Globs);
+Items.push_back(std::move(Item));
+  } while (!Globs.empty());
+}
 
-  if (NextGlob)
-Contains = NextGlob->contains(S, Contains);
+bool GlobList::contains(StringRef S) {
+  bool Contains = false;
+  for (const GlobListItem &Item : Items) {
+if (Item.Regex.match(S))
+  Contains = Item.IsPositive;
+  }
   return Contains;
 }


Index: clang-tools-extra/trunk/clang-tidy/GlobList.h
===
--- clang-tools-extra/trunk/clang-tidy/GlobList.h
+++ clang-tools-extra/trunk/clang-tidy/GlobList.h
@@ -12,30 +12,36 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
-#include 
+#include 
 
 namespace clang {
 namespace tidy {
 
-/// Read-only set of strings represented as a list of positive and
-/// negative globs. Positive globs add all matched strings to the set, negative
-/// globs remove them in the order of appearance in the list.
+/// Read-only set of strings represented as a list of positive and negative
+/// globs.
+///
+/// Positive globs add all matched strings to the set, negative globs remove
+/// them in the order of appearance in the list.
 class GlobList {
 public:
-  /// \p GlobList is a comma-separated list of globs (only '*'
-  /// metacharacter is supported) with optional '-' prefix to denote exclusion.
+  /// \p Globs is a comma-separated list of globs (only the '*' metacharacter is
+  /// supported) with an optional '-' prefix to denote exclusion.
+  ///
+  /// An empty \p Globs string is interpreted as one glob that matches an empty
+  /// string.
   GlobList(StringRef Globs);
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) { return contains(S, false); }
+  bool contains(StringRef S);
 
 private:
-  bool contains(StringRef S, bool Contains);
 
-  bool Posi

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

[Dmitri Gribenko via Phabricator via cfe-commits]

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

LGTM with all comments addressed.




Comment at: clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp:169
 int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void 
*(*start_routine)(void *), void *arg);
 

Maybe pick a simpler function for this test, like pthread_yield?



Comment at: clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp:191
   int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+  int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void 
*(*start_routine)(void *), void *arg);
 

Ditto, pthread_yield would be simpler to declare.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66627



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

r370041 - [clang] Ensure that comment classes are trivially destructible

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Aug 27 04:21:00 2019
New Revision: 370041

URL: http://llvm.org/viewvc/llvm-project?rev=370041&view=rev
Log:
[clang] Ensure that comment classes are trivially destructible

As in D66646, these classes are also allocated with a BumpPtrAllocator,
and therefore should be trivially destructible.

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

Reviewed By: Mordante, gribozavr

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

Modified: cfe/trunk/lib/AST/Comment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Comment.cpp?rev=370041&r1=370040&r2=370041&view=diff
==
--- cfe/trunk/lib/AST/Comment.cpp (original)
+++ cfe/trunk/lib/AST/Comment.cpp Tue Aug 27 04:21:00 2019
@@ -13,10 +13,25 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 
 namespace clang {
 namespace comments {
 
+// Check that no comment class has a non-trival destructor. They are allocated
+// with a BumpPtrAllocator and therefore their destructor is not executed.
+#define ABSTRACT_COMMENT(COMMENT)
+#define COMMENT(CLASS, PARENT) 
\
+  static_assert(std::is_trivially_destructible::value,  
\
+#CLASS " should be trivially destructible!");
+#include "clang/AST/CommentNodes.inc"
+#undef COMMENT
+#undef ABSTRACT_COMMENT
+
+// DeclInfo is also allocated with a BumpPtrAllocator.
+static_assert(std::is_trivially_destructible::value,
+  "DeclInfo should be trivially destructible!");
+
 const char *Comment::getCommentKindName() const {
   switch (getCommentKind()) {
   case NoCommentKind: return "NoCommentKind";


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

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

[Bruno Ricci via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2535f04338c6: [clang] Ensure that comment classes are 
trivially destructible (authored by riccibruno).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66722

Files:
  clang/lib/AST/Comment.cpp


Index: clang/lib/AST/Comment.cpp
===
--- clang/lib/AST/Comment.cpp
+++ clang/lib/AST/Comment.cpp
@@ -13,10 +13,25 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 
 namespace clang {
 namespace comments {
 
+// Check that no comment class has a non-trival destructor. They are allocated
+// with a BumpPtrAllocator and therefore their destructor is not executed.
+#define ABSTRACT_COMMENT(COMMENT)
+#define COMMENT(CLASS, PARENT) 
\
+  static_assert(std::is_trivially_destructible::value,  
\
+#CLASS " should be trivially destructible!");
+#include "clang/AST/CommentNodes.inc"
+#undef COMMENT
+#undef ABSTRACT_COMMENT
+
+// DeclInfo is also allocated with a BumpPtrAllocator.
+static_assert(std::is_trivially_destructible::value,
+  "DeclInfo should be trivially destructible!");
+
 const char *Comment::getCommentKindName() const {
   switch (getCommentKind()) {
   case NoCommentKind: return "NoCommentKind";


Index: clang/lib/AST/Comment.cpp
===
--- clang/lib/AST/Comment.cpp
+++ clang/lib/AST/Comment.cpp
@@ -13,10 +13,25 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 
 namespace clang {
 namespace comments {
 
+// Check that no comment class has a non-trival destructor. They are allocated
+// with a BumpPtrAllocator and therefore their destructor is not executed.
+#define ABSTRACT_COMMENT(COMMENT)
+#define COMMENT(CLASS, PARENT) \
+  static_assert(std::is_trivially_destructible::value,  \
+#CLASS " should be trivially destructible!");
+#include "clang/AST/CommentNodes.inc"
+#undef COMMENT
+#undef ABSTRACT_COMMENT
+
+// DeclInfo is also allocated with a BumpPtrAllocator.
+static_assert(std::is_trivially_destructible::value,
+  "DeclInfo should be trivially destructible!");
+
 const char *Comment::getCommentKindName() const {
   switch (getCommentKind()) {
   case NoCommentKind: return "NoCommentKind";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66797: ArrayRef'ized CompilerInvocation::CreateFromArgs

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gribozavr added a reviewer: ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66797

Files:
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/driver/cc1_main.cpp
  clang/unittests/AST/ExternalASTSourceTest.cpp

Index: clang/unittests/AST/ExternalASTSourceTest.cpp
===
--- clang/unittests/AST/ExternalASTSourceTest.cpp
+++ clang/unittests/AST/ExternalASTSourceTest.cpp
@@ -53,7 +53,6 @@
   "test.cc", MemoryBuffer::getMemBuffer(FileContents).release());
   const char *Args[] = { "test.cc" };
   CompilerInvocation::CreateFromArgs(*Invocation, Args,
- Args + array_lengthof(Args),
  Compiler.getDiagnostics());
   Compiler.setInvocation(std::move(Invocation));
 
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -213,8 +213,8 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
-  bool Success = CompilerInvocation::CreateFromArgs(
-  Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);
+  bool Success =
+  CompilerInvocation::CreateFromArgs(Clang->getInvocation(), Argv, Diags);
 
   if (Clang->getFrontendOpts().TimeTrace) {
 llvm::timeTraceProfilerInitialize(
Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -168,9 +168,7 @@
   std::vector ClangArgv(ClangArgs.size());
   std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
  [](const std::string &s) -> const char * { return s.data(); });
-  CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.data(),
- &ClangArgv.data()[ClangArgv.size()],
- Ins->getDiagnostics());
+  CompilerInvocation::CreateFromArgs(*Inv, ClangArgv, Ins->getDiagnostics());
 
   {
 using namespace driver::types;
Index: clang/tools/arcmt-test/arcmt-test.cpp
===
--- clang/tools/arcmt-test/arcmt-test.cpp
+++ clang/tools/arcmt-test/arcmt-test.cpp
@@ -121,7 +121,7 @@
   }
 
   CompilerInvocation CI;
-  if (!CompilerInvocation::CreateFromArgs(CI, Args.begin(), Args.end(), *Diags))
+  if (!CompilerInvocation::CreateFromArgs(CI, Args, *Diags))
 return true;
 
   if (CI.getFrontendOpts().Inputs.empty()) {
@@ -160,8 +160,7 @@
   new DiagnosticsEngine(DiagID, &*DiagOpts, &*DiagClient));
 
   CompilerInvocation origCI;
-  if (!CompilerInvocation::CreateFromArgs(origCI, Args.begin(), Args.end(),
- *TopDiags))
+  if (!CompilerInvocation::CreateFromArgs(origCI, Args, *TopDiags))
 return true;
 
   if (origCI.getFrontendOpts().Inputs.empty()) {
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -118,9 +118,7 @@
 DiagnosticsEngine *Diagnostics, const llvm::opt::ArgStringList &CC1Args) {
   assert(!CC1Args.empty() && "Must at least contain the program name!");
   CompilerInvocation *Invocation = new CompilerInvocation;
-  CompilerInvocation::CreateFromArgs(
-  *Invocation, CC1Args.data() + 1, CC1Args.data() + CC1Args.size(),
-  *Diagnostics);
+  CompilerInvocation::CreateFromArgs(*Invocation, CC1Args, *Diagnostics);
   Invocation->getFrontendOpts().DisableFree = false;
   Invocation->getCodeGenOpts().DisableFree = false;
   return Invocation;
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -90,9 +90,7 @@
 
   const ArgStringList &CCArgs = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(
-  *CI, const_cast(CCArgs.data()),
-  const_cast(CCArgs.data()) + CCArgs.size(), *Diags) &&
+  if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags) &&
   !ShouldRecoverOnErorrs)
 return nullptr;
   return CI;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang

r370044 - [clang] Ensure that statements, expressions and types are trivially destructible

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Aug 27 04:35:49 2019
New Revision: 370044

URL: http://llvm.org/viewvc/llvm-project?rev=370044&view=rev
Log:
[clang] Ensure that statements, expressions and types are trivially destructible

Since statements, expressions and types are allocated with the BumpPtrAllocator
from ASTContext their destructor is not executed. Two classes are currently
exempted from the check : InitListExpr due to its ASTVector and
ConstantArrayType due to its APInt.

No functional changes.

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

Reviewed By: lebedev.ri, gribozavr

Modified:
cfe/trunk/lib/AST/Stmt.cpp
cfe/trunk/lib/AST/Type.cpp

Modified: cfe/trunk/lib/AST/Stmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=370044&r1=370043&r2=370044&view=diff
==
--- cfe/trunk/lib/AST/Stmt.cpp (original)
+++ cfe/trunk/lib/AST/Stmt.cpp Tue Aug 27 04:35:49 2019
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace clang;
 
@@ -83,6 +84,16 @@ const char *Stmt::getStmtClassName() con
 #CLASS " should not be polymorphic!");
 #include "clang/AST/StmtNodes.inc"
 
+// Check that no statement / expression class has a non-trival destructor.
+// Statements and expressions are allocated with the BumpPtrAllocator from
+// ASTContext and therefore their destructor is not executed.
+#define STMT(CLASS, PARENT)
\
+  static_assert(std::is_trivially_destructible::value,  
\
+#CLASS " should be trivially destructible!");
+// FIXME: InitListExpr is not trivially destructible due to its ASTVector.
+#define INITLISTEXPR(CLASS, PARENT)
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=370044&r1=370043&r2=370044&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Tue Aug 27 04:35:49 2019
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace clang;
 
@@ -299,6 +300,18 @@ QualType QualType::getSingleStepDesugare
 #CLASS "Type should not be polymorphic!");
 #include "clang/AST/TypeNodes.def"
 
+// Check that no type class has a non-trival destructor. Types are
+// allocated with the BumpPtrAllocator from ASTContext and therefore
+// their destructor is not executed.
+//
+// FIXME: ConstantArrayType is not trivially destructible because of its
+// APInt member. It should be replaced in favor of ASTContext allocation.
+#define TYPE(CLASS, BASE)  
\
+  static_assert(std::is_trivially_destructible::value ||  
\
+std::is_same::value,   
\
+#CLASS "Type should be trivially destructible!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)


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

r370045 - [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via cfe-commits]

Author: martong
Date: Tue Aug 27 04:36:10 2019
New Revision: 370045

URL: http://llvm.org/viewvc/llvm-project?rev=370045&view=rev
Log:
[ASTImporter] Fix name conflict handling with different strategies

There are numorous flaws about the name conflict handling, this patch
attempts fixes them. Changes in details:

* HandleNameConflict return with a false DeclarationName

Hitherto we effectively never returned with a NameConflict error, even
if the preceding StructuralMatch indicated a conflict.
Because we just simply returned with the parameter `Name` in
HandleNameConflict and that name is almost always `true` when converted to
`bool`.

* Add tests which indicate wrong NameConflict handling

* Add to ConflictingDecls only if decl kind is different

Note, we might not indicate an ODR error when there is an existing record decl
and a enum is imported with same name.  But there are other cases. E.g. think
about the case when we import a FunctionTemplateDecl with name f and we found a
simple FunctionDecl with name f. They overload.  Or in case of a
ClassTemplateDecl and CXXRecordDecl, the CXXRecordDecl could be the 'templated'
class, so it would be false to report error.  So I think we should report a
name conflict error only when we are 100% sure of that.  That is why I think it
should be a general pattern to report the error only if the kind is the same.

* Fix failing ctu test with EnumConstandDecl

In ctu-main.c we have the enum class 'A' which brings in the enum
constant 'x' with value 0 into the global namespace.
In ctu-other.c we had the enum class 'B' which brought in the same name
('x') as an enum constant but with a different enum value (42). This is clearly
an ODR violation in the global namespace. The solution was to rename the
second enum constant.

 * Introduce ODR handling strategies

Reviewers: a_sidorin, shafik

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

Modified:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.c
cfe/trunk/unittests/AST/ASTImporterFixtures.cpp
cfe/trunk/unittests/AST/ASTImporterFixtures.h
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=370045&r1=370044&r2=370045&view=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Tue Aug 27 04:36:10 2019
@@ -90,6 +90,8 @@ class TypeSourceInfo;
 using FileIDImportHandlerType =
 std::function;
 
+enum class ODRHandlingType { Conservative, Liberal };
+
 // An ImportPath is the list of the AST nodes which we visit during an
 // Import call.
 // If node `A` depends on node `B` then the path contains an `A`->`B` edge.
@@ -236,6 +238,8 @@ class TypeSourceInfo;
 /// Whether to perform a minimal import.
 bool Minimal;
 
+ODRHandlingType ODRHandling;
+
 /// Whether the last diagnostic came from the "from" context.
 bool LastDiagFromFrom = false;
 
@@ -326,6 +330,8 @@ class TypeSourceInfo;
 /// to-be-completed forward declarations when possible.
 bool isMinimalImport() const { return Minimal; }
 
+void setODRHandling(ODRHandlingType T) { ODRHandling = T; }
+
 /// \brief Import the given object, returns the result.
 ///
 /// \param To Import the object into this variable.
@@ -517,12 +523,11 @@ class TypeSourceInfo;
 ///
 /// \param NumDecls the number of conflicting declarations in \p Decls.
 ///
-/// \returns the name that the newly-imported declaration should have.
-virtual DeclarationName HandleNameConflict(DeclarationName Name,
-   DeclContext *DC,
-   unsigned IDNS,
-   NamedDecl **Decls,
-   unsigned NumDecls);
+/// \returns the name that the newly-imported declaration should have. Or
+/// an error if we can't handle the name conflict.
+virtual Expected
+HandleNameConflict(DeclarationName Name, DeclContext *DC, unsigned IDNS,
+   NamedDecl **Decls, unsigned NumDecls);
 
 /// Retrieve the context that AST nodes are being imported into.
 ASTContext &getToContext() const { return ToContext; }

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=370045&r1=370044&r2=370045&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Aug 27 04:36:10 2019
@@ -80,6 +80,7 @@ namespace clang {
   using ExpectedExpr = llvm::Expected;
   using ExpectedDecl = llvm::Expected;
   using ExpectedSL

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Note how I am partially at fault here, which is why the bug reports state that 
the crash originates from `TrackControlDependencyVisitor`, but even after that 
is fixed, the diagnostics construction flopped on an out-of-bounds error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66716



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

[PATCH] D66743: [clangd] Cleans up the semantic highlighting resources if clangd crashes.

[Johan Vikström via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:114
+  // restarts.
+  public crashDispose() {
+this.disposables.forEach((d) => d.dispose());

hokein wrote:
> we just dispose some class members (leading the class to an intermediate 
> state), I'd suggest that we dispose the whole class.
> 
> nit: please remove the "clangd crashes" bit in the API.
The only thing remaining to dispose is the highlighter. Should we remove all 
highlightings if clangd restarts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66743



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

[PATCH] D66743: [clangd] Cleans up the semantic highlighting resources if clangd crashes.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 217367.
jvikstrom added a comment.

Renamed disposables to subscriptions and removed clangd crashes in api.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66743

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
  
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

Index: clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
@@ -107,7 +107,8 @@
 highlighter.highlight('file1', []);
 assert.deepEqual(highlighter.applicationUriHistory, [ 'file1' ]);
 highlighter.initialize(tm);
-assert.deepEqual(highlighter.applicationUriHistory, [ 'file1', 'file1', 'file2' ]);
+assert.deepEqual(highlighter.applicationUriHistory,
+ [ 'file1', 'file1', 'file2' ]);
 // Groups decorations into the scopes used.
 let highlightingsInLine: semanticHighlighting.SemanticHighlightingLine[] = [
   {
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -56,6 +56,8 @@
   scopeLookupTable: string[][];
   // The object that applies the highlightings clangd sends.
   highlighter: Highlighter;
+  // Any disposables that should be cleaned up when clangd crashes.
+  private subscriptions: vscode.Disposable[] = [];
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
@@ -87,15 +89,19 @@
 // Important that highlighter is created before the theme is loading as
 // otherwise it could try to update the themeRuleMatcher without the
 // highlighter being created.
-this.highlighter = new Highlighter(this.scopeLookupTable);
+if (!this.highlighter)
+  // If there already is a highlighter existing there is no need to create a
+  // new one, just reset and keep using it.
+  this.highlighter = new Highlighter(this.scopeLookupTable);
+this.highlighter.clear();
 this.loadCurrentTheme();
 // Event handling for handling with TextDocuments/Editors lifetimes.
-vscode.window.onDidChangeVisibleTextEditors(
+this.subscriptions.push(vscode.window.onDidChangeVisibleTextEditors(
 (editors: vscode.TextEditor[]) =>
 editors.forEach((e) => this.highlighter.applyHighlights(
-e.document.uri.toString(;
-vscode.workspace.onDidCloseTextDocument(
-(doc) => this.highlighter.removeFileHighlightings(doc.uri.toString()));
+e.document.uri.toString();
+this.subscriptions.push(vscode.workspace.onDidCloseTextDocument(
+(doc) => this.highlighter.removeFileHighlightings(doc.uri.toString(;
   }
 
   handleNotification(params: SemanticHighlightingParams) {
@@ -103,6 +109,12 @@
 (line) => ({line : line.line, tokens : decodeTokens(line.tokens)}));
 this.highlighter.highlight(params.textDocument.uri, lines);
   }
+  // Disposes of any resources that are not reused when if initialize is called
+  // again.
+  public restartDispose() {
+this.subscriptions.forEach((d) => d.dispose());
+this.subscriptions = [];
+  }
 }
 
 // Converts a string of base64 encoded tokens into the corresponding array of
@@ -138,6 +150,7 @@
   constructor(scopeLookupTable: string[][]) {
 this.scopeLookupTable = scopeLookupTable;
   }
+  public clear() { this.files.clear(); }
   // This function must be called at least once or no highlightings will be
   // done. Sets the theme that is used when highlighting. Also triggers a
   // recolorization for all current highlighters. Should be called whenever the
@@ -174,6 +187,27 @@
 this.applyHighlights(fileUri);
   }
 
+  // Applies all the highlightings currently stored for a file with fileUri.
+  public applyHighlights(fileUri: string) {
+if (!this.files.has(fileUri))
+  // There are no highlightings for this file, must return early or will get
+  // out of bounds when applying the decorations below.
+  return;
+if (!this.decorationTypes.length)
+  // Can't apply any decorations when there is no theme loaded.
+  return;
+// This must always do a full re-highlighting due to the fact that
+// TextEditorDecorationType are very expensive to create (which makes
+// incremental updates infeasible).

[PATCH] D66646: [clang] Ensure that statements, expressions and types are trivially destructible

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370044: [clang] Ensure that statements, expressions and 
types are trivially destructible (authored by brunoricci, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66646?vs=216816&id=217370#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66646

Files:
  cfe/trunk/lib/AST/Stmt.cpp
  cfe/trunk/lib/AST/Type.cpp


Index: cfe/trunk/lib/AST/Stmt.cpp
===
--- cfe/trunk/lib/AST/Stmt.cpp
+++ cfe/trunk/lib/AST/Stmt.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace clang;
 
@@ -83,6 +84,16 @@
 #CLASS " should not be polymorphic!");
 #include "clang/AST/StmtNodes.inc"
 
+// Check that no statement / expression class has a non-trival destructor.
+// Statements and expressions are allocated with the BumpPtrAllocator from
+// ASTContext and therefore their destructor is not executed.
+#define STMT(CLASS, PARENT)
\
+  static_assert(std::is_trivially_destructible::value,  
\
+#CLASS " should be trivially destructible!");
+// FIXME: InitListExpr is not trivially destructible due to its ASTVector.
+#define INITLISTEXPR(CLASS, PARENT)
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);
Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace clang;
 
@@ -299,6 +300,18 @@
 #CLASS "Type should not be polymorphic!");
 #include "clang/AST/TypeNodes.def"
 
+// Check that no type class has a non-trival destructor. Types are
+// allocated with the BumpPtrAllocator from ASTContext and therefore
+// their destructor is not executed.
+//
+// FIXME: ConstantArrayType is not trivially destructible because of its
+// APInt member. It should be replaced in favor of ASTContext allocation.
+#define TYPE(CLASS, BASE)  
\
+  static_assert(std::is_trivially_destructible::value ||  
\
+std::is_same::value,   
\
+#CLASS "Type should be trivially destructible!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)


Index: cfe/trunk/lib/AST/Stmt.cpp
===
--- cfe/trunk/lib/AST/Stmt.cpp
+++ cfe/trunk/lib/AST/Stmt.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace clang;
 
@@ -83,6 +84,16 @@
 #CLASS " should not be polymorphic!");
 #include "clang/AST/StmtNodes.inc"
 
+// Check that no statement / expression class has a non-trival destructor.
+// Statements and expressions are allocated with the BumpPtrAllocator from
+// ASTContext and therefore their destructor is not executed.
+#define STMT(CLASS, PARENT)\
+  static_assert(std::is_trivially_destructible::value,  \
+#CLASS " should be trivially destructible!");
+// FIXME: InitListExpr is not trivially destructible due to its ASTVector.
+#define INITLISTEXPR(CLASS, PARENT)
+#include "clang/AST/StmtNodes.inc"
+
 void Stmt::PrintStats() {
   // Ensure the table is primed.
   getStmtInfoTableEntry(Stmt::NullStmtClass);
Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace clang;
 
@@ -299,6 +300,18 @@
 #CLASS "Type should not be polymorphic!");
 #include "clang/AST/TypeNodes.def"
 
+// Check that no type class has a non-trival destructor. Types are
+// allocated with the BumpPtrAllocator from ASTContext and therefore
+// their destructor is not executed.
+//
+// FIXME: ConstantArrayType is not trivially destructible because of its
+// APInt member. It should be replaced in favor of ASTContext allocation.
+#define TYPE(CLASS, BASE)  \
+  static_assert(std::is_trivially_destructible::value ||  \
+std::is_same::value,   \
+#CLASS "Type should be trivially destructible!");
+#include "clang/AST/TypeNodes.def"
+
 QualType Type::getLocallyUnqualifiedSingleStepDesugaredType() const {
   switch (getTypeClass()) {
 #define ABSTRACT_TYPE(Class, Parent)
_

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf035b75d8f07: [ASTImporter] Fix name conflict handling with 
different strategies (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/unittests/AST/ASTImporterFixtures.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/include/lldb/Symbol/ClangASTImporter.h

Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -252,7 +252,9 @@
 : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
  master.m_file_manager, true /*minimal*/),
   m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
-  m_master(master), m_source_ctx(source_ctx) {}
+  m_master(master), m_source_ctx(source_ctx) {
+  setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+}
 
 /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate
 /// and deattaches it at the end of the scope. Supports being used multiple
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -1879,7 +1879,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2432,6 +2432,62 @@
   EXPECT_EQ(ToD1, ToD2);
 }
 
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5127,15 +5183,6 @@
   }
 }
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest,
-DefaultTestValuesForRunOptions, );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
-::testing::Values(ArgVector()), );
-
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
-::testing::Values(ArgVector()), );
-
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
@@ -5233,8 +5280,8 @@
   // prototype (inside 'friend') for it comes first in the AST and is not
   // linked to the definition.
   EXPECT_EQ(ImportedDef, ToClassDef);
-}  
-  
+}
+
 struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
   LLDBLookupTest() {
 Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
@@ -5362,10 +5409,197 @@
   EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
 }
 
+struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
+  ConflictingDeclsWithLiberalStrategy() {
+this->ODRHandling = ASTImporter::ODRHandlingType::Liberal;
+  }
+};
+
+// Check that a Decl has been successfully imported into a standalone redecl
+// chain.
+template 
+static void CheckImportedAsNew(llvm::Expected &Result, Decl *ToTU,
+   PatternTy Pattern) {
+  ASSERT_TRUE(isSuccess(Result));
+  Decl *ImportedD = *

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thanks for the contribution and welcome to the LLVM community! A few more 
comments from me. I hope, this will help to tune to the LLVM coding style and 
conventions. This doc may be useful to read, if you haven't done so already: 
http://llvm.org/docs/CodingStandards.html




Comment at: clang-tidy/fpga/FPGATidyModule.cpp:5
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.

Eugene.Zelenko wrote:
> License was changed this year. Same in other files.
Please mark the addressed comments "Done".



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:31
+  // If not a definition, do nothing
+  if (Struct != StructDef)
+return;

If you need to only process struct definitions, it's better to use the 
`isDefinition()` matcher to limit the matches to definitions 
(`recordDecl(isStruct(), isDefinition())`).



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:37
+  unsigned int TotalBitSize = 0;
+  for (auto StructField : Struct->fields()) {
+// For each StructField, record how big it is (in bits)

`const auto *` will make it clear that it's a pointer. Actually, I'd better 
specify the type explicitly.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:45
+.Width;
+FieldSizes.push_back(std::pair(
+StructFieldWidth, StructField->getFieldIndex()));

`emplace_back(StructFieldWidth, StructField->getFieldIndex())` would be more 
succinct.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:47
+StructFieldWidth, StructField->getFieldIndex()));
+// TODO: Recommend a reorganization of the struct (sort by StructField 
size,
+// largest to smallest)

It's more common to use `FIXME` than `TODO` in comments in LLVM code.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:55
+  CharUnits CurrSize = Result.Context->getASTRecordLayout(Struct).getSize();
+  CharUnits MinByteSize = CharUnits::fromQuantity((TotalBitSize + 7) >> 3);
+  CharUnits CurrAlign =

` / 8` would be clearer than ` >> 3` (and any sane compiler would optimize this 
itself: https://gcc.godbolt.org/z/nXdGeU, even though here the optimization 
doesn't bring anything).



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:63
+  NewAlign = NewAlign.alignTo(
+  CharUnits::fromQuantity(((int)NewAlign.getQuantity()) << 1));
+  // Abort if the computed alignment meets the maximum configured alignment

Too many parentheses here. And the `<< 1` would be easier to read as `* 2`, if 
there's no intentional behavior difference here.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:65
+  // Abort if the computed alignment meets the maximum configured alignment
+  if (NewAlign.getQuantity() >= (1 << MAX_ALIGN_POWER_OF_TWO))
+break;

A well named constant for `(1 << MAX_ALIGN_POWER_OF_TWO)` would make the code 
easier to understand, IMO.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:75
+  if (Struct->hasAttrs()) {
+for (auto StructAttribute : Struct->getAttrs()) {
+  if (std::string(StructAttribute->getSpelling()).compare("packed") == 0) {

Same comment about `auto` as above: please specify the type explicitly or at 
least use `const auto *`.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:76
+for (auto StructAttribute : Struct->getAttrs()) {
+  if (std::string(StructAttribute->getSpelling()).compare("packed") == 0) {
+IsPacked = true;

It's better to construct an llvm::StringRef than a std::string: the former 
doesn't copy the contents. Next thing is that `operator==` should be preferred 
here, while `.compare()` is needed when ordering is important.

But here this all is irrelevant: `StructAttribute->getKind() == attr::Packed` 
allows to check this condition without comparing strings.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:100
+diag(Struct->getLocation(),
+ "struct %0 has inefficient access due to poor alignment. Currently "
+ "aligned to %1 bytes, but size %3 bytes is large enough to benefit "

Diagnostic messages are not complete sentences. Thus, different punctuation is 
used: "alignment. Currently" -> "alignment; currently"


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

https://reviews.llvm.org/D66564



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

[PATCH] D66588: [ARM NEON] Avoid duplicated decarations

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio planned changes to this revision.
dnsampaio added a comment.

Breaks the header. Needs to avoid generating calls to functions with predicated 
__noswap when it is BigEndianSafe.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66588



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

r370051 - Testing commit access; NFC

[Joe Ranieri via cfe-commits]

Author: jranieri
Date: Tue Aug 27 05:36:25 2019
New Revision: 370051

URL: http://llvm.org/viewvc/llvm-project?rev=370051&view=rev
Log:
Testing commit access; NFC

Modified:
cfe/trunk/www/index.html

Modified: cfe/trunk/www/index.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/index.html?rev=370051&r1=370050&r2=370051&view=diff
==
--- cfe/trunk/www/index.html (original)
+++ cfe/trunk/www/index.html Tue Aug 27 05:36:25 2019
@@ -105,7 +105,6 @@
  interested in
  following the development of Clang, signing up for a mailing list is a 
good
  way to learn about how the project works.
-
 
 
 


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

r370052 - Implement codegen for MSVC unions with reference members.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Aug 27 05:42:45 2019
New Revision: 370052

URL: http://llvm.org/viewvc/llvm-project?rev=370052&view=rev
Log:
Implement codegen for MSVC unions with reference members.

Currently, clang accepts a union with a reference member when given the 
-fms-extensions flag. This change fixes the codegen for this case.

Patch by Dominic Ferreira.

Added:
cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=370052&r1=370051&r2=370052&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Aug 27 05:42:45 2019
@@ -4056,7 +4056,6 @@ LValue CodeGenFunction::EmitLValueForFie
   unsigned RecordCVR = base.getVRQualifiers();
   if (rec->isUnion()) {
 // For unions, there is no pointer adjustment.
-assert(!FieldType->isReferenceType() && "union has reference member");
 if (CGM.getCodeGenOpts().StrictVTablePointers &&
 hasAnyVptr(FieldType, getContext()))
   // Because unions can easily skip invariant.barriers, we need to add
@@ -4073,27 +4072,30 @@ LValue CodeGenFunction::EmitLValueForFie
   addr.getPointer(), getDebugInfoFIndex(rec, 
field->getFieldIndex()), DbgInfo),
   addr.getAlignment());
 }
-  } else {
 
+if (FieldType->isReferenceType())
+  addr = Builder.CreateElementBitCast(
+  addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
+  } else {
 if (!IsInPreservedAIRegion)
   // For structs, we GEP to the field that the record layout suggests.
   addr = emitAddrOfFieldStorage(*this, addr, field);
 else
   // Remember the original struct field index
   addr = emitPreserveStructAccess(*this, addr, field);
+  }
 
-// If this is a reference field, load the reference right now.
-if (FieldType->isReferenceType()) {
-  LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
-  FieldTBAAInfo);
-  if (RecordCVR & Qualifiers::Volatile)
-RefLVal.getQuals().addVolatile();
-  addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
+  // If this is a reference field, load the reference right now.
+  if (FieldType->isReferenceType()) {
+LValue RefLVal =
+MakeAddrLValue(addr, FieldType, FieldBaseInfo, FieldTBAAInfo);
+if (RecordCVR & Qualifiers::Volatile)
+  RefLVal.getQuals().addVolatile();
+addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);
 
-  // Qualifiers on the struct don't apply to the referencee.
-  RecordCVR = 0;
-  FieldType = FieldType->getPointeeType();
-}
+// Qualifiers on the struct don't apply to the referencee.
+RecordCVR = 0;
+FieldType = FieldType->getPointeeType();
   }
 
   // Make sure that the address is pointing to the right type.  This is 
critical

Added: cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp?rev=370052&view=auto
==
--- cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp Tue Aug 27 05:42:45 2019
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+
+union A {
+  int *&ref;
+  int **ptr;
+};
+
+int *f1(A *a) {
+  return a->ref;
+}
+// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   ret i32* [[IP]]
+
+void f2(A *a) {
+  *a->ref = 1;
+}
+// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   store i32 1, i32* [[IP]]
+
+bool f3(A *a, int *b) {
+  return a->ref != b;
+}
+// CHECK-LABEL: define {{.*}}i1 @_Z2f3P1APi(%union.A* %a, i32* %b)
+// CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
+// CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
+// CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
+// CHECK:   [[IP2:%[^[:space:]]+]]  = load i32*, i32** %b.addr
+// CHECK:   icmp ne i32* [[IP]], [[IP2]]


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

[PATCH] D66797: ArrayRef'ized CompilerInvocation::CreateFromArgs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Yes, please!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66797



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

[PATCH] D66759: [clangd] Surface errors from command-line parsing

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D66759#1646599 , @ilya-biryukov 
wrote:

> How common is this? Do we have any particular examples?


Well, I don't have any data to prove, but my experience has been so far; most 
of the people don't touch Makefiles apart from adding their source files to the
CMakeLists within their project directory, and only some of them are 
comfortable with providing additional flags. For example having a new warning 
flag, or
a flag that has been recently renamed. Then your compiler won't complain during 
builds, but you will get those bogus errors in clangd.

> My worry comes from the fact that I don't think we can guarantee that clangd 
> is working for the users after we encountered those errors.
>  In particular, after D66731 , we will be 
> getting a compiler invocation and building the AST in many more cases. 
> However, the AST might be completely bogus, e.g. because we did not recognize 
> actually important flags.
>  Surfacing this error to the user is useful, because that would explain why 
> clangd is rusty, gives weird results, etc.
> 
> In turn, the users will report bugs and we'll be able to collect data on 
> whether this is noisy or everyone is doing compile_commands.json wrong and we 
> should do something about it or whether it's actually very rare in practice 
> and turns out to be a useful indicator for the users.
>  Either result seems plausible.
>  I'm all in for hiding annoying errors, but I don't know what the failure 
> modes are in that particular case and I'd be tempted to first see whether 
> surfacing the errors would **actually** end up producing those annoying 
> errors and what the errors are.

OK, I was leaning on "not showing up if we could build an AST" side because I 
thought that in those cases even if we have errors while building 
`CompilerInvocation`
these would most of the time be some small things that would not affect how AST 
was generated in a sense that users would care. Therefore we would end up 
annoying
users most of the time and be really useful only in some small portion of the 
cases.

But again this isn't backed by any data, following your approach of turning it 
on and changing it later on if we annoy people also sounds OK to me. Also as 
discussed offline
I misunderstood the scope of the diags generated while building compiler 
invocations. Since this only surfaces errors, it should be mostly OK.




Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:465
+Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
+  // Finally, add diagnostics coming from the AST.
+  {

could you preserve the previous order by inserting `CompilerInvocationDiags` 
here instead of the ones in `AST` ?



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:399
+  // Do not forget to emit a pending diagnostic if there is one.
+  flushLastDiag();
+

I suppose we can get rid of the one in `StoreDiags::EndSourceFile` now ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66759



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

[PATCH] D62571: Implement codegen for MSVC unions with reference members

[Aaron Ballman via Phabricator via cfe-commits]

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

In D62571#1646227 , @domdom wrote:

> Rebased onto master, clang format the patch.
>
> Merge conflict resolve by having the bitcast of the field reference happening 
> after recording access index.


Thanks! I've committed in r370052.


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

https://reviews.llvm.org/D62571



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

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 217383.
SjoerdMeijer edited the summary of this revision.
SjoerdMeijer added a parent revision: D66796: [clang] Loop pragma 
vectorize(disable).
SjoerdMeijer added a comment.

Stripped out the functional change related to vectorize(disable).


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

https://reviews.llvm.org/D66290

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp


Index: clang/test/CodeGenCXX/pragma-loop.cpp
===
--- clang/test/CodeGenCXX/pragma-loop.cpp
+++ clang/test/CodeGenCXX/pragma-loop.cpp
@@ -158,23 +158,41 @@
   for_template_constant_expression_test(List, Length);
 }
 
+void vec_width_1(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}vec_width_1{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]]
+
+  #pragma clang loop vectorize(enable) vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+void width_1(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}width_1{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_16:.*]]
+
+  #pragma clang loop vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
 // CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}
 // CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
 
-// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], 
![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}
+// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], 
![[DISTRIBUTE_DISABLE:.*]], ![[VECTORIZE_ENABLE:.*]], ![[WIDTH_8:.*]], 
![[INTERLEAVE_4:.*]]}
 // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
 // CHECK: ![[DISTRIBUTE_DISABLE]] = !{!"llvm.loop.distribute.enable", i1 false}
+// CHECK: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
 // CHECK: ![[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}
 // CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}
 
-// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], 
![[INTENABLE_1:.*]], ![[FOLLOWUP_VECTOR_3:.*]]}
-// CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], 
![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3:.*]]}
 // CHECK: ![[FOLLOWUP_VECTOR_3]] = !{!"llvm.loop.vectorize.followup_all", 
![[AFTER_VECTOR_3:.*]]}
 // CHECK: ![[AFTER_VECTOR_3]] = distinct !{![[AFTER_VECTOR_3]], 
![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 // CHECK: ![[ISVECTORIZED]] = !{!"llvm.loop.isvectorized"}
 // CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}
 
-// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], 
![[INTERLEAVE_2:.*]]}
+// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[VECTORIZE_ENABLE]], 
![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}
 // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
 
@@ -185,7 +203,7 @@
 // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", 
![[AFTER_VECTOR_6:.*]]}
 // CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]], 
![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 
-// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]]}
+// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[VECTORIZE_ENABLE]], 
![[WIDTH_5:.*]]}
 // CHECK: ![[WIDTH_5]] = !{!"llvm.loop.vectorize.width", i32 5}
 
 // CHECK: ![[LOOP_8]] = distinct !{![[LOOP_8]], ![[WIDTH_5:.*]]}
@@ -213,5 +231,9 @@
 // CHECK: ![[AFTER_VECTOR_13]] = distinct !{![[AFTER_VECTOR_13]], 
![[ISVECTORIZED:.*]], ![[UNROLL_32:.*]]}
 // CHECK: ![[UNROLL_32]] = !{!"llvm.loop.unroll.count", i32 32}
 
-// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]]}
+// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[VECTORIZE_ENABLE]], 
![[WIDTH_10:.*]]}
 // CHECK: ![[WIDTH_10]] = !{!"llvm.loop.vectorize.width", i32 10}
+
+// CHECK:  ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[VEC_WIDTH_1:.*]], 
![[VECTORIZE_ENABLE]]}
+// CHECK-NEXT: ![[VEC_WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK-NEXT: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[VECTORIZE_ENABLE]], 
![[VEC_WIDTH_1]]}
Index: clang/lib/CodeGen/CGLoopInfo.cpp
===
--- clang/lib/CodeGen/CGLoopInfo.cpp
+++ clang/lib/CodeGen/CGLoopInfo.cpp
@@ -270,6 +270,14 @@
 
   // Setting vectorize.width
   if (Attrs.VectorizeWidth > 0) {
+// which implies vectorize.enable = true, but only add it when it is not
+// already enabled.
+if (Attrs.VectorizeEnable != LoopAttributes::Enable)
+  Args.push_back(
+  MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
+ConstantAsMetadata::get(ConstantInt::get(
+llvm::Type::getInt1Ty(Ctx), 1))}));
+
 Metadata *Vals[] = {
 MDString::get(Ctx, "llvm.loop.vecto

[PATCH] D66759: [clangd] Surface errors from command-line parsing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

Just to clarify: we only want to surface errors, not warnings from command-line 
to avoid too much nosie; I'm totally on board with not spamming users with 
"unknown compiler warning" errors at the start of each file.
We would only show things like `unknown argument "-mdisable-fp-elim"`. Given 
that clang understands most flags from other compilers (to be a good drop-in 
replacement), we should end up showing errors when things actually can be 
significantly broken.

And we will now show this errors only in cases when we could not build the AST 
at all before (as `CompilerInvocation` was null), so overall we should not be 
producing too much new noise




Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:465
+Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
+  // Finally, add diagnostics coming from the AST.
+  {

kadircet wrote:
> could you preserve the previous order by inserting `CompilerInvocationDiags` 
> here instead of the ones in `AST` ?
Do you think we should add `CompilerInvocatioDiags` at the end, rather than at 
the start of the diagnostics list?
Why would that be better?

Mostly trying to keep the chronological order here: command-line diagnostics 
came first, followed by preamble, followed by AST diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66759



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

[PATCH] D66199: [docs] loop pragmas

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Many thanks for your clarification!

>   What we were discussing was that these settings would remove 0) from the 
> candidate list as well.

Yep, that's crystal clear now.
And my expectation would indeed be that this would be the case.


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

https://reviews.llvm.org/D66199



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

[PATCH] D66759: [clangd] Surface errors from command-line parsing

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:465
+Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
+  // Finally, add diagnostics coming from the AST.
+  {

ilya-biryukov wrote:
> kadircet wrote:
> > could you preserve the previous order by inserting 
> > `CompilerInvocationDiags` here instead of the ones in `AST` ?
> Do you think we should add `CompilerInvocatioDiags` at the end, rather than 
> at the start of the diagnostics list?
> Why would that be better?
> 
> Mostly trying to keep the chronological order here: command-line diagnostics 
> came first, followed by preamble, followed by AST diagnostics.
I totally agree that your current ordering makes more sense. I just wasn't sure 
why it was done in the opposite way before(first AST, then preamble), if it 
doesn't cause any test breakages we should be good though.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:603
 FillDiagBase(*LastDiag);
 LastDiagWasAdjusted = false;
 if (!InsideMainFile)

I suppose this one is also not required anymore, as you clear it during flush



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:755
+
+  EXPECT_THAT(
+  Diagnostics,

as discussed offline, could you also add a test to make sure this does not fire 
on `"-Wunknown-warning-flag"` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66759



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

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

This LGTM, but given how much discussion there has been about MaxPromoteWidth 
it would be great to get some test coverage for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57450



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Jenkins is not green 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/743/
However, the newly failing test is `TestTargetCommand.py`, which seems to be 
unrelated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692



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

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

First a wall of text to reply to the general comments. Let's chat if this 
doesn't make sense.

In D66751#1646545 , @ilya-biryukov 
wrote:

> Mostly LGTM, thanks! Just a few clarifying questions and suggestions
>
> This API allows us to get a target declaration for a reference, but does not 
> help with finding the source locations for those references. Do you plan to 
> put this into the API? Have this as a separate function?


Yes. I actually ripped this out of the patch, because it wasn't related enough 
and the patch is large.

To restate the relationship between these functions:
Many operations ultimately want to join source location and decl, e.g. gtd is 
location -> node -> decl, xrefs is decl -> node -> location, indexing is node 
-> (location, decl).
So providing node->decl and node->location primitives is sufficient. If you 
want to go in the other direction (decl->node or location->node) then of course 
you need to traverse and/or build an index.

With that said, there are several legitimate node->location models, and I think 
this is a good reason to to keep that decoupled from node->decl (testability is 
another). In particular:

the "flat" model


says that each node may have a token which is its "handle", and each token can 
be the handle for (refer to) only one node.

  int a = X::y;
  iii a   X  y

What is the range for a node? Its handle token.
What is the node for a range? The unique handle token covered by the range.

Strengths: accurate, precise triggering on node names. simplicity (conforms to 
user's view of the code)
Weaknesses: multi-token ranges (selections), implicit nodes (they don't get to 
have locations), requires specific per-node implementation
Use case: go-to-definition/xrefs triggering, xrefs results (nodes referenced by 
name)

the "hierarchical" model


says that each node owns a range of tokens, and these nest.

  int a = X::y;
  X
  XXX
  iii 
  

What is the range for a node? Its full range.
What is the node for a range? The innermost node that entirely contains the 
selection.

Strengths: exposing underlying structure/hierarchy of the code, can be (mostly) 
implemented generically
Weaknesses: loose triggering that doesn't distinguish e.g. node names from 
qualifiers
Use case: extract variable, structured selection, xrefs results (nodes 
referenced implicitly)

---

I think we're going to need/want both. The hierarchical model corresponds to 
SelectionTree. The flat model is the thing we need to build. libIndex looks 
more like the flat model than the hierarchical one, but has elements of both I 
think.
(If you think the above explanation is useful, we can check something like it 
in somewhere)

> E.g. finding a source location of field for DeclRefExpr produced for 
> MyBase::field seems to be the same amount of work (in general case) as 
> finding the Decl* of field.

For the hierarchical model it's easy, this is one of the few methods 
DynTypeNode actually has :-)
For the flat model: it's not trivial, but is less work because while you do 
have to dispatch over all node types, you don't have general multi-hop/graph 
cases (I think).
Usually the "handle" tokens are well-supported by the AST because they're 
precisely the ones that diagnostics want to report. So in the DeclRefExpr case, 
DeclRefExpr::getLoc() returns the right thing.

In D66751#1646675 , @kadircet wrote:

> I agree with Ilya's concerns on SourceLocations, even though most of the time 
> user will have the source locations available in the dyntyped node, they 
> might need some traversals to fetch the exact range especially in cases of 
> nested name specifiers.


I think if you want the (full) range, this is easy as mentioned above: 
getSourceRange() (available on DynTypedNode, but also on each specific node 
type).
If you want just the "handle" token for the flat model, NNS isn't a 
particularly hard case: it's basically a linked list. Each node claims one 
component of the NNS as its "handle". If you want to treat NNS as part of the 
parent (so pointing at X in X::y counts as a ref to Y) then I think you're 
better off with the hierarchical model.

> It would be nice to provide a referencing range in addition to the decls.

Concretely, can you give an example of which node and what you'd want it to 
point to? If it's just the single unqualified-name token, I agree and would 
like to add it (as a separate patch). If it's the full range, I think that's 
getSourceRange(). Is it something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

[Pavel Labath via Phabricator via cfe-commits]

labath added a comment.

In D59692#1646990 , @martong wrote:

> Jenkins is not green 
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/743/
>  However, the newly failing test is `TestTargetCommand.py`, which seems to be 
> unrelated.


That ought to be fixed by r370057.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59692



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

r370058 - Speculatively fix the build bots after r370052.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Aug 27 06:45:42 2019
New Revision: 370058

URL: http://llvm.org/viewvc/llvm-project?rev=370058&view=rev
Log:
Speculatively fix the build bots after r370052.

Modified:
cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp

Modified: cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp?rev=370058&r1=370057&r2=370058&view=diff
==
--- cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/ms-union-member-ref.cpp Tue Aug 27 06:45:42 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fms-extensions %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple=i686-pc-win32 -fms-extensions %s -emit-llvm -o- | 
FileCheck %s
 
 union A {
   int *&ref;
@@ -8,7 +8,7 @@ union A {
 int *f1(A *a) {
   return a->ref;
 }
-// CHECK-LABEL: define {{.*}}i32* @_Z2f1P1A(%union.A* %a)
+// CHECK-LABEL: define {{.*}}i32* @"?f1@@YAPAHPATA@@@Z"(%union.A* %a)
 // CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
 // CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
 // CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
@@ -17,7 +17,7 @@ int *f1(A *a) {
 void f2(A *a) {
   *a->ref = 1;
 }
-// CHECK-LABEL: define {{.*}}void @_Z2f2P1A(%union.A* %a)
+// CHECK-LABEL: define {{.*}}void @"?f2@@YAXPATA@@@Z"(%union.A* %a)
 // CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
 // CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
 // CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]
@@ -26,7 +26,7 @@ void f2(A *a) {
 bool f3(A *a, int *b) {
   return a->ref != b;
 }
-// CHECK-LABEL: define {{.*}}i1 @_Z2f3P1APi(%union.A* %a, i32* %b)
+// CHECK-LABEL: define {{.*}}i1 @"?f3@@YA_NPATA@@PAH@Z"(%union.A* %a, i32* %b)
 // CHECK:   [[REF:%[^[:space:]]+]] = bitcast %union.A* %{{.*}} to i32***
 // CHECK:   [[IPP:%[^[:space:]]+]] = load i32**, i32*** [[REF]]
 // CHECK:   [[IP:%[^[:space:]]+]]  = load i32*, i32** [[IPP]]


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

r370059 - Quote the token being diagnosed for C11 extensions.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Aug 27 06:47:51 2019
New Revision: 370059

URL: http://llvm.org/viewvc/llvm-project?rev=370059&view=rev
Log:
Quote the token being diagnosed for C11 extensions.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/test/Parser/c1x-alignas.c
cfe/trunk/test/Sema/thread-specifier.c
cfe/trunk/test/SemaOpenCLCXX/restricted.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=370059&r1=370058&r2=370059&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Aug 27 06:47:51 
2019
@@ -127,7 +127,7 @@ def note_previous_default_assoc : Note<
   "previous default generic association is here">;
 
 def ext_c11_feature : Extension<
-  "%0 is a C11 extension">, InGroup;
+  "'%0' is a C11 extension">, InGroup;
 
 def ext_c11_noreturn : Extension<
   "_Noreturn functions are a C11-specific feature">, InGroup;

Modified: cfe/trunk/test/Parser/c1x-alignas.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/c1x-alignas.c?rev=370059&r1=370058&r2=370059&view=diff
==
--- cfe/trunk/test/Parser/c1x-alignas.c (original)
+++ cfe/trunk/test/Parser/c1x-alignas.c Tue Aug 27 06:47:51 2019
@@ -9,5 +9,5 @@ char c4 _Alignas(32); // expected-error
 
 char _Alignas(_Alignof(int)) c5;
 
-// CHECK-EXT: _Alignas is a C11 extension
-// CHECK-EXT: _Alignof is a C11 extension
+// CHECK-EXT: '_Alignas' is a C11 extension
+// CHECK-EXT: '_Alignof' is a C11 extension

Modified: cfe/trunk/test/Sema/thread-specifier.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/thread-specifier.c?rev=370059&r1=370058&r2=370059&view=diff
==
--- cfe/trunk/test/Sema/thread-specifier.c (original)
+++ cfe/trunk/test/Sema/thread-specifier.c Tue Aug 27 06:47:51 2019
@@ -11,16 +11,16 @@
 #undef __private_extern__
 #endif
 
-__thread int t1; // thread-local-warning {{_Thread_local is a C11 extension}}
-__thread extern int t2; // thread-local-warning {{_Thread_local is a C11 
extension}}
-__thread static int t3; // thread-local-warning {{_Thread_local is a C11 
extension}}
+__thread int t1; // thread-local-warning {{'_Thread_local' is a C11 extension}}
+__thread extern int t2; // thread-local-warning {{'_Thread_local' is a C11 
extension}}
+__thread static int t3; // thread-local-warning {{'_Thread_local' is a C11 
extension}}
 #ifdef GNU
 // expected-warning@-3 {{'__thread' before 'extern'}}
 // expected-warning@-3 {{'__thread' before 'static'}}
 #endif
 
-__thread __private_extern__ int t4; // thread-local-warning {{_Thread_local is 
a C11 extension}}
-struct t5 { __thread int x; }; // thread-local-warning {{_Thread_local is a 
C11 extension}}
+__thread __private_extern__ int t4; // thread-local-warning {{'_Thread_local' 
is a C11 extension}}
+struct t5 { __thread int x; }; // thread-local-warning {{'_Thread_local' is a 
C11 extension}}
 #ifdef __cplusplus
 // expected-error-re@-2 {{'{{__thread|_Thread_local|thread_local}}' is only 
allowed on variable declarations}}
 #else
@@ -28,7 +28,7 @@ struct t5 { __thread int x; }; // thread
 // expected-error@-5 {{type name does not allow storage class to be specified}}
 #endif
 
-__thread int t6(); // thread-local-warning {{_Thread_local is a C11 extension}}
+__thread int t6(); // thread-local-warning {{'_Thread_local' is a C11 
extension}}
 #if defined(GNU)
 // expected-error@-2 {{'__thread' is only allowed on variable declarations}}
 #elif defined(C11) || defined(C99)
@@ -38,53 +38,53 @@ __thread int t6(); // thread-local-warni
 #endif
 
 int f(__thread int t7) { // expected-error {{' is only allowed on variable 
declarations}} \
- // thread-local-warning {{_Thread_local is a C11 
extension}}
-  __thread int t8; // thread-local-warning {{_Thread_local is a C11 extension}}
+ // thread-local-warning {{'_Thread_local' is a C11 
extension}}
+  __thread int t8; // thread-local-warning {{'_Thread_local' is a C11 
extension}}
 #if defined(GNU)
   // expected-error@-2 {{'__thread' variables must have global storage}}
 #elif defined(C11) || defined(C99)
   // expected-error@-4 {{'_Thread_local' variables must have global storage}}
 #endif
-  extern __thread int t9; // thread-local-warning {{_Thread_local is a C11 
extension}}
-  static __thread int t10; // thread-local-warning {{_Thread_local is a C11 
extension}}
-  __thread __private_extern__ int t11; // thread-local-warning {{_Thread_local 
is a C11 extension}}
+  extern __thread int t9; // thread-local-warning {{'_Thread_local' is a C11 
extension}}
+  static __thread int t10; // thread-local-warning {{'_Thread_local' i

r370060 - Fix text range end columns in SARIF to be exclusive

[Joe Ranieri via cfe-commits]

Author: jranieri
Date: Tue Aug 27 06:49:45 2019
New Revision: 370060

URL: http://llvm.org/viewvc/llvm-project?rev=370060&view=rev
Log:
Fix text range end columns in SARIF to be exclusive

According to the SARIF specification, "a text region does not include the 
character specified by endColumn".

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=370060&r1=370059&r2=370060&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Aug 27 06:49:45 
2019
@@ -143,11 +143,17 @@ static json::Object createFileLocation(c
 }
 
 static json::Object createTextRegion(SourceRange R, const SourceManager &SM) {
-  return json::Object{
+  json::Object Region{
   {"startLine", SM.getExpansionLineNumber(R.getBegin())},
-  {"endLine", SM.getExpansionLineNumber(R.getEnd())},
   {"startColumn", SM.getExpansionColumnNumber(R.getBegin())},
-  {"endColumn", SM.getExpansionColumnNumber(R.getEnd())}};
+  };
+  if (R.getBegin() == R.getEnd()) {
+Region["endColumn"] = SM.getExpansionColumnNumber(R.getBegin());
+  } else {
+Region["endLine"] = SM.getExpansionLineNumber(R.getEnd());
+Region["endColumn"] = SM.getExpansionColumnNumber(R.getEnd()) + 1;
+  }
+  return Region;
 }
 
 static json::Object createPhysicalLocation(SourceRange R, const FileEntry &FE,

Modified: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif?rev=370060&r1=370059&r2=370060&view=diff
==
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 (original)
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 Tue Aug 27 06:49:45 2019
@@ -44,7 +44,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 5,
+"endColumn": 6,
 "endLine": 13,
 "startColumn": 3,
 "startLine": 13
@@ -63,7 +63,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 17,
+"endColumn": 18,
 "endLine": 9,
 "startColumn": 11,
 "startLine": 9
@@ -83,7 +83,7 @@
   "fileIndex": 0,
 },
 "region": {
-  "endColumn": 17,
+  "endColumn": 18,
   "endLine": 9,
   "startColumn": 11,
   "startLine": 9

Modified: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif?rev=370060&r1=370059&r2=370060&view=diff
==
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
 (original)
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
 Tue Aug 27 06:49:45 2019
@@ -64,7 +64,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 5,
+"endColumn": 6,
 "endLine": 24,
 "startColumn": 3,
 "startLine": 24
@@ -83,7 +83,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 17,
+"endColumn": 18,
 "endLine": 9,
 "startColumn": 11,
 "startLine": 9
@@ -103,7 +103,7 @@
   "fileIndex": 0,
 },
 "region": {
-  "endColumn": 17,
+  "endColumn": 18,
   "endLine": 

Re: r363086 - For DR712: store on a DeclRefExpr whether it constitutes an odr-use.

[Hans Wennborg via cfe-commits]

Bisection for https://bugs.llvm.org/show_bug.cgi?id=42861 also points
to this revision.

On Wed, Jun 12, 2019 at 1:49 AM Richard Smith via cfe-commits
 wrote:
>
> Thanks, should be fixed by r363113.
>
> On Tue, 11 Jun 2019 at 16:21, Reid Kleckner via cfe-commits 
>  wrote:
>>
>>
>> The new assert is firing across Chromium, please fix or revert. This is a 
>> reduced test case:
>>
>> int a;
>> struct Foo {
>>   template  void operator()(b, c = a);
>> };
>> Foo d;
>> bool e;
>> decltype(d(e)) gv;
>>
>> This causes the "missing non-odr-use marking for unevaluated decl ref" 
>> assertion on SemaExpr.cpp:16227 to fire.
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65206: [analyzer] Fix text range end columns in SARIF to be exclusive

[Joe Ranieri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68a6a28ef835: Fix text range end columns in SARIF to be 
exclusive (authored by jranieri-grammatech).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65206

Files:
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif

Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -64,7 +64,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 5,
+"endColumn": 6,
 "endLine": 24,
 "startColumn": 3,
 "startLine": 24
@@ -83,7 +83,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 17,
+"endColumn": 18,
 "endLine": 9,
 "startColumn": 11,
 "startLine": 9
@@ -103,7 +103,7 @@
   "fileIndex": 0,
 },
 "region": {
-  "endColumn": 17,
+  "endColumn": 18,
   "endLine": 9,
   "startColumn": 11,
   "startLine": 9
@@ -134,7 +134,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 5,
+"endColumn": 6,
 "endLine": 25,
 "startColumn": 3,
 "startLine": 25
@@ -153,7 +153,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 10,
+"endColumn": 11,
 "endLine": 13,
 "startColumn": 3,
 "startLine": 13
@@ -172,7 +172,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 8,
+"endColumn": 9,
 "endLine": 14,
 "startColumn": 3,
 "startLine": 14
@@ -192,7 +192,7 @@
   "fileIndex": 0,
 },
 "region": {
-  "endColumn": 8,
+  "endColumn": 9,
   "endLine": 14,
   "startColumn": 3,
   "startLine": 14
@@ -223,7 +223,7 @@
 "fileIndex": 0,
   },
   "region": {
-"endColumn": 12,
+"endColumn": 13,
 "endLine": 18,
 "startColumn": 7,
 "startLine": 18
@@ -243,7 +243,6 @@
   },
   "region": {
 "endColumn": 3,
-"endLine": 18,
 "startColumn": 3,
 "startLine": 18
   }
@@ -262,7 +261,6 @@
   },
   "region": {
 "endColumn": 14,
-"endLine": 19,
 "startColumn": 14,
 "startLine": 19
   }
@@ -282,7 +280,6 @@
 },
 "region": {
   "endColumn": 14,
-  "endLine": 19,
   "startColumn": 14,
   "startLine": 19
 }
Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
===
--- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
@@ -44,7 +44,7 @@
 "fileIndex": 0,
   },
   "region": {
-  

r370061 - Fix a SARIF exporter crash with macro expansions

[Joe Ranieri via cfe-commits]

Author: jranieri
Date: Tue Aug 27 07:20:27 2019
New Revision: 370061

URL: http://llvm.org/viewvc/llvm-project?rev=370061&view=rev
Log:
Fix a SARIF exporter crash with macro expansions

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
cfe/trunk/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=370061&r1=370060&r2=370061&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Aug 27 07:20:27 
2019
@@ -219,9 +219,10 @@ static json::Object createThreadFlow(con
   for (const auto &Piece : Pieces) {
 const PathDiagnosticLocation &P = Piece->getLocation();
 Locations.push_back(createThreadFlowLocation(
-createLocation(createPhysicalLocation(P.asRange(),
-  *P.asLocation().getFileEntry(),
-  SMgr, Files),
+createLocation(createPhysicalLocation(
+   P.asRange(),
+   *P.asLocation().getExpansionLoc().getFileEntry(),
+   SMgr, Files),
Piece->getString()),
 calculateImportance(*Piece)));
   }
@@ -255,7 +256,8 @@ static json::Object createResult(const P
   {"locations",
json::Array{createLocation(createPhysicalLocation(
Diag.getLocation().asRange(),
-   *Diag.getLocation().asLocation().getFileEntry(), SMgr, Files))}},
+   *Diag.getLocation().asLocation().getExpansionLoc().getFileEntry(),
+   SMgr, Files))}},
   {"ruleIndex", Iter->getValue()},
   {"ruleId", Diag.getCheckName()}};
 }

Modified: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif?rev=370061&r1=370060&r2=370061&view=diff
==
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
 (original)
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
 Tue Aug 27 07:20:27 2019
@@ -6,7 +6,7 @@
 {
   "fileLocation": {
   },
-  "length": 686,
+  "length": 951,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
@@ -43,6 +43,16 @@
 "name": {
   "text": "core.DivideZero"
 }
+  },
+  {
+"fullDescription": {
+  "text": "Check for memory leaks, double free, and use-after-free 
problems. Traces memory managed by malloc()/free()."
+},
+"helpUri": 
"https://clang-analyzer.llvm.org/available_checks.html#unix.Malloc";,
+"id": "unix.Malloc",
+"name": {
+  "text": "unix.Malloc"
+}
   }
 ]
   },
@@ -65,9 +75,9 @@
   },
   "region": {
 "endColumn": 6,
-"endLine": 24,
+"endLine": 34,
 "startColumn": 3,
-"startLine": 24
+"startLine": 34
   }
 }
   }
@@ -84,9 +94,9 @@
   },
   "region": {
 "endColumn": 18,
-"endLine": 9,
+"endLine": 11,
 "startColumn": 11,
-"startLine": 9
+"startLine": 11
   }
 }
   }
@@ -104,9 +114,9 @@
 },
 "region": {
   "endColumn": 18,
-  "endLine": 9,
+  "endLine": 11,
   "startColumn": 11,
-  "startLine": 9
+  "startLine": 11
 }
   }
 }
@@ -135,9 +145,9 @@
   },
   "region": {
 "endColumn": 6,
-"endLine": 25,
+"endLine": 35,
 "startColumn": 3,
-"startLine": 25
+"startLine": 35

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:79
+  static const Decl *getTemplatePattern(const Decl *D) {
+if (const CXXRecordDecl *SD = dyn_cast(D)) {
+  return SD->getTemplateInstantiationPattern();

kadircet wrote:
> nit: s/SD/CRD/
Done.
(This was copy/pasted from libIndex, I wonder what SD stood for. StructDecl?)



Comment at: clang-tools-extra/clangd/FindTarget.h:86
+  /// This declaration is an alias that was referred to.
+  Alias,
+  /// This is the underlying declaration for an alias, decltype etc.

ilya-biryukov wrote:
> Could you provide examples for `Alias` and `Underlying` in the comments?
> One with a template and one with a namespace alias
Done. Added the code example as a section above the values, and the results 
with the comment on each value - hope this isn't too confusing.



Comment at: clang-tools-extra/clangd/FindTarget.h:92
+  /// (e.g. a delegating constructor has a pure-lexical reference to the class)
+  PureLexical,
+};

ilya-biryukov wrote:
> Could you also provide an example here?
> It this a delegating constructor in the constructor-init-list?
It was, but it was misguided - the delegating constructor calls have a TypeLoc 
which references the type being delegated to, so there's not much point 
handling them I think.

(There would be if we could get the constructor, but we can't: I guess in 
general it may not be resolved)

This was the only user of PureLexical, so I just removed it.




Comment at: clang-tools-extra/clangd/FindTarget.h:97
+class DeclRelationSet {
+  using Set = std::bitset(DeclRelation::PureLexical) + 
1>;
+  Set S;

kadircet wrote:
> Could you rather put an end marker into the `DeclRelation` and make use of it 
> in here? As people might forget to update this one if they add anything into 
> DeclRelation.
> 
> (It seems rare for this enum to change though, so feel free to ignore if you 
> believe it will stay the same).
Hmm, I'm reluctant to add a "last" member to a strongly-typed enum, and force 
switch to handle it etc.

I could add it next to the enum, but then it's ugly and still easy enough to 
miss.

Ultimately, I don't think it's likely this will be missed: `bitset::set()` 
checks its argument and throws out_of_range (i.e. aborts) if we try to set an 
invalid bit. So any test that exercised a new value would crash in all build 
configs.



Comment at: clang-tools-extra/clangd/FindTarget.h:103
+  DeclRelationSet() = default;
+  DeclRelationSet(DeclRelation R) { S.set(static_cast(R)); 
}
+

ilya-biryukov wrote:
> Why not `unsigned`? What are the interesting corner cases?
Oops, this dates back to when I was prematurely-templatizing this class. 
Reverted.



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:49
+EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
+llvm::Annotations::Range R = A.llvm::Annotations::range();
+SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,

kadircet wrote:
> can't you just call `A.range()` ? :D
> I hope it wasn't clangd that somehow code-completed this.
A.range() is an LSP Location (line/col pairs), offsets seem slightly more 
convenient here.

This is ugly though, using Annotations with SourceLocations seems unneccesarily 
clunky. Ideas?

(this reminds me, these test helpers need comments, added some)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751



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

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 217392.
sammccall marked 11 inline comments as done.
sammccall added a comment.

review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66751

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -0,0 +1,469 @@
+//===-- FindSymbolsTests.cpp -*- C++ -*===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "FindTarget.h"
+
+#include "Annotations.h"
+#include "Selection.h"
+#include "TestTU.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// A referenced Decl together with its DeclRelationSet, for assertions.
+//
+// There's no great way to assert on the "content" of a Decl in the general case
+// that's both expressive and unambiguous (e.g. clearly distinguishes between
+// templated decls and their specializations).
+//
+// We use the result of pretty-printing the decl, with the {body} truncated.
+struct PrintedDecl {
+  PrintedDecl(const char *Name, DeclRelationSet Relations = {})
+  : Name(Name), Relations(Relations) {}
+  PrintedDecl(const Decl *D, DeclRelationSet Relations = {})
+  : Relations(Relations) {
+std::string S;
+llvm::raw_string_ostream OS(S);
+D->print(OS);
+llvm::StringRef FirstLine =
+llvm::StringRef(OS.str()).take_until([](char C) { return C == '\n'; });
+FirstLine = FirstLine.rtrim(" {");
+Name = FirstLine.rtrim(" {");
+  }
+
+  std::string Name;
+  DeclRelationSet Relations;
+};
+bool operator==(const PrintedDecl &L, const PrintedDecl &R) {
+  return std::tie(L.Name, L.Relations) == std::tie(R.Name, R.Relations);
+}
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const PrintedDecl &D) {
+  return OS << D.Name << " Rel=" << D.Relations;
+}
+
+// The test cases in for targetDecl() take the form
+//  - a piece of code (Code = "...")
+//  - Code should have a single AST node marked as a [[range]]
+//  - an EXPECT_DECLS() assertion that verify the type of node selected, and
+//all the decls that targetDecl() considers it to reference
+// Despite the name, these cases actually test allTargetDecls() for brevity.
+class TargetDeclTest : public ::testing::Test {
+protected:
+  using Rel = DeclRelation;
+  std::string Code;
+  std::vector Flags;
+
+  // Asserts that `Code` has a marked selection of a node `NodeType`,
+  // and returns allTargetDecls() as PrintedDecl structs.
+  // Use via EXPECT_DECLS().
+  std::vector assertNodeAndPrintDecls(const char *NodeType) {
+Annotations A(Code);
+auto TU = TestTU::withCode(A.code());
+TU.ExtraArgs = Flags;
+auto AST = TU.build();
+EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
+llvm::Annotations::Range R = A.llvm::Annotations::range();
+SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,
+R.End);
+const SelectionTree::Node *N = Selection.commonAncestor();
+if (!N) {
+  ADD_FAILURE() << "No node selected!\n" << Code;
+  return {};
+}
+EXPECT_EQ(N->kind(), NodeType) << Selection;
+
+std::vector ActualDecls;
+for (const auto &Entry : allTargetDecls(N->ASTNode))
+  ActualDecls.emplace_back(Entry.first, Entry.second);
+return ActualDecls;
+  }
+};
+
+// This is a macro to preserve line numbers in assertion failures.
+// It takes the expected decls as varargs to work around comma-in-macro issues.
+#define EXPECT_DECLS(NodeType, ...)\
+  EXPECT_THAT(assertNodeAndPrintDecls(NodeType),   \
+  ::testing::UnorderedElementsAreArray(\
+  std::vector({__VA_ARGS__})))\
+  << Code
+using ExpectedDecls = std::vector;
+
+TEST_F(TargetDeclTest, Exprs) {
+  Code = R"cpp(
+int f();
+int x = [[f]]();
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "int f()");
+
+  Code = R"cpp(
+struct S { S operator+(S) const; };
+auto X = S() [[+]] S();
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "S operator+(S) const");
+}
+
+TEST_F(TargetDeclTes

[PATCH] D65209: [analyzer] Fix a SARIF exporter crash with macro expansions

[Joe Ranieri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3385c5cc4dfd: Fix a SARIF exporter crash with macro 
expansions (authored by jranieri-grammatech).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65209

Files:
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c

Index: clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -1,5 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify -analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.taint,debug.TaintTest,unix.Malloc %s -verify -analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
 #include "../Inputs/system-header-simulator.h"
+#include "../Inputs/system-header-simulator-for-malloc.h"
+#define ERR -1
 
 int atoi(const char *nptr);
 
@@ -20,10 +22,19 @@
   return 0;
 }
 
+int leak(int i) {
+  void *mem = malloc(8);
+  if (i < 4)
+return ERR; // expected-warning {{Potential leak of memory pointed to by 'mem'}}
+  free(mem);
+  return 0;
+}
+
 int main(void) {
   f();
   g();
   h(0);
+  leak(0);
   return 0;
 }
 
Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -6,7 +6,7 @@
 {
   "fileLocation": {
   },
-  "length": 686,
+  "length": 951,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
@@ -43,6 +43,16 @@
 "name": {
   "text": "core.DivideZero"
 }
+  },
+  {
+"fullDescription": {
+  "text": "Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free()."
+},
+"helpUri": "https://clang-analyzer.llvm.org/available_checks.html#unix.Malloc";,
+"id": "unix.Malloc",
+"name": {
+  "text": "unix.Malloc"
+}
   }
 ]
   },
@@ -65,9 +75,9 @@
   },
   "region": {
 "endColumn": 6,
-"endLine": 24,
+"endLine": 34,
 "startColumn": 3,
-"startLine": 24
+"startLine": 34
   }
 }
   }
@@ -84,9 +94,9 @@
   },
   "region": {
 "endColumn": 18,
-"endLine": 9,
+"endLine": 11,
 "startColumn": 11,
-"startLine": 9
+"startLine": 11
   }
 }
   }
@@ -104,9 +114,9 @@
 },
 "region": {
   "endColumn": 18,
-  "endLine": 9,
+  "endLine": 11,
   "startColumn": 11,
-  "startLine": 9
+  "startLine": 11
 }
   }
 }
@@ -135,9 +145,9 @@
   },
   "region": {
 "endColumn": 6,
-"endLine": 25,
+"endLine": 35,
 "startColumn": 3,
-"startLine": 25
+"startLine": 35
   }
 }
   }
@@ -154,9 +164,9 @@
   },
   "region": {
 "endColumn": 11,
-"endLine": 13,
+"endLine": 15,
 "startColumn": 3,
-"startLine": 13
+"startLine": 15
   }
 }
   }
@@ -173,9 +183,9 @@
   },
 

[PATCH] D66743: [clangd] Cleans up the semantic highlighting resources if clangd crashes.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 217396.
jvikstrom added a comment.

Dispose of all resources when disposing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66743

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
  
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

Index: clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
@@ -107,7 +107,8 @@
 highlighter.highlight('file1', []);
 assert.deepEqual(highlighter.applicationUriHistory, [ 'file1' ]);
 highlighter.initialize(tm);
-assert.deepEqual(highlighter.applicationUriHistory, [ 'file1', 'file1', 'file2' ]);
+assert.deepEqual(highlighter.applicationUriHistory,
+ [ 'file1', 'file1', 'file2' ]);
 // Groups decorations into the scopes used.
 let highlightingsInLine: semanticHighlighting.SemanticHighlightingLine[] = [
   {
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -56,6 +56,8 @@
   scopeLookupTable: string[][];
   // The object that applies the highlightings clangd sends.
   highlighter: Highlighter;
+  // Any disposables that should be cleaned up when clangd crashes.
+  private subscriptions: vscode.Disposable[] = [];
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
@@ -88,14 +90,15 @@
 // otherwise it could try to update the themeRuleMatcher without the
 // highlighter being created.
 this.highlighter = new Highlighter(this.scopeLookupTable);
+this.subscriptions.push(vscode.Disposable.from(this.highlighter));
 this.loadCurrentTheme();
 // Event handling for handling with TextDocuments/Editors lifetimes.
-vscode.window.onDidChangeVisibleTextEditors(
+this.subscriptions.push(vscode.window.onDidChangeVisibleTextEditors(
 (editors: vscode.TextEditor[]) =>
 editors.forEach((e) => this.highlighter.applyHighlights(
-e.document.uri.toString(;
-vscode.workspace.onDidCloseTextDocument(
-(doc) => this.highlighter.removeFileHighlightings(doc.uri.toString()));
+e.document.uri.toString();
+this.subscriptions.push(vscode.workspace.onDidCloseTextDocument(
+(doc) => this.highlighter.removeFileHighlightings(doc.uri.toString(;
   }
 
   handleNotification(params: SemanticHighlightingParams) {
@@ -103,6 +106,11 @@
 (line) => ({line : line.line, tokens : decodeTokens(line.tokens)}));
 this.highlighter.highlight(params.textDocument.uri, lines);
   }
+  // Disposes of all disposable resources used by this object.
+  public dispose() {
+this.subscriptions.forEach((d) => d.dispose());
+this.subscriptions = [];
+  }
 }
 
 // Converts a string of base64 encoded tokens into the corresponding array of
@@ -138,6 +146,13 @@
   constructor(scopeLookupTable: string[][]) {
 this.scopeLookupTable = scopeLookupTable;
   }
+  public dispose() {
+this.files.clear();
+this.decorationTypes.forEach((t) => t.dispose());
+// Dispose must not be not called multiple times if initialize is
+// called again.
+this.decorationTypes = [];
+  }
   // This function must be called at least once or no highlightings will be
   // done. Sets the theme that is used when highlighting. Also triggers a
   // recolorization for all current highlighters. Should be called whenever the
@@ -174,6 +189,27 @@
 this.applyHighlights(fileUri);
   }
 
+  // Applies all the highlightings currently stored for a file with fileUri.
+  public applyHighlights(fileUri: string) {
+if (!this.files.has(fileUri))
+  // There are no highlightings for this file, must return early or will get
+  // out of bounds when applying the decorations below.
+  return;
+if (!this.decorationTypes.length)
+  // Can't apply any decorations when there is no theme loaded.
+  return;
+// This must always do a full re-highlighting due to the fact that
+// TextEditorDecorationType are very expensive to create (which makes
+// incremental updates infeasible). For this reason one
+// TextEditorDecorationType is used per scope.
+const ranges = this.getDecorationRanges(fileUri);
+vscod

Re: r363820 - Add a script to help generate expected test output for dumping the AST to JSON.

[Aaron Ballman via cfe-commits]

On Mon, Aug 26, 2019 at 7:53 PM Richard Smith  wrote:
>
> Hi Aaron,
>
> I tried using this script to update a json dump test, and it seems to not 
> really work very well:
>
> 1) it's really inconvenient to invoke; you need to pass in a --clang, copy 
> some options out from two different places in the test file (from the RUN 
> line and from a "--filters" line), and guess how to form the proper command 
> line for the tool
> 2) it generates a file with the wrong name (adding a -json before the 
> extension)
> 3) it doesn't strip out the CHECK lines from the previous run of the tool
> 4) it adds in a bunch of trailing whitespace, causing the file to not match 
> the one in the repository
>
> Have you had a chance to look at making this more usable? I think the 
> approach taken by utils/make-ast-dump-check.sh helps a lot: run the test in 
> its normal configuration to generate the output, and then replace FileCheck 
> with a tool that updates the CHECK lines instead of checking them. (That 
> saves you needing to pass in --clang and --opts, at least, and makes it 
> really easy to update a whole bunch of tests at once by running them all with 
> lit.)

Unfortunately, I've not had the chance to look into improving the
script and the original author of the script has since moved on to
other opportunities as well. I'll see if I can spend some time
improving it, but shell scripting is not my strong suit (I'm
predominately on Windows, so the Python script approach is a bit
easier for me to grok). Does this have to use a shell script, or is
Python still fine?

~Aaron

>
> On Wed, 19 Jun 2019 at 08:21, Aaron Ballman via cfe-commits 
>  wrote:
>>
>> Author: aaronballman
>> Date: Wed Jun 19 08:25:24 2019
>> New Revision: 363820
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=363820&view=rev
>> Log:
>> Add a script to help generate expected test output for dumping the AST to 
>> JSON.
>>
>> Patch by Abhishek Bhaskar.
>>
>> Added:
>> cfe/trunk/test/AST/gen_ast_dump_json_test.py
>>
>> Added: cfe/trunk/test/AST/gen_ast_dump_json_test.py
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/gen_ast_dump_json_test.py?rev=363820&view=auto
>> ==
>> --- cfe/trunk/test/AST/gen_ast_dump_json_test.py (added)
>> +++ cfe/trunk/test/AST/gen_ast_dump_json_test.py Wed Jun 19 08:25:24 2019
>> @@ -0,0 +1,137 @@
>> +#!/usr/bin/env python
>> +
>> +from collections import OrderedDict
>> +from sets import Set
>> +from shutil import copyfile
>> +import argparse
>> +import json
>> +import os
>> +import pprint
>> +import re
>> +import subprocess
>> +
>> +def normalize(dict_var):
>> +for k, v in dict_var.items():
>> +if isinstance(v, OrderedDict):
>> +normalize(v)
>> +elif isinstance(v, list):
>> +for e in v:
>> +if isinstance(e, OrderedDict):
>> +normalize(e)
>> +elif type(v) is unicode:
>> +st = v.encode('utf-8')
>> +if re.match(r"0x[0-9A-Fa-f]+", v):
>> +dict_var[k] = u'0x{{.*}}'
>> +elif os.path.isfile(v):
>> +dict_var[k] = u'{{.*}}'
>> +else:
>> +splits = (v.split(u' '))
>> +out_splits = []
>> +for split in splits:
>> +inner_splits = split.rsplit(u':',2)
>> +if os.path.isfile(inner_splits[0]):
>> +out_splits.append(
>> +u'{{.*}}:%s:%s'
>> +%(inner_splits[1],
>> +  inner_splits[2]))
>> +continue
>> +out_splits.append(split)
>> +
>> +dict_var[k] = ' '.join(out_splits)
>> +
>> +def filter_json(dict_var, filters, out):
>> +for k, v in dict_var.items():
>> +if type(v) is unicode:
>> +st = v.encode('utf-8')
>> +if st in filters:
>> +out.append(dict_var)
>> +break
>> +elif isinstance(v, OrderedDict):
>> +filter_json(v, filters, out)
>> +elif isinstance(v, list):
>> +for e in v:
>> +if isinstance(e, OrderedDict):
>> +filter_json(e, filters, out)
>> +
>> +def main():
>> +parser = argparse.ArgumentParser()
>> +parser.add_argument("--clang", help="The clang binary (could be a 
>> relative or absolute path)",
>> +action="store", required=True)
>> +parser.add_argument("--opts", help="other options",
>> +action="store", default='', type=str)
>> +parser.add_argument("--source", help="the source file. Command used to 
>> generate the json will be of the format  -cc1 -ast-dump=json  
>> ",
>> +action="store", required=True)
>> +parser.add_argument("--filters", help="comma separated list of AST 
>> filters. Ex: -

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I don't see a test for the __cxx_global_array_dtor case?


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

https://reviews.llvm.org/D66328



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

r370066 - Replace some custom C11 extension warnings with the generic warning.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Aug 27 07:41:39 2019
New Revision: 370066

URL: http://llvm.org/viewvc/llvm-project?rev=370066&view=rev
Log:
Replace some custom C11 extension warnings with the generic warning.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/test/Parser/c11-noreturn.c
cfe/trunk/test/Sema/generic-selection.c
cfe/trunk/test/Sema/static-assert.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=370066&r1=370065&r2=370066&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Aug 27 07:41:39 
2019
@@ -119,8 +119,6 @@ def warn_microsoft_qualifiers_ignored :
   "qualifiers after comma in declarator list are ignored">,
   InGroup;
 
-def ext_c11_generic_selection : Extension<
-  "generic selections are a C11-specific feature">, InGroup;
 def err_duplicate_default_assoc : Error<
   "duplicate default generic association">;
 def note_previous_default_assoc : Note<
@@ -129,8 +127,6 @@ def note_previous_default_assoc : Note<
 def ext_c11_feature : Extension<
   "'%0' is a C11 extension">, InGroup;
 
-def ext_c11_noreturn : Extension<
-  "_Noreturn functions are a C11-specific feature">, InGroup;
 def err_c11_noreturn_misplaced : Error<
   "'_Noreturn' keyword must precede function declarator">;
 
@@ -374,8 +370,6 @@ def err_unexpected_token_in_nested_name_
   "'%0' cannot be a part of nested name specifier; did you mean ':'?">;
 def err_bool_redeclaration : Error<
   "redeclaration of C++ built-in type 'bool'">;
-def ext_c11_static_assert : Extension<
-  "_Static_assert is a C11-specific feature">, InGroup;
 def warn_cxx98_compat_static_assert : Warning<
   "static_assert declarations are incompatible with C++98">,
   InGroup, DefaultIgnore;

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=370066&r1=370065&r2=370066&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Aug 27 07:41:39 2019
@@ -3626,7 +3626,7 @@ void Parser::ParseDeclarationSpecifiers(
 }
 case tok::kw__Noreturn:
   if (!getLangOpts().C11)
-Diag(Loc, diag::ext_c11_noreturn);
+Diag(Tok, diag::ext_c11_feature) << Tok.getName();
   isInvalid = DS.setFunctionSpecNoreturn(Loc, PrevSpec, DiagID);
   break;
 

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=370066&r1=370065&r2=370066&view=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Tue Aug 27 07:41:39 2019
@@ -862,7 +862,7 @@ Decl *Parser::ParseStaticAssertDeclarati
  "Not a static_assert declaration");
 
   if (Tok.is(tok::kw__Static_assert) && !getLangOpts().C11)
-Diag(Tok, diag::ext_c11_static_assert);
+Diag(Tok, diag::ext_c11_feature) << Tok.getName();
   if (Tok.is(tok::kw_static_assert))
 Diag(Tok, diag::warn_cxx98_compat_static_assert);
 

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=370066&r1=370065&r2=370066&view=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Tue Aug 27 07:41:39 2019
@@ -2733,11 +2733,10 @@ ExprResult Parser::ParseStringLiteralExp
 /// \endverbatim
 ExprResult Parser::ParseGenericSelectionExpression() {
   assert(Tok.is(tok::kw__Generic) && "_Generic keyword expected");
-  SourceLocation KeyLoc = ConsumeToken();
-
   if (!getLangOpts().C11)
-Diag(KeyLoc, diag::ext_c11_generic_selection);
+Diag(Tok, diag::ext_c11_feature) << Tok.getName();
 
+  SourceLocation KeyLoc = ConsumeToken();
   BalancedDelimiterTracker T(*this, tok::l_paren);
   if (T.expectAndConsume())
 return ExprError();

Modified: cfe/trunk/test/Parser/c11-noreturn.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/c11-noreturn.c?rev=370066&r1=370065&r2=370066&view=diff
==
--- cfe/trunk/test/Parser/c11-noreturn.c (original)
+++ cfe/trunk/test/Parser/c11-noreturn.c Tue Aug 27 07:41:39 2019
@@ -15,4 +15,4 @@ _Noreturn int; // expected-error {{'_Nor
 _Noreturn struct S; // expected-error {{'_Noreturn' can only appear on 
functions}}
 _Noreturn enum E { e }; // expected-error {{'_Noreturn' can only appear on 
func

r370068 - Update the SARIF exporter to SARIF 2.1

[Joe Ranieri via cfe-commits]

Author: jranieri
Date: Tue Aug 27 07:43:54 2019
New Revision: 370068

URL: http://llvm.org/viewvc/llvm-project?rev=370068&view=rev
Log:
Update the SARIF exporter to SARIF 2.1

This updates the SARIF exporter to produce SARIF 2.1 output. The bulk of the 
diffs come from two changes to SARIF:
* https://github.com/oasis-tcs/sarif-spec/issues/309
* https://github.com/oasis-tcs/sarif-spec/issues/179

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
cfe/trunk/test/Analysis/lit.local.cfg

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=370068&r1=370067&r2=370068&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Aug 27 07:43:54 
2019
@@ -106,26 +106,26 @@ static std::string fileNameToURI(StringR
   return Ret.str().str();
 }
 
-static json::Object createFileLocation(const FileEntry &FE) {
+static json::Object createArtifactLocation(const FileEntry &FE) {
   return json::Object{{"uri", fileNameToURI(getFileName(FE))}};
 }
 
-static json::Object createFile(const FileEntry &FE) {
-  return json::Object{{"fileLocation", createFileLocation(FE)},
+static json::Object createArtifact(const FileEntry &FE) {
+  return json::Object{{"location", createArtifactLocation(FE)},
   {"roles", json::Array{"resultFile"}},
   {"length", FE.getSize()},
   {"mimeType", "text/plain"}};
 }
 
-static json::Object createFileLocation(const FileEntry &FE,
-   json::Array &Files) {
+static json::Object createArtifactLocation(const FileEntry &FE,
+   json::Array &Artifacts) {
   std::string FileURI = fileNameToURI(getFileName(FE));
 
-  // See if the Files array contains this URI already. If it does not, create
-  // a new file object to add to the array.
-  auto I = llvm::find_if(Files, [&](const json::Value &File) {
+  // See if the Artifacts array contains this URI already. If it does not,
+  // create a new artifact object to add to the array.
+  auto I = llvm::find_if(Artifacts, [&](const json::Value &File) {
 if (const json::Object *Obj = File.getAsObject()) {
-  if (const json::Object *FileLoc = Obj->getObject("fileLocation")) {
+  if (const json::Object *FileLoc = Obj->getObject("location")) {
 Optional URI = FileLoc->getString("uri");
 return URI && URI->equals(FileURI);
   }
@@ -133,13 +133,13 @@ static json::Object createFileLocation(c
 return false;
   });
 
-  // Calculate the index within the file location array so it can be stored in
+  // Calculate the index within the artifact array so it can be stored in
   // the JSON object.
-  auto Index = static_cast(std::distance(Files.begin(), I));
-  if (I == Files.end())
-Files.push_back(createFile(FE));
+  auto Index = static_cast(std::distance(Artifacts.begin(), I));
+  if (I == Artifacts.end())
+Artifacts.push_back(createArtifact(FE));
 
-  return json::Object{{"uri", FileURI}, {"fileIndex", Index}};
+  return json::Object{{"uri", FileURI}, {"index", Index}};
 }
 
 static json::Object createTextRegion(SourceRange R, const SourceManager &SM) {
@@ -158,9 +158,10 @@ static json::Object createTextRegion(Sou
 
 static json::Object createPhysicalLocation(SourceRange R, const FileEntry &FE,
const SourceManager &SMgr,
-   json::Array &Files) {
-  return json::Object{{{"fileLocation", createFileLocation(FE, Files)},
-   {"region", createTextRegion(R, SMgr)}}};
+   json::Array &Artifacts) {
+  return json::Object{
+  {{"artifactLocation", createArtifactLocation(FE, Artifacts)},
+   {"region", createTextRegion(R, SMgr)}}};
 }
 
 enum class Importance { Important, Essential, Unimportant };
@@ -213,7 +214,7 @@ static Importance calculateImportance(co
 }
 
 static json::Object createThreadFlow(const PathPieces &Pieces,
- json::Array &Files) {
+ json::Array &Artifacts) {
   const SourceManager &SMgr = Pieces.front()->getLocation().getManager();
   json::Array Locations;
   for (const auto &Piece : Pieces) {
@@ -222,7 +223,7 @@ static json::Object createThreadFlow(con
 createLocation(createPhysicalLocation(
P.asRange(),
*P.asLocation().getExpansionLoc().getFileEntry(),
-   

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D66328#1647062 , @probinson wrote:

> I don't see a test for the __cxx_global_array_dtor case?


It is actually the `arraydestroy.*` loop that is covered (generated by 
`CodeGenFunction::emitArrayDestroy`), please see 
`debug-info-destroy-helper.cpp` below.


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

https://reviews.llvm.org/D66328



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

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

[Tom Weaver via Phabricator via cfe-commits]

TWeaver added a comment.

Hi there!

thanks so much for this patch, can confirm that with this patch applied my 
local machine no longer crashes for the loop-widening.cpp test.

LGTM for me, but this isn't really my area of expertise so I can't judge one 
whether these changes are the correct ones to make.

Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66716



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

[PATCH] D65211: [analyzer] Update the SARIF exporter to SARIF 2.1

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370068: Update the SARIF exporter to SARIF 2.1 (authored by 
jranieri, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65211?vs=211496&id=217408#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65211

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  cfe/trunk/test/Analysis/lit.local.cfg

Index: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -106,26 +106,26 @@
   return Ret.str().str();
 }
 
-static json::Object createFileLocation(const FileEntry &FE) {
+static json::Object createArtifactLocation(const FileEntry &FE) {
   return json::Object{{"uri", fileNameToURI(getFileName(FE))}};
 }
 
-static json::Object createFile(const FileEntry &FE) {
-  return json::Object{{"fileLocation", createFileLocation(FE)},
+static json::Object createArtifact(const FileEntry &FE) {
+  return json::Object{{"location", createArtifactLocation(FE)},
   {"roles", json::Array{"resultFile"}},
   {"length", FE.getSize()},
   {"mimeType", "text/plain"}};
 }
 
-static json::Object createFileLocation(const FileEntry &FE,
-   json::Array &Files) {
+static json::Object createArtifactLocation(const FileEntry &FE,
+   json::Array &Artifacts) {
   std::string FileURI = fileNameToURI(getFileName(FE));
 
-  // See if the Files array contains this URI already. If it does not, create
-  // a new file object to add to the array.
-  auto I = llvm::find_if(Files, [&](const json::Value &File) {
+  // See if the Artifacts array contains this URI already. If it does not,
+  // create a new artifact object to add to the array.
+  auto I = llvm::find_if(Artifacts, [&](const json::Value &File) {
 if (const json::Object *Obj = File.getAsObject()) {
-  if (const json::Object *FileLoc = Obj->getObject("fileLocation")) {
+  if (const json::Object *FileLoc = Obj->getObject("location")) {
 Optional URI = FileLoc->getString("uri");
 return URI && URI->equals(FileURI);
   }
@@ -133,13 +133,13 @@
 return false;
   });
 
-  // Calculate the index within the file location array so it can be stored in
+  // Calculate the index within the artifact array so it can be stored in
   // the JSON object.
-  auto Index = static_cast(std::distance(Files.begin(), I));
-  if (I == Files.end())
-Files.push_back(createFile(FE));
+  auto Index = static_cast(std::distance(Artifacts.begin(), I));
+  if (I == Artifacts.end())
+Artifacts.push_back(createArtifact(FE));
 
-  return json::Object{{"uri", FileURI}, {"fileIndex", Index}};
+  return json::Object{{"uri", FileURI}, {"index", Index}};
 }
 
 static json::Object createTextRegion(SourceRange R, const SourceManager &SM) {
@@ -158,9 +158,10 @@
 
 static json::Object createPhysicalLocation(SourceRange R, const FileEntry &FE,
const SourceManager &SMgr,
-   json::Array &Files) {
-  return json::Object{{{"fileLocation", createFileLocation(FE, Files)},
-   {"region", createTextRegion(R, SMgr)}}};
+   json::Array &Artifacts) {
+  return json::Object{
+  {{"artifactLocation", createArtifactLocation(FE, Artifacts)},
+   {"region", createTextRegion(R, SMgr)}}};
 }
 
 enum class Importance { Important, Essential, Unimportant };
@@ -213,7 +214,7 @@
 }
 
 static json::Object createThreadFlow(const PathPieces &Pieces,
- json::Array &Files) {
+ json::Array &Artifacts) {
   const SourceManager &SMgr = Pieces.front()->getLocation().getManager();
   json::Array Locations;
   for (const auto &Piece : Pieces) {
@@ -222,7 +223,7 @@
 createLocation(createPhysicalLocation(
P.asRange(),
*P.asLocation().getExpansionLoc().getFileEntry(),
-   SMgr, Files),
+   SMgr, Artifacts),
Piece->getString()),
 calculateImportance(*Piece)));
   }
@@ -230,19 +231,13 @@
 }
 
 static json::Object createCodeFlow(const PathPieces &Pieces,
-   json::Array &Files) {
+   json::Array &Artifacts) {
   return json::Object{
-  

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D66328#1647094 , @aganea wrote:

> In D66328#1647062 , @probinson wrote:
>
> > I don't see a test for the __cxx_global_array_dtor case?
>
>
> It is actually the `arraydestroy.*` loop that is covered (generated by 
> `CodeGenFunction::emitArrayDestroy`), please see 
> `debug-info-destroy-helper.cpp` below.


I was expecting a check for the Artificial flag, though.


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

https://reviews.llvm.org/D66328



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

[PATCH] D66806: [LifetimeAnalysis] Fix some false positives

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: gribozavr, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs.

We should never track if a gsl::Pointer is created from an unannotated type.

For example the code below is definitely buggy:

  reference_wrapper f() {
int i;
return i; // Invalid!
  }

While the code below can be safe:

  some_iterator f() {
MyUnannotatedSpan local = ...;
return std::begin(local); // this is fine
  }

Note that, this problem will be solved once we have function annotations, as 
each constructor can be annotated what it actually does.

This patch also fixes a false positive from the use of address of operator.

All in all this patch reduces the number of warnings emitted but all it fixes 
all the false positives we know so far. The added tests with the TODO will be 
useful once we start to add function annotations and great for documenting the 
currently known false negatives.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66806

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -7,13 +7,21 @@
 struct [[gsl::Pointer(int)]] MyIntPointer {
   MyIntPointer(int *p = nullptr);
   // Conversion operator and constructor conversion will result in two
-  // different ASTs. The former is tested with another owner and 
+  // different ASTs. The former is tested with another owner and
   // pointer type.
   MyIntPointer(const MyIntOwner &);
   int &operator*();
   MyIntOwner toOwner();
 };
 
+struct MySpecialIntPointer : MyIntPointer {
+};
+
+// We did see examples in the wild when a derived class changes
+// the ownership model. So we have a test for it.
+struct [[gsl::Owner(int)]] MyOwnerIntPointer : MyIntPointer {
+};
+
 struct [[gsl::Pointer(long)]] MyLongPointerFromConversion {
   MyLongPointerFromConversion(long *p = nullptr);
   long &operator*();
@@ -56,21 +64,21 @@
 };
 
 void dangligGslPtrFromTemporary() {
-  MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+  MyIntPointer p = Y{}.a; // TODO
   (void)p;
 }
 
 struct DanglingGslPtrField {
-  MyIntPointer p; // expected-note 2{{pointer member declared here}}
+  MyIntPointer p; // expected-note {{pointer member declared here}}
   MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}}
-  DanglingGslPtrField(int i) : p(&i) {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}}
+  DanglingGslPtrField(int i) : p(&i) {} // TODO
   DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
   DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
 };
 
 MyIntPointer danglingGslPtrFromLocal() {
   int j;
-  return &j; // expected-warning {{address of stack memory associated with local variable 'j' returned}}
+  return &j; // TODO
 }
 
 MyIntPointer returningLocalPointer() {
@@ -124,6 +132,7 @@
 struct basic_iterator {
   basic_iterator operator++();
   T& operator*() const;
+  T* operator->() const;
 };
 
 template
@@ -141,6 +150,12 @@
 template 
 auto data(const C &c) -> decltype(c.data());
 
+template 
+auto begin(C &c) -> decltype(c.begin());
+
+template
+T *begin(T (&array)[N]);
+
 template 
 struct vector {
   typedef __gnu_cxx::basic_iterator iterator;
@@ -158,6 +173,8 @@
 
 template
 struct basic_string {
+  basic_string();
+  basic_string(const T *);
   const T *c_str() const;
   operator basic_string_view () const;
 };
@@ -188,8 +205,23 @@
 
 template
 T any_cast(const any& operand);
+
+template
+struct reference_wrapper {
+  template
+  reference_wrapper(U &&);
+};
+
+template
+reference_wrapper ref(T& t) noexcept;
 }
 
+struct Unannotated {
+  typedef std::vector::iterator iterator;
+  iterator begin();
+  operator iterator() const;
+};
+
 void modelIterators() {
   std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   (void)it;
@@ -298,3 +330,123 @@
   const std::vector &v = getVec();
   const int *val = v.data(); // Ok, it is lifetime extended.
 }
+
+void handleTernaryOperator(bool cond) {
+std::basic_string def;
+std::basic_string_view v = cond ? def : ""; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 217413.
lenary added a comment.

Address review feedback:

- Add Test for MaxAtomicPromoteWidth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57450

Files:
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/riscv-atomics.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -105,6 +105,10 @@
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
+typedef __WINT_TYPE__ wint_t;
+
+
+// Check Alignments
 
 // CHECK: @align_c = dso_local global i32 1
 int align_c = __alignof(char);
@@ -118,6 +122,9 @@
 // CHECK: @align_wc = dso_local global i32 4
 int align_wc = __alignof(wchar_t);
 
+// CHECK: @align_wi = dso_local global i32 4
+int align_wi = __alignof(wint_t);
+
 // CHECK: @align_l = dso_local global i32 8
 int align_l = __alignof(long);
 
@@ -139,6 +146,88 @@
 // CHECK: @align_vl = dso_local global i32 8
 int align_vl = __alignof(va_list);
 
+// CHECK: @align_a_c = dso_local global i32 1
+int align_a_c = __alignof(_Atomic(char));
+
+// CHECK: @align_a_s = dso_local global i32 2
+int align_a_s = __alignof(_Atomic(short));
+
+// CHECK: @align_a_i = dso_local global i32 4
+int align_a_i = __alignof(_Atomic(int));
+
+// CHECK: @align_a_wc = dso_local global i32 4
+int align_a_wc = __alignof(_Atomic(wchar_t));
+
+// CHECK: @align_a_wi = dso_local global i32 4
+int align_a_wi = __alignof(_Atomic(wint_t));
+
+// CHECK: @align_a_l = dso_local global i32 8
+int align_a_l = __alignof(_Atomic(long));
+
+// CHECK: @align_a_ll = dso_local global i32 8
+int align_a_ll = __alignof(_Atomic(long long));
+
+// CHECK: @align_a_p = dso_local global i32 8
+int align_a_p = __alignof(_Atomic(void*));
+
+// CHECK @align_a_f = dso_local global i32 4
+int align_a_f = __alignof(_Atomic(float));
+
+// CHECK: @align_a_d = dso_local global i32 8
+int align_a_d = __alignof(_Atomic(double));
+
+// CHECK: @align_a_ld = dso_local global i32 16
+int align_a_ld = __alignof(_Atomic(long double));
+
+// CHECK: @align_a_s4 = dso_local global i32 4
+int align_a_s4 = __alignof(_Atomic(struct { char _[4]; }));
+
+// CHECK: @align_a_s8 = dso_local global i32 8
+int align_a_s8 = __alignof(_Atomic(struct { char _[8]; }));
+
+// CHECK: @align_a_s16 = dso_local global i32 16
+int align_a_s16 = __alignof(_Atomic(struct { char _[16]; }));
+
+// CHECK: @align_a_s32 = dso_local global i32 1
+int align_a_s32 = __alignof(_Atomic(struct { char _[32]; }));
+
+
+// Check Sizes
+
+// CHECK: @size_a_c = dso_local global i32 1
+int size_a_c = sizeof(_Atomic(char));
+
+// CHECK: @size_a_s = dso_local global i32 2
+int size_a_s = sizeof(_Atomic(short));
+
+// CHECK: @size_a_i = dso_local global i32 4
+int size_a_i = sizeof(_Atomic(int));
+
+// CHECK: @size_a_wc = dso_local global i32 4
+int size_a_wc = sizeof(_Atomic(wchar_t));
+
+// CHECK: @size_a_wi = dso_local global i32 4
+int size_a_wi = sizeof(_Atomic(wint_t));
+
+// CHECK: @size_a_l = dso_local global i32 8
+int size_a_l = sizeof(_Atomic(long));
+
+// CHECK: @size_a_ll = dso_local global i32 8
+int size_a_ll = sizeof(_Atomic(long long));
+
+// CHECK: @size_a_p = dso_local global i32 8
+int size_a_p = sizeof(_Atomic(void*));
+
+// CHECK: @size_a_f = dso_local global i32 4
+int size_a_f = sizeof(_Atomic(float));
+
+// CHECK: @size_a_d = dso_local global i32 8
+int size_a_d = sizeof(_Atomic(double));
+
+// CHECK: @size_a_ld = dso_local global i32 16
+int size_a_ld = sizeof(_Atomic(long double));
+
+
 // Check types
 
 // CHECK: define dso_local zeroext i8 @check_char()
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -105,6 +105,10 @@
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
+typedef __WINT_TYPE__ wint_t;
+
+
+// Check Alignments
 
 // CHECK: @align_c = dso_local global i32 1
 int align_c = __alignof(char);
@@ -118,6 +122,9 @@
 // CHECK: @align_wc = dso_local global i32 4
 int align_wc = __alignof(wchar_t);
 
+// CHECK: @align_wi = dso_local global i32 4
+int align_wi = __alignof(wint_t);
+
 // CHECK: @align_l = dso_local global i32 4
 int align_l = __alignof(long);
 
@@ -139,6 +146,88 @@
 // CHECK: @align_vl = dso_local global i32 4
 int align_vl = __alignof(va_list);
 
+// CHECK: @align_a_c = dso_local global i32 1
+int align_a_c = __alignof(_Atomic(char));
+
+// CHECK: @align_a_s = dso_local global i32 2
+int align_a_s = __alignof(_Atomic(short));
+
+// CHECK: @align_a_i = dso_local global i32 4
+int align_a_i = __alignof(_Atomic(int));
+
+// CHECK: @align_a_wc = dso_local global i32 4
+int align_a_wc = __a

[PATCH] D66783: [clang-doc] Use llvm::createStringError and canonicalize error messages

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM -- thank you!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D66783



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

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb 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/D57450/new/

https://reviews.llvm.org/D57450



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

r370073 - [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via cfe-commits]

Author: lenary
Date: Tue Aug 27 08:41:16 2019
New Revision: 370073

URL: http://llvm.org/viewvc/llvm-project?rev=370073&view=rev
Log:
[RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 
targets with atomics

Summary: This ensures that libcalls aren't generated when the target supports 
atomics. Atomics aren't in the base RV32I/RV64I instruction sets, so 
MaxAtomicInlineWidth and MaxAtomicPromoteWidth are set only when the atomics 
extension is being targeted. This must be done in setMaxAtomicWidth, as this 
should be done after handleTargetFeatures has been called.

Reviewers: jfb, jyknight, wmi, asb

Reviewed By: asb

Subscribers: pzheng, MaskRay, s.egerton, lenary, dexonsmith, psnobl, benna, 
Jim, JohnLLVM, rbar, johnrusso, simoncook, apazos, sabuasal, niosHD, 
kito-cheng, shiva0217, jrtc27, zzheng, edward-jones, rogfer01, MartinMosbeck, 
brucehoult, the_o, rkruppe, PkmX, jocewei, lewis-revill, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGen/riscv-atomics.c
Modified:
cfe/trunk/lib/Basic/Targets/RISCV.h
cfe/trunk/test/Driver/riscv32-toolchain.c
cfe/trunk/test/Driver/riscv64-toolchain.c

Modified: cfe/trunk/lib/Basic/Targets/RISCV.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/RISCV.h?rev=370073&r1=370072&r2=370073&view=diff
==
--- cfe/trunk/lib/Basic/Targets/RISCV.h (original)
+++ cfe/trunk/lib/Basic/Targets/RISCV.h Tue Aug 27 08:41:16 2019
@@ -93,6 +93,13 @@ public:
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 32;
+  }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
 public:
@@ -110,6 +117,13 @@ public:
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 64;
+  }
 };
 } // namespace targets
 } // namespace clang

Added: cfe/trunk/test/CodeGen/riscv-atomics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/riscv-atomics.c?rev=370073&view=auto
==
--- cfe/trunk/test/CodeGen/riscv-atomics.c (added)
+++ cfe/trunk/test/CodeGen/riscv-atomics.c Tue Aug 27 08:41:16 2019
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -triple riscv32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32I
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32IA
+// RUN: %clang_cc1 -triple riscv64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64I
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64IA
+
+// This test demonstrates that MaxAtomicInlineWidth is set appropriately when
+// the atomics instruction set extension is enabled.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // RV32I:  call zeroext i8 @__atomic_load_1
+  // RV32I:  call void @__atomic_store_1
+  // RV32I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV32IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV32IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV32IA: atomicrmw add i8* %a, i8 %b seq_cst
+  // RV64I:  call zeroext i8 @__atomic_load_1
+  // RV64I:  call void @__atomic_store_1
+  // RV64I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV64IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV64IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV64IA: atomicrmw add i8* %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // RV32I:  call i32 @__atomic_load_4
+  // RV32I:  call void @__atomic_store_4
+  // RV32I:  call i32 @__atomic_fetch_add_4
+  // RV32IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV32IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV32IA: atomicrmw add i32* %a, i32 %b seq_cst
+  // RV64I:  call signext i32 @__atomic_load_4
+  // RV64I:  call void @__atomic_store_4
+  // RV64I:  call signext i32 @__atomic_fetch_add_4
+  // RV64IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV64IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV64IA: atomicrmw add i32* %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // RV32I:  call i64 @__atomic_load_8
+  // RV32I:  call void @__atomic_store_8
+  // RV32I:  call i64 @__atomic_fetch_add_8
+  // RV32IA: call i64 @__atomic_load_8
+  // RV32IA: call void @__atomic_store_8
+  // RV32I

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D62739



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

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Sam Elliott via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370073: [RISCV] Set MaxAtomicInlineWidth and 
MaxAtomicPromoteWidth for RV32/RV64… (authored by lenary, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57450?vs=217413&id=217418#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57450

Files:
  cfe/trunk/lib/Basic/Targets/RISCV.h
  cfe/trunk/test/CodeGen/riscv-atomics.c
  cfe/trunk/test/Driver/riscv32-toolchain.c
  cfe/trunk/test/Driver/riscv64-toolchain.c

Index: cfe/trunk/lib/Basic/Targets/RISCV.h
===
--- cfe/trunk/lib/Basic/Targets/RISCV.h
+++ cfe/trunk/lib/Basic/Targets/RISCV.h
@@ -93,6 +93,13 @@
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 32;
+  }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
 public:
@@ -110,6 +117,13 @@
 }
 return false;
   }
+
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = 128;
+
+if (HasA)
+  MaxAtomicInlineWidth = 64;
+  }
 };
 } // namespace targets
 } // namespace clang
Index: cfe/trunk/test/CodeGen/riscv-atomics.c
===
--- cfe/trunk/test/CodeGen/riscv-atomics.c
+++ cfe/trunk/test/CodeGen/riscv-atomics.c
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -triple riscv32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32I
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV32IA
+// RUN: %clang_cc1 -triple riscv64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64I
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=RV64IA
+
+// This test demonstrates that MaxAtomicInlineWidth is set appropriately when
+// the atomics instruction set extension is enabled.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // RV32I:  call zeroext i8 @__atomic_load_1
+  // RV32I:  call void @__atomic_store_1
+  // RV32I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV32IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV32IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV32IA: atomicrmw add i8* %a, i8 %b seq_cst
+  // RV64I:  call zeroext i8 @__atomic_load_1
+  // RV64I:  call void @__atomic_store_1
+  // RV64I:  call zeroext i8 @__atomic_fetch_add_1
+  // RV64IA: load atomic i8, i8* %a seq_cst, align 1
+  // RV64IA: store atomic i8 %b, i8* %a seq_cst, align 1
+  // RV64IA: atomicrmw add i8* %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // RV32I:  call i32 @__atomic_load_4
+  // RV32I:  call void @__atomic_store_4
+  // RV32I:  call i32 @__atomic_fetch_add_4
+  // RV32IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV32IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV32IA: atomicrmw add i32* %a, i32 %b seq_cst
+  // RV64I:  call signext i32 @__atomic_load_4
+  // RV64I:  call void @__atomic_store_4
+  // RV64I:  call signext i32 @__atomic_fetch_add_4
+  // RV64IA: load atomic i32, i32* %a seq_cst, align 4
+  // RV64IA: store atomic i32 %b, i32* %a seq_cst, align 4
+  // RV64IA: atomicrmw add i32* %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // RV32I:  call i64 @__atomic_load_8
+  // RV32I:  call void @__atomic_store_8
+  // RV32I:  call i64 @__atomic_fetch_add_8
+  // RV32IA: call i64 @__atomic_load_8
+  // RV32IA: call void @__atomic_store_8
+  // RV32IA: call i64 @__atomic_fetch_add_8
+  // RV64I:  call i64 @__atomic_load_8
+  // RV64I:  call void @__atomic_store_8
+  // RV64I:  call i64 @__atomic_fetch_add_8
+  // RV64IA: load atomic i64, i64* %a seq_cst, align 8
+  // RV64IA: store atomic i64 %b, i64* %a seq_cst, align 8
+  // RV64IA: atomicrmw add i64* %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
Index: cfe/trunk/test/Driver/riscv32-toolchain.c
===
--- cfe/trunk/test/Driver/riscv32-toolchain.c
+++ cfe/trunk/test/Driver/riscv32-toolchain.c
@@ -105,6 +105,10 @@
 typedef __SIZE_TYPE__ size_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
+typedef __WINT_TYPE__ wint_t;
+
+
+// Check Alignments
 

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

I'm having second thoughts about this -- I prefer the approach I ended up 
taking in https://reviews.llvm.org/D66676, which is subtly different.

However, getRuleMatchLoc() will be useful a different purpose: when only 
reporting a diagnostic, with no corresponding changes.  So, I plan to rework 
this into two revisions: one to match  https://reviews.llvm.org/D66676 (and 
keep the tests esssentially as they are) and one to add getRuleMatchLoc for 
future use.  I can also make both changes in the same revision, if you prefer.




Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:197
 
   // Verify the existence and validity of the AST node that roots this rule.
+  SourceLocation RootLoc = tooling::detail::getRuleMatchLoc(Result);

gribozavr wrote:
> This comment was moved into the function and now looks out of place here.
removed in both places. Even in the function, it didn't add anything meaningful 
to the assert() itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66652



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

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

[Florian Hahn via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D66290



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

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

Therefore, `vectorize(disable)` would also disable interleaving?


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

https://reviews.llvm.org/D66796



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