[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGae451454e32d: Create a warning flag for 
warn_conv_*_not_used (authored by rdwampler).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78442

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,8 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (72):
+CHECK: Warnings without flags (69):
+
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
 CHECK-NEXT:   ext_missing_declspec
@@ -41,9 +42,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8589,11 +8589,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,6 +60,7 @@
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
+def ClassConversion: DiagGroup<"class-conversion">;
 def DeprecatedEnumCompareConditional :
   DiagGroup<"deprecated-enum-compare-conditional">;
 def EnumCompareConditional : DiagGroup<"enum-compare-conditional",


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,8 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (72):
+CHECK: Warnings without flags (69):
+
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
 CHECK-NEXT:   ext_missing_declspec
@@ -41,9 +42,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8589,11 +8589,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that 

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


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

https://reviews.llvm.org/D78442



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

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


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

https://reviews.llvm.org/D78442



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

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


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

https://reviews.llvm.org/D78442



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

[PATCH] D80547: [clang-format] Fix an ObjC regression introduced with new [[likely]][[unlikely]] support in if/else clauses

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1965
   // handle [[likely]] / [[unlikely]]
-  if (FormatTok->is(tok::l_square))
+  if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute())
 parseSquare();

From the peanut gallery, would checking `TT_AttributeSquare` here work (that 
should handle ambiguities around ObjC method calls)?  


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

https://reviews.llvm.org/D80547



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

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


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

https://reviews.llvm.org/D78442



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

[PATCH] D78444: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b53495c4ba2: Perform ActOnConversionDeclarator after 
looking for any virtual functions it… (authored by rdwampler).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78444

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/conversion-function.cpp


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class BaseA {};
+class DerivedA;
+
+class BaseB {
+  virtual operator BaseA &() = 0;
+  virtual operator DerivedA &() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+  virtual operator BaseA &();// OK. Overrides BaseB::operatorBaseA&()
+  virtual operator DerivedA &(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+  virtual operator DerivedB &(); // expected-warning{{conversion function 
converting 'DerivedB' to itself will never be used}}
+  virtual operator BaseA &();// expected-warning{{conversion function 
converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 +10486,12 @@
 
   // Make sure we aren't redeclaring the conversion function.
   QualType ConvType = 
Context.getCanonicalType(Conversion->getConversionType());
-
   // C++ [class.conv.fct]p1:
   //   [...] A conversion function is never used to convert a
   //   (possibly cv-qualified) object to the (possibly cv-qualified)
   //   same object type (or a reference to it), to a (possibly
   //   cv-qualified) base class of that type (or a reference to it),
   //   or to (possibly cv-qualified) void.
-  // FIXME: Suppress this warning if the conversion function ends up being a
-  // virtual function that overrides a virtual function in a base class.
   QualType ClassType
 = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl));
   if (const ReferenceType *ConvTypeRef = ConvType->getAs())
@@ -10502,6 +10499,8 @@
   if (Conversion->getTemplateSpecializationKind() != TSK_Undeclared &&
   Conversion->getTemplateSpecializationKind() != 
TSK_ExplicitSpecialization)
 /* Suppress diagnostics for instantiations. */;
+  else if (Conversion->size_overridden_methods() != 0)
+/* Suppress diagnostics for overriding virtual function in a base class. 
*/;
   else if (ConvType->isRecordType()) {
 ConvType = Context.getCanonicalType(ConvType).getUnqualifiedType();
 if (ConvType == ClassType)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10716,9 +10716,6 @@
   return Redeclaration;
 }
   }
-} else if (CXXConversionDecl *Conversion
-   = dyn_cast(NewFD)) {
-  ActOnConversionDeclarator(Conversion);
 } else if (auto *Guide = dyn_cast(NewFD)) {
   if (auto *TD = Guide->getDescribedFunctionTemplate())
 CheckDeductionGuideTemplate(TD);
@@ -10747,6 +10744,9 @@
 checkThisInStaticMemberFunctionType(Method);
 }
 
+if (CXXConversionDecl *Conversion = dyn_cast(NewFD))
+  ActOnConversionDeclarator(Conversion);
+
 // Extra checking for C++ overloaded operators (C++ [over.oper]).
 if (NewFD->isOverloadedOperator() &&
 CheckOverloadedOperatorDeclaration(NewFD)) {


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class BaseA {};
+class DerivedA;
+
+class BaseB {
+  virtual operator BaseA &() = 0;
+  virtual operator DerivedA &() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+  virtual operator BaseA &();// OK. Overrides BaseB::operatorBaseA&()
+  virtual operator DerivedA &(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+  virtual operator DerivedB &(); // expected-warning{{conversion function converting 'DerivedB' to itself will never be used}}
+  virtual operator BaseA &();// expected-warning{{conversion function converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: 

[PATCH] D79856: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

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

This allows for suppressing warnings about the conversion function never being 
called if it overrides a virtual function in a base class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79856

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/conversion-function.cpp


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class BaseA {};
+class DerivedA;
+
+class BaseB {
+  virtual operator BaseA &() = 0;
+  virtual operator DerivedA &() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+  virtual operator BaseA &();// OK. Overrides BaseB::operatorBaseA&()
+  virtual operator DerivedA &(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+  virtual operator DerivedB &(); // expected-warning{{conversion function 
converting 'DerivedB' to itself will never be used}}
+  virtual operator BaseA &();// expected-warning{{conversion function 
converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 +10486,12 @@
 
   // Make sure we aren't redeclaring the conversion function.
   QualType ConvType = 
Context.getCanonicalType(Conversion->getConversionType());
-
   // C++ [class.conv.fct]p1:
   //   [...] A conversion function is never used to convert a
   //   (possibly cv-qualified) object to the (possibly cv-qualified)
   //   same object type (or a reference to it), to a (possibly
   //   cv-qualified) base class of that type (or a reference to it),
   //   or to (possibly cv-qualified) void.
-  // FIXME: Suppress this warning if the conversion function ends up being a
-  // virtual function that overrides a virtual function in a base class.
   QualType ClassType
 = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl));
   if (const ReferenceType *ConvTypeRef = ConvType->getAs())
@@ -10502,6 +10499,8 @@
   if (Conversion->getTemplateSpecializationKind() != TSK_Undeclared &&
   Conversion->getTemplateSpecializationKind() != 
TSK_ExplicitSpecialization)
 /* Suppress diagnostics for instantiations. */;
+  else if (Conversion->size_overridden_methods() != 0)
+/* Suppress diagnostics for overriding virtual function in a base class. 
*/;
   else if (ConvType->isRecordType()) {
 ConvType = Context.getCanonicalType(ConvType).getUnqualifiedType();
 if (ConvType == ClassType)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10716,9 +10716,6 @@
   return Redeclaration;
 }
   }
-} else if (CXXConversionDecl *Conversion
-   = dyn_cast(NewFD)) {
-  ActOnConversionDeclarator(Conversion);
 } else if (auto *Guide = dyn_cast(NewFD)) {
   if (auto *TD = Guide->getDescribedFunctionTemplate())
 CheckDeductionGuideTemplate(TD);
@@ -10747,6 +10744,9 @@
 checkThisInStaticMemberFunctionType(Method);
 }
 
+if (CXXConversionDecl *Conversion = dyn_cast(NewFD))
+  ActOnConversionDeclarator(Conversion);
+
 // Extra checking for C++ overloaded operators (C++ [over.oper]).
 if (NewFD->isOverloadedOperator() &&
 CheckOverloadedOperatorDeclaration(NewFD)) {


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class BaseA {};
+class DerivedA;
+
+class BaseB {
+  virtual operator BaseA &() = 0;
+  virtual operator DerivedA &() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+  virtual operator BaseA &();// OK. Overrides BaseB::operatorBaseA&()
+  virtual operator DerivedA &(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+  virtual operator DerivedB &(); // expected-warning{{conversion function converting 'DerivedB' to itself will never be used}}
+  virtual operator BaseA &();// expected-warning{{conversion function converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: 

[PATCH] D78444: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 263466.
rdwampler added a comment.

Fixed indentation.


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

https://reviews.llvm.org/D78444

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/conversion-function.cpp


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class BaseA {};
+class DerivedA;
+
+class BaseB {
+  virtual operator BaseA &() = 0;
+  virtual operator DerivedA &() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+  virtual operator BaseA &();// OK. Overrides BaseB::operatorBaseA&()
+  virtual operator DerivedA &(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+  virtual operator DerivedB &(); // expected-warning{{conversion function 
converting 'DerivedB' to itself will never be used}}
+  virtual operator BaseA &();// expected-warning{{conversion function 
converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 +10486,12 @@
 
   // Make sure we aren't redeclaring the conversion function.
   QualType ConvType = 
Context.getCanonicalType(Conversion->getConversionType());
-
   // C++ [class.conv.fct]p1:
   //   [...] A conversion function is never used to convert a
   //   (possibly cv-qualified) object to the (possibly cv-qualified)
   //   same object type (or a reference to it), to a (possibly
   //   cv-qualified) base class of that type (or a reference to it),
   //   or to (possibly cv-qualified) void.
-  // FIXME: Suppress this warning if the conversion function ends up being a
-  // virtual function that overrides a virtual function in a base class.
   QualType ClassType
 = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl));
   if (const ReferenceType *ConvTypeRef = ConvType->getAs())
@@ -10502,6 +10499,8 @@
   if (Conversion->getTemplateSpecializationKind() != TSK_Undeclared &&
   Conversion->getTemplateSpecializationKind() != 
TSK_ExplicitSpecialization)
 /* Suppress diagnostics for instantiations. */;
+  else if (Conversion->size_overridden_methods() != 0)
+/* Suppress diagnostics for overriding virtual function in a base class. 
*/;
   else if (ConvType->isRecordType()) {
 ConvType = Context.getCanonicalType(ConvType).getUnqualifiedType();
 if (ConvType == ClassType)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10716,9 +10716,6 @@
   return Redeclaration;
 }
   }
-} else if (CXXConversionDecl *Conversion
-   = dyn_cast(NewFD)) {
-  ActOnConversionDeclarator(Conversion);
 } else if (auto *Guide = dyn_cast(NewFD)) {
   if (auto *TD = Guide->getDescribedFunctionTemplate())
 CheckDeductionGuideTemplate(TD);
@@ -10747,6 +10744,9 @@
 checkThisInStaticMemberFunctionType(Method);
 }
 
+if (CXXConversionDecl *Conversion = dyn_cast(NewFD))
+  ActOnConversionDeclarator(Conversion);
+
 // Extra checking for C++ overloaded operators (C++ [over.oper]).
 if (NewFD->isOverloadedOperator() &&
 CheckOverloadedOperatorDeclaration(NewFD)) {


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class BaseA {};
+class DerivedA;
+
+class BaseB {
+  virtual operator BaseA &() = 0;
+  virtual operator DerivedA &() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+  virtual operator BaseA &();// OK. Overrides BaseB::operatorBaseA&()
+  virtual operator DerivedA &(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+  virtual operator DerivedB &(); // expected-warning{{conversion function converting 'DerivedB' to itself will never be used}}
+  virtual operator BaseA &();// expected-warning{{conversion function converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 263202.
rdwampler added a comment.

Rebase.


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

https://reviews.llvm.org/D78442

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,8 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (72):
+CHECK: Warnings without flags (69):
+
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
 CHECK-NEXT:   ext_missing_declspec
@@ -41,9 +42,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8572,11 +8572,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,6 +60,7 @@
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
+def ClassConversion: DiagGroup<"class-conversion">;
 def DeprecatedEnumCompareConditional :
   DiagGroup<"deprecated-enum-compare-conditional">;
 def EnumCompareConditional : DiagGroup<"enum-compare-conditional",


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,8 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (72):
+CHECK: Warnings without flags (69):
+
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
 CHECK-NEXT:   ext_missing_declspec
@@ -41,9 +42,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8572,11 +8572,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D78444: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


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

https://reviews.llvm.org/D78444



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

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 261731.
rdwampler added a comment.

Rebase


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

https://reviews.llvm.org/D78442

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (72):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -44,9 +44,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8543,11 +8543,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,6 +60,7 @@
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
+def ClassConversion: DiagGroup<"class-conversion">;
 def DeprecatedEnumCompareConditional :
   DiagGroup<"deprecated-enum-compare-conditional">;
 def EnumCompareConditional : DiagGroup<"enum-compare-conditional",


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (72):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -44,9 +44,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8543,11 +8543,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78442



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

[PATCH] D78444: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 258613.
rdwampler added a comment.

changed to check it overrides a virtual function in a base class and not just 
virtual.


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

https://reviews.llvm.org/D78444

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/conversion-function.cpp


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class BaseA { };
+class DerivedA;
+
+class BaseB {
+virtual operator BaseA&() = 0;
+virtual operator DerivedA&() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+virtual operator BaseA&(); // OK. Overrides BaseB::operatorBaseA&()
+virtual operator DerivedA&(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+virtual operator DerivedB&(); // expected-warning{{conversion function 
converting 'DerivedB' to itself will never be used}}
+virtual operator BaseA&(); // expected-warning{{conversion function 
converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 +10486,12 @@
 
   // Make sure we aren't redeclaring the conversion function.
   QualType ConvType = 
Context.getCanonicalType(Conversion->getConversionType());
-
   // C++ [class.conv.fct]p1:
   //   [...] A conversion function is never used to convert a
   //   (possibly cv-qualified) object to the (possibly cv-qualified)
   //   same object type (or a reference to it), to a (possibly
   //   cv-qualified) base class of that type (or a reference to it),
   //   or to (possibly cv-qualified) void.
-  // FIXME: Suppress this warning if the conversion function ends up being a
-  // virtual function that overrides a virtual function in a base class.
   QualType ClassType
 = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl));
   if (const ReferenceType *ConvTypeRef = ConvType->getAs())
@@ -10502,6 +10499,8 @@
   if (Conversion->getTemplateSpecializationKind() != TSK_Undeclared &&
   Conversion->getTemplateSpecializationKind() != 
TSK_ExplicitSpecialization)
 /* Suppress diagnostics for instantiations. */;
+  else if(Conversion->size_overridden_methods() != 0)
+/* Suppress diagnostics for overriding virtual function in a base class. 
*/;
   else if (ConvType->isRecordType()) {
 ConvType = Context.getCanonicalType(ConvType).getUnqualifiedType();
 if (ConvType == ClassType)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10713,9 +10713,6 @@
   return Redeclaration;
 }
   }
-} else if (CXXConversionDecl *Conversion
-   = dyn_cast(NewFD)) {
-  ActOnConversionDeclarator(Conversion);
 } else if (auto *Guide = dyn_cast(NewFD)) {
   if (auto *TD = Guide->getDescribedFunctionTemplate())
 CheckDeductionGuideTemplate(TD);
@@ -10743,6 +10740,9 @@
   if (Method->isStatic())
 checkThisInStaticMemberFunctionType(Method);
 }
+
+if (CXXConversionDecl *Conversion = dyn_cast(NewFD))
+  ActOnConversionDeclarator(Conversion);
 
 // Extra checking for C++ overloaded operators (C++ [over.oper]).
 if (NewFD->isOverloadedOperator() &&


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,24 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class BaseA { };
+class DerivedA;
+
+class BaseB {
+virtual operator BaseA&() = 0;
+virtual operator DerivedA&() = 0;
+};
+
+class DerivedA : public BaseA, BaseB {
+virtual operator BaseA&(); // OK. Overrides BaseB::operatorBaseA&()
+virtual operator DerivedA&(); // OK. Overrides BaseB::operatorDerivedA&()
+};
+
+class DerivedB : public BaseA {
+virtual operator DerivedB&(); // expected-warning{{conversion function converting 'DerivedB' to itself will never be used}}
+virtual operator BaseA&(); // expected-warning{{conversion function converting 'DerivedB' to its base class 'BaseA' will never be used}}
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- 

[PATCH] D78444: Perform ActOnConversionDeclarator after looking for any virtual functions it overrides

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler created this revision.
rdwampler added reviewers: saar.raz, aaron.ballman, doug.gregor, rsmith.
rdwampler added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

This allows for suppressing warnings about the conversion function never being 
called if it overrides a virtual function in a base class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78444

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/conversion-function.cpp


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,27 @@
   operator const B(); // expected-warning{{conversion function converting 'B' 
to itself will never be used}}
 };
 
+class DerivedB;
+
+class BaseA {
+public:
+  virtual operator BaseA&() = 0; //OK. Virtual function 
+  virtual operator DerivedB&() = 0; //OK. Virtual function
+  virtual operator const void() = 0; //OK. Virtual function
+};
+
+class DerivedB : public BaseA {
+#if __cplusplus >= 201103L
+operator BaseA&() override; //OK. Overrides virtual function in BaseA
+operator DerivedB&() override; //OK. Overrides virtual function BaseA
+operator const void() override; //OK. Overrides virtual function BaseA
+#else
+virtual operator BaseA&(); //OK. Overrides virtual function in BaseA
+virtual operator DerivedB&(); //OK. Overrides virtual function BaseA
+virtual operator const void(); //OK. Overrides virtual function BaseA
+#endif
+};
+
 // This used to crash Clang.
 struct Flip;
 struct Flop {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10486,15 +10486,12 @@
 
   // Make sure we aren't redeclaring the conversion function.
   QualType ConvType = 
Context.getCanonicalType(Conversion->getConversionType());
-
   // C++ [class.conv.fct]p1:
   //   [...] A conversion function is never used to convert a
   //   (possibly cv-qualified) object to the (possibly cv-qualified)
   //   same object type (or a reference to it), to a (possibly
   //   cv-qualified) base class of that type (or a reference to it),
   //   or to (possibly cv-qualified) void.
-  // FIXME: Suppress this warning if the conversion function ends up being a
-  // virtual function that overrides a virtual function in a base class.
   QualType ClassType
 = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl));
   if (const ReferenceType *ConvTypeRef = ConvType->getAs())
@@ -10502,6 +10499,8 @@
   if (Conversion->getTemplateSpecializationKind() != TSK_Undeclared &&
   Conversion->getTemplateSpecializationKind() != 
TSK_ExplicitSpecialization)
 /* Suppress diagnostics for instantiations. */;
+  else if(Conversion->isVirtual())
+/*Suppress diagnostics if the function is virtual*/;
   else if (ConvType->isRecordType()) {
 ConvType = Context.getCanonicalType(ConvType).getUnqualifiedType();
 if (ConvType == ClassType)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10713,9 +10713,6 @@
   return Redeclaration;
 }
   }
-} else if (CXXConversionDecl *Conversion
-   = dyn_cast(NewFD)) {
-  ActOnConversionDeclarator(Conversion);
 } else if (auto *Guide = dyn_cast(NewFD)) {
   if (auto *TD = Guide->getDescribedFunctionTemplate())
 CheckDeductionGuideTemplate(TD);
@@ -10743,6 +10740,9 @@
   if (Method->isStatic())
 checkThisInStaticMemberFunctionType(Method);
 }
+
+if (CXXConversionDecl *Conversion = dyn_cast(NewFD))
+  ActOnConversionDeclarator(Conversion);
 
 // Extra checking for C++ overloaded operators (C++ [over.oper]).
 if (NewFD->isOverloadedOperator() &&


Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -62,6 +62,27 @@
   operator const B(); // expected-warning{{conversion function converting 'B' to itself will never be used}}
 };
 
+class DerivedB;
+
+class BaseA {
+public:
+  virtual operator BaseA&() = 0; //OK. Virtual function 
+  virtual operator DerivedB&() = 0; //OK. Virtual function
+  virtual operator const void() = 0; //OK. Virtual function
+};
+
+class DerivedB : public BaseA {
+#if __cplusplus >= 201103L
+operator BaseA&() override; //OK. Overrides virtual function in BaseA
+operator DerivedB&() override; //OK. Overrides virtual function BaseA
+operator const void() override; //OK. Overrides virtual function BaseA
+#else
+virtual operator BaseA&(); //OK. Overrides virtual function in BaseA
+virtual operator 

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler created this revision.
rdwampler added a reviewer: rsmith.
rdwampler added a project: clang.
Herald added a subscriber: cfe-commits.

These warnings are grouped under '-Wclass-conversion' to be compatiable with 
GCC 9.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78442

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (74):
+CHECK: Warnings without flags (71):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -44,9 +44,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8537,11 +8537,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that may be intended as compound assignment (%0=)">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,6 +60,7 @@
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
+def ClassConversion: DiagGroup<"class-conversion">;
 def DeprecatedEnumCompareConditional :
   DiagGroup<"deprecated-enum-compare-conditional">;
 def EnumCompareConditional : DiagGroup<"enum-compare-conditional",


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (74):
+CHECK: Warnings without flags (71):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -44,9 +44,6 @@
 CHECK-NEXT:   warn_char_constant_too_large
 CHECK-NEXT:   warn_collection_expr_type
 CHECK-NEXT:   warn_conflicting_variadic
-CHECK-NEXT:   warn_conv_to_base_not_used
-CHECK-NEXT:   warn_conv_to_self_not_used
-CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8537,11 +8537,14 @@
 def err_conv_function_redeclared : Error<
   "conversion function cannot be redeclared">;
 def warn_conv_to_self_not_used : Warning<
-  "conversion function converting %0 to itself will never be used">;
+  "conversion function converting %0 to itself will never be used">,
+  InGroup;
 def warn_conv_to_base_not_used : Warning<
-  "conversion function converting %0 to its base class %1 will never be used">;
+  "conversion function converting %0 to its base class %1 will never be used">,
+  InGroup;
 def warn_conv_to_void_not_used : Warning<
-  "conversion function converting %0 to %1 will never be used">;
+  "conversion function converting %0 to %1 will never be used">,
+  InGroup;
 
 def warn_not_compound_assign : Warning<
   "use of unary operator that 

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1612
+  Keywords.kw_final) ||
+isCpp11AttributeSpecifier(*Tok.Next))
   return false;

I think the issue r373922 was fixing is only related to the `delete`. Can this 
check be move further up where we explicitly check if the token is the delete 
keyword? 



Repository:
  rC Clang

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

https://reviews.llvm.org/D69164



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

In D64695#1605676 , @Manikishan wrote:

> In D64695#1590948 , @Manikishan 
> wrote:
>
> > In D64695#1589818 , @lebedev.ri 
> > wrote:
> >
> > > In D64695#1589740 , @Manikishan 
> > > wrote:
> > >
> > > > In D64695#1589508 , 
> > > > @lebedev.ri wrote:
> > > >
> > > > > Is there sufficient test coverage as to what happens if 
> > > > > `SortPriority` is not set?
> > > >
> > > >
> > > > If SortPriority is not set, the Includes will be grouped without 
> > > > sorting,
> > >
> > >
> > > Let me rephrase - for the exiting `.clang-format`s, that don't currently 
> > > specify `SortPriority`,
> > >  this introduction of `SortPriority` should not change the header 
> > > handling.
> > >  Is that the case, and if so is there sufficient test coverage for that?
> >
> >
> > I got your idea now.
> >  No, there is no test coverage for that case, and with the current patch 
> > they have to add SortPriority.
> >  To avoid this shall I set SortPriority as Priority as default if it is not 
> > defined? I think that will fix the issue.
>
>
> any reviews on it ?


That's sounds like it will work. Can you add some additional test cases around 
this in `SortIncludesTest.cpp`. Also, adding a test case specifically for 
sorting the NetBSD headers would be good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Thanks! Can you update the documentation too?




Comment at: lib/Tooling/Inclusions/HeaderIncludes.cpp:211
+  Ret = 0;
+}
+  return Ret;

I think you can drop the else block and just return immediately from the for 
loop.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

> Sorry,  my mistake I meant that I have added Regex for priorities while 
> sorting and If I am not wrong I think IncludeCategories are used while 
> Regrouping after sorting the Includes. In addition to that in my case I have 
> to sort the includes In a particular order then grouping them in different
>  For example:
> 
>   #include   /*  first, */
>   #include   /*next, */
>   #include   /*   and then the rest, */
>   #include 
>   #include 
>   
>   #include 
>   #include 
>   #include 
>   #include 
>   #include 
>   
> 
> 
> As shown in the above example  should follow  then  but while 
> regrouping they should be in the same group.

OK. I think I understand. Since  and  have to come 
first we would need to set the regex and priorities for those individually. But 
then those will be treated as separate block and  grouped like:

#include 

#include 

#include   
more  includes

I wonder if a better approach would be to extend `IncludeCatalog` to allow the 
grouping to specified independent of the regex?

E.g.,  { "{", 0, 1 },  // priority 1, group 1

  {, 1, 1, }, // different priority, but same group
 {, 2, 1}  // more general headers with lower priority, but will 
still be grouped in group 1.
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

I am not quite sure why this change is required to sort the headers for NetBSD, 
you can set the priorities via `IncludeStyle.IncludeCategories`. Is that not 
sufficient?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357027: [clang-format] Add style option 
AllowShortLambdasOnASingleLine (authored by rdwampler, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57687?vs=191872=192337#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57687

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

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -306,6 +306,39 @@
   /// If ``true``, ``if (a) return;`` can be put on a single line.
   ShortIfStyle AllowShortIfStatementsOnASingleLine;
 
+  /// Different styles for merging short lambdas containing at most one
+  /// statement.
+  enum ShortLambdaStyle {
+/// Never merge lambdas into a single line.
+SLS_None,
+/// Only merge empty lambdas.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) {
+///   return a;
+///   };
+/// \endcode
+SLS_Empty,
+/// Merge lambda into a single line if argument of a function.
+/// \code
+///   auto lambda = [](int a) {
+///   return a;
+///   };
+///   sort(a.begin(), a.end(), ()[] { return x < y; })
+/// \endcode
+SLS_Inline,
+/// Merge all lambdas fitting on a single line.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) { return a; };
+/// \endcode
+SLS_All,
+  };
+
+  /// Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  /// single line.
+  ShortLambdaStyle AllowShortLambdasOnASingleLine;
+
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.
   bool AllowShortLoopsOnASingleLine;
@@ -1805,6 +1838,7 @@
R.AllowShortFunctionsOnASingleLine &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
+   AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -65,6 +65,7 @@
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \
   TYPE(LambdaArrow)\
+  TYPE(LambdaLBrace)   \
   TYPE(LambdaLSquare)  \
   TYPE(LeadingJavaAnnotation)  \
   TYPE(LineComment)\
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -119,6 +119,17 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO , FormatStyle::ShortLambdaStyle ) {
+IO.enumCase(Value, "None", FormatStyle::SLS_None);
+IO.enumCase(Value, "false", FormatStyle::SLS_None);
+IO.enumCase(Value, "Empty", FormatStyle::SLS_Empty);
+IO.enumCase(Value, "Inline", FormatStyle::SLS_Inline);
+IO.enumCase(Value, "All", FormatStyle::SLS_All);
+IO.enumCase(Value, "true", FormatStyle::SLS_All);
+  }
+};
+
 template <> struct ScalarEnumerationTraits {
   static void enumeration(IO , FormatStyle::BinPackStyle ) {
 IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
@@ -347,6 +358,8 @@
Style.AllowShortCaseLabelsOnASingleLine);
 IO.mapOptional("AllowShortFunctionsOnASingleLine",
Style.AllowShortFunctionsOnASingleLine);
+IO.mapOptional("AllowShortLambdasOnASingleLine",
+   

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: clang/include/clang/Format/Format.h:264
+/// If Else statements have no braces don't put them
+/// on the same line.
+/// \code

These comments can be used to auto generate the corresponding sections in 
clang/docs/ClangFormatStyleOptions.rst using 
clang/docs/tools/dump_format_style.py, so they should probably be kept in sync.


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

https://reviews.llvm.org/D59408



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

I don't have commit rights, can someone land this for me or would it be better 
to try and get commit access?


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

https://reviews.llvm.org/D57687



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 191872.
rdwampler added a comment.

Rebased on master.


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

https://reviews.llvm.org/D57687

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12239,6 +12239,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1191,11 +1191,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2896,7 +2896,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -3024,6 +3024,19 @@
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
 return true;
 
+  if (Left.is(TT_LambdaLBrace)) {
+if (Left.MatchingParen && Left.MatchingParen->Next &&
+Left.MatchingParen->Next->isOneOf(tok::comma, tok::r_paren) &&
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
+  return false;
+
+if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||
+(!Left.Children.empty() &&
+ Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Empty))
+  return true;
+  }
+
   // Put multiple C# attributes on a new line.
   if 

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2976
+
+return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+   Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||

klimek wrote:
> If SLS_All is supposed to not change anything, should we fall through here if 
> (SLS_All)?
Yes, I think that's a better approach to ensure the current behavior is 
unchanged.


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

https://reviews.llvm.org/D57687



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 191629.
rdwampler marked 2 inline comments as done.
rdwampler added a comment.

Changes since last revision:

-rebased 
-Fall through when SLS_All.


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

https://reviews.llvm.org/D57687

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12239,6 +12239,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1145,11 +1145,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2849,7 +2849,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2977,6 +2977,19 @@
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
 return true;
 
+  if (Left.is(TT_LambdaLBrace)) {
+if (Left.MatchingParen && Left.MatchingParen->Next &&
+Left.MatchingParen->Next->isOneOf(tok::comma, tok::r_paren) &&
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
+  return false;
+
+if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||
+(!Left.Children.empty() &&
+ Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Empty))
+  

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping?


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

https://reviews.llvm.org/D57687



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

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping? Can somebody commit this for me? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58186



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/Format.cpp:649
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
+  LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;

MyDeveloperDay wrote:
> What is the difference between what clang-format does now and using SLS_All, 
> i.e. if your introducing a new style shouldn't the default be to not change 
> the exsiting code?
> 
> Without trying this myself I would think this needs to be SLS_None? (am I 
> wrong?)
AFAICT, the current behavior is to always put lambdas on a single line, if 
possible.


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

https://reviews.llvm.org/D57687



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

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

In D58186#1405687 , @sylvestre.ledru 
wrote:

> Do you have permissions on the repo?


No, I would need someone to commit this for me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58186



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

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58186



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 187260.

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

https://reviews.llvm.org/D57687

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11966,6 +11966,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1436,6 +1436,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1142,11 +1142,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2839,7 +2839,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2967,6 +2967,18 @@
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
 return true;
 
+  if (Left.is(TT_LambdaLBrace)) {
+if (Left.MatchingParen && Left.MatchingParen->Next &&
+Left.MatchingParen->Next->isOneOf(tok::comma, tok::r_paren) &&
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
+  

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler created this revision.
rdwampler added reviewers: eugene, sylvestre.ledru, djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These changes were corrected directly in ClangFormatStyleOptions.rst (llvm-svn: 
350192 and llvm-svn: 351976) but these sections can be produced automatically 
using `dump_format_style.py` so sync the corresponding doc comments in 
`Format.h` as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58186

Files:
  clang/include/clang/Format/Format.h
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h


Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -67,7 +67,7 @@
   /// used for ordering ``#includes``.
   ///
   /// `POSIX extended
-  /// 
`_
+  /// 
`_
   /// regular expressions are supported.
   ///
   /// These regular expressions are matched against the filename of an include
@@ -79,7 +79,7 @@
   /// If none of the regular expressions match, INT_MAX is assigned as
   /// category. The main header for a source file automatically gets category 
0.
   /// so that it is generally kept at the beginning of the ``#includes``
-  /// (http://llvm.org/docs/CodingStandards.html#include-style). However, you
+  /// (https://llvm.org/docs/CodingStandards.html#include-style). However, you
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
   ///
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1130,7 +1130,7 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is seperated by a newline. Static imports will also follow the
+  /// Each group is separated by a newline. Static imports will also follow the
   /// same grouping convention above all non-static imports. One group's prefix
   /// can be a subset of another - the longest prefix is always matched. Within
   /// a group, the imports are ordered lexicographically.


Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -67,7 +67,7 @@
   /// used for ordering ``#includes``.
   ///
   /// `POSIX extended
-  /// `_
+  /// `_
   /// regular expressions are supported.
   ///
   /// These regular expressions are matched against the filename of an include
@@ -79,7 +79,7 @@
   /// If none of the regular expressions match, INT_MAX is assigned as
   /// category. The main header for a source file automatically gets category 0.
   /// so that it is generally kept at the beginning of the ``#includes``
-  /// (http://llvm.org/docs/CodingStandards.html#include-style). However, you
+  /// (https://llvm.org/docs/CodingStandards.html#include-style). However, you
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
   ///
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1130,7 +1130,7 @@
 
   /// A vector of prefixes ordered by the desired groups for Java imports.
   ///
-  /// Each group is seperated by a newline. Static imports will also follow the
+  /// Each group is separated by a newline. Static imports will also follow the
   /// same grouping convention above all non-static imports. One group's prefix
   /// can be a subset of another - the longest prefix is always matched. Within
   /// a group, the imports are ordered lexicographically.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 185848.
rdwampler added a comment.

Update to include an option `Inline` to only put short lambda on a single line 
if used as an argument. See the following code style guide from catboost: 
ps://github.com/catboost/catboost/blob/master/CPP_STYLE_GUIDE.md#lambda-functions

I'm not sure if `Inline` is the best name. I welcome suggestions.

Thanks!


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

https://reviews.llvm.org/D57687

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2835,7 +2835,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2963,6 +2963,18 @@
  

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler created this revision.
rdwampler added reviewers: djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This option `AllowShortLambdasOnASingleLine` similar to the other `AllowShort*` 
options, but applied to C++ lambdas.

I considered making this dependent on `AllowShortFunctionsOnASingleLine`, but 
could not think of how the breaking should be behave when the option is set to 
`Inline` and `InlineOnly`.

I could not find a specific coding style that requires short lambdas to 
//always// break on a new line, but I do think it's fairly common practice. 
Also, I note that the patch: https://reviews.llvm.org/D44609 attempts to add an 
option to allow formatting lambda using the Allman style braces. Here is a 
coding style that requires Allman style braces, but allows lambdas to be on 
single line if they are short. So, maybe this option would be required to 
handle this situation?

Addresses: https://bugs.llvm.org//show_bug.cgi?id=32151


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57687

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,30 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   " return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2835,7 +2835,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2962,6 +2962,10 @@

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 102150.
rdwampler added a comment.

I was able to build and test this on a linux box. The issue was the whitespace 
surrounding the regex.

On Linux, `(unavailable)` is not present. I.e,
`FunctionDecl=foo:3:6  (ios, introduced=3.2, deprecated=4.1)  (macos, 
introduced=10.4, deprecated=10.5, obsoleted=10.7)`
vs on macOS
`FunctionDecl=foo:3:6 (unavailable)  (ios, introduced=3.2, deprecated=4.1) 
(unavailable)  (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)`

I changed it to just match any characters between the function declaration and 
the availabilities.

Sorry for the noise.


https://reviews.llvm.org/D33478

Files:
  test/Index/availability.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7216,15 +7216,11 @@
   return Out;
 }
 
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
-   CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl ) {
   bool HadAvailAttr = false;
-  int N = 0;
   for (auto A : D->attrs()) {
 if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
   HadAvailAttr = true;
@@ -7236,7 +7232,7 @@
   }
   continue;
 }
-
+
 if (UnavailableAttr *Unavailable = dyn_cast(A)) {
   HadAvailAttr = true;
   if (always_unavailable)
@@ -7247,38 +7243,71 @@
   }
   continue;
 }
-
+
 if (AvailabilityAttr *Avail = dyn_cast(A)) {
+  AvailabilityAttrs.push_back(Avail);
   HadAvailAttr = true;
-  if (N < availability_size) {
-availability[N].Platform
-  = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
-  }
-  ++N;
 }
   }
 
   if (!HadAvailAttr)
 if (const EnumConstantDecl *EnumConst = dyn_cast(D))
   return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
-  always_deprecated,
-  deprecated_message,
-  always_unavailable,
-  unavailable_message,
-  availability,
-  availability_size);
-  
-  return N;
+  cast(EnumConst->getDeclContext()), always_deprecated,
+  deprecated_message, always_unavailable, unavailable_message,
+  AvailabilityAttrs);
+
+  if (AvailabilityAttrs.empty())
+return;
+
+  std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+  return LHS->getPlatform() > RHS->getPlatform();
+});
+  ASTContext  = D->getASTContext();
+  auto It = std::unique(
+  AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+  [](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() != RHS->getPlatform())
+  return false;
+
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+  return true;
+
+if ((!LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()))
+  return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+  LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+  LHS->setDeprecated(Ctx, RHS->getDeprecated());
+  if (LHS->getMessage().empty())
+

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Alex, I will look into it. Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D33478



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

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

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

I don't have commit access, but I would grateful if you can commit it.


https://reviews.llvm.org/D33478



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

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 102050.
rdwampler added a comment.

Remove an unnecessary check when determining a mismatch of availabilities
Simplify invocation of FileCheck in availability.c


https://reviews.llvm.org/D33478

Files:
  test/Index/availability.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,11 @@
   return Out;
 }
 
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
-   CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl ) {
   bool HadAvailAttr = false;
-  int N = 0;
   for (auto A : D->attrs()) {
 if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
   HadAvailAttr = true;
@@ -7220,7 +7216,7 @@
   }
   continue;
 }
-
+
 if (UnavailableAttr *Unavailable = dyn_cast(A)) {
   HadAvailAttr = true;
   if (always_unavailable)
@@ -7231,38 +7227,71 @@
   }
   continue;
 }
-
+
 if (AvailabilityAttr *Avail = dyn_cast(A)) {
+  AvailabilityAttrs.push_back(Avail);
   HadAvailAttr = true;
-  if (N < availability_size) {
-availability[N].Platform
-  = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
-  }
-  ++N;
 }
   }
 
   if (!HadAvailAttr)
 if (const EnumConstantDecl *EnumConst = dyn_cast(D))
   return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
-  always_deprecated,
-  deprecated_message,
-  always_unavailable,
-  unavailable_message,
-  availability,
-  availability_size);
-  
-  return N;
+  cast(EnumConst->getDeclContext()), always_deprecated,
+  deprecated_message, always_unavailable, unavailable_message,
+  AvailabilityAttrs);
+
+  if (AvailabilityAttrs.empty())
+return;
+
+  std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+  return LHS->getPlatform() > RHS->getPlatform();
+});
+  ASTContext  = D->getASTContext();
+  auto It = std::unique(
+  AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+  [](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() != RHS->getPlatform())
+  return false;
+
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+  return true;
+
+if ((!LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()))
+  return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+  LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+  LHS->setDeprecated(Ctx, RHS->getDeprecated());
+  if (LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+  if (LHS->getReplacement().empty())
+LHS->setReplacement(Ctx, RHS->getReplacement());
+}
+
+if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) {
+  LHS->setObsoleted(Ctx, RHS->getObsoleted());
+  if (LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+  if (LHS->getReplacement().empty())
+ 

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

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



Comment at: test/Index/availability.c:20
 // CHECK-2: (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
 
 // CHECK-2: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated)

Can we run `FileCheck` once now? I believe the `CHECK-1` and `CHECK-2` were 
added since there were no particular order for the availabilities. With this 
patch the order is guarantee to be stable.



Comment at: tools/libclang/CIndex.cpp:7268
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) ||
+(!LHS->getMessage().empty() && !RHS->getMessage().empty()))
+  return false;

arphaman wrote:
> I think that we don't really need the `(!LHS->getMessage().empty() && 
> !RHS->getMessage().empty())` check here since message has to be either in a 
> deprecated or obsoleted clause, so we should already handle that with 
> previous checks.
Agreed.


https://reviews.llvm.org/D33478



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

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 101787.
rdwampler marked 6 inline comments as done.
rdwampler added a comment.

This should resolve the bug (a conditional was inverted) in the last revision 
along with the other changes requested.


https://reviews.llvm.org/D33478

Files:
  test/Index/availability.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,11 @@
   return Out;
 }
 
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
-   CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl ) {
   bool HadAvailAttr = false;
-  int N = 0;
   for (auto A : D->attrs()) {
 if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
   HadAvailAttr = true;
@@ -7220,7 +7216,7 @@
   }
   continue;
 }
-
+
 if (UnavailableAttr *Unavailable = dyn_cast(A)) {
   HadAvailAttr = true;
   if (always_unavailable)
@@ -7231,38 +7227,72 @@
   }
   continue;
 }
-
+
 if (AvailabilityAttr *Avail = dyn_cast(A)) {
+  AvailabilityAttrs.push_back(Avail);
   HadAvailAttr = true;
-  if (N < availability_size) {
-availability[N].Platform
-  = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
-  }
-  ++N;
 }
   }
 
   if (!HadAvailAttr)
 if (const EnumConstantDecl *EnumConst = dyn_cast(D))
   return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
-  always_deprecated,
-  deprecated_message,
-  always_unavailable,
-  unavailable_message,
-  availability,
-  availability_size);
-  
-  return N;
+  cast(EnumConst->getDeclContext()), always_deprecated,
+  deprecated_message, always_unavailable, unavailable_message,
+  AvailabilityAttrs);
+
+  if (AvailabilityAttrs.empty())
+return;
+
+  std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+  return LHS->getPlatform() > RHS->getPlatform();
+});
+  ASTContext  = D->getASTContext();
+  auto It = std::unique(
+  AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+  [](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() != RHS->getPlatform())
+  return false;
+
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+  return true;
+
+if ((!LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) ||
+(!LHS->getMessage().empty() && !RHS->getMessage().empty()))
+  return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+  LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+  LHS->setDeprecated(Ctx, RHS->getDeprecated());
+  if (LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+  if (LHS->getReplacement().empty())
+LHS->setReplacement(Ctx, RHS->getReplacement());
+}
+
+if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) {
+  LHS->setObsoleted(Ctx, RHS->getObsoleted());
+  if 

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 101569.
rdwampler added a comment.

Rearrange `if` statements in `getCursorPlatformAvailabilityForDecl` to return 
early if we do not need to merge availability attributes.
Use ranged for loop.


https://reviews.llvm.org/D33478

Files:
  test/Index/availability.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,11 @@
   return Out;
 }
 
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
-   CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl ) {
   bool HadAvailAttr = false;
-  int N = 0;
   for (auto A : D->attrs()) {
 if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
   HadAvailAttr = true;
@@ -7220,7 +7216,7 @@
   }
   continue;
 }
-
+
 if (UnavailableAttr *Unavailable = dyn_cast(A)) {
   HadAvailAttr = true;
   if (always_unavailable)
@@ -7231,38 +7227,72 @@
   }
   continue;
 }
-
+
 if (AvailabilityAttr *Avail = dyn_cast(A)) {
+  AvailabilityAttrs.push_back(Avail);
   HadAvailAttr = true;
-  if (N < availability_size) {
-availability[N].Platform
-  = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
-  }
-  ++N;
 }
   }
 
   if (!HadAvailAttr)
 if (const EnumConstantDecl *EnumConst = dyn_cast(D))
-  return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
-  always_deprecated,
-  deprecated_message,
-  always_unavailable,
-  unavailable_message,
-  availability,
-  availability_size);
-  
-  return N;
+  getCursorPlatformAvailabilityForDecl(
+  cast(EnumConst->getDeclContext()), always_deprecated,
+  deprecated_message, always_unavailable, unavailable_message,
+  AvailabilityAttrs);
+
+  if (AvailabilityAttrs.empty())
+return;
+
+  std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+  return LHS->getPlatform() > RHS->getPlatform();
+});
+  ASTContext  = D->getASTContext();
+  auto It = std::unique(
+  AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+  [](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() == RHS->getPlatform()) {
+  if (LHS->getIntroduced() == RHS->getIntroduced() &&
+  LHS->getDeprecated() == RHS->getDeprecated() &&
+  LHS->getObsoleted() == RHS->getObsoleted() &&
+  LHS->getMessage() == RHS->getMessage() &&
+  LHS->getReplacement() == RHS->getReplacement())
+return true;
+
+  if ((!LHS->getIntroduced().empty() &&
+   !RHS->getIntroduced().empty()) ||
+  (!LHS->getDeprecated().empty() &&
+   !RHS->getDeprecated().empty()) ||
+  (!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) ||
+  (!LHS->getMessage().empty() && !RHS->getMessage().empty()))
+return false;
+
+  if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+  if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+LHS->setDeprecated(Ctx, RHS->getDeprecated());
+if (!LHS->getMessage().empty())
+  LHS->setMessage(Ctx, RHS->getMessage());
+if (!LHS->getReplacement().empty())
+  LHS->setReplacement(Ctx, RHS->getReplacement());
+  }
+
+  if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) {

[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: tools/libclang/CIndex.cpp:7322
+
+  for (int I = 0, E = AvailabilityAttrs.size(); I < E && I < availability_size;
+   ++I) {

arphaman wrote:
> You can use a ranged for loop here if you use `take_front`, e.g.
> 
> ```
> for (const auto *Avail : AvailabilityAttrs.take_front(availability_size))
> ```
I would need to covert this to an `ArrayRef`, I believe. Also, I would need to 
check `availability_size` is in bounds. Would something like the following be 
preferred:
```
int N = 0;
for (const auto *Avail : AvailabilityAttrs) {
  if (N < availability_size) {
// populate availability
N++;
  }
}
```


https://reviews.llvm.org/D33478



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