date:20191030

[PATCH] D69498: IR: Invert convergent attribute handling

2019-10-30 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added a comment.

In D69498#1725867 , @jdoerfert wrote:

> In D69498#1725819 , @mehdi_amini 
> wrote:
>
> > Maybe we can start by looking into the motivation for this patch:
> >
> > > There is a burden on frontends in environments that care about convergent 
> > > operations to add the attribute just in case it is needed. This has 
> > > proven to be somewhat problematic, and different frontend projects have 
> > > consistently run into problems related to this.
> >
> > Can you clarify what kind of problems? Do you have concrete examples?
>
>
> The frontend has to put convergent on everything that is not known to be 
> non-convergent right now. That is, the front-end and all function generating 
> constructs need to know about convergent to preserve correctness.

Right, but that does not seem like a problem to me so far: a GPU frontend must 
add a gpu-specific attribute on every function seems like a reasonable 
constraint to me.

> Frontends in the past put convergent only on barriers or similar calls but 
> not on all functions that might transitively call barriers, leading to subtle 
> bugs.

Right, but it seems like trying to fix bugs in the frontend at the LLVM does 
not seems necessarily right: it looks like a big hammer.

In D69498#1725877 , @arsenm wrote:

> Basically no frontend has gotten this right, including clang and non-clang 
> frontends. There's a gradual discovery process that it's necessary in the 
> first place, and then a long tail of contexts where it's discovered it's 
> missing. As I mentioned before, SYCL and OpenMP are both still broken in this 
> regard. For example in clang, convergent wasn't initially added to all 
> functions, and then it was discovered that transforms on indirect callers 
> violated it.

The rule seems fairly straightforward for the frontend: the attributes must be 
always added everywhere if you're using SIMT.
Originally it likely wasn't thought through for convergent, but at this point 
this seems behind us.

> Then later it was discovered this was broken for inline asm.

Can you clarify what you mean here and how this change would fix it?

> Then some specific intrinsic call sites were a problem.

Same, I'm not sure I understand this one.

> Then there are special cases that don't necessarily go through the standard 
> user function codegen paths which don't get the default attributes.

> Some odd places where IR is compiled into the user library that didn't have 
> convergent added.

Bug in the user library? Again I'm not sure why it justifies a change in LLVM.

> The short version is the fact that most developers aren't aware of and don't 
> understand the subtleties of convergence, and making sure the front-end does 
> something in all contexts requires a lot of diligence. It's very easy to 
> introduce these difficult to debug bugs when calls are broken by default.

The fact that most developers aren't aware of convergence means a lot of 
potential bugs in LLVM because passes and transformations won't honor the 
convergent semantics, but the default for the attribute won't change that.
As of the frontend, it seems to me that this is just about structuring the 
code-path for function emission to define the right set of default attribute. 
It isn't clear to me why a refactoring of the frontend isn't a better course of 
actions here.

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

https://reviews.llvm.org/D69498

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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

- Add comments
- Early return when generating edits fails


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -40,9 +40,9 @@
 #include 
 
 using ::testing::AllOf;
+using ::testing::ElementsAre;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
-using ::testing::ElementsAre;
 
 namespace clang {
 namespace clangd {
@@ -1377,7 +1377,7 @@
   // Template body is not parsed until instantiation time on windows, which
   // results in arbitrary failures as function body becomes NULL.
   ExtraArgs.push_back("-fno-delayed-template-parsing");
-  for(const auto &Case : Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1420,7 +1420,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test =R"cpp(
+  auto Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -1489,6 +1489,71 @@
   EXPECT_EQ(apply(Test), Expected);
 }
 
+TEST_F(DefineInlineTest, TransformParamNames) {
+  EXPECT_EQ(apply(R"cpp(
+void foo(int, bool b, int T\
+est);
+
+void ^foo(int f, bool x, int z) {})cpp"),
+R"cpp(
+void foo(int f, bool x, int z){}
+
+)cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+#define PARAM int Z
+void foo(PARAM);
+
+void ^foo(int X) {})cpp"),
+"fail: Cant rename parameter inside macro body.");
+
+  EXPECT_EQ(apply(R"cpp(
+#define TYPE int
+#define PARAM TYPE Z
+#define BODY(x) 5 * (x) + 2
+template 
+void foo(PARAM, TYPE Q, TYPE, TYPE W = BODY(P));
+
+template 
+void ^foo(int Z, int b, int c, int d) {})cpp"),
+R"cpp(
+#define TYPE int
+#define PARAM TYPE Z
+#define BODY(x) 5 * (x) + 2
+template 
+void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){}
+
+)cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+  EXPECT_EQ(apply(R"cpp(
+struct Foo {
+  struct Bar {
+template  class, template class Y,
+  int, int Z>
+void foo(X, Y, int W = 5 * Z + 2);
+  };
+};
+
+template  class V, template class W,
+  int X, int Y>
+void Foo::Bar::f^oo(U, W, int Q) {})cpp"),
+R"cpp(
+struct Foo {
+  struct Bar {
+template  class V, template class W,
+  int X, int Y>
+void foo(U, W, int Q = 5 * Y + 2){}
+  };
+};
+
+)cpp");
+}
+
 TEST_F(DefineInlineTest, TransformInlineNamespaces) {
   auto Test = R"cpp(
 namespace a { inline namespace b { namespace { struct Foo{}; } } }
@@ -1527,7 +1592,7 @@
   void fo^o() { return ; })cpp",
"fail: Couldn't find semicolon for target declaration."},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1585,7 +1650,7 @@
   void TARGET(){ return; }
   )cpp"},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,108 @@
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Generates Replacements for changing template and function parameter names in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  tooling::Replacements Replacements;
+
+  llvm::DenseMap ParamToNewName;
+  llvm::DenseMap> RefLocs;
+  auto HandleParam = [&](const NamedDecl *DestParam,
+ const NamedDecl *SourceParam) {
+// No need to rename if parameters already have the same name.
+if (DestParam->getName() == SourceParam->getName())
+  return;
+std::string NewName;
+// Unnamed parameters won't be visited in findExplicitReferences. So add
+// them here.
+if (DestParam->getName().empty()) {
+  RefLocs[DestParam].push_back(DestParam->getLocation());
+  // If decl is unnamed in destination we pad the new name to avoid gluing
+  // with previous token, e.g. foo(int^) shouldn't turn into foo(intx).
+  NewName = " ";
+}
+NewName.append(SourceParam->getName());
+ParamToNewName[DestParam->getCanonicalDecl()] = std::move(NewName);
+  };
+
+  // Po

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

2019-10-30 Thread Matthias Gehre via Phabricator via cfe-commits

mgehre added a comment.

Friendly Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68074



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

2019-10-30 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

Build result: pass - 59704 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

2019-10-30 Thread Kristof Beyls via Phabricator via cfe-commits

kristof.beyls added inline comments.



Comment at: docs/ReleaseNotes.rst:84
+  In a future release of Clang, we intend to change the default to
+  ``-fno-lax-vector-conversions``.
+

efriedma wrote:
> rsmith wrote:
> > efriedma wrote:
> > > And if you want to allow your code to build with both clang-9 and 
> > > clang-10, you have to do version detection in your build scripts?
> > I guess you'd detect whether the compiler supports 
> > `-flax-vector-conversions=all`, and pass that if so, and otherwise pass 
> > `-flax-vector-conversions`. Well, either that or you fix your code to not 
> > rely on lax vector conversions between int and float vectors. If your code 
> > builds with GCC, you did that already (they never supported lax conversions 
> > between float and int vectors, as far as I can tell).
> > 
> > Do you have a preferred alternative?
> All the alternatives are terrible in their own way:
> 
> 1. This status quo, which breaks compatibility with previous versions of clang
> 2. We could make -flax-vector-conversions mean the same thing as earlier 
> versions of clang.  So the flag wouldn't have the same meaning as gcc's flag.
> 3. Some mix of the previous options: we could wait until there are one or two 
> released versions that support -flax-vector-conversions=all , then change the 
> meaning of -flax-vector-conversions.  But I have no idea how we would decide 
> on a timeline.
> 
> I ran into this issue recently updating our compiler.  That said, the code in 
> question was only using the implicit conversion in a couple places by 
> accident, so it was easy enough to just fix the source.
Maybe adding an entry in the release notes about this change could help with 
making option 1 slightly more palatable?
My guess is that option 1 is the right one for the long term (compatibility 
between gcc and clang so code is more portable between both compilers).


Repository:
  rC Clang

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

https://reviews.llvm.org/D67678



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

[PATCH] D69608: [clangd] Helper for getting nested namespace qualification

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Introduce a new helper for getting minimally required qualifiers
necessary to spell a name at a point in a given DeclContext. Currently takes
using directives and nested namespecifier of DeclContext itself into account.

Initially will be used in define inline and outline actions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69608

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp

Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -7,7 +7,16 @@
 //===--===//
 
 #include "AST.h"
+#include "Annotations.h"
+#include "ParsedAST.h"
+#include "TestTU.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -36,6 +45,66 @@
 "testns1::TestClass", "testns1"));
 }
 
+TEST(ClangdAST, GetQualification) {
+  struct {
+llvm::StringRef Test;
+std::vector Qualifications;
+  } Cases[] = {
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { class Foo {}; } }
+void insert(); // ns1::ns2::Foo
+namespace ns1 {
+  void insert(); // ns2::Foo
+  namespace ns2 {
+void insert(); // Foo
+  }
+  using namespace ns2;
+  void insert(); // Foo
+}
+using namespace ns1;
+void insert(); // ns2::Foo
+using namespace ns2;
+void insert(); // Foo
+  )cpp",
+  {"ns1::ns2::", "ns2::", "", "", "ns2::", ""},
+  },
+  };
+  for (const auto &Case : Cases) {
+Annotations Test(Case.Test);
+TestTU TU = TestTU::withCode(Test.code());
+ParsedAST AST = TU.build();
+std::vector InsertionPoints;
+const NamedDecl *TargetDecl;
+findDecl(AST, [&](const NamedDecl &ND) {
+  if (ND.getNameAsString() == "Foo") {
+TargetDecl = &ND;
+return true;
+  }
+
+  if (ND.getNameAsString() == "insert")
+InsertionPoints.push_back(&ND);
+  return false;
+});
+
+ASSERT_EQ(InsertionPoints.size(), Case.Qualifications.size());
+for (size_t I = 0, E = InsertionPoints.size(); I != E; ++I) {
+  const Decl *D = InsertionPoints[I];
+  auto Qual =
+  getQualification(AST.getASTContext(), D->getLexicalDeclContext(),
+   D->getBeginLoc(), TargetDecl);
+  if (!Qual)
+EXPECT_TRUE(Case.Qualifications[I].empty());
+  else {
+std::string Qualifier;
+llvm::raw_string_ostream OS(Qualifier);
+Qual->print(OS, AST.getASTContext().getPrintingPolicy(), true);
+EXPECT_EQ(OS.str(), Case.Qualifications[I]);
+  }
+}
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/AST.h
===
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -15,8 +15,10 @@
 
 #include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/MacroInfo.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 class SourceManager;
@@ -110,6 +112,20 @@
 /// void foo() -> returns null
 NestedNameSpecifierLoc getQualifierLoc(const NamedDecl &ND);
 
+/// Gets the nested name specifier necessary for spelling \p ND in \p
+/// DestContext, at \p InsertionPoint. It selects the shortest suffix of \p ND
+/// such that it is visible in \p DestContext, considering all the using
+/// directives before \p InsertionPoint.
+/// Returns null if no qualification is necessary. For example, if you want to
+/// qualify clang::clangd::bar::foo in clang::clangd::x, this function will
+/// return bar. Also if you have `using namespace clang::clangd::bar` before \p
+/// InsertionPoint this function will return null, since no qualification is
+/// necessary in that case.
+NestedNameSpecifier *getQualification(ASTContext &Context,
+  const DeclContext *DestContext,
+  SourceLocation InsertionPoint,
+  const NamedDecl *ND);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -244,6 +24

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

- Rebase
- Move qualification logic into AST.h so that it can be used by define outline, 
see D69033 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69033

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1714,6 +1714,138 @@
 template <> inline void foo(){})cpp")));
 }
 
+TEST_F(DefineInlineTest, DropCommonNamespecifiers) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a { namespace b { void aux(); } }
+  namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+  void bar();
+  namespace ns3 { void baz(); }
+  }
+  }
+
+  using namespace a;
+  using namespace a::b;
+  using namespace ns1::qq;
+  void ns1::ns2::ns3::b^az() {
+foo();
+bar();
+baz();
+ns1::ns2::ns3::baz();
+aux();
+test();
+  })cpp"),
+R"cpp(
+  namespace a { namespace b { void aux(); } }
+  namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+  void bar();
+  namespace ns3 { void baz(){
+foo();
+bar();
+baz();
+ns1::ns2::ns3::baz();
+a::b::aux();
+qq::test();
+  } }
+  }
+  }
+
+  using namespace a;
+  using namespace a::b;
+  using namespace ns1::qq;
+  )cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+  namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(); }
+  }
+
+  using namespace ns1::qq;
+  void ns1::ns2::b^az() { Foo f; B b; })cpp"),
+R"cpp(
+  namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(){ qq::Foo f; qq::B b; } }
+  }
+
+  using namespace ns1::qq;
+  )cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+  namespace ns1 {
+  namespace qq {
+template struct Foo { template  struct Bar {}; };
+template
+using B = typename Foo::template Bar;
+  }
+  namespace ns2 { void baz(); }
+  }
+
+  using namespace ns1::qq;
+  void ns1::ns2::b^az() { B b; })cpp"),
+R"cpp(
+  namespace ns1 {
+  namespace qq {
+template struct Foo { template  struct Bar {}; };
+template
+using B = typename Foo::template Bar;
+  }
+  namespace ns2 { void baz(){ qq::B b; } }
+  }
+
+  using namespace ns1::qq;
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
+  // FIXME: The last reference to cux() in body of foo should not be qualified,
+  // since there is a using directive inside the function body.
+  EXPECT_EQ(apply(R"cpp(
+namespace a {
+  void bar();
+  namespace b { struct Foo{}; void aux(); }
+  namespace c { void cux(); }
+}
+using namespace a;
+using X = b::Foo;
+void foo();
+
+using namespace b;
+using namespace c;
+void ^foo() {
+  cux();
+  bar();
+  X x;
+  aux();
+  using namespace c;
+  cux();
+})cpp"), R"cpp(
+namespace a {
+  void bar();
+  namespace b { struct Foo{}; void aux(); }
+  namespace c { void cux(); }
+}
+using namespace a;
+using X = b::Foo;
+void foo(){
+  c::cux();
+  bar();
+  X x;
+  b::aux();
+  using namespace c;
+  c::cux();
+}
+
+using namespace b;
+using namespace c;
+)cpp");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -136,8 +136,10 @@
   return true;
 }
 
-// Rewrites body of FD to fully-qualify all of the decls inside.
-llvm::Expected qualifyAllDecls(const FunctionDecl *FD) {
+// Rewrites body of FD by re-spelling all of the names to make sure they are
+// still valid in context of Target.
+llvm::Expected qualifyAllDecls(const FunctionDecl *FD,
+const FunctionDecl *Target) {
   // There are three types of spellings that needs to be qualified in a function
   // body:
   // - Types:   Foo -> ns::Foo
@@ -148,16 +150,16 @@
   //using ns3 = ns2 -> using ns3 = ns1::ns2
   //
   // Go over all references inside a function body to generate replacements that
-  // will fully qualify those. So that body can be moved into an arbitrary file.
+  // will qualify those. So that body can be moved into an arbitrary file.
   // We perform the qualification by qualyfying the first type/decl in a
   // (un)qualified name. e.g:
   //namespace a { namespace

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

In D69577#1726587 , @lichray wrote:

> Should we find a way to set `->`'s type to `TT_TrailingReturnArrow`?


that's possible then we might be able to use the existing spaces before rule

  if (Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow) ||
  Left.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow))
return true;


Repository:
  rC Clang

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

https://reviews.llvm.org/D69577



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm added a comment.

Revoking my previous request: I now have commit access and I intend to submit 
this shortly.


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

https://reviews.llvm.org/D63607



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

[PATCH] D69318: [analyzer] Add SufficientSizeArrayIndexingChecker

2019-10-30 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/SufficientSizeArrayIndexingChecker.cpp:75
+  // Should not warn on literal index expressions.
+  if (dyn_cast(Index->IgnoreParenCasts()))
+return;

Shoudn't we use `isa<>` if we don't use the resulting pointer?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/SufficientSizeArrayIndexingChecker.cpp:126
+C.getASTContext().UnsignedLongLongTy);
+  if (NumArrayElemsMinusOne.isUnknownOrUndef())
+return;

I think its better to stay consistent regards naming.
Use either number literals expressing numeric values, or spell out its name.
I would prefer the latter like: `ConstantOne` with type `SVal`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69318



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

2019-10-30 Thread Jan Wilken Dörrie via Phabricator via cfe-commits

jdoerrie added a comment.

Friendly Ping :)


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

https://reviews.llvm.org/D69520



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

[clang] 6bf5580 - [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via cfe-commits


Author: Peter Waller
Date: 2019-10-30T10:42:22Z
New Revision: 6bf55804924d5a1d902925ad080b1a2b57c5c75c

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

LOG: [clang][driver] Add basic --driver-mode=flang support for fortran

This patch adds a new Flang mode. When in Flang mode, the driver will
invoke flang for fortran inputs instead of falling back to the GCC
toolchain as it would otherwise do.

The behaviour of other driver modes are left unmodified to preserve
backwards compatibility.

It is intended that a soon to be implemented binary in the flang project
will import libclangDriver and run the clang driver in the new flang
mode.

Please note that since the binary invoked by the driver is under
development, there will no doubt be further tweaks necessary in future
commits.

* Initial support is added for basic driver phases
  * -E, -fsyntax-only, -emit-llvm -S, -emit-llvm, -S, (none specified)
  * -### tests are added for all of the above
  * This is more than is supported by f18 so far, which will emit errors
for those options which are unimplemented.

* A test is added that ensures that clang gives a reasonable error
  message if flang is not available in the path (without -###).

* Test that the driver accepts multiple inputs in --driver-mode=flang.

* Test that a combination of C and Fortran inputs run both clang and
  flang in --driver-mode=flang.

* clang/test/Driver/fortran.f95 is fixed to use the correct fortran
  comment character.

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

Added: 
clang/lib/Driver/ToolChains/Flang.cpp
clang/lib/Driver/ToolChains/Flang.h
clang/test/Driver/flang/Inputs/one.f90
clang/test/Driver/flang/Inputs/other.c
clang/test/Driver/flang/Inputs/two.f90
clang/test/Driver/flang/flang.F90
clang/test/Driver/flang/flang.f90
clang/test/Driver/flang/multiple-inputs-mixed.f90
clang/test/Driver/flang/multiple-inputs.f90

Modified: 
clang/include/clang/Driver/Driver.h
clang/include/clang/Driver/ToolChain.h
clang/include/clang/Driver/Types.h
clang/lib/Driver/CMakeLists.txt
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/Types.cpp
clang/test/Driver/fortran.f95
clang/test/Driver/lit.local.cfg

Removed: 




diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index 5e7283e31ee0..64f1eb8cb5d7 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -65,7 +65,8 @@ class Driver {
 GCCMode,
 GXXMode,
 CPPMode,
-CLMode
+CLMode,
+FlangMode
   } Mode;
 
   enum SaveTempsMode {
@@ -180,6 +181,10 @@ class Driver {
   /// Whether the driver should follow cl.exe like behavior.
   bool IsCLMode() const { return Mode == CLMode; }
 
+  /// Whether the driver should invoke flang for fortran inputs.
+  /// Other modes fall back to calling gcc which in turn calls gfortran.
+  bool IsFlangMode() const { return Mode == FlangMode; }
+
   /// Only print tool bindings, don't build any jobs.
   unsigned CCCPrintBindings : 1;
 
@@ -534,6 +539,10 @@ class Driver {
   /// handle this action.
   bool ShouldUseClangCompiler(const JobAction &JA) const;
 
+  /// ShouldUseFlangCompiler - Should the flang compiler be used to
+  /// handle this action.
+  bool ShouldUseFlangCompiler(const JobAction &JA) const;
+
   /// Returns true if we are performing any kind of LTO.
   bool isUsingLTO() const { return LTOMode != LTOK_None; }
 

diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index c40af1e6e01f..26d8d43dd2fc 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -134,6 +134,7 @@ class ToolChain {
   path_list ProgramPaths;
 
   mutable std::unique_ptr Clang;
+  mutable std::unique_ptr Flang;
   mutable std::unique_ptr Assemble;
   mutable std::unique_ptr Link;
   mutable std::unique_ptr IfsMerge;
@@ -141,6 +142,7 @@ class ToolChain {
   mutable std::unique_ptr OffloadWrapper;
 
   Tool *getClang() const;
+  Tool *getFlang() const;
   Tool *getAssemble() const;
   Tool *getLink() const;
   Tool *getIfsMerge() const;

diff  --git a/clang/include/clang/Driver/Types.h 
b/clang/include/clang/Driver/Types.h
index a605450e6e3d..c7c38fa52593 100644
--- a/clang/include/clang/Driver/Types.h
+++ b/clang/include/clang/Driver/Types.h
@@ -84,6 +84,9 @@ namespace types {
   /// isObjC - Is this an "ObjC" input (Obj-C and Obj-C++ sources and headers).
   bool isObjC(ID Id);
 
+  /// isFortran - Is this a Fortran input.
+  bool isFortran(ID Id);
+
   /// isSrcFile - Is this a source file, i.e. something that still has to be
   /// preprocessed. The logic behind this is the same that decides if the first
   /// compi

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm closed this revision.
peterwaller-arm added a comment.

Submitted in 6bf55804924d5a1d902925ad080b1a2b57c5c75c 
.


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

https://reviews.llvm.org/D63607



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

[PATCH] D69378: [AArch64][SVE] Implement masked store intrinsics

2019-10-30 Thread Kerry McLaughlin via Phabricator via cfe-commits

kmclaughlin updated this revision to Diff 227061.
kmclaughlin added a comment.

- Improve CHECK lines used in sve-masked-ldst-nonext.ll & 
sve-masked-ldst-trunc.ll


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

https://reviews.llvm.org/D69378

Files:
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
  llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll

Index: llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll
@@ -0,0 +1,66 @@
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s | FileCheck %s
+
+;
+; Masked Stores
+;
+
+define void @masked_trunc_store_nxv2i8( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv2i8:
+; CHECK-NEXT: st1b { z0.d }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv2i8( %trunc,  *%b, i32 8,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv2i16( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv2i16:
+; CHECK-NEXT: st1h { z0.d }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv2i16( %trunc,  *%b, i32 8,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv2i32( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv2i32:
+; CHECK-NEXT: st1w { z0.d }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv2i32( %trunc,  *%b, i32 8,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv4i8( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv4i8:
+; CHECK-NEXT: st1b { z0.s }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv4i8( %trunc,  *%b, i32 4,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv4i16( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv4i16:
+; CHECK-NEXT: st1h { z0.s }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv4i16( %trunc,  *%b, i32 4,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv8i8( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv8i8:
+; CHECK-NEXT: st1b { z0.h }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv8i8( %trunc,  *%b, i32 2,  %mask)
+  ret void
+}
+
+declare void @llvm.masked.store.nxv2i8(, *, i32, )
+declare void @llvm.masked.store.nxv2i16(, *, i32, )
+declare void @llvm.masked.store.nxv2i32(, *, i32, )
+declare void @llvm.masked.store.nxv4i8(, *, i32, )
+declare void @llvm.masked.store.nxv4i16(, *, i32, )
+declare void @llvm.masked.store.nxv8i8(, *, i32, )
Index: llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
===
--- llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
+++ llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
@@ -1,79 +1,173 @@
-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s | FileCheck %s
 
 ;
 ; Masked Loads
 ;
 
-define  @masked_load_nxv2i64( *%a,  %mask) {
+define  @masked_load_nxv2i64( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv2i64:
-; CHECK: ld1d { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv2i64( *%a, i32 8,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv4i32( *%a,  %mask) {
+define  @masked_load_nxv4i32( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv4i32:
-; CHECK: ld1w { [[IN:z[0-9]+]].s }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1w { z0.s }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv4i32( *%a, i32 4,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv8i16( *%a,  %mask) {
+define  @masked_load_nxv8i16( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv8i16:
-; CHECK: ld1h { [[IN:z[0-9]+]].h }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1h { z0.h }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv8i16( *%a, i32 2,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv16i8( *%a,  %mask) {
+define  @masked_load_nxv16i8( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv16i8:
-; CHECK: ld1b { [[IN:z[0-9]+]].b }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1b { z0.b }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv16i8( *%a, i32 1,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv2f64( *%a,  %mask) {
+define  @masked_load_nxv2f64( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv2f64:
-; CHECK: ld1d { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1d { z0

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 227062.
MyDeveloperDay added a reviewer: lichray.
MyDeveloperDay added a comment.

Detect deduction guides arrow as a TrailingReturn arrow, allowing use of 
existing space before/after rules


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

https://reviews.llvm.org/D69577

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4977,6 +4977,15 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1397,6 +1397,10 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) 
{
+  // Deduction guides trailing arrow "...) -> A".
+  Current.Type = TT_TrailingReturnArrow;
 } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4977,6 +4977,15 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1397,6 +1397,10 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Deduction guides trailing arrow "...) -> A".
+  Current.Type = TT_TrailingReturnArrow;
 } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69378: [AArch64][SVE] Implement masked store intrinsics

2019-10-30 Thread Sander de Smalen via Phabricator via cfe-commits

sdesmalen accepted this revision.
sdesmalen added a comment.

Nice one! LGTM!


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

https://reviews.llvm.org/D69378



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

[PATCH] D69567: [AArch64][SVE] Implement additional integer arithmetic intrinsics

2019-10-30 Thread Kerry McLaughlin via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe128c2086489: [AArch64][SVE] Implement additional integer 
arithmetic intrinsics (authored by kmclaughlin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69567

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-conversion.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-counting-bits.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-logical.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-logical.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-logical.ll
@@ -0,0 +1,99 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; CNOT
+;
+
+define  @cnot_i8( %a,  %pg,  %b) {
+; CHECK-LABEL: cnot_i8:
+; CHECK: cnot z0.b, p0/m, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.cnot.nxv16i8( %a,
+ %pg,
+ %b)
+  ret  %out
+}
+
+define  @cnot_i16( %a,  %pg,  %b) {
+; CHECK-LABEL: cnot_i16:
+; CHECK: cnot z0.h, p0/m, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.cnot.nxv8i16( %a,
+ %pg,
+ %b)
+  ret  %out
+}
+
+define  @cnot_i32( %a,  %pg,  %b) {
+; CHECK-LABEL: cnot_i32:
+; CHECK: cnot z0.s, p0/m, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.cnot.nxv4i32( %a,
+ %pg,
+ %b)
+  ret  %out
+}
+
+define  @cnot_i64( %a,  %pg,  %b) {
+; CHECK-LABEL: cnot_i64:
+; CHECK: cnot z0.d, p0/m, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.cnot.nxv2i64( %a,
+ %pg,
+ %b)
+  ret  %out
+}
+
+;
+; NOT
+;
+
+define  @not_i8( %a,  %pg,  %b) {
+; CHECK-LABEL: not_i8:
+; CHECK: not z0.b, p0/m, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.not.nxv16i8( %a,
+%pg,
+%b)
+  ret  %out
+}
+
+define  @not_i16( %a,  %pg,  %b) {
+; CHECK-LABEL: not_i16:
+; CHECK: not z0.h, p0/m, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.not.nxv8i16( %a,
+%pg,
+%b)
+  ret  %out
+}
+
+define  @not_i32( %a,  %pg,  %b) {
+; CHECK-LABEL: not_i32:
+; CHECK: not z0.s, p0/m, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.not.nxv4i32( %a,
+%pg,
+%b)
+  ret  %out
+}
+
+define  @not_i64( %a,  %pg,  %b) {
+; CHECK-LABEL: not_i64:
+; CHECK: not z0.d, p0/m, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.not.nxv2i64( %a,
+%pg,
+%b)
+  ret  %out
+}
+
+declare  @llvm.aarch64.sve.cnot.nxv16i8(, , )
+declare  @llvm.aarch64.sve.cnot.nxv8i16(, , )
+declare  @llvm.aarch64.sve.cnot.nxv4i32(, , )
+declare  @llvm.aarch64.sve.cnot.nxv2i64(, , )
+
+declare  @llvm.aarch64.sve.not.nxv16i8(, , )
+declare  @llvm.aarch64.sve.not.nxv8i16(, , )
+declare  @llvm.aarch64.sve.not.nxv4i32(, , )
+declare  @llvm.aarch64.sve.not.nxv2i64(, , )
Index: llvm/test/CodeGen/AArch64/sve-intrinsics-counting-bits.ll
===
--- llvm/test/CodeGen/AArch64/sve-intrinsics-counting-bits.ll
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-counting-bits.ll
@@ -1,6 +1,94 @@
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
 
 ;
+; CLS
+;
+
+define  @cls_i8( %a,  %pg,  %b) {
+; CHECK-LABEL: cls_i8:
+; CHECK: cls z0.b, p0/m, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.cls.nxv16i8( %a,
+%pg,
+%b)
+  ret  %out
+}
+
+define  @cls_i16( %a,  %pg,  %b) {
+; CHECK-LABEL: cls_i16:
+; CHECK: cls z0.h, p0/m, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.cls.nxv8i16( %a,
+%pg,
+%b)
+  ret  %out
+}
+
+define  @cls_i32( %a,  %pg,  %b) {
+; CHECK-LABEL: cls_i32:
+; CHECK: cls z0.s,

[PATCH] D69378: [AArch64][SVE] Implement masked store intrinsics

2019-10-30 Thread Kerry McLaughlin via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c2c94648e42: [AArch64][SVE] Implement masked store 
intrinsics (authored by kmclaughlin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69378

Files:
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
  llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll

Index: llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll
@@ -0,0 +1,66 @@
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s | FileCheck %s
+
+;
+; Masked Stores
+;
+
+define void @masked_trunc_store_nxv2i8( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv2i8:
+; CHECK-NEXT: st1b { z0.d }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv2i8( %trunc,  *%b, i32 8,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv2i16( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv2i16:
+; CHECK-NEXT: st1h { z0.d }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv2i16( %trunc,  *%b, i32 8,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv2i32( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv2i32:
+; CHECK-NEXT: st1w { z0.d }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv2i32( %trunc,  *%b, i32 8,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv4i8( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv4i8:
+; CHECK-NEXT: st1b { z0.s }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv4i8( %trunc,  *%b, i32 4,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv4i16( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv4i16:
+; CHECK-NEXT: st1h { z0.s }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv4i16( %trunc,  *%b, i32 4,  %mask)
+  ret void
+}
+
+define void @masked_trunc_store_nxv8i8( *%a,  %val,  *%b,  %mask) nounwind {
+; CHECK-LABEL: masked_trunc_store_nxv8i8:
+; CHECK-NEXT: st1b { z0.h }, p0, [x1]
+; CHECK-NEXT: ret
+  %trunc = trunc  %val to 
+  call void @llvm.masked.store.nxv8i8( %trunc,  *%b, i32 2,  %mask)
+  ret void
+}
+
+declare void @llvm.masked.store.nxv2i8(, *, i32, )
+declare void @llvm.masked.store.nxv2i16(, *, i32, )
+declare void @llvm.masked.store.nxv2i32(, *, i32, )
+declare void @llvm.masked.store.nxv4i8(, *, i32, )
+declare void @llvm.masked.store.nxv4i16(, *, i32, )
+declare void @llvm.masked.store.nxv8i8(, *, i32, )
Index: llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
===
--- llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
+++ llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
@@ -1,79 +1,173 @@
-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s | FileCheck %s
 
 ;
 ; Masked Loads
 ;
 
-define  @masked_load_nxv2i64( *%a,  %mask) {
+define  @masked_load_nxv2i64( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv2i64:
-; CHECK: ld1d { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv2i64( *%a, i32 8,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv4i32( *%a,  %mask) {
+define  @masked_load_nxv4i32( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv4i32:
-; CHECK: ld1w { [[IN:z[0-9]+]].s }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1w { z0.s }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv4i32( *%a, i32 4,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv8i16( *%a,  %mask) {
+define  @masked_load_nxv8i16( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv8i16:
-; CHECK: ld1h { [[IN:z[0-9]+]].h }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1h { z0.h }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv8i16( *%a, i32 2,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv16i8( *%a,  %mask) {
+define  @masked_load_nxv16i8( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv16i8:
-; CHECK: ld1b { [[IN:z[0-9]+]].b }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ld1b { z0.b }, p0/z, [x0]
+; CHECK-NEXT: ret
   %load = call  @llvm.masked.load.nxv16i8( *%a, i32 1,  %mask,  undef)
   ret  %load
 }
 
-define  @masked_load_nxv2f64( *%a,  %mask) {
+define  @masked_load_nxv2f64( *%a,  %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv2f64:
-; CHECK: ld1d { [[IN:z[0-

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

Could we have the `clang/test/Driver/flang/flang.F90` and 
`clang/test/Driver/flang/flang.f90` files in different directories please? As 
macOS's FS is case-insensitive, those two files have the same path from macOS 
perspective which is causing a whole bunch of issues (including breaking git 
even with 'core.ignorecase').


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

https://reviews.llvm.org/D63607



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

2019-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.

Currently, stencils are defined as a sequence of `StencilParts`. This
differentiation adds an unneeded layer of complexity to the definition of
Stencils. This change significantly simplifies the type structure: a stencil is
now conceptually any object implementing `StencilInterface` and `Stencil` is
just an alias for pointers to this interface.

To account for the sequencing that was supported by the old `Stencil` type, we
introduce a sequencing class that implements `StencilInterface`. That is,
sequences are just another kind of Stencil and no longer have any special
status.

Corresponding to this change in the type structure, we change the way `cat` is
used (and defined). `cat` bundles multiple features: it builds a stencil from a
sequence of subcomponents and admits multiple different types for its arguments,
while coercing them into the right type. Previously, `cat` was also used to
coerce a single `StencilPart` into a `Stencil`. With that distinction gone, many
uses of `cat` (e.g. in the tests) are unnecessary and have, therefore, been
removed.

Finally, we expose the "coercion" functionality with the first-class function
`makeStencil`, which was, previously, the private method `wrap`. This way, other
functions with Stencil-typed arguments can leverage `makeStencil`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69613

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

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -96,7 +96,7 @@
  .bind("expr")))
 .bind("stmt"));
 ASSERT_TRUE(StmtMatch);
-if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+if (auto ResultOrErr = Stencil->eval(StmtMatch->Result)) {
   ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
 } else {
   auto Err = llvm::handleErrors(ResultOrErr.takeError(),
@@ -131,11 +131,11 @@
   // Invert the if-then-else.
   auto Stencil = cat("if (!", node(Condition), ") ", statement(Else), " else ",
  statement(Then));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
-TEST_F(StencilTest, SingleStatementCallOperator) {
+TEST_F(StencilTest, StencilFactoryFunction) {
   StringRef Condition("C"), Then("T"), Else("E");
   const std::string Snippet = R"cc(
 if (true)
@@ -148,9 +148,9 @@
   hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else;
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  Stencil S = cat("if (!", node(Condition), ") ", statement(Else), " else ",
-  statement(Then));
-  EXPECT_THAT_EXPECTED(S(StmtMatch->Result),
+  auto Consumer = stencil("if (!", node(Condition), ") ", statement(Else),
+  " else ", statement(Then));
+  EXPECT_THAT_EXPECTED(Consumer(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
@@ -165,7 +165,7 @@
  hasThen(stmt().bind("a2";
   ASSERT_TRUE(StmtMatch);
   auto Stencil = cat("if(!", node("a1"), ") ", node("UNBOUND"), ";");
-  auto ResultOrErr = Stencil.eval(StmtMatch->Result);
+  auto ResultOrErr = Stencil->eval(StmtMatch->Result);
   EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError()))
   << "Expected unbound node, got " << *ResultOrErr;
 }
@@ -176,14 +176,14 @@
   StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue(Expected));
 }
 
 void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil,
  testing::Matcher MessageMatcher) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
Failed(testing::Property(
&StringError::getMessage, MessageMatcher)));
 }
@@ -195,28 +195,28 @@
 
 TEST_F(StencilTest, IfBoundOpBound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound(Id, text("5"), text("7"))), "5");
+  testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
 }
 
 TEST_F(StencilTest, IfBoundOpUnbound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7");
+  testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
 }
 
 TEST_F(StencilTest,

[PATCH] D69498: IR: Invert convergent attribute handling

2019-10-30 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield added a comment.

Requiring the presence of an attribute for correctness is a bad thing. OpenMP 
was missing this annotation in a number of places and is probably still missing 
it elsewhere. I wouldn't bet on handwritten bitcode libraries getting it right 
everywhere either. An optimisation pass accidentally dropping the attribute 
seems a plausible failure mode as well.

Strongly in favour of replacing convergent with no{n}convergent in the IR.

Not as convinced it should be inserted by the front end. The attribute is 
needed before any CFG rewrites and as far as I know they all occur downstream 
of the front end. That suggests an IR pass that walks over all functions, 
intrinsic calls, inline asm and so forth and marks them as appropriate. 
Standard C++/x86 and similar don't need to run the pass, OpenCL/x86 probably 
does. I'd suggest running it manually across handwritten bitcode as a sanity 
check as well.

Of course, //if// a front end targeting gpus wants to change control flow 
(julia may do this, mlir does, I sincerely hope clang doesn't) //and// use this 
attribute to control the process, then that front end picks up the 
responsibility for inserting the attribute where it wants it.


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

https://reviews.llvm.org/D69498



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Sorry, I had missed the RfC, but it looks like there wasn't a lot of discussion 
on it anyways.

Adding fortran support to clang's driver has been suggested and decided against 
before, see "[cfe-commits] [RFC and PATCH] Fortran"

In D63607#1726895 , @teemperor wrote:

> Could we have the `clang/test/Driver/flang/flang.F90` and 
> `clang/test/Driver/flang/flang.f90` files in different directories please? As 
> macOS's FS is case-insensitive, those two files have the same path from macOS 
> perspective which is causing a whole bunch of issues (including breaking git 
> even with 'core.ignorecase').


One issue is that the test flakily fails about half the time

http://45.33.8.238/mac/2498/summary.html fail
http://45.33.8.238/mac/2499/summary.html pass
http://45.33.8.238/mac/2500/summary.html fail
http://45.33.8.238/mac/2501/summary.html pass
http://45.33.8.238/mac/2502/summary.html fail
http://45.33.8.238/mac/2503/summary.html pass
http://45.33.8.238/mac/2504/summary.html fail


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

https://reviews.llvm.org/D63607



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

2019-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

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

commented sequence operator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613

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

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -96,7 +96,7 @@
  .bind("expr")))
 .bind("stmt"));
 ASSERT_TRUE(StmtMatch);
-if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+if (auto ResultOrErr = Stencil->eval(StmtMatch->Result)) {
   ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
 } else {
   auto Err = llvm::handleErrors(ResultOrErr.takeError(),
@@ -131,11 +131,11 @@
   // Invert the if-then-else.
   auto Stencil = cat("if (!", node(Condition), ") ", statement(Else), " else ",
  statement(Then));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
-TEST_F(StencilTest, SingleStatementCallOperator) {
+TEST_F(StencilTest, StencilFactoryFunction) {
   StringRef Condition("C"), Then("T"), Else("E");
   const std::string Snippet = R"cc(
 if (true)
@@ -148,9 +148,9 @@
   hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else;
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  Stencil S = cat("if (!", node(Condition), ") ", statement(Else), " else ",
-  statement(Then));
-  EXPECT_THAT_EXPECTED(S(StmtMatch->Result),
+  auto Consumer = stencil("if (!", node(Condition), ") ", statement(Else),
+  " else ", statement(Then));
+  EXPECT_THAT_EXPECTED(Consumer(StmtMatch->Result),
HasValue("if (!true) return 0; else return 1;"));
 }
 
@@ -165,7 +165,7 @@
  hasThen(stmt().bind("a2";
   ASSERT_TRUE(StmtMatch);
   auto Stencil = cat("if(!", node("a1"), ") ", node("UNBOUND"), ";");
-  auto ResultOrErr = Stencil.eval(StmtMatch->Result);
+  auto ResultOrErr = Stencil->eval(StmtMatch->Result);
   EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError()))
   << "Expected unbound node, got " << *ResultOrErr;
 }
@@ -176,14 +176,14 @@
   StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue(Expected));
 }
 
 void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil,
  testing::Matcher MessageMatcher) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
Failed(testing::Property(
&StringError::getMessage, MessageMatcher)));
 }
@@ -195,28 +195,28 @@
 
 TEST_F(StencilTest, IfBoundOpBound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound(Id, text("5"), text("7"))), "5");
+  testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
 }
 
 TEST_F(StencilTest, IfBoundOpUnbound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7");
+  testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
 }
 
 TEST_F(StencilTest, ExpressionOpNoParens) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(expression(Id)), "3");
+  testExpr(Id, "3;", expression(Id), "3");
 }
 
 // Don't parenthesize a parens expression.
 TEST_F(StencilTest, ExpressionOpNoParensParens) {
   StringRef Id = "id";
-  testExpr(Id, "(3);", cat(expression(Id)), "(3)");
+  testExpr(Id, "(3);", expression(Id), "(3)");
 }
 
 TEST_F(StencilTest, ExpressionOpBinaryOpParens) {
   StringRef Id = "id";
-  testExpr(Id, "3+4;", cat(expression(Id)), "(3+4)");
+  testExpr(Id, "3+4;", expression(Id), "(3+4)");
 }
 
 // `expression` shares code with other ops, so we get sufficient coverage of the
@@ -224,33 +224,33 @@
 // tests should be added.
 TEST_F(StencilTest, ExpressionOpUnbound) {
   StringRef Id = "id";
-  testFailure(Id, "3;", cat(expression("ACACA")),
+  testFailure(Id, "3;", expression("ACACA"),
   AllOf(HasSubstr("ACACA"), HasSubstr("not bound")));
 }
 
 TEST_F(StencilTest, DerefPointer) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x;", cat(deref(Id)), "*x");
+  testExpr(Id, "int *x; x;", deref(Id), "*x");
 }
 
 TEST_F(StencilTest, DerefBinOp) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x + 1;", cat(deref(Id)), "*(x + 1)");
+  testExpr(Id

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2090
 continue;
+  if (Next->isOneOf(tok::star, tok::arrow))
+continue;

MyDeveloperDay wrote:
> sammccall wrote:
> > This seems like it's supposed to be handled by the token annotator, and the 
> > same cause will result in bugs in other places - why aren't these tokens 
> > being marked as TT_OverloadedOperator?
> > 
> > I'd guess the deeper fix is in the `kw_operator` case in consumeToken(). 
> > The way it's written looks suspect, though I'm not an expert on any of this.
> The * from *() is already labelled as a TT_PointerOrRefernce so can't also be 
> marked as an Overloaded operator so use that in the loop.
Right, the fact that it's incorrectly labeled as a PointerOrReference is a bug, 
is it not possible to fix that instead of working around it here?

In the consumeToken loop, it seems the `*` in `operator *`  is labeled as 
PointerOrReference by logic that's trying to handle e.g. `StringRef::operator 
char*()`. However when the `*` is _immediately_ preceded by `operator`, it's 
always an overloaded operator name rather than a pointer, I think?


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

https://reviews.llvm.org/D69573



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

[PATCH] D69615: [clangd] Implement a function to lex the file to find candidate occurrences.

2019-10-30 Thread Haojian Wu via Phabricator via cfe-commits

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

This will be used for incoming cross-file rename (to detect index
staleness issue).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69615

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

Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -680,6 +680,22 @@
 EXPECT_EQ(Res.EnclosingNamespace, Case.EnclosingNamespace) << Test.code();
   }
 }
+
+TEST(SourceCodeTests, IdentifierRanges) {
+  Annotations Code(R"cpp(
+   class [[Foo]] {};
+   // Foo
+   /* Foo */
+   void f() {
+ [[Foo]] foo;
+   }
+  )cpp");
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus = true;
+  EXPECT_EQ(Code.ranges(),
+collectIdentifierRanges("Foo", Code.code(), LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -232,6 +232,11 @@
 llvm::StringMap collectIdentifiers(llvm::StringRef Content,
  const format::FormatStyle &Style);
 
+/// Collects all ranges of the given identifier in the source code.
+std::vector collectIdentifierRanges(llvm::StringRef IdentifierName,
+   llvm::StringRef Content,
+   const LangOptions &LangOpts);
+
 /// Collects words from the source code.
 /// Unlike collectIdentifiers:
 /// - also finds text in comments:
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -719,27 +719,31 @@
   return formatReplacements(Code, std::move(*CleanReplaces), Style);
 }
 
-void lex(llvm::StringRef Code, const format::FormatStyle &Style,
- llvm::function_ref Action) {
+void lex(llvm::StringRef Code, const LangOptions &LangOpts,
+ llvm::function_ref
+ Action) {
   // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
   std::string NullTerminatedCode = Code.str();
   SourceManagerForFile FileSM("dummy.cpp", NullTerminatedCode);
   auto &SM = FileSM.get();
   auto FID = SM.getMainFileID();
-  Lexer Lex(FID, SM.getBuffer(FID), SM, format::getFormattingLangOpts(Style));
+  Lexer Lex(FID, SM.getBuffer(FID), SM, LangOpts);
   Token Tok;
 
   while (!Lex.LexFromRawLexer(Tok))
-Action(Tok, sourceLocToPosition(SM, Tok.getLocation()));
+Action(Tok, Tok.getLocation(), SM);
   // LexFromRawLexer returns true after it lexes last token, so we still have
   // one more token to report.
-  Action(Tok, sourceLocToPosition(SM, Tok.getLocation()));
+  Action(Tok, Tok.getLocation(), SM);
 }
 
 llvm::StringMap collectIdentifiers(llvm::StringRef Content,
  const format::FormatStyle &Style) {
   llvm::StringMap Identifiers;
-  lex(Content, Style, [&](const clang::Token &Tok, Position) {
+  auto LangOpt = format::getFormattingLangOpts(Style);
+  lex(Content, LangOpt, [&](const clang::Token &Tok,
+SourceLocation, const SourceManager &) {
 switch (Tok.getKind()) {
 case tok::identifier:
   ++Identifiers[Tok.getIdentifierInfo()->getName()];
@@ -754,6 +758,25 @@
   return Identifiers;
 }
 
+std::vector collectIdentifierRanges(llvm::StringRef IdentifierName,
+   llvm::StringRef Content,
+   const LangOptions &LangOpts) {
+  std::vector Ranges;
+  lex(Content, LangOpts,
+  [&](const clang::Token &Tok, SourceLocation Loc,
+  const SourceManager &SM) {
+llvm::StringRef TokenName;
+if (Tok.getKind() == tok::identifier)
+  TokenName = Tok.getIdentifierInfo()->getName();
+else if (Tok.getKind() == tok::raw_identifier)
+  TokenName = Tok.getRawIdentifier();
+if (TokenName == IdentifierName)
+  if (auto Range = getTokenRange(SM, LangOpts, Loc))
+Ranges.push_back(*Range);
+  });
+  return Ranges;
+}
+
 namespace {
 struct NamespaceEvent {
   enum {
@@ -786,8 +809,10 @@
   std::string NSName;
 
   NamespaceEvent Event;
-  lex(Code, Style, [&](const clang::Token &Tok, Position P) {
-Event.Pos = std::move(P);
+  lex(Code, format::getFormattingLangOpts(Style),
+  [&](const clang::Token &Tok,
+  SourceLocation P, const SourceManager& SM) {
+Event.Pos = sourceL

[clang] 6c0a160 - Rename a flang test case

2019-10-30 Thread Jeremy Morse via cfe-commits


Author: Jeremy Morse
Date: 2019-10-30T13:29:47Z
New Revision: 6c0a160c2d33e54aecf1538bf7c85d8da92051be

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

LOG: Rename a flang test case

On Windows and macOS, the filesystem is case insensitive, and these files
interfere with each other. Reading through, the case of the file extension
is part of the test. I've altered the rest of the name instead.

Added: 
clang/test/Driver/flang/flang_ucase.F90

Modified: 
clang/test/Driver/flang/flang.f90

Removed: 
clang/test/Driver/flang/flang.F90



diff  --git a/clang/test/Driver/flang/flang.f90 
b/clang/test/Driver/flang/flang.f90
index 56c1ace6f860..9d47c7c90225 100644
--- a/clang/test/Driver/flang/flang.f90
+++ b/clang/test/Driver/flang/flang.f90
@@ -1,6 +1,6 @@
 ! Check that flang -fc1 is invoked when in --driver-mode=flang.
 
-! This is a copy of flang.F90 because the driver has logic in it which
+! This is a copy of flang_ucase.F90 because the driver has logic in it which
 ! 
diff erentiates between F90 and f90 files. Flang will not treat these files
 ! 
diff erently.
 

diff  --git a/clang/test/Driver/flang/flang.F90 
b/clang/test/Driver/flang/flang_ucase.F90
similarity index 98%
rename from clang/test/Driver/flang/flang.F90
rename to clang/test/Driver/flang/flang_ucase.F90
index 37553c7c2760..323afb21dccf 100644
--- a/clang/test/Driver/flang/flang.F90
+++ b/clang/test/Driver/flang/flang_ucase.F90
@@ -48,4 +48,4 @@
 ! CHECK-EMIT-OBJ-DAG: "-o" "{{[^"]*}}.o"
 
 ! Should end in the input file.
-! ALL: "{{.*}}flang.F90"{{$}}
+! ALL: "{{.*}}flang_ucase.F90"{{$}}



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

Re: [PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Jeremy Morse via cfe-commits

FYI, in llvmorg-10-init-8612-g6c0a160c2d3 [0] I've renamed one of the
two files, Windows too has a case-insensitive filesystem layer and
this broke our CI. (I think I've preserved the original intention of
the tests).

[0] 
https://github.com/llvm/llvm-project/commit/6c0a160c2d33e54aecf1538bf7c85d8da92051be

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

[PATCH] D69615: [clangd] Implement a function to lex the file to find candidate occurrences.

2019-10-30 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

Build result: pass - 59741 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69615



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm added a comment.

In D63607#1726895 , @teemperor wrote:

> Could we have the `clang/test/Driver/flang/flang.F90` and 
> `clang/test/Driver/flang/flang.f90` files in different directories please? As 
> macOS's FS is case-insensitive, those two files have the same path from macOS 
> perspective which is causing a whole bunch of issues (including breaking git 
> even with 'core.ignorecase').


This was fixed by Jeremy Morse in 6c0a160c2d33e54aecf1538bf7c85d8da92051be 
 - thanks.

I'm investigating the non-deterministic failure but don't have access to a mac 
to test.

@thakis is the flaky test resolved by 6c0a160c 
 or is it 
still outstanding?


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

https://reviews.llvm.org/D63607



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 227083.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.

Extend this revision to cover additional 
https://bugs.llvm.org/show_bug.cgi?id=43783 issue (which has overlap)

New revision correctly formats the following code:

  constexpr auto
  operator*() const -> reference {
bfexpects_when_compliant(m_i < m_t->size());
return m_t->get()[m_i];
  }
  
  constexpr auto
  operator->() const -> pointer {
bfexpects_when_compliant(m_i < m_t->size());
return &m_t->get()[m_i];
  }
  
  constexpr auto
  operator[](index_type n) const -> reference {
bfexpects_when_compliant(m_i < m_t->size());
return m_t->get()[m_i];
  }
  
  constexpr auto
  operator++() -> lra_iterator & {
++m_i;
return *this;
  }
  
  constexpr auto
  operator()() const -> reference {}
  
  constexpr auto
  operator++() const -> reference {}
  
  constexpr auto
  operator--() const -> reference {}
  
  constexpr auto
  operator*() const -> reference {}
  
  constexpr auto
  operator->() const -> reference {}
  
  constexpr auto
  operator>>() const -> reference {}
  
  constexpr auto
  operator<<() const -> reference {}
  
  constexpr auto
  operator[]() const -> reference {}
  
  constexpr auto
  operator void*() const -> reference {}
  
  constexpr auto
  operator char*() const -> reference {}
  
  constexpr auto
  operator Foo*() const -> reference {}
  
  constexpr auto
  operator!() const -> reference {}
  
  class Foo {
  public:
bool operator!() const;
bool operator<(Foo const &) const;
bool operator*() const;
bool operator->() const;
bool operator+() const;
bool operator-() const;
bool f() const;
  };
  
  bool
  Foo::operator!() const {
return true;
  }
  bool
  Foo::operator<(Foo const &) const {
return true;
  }
  bool
  Foo::operator*() const {
return true;
  }
  bool
  Foo::operator->() const {
return true;
  }
  bool
  Foo::operator+() const {
return true;
  }
  bool
  Foo::operator-() const {
return true;
  }
  bool
  Foo::f() const {
return true;
  }


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

https://reviews.llvm.org/D69573

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6111,7 +6111,40 @@
"void\n"
"A::operator>>() {}\n"
"void\n"
-   "A::operator+() {}\n",
+   "A::operator+() {}\n"
+   "void\n"
+   "A::operator*() {}\n"
+   "void\n"
+   "A::operator->() {}\n"
+   "void\n"
+   "A::operator void*() {}\n"
+   "void\n"
+   "A::operator char*() {}\n"
+   "void\n"
+   "A::operator[]() {}\n"
+   "void\n"
+   "A::operator!() {}\n",
+   Style);
+  verifyFormat("constexpr auto\n"
+   "operator()() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator>>() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator+() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator*() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator->() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator++() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator void*() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator char*() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator!() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator[]() const -> reference {}\n",
Style);
   verifyFormat("void *operator new(std::size_t s);", // No break here.
Style);
@@ -6953,7 +6986,7 @@
   verifyFormat("bool operator[]();");
   verifyFormat("operator bool();");
   verifyFormat("operator int();");
-  verifyFormat("operator void *();");
+  verifyFormat("operator void*();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -952,7 +952,7 @@
 consumeToken();
 if (CurrentToken &&
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type =

[clang] ba7bde6 - [ASTImporter] Add support for BuiltinTemplateDecl

2019-10-30 Thread Raphael Isemann via cfe-commits


Author: Raphael Isemann
Date: 2019-10-30T14:53:35+01:00
New Revision: ba7bde65dcfff543cefc1de9adcda7f503de

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

LOG: [ASTImporter] Add support for BuiltinTemplateDecl

Summary:
That decl kind is currently not implemented. BuiltinTemplateDecl is for decls 
that are hardcoded in the
ASTContext, so we can import them like we do other builtin decls by just taking 
the equivalent
decl from the target ASTContext.

Reviewers: martong, a.sidorin, shafik

Reviewed By: martong, shafik

Subscribers: rnkovacs, kristina, cfe-commits

Tags: #clang

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

Added: 
clang/test/Import/builtin-template/Inputs/S.cpp
clang/test/Import/builtin-template/test.cpp

Modified: 
clang/lib/AST/ASTImporter.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 54acca7dc62c..9477e414cf55 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -44,6 +44,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -483,6 +484,7 @@ namespace clang {
 ExpectedDecl VisitUsingDirectiveDecl(UsingDirectiveDecl *D);
 ExpectedDecl VisitUnresolvedUsingValueDecl(UnresolvedUsingValueDecl *D);
 ExpectedDecl VisitUnresolvedUsingTypenameDecl(UnresolvedUsingTypenameDecl 
*D);
+ExpectedDecl VisitBuiltinTemplateDecl(BuiltinTemplateDecl *D);
 
 Expected
 ImportObjCTypeParamList(ObjCTypeParamList *list);
@@ -4464,6 +4466,20 @@ ExpectedDecl 
ASTNodeImporter::VisitUnresolvedUsingTypenameDecl(
   return ToUsing;
 }
 
+ExpectedDecl ASTNodeImporter::VisitBuiltinTemplateDecl(BuiltinTemplateDecl *D) 
{
+  Decl* ToD = nullptr;
+  switch (D->getBuiltinTemplateKind()) {
+  case BuiltinTemplateKind::BTK__make_integer_seq:
+ToD = Importer.getToContext().getMakeIntegerSeqDecl();
+break;
+  case BuiltinTemplateKind::BTK__type_pack_element:
+ToD = Importer.getToContext().getTypePackElementDecl();
+break;
+  }
+  assert(ToD && "BuiltinTemplateDecl of unsupported kind!");
+  Importer.MapImported(D, ToD);
+  return ToD;
+}
 
 Error ASTNodeImporter::ImportDefinition(
 ObjCInterfaceDecl *From, ObjCInterfaceDecl *To, ImportDefinitionKind Kind) 
{

diff  --git a/clang/test/Import/builtin-template/Inputs/S.cpp 
b/clang/test/Import/builtin-template/Inputs/S.cpp
new file mode 100644
index ..d5c9a9ae0309
--- /dev/null
+++ b/clang/test/Import/builtin-template/Inputs/S.cpp
@@ -0,0 +1,16 @@
+template 
+struct Seq {
+  static constexpr T PackSize = sizeof...(I);
+};
+
+template 
+using MakeSeq = __make_integer_seq;
+
+
+using SizeT = decltype(sizeof(int));
+
+template 
+using TypePackElement = __type_pack_element;
+
+template 
+struct X;

diff  --git a/clang/test/Import/builtin-template/test.cpp 
b/clang/test/Import/builtin-template/test.cpp
new file mode 100644
index ..3ae7b53e9d45
--- /dev/null
+++ b/clang/test/Import/builtin-template/test.cpp
@@ -0,0 +1,30 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s 
-Xcc -DSEQ | FileCheck --check-prefix=CHECK-SEQ %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s 
-Xcc -DPACK | FileCheck --check-prefix=CHECK-PACK %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s 
-Xcc -DPACK -Xcc -DSEQ | FileCheck --check-prefixes=CHECK-SEQ,CHECK-PACK %s
+
+// CHECK-SEQ: BuiltinTemplateDecl
+// CHECK-SEQ-SAME: 
+// CHECK-SEQ-SAME: implicit
+// CHECK-SEQ-SAME: __make_integer_seq
+
+// CHECK-PACK: BuiltinTemplateDecl
+// CHECK-PACK-SAME: 
+// CHECK-PACK-SAME: implicit
+// CHECK-PACK-SAME: __type_pack_element
+
+void expr() {
+#ifdef SEQ
+  typedef MakeSeq M1;
+  M1 m1;
+  typedef MakeSeq M2;
+  M2 m2;
+  static_assert(M1::PackSize == 3, "");
+  static_assert(M2::PackSize == 4, "");
+#endif
+
+#ifdef PACK
+  static_assert(__is_same(TypePackElement<0, X<0>>, X<0>), "");
+  static_assert(__is_same(TypePackElement<0, X<0>, X<1>>, X<0>), "");
+  static_assert(__is_same(TypePackElement<1, X<0>, X<1>>, X<1>), "");
+#endif
+}



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

[PATCH] D69566: [ASTImporter] Add support for BuiltinTemplateDecl

2019-10-30 Thread Raphael Isemann via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba7bde65dcff: [ASTImporter] Add support for 
BuiltinTemplateDecl (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69566

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Import/builtin-template/Inputs/S.cpp
  clang/test/Import/builtin-template/test.cpp


Index: clang/test/Import/builtin-template/test.cpp
===
--- /dev/null
+++ clang/test/Import/builtin-template/test.cpp
@@ -0,0 +1,30 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s 
-Xcc -DSEQ | FileCheck --check-prefix=CHECK-SEQ %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s 
-Xcc -DPACK | FileCheck --check-prefix=CHECK-PACK %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s 
-Xcc -DPACK -Xcc -DSEQ | FileCheck --check-prefixes=CHECK-SEQ,CHECK-PACK %s
+
+// CHECK-SEQ: BuiltinTemplateDecl
+// CHECK-SEQ-SAME: 
+// CHECK-SEQ-SAME: implicit
+// CHECK-SEQ-SAME: __make_integer_seq
+
+// CHECK-PACK: BuiltinTemplateDecl
+// CHECK-PACK-SAME: 
+// CHECK-PACK-SAME: implicit
+// CHECK-PACK-SAME: __type_pack_element
+
+void expr() {
+#ifdef SEQ
+  typedef MakeSeq M1;
+  M1 m1;
+  typedef MakeSeq M2;
+  M2 m2;
+  static_assert(M1::PackSize == 3, "");
+  static_assert(M2::PackSize == 4, "");
+#endif
+
+#ifdef PACK
+  static_assert(__is_same(TypePackElement<0, X<0>>, X<0>), "");
+  static_assert(__is_same(TypePackElement<0, X<0>, X<1>>, X<0>), "");
+  static_assert(__is_same(TypePackElement<1, X<0>, X<1>>, X<1>), "");
+#endif
+}
Index: clang/test/Import/builtin-template/Inputs/S.cpp
===
--- /dev/null
+++ clang/test/Import/builtin-template/Inputs/S.cpp
@@ -0,0 +1,16 @@
+template 
+struct Seq {
+  static constexpr T PackSize = sizeof...(I);
+};
+
+template 
+using MakeSeq = __make_integer_seq;
+
+
+using SizeT = decltype(sizeof(int));
+
+template 
+using TypePackElement = __type_pack_element;
+
+template 
+struct X;
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -44,6 +44,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -483,6 +484,7 @@
 ExpectedDecl VisitUsingDirectiveDecl(UsingDirectiveDecl *D);
 ExpectedDecl VisitUnresolvedUsingValueDecl(UnresolvedUsingValueDecl *D);
 ExpectedDecl VisitUnresolvedUsingTypenameDecl(UnresolvedUsingTypenameDecl 
*D);
+ExpectedDecl VisitBuiltinTemplateDecl(BuiltinTemplateDecl *D);
 
 Expected
 ImportObjCTypeParamList(ObjCTypeParamList *list);
@@ -4464,6 +4466,20 @@
   return ToUsing;
 }
 
+ExpectedDecl ASTNodeImporter::VisitBuiltinTemplateDecl(BuiltinTemplateDecl *D) 
{
+  Decl* ToD = nullptr;
+  switch (D->getBuiltinTemplateKind()) {
+  case BuiltinTemplateKind::BTK__make_integer_seq:
+ToD = Importer.getToContext().getMakeIntegerSeqDecl();
+break;
+  case BuiltinTemplateKind::BTK__type_pack_element:
+ToD = Importer.getToContext().getTypePackElementDecl();
+break;
+  }
+  assert(ToD && "BuiltinTemplateDecl of unsupported kind!");
+  Importer.MapImported(D, ToD);
+  return ToD;
+}
 
 Error ASTNodeImporter::ImportDefinition(
 ObjCInterfaceDecl *From, ObjCInterfaceDecl *To, ImportDefinitionKind Kind) 
{


Index: clang/test/Import/builtin-template/test.cpp
===
--- /dev/null
+++ clang/test/Import/builtin-template/test.cpp
@@ -0,0 +1,30 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s -Xcc -DSEQ | FileCheck --check-prefix=CHECK-SEQ %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s -Xcc -DPACK | FileCheck --check-prefix=CHECK-PACK %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s -Xcc -DPACK -Xcc -DSEQ | FileCheck --check-prefixes=CHECK-SEQ,CHECK-PACK %s
+
+// CHECK-SEQ: BuiltinTemplateDecl
+// CHECK-SEQ-SAME: 
+// CHECK-SEQ-SAME: implicit
+// CHECK-SEQ-SAME: __make_integer_seq
+
+// CHECK-PACK: BuiltinTemplateDecl
+// CHECK-PACK-SAME: 
+// CHECK-PACK-SAME: implicit
+// CHECK-PACK-SAME: __type_pack_element
+
+void expr() {
+#ifdef SEQ
+  typedef MakeSeq M1;
+  M1 m1;
+  typedef MakeSeq M2;
+  M2 m2;
+  static_assert(M1::PackSize == 3, "");
+  static_assert(M2::PackSize == 4, "");
+#endif
+
+#ifdef PACK
+  static_assert(__is_same(TypePackElement<0, X<0>>, X<0>), "");
+  static_assert(__is_same(TypePackElement<0, X<0>, X<1>>, X<0>), "");
+  static_assert(__is_same(TypePackElement<1, X<0>, X<

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:955
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type = TT_OverloadedOperator;

I'm confused about this: ISTM that where we were previously always treating 
star as a pointer, now we're always treating it as an operator name.
Whereas it's sometimes an operator name (immediately after the `operator` 
keyword) and sometimes a pointer (following a type name).

Maybe we should shift the OverloadedOperator labelling outside the loop (since 
AFAICT it should only apply to the first token) and then keep the loop to mark 
stars/amps elsewhere in the operator name as PointerOrReference?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2626
+  Left.Previous && Left.Previous->is(tok::kw_operator))
+// No space between the type and the *
+// operator void*(), operator char*(), operator Foo*().

why?



Comment at: clang/unittests/Format/FormatTest.cpp:6989
   verifyFormat("operator int();");
-  verifyFormat("operator void *();");
+  verifyFormat("operator void*();");
   verifyFormat("operator SomeType();");

this looks like a regression at first glance, in LLVM style there's a space 
between type and *


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

https://reviews.llvm.org/D69573



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

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added inline comments.

Comment at: clang/lib/Sema/SemaCoroutine.cpp:896
+  if (S.CanPerformCopyInitialization(Entity, &AsRvalue))
+return true;
+} else if (auto *FTD = dyn_cast(D)) {

aaronpuchert wrote:
> Overlad resolution can actually still fail if there is a viable candidate, 
> namely when there are multiple candidates and none is better than all others. 
> It's a bit weird though to fall back to lvalue parameter then as if nothing 
> happened.
That is an interesting point! I had not considered it during 
[P1155](https://wg21.link/p1155r2). I imagine that it might make implementation 
of [P1155](https://wg21.link/p1155r2)'s new logic more difficult.

GCC 8 (but not trunk) implements the behavior I would expect to see for 
derived-to-base conversions: https://godbolt.org/z/fj_lNw

C++ always treats "an embarrassment of riches" equivalently to a "famine"; 
overload resolution //can// fail due to ambiguity just as easily as it can fail 
due to no candidates at all. I agree it's "a bit weird," but it would be vastly 
weirder if C++ did anything //different// from its usual behavior in this case.

I'm still amenable to the idea that `co_return` should simply not do the 
copy-elision or implicit-move optimizations at all. I wish I knew some 
use-cases for `co_return`, so that we could see if the optimization is useful 
to anyone.

Comment at: clang/test/SemaCXX/coroutine-rvo.cpp:65
+  NoCopyNoMove value;
+  co_return value;
+}

@aaronpuchert wrote:
> Given the complexities of this implementation, I'm beginning to doubt whether 
> implicit moves make sense for `co_return` at all. Since there can never be 
> any kind of RVO, why not always require an explicit `std::move`? Implicit 
> moves on return were introduced because an explicit move would inhibit NRVO, 
> and without move there wouldn't be a graceful fallback for implementations 
> that don't have NRVO.

I still think that it makes sense to do implicit-move on //any// control-flow 
construct that "exits" the current function. Right now that's `return` and 
`throw`, and they both do implicit-move (albeit inconsistently with plenty of 
implementation divergence documented in [P1155](http://wg21.link/p1155r2)). 
C++2a adds `co_return` as another way to "exit." I think it would be 
un-graceful and inconsistent to permit `return p` but require `co_return 
std::move(p)`.

Now, I do think that C++2a Coroutines are needlessly complicated, which makes 
the implementation needlessly complicated. You've noticed that to codegen 
`t.return_value(x)` requires not just overload resolution, but actually 
figuring out whether `task::return_value` is a function at all — it might be a 
static data member of function-pointer type, or something [even 
wilder](https://quuxplusone.github.io/blog/2019/09/18/cppcon-whiteboard-puzzle/).
 But eliminating implicit-move won't make that complexity go away, will it? 
Doing implicit-move just means doing the required overload resolution twice 
instead of once. That //should// be easy, compared to the rest of the mess.

That said, I would be happy to start a thread among you, me, David Stone, and 
the EWG mailing list to consider removing the words "or `co_return`" from 
[class.copy.elision/3](http://eel.is/c++draft/class.copy.elision#3.1). Say the 
word.

[EDIT: aw shoot, I never submitted this comment last time]

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68845

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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

6c0a160c2d33e54aecf1538bf7c85d8da92051be 
 renamed 
the file (thanks Jeremy).

In D63607#1726978 , @peterwaller-arm 
wrote:

> In D63607#1726895 , @teemperor wrote:
>
> > Could we have the `clang/test/Driver/flang/flang.F90` and 
> > `clang/test/Driver/flang/flang.f90` files in different directories please? 
> > As macOS's FS is case-insensitive, those two files have the same path from 
> > macOS perspective which is causing a whole bunch of issues (including 
> > breaking git even with 'core.ignorecase').
>
>
> This was fixed by Jeremy Morse in 6c0a160c2d33e54aecf1538bf7c85d8da92051be 
>  - 
> thanks.
>
> I'm investigating the non-deterministic failure but don't have access to a 
> mac to test.
>
> @thakis is the flaky test resolved by 6c0a160c 
>  or is 
> it still outstanding?


Green dragon survived the change (because it doesn't run `git pull` I assume) 
and is running the tests soon-ish: 
http://green.lab.llvm.org/green/view/Clang/job/clang-stage1-cmake-RA-incremental/4685/console


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

https://reviews.llvm.org/D63607



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

[PATCH] D69602: [analyzer] Test case for lambda capture by value modelling

2019-10-30 Thread Endre Fülöp via Phabricator via cfe-commits

gamesh411 updated this revision to Diff 227090.
gamesh411 added a comment.

- Add capture by reference test
- Add another class to ensure reachability inside function call is independent 
from each other in the first 2 test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69602

Files:
  
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp


Index: 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
===
--- /dev/null
+++ 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection %s -verify
+
+// Handle constructors for lambda captures
+// Variables which are captured by value into a lambda require a call to a copy
+// constructor.
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+void reached_function_from_simple_copy() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+};
+
+void reached_function_from_lambda_capture() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+};
+
+struct incr_on_copy_for_simple_usage {
+  int &i;
+  incr_on_copy_for_simple_usage(int &i) : i(i) {}
+  incr_on_copy_for_simple_usage(const incr_on_copy_for_simple_usage &o)
+  : i(++o.i) {
+reached_function_from_simple_copy();
+  }
+  incr_on_copy_for_simple_usage &
+  operator=(const incr_on_copy_for_simple_usage &o) = delete;
+  incr_on_copy_for_simple_usage &
+  operator=(incr_on_copy_for_simple_usage &&o) = delete;
+  ~incr_on_copy_for_simple_usage() = default;
+};
+
+struct incr_on_copy_for_lambda_usage {
+  int &i;
+  incr_on_copy_for_lambda_usage(int &i) : i(i) {}
+  incr_on_copy_for_lambda_usage(const incr_on_copy_for_lambda_usage &o)
+  : i(++o.i) {
+reached_function_from_lambda_capture();
+  }
+  incr_on_copy_for_lambda_usage &
+  operator=(const incr_on_copy_for_lambda_usage &o) = delete;
+  incr_on_copy_for_lambda_usage &
+  operator=(incr_on_copy_for_lambda_usage &&o) = delete;
+  ~incr_on_copy_for_lambda_usage() = default;
+};
+
+void test_simple_copy() {
+  int a = 0;
+
+  incr_on_copy_for_simple_usage ioc(a);
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+
+  // Explicit copy constructor call.
+  incr_on_copy_for_simple_usage ioc2(ioc);
+
+  clang_analyzer_eval(ioc2.i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 1);  // expected-warning{{TRUE}}
+}
+
+void test_lambda_capture_by_value() {
+  int a = 0;
+
+  incr_on_copy_for_lambda_usage ioc(a);
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+
+  // Implicitly call copy constructor in case of capture-by-value.
+  [ioc]() {
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }();
+
+  clang_analyzer_eval(ioc.i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 1); // expected-warning{{TRUE}}
+}
+
+void test_lambda_capture_by_ref() {
+  int a = 0;
+
+  incr_on_copy_for_lambda_usage ioc(a);
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+
+  // Do not call copy constructor in case of capture-by-reference.
+  [&ioc]() {
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }();
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+};


Index: clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
===
--- /dev/null
+++ clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection %s -verify
+
+// Handle constructors for lambda captures
+// Variables which are captured by value into a lambda require a call to a copy
+// constructor.
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+void reached_function_from_simple_copy() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+};
+
+void reached_function_from_lambda_capture() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+};
+
+struct incr_on_copy_for_simple_usage {
+  int &i;
+  incr_on_copy_for_simple_usage(int &i) : i(i) {}
+  incr_on_copy_for_simple_usage(const incr_on_copy_for_simple_usage &o)
+  : i(++o.i) {
+reached_function_from_simple_copy();
+  }
+  incr_on_copy_for_simple_usage &
+  operator=(const incr_on_copy_for_simple_usage &o) = delete;
+

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

2019-10-30 Thread Michael Liao via Phabricator via cfe-commits

hliao marked 3 inline comments as done.
hliao added inline comments.



Comment at: clang/include/clang/AST/MangleNumberingContext.h:57-58
+
+  /// Has device mangle numbering context.
+  virtual bool hasDeviceMangleNumberingContext() const { return false; }
+

rnk wrote:
> It would be nicer if there were a single entry point that does the right 
> thing for all mangling contexts.
could you elaborate in more details?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:2090
 continue;
+  if (Next->isOneOf(tok::star, tok::arrow))
+continue;

sammccall wrote:
> MyDeveloperDay wrote:
> > sammccall wrote:
> > > This seems like it's supposed to be handled by the token annotator, and 
> > > the same cause will result in bugs in other places - why aren't these 
> > > tokens being marked as TT_OverloadedOperator?
> > > 
> > > I'd guess the deeper fix is in the `kw_operator` case in consumeToken(). 
> > > The way it's written looks suspect, though I'm not an expert on any of 
> > > this.
> > The * from *() is already labelled as a TT_PointerOrRefernce so can't also 
> > be marked as an Overloaded operator so use that in the loop.
> Right, the fact that it's incorrectly labeled as a PointerOrReference is a 
> bug, is it not possible to fix that instead of working around it here?
> 
> In the consumeToken loop, it seems the `*` in `operator *`  is labeled as 
> PointerOrReference by logic that's trying to handle e.g. `StringRef::operator 
> char*()`. However when the `*` is _immediately_ preceded by `operator`, it's 
> always an overloaded operator name rather than a pointer, I think?
Treating `*` and `->` as an TT_OverloadedOperator is probably ok, although this 
breaks unit test I highlight below, which I almost think is incorrect anyway.

I'm looking into a related issue https://bugs.llvm.org/show_bug.cgi?id=43783, 
which is changing this code again, I may roll both changes into the same fix

However this is made much more complex to deal with it just by using the 
`TT_OverloadedOperator` when the operator consists of 2 tokens say like `[` and 
`]` unless I combine them into a single token.

```
operator void*()
operator char*()
operator []()
```

hence the need for code to handle in this loop for handling.

```
operator new[]()
operator delete[]()
```



Comment at: clang/lib/Format/TokenAnnotator.cpp:955
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type = TT_OverloadedOperator;

sammccall wrote:
> I'm confused about this: ISTM that where we were previously always treating 
> star as a pointer, now we're always treating it as an operator name.
> Whereas it's sometimes an operator name (immediately after the `operator` 
> keyword) and sometimes a pointer (following a type name).
> 
> Maybe we should shift the OverloadedOperator labelling outside the loop 
> (since AFAICT it should only apply to the first token) and then keep the loop 
> to mark stars/amps elsewhere in the operator name as PointerOrReference?
Let me take another look..



Comment at: clang/unittests/Format/FormatTest.cpp:6989
   verifyFormat("operator int();");
-  verifyFormat("operator void *();");
+  verifyFormat("operator void*();");
   verifyFormat("operator SomeType();");

sammccall wrote:
> this looks like a regression at first glance, in LLVM style there's a space 
> between type and *
yes, I was a little concerned about this change too, it's hard to know if 
`operator void *` was intended as it falls off the bottom of the spacesBetween 
function with a return true; but if you are concerned then perhaps its better 
to change it back.



Comment at: clang/unittests/Format/FormatTest.cpp:6962
   verifyFormat("operator int();");
   verifyFormat("operator void *();");
   verifyFormat("operator SomeType();");

Treating * as TT_OverloadedOperator will break this test, which perhaps is 
already wrong!


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

https://reviews.llvm.org/D69573



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

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

2019-10-30 Thread Michael Liao via Phabricator via cfe-commits

hliao updated this revision to Diff 227091.
hliao added a comment.

revise `MSHIPNumberingContext`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/Mangle.h
  clang/include/clang/AST/MangleNumberingContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/CXXABI.h
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGenCUDA/unnamed-types.cu

Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- clang/test/CodeGenCUDA/unnamed-types.cu
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -1,12 +1,17 @@
 // RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-linux-gnu -aux-triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s --check-prefix=HOST
+// RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-pc-windows-msvc -aux-triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s --check-prefix=MSVC
 // RUN: %clang_cc1 -std=c++11 -x hip -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm %s -o - | FileCheck %s --check-prefix=DEVICE
 
 #include "Inputs/cuda.h"
 
 // HOST: @0 = private unnamed_addr constant [43 x i8] c"_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_\00", align 1
+// HOST: @1 = private unnamed_addr constant [60 x i8] c"_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_\00", align 1
+// Check that, on MSVC, the same device kernel mangling name is generated.
+// MSVC: @0 = private unnamed_addr constant [43 x i8] c"_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_\00", align 1
+// MSVC: @1 = private unnamed_addr constant [60 x i8] c"_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_\00", align 1
 
 __device__ float d0(float x) {
-  return [](float x) { return x + 2.f; }(x);
+  return [](float x) { return x + 1.f; }(x);
 }
 
 __device__ float d1(float x) {
@@ -14,11 +19,21 @@
 }
 
 // DEVICE: amdgpu_kernel void @_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_(
+// DEVICE: define internal float @_ZZZ2f1PfENKUlS_E_clES_ENKUlfE_clEf(
 template 
 __global__ void k0(float *p, F f) {
   p[0] = f(p[0]) + d0(p[1]) + d1(p[2]);
 }
 
+// DEVICE: amdgpu_kernel void @_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_(
+// DEVICE: define internal float @_ZZ2f1PfENKUlfE_clEf(
+// DEVICE: define internal float @_ZZ2f1PfENKUlffE_clEff(
+// DEVICE: define internal float @_ZZ2f1PfENKUlfE0_clEf(
+template 
+__global__ void k1(float *p, F0 f0, F1 f1, F2 f2) {
+  p[0] = f0(p[0]) + f1(p[1], p[2]) + f2(p[3]);
+}
+
 void f0(float *p) {
   [](float *p) {
 *p = 1.f;
@@ -29,11 +44,17 @@
 // linkages are still required to keep the original `internal` linkage.
 
 // HOST: define internal void @_ZZ2f1PfENKUlS_E_clES_(
-// DEVICE: define internal float @_ZZZ2f1PfENKUlS_E_clES_ENKUlfE_clEf(
 void f1(float *p) {
   [](float *p) {
-k0<<<1,1>>>(p, [] __device__ (float x) { return x + 1.f; });
+k0<<<1,1>>>(p, [] __device__ (float x) { return x + 3.f; });
   }(p);
+  k1<<<1,1>>>(p,
+  [] __device__ (float x) { return x + 4.f; },
+  [] __device__ (float x, float y) { return x * y; },
+  [] __device__ (float x) { return x + 5.f; });
 }
 // HOST: @__hip_register_globals
 // HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
+// HOST: __hipRegisterFunction{{.*}}@_Z2k1IZ2f1PfEUlfE_Z2f1S0_EUlffE_Z2f1S0_EUlfE0_EvS0_T_T0_T1_{{.*}}@1
+// MSVC: __hipRegisterFunction{{.*}}@"??$k0@V@?0???R1?0??f1@@YAXPEAM@Z@QEBA@0@Z@@@YAXPEAMV@?0???R0?0??f1@@YAX0@Z@QEBA@0@Z@@Z{{.*}}@0
+// MSVC: __hipRegisterFunction{{.*}}@"??$k1@V@?0??f1@@YAXPEAM@Z@V@?0??2@YAX0@Z@V@?0??2@YAX0@Z@@@YAXPEAMV@?0??f1@@YAX0@Z@V@?0??1@YAX0@Z@V@?0??1@YAX0@Z@@Z{{.*}}@1
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6226,6 +6226,7 @@
 Record->push_back(Lambda.NumExplicitCaptures);
 Record->push_back(Lambda.HasKnownInternalLinkage);
 Record->push_back(Lambda.ManglingNumber);
+Record->push_back(Lambda.DeviceManglingNumber);
 AddDeclRef(D->getLambdaContextDecl());
 AddTypeSourceInfo(Lambda.MethodTyInfo);
 for (unsigned I = 0, N = Lambda.NumCaptures; I != N; ++I) {
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1692,6 +1692,7 @@
 Lambda.NumExplicitCaptures = Record.readInt();
 Lambda.HasKnownInternalLinkage = Record.readInt();
 Lambda.ManglingNumber = Record.

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm added a comment.

Thanks for chiming in.

> Sorry, I had missed the RfC, but it looks like there wasn't a lot of 
> discussion on it anyways.

Apologies @thakis - did I jump the gun? And if so, what could I have done 
differently?

> Adding fortran support to clang's driver has been suggested and decided 
> against before, see "[cfe-commits] [RFC and PATCH] Fortran"

Interesting, I had missed that conversation! Google and DDG fail to find it for 
me even quoting sentences out of it. Found it by wgetting the gzip mailing list 
archives and grepping them - is there a better way? :)

That conversation was in 2012, and since then flang has been accepted as a 
subproject to LLVM in "[llvm-dev] f18 is accepted as part of LLVM project! 
". Since the 
acceptance, the project has renamed to flang, and is in the process of actively 
being integrated into the llvm project.

As discussed in the RFC, I understand that libclangDriver was always intended 
to be a flexible driver used by other frontends, so in light of that, does the 
change still seem unreasonable? My expectation is that the footprint of fortran 
on libclangDriver should only be quite modest.


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

https://reviews.llvm.org/D63607



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

[clang] 8aa7441 - [OPENMP][DOC]Update list of supported functions, NFC.

2019-10-30 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2019-10-30T10:33:04-04:00
New Revision: 8aa74414bfb88e8745d5277cb7172efb67202099

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

LOG: [OPENMP][DOC]Update list of supported functions, NFC.

Added support for parallel master taskloop simd construct.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index cbb4888c0731..ab95501a5057 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -58,7 +58,7 @@ General improvements
   value of threads or as specified by the `thread_limit` clause if
   present). For the `for` construct, the schedule is static with chunk
   size of one.
-  
+
 - Simplified SPMD code generation for `distribute parallel for` when
   the new default schedules are applicable.
 
@@ -185,7 +185,7 @@ implementation.
 
+--+--+--+---+
 | task extension   | master taskloop simd  
   | :none:`done` | 
  |
 
+--+--+--+---+
-| task extension   | parallel master taskloop simd 
   | :none:`unclaimed`| 
  |
+| task extension   | parallel master taskloop simd 
   | :none:`done` | 
  |
 
+--+--+--+---+
 | SIMD extension   | atomic and critical constructs inside SIMD 
code  | :none:`unclaimed`|  
 |
 
+--+--+--+---+



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

2019-10-30 Thread Balázs Kéri via Phabricator via cfe-commits

balazske updated this revision to Diff 227093.
balazske added a comment.

- Redesign again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream.c
  clang/test/Analysis/stream.cpp

Index: clang/test/Analysis/stream.cpp
===
--- /dev/null
+++ clang/test/Analysis/stream.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
+
+typedef struct _IO_FILE FILE;
+extern FILE *fopen(const char *path, const char *mode);
+
+struct X {
+  int A;
+  int B;
+};
+
+void *fopen(X x, const char *mode) {
+  return new char[4];
+}
+
+void f1() {
+  X X1;
+  void *p = fopen(X1, "oo");
+} // no-warning
+
+void f2() {
+  FILE *f = fopen("file", "r");
+} // expected-warning {{Opened File never closed. Potential Resource leak}}
Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
 
 typedef __typeof__(sizeof(int)) size_t;
+typedef __typeof__(sizeof(int)) fpos_t;
 typedef struct _IO_FILE FILE;
 #define SEEK_SET	0	/* Seek from beginning of file.  */
 #define SEEK_CUR	1	/* Seek from current position.  */
@@ -9,36 +10,93 @@
 extern FILE *tmpfile(void);
 extern int fclose(FILE *fp);
 extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
 extern int fseek (FILE *__stream, long int __off, int __whence);
 extern long int ftell (FILE *__stream);
 extern void rewind (FILE *__stream);
+extern int fgetpos(FILE *stream, fpos_t *pos);
+extern int fsetpos(FILE *stream, const fpos_t *pos);
+extern void clearerr(FILE *stream);
+extern int feof(FILE *stream);
+extern int ferror(FILE *stream);
+extern int fileno(FILE *stream);
 
-void f1(void) {
-  FILE *p = fopen("foo", "r");
-  char buf[1024];
-  fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
+void check_fread() {
+  FILE *fp = tmpfile();
+  fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
 }
 
-void f2(void) {
-  FILE *p = fopen("foo", "r");
-  fseek(p, 1, SEEK_SET); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
+void check_fwrite() {
+  FILE *fp = tmpfile();
+  fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
 }
 
-void f3(void) {
-  FILE *p = fopen("foo", "r");
-  ftell(p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
+void check_fseek() {
+  FILE *fp = tmpfile();
+  fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_ftell() {
+  FILE *fp = tmpfile();
+  ftell(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_rewind() {
+  FILE *fp = tmpfile();
+  rewind(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_fgetpos() {
+  FILE *fp = tmpfile();
+  fpos_t pos;
+  fgetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_fsetpos() {
+  FILE *fp = tmpfile();
+  fpos_t pos;
+  fsetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_clearerr() {
+  FILE *fp = tmpfile();
+  clearerr(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_feof() {
+  FILE *fp = tmpfile();
+  feof(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_ferror() {
+  FILE *fp = tmpfile();
+  ferror(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
 }
 
-void f4(void) {
+void check_fileno() {
+  FILE *fp = tmpfile();
+  fileno(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void f_open(void) {
   FILE *p = fopen("foo", "r");
-  rewind(p); // expected-warning {{Stream pointer might be NULL}}
+  char buf[1024];
+  fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
   fclose(p);
 }
 
-void f5(void) {
+void f_seek(void) {
   FILE *p = fopen("foo", "r");
   if (!p)
 return;
@@ -47,26 +105,20 @@
   fclose(p);
 }
 
-void f6(void) {
+void f_double_close(void) {
   FILE *p = fopen("foo", "r");
   fclose(p); 
   fclose(p); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
 }
 
-void f7(void) {
-  FILE *p = tmpfile();
-  ftell(p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
-}
-
-void f8(int c) {
+void f_leak(int c) {
   FILE *p = fopen("foo.c", "r");
   if(c)
 return; // expected-warning {{Opened File never close

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

2019-10-30 Thread Sam McCall via Phabricator via cfe-commits

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

LG, just some style nits, thanks!




Comment at: clang-tools-extra/clangd/XRefs.cpp:226
 
-  // Emit all symbol locations (declaration or definition) from AST.
-  for (const Decl *D : getDeclAtPosition(AST, SourceLocationBeg)) {
-auto Loc =
-makeLocation(AST.getASTContext(), spellingLocIfSpelled(findName(D), 
SM),
- *MainFilePath);
-if (!Loc)
-  continue;
-
-Result.emplace_back();
-if (auto *ND = dyn_cast(D))
-  Result.back().Name = printName(AST.getASTContext(), *ND);
-Result.back().PreferredDeclaration = *Loc;
-// DeclInfo.D is always a definition if possible, so this check works.
-if (getDefinition(D) == D)
-  Result.back().Definition = *Loc;
-
-// Record SymbolID for index lookup later.
-if (auto ID = getSymbolID(D))
-  ResultIndex[*ID] = Result.size() - 1;
+  // For getDeclAtPosition(), use the actual input location, not the location
+  // of the beginning of the identifier, because SelectionTree may select

rather than justifying *not* doing something here, can we make it clearer that 
macros are the special case?
maybe move the "SourceLocationBeg" initialization under the "macros are simple" 
comment, and rename it to "MaybeMacroLocation" or so.



Comment at: clang-tools-extra/clangd/XRefs.cpp:233
+  } else {
+log("locateSymbolAt: {0}", L.takeError());
+  }

log should have more context, and be an elog instead.
Return afterward?



Comment at: clang-tools-extra/clangd/XRefs.cpp:236
+
+  // If we found a macro, don't bother calling getDeclAtPosition(). It would
+  // just return declarations referenced from the macro's expansion.

I don't really understand this comment.

If the intention is just not to look at the AST or index when we have a macro 
(which seems reasonable to me), then just `return Result` instead of `HaveMacro 
= true` above?



Comment at: clang-tools-extra/clangd/XRefs.cpp:896
   // TODO: should we handle macros, too?
-  auto Decls = getDeclAtPosition(AST, Loc);
+  // We also want the targets of using-decls, so we include
+  // DeclRelation::Underlying.

want -> show references to



Comment at: clang-tools-extra/clangd/XRefs.cpp:1133
+  DeclRelationSet Relations =
+  DeclRelation::TemplatePattern | DeclRelation::Alias;
+  auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);

Are you sure you don't want type hierarchy to work on aliases to record types? 
(i.e. specify underlying rather than alias)



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:526
+
+  // Test constructor targeting. This currently works if the cursor
+  // is just before the opening parenthesis.

I'd suggest dropping these two lines of comment. The fact that it's testing 
constructors is clear from the range name. The FIXME comment describes the 
future change.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:528
+  // is just before the opening parenthesis.
+  // FIXME: Ideally, we would like this to work for the whole
+  // identifier, and in cases where there is no parenthesis (e.g. case #7).

This could just be `// FIXME: target constructor instead of class.`. 
We already have a test, so the comment belongs above point("9") instead of here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237



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

[clang] 37c8baf - [OPENMP][DOC]Provide correct info about supported features, NFC.

2019-10-30 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2019-10-30T10:38:11-04:00
New Revision: 37c8baf821e8cb66a3d49e7b570970b2606983b2

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

LOG: [OPENMP][DOC]Provide correct info about supported features, NFC.

Removed the explicit list of supported features from OpenMP 5.0 and used
the reference to the table instead. Also, fixed info about constructs
that can be executed in SPMD mode, if and num_threads clauses do not
affect it anymore.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index ab95501a5057..5e88276794a6 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -17,23 +17,6 @@
 OpenMP Support
 ==
 
-Clang supports the following OpenMP 5.0 features (see also `OpenMP 
implementation details`_):
-
-* The `reduction`-based clauses in the `task` and `target`-based directives.
-
-* Support relational-op != (not-equal) as one of the canonical forms of random
-  access iterator.
-
-* Support for mapping of the lambdas in target regions.
-
-* Parsing/sema analysis for the requires directive.
-
-* Nested declare target directives.
-
-* Make the `this` pointer implicitly mapped as `map(this[:1])`.
-
-* The `close` *map-type-modifier*.
-
 Clang fully supports OpenMP 4.5. Clang supports offloading to X86_64, AArch64,
 PPC64[LE] and has `basic support for Cuda devices`_.
 
@@ -44,6 +27,8 @@ PPC64[LE] and has `basic support for Cuda devices`_.
 In addition, the LLVM OpenMP runtime `libomp` supports the OpenMP Tools
 Interface (OMPT) on x86, x86_64, AArch64, and PPC64 on Linux, Windows, and 
macOS.
 
+For the list of supported features from OpenMP 5.0 see `OpenMP implementation 
details`_.
+
 General improvements
 
 - New collapse clause scheme to avoid expensive remainder operations.
@@ -78,12 +63,6 @@ supporting some OpenMP features. The non-SPMD mode is the 
most generic mode and
 supports all currently available OpenMP features. The compiler will always
 attempt to use the SPMD mode wherever possible. SPMD mode will not be used if:
 
-   - The target region contains an `if()` clause that refers to a `parallel`
- directive.
-
-   - The target region contains a `parallel` directive with a `num_threads()`
- clause.
-
- The target region contains user code (other than OpenMP-specific
  directives) in between the `target` and the `parallel` directives.
 



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

[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

2019-10-30 Thread Anna Welker via Phabricator via cfe-commits

anwel updated this revision to Diff 227094.
anwel marked 9 inline comments as done.
anwel added a comment.

Rebase and make some variables const


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

https://reviews.llvm.org/D68862

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Driver/arm-reserved-reg-options.c
  clang/test/Sema/arm-global-regs.c
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMAsmPrinter.cpp
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMFrameLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMSubtarget.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/ARMTargetTransformInfo.h
  llvm/test/CodeGen/ARM/reg-alloc-fixed-r6-vla.ll
  llvm/test/CodeGen/ARM/reg-alloc-with-fixed-reg-r6-modified.ll
  llvm/test/CodeGen/ARM/reg-alloc-with-fixed-reg-r6.ll
  llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll
  llvm/test/CodeGen/Thumb/callee_save_reserved.ll
  llvm/test/Feature/reserve_global_reg.ll

Index: llvm/test/Feature/reserve_global_reg.ll
===
--- /dev/null
+++ llvm/test/Feature/reserve_global_reg.ll
@@ -0,0 +1,29 @@
+; RUN: not llc < %s -mtriple=thumbv7-apple-darwin -mattr=+reserve-r7 -o - 2>&1 | FileCheck -check-prefix=CHECK-RESERVE-FP7 %s
+; RUN: not llc < %s -mtriple=armv7-windows-msvc -mattr=+reserve-r11 -o - 2>&1 | FileCheck -check-prefix=CHECK-RESERVE-FP11 %s
+; RUN: not llc < %s -mtriple=thumbv7-windows -mattr=+reserve-r11 -o - 2>&1 | FileCheck -check-prefix=CHECK-RESERVE-FP11-2 %s
+
+; int test(int a, int b, int c) {
+;   return a + b + c;
+; }
+
+; Function Attrs: noinline nounwind optnone
+define hidden i32 @_Z4testiii(i32 %a, i32 %b, i32 %c) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b.addr = alloca i32, align 4
+  %c.addr = alloca i32, align 4
+  store i32 %a, i32* %a.addr, align 4
+  store i32 %b, i32* %b.addr, align 4
+  store i32 %c, i32* %c.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %1 = load i32, i32* %b.addr, align 4
+  %add = add nsw i32 %0, %1
+  %2 = load i32, i32* %c.addr, align 4
+  %add1 = add nsw i32 %add, %2
+  ret i32 %add1
+}
+
+; CHECK-RESERVE-FP7: Register r7 has been specified but is used as a frame pointer on this target.
+; CHECK-RESERVE-FP11: Register r11 has been specified but is used as a frame pointer on this target.
+; CHECK-RESERVE-FP11-2: Register r11 has been specified but is used as a frame pointer on this target.
+
Index: llvm/test/CodeGen/Thumb/callee_save_reserved.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb/callee_save_reserved.ll
@@ -0,0 +1,15 @@
+; RUN: llc < %s -mtriple=thumbv6m-none-eabi -verify-machineinstrs -frame-pointer=none -mattr=+reserve-r6,+reserve-r8 \
+; RUN: -asm-verbose=false | FileCheck --check-prefix=CHECK-INVALID %s
+
+; Reserved low registers should not be used to correct reg deficit.
+define <4 x i32> @four_high_four_return_reserved() {
+entry:
+  ; CHECK-INVALID-NOT: r{{6|8}}
+  tail call void asm sideeffect "", "~{r8},~{r9}"()
+  %vecinit = insertelement <4 x i32> undef, i32 1, i32 0
+  %vecinit11 = insertelement <4 x i32> %vecinit, i32 2, i32 1
+  %vecinit12 = insertelement <4 x i32> %vecinit11, i32 3, i32 2
+  %vecinit13 = insertelement <4 x i32> %vecinit12, i32 4, i32 3
+  ret <4 x i32> %vecinit13
+}
+
Index: llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll
@@ -0,0 +1,58 @@
+; RUN: llc < %s -mtriple=arm-linux-gnueabi  -O0 -filetype=asm --regalloc=fast 2>&1 | FileCheck %s
+;
+; Equivalent C source code
+; void bar(unsigned int i,
+;  unsigned int j,
+;  unsigned int k,
+;  unsigned int l,
+;  unsigned int m,
+;  unsigned int n,
+;  unsigned int o,
+;  unsigned int p)
+; {
+; unsigned int result = i + j + k + l + m + n + o + p;
+; }
+
+define void @bar(i32 %i, i32 %j, i32 %k, i32 %l, i32 %m, i32 %n, i32 %o, i32 %p) nounwind {
+entry:
+; CHECK: push {{{.*}}r4, r5{{.*}}}
+  %i.addr = alloca i32, align 4
+  %j.addr = alloca i32, align 4
+  %k.addr = alloca i32, align 4
+  %l.addr = alloca i32, align 4
+  %m.addr = alloca i32, align 4
+  %n.addr = alloca i32, align 4
+  %o.addr = alloca i32, align 4
+  %p.addr = alloca i32, align 4
+  %result = alloca i32, align 4
+  store i32 %i, i32* %i.addr, align 4
+  store i32 %j, i32* %j.addr, align 4
+  store i32 %k, i32* %k.addr, align 4
+  store i32 %l, i32* %l.addr, align 4
+

[PATCH] D69498: IR: Invert convergent attribute handling

2019-10-30 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

In D69498#1726610 , @mehdi_amini wrote:

> > The short version is the fact that most developers aren't aware of and 
> > don't understand the subtleties of convergence, and making sure the 
> > front-end does something in all contexts requires a lot of diligence. It's 
> > very easy to introduce these difficult to debug bugs when calls are broken 
> > by default.
>
> The fact that most developers aren't aware of convergence means a lot of 
> potential bugs in LLVM because passes and transformations won't honor the 
> convergent semantics, but the default for the attribute won't change that.

While you are right, a default non-convergent behavior will not make all bug 
sources disappear, it will make *a lot of bug sources disappear*. Let me quote 
@arsenm here because this is so important: "Basically no frontend has gotten 
this right, including clang and non-clang frontends." For me, that statement 
alone is reason to change the status quo.

> As of the frontend, it seems to me that this is just about structuring the 
> code-path for function emission to define the right set of default attribute. 
> It isn't clear to me why a refactoring of the frontend isn't a better course 
> of actions here.

Whatever we do, there will be consequences for current and future front-ends. 
At the end of the day, the underlying question we need to answer here is: Do we 
want to have "optimization with opt-in soundness" or "soundness with opt-in 
optimizations", and I would choose the latter any time.

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

https://reviews.llvm.org/D69498

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

[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

2019-10-30 Thread Anna Welker via Phabricator via cfe-commits

anwel added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:903-907
+  for (std::string &Feature : Features) {
+if (Feature.compare(SearchFeature) == 0)
+  return true;
+  }
+  return false;

chill wrote:
> This explicit loop can be written like:
> ```
> return llvm::any_of(getTargetOpts().Features(),
>[&](auto &P) { return P == SearchFeature; });
> ```
> 
I see your point, but using a for loop seems to better match the style of the 
code.



Comment at: llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll:3
+;
+; Equivalent C source code
+; void bar(unsigned int i,

SjoerdMeijer wrote:
> As all these tests (this file and the ones above) are the same, the 
> "equivalent C source code" is the same, perhaps move all these cases into 1 
> file.
I see your point, but I'd still prefer to leave them as they are because in my 
opinion having the test cases separated into individual files it's much easier 
to grasp what they are doing.


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

https://reviews.llvm.org/D68862



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

[PATCH] D69602: [analyzer] Test case for lambda capture by value modelling

2019-10-30 Thread Kristóf Umann via Phabricator via cfe-commits

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

LGTM, can we remove the open projects entry under the same breath?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69602



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

2019-10-30 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

Its becoming a bit difficult to navigate inlines, could you please mark them as 
done as you address them?




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:61
+  using FnCheck = bool (StreamChecker::*)(const CallEvent &,
+  CheckerContext &) const;
+

Charusso wrote:
> I prefer `std::function`, because it is modern.
> ```
>   using StreamCheck = 
>   std::function  CheckerContext &)>;
> ```
> I think it is fine with pointers, but at some point we need to modernize this.
But its also a lot more expensive. 
https://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/

`std::function` is able to wrap lambdas with different captures and all sorts 
of things like that, which comes at a cost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[PATCH] D69602: [analyzer] Test case for lambda capture by value modelling

2019-10-30 Thread Endre Fülöp via Phabricator via cfe-commits

gamesh411 updated this revision to Diff 227096.
gamesh411 added a comment.

- Remove link reference from commit message
- Remove superfluous semicolons left in the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69602

Files:
  
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp


Index: 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
===
--- /dev/null
+++ 
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection %s -verify
+
+// Handle constructors for lambda captures
+// Variables which are captured by value into a lambda require a call to a copy
+// constructor.
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+void reached_function_from_simple_copy() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void reached_function_from_lambda_capture() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+struct incr_on_copy_for_simple_usage {
+  int &i;
+  incr_on_copy_for_simple_usage(int &i) : i(i) {}
+  incr_on_copy_for_simple_usage(const incr_on_copy_for_simple_usage &o)
+  : i(++o.i) {
+reached_function_from_simple_copy();
+  }
+  incr_on_copy_for_simple_usage &
+  operator=(const incr_on_copy_for_simple_usage &o) = delete;
+  incr_on_copy_for_simple_usage &
+  operator=(incr_on_copy_for_simple_usage &&o) = delete;
+  ~incr_on_copy_for_simple_usage() = default;
+};
+
+struct incr_on_copy_for_lambda_usage {
+  int &i;
+  incr_on_copy_for_lambda_usage(int &i) : i(i) {}
+  incr_on_copy_for_lambda_usage(const incr_on_copy_for_lambda_usage &o)
+  : i(++o.i) {
+reached_function_from_lambda_capture();
+  }
+  incr_on_copy_for_lambda_usage &
+  operator=(const incr_on_copy_for_lambda_usage &o) = delete;
+  incr_on_copy_for_lambda_usage &
+  operator=(incr_on_copy_for_lambda_usage &&o) = delete;
+  ~incr_on_copy_for_lambda_usage() = default;
+};
+
+void test_simple_copy() {
+  int a = 0;
+
+  incr_on_copy_for_simple_usage ioc(a);
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+
+  // Explicit copy constructor call.
+  incr_on_copy_for_simple_usage ioc2(ioc);
+
+  clang_analyzer_eval(ioc2.i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 1);  // expected-warning{{TRUE}}
+}
+
+void test_lambda_capture_by_value() {
+  int a = 0;
+
+  incr_on_copy_for_lambda_usage ioc(a);
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+
+  // Implicitly call copy constructor in case of capture-by-value.
+  [ioc]() {
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }();
+
+  clang_analyzer_eval(ioc.i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 1); // expected-warning{{TRUE}}
+}
+
+void test_lambda_capture_by_ref() {
+  int a = 0;
+
+  incr_on_copy_for_lambda_usage ioc(a);
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+
+  // Do not call copy constructor in case of capture-by-reference.
+  [&ioc]() {
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }();
+
+  clang_analyzer_eval(ioc.i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == 0); // expected-warning{{TRUE}}
+}


Index: clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
===
--- /dev/null
+++ clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_lambda_captures.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection %s -verify
+
+// Handle constructors for lambda captures
+// Variables which are captured by value into a lambda require a call to a copy
+// constructor.
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+void reached_function_from_simple_copy() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void reached_function_from_lambda_capture() {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+struct incr_on_copy_for_simple_usage {
+  int &i;
+  incr_on_copy_for_simple_usage(int &i) : i(i) {}
+  incr_on_copy_for_simple_usage(const incr_on_copy_for_simple_usage &o)
+  : i(++o.i) {
+reached_function_from_simple_copy();
+  }
+  incr_on_copy_for_simple_usage &
+  operator=(const incr_on_copy_for_simple_usage &o) = delete;
+  incr_on_copy_for_simple_usage &
+  operator=(incr_on_copy_for_sim

[PATCH] D69308: [analyzer] Test cases for the unsupported features for Clang Static Analyzer

2019-10-30 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

LGTM given that the inlines are fixed.

In D69308#1722139 , @NoQ wrote:

> Thanks for the tests!
>
> Both of these features are relatively hard.
>
> ...


Would love to see this comment in its entirety on the open projects page :^)


Repository:
  rC Clang

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

https://reviews.llvm.org/D69308



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

[PATCH] D65912: [clang-tidy] Add new check for math constants

2019-10-30 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus resigned from this revision.
Szelethus added a comment.

I have no authority in clang-tidy, but the idea is nice! You may wanna reupload 
with full context though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65912



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

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:55
+  // Include template parameter list.
+  if (auto *FTD = FD->getDescribedFunctionTemplate())
+return FTD->getBeginLoc();

hokein wrote:
> Could you confirm the code handle template specializations as well? I think 
> `getDescribedFunctionTemplate` will return null when FD is a specialization, 
> and we still miss the template list.
added a unittest


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298



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

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 227108.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1644,6 +1644,45 @@
 };)cpp");
 }
 
+TEST_F(DefineOutlineTest, ApplyTest) {
+  FileName = "Test.hpp";
+
+  // No implementation file.
+  EXPECT_EQ(apply("void fo^o() { return; }"),
+"fail: Couldn't find a suitable implementation file.");
+
+  llvm::StringMap EditedFiles;
+  ExtraFiles["Test.cpp"] = "";
+
+  EXPECT_EQ(apply("void fo^o() { return; }", &EditedFiles), "void foo() ;");
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("Test.cpp"),
+"void foo() { return; }")));
+
+  EditedFiles.clear();
+  // Templated function.
+  EXPECT_EQ(apply("template  void fo^o(T, T x) { return; }",
+  &EditedFiles),
+"template  void foo(T, T x) ;");
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("Test.cpp"),
+  "template  void foo(T, T x) { return; }")));
+
+  EditedFiles.clear();
+  // Template specialization.
+  EXPECT_EQ(apply(R"cpp(
+template  void foo();
+template <> void fo^o() { return; })cpp",
+  &EditedFiles),
+R"cpp(
+template  void foo();
+template <> void foo() ;)cpp");
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("Test.cpp"),
+   "template <> void foo() { return; }")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -9,12 +9,21 @@
 #include "HeaderSourceSwitch.h"
 #include "Path.h"
 #include "Selection.h"
+#include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Driver/Types.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/Support/Path.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -39,6 +48,60 @@
   return nullptr;
 }
 
+// Returns the begining location for a FunctionDecl. Returns location of
+// template keyword for templated functions.
+// FIXME: This is shared with define inline, move them to a common header once
+// we have a place for such.
+const SourceLocation getBeginLoc(const FunctionDecl *FD) {
+  // Include template parameter list.
+  if (auto *FTD = FD->getDescribedFunctionTemplate())
+return FTD->getBeginLoc();
+  return FD->getBeginLoc();
+}
+
+llvm::Optional getSourceFile(llvm::StringRef FileName,
+   const Tweak::Selection &Sel) {
+  if (auto Source = getCorrespondingHeaderOrSource(
+  FileName,
+  &Sel.AST.getSourceManager().getFileManager().getVirtualFileSystem()))
+return *Source;
+  return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
+}
+
+// Creates a modified version of function definition that can be inserted at a
+// different location. Contains both function signature and body.
+llvm::Optional getFunctionSourceCode(const FunctionDecl *FD) {
+  auto &SM = FD->getASTContext().getSourceManager();
+  auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
+   FD->getSourceRange());
+  if (!CharRange)
+return llvm::None;
+  CharRange->setBegin(getBeginLoc(FD));
+
+  // FIXME: Qualify return type.
+  // FIXME: Qualify function name depending on the target context.
+  return toSourceCode(SM, *CharRange);
+}
+
+// Returns the most natural insertion point for \p QualifiedName in \p Contents.
+// This currently cares about only the namespace proximity, but in feature it
+// should also try to follow ordering of declarations. For example, if decls
+// come in order `foo, bar, baz` then this function should return some point
+// between foo and baz for inserting bar.
+llvm::Expected

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:2626
+  Left.Previous && Left.Previous->is(tok::kw_operator))
+// No space between the type and the *
+// operator void*(), operator char*(), operator Foo*().

sammccall wrote:
> why?
LLVM style and google style is different based on pointer alignment



Comment at: clang/unittests/Format/FormatTest.cpp:6989
   verifyFormat("operator int();");
-  verifyFormat("operator void *();");
+  verifyFormat("operator void*();");
   verifyFormat("operator SomeType();");

MyDeveloperDay wrote:
> sammccall wrote:
> > this looks like a regression at first glance, in LLVM style there's a space 
> > between type and *
> yes, I was a little concerned about this change too, it's hard to know if 
> `operator void *` was intended as it falls off the bottom of the 
> spacesBetween function with a return true; but if you are concerned then 
> perhaps its better to change it back.
reverted


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

https://reviews.llvm.org/D69573



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

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

Remove test regression


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

https://reviews.llvm.org/D69573

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6111,7 +6111,40 @@
"void\n"
"A::operator>>() {}\n"
"void\n"
-   "A::operator+() {}\n",
+   "A::operator+() {}\n"
+   "void\n"
+   "A::operator*() {}\n"
+   "void\n"
+   "A::operator->() {}\n"
+   "void\n"
+   "A::operator void *() {}\n"
+   "void\n"
+   "A::operator char *() {}\n"
+   "void\n"
+   "A::operator[]() {}\n"
+   "void\n"
+   "A::operator!() {}\n",
+   Style);
+  verifyFormat("constexpr auto\n"
+   "operator()() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator>>() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator+() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator*() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator->() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator++() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator void *() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator char *() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator!() const -> reference {}\n"
+   "constexpr auto\n"
+   "operator[]() const -> reference {}\n",
Style);
   verifyFormat("void *operator new(std::size_t s);", // No break here.
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -952,7 +952,7 @@
 consumeToken();
 if (CurrentToken &&
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type = TT_OverloadedOperator;
   }
   if (CurrentToken) {
@@ -1344,8 +1344,12 @@
 Contexts.back().IsExpression = false;
 } else if (Current.is(tok::kw_new)) {
   Contexts.back().CanBeExpression = false;
-} else if (Current.isOneOf(tok::semi, tok::exclaim)) {
+} else if (Current.is(tok::semi) ||
+   (Current.is(tok::exclaim) && Current.Previous &&
+!Current.Previous->is(tok::kw_operator))) {
   // This should be the condition or increment in a for-loop.
+  // But not operator !() (can't use TT_OverloadedOperator here as its not
+  // been annotated yet).
   Contexts.back().IsExpression = true;
 }
   }
@@ -2087,11 +2091,22 @@
 continue;
   if (Next->isOneOf(tok::kw_new, tok::kw_delete)) {
 // For 'new[]' and 'delete[]'.
-if (Next->Next && Next->Next->is(tok::l_square) && Next->Next->Next &&
-Next->Next->Next->is(tok::r_square))
+if (Next->Next &&
+Next->Next->startsSequence(tok::l_square, tok::r_square))
   Next = Next->Next->Next;
 continue;
   }
+  if (Next->startsSequence(tok::l_square, tok::r_square)) {
+// For operator[]().
+Next = Next->Next;
+continue;
+  }
+  if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) &&
+  Next->Next && Next->Next->is(tok::star)) {
+// For operator void*(), operator char*(), operator Foo*().
+Next = Next->Next;
+continue;
+  }
 
   break;
 }
@@ -2605,6 +2620,12 @@
 tok::l_square));
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
+  if (Right.is(tok::star) &&
+  (Left.is(tok::identifier) || Left.isSimpleTypeSpecifier()) &&
+  Left.Previous && Left.Previous->is(tok::kw_operator))
+// No space between the type and the *
+// operator void*(), operator char*(), operator Foo*().
+return (Style.PointerAlignment == FormatStyle::PAS_Right);
   const auto SpaceRequiredForArrayInitializerLSquare =
   [](const FormatToken &LSquareTok, const FormatStyle &Style) {
 return Style.SpacesInContainerLiterals ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

2019-10-30 Thread Balázs Kéri via Phabricator via cfe-commits

balazske marked 22 inline comments as done and an inline comment as not done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:90-106
+  FnCheck identifyCall(const CallEvent &Call, CheckerContext &C,
+   const CallExpr *CE) const;
+
+  void evalFopen(CheckerContext &C, const CallExpr *CE) const;
+  void evalTmpfile(CheckerContext &C, const CallExpr *CE) const;
+  void evalFclose(CheckerContext &C, const CallExpr *CE) const;
+  void evalFread(CheckerContext &C, const CallExpr *CE) const;

NoQ wrote:
> balazske wrote:
> > NoQ wrote:
> > > For most purposes it's more convenient to pass `CallEvent` around. The 
> > > only moment when you actually need the `CallExpr` is when you're doing 
> > > the binding for the return value, which happens once, so it's fine to 
> > > call `.getOriginExpr()` directly at this point.
> > The CallExpr is used at many places to get arguments and other data. There 
> > is a `CallEvent::getArgSVal` that can be used instead but at some places 
> > CallExpr is used in other ways. I do not see much benefit of change the 
> > passed `CallExpr` to `CallEvent`.
> > at some places CallExpr is used in other ways
> 
> Can we add more convenient getters to `CallEvent` to help with those? 'Cause 
> `CallEvent` has strictly more information than `CallExpr` (combining 
> syntactic information with path-sensitive information), it's all about 
> convenience.
> 
> I also see that `CallExpr` is stored in `StreamState` (which is something you 
> can't do with a `CallEvent` because it's short-lived) but i suspect that it's 
> actually never read from there and i doubt that it was the right solution 
> anyway. I.e., no other checker needs that and i don't have a reason to 
> believe that `StreamChecker` is special.
In the new code `CallEvent` is passed.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:61
+  using FnCheck = bool (StreamChecker::*)(const CallEvent &,
+  CheckerContext &) const;
+

Szelethus wrote:
> Charusso wrote:
> > I prefer `std::function`, because it is modern.
> > ```
> >   using StreamCheck = 
> >   std::function >  CheckerContext &)>;
> > ```
> > I think it is fine with pointers, but at some point we need to modernize 
> > this.
> But its also a lot more expensive. 
> https://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/
> 
> `std::function` is able to wrap lambdas with different captures and all sorts 
> of things like that, which comes at a cost.
Now `std::function` and `std::bind` is used. Probably more expensive but it is 
called once in a `evalCall`, that should be no problem?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:99
+  Optional CheckNullStream(SVal SV, CheckerContext &C) const;
+  Optional CheckDoubleClose(const CallEvent &Call,
+ CheckerContext &C) const;

Charusso wrote:
> This `Optional` is not necessary. When the state is changed, you can rely on 
> `CheckerContext::isDifferent()` to see whether the modeling was succeeded. 
> Therefore you could revert the `bool` return values as well.
`Optional` is not used now. The functions only return if the passed state was 
modified (if applicable). To detect if error was generated the `isDifferent` is 
used.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:137
+  return *Callback;
 }
 

Charusso wrote:
> I would move this tiny `identifyCall()` into `evalCall()` as the purpose of 
> `evalCall()` is the validation of the `Callback` in this case.
`identifyCall` is removed now.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:196
 
   if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
 if (!BT_illegalwhence)

NoQ wrote:
> This is getting pretty messy, given that you've already made a transition in 
> this function. If you do `addTransition` and then 
> `generateNonFatalErrorNode`, then you'll get two parallel successor nodes, 
> but you only need one. You should either delay the transition that happens 
> immediately after `CheckNullStream`, or chain the two transitions together 
> sequentially (as opposed to in parallel).
`evalFseek` has now a transition to new (non-null stream) state or transition 
to (non-fatal) error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[clang] 8dcf1c6 - Updating the documentation for the _Noreturn attribute; NFC.

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2019-10-30T11:42:18-04:00
New Revision: 8dcf1c654ed4e95a618868d4fab11af2068a1471

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

LOG: Updating the documentation for the _Noreturn attribute; NFC.

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 9d0d27407573..3a9093124637 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -258,7 +258,9 @@ def C11NoReturnDocs : Documentation {
   let Content = [{
 A function declared as ``_Noreturn`` shall not return to its caller. The
 compiler will generate a diagnostic for a function declared as ``_Noreturn``
-that appears to be capable of returning to its caller.
+that appears to be capable of returning to its caller. Despite being a type
+specifier, the ``_Noreturn`` attribute cannot be specified on a function
+pointer type.
   }];
 }
 



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

> Interesting, I had missed that conversation! Google and DDG fail to find it 
> for me even quoting sentences out of it. Found it by wgetting the gzip 
> mailing list archives and grepping them - is there a better way? :)

(If you want, I can forward you the thread. But as I said, I just failed to 
delete my toplevel comment; all good.)


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

https://reviews.llvm.org/D63607



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

In D63607#1727037 , @peterwaller-arm 
wrote:

> Thanks for chiming in.
>
> > Sorry, I had missed the RfC, but it looks like there wasn't a lot of 
> > discussion on it anyways.
>
> Apologies @thakis - did I jump the gun? And if so, what could I have done 
> differently?

Whoops, sorry! I had meant to delete that bit. I started writing that, then 
went back to the old discussion thread, and saw that there was actually 
agreement on giving clang's driver knowledge about fortran (but not the 
preprocessor etc). So this is all good as far as I can tell. Please ignore that 
part.

The test is still failing fairly consistently on my bot, even after 6c0a160c 
 . (And 
that change required me to change my bots from `git merge --ff-only 
origin/master` to `git reset --hard origin/master` after fetching, but that's a 
good change to do anyways ;) ). Does http://45.33.8.238/mac/2528/step_6.txt 
make any sense to you, or should I ssh in and debug?

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

https://reviews.llvm.org/D63607

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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm added a comment.

@thakis: I found the thread at 
https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20120430/057199.html

@thakis: I can't make quick sense of the failure from those logs alone. I would 
appreciate it very much to see the output of the `clang -###` run lines to see 
what's missing. It should be showing that it would invoke flang.


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

https://reviews.llvm.org/D63607



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:955
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type = TT_OverloadedOperator;

MyDeveloperDay wrote:
> sammccall wrote:
> > I'm confused about this: ISTM that where we were previously always treating 
> > star as a pointer, now we're always treating it as an operator name.
> > Whereas it's sometimes an operator name (immediately after the `operator` 
> > keyword) and sometimes a pointer (following a type name).
> > 
> > Maybe we should shift the OverloadedOperator labelling outside the loop 
> > (since AFAICT it should only apply to the first token) and then keep the 
> > loop to mark stars/amps elsewhere in the operator name as 
> > PointerOrReference?
> Let me take another look..
@sammccall  In trying to understand what you were suggesting I tried the 
following:

```
bool
Foo::operator*(void *) const {
  return true;
}
```

The second `*` will be as seen correctly below a PointerOrReference by the 
nature that we already hit the OverloadedOperatorLParen as the loop only goes 
until it see a `(` `)` or `;`

I'm trying to think of a case where that perhaps wouldn't be the case? such 
that we might get 2 OverloadedOperators

```
M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=bool L=4 PPK=2 FakeLParens= 
FakeRParens=0 II=0x1cea7c36c40 Text='bool'
 M=1 C=1 T=FunctionDeclarationName S=1 B=0 BK=0 P=80 Name=identifier L=84 PPK=2 
FakeLParens= FakeRParens=0 II=0x1cea7c42298 Text='Foo'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=coloncolon L=86 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=520 Name=operator L=94 PPK=2 FakeLParens= 
FakeRParens=0 II=0x1cea7c36ed8 Text='operator'
 M=0 C=0 T=OverloadedOperator S=0 B=0 BK=0 P=34 Name=star L=95 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='*'
 M=0 C=0 T=OverloadedOperatorLParen S=0 B=0 BK=0 P=34 Name=l_paren L=96 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=void L=100 PPK=2 FakeLParens= 
FakeRParens=0 II=0x1cea7c368e0 Text='void'
 M=0 C=1 T=PointerOrReference S=1 B=0 BK=0 P=230 Name=star L=102 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='*'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=54 Name=r_paren L=103 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=TrailingAnnotation S=1 B=0 BK=0 P=170 Name=const L=109 PPK=2 
FakeLParens= FakeRParens=0 II=0x1cea7c363e8 Text='const'
 M=0 C=0 T=FunctionLBrace S=1 B=0 BK=1 P=23 Name=l_brace L=111 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='{'
```



Comment at: clang/lib/Format/TokenAnnotator.cpp:2626
+  Left.Previous && Left.Previous->is(tok::kw_operator))
+// No space between the type and the *
+// operator void*(), operator char*(), operator Foo*().

MyDeveloperDay wrote:
> sammccall wrote:
> > why?
> LLVM style and google style is different based on pointer alignment
I believe that now I'm no longer marking that `*` as a TT_PointerOrReference, 
one of the other `spaceRequiredBetween` rules is missing for this case and now 
we need to consider the pointer alignment.

Without adding this rule to handle this case one of the 2 items from the same 
`UnderstandsOverloadedOperators` test fail

```
verifyFormat("operator void *();");
...
verifyGoogleFormat("operator void*();");
```

its possible I could clarify this further make ensuring the star is also an 
overloaded operator e.g.

```
if (Right.is(tok::star)  && Right.is(TT_OverloadedOperator)
```


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

https://reviews.llvm.org/D69573



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

[PATCH] D69620: Add AIX assembler support

2019-10-30 Thread Steven Wan via Phabricator via cfe-commits

stevewan created this revision.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.

This is a follow on patch to D68340 . Given 
the AIX toolchain skeleton and system linker support introduced in D68340 
, this patch adds suuport to system assembler 
invocation on AIX.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69620

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h
  clang/test/Driver/aix-as.c

Index: clang/test/Driver/aix-as.c
===
--- /dev/null
+++ clang/test/Driver/aix-as.c
@@ -0,0 +1,48 @@
+// General tests that as invocations on AIX targets are sane. Note that we
+// only test assembler functionalities in this suite.
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS32 %s
+// CHECK-AS32-NOT: warning:
+// CHECK-AS32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-AS32: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS32: "-a32" 
+// CHECK-AS32: "-many" 
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS64 %s
+// CHECK-AS64-NOT: warning:
+// CHECK-AS64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-AS64: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS64: "-a64" 
+// CHECK-AS64: "-many"
+
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -Xassembler  option. 
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -Xassembler -w \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS32-PTHREAD %s
+// CHECK-AS32-PTHREAD-NOT: warning:
+// CHECK-AS32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-AS32-PTHREAD: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS32-PTHREAD: "-a32" 
+// CHECK-AS32-PTHREAD: "-many"
+// CHECK-AS32-PTHREAD: "-w"
+
+// Check powerpc-ibm-aix7.1.0.0, 64-bit. -Wa,, option.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -Wa,-v,-w \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS64-PTHREAD %s
+// CHECK-AS64-PTHREAD-NOT: warning:
+// CHECK-AS64-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-AS64-PTHREAD: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS64-PTHREAD: "-a64" 
+// CHECK-AS64-PTHREAD: "-many"
+// CHECK-AS64-PTHREAD: "-v"
+// CHECK-AS64-PTHREAD: "-w"
Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -16,10 +16,21 @@
 namespace driver {
 namespace tools {
 
-/// aix -- Directly call system default linker.
-// TODO: Enable direct call to system default assembler.
+/// aix -- Directly call system default assembler and linker.
 namespace aix {
 
+class LLVM_LIBRARY_VISIBILITY Assembler : public Tool {
+public:
+  Assembler(const ToolChain &TC) : Tool("aix::Assembler", "assembler", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+
+  void ConstructJob(Compilation &C, const JobAction &JA,
+const InputInfo &Output, const InputInfoList &Inputs,
+const llvm::opt::ArgList &TCArgs,
+const char *LinkingOutput) const override;
+};
+
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
 public:
   Linker(const ToolChain &TC) : Tool("aix::Linker", "linker", TC) {}
@@ -53,6 +64,7 @@
   bool isPICDefaultForced() const override { return true; }
 
 protected:
+  Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
 };
 
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -20,6 +20,47 @@
 
 using namespace llvm::opt;
 
+void aix::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
+  const InputInfo &Output,
+  const InputInfoList &Inputs,
+  const ArgList &Args,
+  const char *LinkingOutput) const {
+  claimNoWarnArgs(Args);
+  ArgStringList CmdArgs;
+
+  const bool IsArch32Bit = getToolChain().getTriple().isArch32Bit();
+  const bool IsArch64Bit = getToolChain().getTriple().isArch64Bit();
+  // Only support 32 and 64 bit.
+  if (!IsArch32Bit && !IsArch64Bit)
+llvm_unreachable("Unsupported bit width value.");
+
+  // Specify the mode in which the as command operates.
+  if (IsArch32Bit) {
+CmdArgs.push_back("-a32");
+  } else {
+

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

> @thakis: I can't make quick sense of the failure from those logs alone. I 
> would appreciate it very much to see the output of the `clang -###` run lines 
> to see what's missing. It should be showing that it would invoke flang.

It's the very first run line that's failing. The output looks like so:

  $ out/gn/bin/clang --driver-mode=flang -### -E 
/Users/thakis/src/llvm-project/clang/test/Driver/flang/flang.f90
  clang version 10.0.0 
  Target: x86_64-apple-darwin18.7.0
  Thread model: posix
  InstalledDir: /Users/thakis/src/llvm-project/out/gn/bin
  clang: warning: 
/Users/thakis/src/llvm-project/clang/test/Driver/flang/flang.f90: previously 
preprocessed input [-Wunused-command-line-argument]


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

https://reviews.llvm.org/D63607



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) 
{
+  // Deduction guides trailing arrow "...) -> A".
+  Current.Type = TT_TrailingReturnArrow;

Why doesn't this trigger on function templates:
  c()->f();



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

https://reviews.llvm.org/D69577



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm added a comment.

I've thought about it for a moment and I'm currently at a loss to quickly 
explain why this would only fail on darwin. In my patch, the change to 
LookupTypeForExtension should prevent clang from reaching this state where it 
complains about a preprocessed input.

I don't have access to a darwin machine right now to dive in, understand and 
fix it.

I propose to mark the test `UNSUPPORTED: darwin` until more is understood about 
the breakage. I assume there is no issue with that?


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

https://reviews.llvm.org/D63607



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread Zhihao Yuan via Phabricator via cfe-commits

lichray added inline comments.

Comment at: clang/lib/Format/TokenAnnotator.cpp:1400
   Current.Type = TT_TrailingReturnArrow;
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) 
{

klimek wrote:
> Why doesn't this trigger on function templates:
>   c()->f();
> 
Comparing to the `else if` branch above, several questions can arise:

1. Has deduction-guide be considered a declaration (it is, of course, in 
standard)?  If yes, without `MustBeDeclaration`, how `x = p->foo<3>();` being 
formatted?
2. Without restrictions on `NestingLevel`, how `A() -> 
Afoo<3>())>;` being formatted?
3. How `x()->foo<1>;` being formatted?  What's the difference between this and 
a deduction-guide?  A deduction-guide has to follow `TheSameType(...) -> 
TheSameType<>;` and appears only at namespace level, do these help?

Oh no, `auto x = p -> foo<1>();` this is a bug (I will look for bug reports, 
don't mind).

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

https://reviews.llvm.org/D69577

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

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

2019-10-30 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69624

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  DynTypedNode::create(*NNSL->getNestedNameSpecifier()),
+  DeclRelation::Alias)}};
+}
 if (const TypeLoc *TL = N.get())
   return refInTypeLoc(*TL);
 if (const CXXCtorInitializer *CCI = N.get()) {


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  DynTypedNode::create(*NNSL->getNestedNameSpecifier()),
+  DeclRelation::Alias)}};
+}
 if (const TypeLoc *TL = N.g

[PATCH] D69625: [libTooling] Support implicit coercions in Stencil's `access` combinator.

2019-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.
ymandel added a parent revision: D69613: [libTooling] Simplify type structure 
of `Stencil`s..

Changes `clang::transformer::access` to also support `RangeSelector` as the
second argument.  This change makes `access` consistent with `cat` in that it
will accept text, `RangeSelector` or `Stencil`. The plan is for all Stencil
arguments to be supported in this way to provide a uniform user experience for
Stencil arguments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69625

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


Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -396,7 +396,7 @@
 }
 
 TEST(StencilToStringTest, AccessOpSelector) {
-  auto S = access("Id", selection(name("otherId")));
+  auto S = access("Id", name("otherId"));
   StringRef Expected = R"repr(access("Id", selection(...)))repr";
   EXPECT_EQ(S->toString(), Expected);
 }
Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -112,8 +112,8 @@
 /// `e->m`, when e is a pointer, `e2->m` when e = `*e2` and `e.m` otherwise.
 /// Additionally, `e` is wrapped in parentheses, if needed.
 Stencil access(llvm::StringRef BaseId, Stencil Member);
-inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
-  return access(BaseId, text(Member));
+template  Stencil access(llvm::StringRef BaseId, T &&Member) {
+  return access(BaseId, makeStencil(std::forward(Member)));
 }
 
 /// Chooses between the two stencil parts, based on whether \p ID is bound in


Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -396,7 +396,7 @@
 }
 
 TEST(StencilToStringTest, AccessOpSelector) {
-  auto S = access("Id", selection(name("otherId")));
+  auto S = access("Id", name("otherId"));
   StringRef Expected = R"repr(access("Id", selection(...)))repr";
   EXPECT_EQ(S->toString(), Expected);
 }
Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -112,8 +112,8 @@
 /// `e->m`, when e is a pointer, `e2->m` when e = `*e2` and `e.m` otherwise.
 /// Additionally, `e` is wrapped in parentheses, if needed.
 Stencil access(llvm::StringRef BaseId, Stencil Member);
-inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
-  return access(BaseId, text(Member));
+template  Stencil access(llvm::StringRef BaseId, T &&Member) {
+  return access(BaseId, makeStencil(std::forward(Member)));
 }
 
 /// Chooses between the two stencil parts, based on whether \p ID is bound in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

2019-10-30 Thread Eric Astor via Phabricator via cfe-commits

epastor created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
epastor added reviewers: rnk, thakis.

In Microsoft-compatibility mode, single commas from nested macro expansions
should not be considered as argument separators; we emulate this by marking
them to be ignored. However, in MSVC's preprocessor, subsequent expansions
DO treat these commas as argument separators... so we now ignore each comma
at most once.

Includes a small unit test that validates we match MSVC's behavior as shown in 
https://gcc.godbolt.org/z/y0twaq


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69626

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/microsoft-ext.c


Index: clang/test/Preprocessor/microsoft-ext.c
===
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
 HAS_1_TEMPLATE_PARAMS(int, k),
 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -820,18 +820,26 @@
 }
   } else if (Tok.is(tok::l_paren)) {
 ++NumParens;
-  } else if (Tok.is(tok::comma) && NumParens == 0 &&
- !(Tok.getFlags() & Token::IgnoredComma)) {
+  } else if (Tok.is(tok::comma)) {
 // In Microsoft-compatibility mode, single commas from nested macro
 // expansions should not be considered as argument separators. We test
-// for this with the IgnoredComma token flag above.
-
-// Comma ends this argument if there are more fixed arguments expected.
-// However, if this is a variadic macro, and this is part of the
-// variadic part, then the comma is just an argument token.
-if (!isVariadic) break;
-if (NumFixedArgsLeft > 1)
-  break;
+// for this with the IgnoredComma token flag.
+if (Tok.getFlags() & Token::IgnoredComma) {
+  // However, in MSVC's preprocessor, subsequent expansions do treat
+  // these commas as argument separators. This leads to a common
+  // workaround used in macros that need to work in both MSVC and
+  // compliant preprocessors. Therefore, the IgnoredComma flag can only
+  // apply once to any given token.
+  Tok.clearFlag(Token::IgnoredComma);
+} else if (NumParens == 0) {
+  // Comma ends this argument if there are more fixed arguments
+  // expected. However, if this is a variadic macro, and this is part 
of
+  // the variadic part, then the comma is just an argument token.
+  if (!isVariadic)
+break;
+  if (NumFixedArgsLeft > 1)
+break;
+}
   } else if (Tok.is(tok::comment) && !KeepMacroComments) {
 // If this is a comment token in the argument list and we're just in
 // -C mode (not -CC mode), discard the comment.


Index: clang/test/Preprocessor/microsoft-ext.c
===
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
 HAS_1_TEMPLATE_PARAMS(int, k),
 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPM

[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

2019-10-30 Thread Carey Williams via Phabricator via cfe-commits

carwil added a comment.

Just want to double check with @efriedma, before we except this. I believe this 
patch now catches all the points you made on https://reviews.llvm.org/D56005. 
Anything we've missed?


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

https://reviews.llvm.org/D68862



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a subscriber: hans.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) 
{
+  // Deduction guides trailing arrow "...) -> A".
+  Current.Type = TT_TrailingReturnArrow;

lichray wrote:
> klimek wrote:
> > Why doesn't this trigger on function templates:
> >   c()->f();
> > 
> Comparing to the `else if` branch above, several questions can arise:
> 
> 1. Has deduction-guide be considered a declaration (it is, of course, in 
> standard)?  If yes, without `MustBeDeclaration`, how `x = p->foo<3>();` being 
> formatted?
> 2. Without restrictions on `NestingLevel`, how `A() -> 
> Afoo<3>())>;` being formatted?
> 3. How `x()->foo<1>;` being formatted?  What's the difference between this 
> and a deduction-guide?  A deduction-guide has to follow `TheSameType(...) -> 
> TheSameType<>;` and appears only at namespace level, do these help?
> 
> Oh no, `auto x = p -> foo<1>();` this is a bug (I will look for bug reports, 
> don't mind).
This case I agree is wrong  (but that comes from the existing rule no?)
```
auto x = p -> foo<1>();
```

The other examples are currently as follows
```
c()->f();
x()->foo<1>;
x = p->foo<3>();
A()->Afoo<3>())>;
```

This is how they look from the last build @hans  made on the 17th October

```
auto x = p -> foo<1>();
c()->f();
x()->foo<1>;
x = p->foo<3>();
A()->Afoo<3>())>;
```

Debug info

```

AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=auto L=4 PPK=2 FakeLParens=2/ 
FakeRParens=0 II=0x2c52129f718 Text='auto'
 M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=220 Name=identifier L=6 PPK=2 
FakeLParens= FakeRParens=0 II=0x2c5212a2da8 Text='x'
 M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=22 Name=equal L=8 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='='
 M=0 C=1 T=Unknown S=1 B=0 BK=0 P=22 Name=identifier L=10 PPK=2 FakeLParens=0/ 
FakeRParens=0 II=0x2c5212a2dd8 Text='p'
 M=0 C=1 T=TrailingReturnArrow S=1 B=0 BK=0 P=23 Name=arrow L=13 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=23 Name=identifier L=17 PPK=2 FakeLParens= 
FakeRParens=0 II=0x2c5212a2d78 Text='foo'
 M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=18 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='<'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Name=numeric_constant L=19 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='1'
 M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=270 Name=greater L=20 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='>'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=30 Name=l_paren L=21 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=59 Name=r_paren L=22 PPK=2 FakeLParens= 
FakeRParens=2 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=23 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=';'

AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=identifier L=1 PPK=2 FakeLParens=0/ 
FakeRParens=0 II=0x2c5212a2e08 Text='c'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=2 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=3 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=170 Name=arrow L=5 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=TrailingAnnotation S=0 B=0 BK=0 P=190 Name=identifier L=6 PPK=2 
FakeLParens= FakeRParens=0 II=0x2c5212a2d48 Text='f'
 M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=7 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='<'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Name=int L=10 PPK=2 FakeLParens= 
FakeRParens=0 II=0x2c52129fa50 Text='int'
 M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=270 Name=greater L=11 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='>'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=30 Name=l_paren L=12 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=13 PPK=2 FakeLParens= 
FakeRParens=1 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=14 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=';'

AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=identifier L=1 PPK=2 FakeLParens=0/ 
FakeRParens=0 II=0x2c5212a2da8 Text='x'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=2 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=3 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=170 Name=arrow L=5 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=TrailingAnnotation S=0 B=0 BK=0 P=190 Name=identifier L=8 PPK=2 
FakeLParens= FakeRParens=0 II=0x2c5212a2d78 Text='foo'
 M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=9 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='<'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Nam

[clang] c75cd3c - [clang][driver][flang] Mark test as unsupported on darwin

2019-10-30 Thread Peter Waller via cfe-commits


Author: Peter Waller
Date: 2019-10-30T17:01:59Z
New Revision: c75cd3c7f0f924d53f07a9cce60c362751678e0c

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

LOG: [clang][driver][flang] Mark test as unsupported on darwin

D63607 made mac builders unhappy by failing this test, and it isn't
yet obvious why. Mark as unsupported as a temporary measure.

Signed-off-by: Peter Waller 

Added: 


Modified: 
clang/test/Driver/flang/flang.f90

Removed: 




diff  --git a/clang/test/Driver/flang/flang.f90 
b/clang/test/Driver/flang/flang.f90
index 9d47c7c90225..97e4847f8439 100644
--- a/clang/test/Driver/flang/flang.f90
+++ b/clang/test/Driver/flang/flang.f90
@@ -1,3 +1,7 @@
+! D63607 made mac builders unhappy by failing this test, and it isn't
+! yet obvious why. Mark as unsupported as a temporary measure.
+! UNSUPPORTED: darwin
+
 ! Check that flang -fc1 is invoked when in --driver-mode=flang.
 
 ! This is a copy of flang_ucase.F90 because the driver has logic in it which



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

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

2019-10-30 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

Build result: pass - 59703 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69624



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

Sorry ignore my last results, let me rerun, I was using the revision without 
the change.


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

https://reviews.llvm.org/D69577



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm added a comment.

@thakis: I've marked the test unsupported in c75cd3c7f0f 
. 
Hopefully that makes your builder happy! I'll figure out what is going on and 
fix it.


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

https://reviews.llvm.org/D63607



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread Zhihao Yuan via Phabricator via cfe-commits

lichray added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403
+} else if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+   Current.startsSequence(tok::arrow, tok::identifier, tok::less)) 
{
+  // Deduction guides trailing arrow "...) -> A".
+  Current.Type = TT_TrailingReturnArrow;

MyDeveloperDay wrote:
> lichray wrote:
> > klimek wrote:
> > > Why doesn't this trigger on function templates:
> > >   c()->f();
> > > 
> > Comparing to the `else if` branch above, several questions can arise:
> > 
> > 1. Has deduction-guide be considered a declaration (it is, of course, in 
> > standard)?  If yes, without `MustBeDeclaration`, how `x = p->foo<3>();` 
> > being formatted?
> > 2. Without restrictions on `NestingLevel`, how `A() -> 
> > Afoo<3>())>;` being formatted?
> > 3. How `x()->foo<1>;` being formatted?  What's the difference between this 
> > and a deduction-guide?  A deduction-guide has to follow `TheSameType(...) 
> > -> TheSameType<>;` and appears only at namespace level, do these help?
> > 
> > Oh no, `auto x = p -> foo<1>();` this is a bug (I will look for bug 
> > reports, don't mind).
> This case I agree is wrong  (but that comes from the existing rule no?)
> ```
> auto x = p -> foo<1>();
> ```
> 
> The other examples are currently as follows
> ```
> c()->f();
> x()->foo<1>;
> x = p->foo<3>();
> A()->Afoo<3>())>;
> ```
> 
> This is how they look from the last build @hans  made on the 17th October
> 
> ```
> auto x = p -> foo<1>();
> c()->f();
> x()->foo<1>;
> x = p->foo<3>();
> A()->Afoo<3>())>;
> ```
> 
> Debug info
> 
> ```
> 
> AnnotatedTokens(L=0):
>  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=auto L=4 PPK=2 FakeLParens=2/ 
> FakeRParens=0 II=0x2c52129f718 Text='auto'
>  M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=220 Name=identifier L=6 PPK=2 
> FakeLParens= FakeRParens=0 II=0x2c5212a2da8 Text='x'
>  M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=22 Name=equal L=8 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='='
>  M=0 C=1 T=Unknown S=1 B=0 BK=0 P=22 Name=identifier L=10 PPK=2 
> FakeLParens=0/ FakeRParens=0 II=0x2c5212a2dd8 Text='p'
>  M=0 C=1 T=TrailingReturnArrow S=1 B=0 BK=0 P=23 Name=arrow L=13 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='->'
>  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=23 Name=identifier L=17 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x2c5212a2d78 Text='foo'
>  M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=18 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='<'
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Name=numeric_constant L=19 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='1'
>  M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=270 Name=greater L=20 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='>'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=30 Name=l_paren L=21 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=59 Name=r_paren L=22 PPK=2 FakeLParens= 
> FakeRParens=2 II=0x0 Text=')'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=23 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=';'
> 
> AnnotatedTokens(L=0):
>  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=identifier L=1 PPK=2 FakeLParens=0/ 
> FakeRParens=0 II=0x2c5212a2e08 Text='c'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=2 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=3 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=')'
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=170 Name=arrow L=5 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='->'
>  M=0 C=0 T=TrailingAnnotation S=0 B=0 BK=0 P=190 Name=identifier L=6 PPK=2 
> FakeLParens= FakeRParens=0 II=0x2c5212a2d48 Text='f'
>  M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=30 Name=less L=7 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='<'
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=360 Name=int L=10 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x2c52129fa50 Text='int'
>  M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=270 Name=greater L=11 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='>'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=30 Name=l_paren L=12 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=13 PPK=2 FakeLParens= 
> FakeRParens=1 II=0x0 Text=')'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=14 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=';'
> 
> AnnotatedTokens(L=0):
>  M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=identifier L=1 PPK=2 FakeLParens=0/ 
> FakeRParens=0 II=0x2c5212a2da8 Text='x'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=2 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=140 Name=r_paren L=3 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=')'
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=170 Name=arrow L=5 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='->'
>  M=0 C=0 T=TrailingAnnotation S=0 B=0 BK=0 P=190 Name=identifier L=8 PPK=2 
> FakeLParens= FakeRParens=0 II=0x2c5212a2d78 Text='foo'
>  M=0 C=0 T

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

2019-10-30 Thread Sjoerd Meijer via Phabricator via cfe-commits

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: Meinersbur, fhahn, rupprecht.

This got reverted because given the following source:

  void a() {
#pragma clang loop vectorize(disable)
for (;;)
  ;
  }


it incorrectly enabled vectorisation and set metadata due to a logic error. 
With this fixed, we now imply vectorisation when:

1. vectorisation is enabled, which means: VectorizeWidth > 1
2. and don't want to add it when it is disabled or enabled, otherwise we would 
be incorrectly setting it or duplicating the metadata, respectively.


https://reviews.llvm.org/D69628

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop-predicate.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]], ![[WIDTH_1]], ![[VECTORIZE_ENABLE]]}
+
+// CHECK-NEXT: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[WIDTH_1]]}
Index: clang/test/CodeGenCXX/pragma-loop-predicate.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -58,7 +58,6 @@
 List[i] = i * 2;
 }
 
-
 // CHECK:  ![[LOOP0]] = distinct !{![[LOOP0]], !3}
 // CHECK-NEXT: !3 = !{!"llvm.loop.vectorize.enable", i1 true}
 
Index: clang/lib/CodeGen/CGLoopInfo.cpp
===
--- clang/lib/CodeGen/CGLoopInfo.cpp
+++ clang/lib/CodeGen/CGLoopInfo.cpp
@@ -268,6 +268,14 @@

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread Chris Bieneman via Phabricator via cfe-commits

beanz added a comment.

In D69356#1726527 , 
@hubert.reinterpretcast wrote:

> For your second question: As the patch author indicated, the change to adjust 
> the behaviour on the affected platform is forthcoming. Your question seems 
> predicated upon that not happening.

I'm actually opposed to that happening, on two fronts.
(1) dead stripping support is useful independent of plugins, so it is valuable 
to have that option be separate
(2) we already have `LLVM_ENABLE_PLUGINS` why do we also need 
`LLVM_SUPPORT_PLUGINS` seems like duplication and bad design.

> Just because the current implementation of plug-in support is by disabling 
> dead strip does not make it a great reason for "enabling plug-in support" to 
> be called `LLVM_NO_DEAD_STRIP`.

`LLVM_NO_DEAD_STRIP` isn't the implementation of enabling plugin support.

> Maybe `LLVM_EXPORT_SYMBOLS_FROM_EXEC` is a better name?

Not really because exporting symbols and dead stripping aren't actually the 
same thing. You can dead strip unused symbols *and* export everything else.

I think this change needs to be rolled back, and future changes to the build 
system should include reviewers with experience maintaining the build system.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356

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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2019-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

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

I've commit on your behalf in 29dc0b17de6b04afa6110a040053a19b02ca1a87 
, thank 
you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55793



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

[clang-tools-extra] 29dc0b1 - Add the readability-redundant-access-specifiers check.

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2019-10-30T13:30:57-04:00
New Revision: 29dc0b17de6b04afa6110a040053a19b02ca1a87

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

LOG: Add the readability-redundant-access-specifiers check.

This finds redundant access specifier declarations inside classes, structs, and 
unions.

Patch by Mateusz Mackowski.

Added: 
clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.h

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-access-specifiers.rst

clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp

clang-tools-extra/test/clang-tidy/readability-redundant-access-specifiers.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 2d226b10334a..7455e10831ce 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -21,6 +21,7 @@ add_clang_library(clangTidyReadabilityModule
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
   ReadabilityTidyModule.cpp
+  RedundantAccessSpecifiersCheck.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp

diff  --git 
a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp 
b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 5005ba3df61f..2fd2870ebe66 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -27,6 +27,7 @@
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
+#include "RedundantAccessSpecifiersCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
@@ -82,6 +83,8 @@ class ReadabilityModule : public ClangTidyModule {
 "readability-misleading-indentation");
 CheckFactories.registerCheck(
 "readability-misplaced-array-index");
+CheckFactories.registerCheck(
+"readability-redundant-access-specifiers");
 CheckFactories.registerCheck(
 "readability-redundant-function-ptr-dereference");
 CheckFactories.registerCheck(

diff  --git 
a/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
new file mode 100644
index ..22625c912580
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
@@ -0,0 +1,85 @@
+//===--- RedundantAccessSpecifiersCheck.cpp - clang-tidy 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "RedundantAccessSpecifiersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantAccessSpecifiersCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Finder->addMatcher(
+  cxxRecordDecl(has(accessSpecDecl())).bind("redundant-access-specifiers"),
+  this);
+}
+
+void RedundantAccessSpecifiersCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+  Result.Nodes.getNodeAs("redundant-access-specifiers");
+
+  const AccessSpecDecl *LastASDecl = nullptr;
+  for (DeclContext::specific_decl_iterator
+   AS(MatchedDecl->decls_begin()),
+   ASEnd(MatchedDecl->decls_end());
+   AS != ASEnd; ++AS) {
+const AccessSpecDecl *ASDecl = *AS;
+
+// Ignore macro expansions.
+if (ASDecl->getLocation().isMacroID()) {
+  LastASDecl = ASDecl;
+  continue;
+}
+
+if (LastASDecl == nullptr) {
+  // First declaration.
+  LastASDecl = ASDecl;
+
+  if (CheckFirstDeclaration) {
+AccessSpecifier DefaultSpecifier =
+MatchedDecl->isClass() ? AS_private : AS_public;
+if (ASDecl->getAccess() == DefaultSpecifier) {
+  diag(ASDecl-

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread David Tenty via Phabricator via cfe-commits

daltenty added a comment.

In D69356#1726527 , 
@hubert.reinterpretcast wrote:

> In D69356#1726354 , @beanz wrote:
>
> > If what you say is correct that "no dead strip" is harmful to plugins on 
> > some platforms it seems like this rename was a step in the direction of 
> > more confusion.
>
>
> Only when focusing on the implementation and not the intent.

Indeed, the goal of this rename was trying to clarify the the intent of this 
option and separate it from it's implementation, which will need to vary by 
platform and will do so in a follow on patch on AIX.

> Perhaps the more accurate intent for both the JIT host process and the 
> plug-in support case is that the executable is expected to make symbols 
> available for run-time linking. Maybe `LLVM_EXPORT_SYMBOLS_FROM_EXEC` is a 
> better name?

I'm in favor of this name, the proposed option name was chosen because the only 
current usage seemed to be with plugins. I think LLVM_EXPORT_SYMBOLS_FROM_EXEC 
much more correctly expresses what users expect the option to do for them with 
out tying it to the specific mechanism  (which may include avoiding dead 
striping on some platforms but not others) and would accommodate both uses

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356

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

[clang-tools-extra] 1caa66d - Fix a false positive in misc-redundant-expression check

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2019-10-30T13:38:25-04:00
New Revision: 1caa66d0759f6bd0851a40645afac8e8a7f84341

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

LOG: Fix a false positive in misc-redundant-expression check

Do not warn for redundant conditional expressions when the true and false
branches are expanded from different macros even when they are defined by
one another.

Patch by Daniel Krupp.

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index e646ee91ac52..e3e84b71601b 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -131,6 +131,20 @@ static bool areEquivalentExpr(const Expr *Left, const Expr 
*Right) {
   case Stmt::BinaryOperatorClass:
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
+  case Stmt::UnaryExprOrTypeTraitExprClass:
+const auto *LeftUnaryExpr =
+cast(Left);
+const auto *RightUnaryExpr =
+cast(Right);
+if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
+  return LeftUnaryExpr->getArgumentType() ==
+ RightUnaryExpr->getArgumentType();
+else if (!LeftUnaryExpr->isArgumentType() &&
+ !RightUnaryExpr->isArgumentType())
+  return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(),
+   RightUnaryExpr->getArgumentExpr());
+
+return false;
   }
 }
 
@@ -604,23 +618,62 @@ static bool retrieveConstExprFromBothSides(const 
BinaryOperator *&BinOp,
   return true;
 }
 
+static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
+const SourceManager &SM) {
+  if (T1.getKind() != T2.getKind())
+return false;
+  if (T1.isNot(tok::raw_identifier))
+return true;
+  if (T1.getLength() != T2.getLength())
+return false;
+  return StringRef(SM.getCharacterData(T1.getLocation()), T1.getLength()) ==
+ StringRef(SM.getCharacterData(T2.getLocation()), T2.getLength());
+}
+
+bool isTokAtEndOfExpr(SourceRange ExprSR, Token T, const SourceManager &SM) {
+  return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
+}
+
+/// Returns true if both LhsEpxr and RhsExpr are
+/// macro expressions and they are expanded
+/// from 
diff erent macros.
 static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const Expr *RhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
-
-  if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
+  SourceRange Lsr = LhsExpr->getSourceRange();
+  SourceRange Rsr = RhsExpr->getSourceRange();
+  if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
 
   const SourceManager &SM = AstCtx->getSourceManager();
   const LangOptions &LO = AstCtx->getLangOpts();
 
-  return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
-  Lexer::getImmediateMacroName(RhsLoc, SM, LO));
+  std::pair LsrLocInfo =
+  SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
+  std::pair RsrLocInfo =
+  SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
+  const llvm::MemoryBuffer *MB = SM.getBuffer(LsrLocInfo.first);
+
+  const char *LTokenPos = MB->getBufferStart() + LsrLocInfo.second;
+  const char *RTokenPos = MB->getBufferStart() + RsrLocInfo.second;
+  Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
+MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
+  Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
+MB->getBufferStart(), RTokenPos, MB->getBufferEnd());
+
+  Token LTok, RTok;
+  do { // Compare the expressions token-by-token.
+LRawLex.LexFromRawLexer(LTok);
+RRawLex.LexFromRawLexer(RTok);
+  } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) &&
+   isSameRawIdentifierToken(LTok, RTok, SM) &&
+   !isTokAtEndOfExpr(Lsr, LTok, SM) &&
+   !isTokAtEndOfExpr(Rsr, RTok, SM));
+  return (!isTokAtEndOfExpr(Lsr, LTok, SM) ||
+  !isTokAtEndOfExpr(Rsr, RTok, SM)) ||
+ !isSameRawIdentifierToken(LTok, RTok, SM);
 }
 
 static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
index f6b47eb79fb9..50

[PATCH] D69498: IR: Invert convergent attribute handling

2019-10-30 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added a comment.

In D69498#1727080 , @jdoerfert wrote:

> Let me quote @arsenm here because this is so important: "Basically no 
> frontend has gotten this right, including clang and non-clang frontends." For 
> me, that statement alone is reason to change the status quo.

I am not convinced by this: this seems overly simplistic. The fact that 
convergent wasn't rolled out properly in frontend can also been as an artifact 
of the past. There is a logical gap between this and concluding that the only 
solution is to change the default. For instance someone mentioned a pass that 
could be inserted as the very beginning of the pipeline by any GPU compiler and 
add the convergent attribute everywhere.

>> As of the frontend, it seems to me that this is just about structuring the 
>> code-path for function emission to define the right set of default 
>> attribute. It isn't clear to me why a refactoring of the frontend isn't a 
>> better course of actions here.
> 
> Whatever we do, there will be consequences for current and future front-ends. 
> At the end of the day, the underlying question we need to answer here is: Do 
> we want to have "optimization with opt-in soundness" or "soundness with 
> opt-in optimizations", and I would choose the latter any time.

This view seems appropriate to me only if you consider a GPU (or SIMT) compiler 
alone. Another view (which seems to be what @rjmccall has) is that SIMT/GPU is 
a marginal use-case and "soundness" is already existing for most LLVM use-cases.

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

https://reviews.llvm.org/D69498

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

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2019-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

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

In D55125#1717910 , @dkrupp wrote:

> @aaron.ballman  could you please check now? Thanks!


Sorry for the delay, but I've committed this on your behalf in 
1caa66d0759f6bd0851a40645afac8e8a7f84341 



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

https://reviews.llvm.org/D55125



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2019-10-30 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

@aaron.ballman: Please move Release Notes entry to new checks section (in 
alphabetical order). Currently it's located in //Improvements to 
include-fixer//. Please also close PR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55793



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

[clang-tools-extra] 661d2ce - Fix modernize-use-nodiscard for classes marked [[nodiscard]]

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2019-10-30T13:45:37-04:00
New Revision: 661d2ce619e05dc47a9a232333c01dcba445cd13

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

LOG: Fix modernize-use-nodiscard for classes marked [[nodiscard]]

Current implementation suggests to add [[nodiscard]] to methods even if the
return type is marked already as [[nodiscard]].

Patch by Eugene Sedykh.

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-use-nodiscard.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp
index d2ac7d529ce6..f9d72f5d62c2 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp
@@ -100,7 +100,9 @@ void UseNodiscardCheck::registerMatchers(MatchFinder 
*Finder) {
   cxxMethodDecl(
   allOf(isConst(), isDefinitionOrInline(),
 unless(anyOf(
-returns(voidType()), isNoReturn(), isOverloadedOperator(),
+returns(voidType()),
+
returns(hasDeclaration(decl(hasAttr(clang::attr::WarnUnusedResult,
+isNoReturn(), isOverloadedOperator(),
 isVariadic(), hasTemplateReturnType(),
 hasClassMutableFields(), isConversionOperator(),
 hasAttr(clang::attr::WarnUnusedResult),

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-nodiscard.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-nodiscard.cpp
index a571f0963637..7340713eefa4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-nodiscard.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-nodiscard.cpp
@@ -23,6 +23,8 @@ typedef unsigned my_unsigned;
 typedef unsigned &my_unsigned_reference;
 typedef const unsigned &my_unsigned_const_reference;
 
+struct NO_DISCARD NoDiscardStruct{};
+
 class Foo {
 public:
 using size_type = unsigned;
@@ -160,6 +162,9 @@ class Foo {
 
 // Do not add ``[[nodiscard]]`` to conversion functions.
 // explicit operator bool() const { return true; }
+
+// Do not add ``[[nodiscard]]`` to functions returning types marked 
[[nodiscard]].
+NoDiscardStruct f50() const;
 };
 
 // Do not add ``[[nodiscard]]`` to Lambda.



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

[clang-tools-extra] 0de262d - Move this release note to its appropriate location; NFC.

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2019-10-30T13:48:26-04:00
New Revision: 0de262d7189c68897e8328d891d3daaf3aab3156

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

LOG: Move this release note to its appropriate location; NFC.

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index d2819258f17e..e6ef3cd5af0d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,17 +130,17 @@ Improvements to clang-tidy
 - The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
   `
 
-Improvements to include-fixer
--
-
-The improvements are...
-
 - New :doc:`readability-redundant-access-specifiers
   ` check.
 
   Finds classes, structs, and unions that contain redundant member
   access specifiers.
 
+Improvements to include-fixer
+-
+
+The improvements are...
+
 Improvements to clang-include-fixer
 ---
 



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

[PATCH] D69388: [clang-tidy] Fix modernize-use-nodiscard check for classes marked as [[nodiscard]]

2019-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

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

Thank you for the patch, I've commit on your behalf in 
661d2ce619e05dc47a9a232333c01dcba445cd13 



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69388



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2019-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D55793#1727423 , @Eugene.Zelenko 
wrote:

> @aaron.ballman: Please move Release Notes entry to new checks section (in 
> alphabetical order). Currently it's located in //Improvements to 
> include-fixer//. Please also close PR.


Both should be done now, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55793



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2019-10-30 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

In D55793#1727437 , @aaron.ballman 
wrote:

> In D55793#1727423 , @Eugene.Zelenko 
> wrote:
>
> > @aaron.ballman: Please move Release Notes entry to new checks section (in 
> > alphabetical order). Currently it's located in //Improvements to 
> > include-fixer//. Please also close PR.
>
>
> Both should be done now, thanks!


Thank you for quick fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55793



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

Re: [clang-tools-extra] 0de262d - Move this release note to its appropriate location; NFC.

2019-10-30 Thread Nico Weber via cfe-commits

Aren't include-fixer and clang-include-fixer the same thing? Why do we have
to entries for it?

On Wed, Oct 30, 2019 at 1:48 PM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Aaron Ballman
> Date: 2019-10-30T13:48:26-04:00
> New Revision: 0de262d7189c68897e8328d891d3daaf3aab3156
>
> URL:
> https://github.com/llvm/llvm-project/commit/0de262d7189c68897e8328d891d3daaf3aab3156
> DIFF:
> https://github.com/llvm/llvm-project/commit/0de262d7189c68897e8328d891d3daaf3aab3156.diff
>
> LOG: Move this release note to its appropriate location; NFC.
>
> Added:
>
>
> Modified:
> clang-tools-extra/docs/ReleaseNotes.rst
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst
> b/clang-tools-extra/docs/ReleaseNotes.rst
> index d2819258f17e..e6ef3cd5af0d 100644
> --- a/clang-tools-extra/docs/ReleaseNotes.rst
> +++ b/clang-tools-extra/docs/ReleaseNotes.rst
> @@ -130,17 +130,17 @@ Improvements to clang-tidy
>  - The 'objc-avoid-spinlock' check was renamed to
> :doc:`darwin-avoid-spinlock
>`
>
> -Improvements to include-fixer
> --
> -
> -The improvements are...
> -
>  - New :doc:`readability-redundant-access-specifiers
>` check.
>
>Finds classes, structs, and unions that contain redundant member
>access specifiers.
>
> +Improvements to include-fixer
> +-
> +
> +The improvements are...
> +
>  Improvements to clang-include-fixer
>  ---
>
>
>
>
> ___
> 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] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread David Tenty via Phabricator via cfe-commits

daltenty added a comment.

> I'm actually opposed to that happening, on two fronts.
>  (2) we already have `LLVM_ENABLE_PLUGINS` why do we also need 
> `LLVM_SUPPORT_PLUGINS` seems like duplication and bad design.

As I understand it LLVM_ENABLE_PLUGINS is a user-facing option which indicates 
whether the user wants to build with plugin support.

The intention of LLVM_SUPPORT_PLUGINS was as an internal option set by tools 
which may have plugins and need to be built in a specific way (such as avoiding 
deadstriping) if plugins are enabled.

> (1) dead stripping support is useful independent of plugins, so it is 
> valuable to have that option be separate

We could keep LLVM_NO_DEAD_STRIP as is and have it set by LLVM_SUPPORT_PLUGINS 
when appropriate. That should accommodate both uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356



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

[PATCH] D69498: IR: Invert convergent attribute handling

2019-10-30 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added a comment.

In D69498#1727419 , @mehdi_amini wrote:

> In D69498#1727080 , @jdoerfert wrote:
>
> > Let me quote @arsenm here because this is so important: "Basically no 
> > frontend has gotten this right, including clang and non-clang frontends." 
> > For me, that statement alone is reason to change the status quo.
>
>
> I am not convinced by this: this seems overly simplistic. The fact that 
> convergent wasn't rolled out properly in frontend can also been as an 
> artifact of the past. There is a logical gap between this and concluding that 
> the only solution is to change the default. For instance someone mentioned a 
> pass that could be inserted as the very beginning of the pipeline by any GPU 
> compiler and add the convergent attribute everywhere.

Avoiding human error is fundamental to good design. If you have to have an 
additional IR pass, then there's already a phase where the IR is broken and 
something could legally break the IR.

> 
> 
>>> As of the frontend, it seems to me that this is just about structuring the 
>>> code-path for function emission to define the right set of default 
>>> attribute. It isn't clear to me why a refactoring of the frontend isn't a 
>>> better course of actions here.
>> 
>> Whatever we do, there will be consequences for current and future 
>> front-ends. At the end of the day, the underlying question we need to answer 
>> here is: Do we want to have "optimization with opt-in soundness" or 
>> "soundness with opt-in optimizations", and I would choose the latter any 
>> time.
> 
> This view seems appropriate to me only if you consider a GPU (or SIMT) 
> compiler alone. Another view (which seems to be what @rjmccall has) is that 
> SIMT/GPU is a marginal use-case and "soundness" is already existing for most 
> LLVM use-cases.

I think this is a concerning argument. Declaring that GPUs are a "marginal" use 
case and LLVM only follows its design principles in cases where it benefits C++ 
x86/ARM users is an issue. In that case LLVM is no longer trying to be the 
compiler infrastructure for everyone that it tries to be. Soundness for most 
doesn't sound like a good design. I've proposed attributes in the past that 
were shot down for not following a correct by-default design and to me it seems 
like a problem if this principle isn't going to be universally followed. It's 
additionally concerning since most GPUs uses LLVM in some fashion if they're 
going to be declared a second class use case.

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

https://reviews.llvm.org/D69498

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

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

2019-10-30 Thread Michael Kruse via Phabricator via cfe-commits

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



Comment at: clang/lib/CodeGen/CGLoopInfo.cpp:272-277
+  if (Attrs.VectorizeWidth > 1 &&
+  Attrs.VectorizeEnable == LoopAttributes::Unspecified)
+Args.push_back(
+MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
+  ConstantAsMetadata::get(ConstantInt::get(
+  llvm::Type::getInt1Ty(Ctx), 1))}));

[serious] Please handle the `llvm.loop.vectorize.enable` metadata in one place, 
i.e. where the other `llvm.loop.vectorize.enable` is handled. This introduces 
yet another mechanism when to add `llvm.loop.vectorize.enable` besides the one 
for `IsVectorPredicateEnabled`. Btw, with `vectorize_predicate(enable) 
vectorize_width(2)` this emits **two** `llvm.loop.vectorize.enable`.

Also, the changing relative order of `llvm.loop.vectorize.enable` to other 
metadata makes D69092 difficult.



Comment at: clang/test/CodeGenCXX/pragma-loop-predicate.cpp:61
 
-
 // CHECK:  ![[LOOP0]] = distinct !{![[LOOP0]], !3}

unintentional change?



Comment at: clang/test/CodeGenCXX/pragma-loop.cpp:161-177
+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;

As you might have noticed, updating FileCheck for metadata in multiple test 
cases is a lot of work since they influence each other. Why not adding the new 
test in separate files?



Comment at: clang/test/CodeGenCXX/pragma-loop.cpp:239
+
+// CHECK-NEXT: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[WIDTH_1]]}

`-NEXT` should be unnecessary here. I'd even go towards `CHECK-DAG` since the 
order of the metadata is unimportant.


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

https://reviews.llvm.org/D69628



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

[clang-tools-extra] 4de6b15 - Add an option to hicpp-signed-bitwise for positive integer literals.

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Vladimir Plyashkun
Date: 2019-10-30T14:11:29-04:00
New Revision: 4de6b1586807285e20a5db6596519c2336a64568

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

LOG: Add an option to hicpp-signed-bitwise for positive integer literals.

This gives developers a way to deviate from the coding standard to reduce the
chattiness of the check.

Added: 


Modified: 
clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp 
b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
index 781a4430545e..b9c60772415a 100644
--- a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
@@ -17,9 +17,24 @@ namespace clang {
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(integerLiteral()))
+   : expr(ignoringImpCasts(hasType(isSignedInteger()
+  .bind("signed-operand");
 
   // The standard [bitmask.types] allows some integral types to be implemented
   // as signed types. Exclude these types from diagnosing for bitwise or(|) and

diff  --git a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h 
b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
index 34d5f096df6e..74b3f0eb235d 100644
--- a/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ b/clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@ namespace hicpp {
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index e6ef3cd5af0d..c706ae13c785 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,11 @@ Improvements to clang-tidy
   Finds uses of deprecated Googletest APIs with names containing ``case`` and
   replaces them with equivalent APIs with ``suite``.
 
+- Improved :doc:`hicpp-signed-bitwise
+  ` check.
+
+  The check now supports the ``IgnorePositiveIntegerLiterals`` option.
+
 - New :doc:`linuxkernel-must-use-errs
   ` check.
 

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst 
b/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
index 59b19b6f3044..4c6bc005a8ec 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
@@ -7,3 +7,11 @@ Finds uses of bitwise operations on signed integer types, 
which may lead to
 undefined or implementation defined behaviour.
 
 The according rule is defined in the `High Integrity C++ Standard, Section 
5.6.1 `_.
+
+Options
+---
+
+.. option:: IgnorePositiveIntegerLiterals
+
+   If this option is set to `true`, the check will not warn on bitwise 
operations with positive integer literals, e.g. `~0`, `2 << 1`, etc.
+   Default value is `false`.



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

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

2019-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

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

Thank you for the patch! I've committed on your behalf in 
4de6b1586807285e20a5db6596519c2336a64568 
.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-10-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Thanks!

Looks like it's just the `-E` test that's causing problems; the test passes 
with that disabled:

  $ git diff
  diff --git a/clang/test/Driver/flang/flang.f90 
b/clang/test/Driver/flang/flang.f90
  index 97e4847f843..4cbc2cd8754 100644
  --- a/clang/test/Driver/flang/flang.f90
  +++ b/clang/test/Driver/flang/flang.f90
  @@ -1,6 +1,5 @@
   ! D63607 made mac builders unhappy by failing this test, and it isn't
   ! yet obvious why. Mark as unsupported as a temporary measure.
  -! UNSUPPORTED: darwin
   
   ! Check that flang -fc1 is invoked when in --driver-mode=flang.
   
  @@ -21,10 +20,9 @@
   
   ! Check that f90 files are not treated as "previously preprocessed"
   ! ... in --driver-mode=flang.
  -! RUN: %clang --driver-mode=flang -### -E  %s 2>&1 | 
FileCheck --check-prefixes=ALL,CHECK-E %s
  +! RUN: %clang --driver-mode=flang -###   %s 2>&1 | FileCheck 
--check-prefixes=ALL,CHECK-E %s
   ! CHECK-E-NOT: previously preprocessed input
  -! CHECK-E-DAG: "-E"
  -! CHECK-E-DAG: "-o" "-"
  +! CHECK-E-DAG: "-o"
   
   ! RUN: %clang --driver-mode=flang -### -emit-ast   %s 2>&1 | 
FileCheck --check-prefixes=ALL,CHECK-EMIT-AST %s
   ! CHECK-EMIT-AST-DAG: "-triple"

I'll debug a bit more.




Comment at: clang/lib/Driver/Driver.cpp:4883
+  // And say "no" if this is not a kind of action flang understands.
+  if (!isa(JA) && !isa(JA) && 
!isa(JA))
+return false;

(please run clang-format on your new code -- it might also answer why some 
enums have a comma after the last entry ;) )


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

https://reviews.llvm.org/D63607



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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

2019-10-30 Thread Chris Bieneman via Phabricator via cfe-commits

beanz added a comment.

In D69356#1727451 , @daltenty wrote:

> The intention of LLVM_SUPPORT_PLUGINS was as an internal option set by tools 
> which may have plugins and need to be built in a specific way (such as 
> avoiding deadstriping) if plugins are enabled.

Non-user facing options shouldn't be exposed this way. `LLVM_NO_DEAD_STRIP` has 
been around a long time and predates many of the modern CMake patterns. We 
should use arguments passed in explicitly to the calling functions rather than 
setting variables that are passed down.

> We could keep LLVM_NO_DEAD_STRIP as is and have it set by 
> LLVM_SUPPORT_PLUGINS when appropriate. That should accommodate both uses.

We should not be adding more variables that are passed around by CMake's scope 
inheritance. Instead if we need to change this we should do it correctly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356

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

[clang-tools-extra] e477988 - Fix readability-identifier-naming to prevent variables becoming keywords.

2019-10-30 Thread Aaron Ballman via cfe-commits


Author: Daniel
Date: 2019-10-30T14:18:40-04:00
New Revision: e477988309dbde214a6d16ec690a416882714aac

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

LOG: Fix readability-identifier-naming to prevent variables becoming keywords.

Do not provide a fix-it when clang-tidy encounters a name that would become
a keyword.

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang/include/clang/Basic/IdentifierTable.h

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 322894041325..5b78155cd546 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -691,10 +691,11 @@ static void 
addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
   if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
 return;
 
-  if (!Failure.ShouldFix)
+  if (!Failure.ShouldFix())
 return;
 
-  Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
+  if (!utils::rangeCanBeFixed(Range, SourceMgr))
+Failure.FixStatus = IdentifierNamingCheck::ShouldFixStatus::InsideMacro;
 }
 
 /// Convenience method when the usage to be added is a NamedDecl
@@ -873,6 +874,16 @@ void IdentifierNamingCheck::check(const 
MatchFinder::MatchResult &Result) {
   DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation())
   .getSourceRange();
 
+  const IdentifierTable &Idents = Decl->getASTContext().Idents;
+  auto CheckNewIdentifier = Idents.find(Fixup);
+  if (CheckNewIdentifier != Idents.end()) {
+const IdentifierInfo *Ident = CheckNewIdentifier->second;
+if (Ident->isKeyword(getLangOpts()))
+  Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;
+else if (Ident->hasMacroDefinition())
+  Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition;
+  }
+
   Failure.Fixup = std::move(Fixup);
   Failure.KindName = std::move(KindName);
   addUsage(NamingCheckFailures, Decl, Range);
@@ -935,24 +946,35 @@ void IdentifierNamingCheck::onEndOfTranslationUnit() {
 if (Failure.KindName.empty())
   continue;
 
-if (Failure.ShouldFix) {
-  auto Diag = diag(Decl.first, "invalid case style for %0 '%1'")
-  << Failure.KindName << Decl.second;
-
-  for (const auto &Loc : Failure.RawUsageLocs) {
-// We assume that the identifier name is made of one token only. This 
is
-// always the case as we ignore usages in macros that could build
-// identifier names by combining multiple tokens.
-//
-// For destructors, we alread take care of it by remembering the
-// location of the start of the identifier and not the start of the
-// tilde.
-//
-// Other multi-token identifiers, such as operators are not checked at
-// all.
-Diag << FixItHint::CreateReplacement(
-SourceRange(SourceLocation::getFromRawEncoding(Loc)),
-Failure.Fixup);
+if (Failure.ShouldNotify()) {
+  auto Diag =
+  diag(Decl.first,
+   "invalid case style for %0 '%1'%select{|" // Case 0 is empty on
+ // purpose, because we
+ // intent to provide a
+ // fix
+   "; cannot be fixed because '%3' would conflict with a keyword|"
+   "; cannot be fixed because '%3' would conflict with a macro "
+   "definition}2")
+  << Failure.KindName << Decl.second
+  << static_cast(Failure.FixStatus) << Failure.Fixup;
+
+  if (Failure.ShouldFix()) {
+for (const auto &Loc : Failure.RawUsageLocs) {
+  // We assume that the identifier name is made of one token only. This
+  // is always the case as we ignore usages in macros that could build
+  // identifier names by combining multiple tokens.
+  //
+  // For destructors, we already take care of it by remembering the
+  // location of the start of the identifier and not the start of the
+  // tilde.
+  //
+  // Other multi-token identifiers, such as operators are not checked 
at
+  // all.
+  Diag << FixItHint::CreateReplacement(
+  SourceRange(SourceLocation::getFromRawEncoding(Loc)),
+  Failure.Fixup);
+}
   }
 }
   }

diff  --git a/clang-tools-extra/clang-tidy/r

1 2 >

1 - 100 of 191 matches

Mail list logo