r295698 - When deducing an array bound from the length of an initializer list, don't

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 21 01:22:31 2017
New Revision: 295698

URL: http://llvm.org/viewvc/llvm-project?rev=295698=rev
Log:
When deducing an array bound from the length of an initializer list, don't
assume the bound has a non-dependent integral type.

Modified:
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
cfe/trunk/test/SemaTemplate/deduction.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=295698=295697=295698=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Tue Feb 21 01:22:31 2017
@@ -730,6 +730,11 @@ public:
 std::copy(Pack.New.begin(), Pack.New.end(), ArgumentPack);
 NewPack = DeducedTemplateArgument(
 TemplateArgument(llvm::makeArrayRef(ArgumentPack, 
Pack.New.size())),
+// FIXME: This is wrong, it's possible that some pack elements are
+// deduced from an array bound and others are not:
+//   template void g(const T (&...p)[V]);
+//   g({1, 2, 3}, {{}, {}});
+// ... should deduce T = {int, size_t (from array bound)}.
 Pack.New[0].wasDeducedFromArrayBound());
   }
 
@@ -3353,10 +3358,12 @@ static Sema::TemplateDeductionResult Ded
 getDeducedParameterFromExpr(Info, DependentArrTy->getSizeExpr())) {
   // We can perform template argument deduction for the given non-type
   // template parameter.
-  llvm::APInt Size(S.Context.getIntWidth(NTTP->getType()),
-   ILE->getNumInits());
+  // C++ [temp.deduct.type]p13:
+  //   The type of N in the type T[N] is std::size_t.
+  QualType T = S.Context.getSizeType();
+  llvm::APInt Size(S.Context.getIntWidth(T), ILE->getNumInits());
   if (auto Result = DeduceNonTypeTemplateArgument(
-  S, TemplateParams, NTTP, llvm::APSInt(Size), NTTP->getType(),
+  S, TemplateParams, NTTP, llvm::APSInt(Size), T,
   /*ArrayBound=*/true, Info, Deduced))
 return Result;
 }

Modified: cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp?rev=295698=295697=295698=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp 
(original)
+++ cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp Tue Feb 
21 01:22:31 2017
@@ -194,11 +194,9 @@ namespace transform_params {
   A a(qn, qn); // expected-error {{no matching constructor for initialization 
of 'transform_params::A'}}
   static_assert(a.v == 12);
 
-  // FIXME: This causes a crash right now (not class template deduction 
related).
-#if 0
-  template struct B {
-template B(T (&...p)[V]);
+  // FIXME: This should be accepted.
+  template struct B { // expected-note {{candidate}}
+template B(const T (&...p)[V]); // expected-note {{substitution 
failure}}
   };
-  B b({1, 2, 3}, {"foo", "bar"}, {'x', 'y', 'z', 'w'});
-#endif
+  B b({1, 2, 3}, {"foo", "bar"}, {'x', 'y', 'z', 'w'}); // expected-error {{no 
viable constructor or deduction guide}}
 }

Modified: cfe/trunk/test/SemaTemplate/deduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/deduction.cpp?rev=295698=295697=295698=diff
==
--- cfe/trunk/test/SemaTemplate/deduction.cpp (original)
+++ cfe/trunk/test/SemaTemplate/deduction.cpp Tue Feb 21 01:22:31 2017
@@ -494,3 +494,45 @@ namespace dependent_template_template_pa
   // FIXME: This should be accepted, but we somehow fail to deduce W.
   A<0> a(qn); // expected-error {{no matching constructor for initialization}}
 }
+
+namespace dependent_list_deduction {
+  template void a(const int (&)[V]) {
+static_assert(is_same::value, "");
+static_assert(V == 3, "");
+  }
+  template void b(const T (&)[V]) {
+static_assert(is_same::value, "");
+static_assert(V == 3, "");
+  }
+  template void c(const T (&)[V]) {
+static_assert(is_same::value, "");
+static_assert(V == 3, "");
+  }
+  void d() {
+a({1, 2, 3});
+#if __cplusplus <= 201402L
+// expected-error@-2 {{no match}} expected-note@-15 {{couldn't infer 
template argument 'T'}}
+#endif
+b({1, 2, 3});
+c({{}, {}, {}});
+#if __cplusplus <= 201402L
+// expected-error@-2 {{no match}} expected-note@-12 {{couldn't infer 
template argument 'T'}}
+#endif
+  }
+
+  template struct X;
+  template struct Y;
+  template void f(const T (&...p)[V]) {
+static_assert(is_same>::value, 

r295696 - PR32010: Fix template argument depth mixup when forming implicit constructor

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 21 00:30:38 2017
New Revision: 295696

URL: http://llvm.org/viewvc/llvm-project?rev=295696=rev
Log:
PR32010: Fix template argument depth mixup when forming implicit constructor
template deduction guides for class template argument deduction.

Ensure that we have a local instantiation scope for tracking the instantiated
parameters. Additionally, unusually, we're substituting at depth 1 and leaving
depth 0 alone; make sure that we don't reduce template parameter depth by 2 for
inner parameters in the process. (This is probably also broken for alias
templates in the case where they're expanded within a dependent context, but
this patch doesn't fix that.)

Modified:
cfe/trunk/include/clang/AST/DeclTemplate.h
cfe/trunk/include/clang/Sema/Template.h
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
cfe/trunk/test/SemaTemplate/deduction.cpp

Modified: cfe/trunk/include/clang/AST/DeclTemplate.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=295696=295695=295696=diff
==
--- cfe/trunk/include/clang/AST/DeclTemplate.h (original)
+++ cfe/trunk/include/clang/AST/DeclTemplate.h Tue Feb 21 00:30:38 2017
@@ -1479,6 +1479,7 @@ public:
   unsigned NumExpansions);
   
   using TemplateParmPosition::getDepth;
+  using TemplateParmPosition::setDepth;
   using TemplateParmPosition::getPosition;
   using TemplateParmPosition::setPosition;
   using TemplateParmPosition::getIndex;

Modified: cfe/trunk/include/clang/Sema/Template.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Template.h?rev=295696=295695=295696=diff
==
--- cfe/trunk/include/clang/Sema/Template.h (original)
+++ cfe/trunk/include/clang/Sema/Template.h Tue Feb 21 00:30:38 2017
@@ -46,6 +46,10 @@ namespace clang {
 /// \brief The template argument lists, stored from the innermost template
 /// argument list (first) to the outermost template argument list (last).
 SmallVector TemplateArgumentLists;
+
+/// \brief The number of outer levels of template arguments that are not
+/// being substituted.
+unsigned NumRetainedOuterLevels = 0;
 
   public:
 /// \brief Construct an empty set of template argument lists.
@@ -59,11 +63,19 @@ namespace clang {
 
 /// \brief Determine the number of levels in this template argument
 /// list.
-unsigned getNumLevels() const { return TemplateArgumentLists.size(); }
-
+unsigned getNumLevels() const {
+  return TemplateArgumentLists.size() + NumRetainedOuterLevels;
+}
+
+/// \brief Determine the number of substituted levels in this template
+/// argument list.
+unsigned getNumSubstitutedLevels() const {
+  return TemplateArgumentLists.size();
+}
+
 /// \brief Retrieve the template argument at a given depth and index.
 const TemplateArgument ()(unsigned Depth, unsigned Index) const {
-  assert(Depth < TemplateArgumentLists.size());
+  assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels());
   assert(Index < TemplateArgumentLists[getNumLevels() - Depth - 1].size());
   return TemplateArgumentLists[getNumLevels() - Depth - 1][Index];
 }
@@ -73,7 +85,10 @@ namespace clang {
 ///
 /// There must exist a template argument list at the given depth.
 bool hasTemplateArgument(unsigned Depth, unsigned Index) const {
-  assert(Depth < TemplateArgumentLists.size());
+  assert(Depth < getNumLevels());
+
+  if (Depth < NumRetainedOuterLevels)
+return false;
   
   if (Index >= TemplateArgumentLists[getNumLevels() - Depth - 1].size())
 return false;
@@ -84,7 +99,7 @@ namespace clang {
 /// \brief Clear out a specific template argument.
 void setArgument(unsigned Depth, unsigned Index,
  TemplateArgument Arg) {
-  assert(Depth < TemplateArgumentLists.size());
+  assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels());
   assert(Index < TemplateArgumentLists[getNumLevels() - Depth - 1].size());
   const_cast(
 TemplateArgumentLists[getNumLevels() - Depth - 1][Index])
@@ -101,9 +116,18 @@ namespace clang {
 /// \brief Add a new outmost level to the multi-level template argument
 /// list.
 void addOuterTemplateArguments(ArgList Args) {
+  assert(!NumRetainedOuterLevels &&
+ "substituted args outside retained args?");
   TemplateArgumentLists.push_back(Args);
 }
 
+/// \brief Add an outermost level that we are not substituting. We have no
+/// arguments at this level, and do not 

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

[Daphne Pfister via Phabricator via cfe-commits]

daphnediane added a comment.

Rebuilt with the latest patch and got one compile error. See line comment 
worked okay after fixing it.




Comment at: lib/Format/WhitespaceManager.cpp:215
+
+if (i != Start) {
+  if (Changes[i].nestingAndIndentLevel() >

These ifs can get merged again, when you merged my changes in it was based on a 
version before you merged them.



Comment at: lib/Format/WhitespaceManager.h:157
+std::pair
+WhitespaceManager::Change::nestingAndIndentLevel() const {
+  return std::make_pair(Tok->NestingLevel, Tok->IndentLevel);

Extra WhitespaceManager::Change:: prefix here


https://reviews.llvm.org/D21279



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

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D30174#681843, @arphaman wrote:

> Yes, we do. Primarily for the following reason: even if some target may 
> return a struct in a register, another target isn't guaranteed to do the same 
> thing. It's better to always warn about this rather than accept some small 
> structs. Furthermore, the official documentation states that "For methods 
> that return anything other than an object, use NSInvocation." [1], so methods 
> that return records are completely prohibited by the docs.


OK, I think you are right. It doesn't seem like a good idea to warn when 
compiling for one target and not warn when compiling for another target.

If the official documentation says the method should return an object, why not 
prohibit all methods that don't return a pointer to an object (methods like 
returnsInt in the test case)? Doing so will also take care of methods that 
don't return struct but still return via sret (for example, I think some 
targets return vectors via sret).

Also, the documentation says that methods passed to performSelectorInBackground 
and performSelectorOnMainThread should not have a significant return value. 
Does it mean that the methods can have any return type but the return value 
will be ignored (I noticed that those two methods return "void" unlike 
performSelector which returns "id")?


Repository:
  rL LLVM

https://reviews.llvm.org/D30174



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

[PATCH] D30183: Add -iframeworkwithsysroot compiler option

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

This patch adds support for a new `-iframeworkwithsysroot` compiler option 
which allows the user to specify a framework path that can be prefixed with the 
sysroot.

This option is similar to the `-iwithsysroot` option that exists to supplement 
`-isystem`.


Repository:
  rL LLVM

https://reviews.llvm.org/D30183

Files:
  include/clang/Driver/Options.td
  lib/Frontend/CompilerInvocation.cpp
  test/Frontend/iframework.c


Index: test/Frontend/iframework.c
===
--- test/Frontend/iframework.c
+++ test/Frontend/iframework.c
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s 
-Xclang -verify
 // expected-no-diagnostics
 
 #include 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1505,6 +1505,9 @@
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1524,6 +1524,11 @@
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, 
Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, 
Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;


Index: test/Frontend/iframework.c
===
--- test/Frontend/iframework.c
+++ test/Frontend/iframework.c
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s -Xclang -verify
 // expected-no-diagnostics
 
 #include 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1505,6 +1505,9 @@
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1524,6 +1524,11 @@
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30131: [profiling] PR31992: Don't skip interesting non-base constructors

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 89151.
vsk retitled this revision from "[profiling] Don't skip non-base constructors 
if there is a virtual base (fixes PR31992)" to "[profiling] PR31992: Don't skip 
interesting non-base constructors".
vsk edited the summary of this revision.
vsk added a comment.

- Use IsConstructorDelegationValid per Alex's suggestion. This exposed the 
variadic constructor case which we had also missed previously.


https://reviews.llvm.org/D30131

Files:
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenPGO.cpp
  test/Profile/Inputs/cxx-class.proftext
  test/Profile/cxx-class.cpp
  test/Profile/cxx-structors.cpp

Index: test/Profile/cxx-structors.cpp
===
--- test/Profile/cxx-structors.cpp
+++ test/Profile/cxx-structors.cpp
@@ -16,12 +16,28 @@
   ~Bar();
 };
 
+struct Baz : virtual public Foo {
+  Baz() {}
+  Baz(int x) : Foo(x) {}
+  ~Baz();
+};
+
+struct Quux : public Foo {
+  Quux(const char *y, ...) : Foo(0) {}
+};
+
 Foo foo;
 Foo foo2(1);
 Bar bar;
+Baz baz;
+Baz baz2(1);
+Quux qux("fi", "fo", "fum");
 
 // Profile data for complete constructors and destructors must absent.
 
+// INSTR: @__profc__ZN3BazC1Ev =
+// INSTR: @__profc__ZN3BazC1Ei =
+// INSTR: @__profc__ZN4QuuxC1EPKcz =
 // INSTR: @__profc_main =
 // INSTR: @__profc__ZN3FooC2Ev =
 // INSTR: @__profc__ZN3FooD2Ev =
Index: test/Profile/cxx-class.cpp
===
--- test/Profile/cxx-class.cpp
+++ test/Profile/cxx-class.cpp
@@ -5,17 +5,21 @@
 // RUN: FileCheck --input-file=%tgen -check-prefix=DTRGEN %s
 // RUN: FileCheck --input-file=%tgen -check-prefix=MTHGEN %s
 // RUN: FileCheck --input-file=%tgen -check-prefix=WRPGEN %s
+// RUN: FileCheck --input-file=%tgen -check-prefix=VCTRGEN %s
+// RUN: FileCheck --input-file=%tgen -check-prefix=VDTRGEN %s
 
 // RUN: llvm-profdata merge %S/Inputs/cxx-class.proftext -o %t.profdata
 // RUN: %clang_cc1 %s -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -triple %itanium_abi_triple > %tuse
 // RUN: FileCheck --input-file=%tuse -check-prefix=CTRUSE %s
 // RUN: FileCheck --input-file=%tuse -check-prefix=DTRUSE %s
 // RUN: FileCheck --input-file=%tuse -check-prefix=MTHUSE %s
 // RUN: FileCheck --input-file=%tuse -check-prefix=WRPUSE %s
+// RUN: FileCheck --input-file=%tuse -check-prefix=VCTRUSE %s
+// RUN: FileCheck --input-file=%tuse -check-prefix=VDTRUSE %s
 
 class Simple {
-  int Member;
 public:
+  int Member;
   // CTRGEN-LABEL: define {{.*}} @_ZN6SimpleC2Ei(
   // CTRUSE-LABEL: define {{.*}} @_ZN6SimpleC2Ei(
   // CTRGEN: store {{.*}} @[[SCC:__profc__ZN6SimpleC2Ei]], i64 0, i64 0
@@ -56,13 +60,43 @@
   // MTHUSE: ![[SM1]] = !{!"branch_weights", i32 100, i32 2}
 };
 
+class Derived : virtual public Simple {
+public:
+  // VCTRGEN-LABEL: define {{.*}} @_ZN7DerivedC1Ev(
+  // VCTRUSE-LABEL: define {{.*}} @_ZN7DerivedC1Ev(
+  // VCTRGEN: store {{.*}} @[[SCC:__profc__ZN7DerivedC1Ev]], i64 0, i64 0
+  Derived() : Simple(0) {
+// VCTRGEN: store {{.*}} @[[SCC]], i64 0, i64 1
+// VCTRUSE: br {{.*}} !prof ![[SC1:[0-9]+]]
+if (Member) {}
+// VCTRGEN-NOT: store {{.*}} @[[SCC]],
+// VCTRUSE-NOT: br {{.*}} !prof ![0-9]+
+// VCTRUSE: ret
+  }
+  // VCTRUSE: ![[SC1]] = !{!"branch_weights", i32 100, i32 2}
+
+  // VDTRGEN-LABEL: define {{.*}} @_ZN7DerivedD2Ev(
+  // VDTRUSE-LABEL: define {{.*}} @_ZN7DerivedD2Ev(
+  // VDTRGEN: store {{.*}} @[[SDC:__profc__ZN7DerivedD2Ev]], i64 0, i64 0
+  ~Derived() {
+// VDTRGEN: store {{.*}} @[[SDC]], i64 0, i64 1
+// VDTRUSE: br {{.*}} !prof ![[SD1:[0-9]+]]
+if (Member) {}
+// VDTRGEN-NOT: store {{.*}} @[[SDC]],
+// VDTRUSE-NOT: br {{.*}} !prof ![0-9]+
+// VDTRUSE: ret
+  }
+  // VDTRUSE: ![[SD1]] = !{!"branch_weights", i32 100, i32 2}
+};
+
 // WRPGEN-LABEL: define {{.*}} @_Z14simple_wrapperv(
 // WRPUSE-LABEL: define {{.*}} @_Z14simple_wrapperv(
 // WRPGEN: store {{.*}} @[[SWC:__profc__Z14simple_wrapperv]], i64 0, i64 0
 void simple_wrapper() {
   // WRPGEN: store {{.*}} @[[SWC]], i64 0, i64 1
   // WRPUSE: br {{.*}} !prof ![[SW1:[0-9]+]]
   for (int I = 0; I < 100; ++I) {
+Derived d;
 Simple S(I);
 S.method();
   }
Index: test/Profile/Inputs/cxx-class.proftext
===
--- test/Profile/Inputs/cxx-class.proftext
+++ test/Profile/Inputs/cxx-class.proftext
@@ -39,3 +39,14 @@
 100
 99
 
+_ZN7DerivedC1Ev
+10
+2
+100
+99
+
+_ZN7DerivedD2Ev
+10
+2
+100
+99
Index: lib/CodeGen/CodeGenPGO.cpp
===
--- lib/CodeGen/CodeGenPGO.cpp
+++ lib/CodeGen/CodeGenPGO.cpp
@@ -626,10 +626,14 @@
   // Constructors and destructors may be represented by several functions in IR.
   // If so, instrument only base variant, others are implemented by delegation
   // to the base one, it would be counted twice otherwise.
-  if 

r295689 - Add template parameter depth and index to -ast-dump output.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Feb 20 20:04:03 2017
New Revision: 295689

URL: http://llvm.org/viewvc/llvm-project?rev=295689=rev
Log:
Add template parameter depth and index to -ast-dump output.

Modified:
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/test/Misc/ast-dump-decl.cpp
cfe/trunk/test/SemaTemplate/default-expr-arguments-3.cpp

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=295689=295688=295689=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Mon Feb 20 20:04:03 2017
@@ -1463,6 +1463,7 @@ void ASTDumper::VisitTemplateTypeParmDec
 OS << " typename";
   else
 OS << " class";
+  OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
 OS << " ...";
   dumpName(D);
@@ -1472,6 +1473,7 @@ void ASTDumper::VisitTemplateTypeParmDec
 
 void ASTDumper::VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D) 
{
   dumpType(D->getType());
+  OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
 OS << " ...";
   dumpName(D);
@@ -1481,6 +1483,7 @@ void ASTDumper::VisitNonTypeTemplateParm
 
 void ASTDumper::VisitTemplateTemplateParmDecl(
 const TemplateTemplateParmDecl *D) {
+  OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
 OS << " ...";
   dumpName(D);

Modified: cfe/trunk/test/Misc/ast-dump-decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-decl.cpp?rev=295689=295688=295689=diff
==
--- cfe/trunk/test/Misc/ast-dump-decl.cpp (original)
+++ cfe/trunk/test/Misc/ast-dump-decl.cpp Mon Feb 20 20:04:03 2017
@@ -355,8 +355,8 @@ namespace TestTemplateTypeParmDecl {
 }
 // CHECK:  NamespaceDecl{{.*}} TestTemplateTypeParmDecl
 // CHECK-NEXT:   FunctionTemplateDecl
-// CHECK-NEXT: TemplateTypeParmDecl{{.*}} typename ... T
-// CHECK-NEXT: TemplateTypeParmDecl{{.*}} class U
+// CHECK-NEXT: TemplateTypeParmDecl{{.*}} typename depth 0 index 0 ... T
+// CHECK-NEXT: TemplateTypeParmDecl{{.*}} class depth 0 index 1 U
 // CHECK-NEXT:   TemplateArgument type 'int'
 
 namespace TestNonTypeTemplateParmDecl {
@@ -364,10 +364,10 @@ namespace TestNonTypeTemplateParmDecl {
 }
 // CHECK:  NamespaceDecl{{.*}} TestNonTypeTemplateParmDecl
 // CHECK-NEXT:   FunctionTemplateDecl
-// CHECK-NEXT: NonTypeTemplateParmDecl{{.*}} 'int' I
+// CHECK-NEXT: NonTypeTemplateParmDecl{{.*}} 'int' depth 0 index 0 I
 // CHECK-NEXT:   TemplateArgument expr
 // CHECK-NEXT: IntegerLiteral{{.*}} 'int' 1
-// CHECK-NEXT: NonTypeTemplateParmDecl{{.*}} 'int' ... J
+// CHECK-NEXT: NonTypeTemplateParmDecl{{.*}} 'int' depth 0 index 1 ... J
 
 namespace TestTemplateTemplateParmDecl {
   template class A;

Modified: cfe/trunk/test/SemaTemplate/default-expr-arguments-3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/default-expr-arguments-3.cpp?rev=295689=295688=295689=diff
==
--- cfe/trunk/test/SemaTemplate/default-expr-arguments-3.cpp (original)
+++ cfe/trunk/test/SemaTemplate/default-expr-arguments-3.cpp Mon Feb 20 
20:04:03 2017
@@ -37,7 +37,7 @@ template  struct class2 {
 template struct class2;
 
 // CHECK: FunctionTemplateDecl {{.*}} f1
-// CHECK-NEXT: TemplateTypeParmDecl {{.*}} typename T
+// CHECK-NEXT: TemplateTypeParmDecl {{.*}} typename depth 0 index 0 T
 // CHECK-NEXT: FunctionDecl {{.*}} f1 'void (void)'
 // CHECK: FunctionDecl {{.*}} f1 'void (void)'
 // CHECK-NEXT: TemplateArgument type 'int'


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

r295686 - Factor out function to determine whether we're performing a template

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Feb 20 19:17:38 2017
New Revision: 295686

URL: http://llvm.org/viewvc/llvm-project?rev=295686=rev
Log:
Factor out function to determine whether we're performing a template
instantiation.

In preparation for converting the template stack to a more general context
stack (so we can include context notes for other kinds of context).

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=295686=295685=295686=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Feb 20 19:17:38 2017
@@ -7182,6 +7182,20 @@ public:
 operator=(const InstantiatingTemplate&) = delete;
   };
 
+  /// Determine whether we are currently performing template instantiation.
+  bool inTemplateInstantiation() const {
+return ActiveTemplateInstantiations.size() > NonInstantiationEntries;
+  }
+
+  void PrintContextStack() {
+if (!ActiveTemplateInstantiations.empty() &&
+ActiveTemplateInstantiations.back() !=
+LastTemplateInstantiationErrorContext) {
+  PrintInstantiationStack();
+  LastTemplateInstantiationErrorContext =
+  ActiveTemplateInstantiations.back();
+}
+  }
   void PrintInstantiationStack();
 
   /// \brief Determines whether we are currently in a context where

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=295686=295685=295686=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Mon Feb 20 19:17:38 2017
@@ -327,7 +327,7 @@ bool Sema::makeUnavailableInSystemHeader
   if (!fn) return false;
 
   // If we're in template instantiation, it's an error.
-  if (!ActiveTemplateInstantiations.empty())
+  if (inTemplateInstantiation())
 return false;
 
   // If that function's not in a system header, it's an error.
@@ -1006,7 +1006,7 @@ void Sema::EmitCurrentDiagnostic(unsigne
   // and yet we also use the current diag ID on the DiagnosticsEngine. This has
   // been made more painfully obvious by the refactor that introduced this
   // function, but it is possible that the incoming argument can be
-  // eliminnated. If it truly cannot be (for example, there is some reentrancy
+  // eliminated. If it truly cannot be (for example, there is some reentrancy
   // issue I am not seeing yet), then there should at least be a clarifying
   // comment somewhere.
   if (Optional Info = isSFINAEContext()) {
@@ -1094,13 +1094,8 @@ void Sema::EmitCurrentDiagnostic(unsigne
   // that is different from the last template instantiation where
   // we emitted an error, print a template instantiation
   // backtrace.
-  if (!DiagnosticIDs::isBuiltinNote(DiagID) &&
-  !ActiveTemplateInstantiations.empty() &&
-  ActiveTemplateInstantiations.back()
-!= LastTemplateInstantiationErrorContext) {
-PrintInstantiationStack();
-LastTemplateInstantiationErrorContext = 
ActiveTemplateInstantiations.back();
-  }
+  if (!DiagnosticIDs::isBuiltinNote(DiagID))
+PrintContextStack();
 }
 
 Sema::SemaDiagnosticBuilder

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=295686=295685=295686=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Feb 20 19:17:38 2017
@@ -244,7 +244,7 @@ static bool SemaBuiltinSEHScopeCheck(Sem
   // Scopes aren't available during instantiation. Fortunately, builtin
   // functions cannot be template args so they cannot be formed through 
template
   // instantiation. Therefore checking once during the parse is sufficient.
-  if (!SemaRef.ActiveTemplateInstantiations.empty())
+  if (SemaRef.inTemplateInstantiation())
 return false;
 
   Scope *S = SemaRef.getCurScope();
@@ -6782,7 +6782,7 @@ void Sema::CheckMaxUnsignedZero(const Ca
   if (!Call || !FDecl) return;
 
   // Ignore template specializations and macros.
-  if (!ActiveTemplateInstantiations.empty()) return;
+  if (inTemplateInstantiation()) return;
   if (Call->getExprLoc().isMacroID()) return;
 
   // Only care about the one template argument, two function 

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D29819



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

[PATCH] D29967: Get class property selectors from property decl if it exists

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd closed this revision.
compnerd added a comment.

SVN r295683


https://reviews.llvm.org/D29967



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

r295683 - Sema: use PropertyDecl for property selector

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Mon Feb 20 17:45:49 2017
New Revision: 295683

URL: http://llvm.org/viewvc/llvm-project?rev=295683=rev
Log:
Sema: use PropertyDecl for property selector

Using the constructed name for the class properties with dot syntax may
yield an inappropriate selector (i.e. if it is specified via property
attributes).  Prefer the declaration for the selector, falling back to
the constructed name otherwise.

Patch by David Herzka!

Modified:
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/test/SemaObjC/objc-class-property.m

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=295683=295682=295683=diff
==
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Mon Feb 20 17:45:49 2017
@@ -1984,13 +1984,24 @@ ActOnClassPropertyRefExpr(IdentifierInfo
 }
   }
 
+  Selector GetterSel;
+  Selector SetterSel;
+  if (auto PD = IFace->FindPropertyDeclaration(
+  , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+  } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel = SelectorTable::constructSetterSelector(
+PP.getIdentifierTable(), PP.getSelectorTable(), );
+  }
+
   // Search for a declared property first.
-  Selector Sel = PP.getSelectorTable().getNullarySelector();
-  ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+  ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
 
   // If this reference is in an @implementation, check for 'private' methods.
   if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
 
   if (Getter) {
 // FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2011,6 @@ ActOnClassPropertyRefExpr(IdentifierInfo
   }
 
   // Look for the matching setter, in case it is needed.
-  Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
-   );
-
   ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
   if (!Setter) {
 // If this reference is in an @implementation, also check for 'private'

Modified: cfe/trunk/test/SemaObjC/objc-class-property.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-class-property.m?rev=295683=295682=295683=diff
==
--- cfe/trunk/test/SemaObjC/objc-class-property.m (original)
+++ cfe/trunk/test/SemaObjC/objc-class-property.m Mon Feb 20 17:45:49 2017
@@ -21,6 +21,8 @@
 @property (class) int c2; // expected-note {{property declared here}} \
   // expected-note {{property declared here}}
 @property (class) int x;
+@property (class, setter=customSet:) int customSetterProperty;
+@property (class, getter=customGet) int customGetterProperty;
 @end
 
 @implementation A // expected-warning {{class property 'c2' requires method 
'c2' to be defined}} \
@@ -29,6 +31,8 @@
 @dynamic (class) x; // refers to the class property
 @synthesize z, c2; // expected-error {{@synthesize not allowed on a class 
property 'c2'}}
 @dynamic c; // refers to the class property
+@dynamic customSetterProperty;
+@dynamic customGetterProperty;
 @end
 
 int test() {
@@ -37,6 +41,11 @@ int test() {
   return a.x + A.c;
 }
 
+void customSelectors() {
+  A.customSetterProperty = 1;
+  (void)A.customGetterProperty;
+}
+
 void message_id(id me) {
   [me y];
 }


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

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D30174#681801, @ahatanak wrote:

> Do we still issue a warning even when the struct can be returned in a 
> register? For example, x86 can return a small struct (for example, a struct 
> with one int field) in a single register, in which case it's fine to pass it 
> to performSelector via @selector.


Yes, we do. Primarily for the following reason: even if some target may return 
a struct in a register, another target isn't guaranteed to do the same thing. 
It's better to always warn about this rather than accept some small structs. 
Furthermore, the official documentation states that "For methods that return 
anything other than an object, use NSInvocation." [1], so methods that return 
records are completely prohibited by the docs.

> If we should warn only when the method has to return via sret, then it looks 
> like we have to delay issuing the warning until we know where the return 
> value goes (IRGen?).

[1] 
https://developer.apple.com/reference/objectivec/1418956-nsobject/1418867-performselector?language=objc


Repository:
  rL LLVM

https://reviews.llvm.org/D30174



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

Re: r295592 - clang/CMakeLists.txt: Rework r294954 -- use file(TO_CMAKE_PATH).

[NAKAMURA Takumi via cfe-commits]

Hans, could you pull this (and r294954) into release_40, please?

This is a regression from previous releases that clang standalone build
cannot be configured on msbuild.

On Sun, Feb 19, 2017 at 12:29 PM NAKAMURA Takumi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: chapuni
> Date: Sat Feb 18 21:17:31 2017
> New Revision: 295592
>
> URL: http://llvm.org/viewvc/llvm-project?rev=295592=rev
> Log:
> clang/CMakeLists.txt: Rework r294954 -- use file(TO_CMAKE_PATH).
>
> Modified:
> cfe/trunk/CMakeLists.txt
>
> Modified: cfe/trunk/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=295592=295591=295592=diff
>
> ==
> --- cfe/trunk/CMakeLists.txt (original)
> +++ cfe/trunk/CMakeLists.txt Sat Feb 18 21:17:31 2017
> @@ -59,7 +59,7 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
>
># Normalize LLVM_CMAKE_PATH. --cmakedir might contain backslashes.
># CMake assumes slashes as PATH.
> -  get_filename_component(LLVM_CMAKE_PATH ${LLVM_CONFIG_CMAKE_PATH}
> ABSOLUTE)
> +  file(TO_CMAKE_PATH ${LLVM_CONFIG_CMAKE_PATH} LLVM_CMAKE_PATH)
>
>find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
>  NO_DEFAULT_PATH)
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30166: Honor __unaligned in codegen for declarations and expressions

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/AST/ASTContext.h:1910
+if (T.getQualifiers().hasUnaligned())
+  TI.Align = 8;
+return TI;

Is it better to call TargetInfo::getCharWidth() instead of assigning a 
hardcoded number here?



Comment at: lib/AST/ExprConstant.cpp:5678
+return CharUnits::One();
+  else
+return Info.Ctx.toCharUnitsFromBits(

You can remove an else after return.


https://reviews.llvm.org/D30166



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

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Do we still issue a warning even when the struct can be returned in a register? 
For example, x86 can return a small struct (for example, a struct with one int 
field) in a single register, in which case it's fine to pass it to 
performSelector via @selector.

If we should warn only when the method has to return via sret, then it looks 
like we have to delay issuing the warning until we know where the return value 
goes (IRGen?).


Repository:
  rL LLVM

https://reviews.llvm.org/D30174



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

Re: [PATCH] D27827: [ObjC] CodeGen support for @available on macOS

[Duncan P. N. Exon Smith via cfe-commits]

> On 2017-Feb-20, at 13:11, Alex Lorenz via Phabricator 
>  wrote:
> 
> arphaman added inline comments.
> 
> 
> 
> Comment at: lib/CodeGen/CodeGenFunction.h:2479
> 
> +  llvm::Value *EmitObjCIsOSVersionAtLeast(ArrayRef Args);
> +
> 
> I think it's better to treat this as a builtin in its own right, without 
> including the ObjC part in the names. This function could be renamed to 
> something like `EmitBuiltinAvailable` and the variable that holds the 
> function pointer should be renamed appropriately as well.
> 
> 
> 
> Comment at: test/CodeGenObjC/availability-check.m:5
> +void use_at_available() {
> +  // CHECK: call i32 @_IsOSVersionAtLeast(i32 10, i32 12, i32 0)
> +  // CHECK-NEXT: icmp ne
> 
> I think the function name should have the lowercase `is`

Keep in mind that we need this to be a reserved identifier, so it either has to 
start with two underscores or one underscore with an uppercase letter.  In 
other words, don't forget to add an underscore if you lowercase it.

> 
> 
> 
> Comment at: test/CodeGenObjC/availability-check.m:7
> +  // CHECK-NEXT: icmp ne
> +  if (__builtin_available(macos 10.12, *))
> +;
> 
> Can you add a call to the builtin that has a non-zero sub-minor version as 
> well?
> 
> 
> https://reviews.llvm.org/D27827
> 
> 
> 

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

[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.h:2479
 
+  llvm::Value *EmitObjCIsOSVersionAtLeast(ArrayRef Args);
+

I think it's better to treat this as a builtin in its own right, without 
including the ObjC part in the names. This function could be renamed to 
something like `EmitBuiltinAvailable` and the variable that holds the function 
pointer should be renamed appropriately as well.



Comment at: test/CodeGenObjC/availability-check.m:5
+void use_at_available() {
+  // CHECK: call i32 @_IsOSVersionAtLeast(i32 10, i32 12, i32 0)
+  // CHECK-NEXT: icmp ne

I think the function name should have the lowercase `is`



Comment at: test/CodeGenObjC/availability-check.m:7
+  // CHECK-NEXT: icmp ne
+  if (__builtin_available(macos 10.12, *))
+;

Can you add a call to the builtin that has a non-zero sub-minor version as well?


https://reviews.llvm.org/D27827



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

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp:375
+  "MismatchedIterator");
+auto *N = C.generateNonFatalErrorNode(State, );
+if (!N) {

This can be rewritten as:

```
if (auto *N = C.generateNonFatalErrorNode(State, ))
  reportMismatchedBug("Iterator access mismatched.", Iter1, C, N);

```


https://reviews.llvm.org/D29419



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

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

The `performSelector` family of methods from Foundation use `objc_msgSend` to 
dispatch the selector invocations to objects. However, method calls to methods 
that return record types might have to use the `objc_msgSend_stret` as the 
return value won't find into the register [1]. This is also supported by this 
sentence from `performSelector` documentation: "The method should not have a 
significant return value and should take a single argument of type id, or no 
arguments" [2]. This patch adds a new warning that warns when a selector which 
corresponds to a method that returns a record type is passed into 
`performSelector`.

[1] 
http://www.tomdalling.com/blog/cocoa/why-performselector-is-more-dangerous-than-i-thought/
[2] 
https://developer.apple.com/reference/objectivec/nsobject/1416176-performselector?language=objc


Repository:
  rL LLVM

https://reviews.llvm.org/D30174

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/DeclObjC.cpp
  lib/Basic/IdentifierTable.cpp
  lib/Sema/SemaExprObjC.cpp
  test/SemaObjC/unsafe-perform-selector.m

Index: test/SemaObjC/unsafe-perform-selector.m
===
--- /dev/null
+++ test/SemaObjC/unsafe-perform-selector.m
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -verify %s
+// rdar://12056271
+
+__attribute__((objc_root_class))
+@interface NSObject
+
+- (id)performSelector:(SEL)sel;
+- (void)performSelectorInBackground:(SEL)sel withObject:(id)arg;
+- (void)performSelectorOnMainThread:(SEL)sel;
+
+@end
+
+typedef struct { int x; int y; int width; int height; } Rectangle;
+
+struct Struct { Rectangle r; };
+
+typedef union { int x; float f; } Union;
+
+@interface Base : NSObject
+
+- (struct Struct)returnsStruct2; // expected-note {{method 'returnsStruct2' that returns 'struct Struct' declared here}}
+- (Union)returnsId;
+
+@end
+
+@protocol IP
+
+- (Union)returnsUnion; // expected-note {{method 'returnsUnion' that returns 'Union' declared here}}
+
+@end
+
+@interface I : Base
+
+- (Rectangle)returnsStruct; // expected-note 2 {{method 'returnsStruct' that returns 'Rectangle' declared here}}
+- (id)returnsId; // shadows base 'returnsId'
+- (int)returnsInt;
+- (I *)returnPtr;
+
++ (Rectangle)returnsStructClass; // expected-note {{method 'returnsStructClass' that returns 'Rectangle' declared here}}
++ (void)returnsUnion; // Not really
+
+@end
+
+void foo(I *i) {
+  [i performSelector: @selector(returnsStruct)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+  [i performSelectorInBackground: @selector(returnsStruct) withObject:0]; // expected-warning {{'performSelectorInBackground:withObject:' is incompatible with selectors that return a struct type}}
+  [i performSelector: ((@selector(returnsUnion)))]; // expected-warning {{'performSelector:' is incompatible with selectors that return a union type}}
+  [i performSelectorOnMainThread: @selector(returnsStruct2)]; // expected-warning {{'performSelectorOnMainThread:' is incompatible with selectors that return a struct type}}
+  [I performSelector: (@selector(returnsStructClass))]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+
+  [i performSelector: @selector(returnsId)];
+  [i performSelector: @selector(returnsInt)];
+  [i performSelector: @selector(returnsPtr)];
+  [I performSelector: @selector(returnsUnion)]; // No warning expected
+}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2260,6 +2260,41 @@
  edit::rewriteObjCRedundantCallWithLiteral);
 }
 
+static void checkFoundationAPI(Sema , SourceLocation Loc,
+   const ObjCMethodDecl *Method,
+   ArrayRef Args, QualType ReceiverType,
+   bool IsInstanceMethod) {
+  // Check if this is a performSelector method that uses a selector that returns
+  // a record type.
+  if (Method->getMethodFamily() != OMF_performSelector || Args.empty())
+return;
+  const auto *SE = dyn_cast(Args[0]->IgnoreParens());
+  if (!SE)
+return;
+  ObjCMethodDecl *ImpliedMethod;
+  if (IsInstanceMethod) {
+const auto *OPT = ReceiverType->getAs();
+if (!OPT)
+  return;
+ImpliedMethod =
+OPT->getInterfaceDecl()->lookupInstanceMethod(SE->getSelector());
+  } else {
+const auto *IT = ReceiverType->getAs();
+if (!IT)
+  return;
+ImpliedMethod = IT->getDecl()->lookupClassMethod(SE->getSelector());
+  }
+  if (ImpliedMethod && ImpliedMethod->getReturnType()->isRecordType()) {
+QualType Ret = ImpliedMethod->getReturnType();
+S.Diag(Loc, diag::warn_objc_unsafe_perform_selector)
+<< Method->getSelector()
+<< (Ret->isUnionType() ? /*Union*/ 1 : /*Struct*/ 0);
+

r295674 - [Sema][ObjC] perform-selector ARC check should see @selector in parens

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Feb 20 11:55:15 2017
New Revision: 295674

URL: http://llvm.org/viewvc/llvm-project?rev=295674=rev
Log:
[Sema][ObjC] perform-selector ARC check should see @selector in parens

Modified:
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/test/SemaObjC/arc-peformselector.m

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=295674=295673=295674=diff
==
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Mon Feb 20 11:55:15 2017
@@ -2920,7 +2920,8 @@ ExprResult Sema::BuildInstanceMessage(Ex
 
 case OMF_performSelector:
   if (Method && NumArgs >= 1) {
-if (ObjCSelectorExpr *SelExp = dyn_cast(Args[0])) {
+if (const auto *SelExp =
+dyn_cast(Args[0]->IgnoreParens())) {
   Selector ArgSel = SelExp->getSelector();
   ObjCMethodDecl *SelMethod = 
 LookupInstanceMethodInGlobalPool(ArgSel,

Modified: cfe/trunk/test/SemaObjC/arc-peformselector.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-peformselector.m?rev=295674=295673=295674=diff
==
--- cfe/trunk/test/SemaObjC/arc-peformselector.m (original)
+++ cfe/trunk/test/SemaObjC/arc-peformselector.m Mon Feb 20 11:55:15 2017
@@ -27,6 +27,7 @@
   return [self performSelector : @selector(init)];
   return [self performSelector : sel1]; // expected-warning {{performSelector 
may cause a leak because its selector is unknown}} \
// expected-note {{used here}}
+  return [self performSelector: (@selector(PlusZero))];
 
   return [self performSelector : @selector(PlusZero)];
   return [self performSelector : @selector(PlusOne)]; // expected-error 
{{performSelector names a selector which retains the object}}


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

[PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

[Michael via Phabricator via cfe-commits]

redm123 added inline comments.



Comment at: test/Misc/diag-template-diffing.cpp:27
 // CHECK-ELIDE-NOTREE: no matching function for call to 'f'
-// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 
'vector' to 'vector' for 1st argument
+// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 
'vector' to 'vector' for 1st argument
 // CHECK-NOELIDE-NOTREE: no matching function for call to 'f'

redm123 wrote:
> arphaman wrote:
> > I think the majority of test changes make sense, we are just adding 
> > qualifiers to the typedefs in the diagnostics. I'm curious about this one 
> > though, as we are essentially replacing the old diagnostic note `no known 
> > conversion from 'vector' to 'vector' for 
> > 1st argument` by the new note `no known conversion from 
> > 'vector' to 'vector' for 1st argument`. Is one better 
> > than the other? It seems that GCC prefers the former, while ICC the latter. 
> > Does it even matter which one we have?
> Right, wanted to add a note... forgot about that. This "fix" here is probably 
> entirely nonsense. I guess I broke some other feature here. I was hoping for 
> some input from the review ;) As I see it it's supposed to print the 
> canonical type in case both types would be printed the same, but apparently 
> the comparison breaks.  The question where this happens... I'll try to dig 
> through the history and see if I find something.
So the problematic place seems to be in lib/AST/ASTDiagnostic.cpp: 
PrintTypeNames() which compares the printed type names, and as this 
versa_string has no namespace it breaks... We could restore the old behavior 
for these tests with something like:

  Index: lib/AST/ASTDiagnostic.cpp
  ===
  --- lib/AST/ASTDiagnostic.cpp   (revision 294732)
  +++ lib/AST/ASTDiagnostic.cpp   (working copy)
  @@ -1612,10 +1612,12 @@
 return;
   }
  
  +PrintingPolicy ComparePolicy = Policy;
  +ComparePolicy.SuppressScope = true;
   std::string FromTypeStr = FromType.isNull() ? "(no argument)"
  -: 
FromType.getAsString(Policy);
  +: 
FromType.getAsString(ComparePolicy);
   std::string ToTypeStr = ToType.isNull() ? "(no argument)"
  -: ToType.getAsString(Policy);
  +: 
ToType.getAsString(ComparePolicy);
   // Switch to canonical typename if it is better.
   // TODO: merge this with other aka printing above.
   if (FromTypeStr == ToTypeStr) {
  
But as this disables namespace scopes also for normal types (not just 
typedefs), things now break further down...

So what would be a proper fix...?

If I see it right this test case only breaks in the first place because it uses 
typedefs and no namespaces were printed for typedefs up to now. I.e. you get 
"no known conversion from 'vector' to 'vector'", as the comment 
says, which is indeed somewhat ambiguous. With my patch you now get "no known 
conversion from 'vector' to 'vector'" ... better.

Interestingly if you have the same example with no typedefs, but just "class 
string;", you get the same error, as in case of non-typedefs the namespaces 
were always printed. In this case it seems to be sufficiently non-ambiguous. 

**So**: As with my patch we seem to achive the same result, we could also 
remove this entire test, because this case can't happen anymore.
**Further so**: Actually we might also remove the entire check and fallback to 
canonical types in PrintTypeNames(), as this also can't happen anymore... 

At least AFACIS... 

Thoughts?




https://reviews.llvm.org/D29944



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

[PATCH] D30170: Function definition may have uninstantiated body

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.

Current implementation of `FunctionDecl::isDefined` does not take into
account declarations that do not have a body, but it can be instantiated
from a templated definition. This behavior creates problems when
processing friend functions defined in class templates. For instance,
the code:

  template struct X { friend void f() {} };
  X xi;
  void f() {}

compiles successfully but must fail due to redefinition of `f`. The
declaration of the friend `f` is created when the containing template
`X` is instantiated, but it does not have a body as per 14.5.4p4
because `f` is not odr-used.

With this change the function `Sema::CheckForFunctionRedefinition`
considers functions with uninstantiated bodies as definitions.


https://reviews.llvm.org/D30170

Files:
  include/clang/AST/Decl.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -101,6 +101,34 @@
   friend void func_12(int x = 0);  // expected-error{{friend declaration specifying a default argument must be the only declaration}}
 };
 
+// Friend function with uninstantiated body is still a definition.
+
+template struct C20 {
+  friend void func_20() {} // expected-note{{previous definition is here}}
+};
+C20 c20i;
+void func_20() {} // expected-error{{redefinition of 'func_20'}}
+
+template struct C21a {
+  friend void func_21() {} // expected-note{{previous definition is here}}
+};
+template struct C21b {
+  friend void func_21() {} // expected-error{{redefinition of 'func_21'}}
+};
+C21a c21ai;
+C21b c21bi; // expected-note{{in instantiation of template class 'C21b' requested here}}
+
+template struct C22a {
+  friend void func_22() {} // expected-note{{previous definition is here}}
+};
+template struct C22b {
+  friend void func_22();
+};
+C22a c22ai;
+C22b c22bi;
+void func_22() {} // expected-error{{redefinition of 'func_22'}}
+
+
 
 namespace pr22307 {
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11692,8 +11692,12 @@
SkipBodyInfo *SkipBody) {
   const FunctionDecl *Definition = EffectiveDefinition;
   if (!Definition)
-if (!FD->isDefined(Definition))
+if (!FD->isOdrDefined(Definition))
   return;
+  assert(Definition);
+
+  if (FD == Definition)
+return;
 
   if (canRedefineFunction(Definition, getLangOpts()))
 return;
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2538,6 +2538,17 @@
   return false;
 }
 
+bool FunctionDecl::isOdrDefined(const FunctionDecl *) const {
+  for (auto I : redecls()) {
+if (I->isThisDeclarationAnOdrDefinition()) {
+  Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
+  return true;
+}
+  }
+
+  return false;
+}
+
 Stmt *FunctionDecl::getBody(const FunctionDecl *) const {
   if (!hasBody(Definition))
 return nullptr;
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1781,32 +1781,103 @@
 
   SourceRange getSourceRange() const override LLVM_READONLY;
 
-  /// \brief Returns true if the function has a body (definition). The
-  /// function body might be in any of the (re-)declarations of this
-  /// function. The variant that accepts a FunctionDecl pointer will
-  /// set that function declaration to the actual declaration
-  /// containing the body (if there is one).
+  /// \name Definition and body checks
+  ///
+  /// A function declaration may be:
+  /// - a non defining declaration,
+  /// - a definition. A function may be defined because:
+  ///   - it has a body, or will have it in the case of late parsing.
+  ///   - it has an uninstantiated body. The body does not exist because the
+  /// function is not used yet, but the declaration is considered a
+  /// definition from viewpoint of ODR checks.
+  ///   - it does not have a user specified body, but it does not allow
+  /// redefinition, because it is deleted/defaulted or is defined through
+  /// some other mechanism (alias, ifunc).
+  ///
+  /// Depending on whether the redeclaration chain contains a definition and of
+  /// what kind, the same classification applies to the function represented by
+  /// a set of redeclarations.
+  ///
+  /// \{
+
+  /// Returns true if the function has a body.
+  ///
+  /// The function body might be in any of the (re-)declarations of this
+  /// function. The variant that accepts a FunctionDecl pointer will set that
+  /// function declaration to the actual declaration containing the body (if
+  /// there is one).
+  ///
   bool hasBody(const FunctionDecl *) const;
 
   bool hasBody() const override {
 const 

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

[Balogh , Ádám via Phabricator via cfe-commits]

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



Comment at: lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp:311
+
+void MismatchedIteratorChecker::checkPostStmt(const DeclStmt *DS,
+  CheckerContext ) const {

NoQ wrote:
> Hmm. Now i suspect that the `checkBind()` callback should have covered this, 
> both here and in the previous checker. Did you try using that instead, and 
> see if other callbacks are covered by `checkBind()` as well?
You are right. It seems to eliminate the need for checking C++ construct 
expressions, declarations with initial values and assignments.


https://reviews.llvm.org/D29419



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

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 89123.
baloghadamsoftware added a comment.

Updated according to the comments.


https://reviews.llvm.org/D29419

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- /dev/null
+++ test/Analysis/mismatched-iterator.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-eagerly-assume %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void good_insert1(std::vector , int n) {
+  v.insert(v.cbegin(), n); // no-warning
+}
+
+
+void good_insert2(std::vector , int len, int n) {
+  v.insert(v.cbegin(), len, n); // no-warning
+}
+
+void good_insert3(std::vector , std::vector ) {
+  v1.insert(v1.cbegin(), v2.cbegin(), v2.cend()); // no-warning
+}
+
+void good_insert4(std::vector , int len, int n) {
+  v.insert(v.cbegin(), {n-1, n, n+1}); // no-warning
+}
+
+void good_insert_find(std::vector , int n, int m) {
+  auto i = std::find(v.begin(), v.end(), n);
+  v.insert(i, m); // no-warning
+}
+
+void good_erase1(std::vector ) {
+  v.erase(v.cbegin()); // no-warning
+}
+
+void good_erase2(std::vector ) {
+  v.erase(v.cbegin(), v.cend()); // no-warning
+}
+
+void good_emplace(std::vector , int n) {
+  v.emplace(v.cbegin(), n); // no-warning
+}
+
+void good_ctor(std::vector ) {
+  std::vector new_v(v.begin(), v.end()); // no-warning
+}
+
+void good_find(std::vector , int n) {
+  std::find(v.begin(), v.end(), n); // no-warning
+}
+
+void good_find_first_of(std::vector , std::vector ) {
+  std::find_first_of(v1.begin(), v1.end(), v2.begin(), v2.end()); // no-warning
+}
+
+void good_comparison(std::vector ) {
+  if (v.begin() == v.end()) {} // no-warning
+}
+
+void bad_insert1(std::vector , std::vector , int n) {
+  v2.insert(v1.cbegin(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_insert2(std::vector , std::vector , int len, int n) {
+  v2.insert(v1.cbegin(), len, n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_insert3(std::vector , std::vector ) {
+  v2.insert(v1.cbegin(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
+  v1.insert(v1.cbegin(), v1.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
+  v1.insert(v1.cbegin(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_insert4(std::vector , std::vector , int len, int n) {
+  v2.insert(v1.cbegin(), {n-1, n, n+1}); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_erase1(std::vector , std::vector ) {
+  v2.erase(v1.cbegin()); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_erase2(std::vector , std::vector ) {
+  v2.erase(v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
+  v2.erase(v1.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
+  v2.erase(v1.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_emplace(std::vector , std::vector , int n) {
+  v2.emplace(v1.cbegin(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_ctor(std::vector , std::vector ) {
+  std::vector new_v(v1.begin(), v2.end()); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_find(std::vector , std::vector , int n) {
+  std::find(v1.begin(), v2.end(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_find_first_of(std::vector , std::vector ) {
+  std::find_first_of(v1.begin(), v2.end(), v2.begin(), v2.end()); // expected-warning{{Iterator access mismatched}}
+  std::find_first_of(v1.begin(), v1.end(), v2.begin(), v1.end()); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_comparison(std::vector , std::vector ) {
+  if (v1.begin() != v2.end()) { // expected-warning{{Iterator access mismatched}}
+*v1.begin();
+  }
+}
+
+void bad_insert_find(std::vector , std::vector , int n, int m) {
+  auto i = std::find(v1.begin(), v1.end(), n);
+  v2.insert(i, m); // expected-warning{{Iterator access mismatched}}
+}
+
+void good_overwrite(std::vector , std::vector , int n) {
+  auto i = v1.cbegin();
+  i = v2.cbegin();
+  v2.insert(i, n); // no-warning
+}
+
+void bad_overwrite(std::vector , std::vector , int n) {
+  auto i = v1.cbegin();
+  i = v2.cbegin();
+  v1.insert(i, n); // expected-warning{{Iterator access mismatched}}
+}
+
+template
+bool is_end(Container cont, Iterator it) {
+  return it == cont.end();
+}
+
+void good_empty(std::vector ) {
+  is_end(v, v.begin()); // no-warning
+}
+
+void bad_empty(std::vector , std::vector ) {
+  is_end(v1, v2.begin()); // 

[PATCH] D29770: [Assembler] Inline assembly diagnostics test.

[Sanne Wouda via Phabricator via cfe-commits]

sanwou01 abandoned this revision.
sanwou01 added a comment.

Please see https://reviews.llvm.org/D30167 for an attempt to test this from llc.


https://reviews.llvm.org/D29770



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

[PATCH] D30166: Honor __unaligned in codegen for declarations and expressions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.

See related proposal in 
http://lists.llvm.org/pipermail/cfe-dev/2017-February/052739.html

This patch honors the unaligned type qualifier (currently available through he 
keyword __unaligned and -fms-extensions) in CodeGen. In the current form the 
patch affects declarations and expressions. It does not affect fields of 
classes.


https://reviews.llvm.org/D30166

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  test/CodeGen/unaligned-decl.c
  test/CodeGen/unaligned-expr.c
  test/CodeGen/unaligned-field.c
  test/CodeGenCXX/unaligned.cpp

Index: test/CodeGenCXX/unaligned.cpp
===
--- /dev/null
+++ test/CodeGenCXX/unaligned.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+int foo() {
+  // CHECK: ret i32 1
+  return alignof(__unaligned int);
+}
Index: test/CodeGen/unaligned-field.c
===
--- /dev/null
+++ test/CodeGen/unaligned-field.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+// Test that __unaligned does not impact the layout of the fields.
+
+struct A
+{
+char a;
+__unaligned int b;
+} a;
+// CHECK: %struct.A = type { i8, i32 }
+
+struct A2
+{
+int b;
+char a;
+__unaligned int c;
+} a2;
+// CHECK: %struct.A2 = type { i32, i8, i32 }
Index: test/CodeGen/unaligned-expr.c
===
--- /dev/null
+++ test/CodeGen/unaligned-expr.c
@@ -0,0 +1,217 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+// -
+// Scalar integer
+// -
+__unaligned int x;
+void test1(void) {
+  // CHECK: {{%.*}} = load i32, i32* @x, align 1
+  // CHECK: store i32 {{%.*}}, i32* @x, align 1
+  x++;
+}
+
+void test2(void) {
+  // CHECK: %y = alloca i32, align 1
+  // CHECK: {{%.*}} = load i32, i32* %y, align 1
+  // CHECK: store i32 {{%.*}}, i32* %y, align 1
+  __unaligned int y;
+  y++;
+}
+
+void test2_1(void) {
+  // CHECK: %y = alloca i32, align 1
+  // CHECK: store i32 1, i32* %y, align 1
+  __unaligned int y = 1;
+}
+
+// -
+// Global pointer
+// -
+int *__unaligned p1;
+void test3(void) {
+
+  // CHECK: {{%.*}} = load i32*, i32** @p1, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 4
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 4
+  (*p1)++;
+}
+
+int __unaligned *p2;
+void test4(void) {
+  // CHECK: {{%.*}} = load i32*, i32** @p2, align 8
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  (*p2)++;
+}
+
+int __unaligned *__unaligned p3;
+void test5(void) {
+  // CHECK: {{%.*}} = load i32*, i32** @p3, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  (*p3)++;
+}
+
+// -
+// Local pointer
+// -
+void test6(void) {
+  // CHECK: %lp1 = alloca i32*, align 1
+  // CHECK: {{%.*}} = load i32*, i32** %lp1, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 4
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 4
+  int *__unaligned lp1;
+  (*lp1)++;
+}
+
+void test7(void) {
+  // CHECK: %lp2 = alloca i32*, align 8
+  // CHECK: {{%.*}} = load i32*, i32** %lp2, align 8
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  int __unaligned *lp2;
+  (*lp2)++;
+}
+
+void test8(void) {
+  // CHECK: %lp3 = alloca i32*, align 1
+  // CHECK: {{%.*}} = load i32*, i32** %lp3, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  int __unaligned *__unaligned lp3;
+  (*lp3)++;
+}
+
+// -
+// Global array
+// -
+__unaligned int a[10];
+void test9(void) {
+  // CHECK: {{%.*}} = load i32, i32* getelementptr inbounds ([10 x i32], [10 x i32]* @a, i64 0, i64 3), align 1
+  // CHECK: store i32 {{%.*}}, i32* getelementptr inbounds ([10 x i32], [10 x i32]* @a, i64 0, i64 3), align 1
+  (a[3])++;
+}
+
+// -
+// Local array
+// -
+void test10(void) {
+  // CHECK: %la = alloca [10 x i32], align 1
+  // CHECK: {{%.*}} = getelementptr inbounds [10 x i32], [10 x i32]* %la, i64 0, i64 3
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  __unaligned int la[10];
+  (la[3])++;
+}
+
+// 
+// Typedefs
+// 
+
+typedef __unaligned int UnalignedInt;
+void test13() {
+  // CHECK: %i = alloca i32, align 1
+  // CHECK: {{%.*}} = load i32, i32* %i, align 1
+  // CHECK: store i32 {{%.*}}, i32* %i, align 1
+  UnalignedInt i;
+  i++;
+}
+
+typedef int Aligned;
+typedef __unaligned Aligned UnalignedInt2;
+void test14() {

r295663 - clang-format: [JS] Improve line-wrapping behavior of template strings.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Feb 20 08:51:16 2017
New Revision: 295663

URL: http://llvm.org/viewvc/llvm-project?rev=295663=rev
Log:
clang-format: [JS] Improve line-wrapping behavior of template strings.

Specifically, similar to other blocks, clang-format now wraps both
after "${" and before the corresponding "}", if the contained
expression spans multiple lines.

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=295663=295662=295663=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Feb 20 08:51:16 2017
@@ -383,6 +383,8 @@ void ContinuationIndenter::addTokenOnCur
   Current.FakeLParens.size() > 0 &&
   Current.FakeLParens.back() > prec::Unknown)
 State.Stack.back().NoLineBreak = true;
+  if (Previous.is(TT_TemplateString) && Previous.opensScope())
+State.Stack.back().NoLineBreak = true;
 
   if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&
   Previous.opensScope() && Previous.isNot(TT_ObjCMethodExpr) &&
@@ -398,7 +400,7 @@ void ContinuationIndenter::addTokenOnCur
 State.Stack.back().NoLineBreak = true;
   if (Current.isMemberAccess() && Previous.is(tok::r_paren) &&
   (Previous.MatchingParen &&
-   (Previous.TotalLength - Previous.MatchingParen->TotalLength > 10))) {
+   (Previous.TotalLength - Previous.MatchingParen->TotalLength > 10)))
 // If there is a function call with long parameters, break before trailing
 // calls. This prevents things like:
 //   EXPECT_CALL(SomeLongParameter).Times(
@@ -406,7 +408,6 @@ void ContinuationIndenter::addTokenOnCur
 // We don't want to do this for short parameters as they can just be
 // indexes.
 State.Stack.back().NoLineBreak = true;
-  }
 
   // Don't allow the RHS of an operator to be split over multiple lines unless
   // there is a line-break right after the operator.
@@ -618,7 +619,9 @@ unsigned ContinuationIndenter::addTokenO
   // If we break after { or the [ of an array initializer, we should also break
   // before the corresponding } or ].
   if (PreviousNonComment &&
-  (PreviousNonComment->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare)))
+  (PreviousNonComment->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
+   (PreviousNonComment->is(TT_TemplateString) &&
+PreviousNonComment->opensScope(
 State.Stack.back().BreakBeforeClosingBrace = true;
 
   if (State.Stack.back().AvoidBinPacking) {
@@ -666,6 +669,8 @@ unsigned ContinuationIndenter::getNewLin
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
+  if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
+return State.Stack[State.Stack.size() - 2].LastSpace;
   if (Current.is(tok::identifier) && Current.Next &&
   Current.Next->is(TT_DictLiteral))
 return State.Stack.back().Indent;
@@ -840,6 +845,11 @@ unsigned ContinuationIndenter::moveState
 
   moveStatePastFakeLParens(State, Newline);
   moveStatePastScopeCloser(State);
+  if (Current.is(TT_TemplateString) && Current.opensScope())
+State.Stack.back().LastSpace =
+(Current.IsMultiline ? Current.LastLineColumnWidth
+ : State.Column + Current.ColumnWidth) -
+strlen("${");
   moveStatePastScopeOpener(State, Newline);
   moveStatePastFakeRParens(State);
 

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=295663=295662=295663=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Mon Feb 20 08:51:16 2017
@@ -455,6 +455,8 @@ struct FormatToken {
   /// \brief Returns \c true if this tokens starts a block-type list, i.e. a
   /// list that should be indented with a block indent.
   bool opensBlockOrBlockTypeList(const FormatStyle ) const {
+if (is(TT_TemplateString) && opensScope())
+  return true;
 return is(TT_ArrayInitializerLSquare) ||
(is(tok::l_brace) &&
 (BlockKind == BK_Block || is(TT_DictLiteral) ||
@@ -463,6 +465,8 @@ struct FormatToken {
 
   /// \brief Same as opensBlockOrBlockTypeList, but for the closing token.
   bool closesBlockOrBlockTypeList(const FormatStyle ) const {
+if (is(TT_TemplateString) && closesScope())
+  return true;
 return MatchingParen && MatchingParen->opensBlockOrBlockTypeList(Style);
   }
 

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 

[PATCH] D28445: [Analyzer] Extend taint propagation and checking

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yeah, i think this is now as easy as i expected it to be :)

Still, the new API is in need of better documentation, because the notion of 
default binding is already confusing, and the new use-case we now have for this 
API is even more confusing. I don't instantly see a good way to justify this 
hack, but some day we'd need to either do that or refactor further. The notion 
of default binding needs to become more "material".

A more expanded doxygen comment should probably be a great start.




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/Store.h:66
+  /// \return The default value bound to the LazyCompoundVal \c lcv.
+  virtual SVal getDefaultBinding(Store store, nonloc::LazyCompoundVal lcv) = 0;
+

I think there are two different use cases here:
1. `getDefaultBinding(Store, Region)` would retrieve default binding in a given 
store for the region's base region,
2. `getDefaultBinding(LazyCompoundVal)` would retrieve default binding for the 
lazy compound value's base region from the lazy compound value's store - a 
shorthand for `getDefaultBinding(lcv.getStore(), lcv.getRegion())`.

Otherwise we're passing two different stores into the function (one directly, 
another as part of the lazy compound value), and it's confusing which one is 
actually used.





Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:498
+  SVal getDefaultBinding(Store S, nonloc::LazyCompoundVal L) override {
+if (!L.getRegion())
+  return UnknownVal();

`LazyCompoundVal` always has a region.



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:505
+
+return UnknownVal();
+  }

Because UnknownVal is an interesting default binding, quite different from lack 
of default binding, i'd rather return `Optional` from that function (and 
None here).


https://reviews.llvm.org/D28445



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

[PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

[Michael via Phabricator via cfe-commits]

redm123 added inline comments.



Comment at: test/Misc/diag-template-diffing.cpp:27
 // CHECK-ELIDE-NOTREE: no matching function for call to 'f'
-// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 
'vector' to 'vector' for 1st argument
+// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 
'vector' to 'vector' for 1st argument
 // CHECK-NOELIDE-NOTREE: no matching function for call to 'f'

arphaman wrote:
> I think the majority of test changes make sense, we are just adding 
> qualifiers to the typedefs in the diagnostics. I'm curious about this one 
> though, as we are essentially replacing the old diagnostic note `no known 
> conversion from 'vector' to 'vector' for 1st 
> argument` by the new note `no known conversion from 'vector' to 
> 'vector' for 1st argument`. Is one better than the other? It seems 
> that GCC prefers the former, while ICC the latter. Does it even matter which 
> one we have?
Right, wanted to add a note... forgot about that. This "fix" here is probably 
entirely nonsense. I guess I broke some other feature here. I was hoping for 
some input from the review ;) As I see it it's supposed to print the canonical 
type in case both types would be printed the same, but apparently the 
comparison breaks.  The question where this happens... I'll try to dig through 
the history and see if I find something.


https://reviews.llvm.org/D29944



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

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 89110.
Hahnfeld added a comment.

rebase


https://reviews.llvm.org/D30087

Files:
  lib/Driver/Tools.cpp
  test/Driver/fopenmp.c

Index: test/Driver/fopenmp.c
===
--- test/Driver/fopenmp.c
+++ test/Driver/fopenmp.c
@@ -18,29 +18,33 @@
 // CHECK-CC1-NO-OPENMP-NOT: "-fopenmp"
 //
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
-// RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-RT
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
 //
 // RUN: %clang -target x86_64-darwin -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
-// RUN: %clang -target x86_64-darwin -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP
+// RUN: %clang -target x86_64-darwin -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
 // RUN: %clang -target x86_64-darwin -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
 // RUN: %clang -nostdlib -target x86_64-darwin -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-darwin -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-darwin -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
 //
-// RUN: %clang -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
-// RUN: %clang -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP
-// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+// RUN: %clang -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+// RUN: %clang -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
+// RUN: %clang -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
 //
+// RUN: %clang -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+// RUN: %clang -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
+// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+//
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -50,6 +54,8 @@
 //
 // CHECK-LD-GOMP: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-GOMP: "-lgomp"
+// CHECK-LD-GOMP-RT: "-lrt"
+// CHECK-LD-GOMP-NO-RT-NOT: "-lrt"
 //
 // CHECK-LD-IOMP5: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-IOMP5: "-liomp5"
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3246,26 +3246,37 @@
   }
 }
 
-static void addOpenMPRuntime(ArgStringList , const ToolChain ,
-  const ArgList ) {
+// Returns true, if an OpenMP runtime has been added.
+static bool addOpenMPRuntime(ArgStringList , const ToolChain ,
+ const ArgList , const JobAction ,
+ bool GompNeedsRT = false) {
   if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
 options::OPT_fno_openmp, 

[PATCH] D30157: [analyzer] Improve valist check

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:178
+VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay;
+  const MemRegion *Reg = SV.getAsRegion();
+  if (const auto *DeclReg = Reg->getAs()) {

I suspect that UnknownVal should also be handled before that, otherwise we'd 
have null dereference on the next line.



Comment at: test/Analysis/valist-uninitialized-no-undef.c:5
+
+// This is the same function as the previous one, but it is called in 
call_inlined_uses_arg(),
+// and the warning is generated during the analysis of call_inlined_uses_arg().

Hmm, where's the previous one?



Comment at: test/Analysis/valist-uninitialized-no-undef.c:19
+  // FIXME: There should be no warning for this.
+  (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an 
uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized 
va_list}}
+  va_end(*fst);

As the patch tries to handle symbolic va_list regions, i wonder what's so 
particularly hard about this false positive (apart from its being obviously 
rare, by the way did you actually see such code?).


https://reviews.llvm.org/D30157



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

r295659 - clang-format: Prevent weird line-wraps in complex lambda introducers

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Feb 20 06:43:48 2017
New Revision: 295659

URL: http://llvm.org/viewvc/llvm-project?rev=295659=rev
Log:
clang-format: Prevent weird line-wraps in complex lambda introducers

Before:
  a.(
  [aaa]() -> ::std::
  unordered_set {
//
  });

After:
  a.(
  [aaa]()
  -> ::std::unordered_set<
  > {
//
  });

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

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=295659=295658=295659=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Feb 20 06:43:48 2017
@@ -810,6 +810,8 @@ unsigned ContinuationIndenter::moveState
   if (Current.isOneOf(TT_BinaryOperator, TT_ConditionalExpr) && Newline)
 State.Stack.back().NestedBlockIndent =
 State.Column + Current.ColumnWidth + 1;
+  if (Current.isOneOf(TT_LambdaLSquare, TT_LambdaArrow))
+State.Stack.back().LastSpace = State.Column;
 
   // Insert scopes created by fake parenthesis.
   const FormatToken *Previous = Current.getPreviousNonComment();

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=295659=295658=295659=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Feb 20 06:43:48 2017
@@ -9262,10 +9262,11 @@ TEST_F(FormatTest, FormatsLambdas) {
" << std::count_if(v.begin(), v.end(), [](int x) 
{\n"
"  return x == 2; // force break\n"
"});");
-  verifyFormat("return aa([=](\n"
-   "int ) {\n"
-   "  return aaa != aaa;\n"
-   "});",
+  verifyFormat("return aa(\n"
+   "[=](int ) {\n"
+   "  return aaa !=\n"
+   " aaa;\n"
+   "});",
getLLVMStyleWithColumns(60));
   verifyFormat("SomeFunction({[&] {\n"
"// comment\n"
@@ -9351,6 +9352,15 @@ TEST_F(FormatTest, FormatsLambdas) {
"#endif\n"
"  ;\n"
"};");
+
+  // Lambdas with complex multiline introducers.
+  verifyFormat(
+  "a.(\n"
+  "[aaa]()\n"
+  "-> ::std::unordered_set<\n"
+  "> {\n"
+  "  //\n"
+  "});");
 }
 
 TEST_F(FormatTest, FormatsBlocks) {


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

r295658 - clang-format: [JS/TS] Improve detection for array subscripts in types.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Feb 20 06:43:41 2017
New Revision: 295658

URL: http://llvm.org/viewvc/llvm-project?rev=295658=rev
Log:
clang-format: [JS/TS] Improve detection for array subscripts in types.

Before:
  var someValue = (v as [
  ]).someFunction(aa);

After:
  var someValue = (v as [])
  .someFunction(aa);

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=295658=295657=295658=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Feb 20 06:43:41 2017
@@ -337,6 +337,9 @@ private:
  Contexts.back().ContextKind == tok::l_brace &&
  Parent->isOneOf(tok::l_brace, tok::comma)) {
 Left->Type = TT_JsComputedPropertyName;
+  } else if (CurrentToken->is(tok::r_square) && Parent &&
+ Parent->is(TT_TemplateCloser)) {
+Left->Type = TT_ArraySubscriptLSquare;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  (!CppArrayTemplates && Parent &&
   Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser, 
tok::at,

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=295658=295657=295658=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon Feb 20 06:43:41 2017
@@ -1095,6 +1095,9 @@ TEST_F(FormatTestJS, TypeAnnotations) {
   verifyFormat("function someFunc(args: string[]):\n"
"{longReturnValue: string[]} {}",
getGoogleJSStyleWithColumns(60));
+  verifyFormat(
+  "var someValue = (v as [])\n"
+  ".someFunction(aa);");
 }
 
 TEST_F(FormatTestJS, UnionIntersectionTypes) {


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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 89106.
Hahnfeld marked 3 inline comments as done.
Hahnfeld edited the summary of this revision.
Hahnfeld added a comment.

Address review comment's and apply new naming style to checkNestingOfRegions


https://reviews.llvm.org/D30135

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/cancel_messages.cpp
  test/OpenMP/cancellation_point_messages.cpp

Index: test/OpenMP/cancellation_point_messages.cpp
===
--- test/OpenMP/cancellation_point_messages.cpp
+++ test/OpenMP/cancellation_point_messages.cpp
@@ -4,8 +4,16 @@
 #pragma omp cancellation   // expected-error {{expected an OpenMP directive}}
 #pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
   ;
+#pragma omp parallel
+  {
+#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancellation point parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancellation point'}}
 #pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+  {
+#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancellation point sections(   // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
 #pragma omp cancellation point for, )  // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
 #pragma omp cancellation point taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
Index: test/OpenMP/cancel_messages.cpp
===
--- test/OpenMP/cancel_messages.cpp
+++ test/OpenMP/cancel_messages.cpp
@@ -4,8 +4,16 @@
 #pragma omp cancellation   // expected-error {{expected an OpenMP directive}}
 #pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
   ;
+#pragma omp parallel
+  {
+#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancel parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancel'}}
 #pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+  {
+#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancel sections(   // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
 #pragma omp cancel for, )  // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
 #pragma omp cancel taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -1956,7 +1956,23 @@
   return SR;
 }
 
-static bool CheckNestingOfRegions(Sema , DSAStackTy *Stack,
+static bool checkCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+  OpenMPDirectiveKind CancelRegion,
+  SourceLocation StartLoc) {
+  // CancelRegion is only needed for cancel and cancellation_point.
+  if (CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_cancellation_point)
+return false;
+
+  if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
+  CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
+return false;
+
+  SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+  << getOpenMPDirectiveName(CancelRegion);
+  return true;
+}
+
+static bool checkNestingOfRegions(Sema , DSAStackTy *Stack,
   OpenMPDirectiveKind CurrentRegion,
   const DeclarationNameInfo ,
   OpenMPDirectiveKind CancelRegion,
@@ -2256,7 +2272,9 @@
 OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
 Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc) {
   StmtResult Res = StmtError();
-  if (CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
+  // First check CancelRegion which is then used in checkNestingOfRegions.
+  if (checkCancelRegion(*this, Kind, CancelRegion, StartLoc) ||
+  checkNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
 StartLoc))
 return StmtError();
 
@@ -5860,12 +5878,6 @@
 Sema::ActOnOpenMPCancellationPointDirective(SourceLocation StartLoc,
 

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+  CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+  }

Hahnfeld wrote:
> ABataev wrote:
> > Hahnfeld wrote:
> > > ABataev wrote:
> > > > It is better to convert this to return `false` and make error message 
> > > > and `return true` statement unconditional
> > > I wanted to keep the style in `CheckNestingOfRegions`: That way it's 
> > > easier to add more checks later on if needed like additional restrictions 
> > > on `CancelRegion`
> > I just meant that it's better to make it look like this:
> > ```
> > if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
> > CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
> >   return false;
> > 
> > SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
> >   << getOpenMPDirectiveName(CancelRegion);
> > return true;
> > ```
> I understood what you want it to look like. However, that style makes it 
> impossible to add additional diagnostics to this function
Let's think about it later, if(!) some changes will be required


https://reviews.llvm.org/D30135



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

r295654 - [ASTImporter] Support default argument initialization of ParmVarDecls

[Aleksei Sidorin via cfe-commits]

Author: a.sidorin
Date: Mon Feb 20 05:57:12 2017
New Revision: 295654

URL: http://llvm.org/viewvc/llvm-project?rev=295654=rev
Log:
[ASTImporter] Support default argument initialization of ParmVarDecls

Patch by Peter Szecsi!

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/ASTMerge/exprs-cpp/Inputs/exprs3.cpp
cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=295654=295653=295654=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Feb 20 05:57:12 2017
@@ -3859,8 +3859,27 @@ Decl *ASTNodeImporter::VisitParmVarDecl(
  Importer.Import(D->getInnerLocStart()),
 Loc, Name.getAsIdentifierInfo(),
 T, TInfo, D->getStorageClass(),
-/*FIXME: Default 
argument*/nullptr);
+/*DefaultArg*/ nullptr);
+
+  // Set the default argument.
   ToParm->setHasInheritedDefaultArg(D->hasInheritedDefaultArg());
+  ToParm->setKNRPromoted(D->isKNRPromoted());
+
+  Expr *ToDefArg = nullptr;
+  Expr *FromDefArg = nullptr;
+  if (D->hasUninstantiatedDefaultArg()) {
+FromDefArg = D->getUninstantiatedDefaultArg();
+ToDefArg = Importer.Import(FromDefArg);
+ToParm->setUninstantiatedDefaultArg(ToDefArg);
+  } else if (D->hasUnparsedDefaultArg()) {
+ToParm->setUnparsedDefaultArg();
+  } else if (D->hasDefaultArg()) {
+FromDefArg = D->getDefaultArg();
+ToDefArg = Importer.Import(FromDefArg);
+ToParm->setDefaultArg(ToDefArg);
+  }
+  if (FromDefArg && !ToDefArg)
+return nullptr;
 
   if (D->isUsed())
 ToParm->setIsUsed();

Modified: cfe/trunk/test/ASTMerge/exprs-cpp/Inputs/exprs3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/exprs-cpp/Inputs/exprs3.cpp?rev=295654=295653=295654=diff
==
--- cfe/trunk/test/ASTMerge/exprs-cpp/Inputs/exprs3.cpp (original)
+++ cfe/trunk/test/ASTMerge/exprs-cpp/Inputs/exprs3.cpp Mon Feb 20 05:57:12 2017
@@ -108,6 +108,10 @@ int testDefaultArg(int a = 2*2) {
   return a;
 }
 
+int testDefaultArgExpr() {
+  return testDefaultArg();
+}
+
 template  // T has TemplateTypeParmType
 void testTemplateTypeParmType(int i);
 

Modified: cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp?rev=295654=295653=295654=diff
==
--- cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp (original)
+++ cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp Mon Feb 20 05:57:12 2017
@@ -41,5 +41,7 @@ void testImport(int *x, const S1 , S
   testScalarInit(42);
   testOffsetOf();
   testDefaultArg(12);
+  testDefaultArg();
+  testDefaultArgExpr();
   useTemplateType();
 }


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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+  CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+  }

ABataev wrote:
> Hahnfeld wrote:
> > ABataev wrote:
> > > It is better to convert this to return `false` and make error message and 
> > > `return true` statement unconditional
> > I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier 
> > to add more checks later on if needed like additional restrictions on 
> > `CancelRegion`
> I just meant that it's better to make it look like this:
> ```
> if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
> CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
>   return false;
> 
> SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
>   << getOpenMPDirectiveName(CancelRegion);
> return true;
> ```
I understood what you want it to look like. However, that style makes it 
impossible to add additional diagnostics to this function


https://reviews.llvm.org/D30135



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

problem when using scan-build

[????x???? via cfe-commits]

With --use-c=gcc and --use-analyzer=clang when run "scan-build" it will use 
include<...> of clang , but we need include<...> of gcc. How can I define the 
default include path of gcc when I run "scan-build --use-c=gcc 
--use-analyzer=clang make" ?___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:1959
 
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+  OpenMPDirectiveKind CancelRegion,

Hahnfeld wrote:
> ABataev wrote:
> > Should be `checkCancelRegion`
> It's also `CheckNestingOfRegions`, but can do
Old formatting, this should be fixed



Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+  CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+  }

Hahnfeld wrote:
> ABataev wrote:
> > It is better to convert this to return `false` and make error message and 
> > `return true` statement unconditional
> I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier 
> to add more checks later on if needed like additional restrictions on 
> `CancelRegion`
I just meant that it's better to make it look like this:
```
if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
  return false;

SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
  << getOpenMPDirectiveName(CancelRegion);
return true;
```


https://reviews.llvm.org/D30135



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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:1959
 
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+  OpenMPDirectiveKind CancelRegion,

ABataev wrote:
> Should be `checkCancelRegion`
It's also `CheckNestingOfRegions`, but can do



Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+  CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+  }

ABataev wrote:
> It is better to convert this to return `false` and make error message and 
> `return true` statement unconditional
I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier to 
add more checks later on if needed like additional restrictions on 
`CancelRegion`


https://reviews.llvm.org/D30135



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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:1959
 
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+  OpenMPDirectiveKind CancelRegion,

Should be `checkCancelRegion`



Comment at: lib/Sema/SemaOpenMP.cpp:1964-1966
+  CurrentRegion != OMPD_cancellation_point) {
+return false;
+  }

No need for braces



Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+  CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+  }

It is better to convert this to return `false` and make error message and 
`return true` statement unconditional


https://reviews.llvm.org/D30135



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

RE: D29829: [OpenCL][Doc] Description for adding OpenCL vendor extension in user manual

[Anastasia Stulova via cfe-commits]

Perfect! Thanks!

Anastasia 

-Original Message-
From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of Hans 
Wennborg
Sent: 16 February 2017 17:22
To: Anastasia Stulova
Cc: cfe-commits@lists.llvm.org
Subject: Re: D29829: [OpenCL][Doc] Description for adding OpenCL vendor 
extension in user manual

I've merged it in r295340.

Cheers,
Hans

On Thu, Feb 16, 2017 at 6:26 AM, Anastasia Stulova  
wrote:
> Hans, could we merge this documentation only change (r295313) in 
> release40 branch. I can commit myself if needed. :)
>
> Thanks in advance,
> Anastasia
>
> -Original Message-
> From: Anastasia Stulova via Phabricator 
> [mailto:revi...@reviews.llvm.org]
> Sent: 16 February 2017 14:15
> To: Anastasia Stulova; yaxun@amd.com
> Cc: cfe-commits@lists.llvm.org
> Subject: [PATCH] D29829: [OpenCL][Doc] Description for adding OpenCL 
> vendor extension in user manual
>
> Anastasia closed this revision.
> Anastasia added a comment.
>
> Committed in 295313!
>
>
> https://reviews.llvm.org/D29829
>
>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 89099.
Hahnfeld edited the summary of this revision.
Hahnfeld added a comment.

new static function `CheckCancelRegion`


https://reviews.llvm.org/D30135

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/cancel_messages.cpp
  test/OpenMP/cancellation_point_messages.cpp

Index: test/OpenMP/cancellation_point_messages.cpp
===
--- test/OpenMP/cancellation_point_messages.cpp
+++ test/OpenMP/cancellation_point_messages.cpp
@@ -4,8 +4,16 @@
 #pragma omp cancellation   // expected-error {{expected an OpenMP directive}}
 #pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
   ;
+#pragma omp parallel
+  {
+#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancellation point parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancellation point'}}
 #pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+  {
+#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancellation point sections(   // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
 #pragma omp cancellation point for, )  // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
 #pragma omp cancellation point taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
Index: test/OpenMP/cancel_messages.cpp
===
--- test/OpenMP/cancel_messages.cpp
+++ test/OpenMP/cancel_messages.cpp
@@ -4,8 +4,16 @@
 #pragma omp cancellation   // expected-error {{expected an OpenMP directive}}
 #pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
   ;
+#pragma omp parallel
+  {
+#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancel parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancel'}}
 #pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+  {
+#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+  }
 #pragma omp cancel sections(   // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
 #pragma omp cancel for, )  // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
 #pragma omp cancel taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -1956,6 +1956,25 @@
   return SR;
 }
 
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+  OpenMPDirectiveKind CancelRegion,
+  SourceLocation StartLoc) {
+  // CancelRegion is only needed for cancel and cancellation_point.
+  if (CurrentRegion != OMPD_cancel &&
+  CurrentRegion != OMPD_cancellation_point) {
+return false;
+  }
+
+  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+  CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+  }
+
+  return false;
+}
+
 static bool CheckNestingOfRegions(Sema , DSAStackTy *Stack,
   OpenMPDirectiveKind CurrentRegion,
   const DeclarationNameInfo ,
@@ -2256,7 +2275,9 @@
 OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
 Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc) {
   StmtResult Res = StmtError();
-  if (CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
+  // First CheckCancelRegion which is then used in CheckNestingOfRegions.
+  if (CheckCancelRegion(*this, Kind, CancelRegion, StartLoc) ||
+  CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
 StartLoc))
 return StmtError();
 
@@ -5860,12 +5881,6 @@
 Sema::ActOnOpenMPCancellationPointDirective(SourceLocation StartLoc,
 SourceLocation EndLoc,
 OpenMPDirectiveKind CancelRegion) {
-  if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
-  

r295647 - [ARM] Add regression tests for Cortex-M23 and Cortex-M33

[Sanne Wouda via cfe-commits]

Author: sanwou01
Date: Mon Feb 20 04:37:01 2017
New Revision: 295647

URL: http://llvm.org/viewvc/llvm-project?rev=295647=rev
Log:
[ARM] Add regression tests for Cortex-M23 and Cortex-M33

Reviewers: rengolin, t.p.northover

Reviewed By: t.p.northover

Subscribers: aemerson, llvm-commits

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

Modified:
cfe/trunk/test/Driver/arm-cortex-cpus.c
cfe/trunk/test/Preprocessor/arm-target-features.c

Modified: cfe/trunk/test/Driver/arm-cortex-cpus.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=295647=295646=295647=diff
==
--- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c Mon Feb 20 04:37:01 2017
@@ -579,6 +579,12 @@
 // CHECK-CORTEX-A73-SOFT: "-target-feature" "+soft-float"
 // CHECK-CORTEX-A73-SOFT: "-target-feature" "+soft-float-abi"
 
+// RUN: %clang -target arm -mcpu=cortex-m23 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MBASE %s
+// CHECK-CPUV8MBASE:  "-cc1"{{.*}} "-triple" "thumbv8m.base-
+
+// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MMAIN %s
+// CHECK-CPUV8MMAIN:  "-cc1"{{.*}} "-triple" "thumbv8m.main-
+
 // == Check whether -mcpu accepts mixed-case values.
 // RUN: %clang -target arm-linux-gnueabi -mcpu=Cortex-a5 -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
 // RUN: %clang -target arm-linux-gnueabi -mcpu=cortex-A7 -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s

Modified: cfe/trunk/test/Preprocessor/arm-target-features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/arm-target-features.c?rev=295647=295646=295647=diff
==
--- cfe/trunk/test/Preprocessor/arm-target-features.c (original)
+++ cfe/trunk/test/Preprocessor/arm-target-features.c Mon Feb 20 04:37:01 2017
@@ -398,6 +398,31 @@
 // M7-THUMB:#define __ARM_FP 0xE
 // M7-THUMB:#define __ARM_FPV5__ 1
 
+// Test whether predefines are as expected when targeting v8m cores
+// RUN: %clang -target arm -mcpu=cortex-m23 -x c -E -dM %s -o - | FileCheck 
-match-full-lines --check-prefix=M23 %s
+// M23: #define __ARM_ARCH 8
+// M23: #define __ARM_ARCH_8M_BASE__ 1
+// M23: #define __ARM_ARCH_EXT_IDIV__ 1
+// M23-NOT: __ARM_ARCH_ISA_ARM
+// M23: #define __ARM_ARCH_ISA_THUMB 1
+// M23: #define __ARM_ARCH_PROFILE 'M'
+// M23-NOT: __ARM_FEATURE_CRC32
+// M23-NOT: __ARM_FEATURE_DSP
+// M23-NOT: __ARM_FP 0x{{.*}}
+// M23-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
+
+// RUN: %clang -target arm -mcpu=cortex-m33 -x c -E -dM %s -o - | FileCheck 
-match-full-lines --check-prefix=M33 %s
+// M33: #define __ARM_ARCH 8
+// M33: #define __ARM_ARCH_8M_MAIN__ 1
+// M33: #define __ARM_ARCH_EXT_IDIV__ 1
+// M33-NOT: __ARM_ARCH_ISA_ARM
+// M33: #define __ARM_ARCH_ISA_THUMB 2
+// M33: #define __ARM_ARCH_PROFILE 'M'
+// M33-NOT: __ARM_FEATURE_CRC32
+// M33: #define __ARM_FEATURE_DSP 1
+// M33: #define __ARM_FP 0x6
+// M33: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
+
 // Test whether predefines are as expected when targeting krait.
 // RUN: %clang -target armv7 -mcpu=krait -x c -E -dM %s -o - | FileCheck 
-match-full-lines --check-prefix=KRAIT %s
 // RUN: %clang -target armv7 -mthumb -mcpu=krait -x c -E -dM %s -o - | 
FileCheck -match-full-lines --check-prefix=KRAIT %s


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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+  Stream << "shuffle(";
+  FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  Stream << ", ";

xazax.hun wrote:
> madsravn wrote:
> > xazax.hun wrote:
> > > madsravn wrote:
> > > > xazax.hun wrote:
> > > > > Wouldn't just using the original source range of the argument work?
> > > > > What about preserving the comments next to the arguments? 
> > > > I am not sure what you mean about the original source range. Can I just 
> > > > put that onto the Stream? 
> > > > 
> > > > Do you have an idea for the comments? Do you mean something like
> > > > 
> > > > 
> > > > ```
> > > > std::random_shuffle(
> > > >vec.begin(), // Comment
> > > >vec.end() // Comment
> > > > );
> > > > ```
> > > Or even:
> > > 
> > > 
> > > ```
> > > std::random_shuffle(vec.begin(), /*inlinecomment*/ vec.end());
> > > ```
> > Thanks for you other comments. Can you elaborate on how I might fix this?
> You might do a different strategy, like not touching the arguments at all, 
> but only inserting a new argument before the closing paren of the function 
> call. This way all the comments should be preserved. 
I could try that. So just change the name of the function, random_shuffle to 
shuffle, and then remove the third argument if present and insert third 
argument. I guess it will work, but it will make the code less elegant.


https://reviews.llvm.org/D30158



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

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

xazax.hun added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+  Stream << "shuffle(";
+  FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  Stream << ", ";

madsravn wrote:
> xazax.hun wrote:
> > madsravn wrote:
> > > xazax.hun wrote:
> > > > Wouldn't just using the original source range of the argument work?
> > > > What about preserving the comments next to the arguments? 
> > > I am not sure what you mean about the original source range. Can I just 
> > > put that onto the Stream? 
> > > 
> > > Do you have an idea for the comments? Do you mean something like
> > > 
> > > 
> > > ```
> > > std::random_shuffle(
> > >vec.begin(), // Comment
> > >vec.end() // Comment
> > > );
> > > ```
> > Or even:
> > 
> > 
> > ```
> > std::random_shuffle(vec.begin(), /*inlinecomment*/ vec.end());
> > ```
> Thanks for you other comments. Can you elaborate on how I might fix this?
You might do a different strategy, like not touching the arguments at all, but 
only inserting a new argument before the closing paren of the function call. 
This way all the comments should be preserved. 


https://reviews.llvm.org/D30158



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+  Stream << "shuffle(";
+  FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  Stream << ", ";

xazax.hun wrote:
> madsravn wrote:
> > xazax.hun wrote:
> > > Wouldn't just using the original source range of the argument work?
> > > What about preserving the comments next to the arguments? 
> > I am not sure what you mean about the original source range. Can I just put 
> > that onto the Stream? 
> > 
> > Do you have an idea for the comments? Do you mean something like
> > 
> > 
> > ```
> > std::random_shuffle(
> >vec.begin(), // Comment
> >vec.end() // Comment
> > );
> > ```
> Or even:
> 
> 
> ```
> std::random_shuffle(vec.begin(), /*inlinecomment*/ vec.end());
> ```
Thanks for you other comments. Can you elaborate on how I might fix this?


https://reviews.llvm.org/D30158



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

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 89094.
sylvestre.ledru added a comment.

Sorry, I tried to rename the file but this is too confusing for Phabricator it 
seems...


https://reviews.llvm.org/D30111

Files:
  unittests/Format/check-coding-style-mozilla.cpp

Index: unittests/Format/check-coding-style-mozilla.cpp
===
--- unittests/Format/check-coding-style-mozilla.cpp
+++ unittests/Format/check-coding-style-mozilla.cpp
@@ -0,0 +1,132 @@
+// RUN: clang-format -style=Mozilla %s > %T/foo.cpp 2>&1
+// RUN: diff -u %s %T/foo.cpp
+// RUN: rm -rf %T/foo.cpp
+
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {
+} else if (false) {
+} else {
+}
+
+while (true) {
+}
+
+do {
+} while (true);
+
+for (auto bar::in) {
+}
+
+switch (var) {
+  case 1: {
+// When you need to declare a variable in a switch, put the block in braces
+int var;
+break;
+  }
+  case 2:
+foo();
+break;
+  default:
+break;
+}
+
+namespace mozilla {
+
+class MyClass : public A
+{
+  void Myclass();
+};
+
+// Not supported yet
+// See https://bugs.llvm.org/show_bug.cgi?id=32017 for the feature request
+class MyClass : public X // When deriving from more than one class, put each on
+ // its own line.
+,
+public Y
+{
+public:
+  MyClass(int aVar, int aVar2)
+: mVar(aVar)
+, mVar2(aVar2)
+  {
+foo();
+  }
+
+  // Tiny constructors and destructors can be written on a single line.
+  MyClass() {}
+
+  // Unless it's a copy or move constructor or you have a specific reason to
+  // allow implicit conversions, mark all single-argument constructors explicit.
+  explicit MyClass(OtherClass aArg) { bar(); }
+
+  // This constructor can also take a single argument, so it also needs to be
+  // marked explicit.
+  explicit MyClass(OtherClass aArg, AnotherClass aArg2 = AnotherClass())
+  {
+foo();
+  }
+
+  int TinyFunction()
+  {
+return mVar;
+  } // Tiny functions can be written in a single line.
+
+  int LargerFunction()
+  {
+bar();
+foo();
+  }
+
+private:
+  int mVar;
+};
+
+} // namespace mozilla
+
+template // Templates on own line.
+static int
+MyFunction(int a) // Return type on own line for top-level functions.
+{
+  foo();
+}
+
+int
+MyClass::Method(long b)
+{
+  bar();
+}
+
+T* p; // Correct declaration style
+
+// Test the && and || with parentheses
+return (nextKeyframe < aTimeThreshold ||
+(mVideo.mTimeThreshold &&
+ mVideo.mTimeThreshold.ref().EndTime() < aTimeThreshold)) &&
+   nextKeyframe.ToMicroseconds() >= 0 && !nextKeyframe.IsInfinite();
+
+// The ? should be placed 2 spaces after the previous declaration
+LOGV("%d audio samples demuxed (sid:%d)",
+ aSamples->mSamples.Length(),
+ aSamples->mSamples[0]->mTrackInfo
+   ? aSamples->mSamples[0]->mTrackInfo->GetID()
+   : 0);
+
+// Test with the 80 chars limit
+return (aDecoder.HasPromise() || aDecoder.mTimeThreshold.isSome()) &&
+   !aDecoder.HasPendingDrain() && !aDecoder.HasFatalError() &&
+   !aDecoder.mDemuxRequest.Exists() && !aDecoder.mOutput.Length() &&
+   !aDecoder.HasInternalSeekPending() && !aDecoder.mDecodeRequest.Exists();
+
+template
+void
+foo();
+
+#define MACRO(V)   \
+  V(Rt2) /* one more char */   \
+  V(Rs)  /* than here  */  \
+/* comment 3 */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added inline comments.



Comment at: test/Format/check-coding-style-mozilla.cpp:48
+,
+public Y
+{

krasimir wrote:
> sylvestre.ledru wrote:
> > krasimir wrote:
> > > This does not check precisely what the comment says, because the comment 
> > > affects the indentation decisions. Better put the comment before the 
> > > class declaration.
> > I know, this is one of the thing I would like to see fixed in clang format 
> > or us. 
> > I am adding it in the test suite to make sure that we address it
> I think this might be better addressed through a bug/feature request, plus an 
> explicit comment here that this is not yet supported, because this is not 
> obvious from just starring at the code.
Make sense. I reported https://bugs.llvm.org/show_bug.cgi?id=32017 for this




Comment at: test/Format/check-coding-style-mozilla.cpp:90
+template // Templates on own line.
+static int   // Return type on own line for top-level functions.
+  MyFunction(int a)

sylvestre.ledru wrote:
> krasimir wrote:
> > Trailing comments affect line breaking, so this is not really testing what 
> > the comments say. Suggest to put them on a line before the template.
> Yeah, we are trying to fix this issue.
> but you are correct, I will move it
Reported here:
https://bugs.llvm.org/show_bug.cgi?id=32016




Comment at: test/Format/check-coding-style-mozilla.cpp:7-9
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

klimek wrote:
> Note that I'm not a license expert, but I'd be surprised if it was ok to put 
> code in random licenses into the repo.
I don't think this is a problem as it is in the test and there is no actual 
code but I can remove it if you prefer.

This was to test the formatting of comment


https://reviews.llvm.org/D30111



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

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 89093.
sylvestre.ledru marked 3 inline comments as done.

https://reviews.llvm.org/D30111

Files:
  unittests/Format/CheckCodingStyleMozilla.cpp

Index: unittests/Format/CheckCodingStyleMozilla.cpp
===
--- unittests/Format/CheckCodingStyleMozilla.cpp
+++ unittests/Format/CheckCodingStyleMozilla.cpp
@@ -0,0 +1,132 @@
+// RUN: clang-format -style=Mozilla %s > %T/foo.cpp 2>&1
+// RUN: diff -u %s %T/foo.cpp
+// RUN: rm -rf %T/foo.cpp
+
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {
+} else if (false) {
+} else {
+}
+
+while (true) {
+}
+
+do {
+} while (true);
+
+for (auto bar::in) {
+}
+
+switch (var) {
+  case 1: {
+// When you need to declare a variable in a switch, put the block in braces
+int var;
+break;
+  }
+  case 2:
+foo();
+break;
+  default:
+break;
+}
+
+namespace mozilla {
+
+class MyClass : public A
+{
+  void Myclass();
+};
+
+// Not supported yet
+// See https://bugs.llvm.org/show_bug.cgi?id=32017 for the feature request
+class MyClass : public X // When deriving from more than one class, put each on
+ // its own line.
+,
+public Y
+{
+public:
+  MyClass(int aVar, int aVar2)
+: mVar(aVar)
+, mVar2(aVar2)
+  {
+foo();
+  }
+
+  // Tiny constructors and destructors can be written on a single line.
+  MyClass() {}
+
+  // Unless it's a copy or move constructor or you have a specific reason to
+  // allow implicit conversions, mark all single-argument constructors explicit.
+  explicit MyClass(OtherClass aArg) { bar(); }
+
+  // This constructor can also take a single argument, so it also needs to be
+  // marked explicit.
+  explicit MyClass(OtherClass aArg, AnotherClass aArg2 = AnotherClass())
+  {
+foo();
+  }
+
+  int TinyFunction()
+  {
+return mVar;
+  } // Tiny functions can be written in a single line.
+
+  int LargerFunction()
+  {
+bar();
+foo();
+  }
+
+private:
+  int mVar;
+};
+
+} // namespace mozilla
+
+template // Templates on own line.
+static int
+MyFunction(int a) // Return type on own line for top-level functions.
+{
+  foo();
+}
+
+int
+MyClass::Method(long b)
+{
+  bar();
+}
+
+T* p; // Correct declaration style
+
+// Test the && and || with parentheses
+return (nextKeyframe < aTimeThreshold ||
+(mVideo.mTimeThreshold &&
+ mVideo.mTimeThreshold.ref().EndTime() < aTimeThreshold)) &&
+   nextKeyframe.ToMicroseconds() >= 0 && !nextKeyframe.IsInfinite();
+
+// The ? should be placed 2 spaces after the previous declaration
+LOGV("%d audio samples demuxed (sid:%d)",
+ aSamples->mSamples.Length(),
+ aSamples->mSamples[0]->mTrackInfo
+   ? aSamples->mSamples[0]->mTrackInfo->GetID()
+   : 0);
+
+// Test with the 80 chars limit
+return (aDecoder.HasPromise() || aDecoder.mTimeThreshold.isSome()) &&
+   !aDecoder.HasPendingDrain() && !aDecoder.HasFatalError() &&
+   !aDecoder.mDemuxRequest.Exists() && !aDecoder.mOutput.Length() &&
+   !aDecoder.HasInternalSeekPending() && !aDecoder.mDecodeRequest.Exists();
+
+template
+void
+foo();
+
+#define MACRO(V)   \
+  V(Rt2) /* one more char */   \
+  V(Rs)  /* than here  */  \
+/* comment 3 */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

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

xazax.hun added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30
+" The old user defined 'RandomFunction' is not usable for 'shuffle'. You "
+"need to "
+"make additional changes if you want a specific random function.";

madsravn wrote:
> xazax.hun wrote:
> > Maybe it would be worth to reflow this literal.
> It seems a little weird, but this is the result of clang-format. 
I usually just make it one big line and than clang format will do a better 
reflow. By default it do not remove linebreaks, even if it could improve the 
layout. 



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:70
+  llvm::raw_string_ostream Stream(Buffer);
+  DRef->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  StringRef StrRef(Stream.str());

madsravn wrote:
> xazax.hun wrote:
> > What about accessing the buffer of the source file instead of pretty 
> > printing?
> How would you do this? printPretty was the best that I could find fitting my 
> needs. If you have something better fitting, please share :)
I was thinking about something like getSourceText of Lexer. 



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+  Stream << "shuffle(";
+  FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  Stream << ", ";

madsravn wrote:
> xazax.hun wrote:
> > Wouldn't just using the original source range of the argument work?
> > What about preserving the comments next to the arguments? 
> I am not sure what you mean about the original source range. Can I just put 
> that onto the Stream? 
> 
> Do you have an idea for the comments? Do you mean something like
> 
> 
> ```
> std::random_shuffle(
>vec.begin(), // Comment
>vec.end() // Comment
> );
> ```
Or even:


```
std::random_shuffle(vec.begin(), /*inlinecomment*/ vec.end());
```



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:101
+
+  auto Diag = diag(MatchedCallExpr->getLocStart(), Message);
+  Diag << FixItHint::CreateRemoval(MatchedCallExpr->getSourceRange());

madsravn wrote:
> xazax.hun wrote:
> > You do not want to do fixits for code that is the result of macro 
> > expansions. 
> Why not? And how would I fix that? 
Because that might cause the code not to compile when the macro is expanded in 
a different context. It is a conservative approach but other tidy checkers 
usually do this as well. You can use the isMacroID() method of the source 
locations. 


https://reviews.llvm.org/D30158



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

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: test/Format/check-coding-style-mozilla.cpp:10
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {

sylvestre.ledru wrote:
> krasimir wrote:
> > What is tested here? Brace styles?
> Yes, do you want me to add a comment to explicit that?
I think that after the comments on the non-brace-style features below, it's 
obvious enough and doesn't need more comments.



Comment at: test/Format/check-coding-style-mozilla.cpp:48
+,
+public Y
+{

sylvestre.ledru wrote:
> krasimir wrote:
> > This does not check precisely what the comment says, because the comment 
> > affects the indentation decisions. Better put the comment before the class 
> > declaration.
> I know, this is one of the thing I would like to see fixed in clang format or 
> us. 
> I am adding it in the test suite to make sure that we address it
I think this might be better addressed through a bug/feature request, plus an 
explicit comment here that this is not yet supported, because this is not 
obvious from just starring at the code.


https://reviews.llvm.org/D30111



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added inline comments.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30
+" The old user defined 'RandomFunction' is not usable for 'shuffle'. You "
+"need to "
+"make additional changes if you want a specific random function.";

xazax.hun wrote:
> Maybe it would be worth to reflow this literal.
It seems a little weird, but this is the result of clang-format. 



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:70
+  llvm::raw_string_ostream Stream(Buffer);
+  DRef->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  StringRef StrRef(Stream.str());

xazax.hun wrote:
> What about accessing the buffer of the source file instead of pretty printing?
How would you do this? printPretty was the best that I could find fitting my 
needs. If you have something better fitting, please share :)



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+  Stream << "shuffle(";
+  FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  Stream << ", ";

xazax.hun wrote:
> Wouldn't just using the original source range of the argument work?
> What about preserving the comments next to the arguments? 
I am not sure what you mean about the original source range. Can I just put 
that onto the Stream? 

Do you have an idea for the comments? Do you mean something like


```
std::random_shuffle(
   vec.begin(), // Comment
   vec.end() // Comment
);
```



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:101
+
+  auto Diag = diag(MatchedCallExpr->getLocStart(), Message);
+  Diag << FixItHint::CreateRemoval(MatchedCallExpr->getSourceRange());

xazax.hun wrote:
> You do not want to do fixits for code that is the result of macro expansions. 
Why not? And how would I fix that? 


https://reviews.llvm.org/D30158



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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I see. I think it is better to check the `CancelRegion` just before call of 
`CheckNestingOfRegions()` function. You need to extract checks for 
`CancelRegion` from `ActOnOpenMPCancellationPointDirective()` and 
`ActOnOpenMPCancelDirective()` functions into a standalone function and call it 
before `CheckNestingOfRegions()`.


https://reviews.llvm.org/D30135



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

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: test/Format/check-coding-style-mozilla.cpp:7-9
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Note that I'm not a license expert, but I'd be surprised if it was ok to put 
code in random licenses into the repo.


https://reviews.llvm.org/D30111



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

[PATCH] D29943: [clang-format] Align block comment decorations

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

In https://reviews.llvm.org/D29943#678618, @sylvestre.ledru wrote:

> Maybe this could be added to the release notes?


Sounds good! Could you please point me to the release docs? I don't seem to 
find a clang-format--specific section online.


Repository:
  rL LLVM

https://reviews.llvm.org/D29943



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

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

xazax.hun added a comment.

Nice check! Thank you for working on this!




Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30
+" The old user defined 'RandomFunction' is not usable for 'shuffle'. You "
+"need to "
+"make additional changes if you want a specific random function.";

Maybe it would be worth to reflow this literal.



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:70
+  llvm::raw_string_ostream Stream(Buffer);
+  DRef->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  StringRef StrRef(Stream.str());

What about accessing the buffer of the source file instead of pretty printing?



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+  Stream << "shuffle(";
+  FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+  Stream << ", ";

Wouldn't just using the original source range of the argument work?
What about preserving the comments next to the arguments? 



Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:101
+
+  auto Diag = diag(MatchedCallExpr->getLocStart(), Message);
+  Diag << FixItHint::CreateRemoval(MatchedCallExpr->getSourceRange());

You do not want to do fixits for code that is the result of macro expansions. 



Comment at: docs/clang-tidy/checks/modernize-replace-random-shuffle.rst:24
+
+  std::shuffle(vec.begin(), vec.end(), std::mt19937(std::random_device()()));
+

Isn't it a performance problem in general to always reinitialize a random 
engine? Maybe the documentation should contain a notice that in case of 
performance critical code the user might want to factor the last parameter out 
somewhere.



Comment at: test/clang-tidy/modernize-replace-random-shuffle.cpp:50
+  // CHECK-MESSAGE: [[@LINE-1]]:3: warning: do not use 'random_shuffle'. It is 
deprecated and replaced by 'shuffle'. The old user defined 'RandomFunction' is 
not usable for shuffle. You need to make additional changes if you want a 
specific random function
+  // CHECK-FIXES: shuffle(vec.begin(), vec.begin(), 
std::mt19937(std::random_device()()));
+

This check-fix might match the previous fix instead of this one. You might want 
to make the fixes unique, e.g.: with a comment after a line. Note that it is 
also worth to test that line ending comments are preserved.

Also, are you sure you want to do the fixit when a custom random function is 
passed to random_shuffle?


https://reviews.llvm.org/D30158



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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D30135#681354, @ABataev wrote:

> Not sure that this is better because at first, we need to be sure that this 
> nesting is allowed. Why do we need to perform some additional analysis if 
> nesting is not allowed at all?


`CheckNestingOfRegions` uses `CancelRegion` to determine whether cancel and 
cancellation point may be nested inside the parent construct. However with the 
current code, `CancelRegion` would only be checked afterwards.

  #pragma omp parallel
{
  #pragma omp cancellation point unknown
}

therefore produces `region cannot be closely nested inside 'parallel' region`. 
After this change, it says `one of 'for', 'parallel', 'sections' or 'taskgroup' 
is expected` as in the test case which is better IMO.

Should I try to improve the summary to explain that?


https://reviews.llvm.org/D30135



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

[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.

[Aleksei Sidorin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL295644: [analyzer] Do not duplicate call graph nodes for 
functions that have definition… (authored by a.sidorin).

Changed prior to commit:
  https://reviews.llvm.org/D29643?vs=87388=89089#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29643

Files:
  cfe/trunk/include/clang/Analysis/CallGraph.h
  cfe/trunk/test/Analysis/debug-CallGraph.c


Index: cfe/trunk/include/clang/Analysis/CallGraph.h
===
--- cfe/trunk/include/clang/Analysis/CallGraph.h
+++ cfe/trunk/include/clang/Analysis/CallGraph.h
@@ -98,7 +98,7 @@
   bool VisitFunctionDecl(FunctionDecl *FD) {
 // We skip function template definitions, as their semantics is
 // only determined when they are instantiated.
-if (includeInGraph(FD)) {
+if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) {
   // Add all blocks declared inside this function to the graph.
   addNodesForBlocks(FD);
   // If this function has external linkage, anything could call it.
Index: cfe/trunk/test/Analysis/debug-CallGraph.c
===
--- cfe/trunk/test/Analysis/debug-CallGraph.c
+++ cfe/trunk/test/Analysis/debug-CallGraph.c
@@ -43,8 +43,18 @@
 void eee() {}
 void fff() { eee(); }
 
+// This test case tests that forward declaration for the top-level function
+// does not affect call graph construction.
+void do_nothing() {}
+void test_single_call();
+void test_single_call() {
+  do_nothing();
+}
+
 // CHECK:--- Call graph Dump ---
-// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > 
bbb ccc ddd eee fff $}}
+// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > 
bbb ddd ccc eee fff do_nothing test_single_call $}}
+// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}}
+// CHECK-NEXT: {{Function: do_nothing calls: $}}
 // CHECK-NEXT: {{Function: fff calls: eee $}}
 // CHECK-NEXT: {{Function: eee calls: $}}
 // CHECK-NEXT: {{Function: ddd calls: ccc $}}


Index: cfe/trunk/include/clang/Analysis/CallGraph.h
===
--- cfe/trunk/include/clang/Analysis/CallGraph.h
+++ cfe/trunk/include/clang/Analysis/CallGraph.h
@@ -98,7 +98,7 @@
   bool VisitFunctionDecl(FunctionDecl *FD) {
 // We skip function template definitions, as their semantics is
 // only determined when they are instantiated.
-if (includeInGraph(FD)) {
+if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) {
   // Add all blocks declared inside this function to the graph.
   addNodesForBlocks(FD);
   // If this function has external linkage, anything could call it.
Index: cfe/trunk/test/Analysis/debug-CallGraph.c
===
--- cfe/trunk/test/Analysis/debug-CallGraph.c
+++ cfe/trunk/test/Analysis/debug-CallGraph.c
@@ -43,8 +43,18 @@
 void eee() {}
 void fff() { eee(); }
 
+// This test case tests that forward declaration for the top-level function
+// does not affect call graph construction.
+void do_nothing() {}
+void test_single_call();
+void test_single_call() {
+  do_nothing();
+}
+
 // CHECK:--- Call graph Dump ---
-// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ccc ddd eee fff $}}
+// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > bbb ddd ccc eee fff do_nothing test_single_call $}}
+// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}}
+// CHECK-NEXT: {{Function: do_nothing calls: $}}
 // CHECK-NEXT: {{Function: fff calls: eee $}}
 // CHECK-NEXT: {{Function: eee calls: $}}
 // CHECK-NEXT: {{Function: ddd calls: ccc $}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r295644 - [analyzer] Do not duplicate call graph nodes for functions that have definition and forward declaration

[Aleksei Sidorin via cfe-commits]

Author: a.sidorin
Date: Mon Feb 20 03:16:48 2017
New Revision: 295644

URL: http://llvm.org/viewvc/llvm-project?rev=295644=rev
Log:
[analyzer] Do not duplicate call graph nodes for functions that have definition 
and forward declaration

Patch by Ivan Sidorenko!

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

Modified:
cfe/trunk/include/clang/Analysis/CallGraph.h
cfe/trunk/test/Analysis/debug-CallGraph.c

Modified: cfe/trunk/include/clang/Analysis/CallGraph.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CallGraph.h?rev=295644=295643=295644=diff
==
--- cfe/trunk/include/clang/Analysis/CallGraph.h (original)
+++ cfe/trunk/include/clang/Analysis/CallGraph.h Mon Feb 20 03:16:48 2017
@@ -98,7 +98,7 @@ public:
   bool VisitFunctionDecl(FunctionDecl *FD) {
 // We skip function template definitions, as their semantics is
 // only determined when they are instantiated.
-if (includeInGraph(FD)) {
+if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) {
   // Add all blocks declared inside this function to the graph.
   addNodesForBlocks(FD);
   // If this function has external linkage, anything could call it.

Modified: cfe/trunk/test/Analysis/debug-CallGraph.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/debug-CallGraph.c?rev=295644=295643=295644=diff
==
--- cfe/trunk/test/Analysis/debug-CallGraph.c (original)
+++ cfe/trunk/test/Analysis/debug-CallGraph.c Mon Feb 20 03:16:48 2017
@@ -43,8 +43,18 @@ void eee();
 void eee() {}
 void fff() { eee(); }
 
+// This test case tests that forward declaration for the top-level function
+// does not affect call graph construction.
+void do_nothing() {}
+void test_single_call();
+void test_single_call() {
+  do_nothing();
+}
+
 // CHECK:--- Call graph Dump ---
-// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > 
bbb ccc ddd eee fff $}}
+// CHECK-NEXT: {{Function: < root > calls: get5 add test_add mmm foo aaa < > 
bbb ddd ccc eee fff do_nothing test_single_call $}}
+// CHECK-NEXT: {{Function: test_single_call calls: do_nothing $}}
+// CHECK-NEXT: {{Function: do_nothing calls: $}}
 // CHECK-NEXT: {{Function: fff calls: eee $}}
 // CHECK-NEXT: {{Function: eee calls: $}}
 // CHECK-NEXT: {{Function: ddd calls: ccc $}}


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

[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Anna, I will commit. Thank you!


https://reviews.llvm.org/D29643



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

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Not sure that this is better because at first, we need to be sure that this 
nesting is allowed. Why do we need to perform some additional analysis if 
nesting is not allowed at all?


https://reviews.llvm.org/D30135



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

[PATCH] D23418: [analyzer] Added a reusable constraint system to the CloneDetector

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 89084.
teemperor added a comment.

- Removed all the deprecated `\brief`s

I couldn't find any actual regression in the code now, so from my side it's ok 
to merge it.


https://reviews.llvm.org/D23418

Files:
  include/clang/Analysis/CloneDetection.h
  lib/Analysis/CloneDetection.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp

Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -38,14 +38,15 @@
   void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
  AnalysisManager , BugReporter ) const;
 
-  /// \brief Reports all clones to the user.
+  /// Reports all clones to the user.
   void reportClones(BugReporter , AnalysisManager ,
-int MinComplexity) const;
+std::vector ) const;
 
-  /// \brief Reports only suspicious clones to the user along with informaton
+  /// Reports only suspicious clones to the user along with informaton
   ///that explain why they are suspicious.
-  void reportSuspiciousClones(BugReporter , AnalysisManager ,
-  int MinComplexity) const;
+  void reportSuspiciousClones(
+  BugReporter , AnalysisManager ,
+  std::vector ) const;
 };
 } // end anonymous namespace
 
@@ -72,11 +73,29 @@
   bool ReportNormalClones = Mgr.getAnalyzerOptions().getBooleanOption(
   "ReportNormalClones", true, this);
 
+  // Let the CloneDetector create a list of clones from all the analyzed
+  // statements. We don't filter for matching variable patterns at this point
+  // because reportSuspiciousClones() wants to search them for errors.
+  std::vector AllCloneGroups;
+
+  Detector.findClones(AllCloneGroups, RecursiveCloneTypeIIConstraint(),
+  MinComplexityConstraint(MinComplexity),
+  MinGroupSizeConstraint(2), OnlyLargestCloneConstraint());
+
   if (ReportSuspiciousClones)
-reportSuspiciousClones(BR, Mgr, MinComplexity);
+reportSuspiciousClones(BR, Mgr, AllCloneGroups);
+
+  // We are done for this translation unit unless we also need to report normal
+  // clones.
+  if (!ReportNormalClones)
+return;
 
-  if (ReportNormalClones)
-reportClones(BR, Mgr, MinComplexity);
+  // Now that the suspicious clone detector has checked for pattern errors,
+  // we also filter all clones who don't have matching patterns
+  Detector.constrainClones(AllCloneGroups, MatchingVariablePatternConstraint(),
+   MinGroupSizeConstraint(2));
+
+  reportClones(BR, Mgr, AllCloneGroups);
 }
 
 static PathDiagnosticLocation makeLocation(const StmtSequence ,
@@ -87,37 +106,55 @@
   Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl()));
 }
 
-void CloneChecker::reportClones(BugReporter , AnalysisManager ,
-int MinComplexity) const {
-
-  std::vector CloneGroups;
-  Detector.findClones(CloneGroups, MinComplexity);
+void CloneChecker::reportClones(
+BugReporter , AnalysisManager ,
+std::vector ) const {
 
   if (!BT_Exact)
 BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone"));
 
-  for (CloneDetector::CloneGroup  : CloneGroups) {
+  for (const CloneDetector::CloneGroup  : CloneGroups) {
 // We group the clones by printing the first as a warning and all others
 // as a note.
-auto R = llvm::make_unique(
-*BT_Exact, "Duplicate code detected",
-makeLocation(Group.Sequences.front(), Mgr));
-R->addRange(Group.Sequences.front().getSourceRange());
-
-for (unsigned i = 1; i < Group.Sequences.size(); ++i)
-  R->addNote("Similar code here",
-  makeLocation(Group.Sequences[i], Mgr),
-  Group.Sequences[i].getSourceRange());
+auto R = llvm::make_unique(*BT_Exact, "Duplicate code detected",
+  makeLocation(Group.front(), Mgr));
+R->addRange(Group.front().getSourceRange());
+
+for (unsigned i = 1; i < Group.size(); ++i)
+  R->addNote("Similar code here", makeLocation(Group[i], Mgr),
+ Group[i].getSourceRange());
 BR.emitReport(std::move(R));
   }
 }
 
-void CloneChecker::reportSuspiciousClones(BugReporter ,
-  AnalysisManager ,
-  int MinComplexity) const {
-
-  std::vector Clones;
-  Detector.findSuspiciousClones(Clones, MinComplexity);
+void CloneChecker::reportSuspiciousClones(
+BugReporter , AnalysisManager ,
+std::vector ) const {
+  std::vector Pairs;
+
+  for (const CloneDetector::CloneGroup  : CloneGroups) {
+for (unsigned i = 0; i < Group.size(); ++i) {
+  VariablePattern PatternA(Group[i]);
+
+  for (unsigned j = i + 1; j < Group.size(); ++j) {
+VariablePattern PatternB(Group[j]);
+
+