[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f3b0b417143: [clang] adds 
`__reference_constructs_from_temporary` (authored by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -292,10 +292,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  is implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2542,6 +2542,51 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5410,14 +5410,15 @@
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
-  // Evaluate BTT_ReferenceBindsToTemporary alongside the IsConstructible
-  // traits to avoid duplication.
-  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary)
+  // Evaluate ReferenceBindsToTemporary and ReferenceConstructsFromTemporary
+  // alongside the IsConstructible traits to avoid duplication.
+  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary && Kind != BTT_ReferenceConstructsFromTemporary)
 return 

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 556693.
cjdb marked an inline comment as done.
cjdb added a comment.

partially reverts change and fixes typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -292,10 +292,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  is implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2542,6 +2542,51 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5410,14 +5410,15 @@
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
-  // Evaluate BTT_ReferenceBindsToTemporary alongside the IsConstructible
-  // traits to avoid duplication.
-  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary)
+  // Evaluate ReferenceBindsToTemporary and ReferenceConstructsFromTemporary
+  // alongside the IsConstructible traits to avoid duplication.
+  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary && Kind != BTT_ReferenceConstructsFromTemporary)
 return EvaluateBinaryTypeTrait(S, Kind, Args[0]->getType(),
Args[1]->getType(), 

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5515
+  QualType UPtr = S.BuiltinAddPointer(S.BuiltinRemoveReference(U, 
UnaryTransformType::RemoveCVRef, {}), {});
+  return EvaluateBinaryTypeTrait(S, TypeTrait::BTT_IsConvertibleTo, UPtr, 
TPtr, RParenLoc);
 }

cor3ntin wrote:
> cor3ntin wrote:
> > Could you put the implementation of `BTT_IsConvertibleTo` in its own 
> > function?
> I was thinking something like
> 
> ```
> QualType TPtr = S.Context.getPointerType(S.BuiltinRemoveReference(T, 
> UnaryTransformType::RemoveCVRef, {}));
> QualType UPtr = S.Context.getPointerType(S.BuiltinRemoveReference(U, 
> UnaryTransformType::RemoveCVRef, {}));
> return IsConvertibleTo(S, UPtr, TPtr);
> ```
> 
> and then EvaluateBinaryTypeTrait can use this new `IsConvertibleTo` (which we 
> can extract from EvaluateBinaryTypeTrait)
That sounds like an orthogonal change. How about I revert to what was 
previously there and do that as a second PR? It'll also need a location 
parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[PATCH] D129538: [clang] adds prototype for being able to alternate diagnostic formats

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

Abandoning since this design is obsolete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129538

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

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:33
 #include "clang/Basic/TypeTraits.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Preprocessor.h"

cor3ntin wrote:
> Superfluous change
Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 556609.
cjdb marked 3 inline comments as done.
cjdb added a comment.

- responds to @cor3ntin's feedback
- updates commit message to include `[clang]` tag and also indicate this is a 
revert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -292,10 +292,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2542,6 +2542,51 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5399,6 +5399,13 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
+static bool EvaluateIsConvertibleTo(Sema , QualType T, QualType U, SourceLocation Loc)
+{
+  QualType TPtr = S.Context.getPointerType(S.BuiltinRemoveReference(T, UnaryTransformType::RemoveCVRef, {}));
+  QualType UPtr = S.Context.getPointerType(S.BuiltinRemoveReference(U, UnaryTransformType::RemoveCVRef, {}));
+  return EvaluateBinaryTypeTrait(S, TypeTrait::BTT_IsConvertibleTo, UPtr, TPtr, Loc);
+}
+
 static bool 

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Sorry, I had read @erichkeane's LGTM and taken that as actually approved. Would 
you like me to revert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe1bfeb6bcc62: adds `__reference_constructs_from_temporary` 
(authored by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -292,10 +292,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2542,6 +2542,51 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -30,6 +30,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
@@ -5410,14 +5411,15 @@
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
-  // Evaluate BTT_ReferenceBindsToTemporary alongside the IsConstructible
-  // traits to 

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 556500.
cjdb added a comment.

runs 'arc diff `git merge-base HEAD origin` --update D135238 
' to try and get CI happy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135238

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -292,10 +292,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2542,6 +2542,51 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -30,6 +30,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
@@ -5410,14 +5411,15 @@
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
-  // Evaluate BTT_ReferenceBindsToTemporary alongside the IsConstructible
-  // traits to avoid duplication.
-  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary)
+  // Evaluate ReferenceBindsToTemporary 

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 556499.
cjdb added a comment.

I somehow made cxx_status.html worse in the previous commit, not better


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -292,10 +292,9 @@
   https://wg21.link/P2255R2;>P2255R2
   
 Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  implemented. __reference_converts_from_temporary needs to be
   provided, following the normal cross-vendor convention to implement
   traits requiring compiler support directly.
 
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2542,6 +2542,51 @@
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef;
+  using RRef = ConvertsToRef;
+  using CLRef = ConvertsToRef;
+  using LongRef = ConvertsToRef;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -30,6 +30,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
@@ -5410,14 +5411,15 @@
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
-  // Evaluate BTT_ReferenceBindsToTemporary alongside the IsConstructible
-  // traits to avoid duplication.
-  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary)
+  // Evaluate ReferenceBindsToTemporary and ReferenceConstructsFromTemporary
+  // 

[PATCH] D135341: adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 556497.
cjdb retitled this revision from "[clang] adds 
`__reference_constructs_from_temporary`" to "adds 
`__reference_constructs_from_temporary`".
cjdb edited the summary of this revision.
cjdb added a comment.

Corrects cxx_status.html, which accidentally duplicated a lot of the file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -228,199 +228,6 @@
   https://wg21.link/P2797R0;>P2797R0
   No
 
-
-  Change scope of lambda trailing-return-type
-  https://wg21.link/P2036R3;>P2036R3
-  Clang 17
-
-
-  https://wg21.link/P2579R0;>P2579R0
-
-
-  Multidimensional subscript operator
-  https://wg21.link/P2128R6;>P2128R6
-  Clang 15
-
-
-  Non-literal variables (and labels and gotos) in constexpr functions
-  https://wg21.link/P2242R3;>P2242R3
-  Clang 15
-
-
-  Character encoding of diagnostic text
-  https://wg21.link/P2246R1;>P2246R1
-  Yes
-
-
-  Character sets and encodings
-  https://wg21.link/P2314R4;>P2314R4
-  Yes
-
-
-  Consistent character literal encoding
-  https://wg21.link/P2316R2;>P2316R2
-  Yes
-
-
-  Add support for preprocessing directives elifdef and elifndef
-  https://wg21.link/P2334R1;>P2334R1
-  Clang 13
-
-
-  Extend init-statement to allow alias-declaration
-  https://wg21.link/P2360R0;>P2360R0
-  Clang 14
-
-
-  auto(x): decay-copy in the language
-  https://wg21.link/P0849R8;>P0849R8
-  Clang 15
-
-
-
-  Attributes on Lambda-Expressions
-  https://wg21.link/P2173R1;>P2173R1
-  Clang 13
-
-
-  constexpr for cmath and cstdlib
-  https://wg21.link/P0533R9;>P0533R9
-  No
-
-
-  Type trait to determine if a reference binds to a temporary
-  https://wg21.link/P2255R2;>P2255R2
-  
-Partial
-  Clang provides a __reference_binds_to_temporary type trait
-  builtin, with which the library facility can be partially implemented.
-  Both __reference_constructs_from_temporary and
-  __reference_converts_from_temporary builtins should be
-  provided, following the normal cross-vendor convention to implement
-  traits requiring compiler support directly.
-
-  
-
-
-
-  The Equality Operator You Are Looking For
-  https://wg21.link/P2468R2;>P2468R2
-  Clang 16
-
-
-  De-deprecating volatile compound operations
-  https://wg21.link/P2327R1;>P2327R1
-  Clang 15
-
-
-  Support for #warning
-  https://wg21.link/P2437R1;>P2437R1
-  Yes
-
-
-  Remove non-encodable wide character literals and multicharacter wide character literals
-  https://wg21.link/P2362R3;>P2362R3
-  Clang 14
-
-
-  Labels at the end of compound statements
-  https://wg21.link/P2324R2;>P2324R2
-  Clang 16
-
-
-  Delimited escape sequences
-  https://wg21.link/P2290R3;>P2290R3
-  Clang 15
-
-
-  Named universal character escapes
-  https://wg21.link/P2071R2;>P2071R2
-  Clang 15
-
-
-  Relaxing some constexpr restrictions
-  https://wg21.link/P2448R2;>P2448R2
-  
-Clang 17 (Partial)
-	  We do not support outside of defaulted special memeber functions the change that constexpr functions no
-  longer have to be constexpr compatible but rather support a less restricted requirements for constexpr
-  functions. Which include allowing non-literal types as return values and parameters, allow calling of
-  non-constexpr functions and constructors.
-
-  
-
-
-  Using unknown pointers and references in constant expressions
-  https://wg21.link/P2280R4;>P2280R4 (DR)
-  No
-
-
-  static operator()
-  https://wg21.link/P1169R4;>P1169R4
-  Clang 16
-
-
-  Extended floating-point types and standard names
-  https://wg21.link/P1467R9;>P1467R9
-  No
-
-
-  Class template argument deduction from inherited constructors
-  https://wg21.link/P2582R1;>P2582R1
-  No
-
-
-  Portable assumptions
-  https://wg21.link/P1774R8;>P1774R8
-  No
-
-
-  Support for UTF-8 as a portable source file encoding
-  https://wg21.link/P2295R6;>P2295R6
-  Clang 15

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Oh, I shouldn't have so much green in the status, whoops!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D135341#4643712 , @erichkeane 
wrote:

> Is the changes to cxx_status.html still relevant/current?

Yeah.

> Also, did we ever figure out what GCC did for a builtin here?  Do they have 
> one yet?  If not, have we encouraged them to implement this one?  If so, DID 
> they implement this one?

They have implemented both, it seems :)

See https://godbolt.org/z/G8eeMfsej


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 556488.
cjdb added a comment.

responds to feedback:

- removes diagnostic
- adds documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1091,6 +1091,197 @@
   https://wg21.link/p0195r2;>P0195R2
   Clang 4
 
+  Change scope of lambda trailing-return-type
+  https://wg21.link/P2036R3;>P2036R3
+  Clang 17
+
+
+  https://wg21.link/P2579R0;>P2579R0
+
+
+  Multidimensional subscript operator
+  https://wg21.link/P2128R6;>P2128R6
+  Clang 15
+
+
+  Non-literal variables (and labels and gotos) in constexpr functions
+  https://wg21.link/P2242R3;>P2242R3
+  Clang 15
+
+
+  Character encoding of diagnostic text
+  https://wg21.link/P2246R1;>P2246R1
+  Yes
+
+
+  Character sets and encodings
+  https://wg21.link/P2314R4;>P2314R4
+  Yes
+
+
+  Consistent character literal encoding
+  https://wg21.link/P2316R2;>P2316R2
+  Yes
+
+
+  Add support for preprocessing directives elifdef and elifndef
+  https://wg21.link/P2334R1;>P2334R1
+  Clang 13
+
+
+  Extend init-statement to allow alias-declaration
+  https://wg21.link/P2360R0;>P2360R0
+  Clang 14
+
+
+  auto(x): decay-copy in the language
+  https://wg21.link/P0849R8;>P0849R8
+  Clang 15
+
+
+
+  Attributes on Lambda-Expressions
+  https://wg21.link/P2173R1;>P2173R1
+  Clang 13
+
+
+  constexpr for cmath and cstdlib
+  https://wg21.link/P0533R9;>P0533R9
+  No
+
+
+  Type trait to determine if a reference binds to a temporary
+  https://wg21.link/P2255R2;>P2255R2
+  
+Partial
+  Clang provides __reference_constructs_from_temporary type
+  trait builtin, with which std::reference_constructs_from_temporary
+  implemented. __reference_converts_from_temporary needs to be
+  provided, following the normal cross-vendor convention to implement
+  traits requiring compiler support directly.
+
+  
+
+
+
+  The Equality Operator You Are Looking For
+  https://wg21.link/P2468R2;>P2468R2
+  Clang 16
+
+
+  De-deprecating volatile compound operations
+  https://wg21.link/P2327R1;>P2327R1
+  Clang 15
+
+
+  Support for #warning
+  https://wg21.link/P2437R1;>P2437R1
+  Yes
+
+
+  Remove non-encodable wide character literals and multicharacter wide character literals
+  https://wg21.link/P2362R3;>P2362R3
+  Clang 14
+
+
+  Labels at the end of compound statements
+  https://wg21.link/P2324R2;>P2324R2
+  Clang 16
+
+
+  Delimited escape sequences
+  https://wg21.link/P2290R3;>P2290R3
+  Clang 15
+
+
+  Named universal character escapes
+  https://wg21.link/P2071R2;>P2071R2
+  Clang 15
+
+
+  Relaxing some constexpr restrictions
+  https://wg21.link/P2448R2;>P2448R2
+  
+Clang 17 (Partial)
+	  We do not support outside of defaulted special memeber functions the change that constexpr functions no
+  longer have to be constexpr compatible but rather support a less restricted requirements for constexpr
+  functions. Which include allowing non-literal types as return values and paremeters, allow calling of
+  non-constexpr functions and constructors.
+
+  
+
+
+  Using unknown pointers and references in constant expressions
+  https://wg21.link/P2280R4;>P2280R4
+  No
+
+
+  static operator()
+  https://wg21.link/P1169R4;>P1169R4
+  Clang 16
+
+
+  Extended floating-point types and standard names
+  https://wg21.link/P1467R9;>P1467R9
+  No
+
+
+  Class template argument deduction from inherited constructors
+  https://wg21.link/P2582R1;>P2582R1
+  No
+
+
+  Portable assumptions
+  https://wg21.link/P1774R8;>P1774R8
+  No
+
+
+  Support for UTF-8 as a portable source file encoding
+  https://wg21.link/P2295R6;>P2295R6
+  Clang 15
+
+
+  char8_t Compatibility and Portability Fix
+  https://wg21.link/P2513R3;>P2513R3
+  Clang 16
+
+
+  Relax requirements on wchar_t to match existing practices
+  https://wg21.link/P2460R2;>P2460R2
+  Yes
+   

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D135341#4126101 , @erichkeane 
wrote:

> On the fence about the diagnostic at all, but definitely should not be doing 
> string magic to make it quoted.  Otherwise this is a LGTM.

Whoops, missed this comment. I'll be addressing your feedback and  merging 
later this week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Started a thread: 
https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D157572#4617504 , @aaron.ballman 
wrote:

> In D157572#4614561 , @cjdb wrote:
>
>> In D157572#4613622 , 
>> @aaron.ballman wrote:
>>
>>> In D157572#4612141 , @cjdb wrote:
>>>
 I don't dislike it, but I am a bit concerned about misuse being noisy.
>>>
>>> So you're concerned that a library author uses `diagnose_if` to add a 
>>> diagnostic to a warning group that makes the diagnostic seem too chatty, so 
>>> the user disables the group and loses the compiler's diagnostics? Or are 
>>> there other kinds of misuse you're worried about?
>>
>> More or less the former. I don't know if it'll actually manifest in practice 
>> though, I don't see many `diagnose_if` diagnostics to begin with.
>
> I think the former is sort of a "doctor, doctor, it hurts" situation; the 
> issue isn't really with `diagnose_if`, it's with the library author's use of 
> it. But at the same time, I can see why it would be beneficial to be able to 
> work around it if needed.
>
 As much as I hate suppressing diagnostics, I think there needs to be a way 
 to suppress the `diagnose_if` forms of warning without suppressing 
 something that the compiler would otherwise generate. Something like:

 - `-Wno-deprecated`: suppresses anything that `-Wdeprecated` would turn on.
 - `-Wno-deprecated=diagnose_if`: just the ones flagged by `diagnose_if`.
 - `-Wno-deprecated=non-diagnose_if`: complement to #2.

 (and similarly for `-Wno-error=`.)

 I'm not sure about the system header knob though: `[[deprecated]]` and 
 `[[nodiscard]]` still show up even when the declaration is in a system 
 header?
>>>
>>> Correct, those will still show up when the declaration is in a system 
>>> header but not when the use is in a system header: 
>>> https://godbolt.org/z/PjqKbGsrr
>>
>> Right, my question was more "what is this knob doing?"
>
> Ah! We have various knobs on our diagnostics, like:
> `ShowInSystemHeader` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L7818
>  (if used, the diagnostic will be shown in a system header)
> `SFINAEFailure` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L93
>  (if used, the diagnostic causes SFINAE to fail in a SFINAE context)
> `DefaultIgnore` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L141
>  (if used, the warning is off by default and must be explicitly enabled via 
> its group)
> `DefaultError` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L262
>  (if used, the warning defaults to being an error but users can disable the 
> error with `-Wno`)
> (etc)
>
> All of these have been of use to us as implementers, so it seems likely that 
> these same knobs would be of use to library authors adding their own compiler 
> diagnostics, so perhaps we should consider that as part of the design?
>
>>> We currently have `-Wuser-defined-warnings` as the warning group for 
>>> `diagnose_if` warning diagnostics, so I wonder if it would make sense to 
>>> allow `-Wno-deprecated` suppresses anything that `-Wdeprecated` would turn 
>>> on, while `-Wdeprecated -Wno-user-defined-warnings` would turn on only the 
>>> compiler-generated deprecation warnings and not the diagnose_if-generated 
>>> ones?
>>
>> Oh neat, this simplifies things a lot!
>
> I think it could make for a reasonable user experience.
>
> I'm curious what @erichkeane thinks as attributes code owner, but personally, 
> I like the idea of extending `diagnose_if` over the idea of adding 
> `clang::library_extension` because it's a more general solution that I think 
> will give more utility to our users. However, I also want to know if @philnik 
> @Mordante @ldionne (and others) share that preference because I think they're 
> going to be the guinea pigs^W^Wearly adopters of this functionality.

I like this idea, but we need to acknowledge that it'll potentially lead to 
diagnostics with varying quality. This conversation may be better suited to a 
Discourse thread at this point: there seems to be a //lot// of design before we 
reach consenus, and a CL isn't the optimal place to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D157572#4613622 , @aaron.ballman 
wrote:

> In D157572#4612141 , @cjdb wrote:
>
>> I don't dislike it, but I am a bit concerned about misuse being noisy.
>
> So you're concerned that a library author uses `diagnose_if` to add a 
> diagnostic to a warning group that makes the diagnostic seem too chatty, so 
> the user disables the group and loses the compiler's diagnostics? Or are 
> there other kinds of misuse you're worried about?

More or less the former. I don't know if it'll actually manifest in practice 
though, I don't see many `diagnose_if` diagnostics to begin with.

>> As much as I hate suppressing diagnostics, I think there needs to be a way 
>> to suppress the `diagnose_if` forms of warning without suppressing something 
>> that the compiler would otherwise generate. Something like:
>>
>> - `-Wno-deprecated`: suppresses anything that `-Wdeprecated` would turn on.
>> - `-Wno-deprecated=diagnose_if`: just the ones flagged by `diagnose_if`.
>> - `-Wno-deprecated=non-diagnose_if`: complement to #2.
>>
>> (and similarly for `-Wno-error=`.)
>>
>> I'm not sure about the system header knob though: `[[deprecated]]` and 
>> `[[nodiscard]]` still show up even when the declaration is in a system 
>> header?
>
> Correct, those will still show up when the declaration is in a system header 
> but not when the use is in a system header: https://godbolt.org/z/PjqKbGsrr

Right, my question was more "what is this knob doing?"

> We currently have `-Wuser-defined-warnings` as the warning group for 
> `diagnose_if` warning diagnostics, so I wonder if it would make sense to 
> allow `-Wno-deprecated` suppresses anything that `-Wdeprecated` would turn 
> on, while `-Wdeprecated -Wno-user-defined-warnings` would turn on only the 
> compiler-generated deprecation warnings and not the diagnose_if-generated 
> ones?

Oh neat, this simplifies things a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D157572#4606513 , @aaron.ballman 
wrote:

> In D157572#4604595 , @philnik wrote:
>
>> In D157572#4604482 , 
>> @aaron.ballman wrote:
>>
 This allows standard libraries to mark symbols as extensions, so the 
 compiler can generate extension warnings when they are used.
>>>
>>> Huh, so this is basically the opposite of the `__extension__` macro (which 
>>> is used to silence extension warnings)?
>>
>> I guess, kind-of. I never really understood the semantics of `__extension__` 
>> though, so I'm not 100% certain.
>
> It's used in system headers to say "I'm using an extension here, don't warn 
> about it in -pedantic mode".
>
>>> I don't think we need to introduce a new attribute to do this, we already 
>>> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that 
>>> suffice?
>>
>> Part of the idea here is that the diagnostics should be part of 
>> `-Wc++ab-extension`.
>
> Hmmm, okay. And I'm assuming `-Wsystem-headers -pedantic` is too chatty 
> because it's telling the user about all use of extensions, not extensions 
> being introduced by the library itself? (e.g., 
> https://godbolt.org/z/Gs3YGheMM is also not what you're after)
>
>> I guess we could allow warning flags instead of just `"warning"` and 
>> `"error"` in `diagnose_if` that specifies which warning group the diagnostic 
>> should be part of. Something like 
>> `__attribute__((__diagnose_if__(__cplusplus >= 201703L, "basic_string_view 
>> is a C++17 extension", "-Wc++17-extensions")))`. I'm not sure how one could 
>> implement that, but I guess there is some mechanism to translate 
>> "-Wwhatever" to a warning group, since you can push and pop warnings.  That 
>> would open people up to add a diagnostic to pretty much any warning group. I 
>> don't know if that's a good idea. I don't really see a problem with that 
>> other than people writing weird code, but people do that all the time 
>> anyways. Maybe I'm missing something really problematic though.
>
> That's actually a pretty interesting idea; `diagnose_if` could be given 
> another parameter to specify a diagnostic group to associate the diagnostic 
> with. This would let you do some really handy things like:
>
>   void func(int i) __attribute__((diagnose_if(i < 0, "passing a negative 
> value to 'func' is deprecated", "warning", "-Wdeprecated")));
>
> But if we went this route, would we want to expose other diagnostic-related 
> knobs like "show in system header" and "default to an error"? Also, the 
> attribute currently can only be associated with a function; we can use this 
> for classes by sticking it on a constructor but there's not much help for 
> putting it on say a namespace or an enumeration. So we may need to extend the 
> attribute in other ways. CC @cjdb as this seems of interest to you as well.

I don't dislike it, but I am a bit concerned about misuse being noisy. As much 
as I hate suppressing diagnostics, I think there needs to be a way to suppress 
the `diagnose_if` forms of warning without suppressing something that the 
compiler would otherwise generate. Something like:

- `-Wno-deprecated`: suppresses anything that `-Wdeprecated` would turn on.
- `-Wno-deprecated=diagnose_if`: just the ones flagged by `diagnose_if`.
- `-Wno-deprecated=non-diagnose_if`: complement to #2.

(and similarly for `-Wno-error=`.)

I'm not sure about the system header knob though: `[[deprecated]]` and 
`[[nodiscard]]` still show up even when the declaration is in a system header?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D151575: [clang][diagnostics] Always show include stacks on top-level diagnostics

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D151575#4607047 , @aaron.ballman 
wrote:

> I am wrestling with this one because I think the status quo is unfortunate 
> (we silently drop relevant information in ways the user may not immediately 
> understand) but I think always printing the include stack could be verbose 
> and thus make the diagnostics harder to act on. If we had the ability to 
> hyperlink to other parts of the diagnostic output, then I'd ask if we could 
> perhaps print the full stack once and link to it from anywhere else it's 
> being used, but I can't think of an effective way to do that purely in text. 
> We could also add an explicit option to let the user control the behavior, 
> but I'd prefer to avoid that if possible as the combination of modes makes 
> testing a challenge.
>
> Maybe we should consider adding a flag to set diagnostic verbosity level? 
> e.g., `-fdiagnostic-verbosity=terse|default|verbose` where `terse` never 
> prints notes or include stacks, `default` is what we do today, and `verbose` 
> always prints include stacks?

I'm not fond of `terse` unless each note is printed once: I think that'll get 
misused and lead to a lot of frustration (most notes are genuinely helpful). 
There's a lot of design space here, but I don't think any of it can be achieved 
until we have nested diagnostics, but that requires a complete rewrite of the 
diagnostics engine.

`default` and `verbose` might be interesting though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151575

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

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292
+  std::string NewFieldName =
+  PackedField->getName().str() + "@" + std::to_string(Arg);
+  PackedField->setDeclName((NewFieldName));

Does LLVM have some form of `absl::StrCat` that we can use instead of 
`operator+`?



Comment at: clang/lib/Sema/TreeTransform.h:4191-4218
+  // Transform unexpanded field name and create a new member 
expression.
+  DeclarationName ExpandedName = (
+  UnExpanedNameStr + "@" + std::to_string(Arg));
+  // Construct name info with new name and keep other members the same.
+  DeclarationNameInfo ExpandedNameInfo = DeclarationNameInfo(
+  ExpandedName, MemberExpr->getMemberNameInfo().getLoc(),
+  MemberExpr->getMemberNameInfo().getInfo());

It may be worth putting this into its own named function to help with 
readability.



Comment at: clang/test/CodeGenCXX/data_member_packs.cpp:73
+  // CHECK: i32 @_Z3sumIJiiEEDaDpT_(i32 noundef %ts, i32 noundef %ts1)
+  sum_pack2(s6);
+  // Check instantiation of sum(int, long, float, double)

This needs to be passed to one of the sum functions and checked that it's 
generating the correct code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158006

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

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp:38
 constexpr int fn5() {
-  if constexpr (__builtin_is_constant_evaluated()) // expected-warning 
{{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a 
manifestly constant-evaluated expression}}
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning 
{{'__builtin_is_constant_evaluated' will always evaluate to true in this 
context}}
 return 0;

This should also generate a fix-it hint, since we can automate the fix.



Comment at: clang/test/SemaCXX/warn-tautological-meta-constant.cpp:18
+  } else {
+if constexpr (std::is_constant_evaluated()) { // expected-warning {{always 
evaluate to true}}
+  return 0;

I'm not a fan of this diagnostic text: it doesn't offer insight into what's 
gone wrong or provide actionable feedback on how to fix the code.


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

https://reviews.llvm.org/D155064

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5690-5691
+def warn_unknown_ext : Warning<"Unknown extension kind: %0">;
+def warn_cxx11_ext : Warning<"%0 is a C++11 extension">,
+InGroup;
+def warn_cxx14_ext : Warning<"%0 is a C++14 extension">,

We shouldn't need this one, since Clang (almost) doesn't distinguish between 
C++03 and C++11. To my knowledge, C++03 doesn't even support attributes, so 
this would be a moot addition.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8227-8229
+  if (llvm::none_of(std::array{"C++11", "C++14", "C++17", "C++20", "C++23",
+   "C++26", "GNU"},
+[&](const char *K) { return K == Kind; })) {





Comment at: clang/test/SemaCXX/attr-library-extension.cpp:9-13
+#ifdef GNUAttr
+#define EXTENSION(name) __attribute__((library_extension(name)))
+#else
+#define EXTENSION(name) [[clang::library_extension(name)]]
+#endif

I don't think we need to test for both of these: using 
`[[clang::library_extension]]` directly should suffice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3270-3339
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std::optional NumArgumentsInExpansion =

In order to help improve the readability of this function, I suggest breaking 
the branches into two separate functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:5930-5931
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:

dblaikie wrote:
> Perhaps a few more words here would be good - quoting a WG21 proposal paper 
> that this code implements, similar to the standard quotations in nearby code. 
> (be good to mention the WG21 proposal paper/whatnot in the patch 
> description/commit message too)
I would suggest a link to the discourse discussion instead of [[ 
http://wg21.link/P1858 | P1858]], since that thread talks about the proposal 
and goes into a bit more detail than the [[ 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1858r2.html#member-packs
 | member packs ]] section of the proposal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156546

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

[PATCH] D155549: [clang] adds `conceptDecl` as an ASTMatcher

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG38e1c597033d: [clang] adds `conceptDecl` as an ASTMatcher 
(authored by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155549

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/AST/DeclTest.cpp

Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -12,9 +12,11 @@
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Mangle.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
@@ -140,6 +142,22 @@
   ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
 }
 
+TEST(Decl, ConceptDecl) {
+  llvm::StringRef Code(R"(
+template
+concept integral = __is_integral(T);
+  )");
+
+  auto AST = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++20"});
+  ASTContext  = AST->getASTContext();
+  SourceManager  = Ctx.getSourceManager();
+
+  const auto *Decl =
+  selectFirst("decl", match(conceptDecl().bind("decl"), Ctx));
+  ASSERT_TRUE(Decl != nullptr);
+  EXPECT_EQ(Decl->getName(), "integral");
+}
+
 TEST(Decl, EnumDeclRange) {
   llvm::Annotations Code(R"(
 typedef int Foo;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -172,6 +172,7 @@
   REGISTER_MATCHER(compoundLiteralExpr);
   REGISTER_MATCHER(compoundStmt);
   REGISTER_MATCHER(coawaitExpr);
+  REGISTER_MATCHER(conceptDecl);
   REGISTER_MATCHER(conditionalOperator);
   REGISTER_MATCHER(constantArrayType);
   REGISTER_MATCHER(constantExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -800,6 +800,7 @@
 const internal::VariadicDynCastAllOfMatcher cxxMethodDecl;
 const internal::VariadicDynCastAllOfMatcher
 cxxConversionDecl;
+const internal::VariadicDynCastAllOfMatcher conceptDecl;
 const internal::VariadicDynCastAllOfMatcher varDecl;
 const internal::VariadicDynCastAllOfMatcher fieldDecl;
 const internal::VariadicDynCastAllOfMatcher
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1334,6 +1334,16 @@
 extern const internal::VariadicDynCastAllOfMatcher
 cxxDeductionGuideDecl;
 
+/// Matches concept declarations.
+///
+/// Example matches integral
+/// \code
+///   template
+///   concept integral = std::is_integral_v;
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+conceptDecl;
+
 /// Matches variable declarations.
 ///
 /// Note: this does not match declarations of member variables, which are
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -262,6 +262,7 @@
 - Added ``__builtin_elementwise_nearbyint`` for floating point
   types. This allows access to ``llvm.nearbyint`` for arbitrary
   floating-point and vector of floating-point types.
+- Clang AST matcher now matches concept declarations with `conceptDecl`.
 
 New Compiler Flags
 --
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -682,6 +682,15 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclconceptDeclMatcherhttps://clang.llvm.org/doxygen/classclang_1_1ConceptDecl.html;>ConceptDecl...
+Matches concept declarations.
+
+Example matches integral
+  templatetypename T
+  concept integral = std::is_integral_vT;
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclcxxConstructorDeclMatcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDecl...
 Matches C++ constructor declarations.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155549: [clang] adds `conceptDecl` as an ASTMatcher

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 542706.
cjdb edited the summary of this revision.
cjdb added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

just trying to get CI close to green :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155549

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/AST/DeclTest.cpp

Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -12,9 +12,11 @@
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Mangle.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
@@ -140,6 +142,22 @@
   ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
 }
 
+TEST(Decl, ConceptDecl) {
+  llvm::StringRef Code(R"(
+template
+concept integral = __is_integral(T);
+  )");
+
+  auto AST = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++20"});
+  ASTContext  = AST->getASTContext();
+  SourceManager  = Ctx.getSourceManager();
+
+  const auto *Decl =
+  selectFirst("decl", match(conceptDecl().bind("decl"), Ctx));
+  ASSERT_TRUE(Decl != nullptr);
+  EXPECT_EQ(Decl->getName(), "integral");
+}
+
 TEST(Decl, EnumDeclRange) {
   llvm::Annotations Code(R"(
 typedef int Foo;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -172,6 +172,7 @@
   REGISTER_MATCHER(compoundLiteralExpr);
   REGISTER_MATCHER(compoundStmt);
   REGISTER_MATCHER(coawaitExpr);
+  REGISTER_MATCHER(conceptDecl);
   REGISTER_MATCHER(conditionalOperator);
   REGISTER_MATCHER(constantArrayType);
   REGISTER_MATCHER(constantExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -800,6 +800,7 @@
 const internal::VariadicDynCastAllOfMatcher cxxMethodDecl;
 const internal::VariadicDynCastAllOfMatcher
 cxxConversionDecl;
+const internal::VariadicDynCastAllOfMatcher conceptDecl;
 const internal::VariadicDynCastAllOfMatcher varDecl;
 const internal::VariadicDynCastAllOfMatcher fieldDecl;
 const internal::VariadicDynCastAllOfMatcher
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1334,6 +1334,16 @@
 extern const internal::VariadicDynCastAllOfMatcher
 cxxDeductionGuideDecl;
 
+/// Matches concept declarations.
+///
+/// Example matches integral
+/// \code
+///   template
+///   concept integral = std::is_integral_v;
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+conceptDecl;
+
 /// Matches variable declarations.
 ///
 /// Note: this does not match declarations of member variables, which are
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -262,6 +262,7 @@
 - Added ``__builtin_elementwise_nearbyint`` for floating point
   types. This allows access to ``llvm.nearbyint`` for arbitrary
   floating-point and vector of floating-point types.
+- Clang AST matcher now matches concept declarations with `conceptDecl`.
 
 New Compiler Flags
 --
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -682,6 +682,15 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclconceptDeclMatcherhttps://clang.llvm.org/doxygen/classclang_1_1ConceptDecl.html;>ConceptDecl...
+Matches concept declarations.
+
+Example matches integral
+  templatetypename T
+  concept integral = std::is_integral_vT;
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclcxxConstructorDeclMatcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXConstructorDecl.html;>CXXConstructorDecl...
 Matches C++ constructor declarations.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I think Tom's suggestion about using escapes and UCNs is great. I have no real 
opinion on whether the numeric values is the one that's parenthesised.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155610

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

[PATCH] D154366: [clang][ExprConstant] Print template arguments when describing stack frame

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb accepted this revision.
cjdb added a comment.

If there aren't any concerns from others, then LGTM!


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

https://reviews.llvm.org/D154366

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

[PATCH] D153623: [clang][Sema] Add fixit for scoped enum format error

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Sorry, forgot to come back and LGTM, but it did indeed LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153623

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

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

[Christopher Di Bella via Phabricator via cfe-commits]

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

Yep! Thanks for working on this :-)


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

https://reviews.llvm.org/D153359

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

[PATCH] D154366: [clang][ExprConstant] Print template arguments when describing stack frame

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a subscriber: dblaikie.
cjdb added a comment.

Generally in favour of this, but I wonder if we ought to have a way of 
suppressing it when the specialisation is known and ends up being a gargantuan 
name (this is a discussion, not an action item). cc @dblaikie


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154366

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

[PATCH] D154253: [clang] detect integer overflow through temporary values

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

> Fixes GH63629.

Would you mind changing this to `Fixes #63629` please? That'll get it connected 
in GitHub.

The patch looks good, would you mind adding some release notes before I give 
LGTM please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154253

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

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D153359#4463009 , @tbaeder wrote:

> @cjdb I know this is kind of silly after @hazohelet has already added it to 
> the release notes... but it seems like showing the difference is useless 
> since the difference was only introduced during the clang 17 development.(?)  
> For a 16 -> 17 transition, it probably causes almost no change.

Ah, fair enough then.


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

https://reviews.llvm.org/D153359

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

[PATCH] D153849: [clang][Diagnostics] Fix diagnostic line numbers

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

LGTM. Not sure if the `Fixes https://github.com/llvm/llvm-project/issues/63524` 
line autocloses the issue, but note that `Fixes #63524` does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153849

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

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Per this Discourse post 
,
 I think it'd be best for the example I asked to be in your commit message to 
actually be in your release notes. Sorry for the churn!


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

https://reviews.llvm.org/D153359

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

[PATCH] D153690: [clang][Sema] Remove dead diagnostic for loss of __unaligned qualifier

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb accepted this revision.
cjdb added a comment.

Excellent, thanks! LGTM, do you need assistance with merging?


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

https://reviews.llvm.org/D153690

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

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6589
+def err_using_placeholder_variable : Error<
+  "referring to placeholder '_' is not allowed">;
+def note_reference_placeholder : Note<

aaron.ballman wrote:
> I don't think this helps the user understand what's wrong with their code, 
> especially given the somewhat odd language rules around the feature. How 
> about: `ambiguous reference to multiply-defined placeholder '_'` or something 
> along those lines? Then the note can show the previous declarations of the 
> placeholders that are in scope? e.g.,
> ```
> void g() {
> int _; // note: placeholder declared here
> _ = 0;
> int _; // note: placeholder declared here
> _ = 0; // error: `ambiguous reference to multiply-defined placeholder '_'`
> }
> ```
> CC @cjdb 
Agreed. I'd suggest a rewording though: I took "multiply" to mean the maths 
term until completing the sentence, rather than its alternative meaning of 
"multiple instances" (which is more or less the same meaning, but "multiply" 
maps to the `x * y` operation for me).

Perhaps `ambiguous reference to placeholder '_', which has multiple 
definitions`? Not sold on that being the best wording, but it does avoid the 
hardcoded-word-at-8yo problem :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D153690: [clang][Sema] Remove dead diagnostic for loss of __unaligned qualifier

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Can we get a snippet of code to demonstrate what isn't being diagnosed anymore, 
please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153690

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

[PATCH] D152686: [clangd] Enforce strict unused includes by default

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

This patch seems to have broken clangd for a project of mine. Most of my 
headers are now flagged as not being directly used even though they're the root 
header for the symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152686

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

[PATCH] D153267: [clang][Diagnostics] Provide parameter source range to arity-mismatch notes

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb accepted this revision.
cjdb added a comment.

Thanks! This LGTM now. Do you need assistance with merging?


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

https://reviews.llvm.org/D153267

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

[PATCH] D153267: [clang][Diagnostics] Provide parameter source range to arity-mismatch notes

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I think this will be a good addition because it underscores the salient part of 
why the function isn't viable.

Would it be possible for patches like this to have both examples of before and 
after this patch in their commit message please? That'll help expose why 
patches like this are desirable. Happy to LGTM afterwards.




Comment at: clang/test/Misc/diag-func-call-ranges.cpp:13
+void arity_mismatch() {
+  (void)func(2, 4);
+}

Why are you casting to void?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153267

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

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I like this patch, thanks for working on it 

In D153359#4436873 , @hazohelet wrote:

> Consider the following code. (I added another parameter to the original code 
> so that the covered range becomes clearer)
>
>   void func(int aa, int bb);
>   
>   
>   void test() { func(1, "two"); }
>
> BEFORE this patch:
>
>   source:4:15: error: no matching function for call to 'func'
>   4 | void test() { func(1, "two"); }
> |   ^~~~
>   source:1:6: note: candidate function not viable: no known conversion from 
> 'const char[4]' to 'int' for 2nd argument
>   1 | void func(int aa, int bb);
> |  ^
>   2 |
>   3 |
>   4 | void test() { func(1, "two"); }
> |   ~
>
> AFTER this patch:
>
>   source:4:15: error: no matching function for call to 'func'
>   4 | void test() { func(1, "two"); }
> |   ^~~~
>   source:1:6: note: candidate function not viable: no known conversion from 
> 'const char[4]' to 'int' for 2nd argument
>   1 | void func(int aa, int bb);
> |  ^~~

Having this in the commit message would be great, thanks!




Comment at: clang/lib/Sema/SemaOverload.cpp:10821
 
+// FIXME: No test case for this. Can we remove this block?
 if (FromQs.hasUnaligned() != ToQs.hasUnaligned()) {

Please don't commit until this is resolved (either tests added or it's removed).


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

https://reviews.llvm.org/D153359

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

[PATCH] D151720: [clang][ExprConstant] Fix display of syntactically-invalid note for member function calls

[Christopher Di Bella via Phabricator via cfe-commits]

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

This patch seems like a win to me.




Comment at: clang/docs/ReleaseNotes.rst:325
+  invalid code. Furthermore, it also displays the object as written by the 
user.
+  (`#57081: `_).
 

I like that we're referencing bugs in our release notes now :)


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

https://reviews.llvm.org/D151720

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

[PATCH] D152525: [clang][Diagnostics] Don't expand label fixit to the next line

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb accepted this revision.
cjdb added a comment.

Thanks for fixing this, it'll hopefully cause less frustration for devs :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152525

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

[PATCH] D151833: Respect "-fdiagnostics-absolute-paths" on emit include location

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG91be60b34715: Respect 
-fdiagnostics-absolute-paths on emit include location (authored by 
charmitro, committed by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151833

Files:
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Frontend/absolute-paths-import.h
  clang/test/Frontend/absolute-paths.c


Index: clang/test/Frontend/absolute-paths.c
===
--- clang/test/Frontend/absolute-paths.c
+++ clang/test/Frontend/absolute-paths.c
@@ -1,5 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | 
FileCheck -check-prefix=NORMAL -check-prefix=CHECK %s
-// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. 
-fdiagnostics-absolute-paths %s 2>&1 | FileCheck -check-prefix=ABSOLUTE 
-check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | 
FileCheck -DROOT_ABSOLUTE=%s -check-prefix=NORMAL -check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. 
-fdiagnostics-absolute-paths %s 2>&1 | FileCheck -DROOT_ABSOLUTE=%s 
-check-prefix=ABSOLUTE -check-prefix=CHECK %s
+
+#include "absolute-paths-import.h"
+// NORMAL: In file included from {{.*}}absolute-paths.c:4:
+// NORMAL-NOT: In file included from [[ROOT_ABSOLUTE]]:4:
+// ABSOLUTE: In file included from [[ROOT_ABSOLUTE]]:4:
 
 #include "absolute-paths.h"
 
Index: clang/test/Frontend/absolute-paths-import.h
===
--- /dev/null
+++ clang/test/Frontend/absolute-paths-import.h
@@ -0,0 +1 @@
+#warning abc
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -868,10 +868,11 @@
 }
 
 void TextDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) {
-  if (DiagOpts->ShowLocation && PLoc.isValid())
-OS << "In file included from " << PLoc.getFilename() << ':'
-   << PLoc.getLine() << ":\n";
-  else
+  if (DiagOpts->ShowLocation && PLoc.isValid()) {
+OS << "In file included from ";
+emitFilename(PLoc.getFilename(), Loc.getManager());
+OS << ':' << PLoc.getLine() << ":\n";
+  } else
 OS << "In included file:\n";
 }
 


Index: clang/test/Frontend/absolute-paths.c
===
--- clang/test/Frontend/absolute-paths.c
+++ clang/test/Frontend/absolute-paths.c
@@ -1,5 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | FileCheck -check-prefix=NORMAL -check-prefix=CHECK %s
-// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. -fdiagnostics-absolute-paths %s 2>&1 | FileCheck -check-prefix=ABSOLUTE -check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | FileCheck -DROOT_ABSOLUTE=%s -check-prefix=NORMAL -check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. -fdiagnostics-absolute-paths %s 2>&1 | FileCheck -DROOT_ABSOLUTE=%s -check-prefix=ABSOLUTE -check-prefix=CHECK %s
+
+#include "absolute-paths-import.h"
+// NORMAL: In file included from {{.*}}absolute-paths.c:4:
+// NORMAL-NOT: In file included from [[ROOT_ABSOLUTE]]:4:
+// ABSOLUTE: In file included from [[ROOT_ABSOLUTE]]:4:
 
 #include "absolute-paths.h"
 
Index: clang/test/Frontend/absolute-paths-import.h
===
--- /dev/null
+++ clang/test/Frontend/absolute-paths-import.h
@@ -0,0 +1 @@
+#warning abc
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -868,10 +868,11 @@
 }
 
 void TextDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) {
-  if (DiagOpts->ShowLocation && PLoc.isValid())
-OS << "In file included from " << PLoc.getFilename() << ':'
-   << PLoc.getLine() << ":\n";
-  else
+  if (DiagOpts->ShowLocation && PLoc.isValid()) {
+OS << "In file included from ";
+emitFilename(PLoc.getFilename(), Loc.getManager());
+OS << ':' << PLoc.getLine() << ":\n";
+  } else
 OS << "In included file:\n";
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152034: [clang][NFC] refactors value type traits so we can have more than bools

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG798c5ba770d3: [clang][NFC] refactors value type traits so we 
can have more than bools (authored by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152034

Files:
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5378,9 +5378,14 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
-static bool evaluateTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
-  ArrayRef Args,
-  SourceLocation RParenLoc) {
+static bool EvaluateBooleanTypeTrait(Sema , TypeTrait Kind,
+ SourceLocation KWLoc,
+ ArrayRef Args,
+ SourceLocation RParenLoc,
+ bool IsDependent) {
+  if (IsDependent)
+return false;
+
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
@@ -5548,12 +5553,19 @@
   return true;
 }
 
+enum class TypeTraitReturnType {
+  Bool,
+};
+
+static TypeTraitReturnType GetReturnType(TypeTrait Kind) {
+  return TypeTraitReturnType::Bool;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
   if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
 return ExprError();
-  QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
*this, Kind, KWLoc, Args[0]->getType()))
@@ -5569,12 +5581,17 @@
 }
   }
 
-  bool Result = false;
-  if (!Dependent)
-Result = evaluateTypeTrait(*this, Kind, KWLoc, Args, RParenLoc);
-
-  return TypeTraitExpr::Create(Context, ResultType, KWLoc, Kind, Args,
-   RParenLoc, Result);
+  switch (GetReturnType(Kind)) {
+  case TypeTraitReturnType::Bool: {
+bool Result = EvaluateBooleanTypeTrait(*this, Kind, KWLoc, Args, RParenLoc,
+   Dependent);
+return TypeTraitExpr::Create(Context, Context.getLogicalOperationType(),
+ KWLoc, Kind, Args, RParenLoc, Result);
+  }
+  default:
+llvm_unreachable("reached the end of BuildTypeTrait because the type "
+ "trait's type is unaccounted for");
+  }
 }
 
 ExprResult Sema::ActOnTypeTrait(TypeTrait Kind, SourceLocation KWLoc,


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5378,9 +5378,14 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
-static bool evaluateTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
-  ArrayRef Args,
-  SourceLocation RParenLoc) {
+static bool EvaluateBooleanTypeTrait(Sema , TypeTrait Kind,
+ SourceLocation KWLoc,
+ ArrayRef Args,
+ SourceLocation RParenLoc,
+ bool IsDependent) {
+  if (IsDependent)
+return false;
+
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
@@ -5548,12 +5553,19 @@
   return true;
 }
 
+enum class TypeTraitReturnType {
+  Bool,
+};
+
+static TypeTraitReturnType GetReturnType(TypeTrait Kind) {
+  return TypeTraitReturnType::Bool;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
   if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
 return ExprError();
-  QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
*this, Kind, KWLoc, Args[0]->getType()))
@@ -5569,12 +5581,17 @@
 }
   }
 
-  bool Result = false;
-  if (!Dependent)
-Result = evaluateTypeTrait(*this, Kind, KWLoc, Args, RParenLoc);
-
-  return TypeTraitExpr::Create(Context, ResultType, KWLoc, Kind, Args,
-   RParenLoc, Result);
+  switch (GetReturnType(Kind)) {
+  case TypeTraitReturnType::Bool: {
+bool Result = EvaluateBooleanTypeTrait(*this, Kind, KWLoc, Args, RParenLoc,
+  

[PATCH] D151833: Respect "-fdiagnostics-absolute-paths" on emit include location

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I already have another commit I plan to push, will do these together :)


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

https://reviews.llvm.org/D151833

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

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D151952#4392329 , @rsmith wrote:

> Taking a step back for a moment: what is the intended use case for this? My 
> concern is that most of the time you're going to want to make O(N) such 
> queries into a pack of N elements, resulting in O(N^2) compile time, much 
> like we regrettably get from uses of `__type_pack_element`. If we can avoid 
> the regret in this case by providing a different intrinsic, perhaps we should 
> do so. (For example, we could take two packs of types, and produce a sequence 
> of integers giving the indexes of each of the first types in the second list, 
> in O(N) time. And similarly we could add a `__type_pack_elements` that takes 
> a pack of types and a pack of indexes and performs N indexing operations at 
> once, in O(N) time, rather than having the program do it in O(N^2) time.)

When I wrote this I narrowly had `std::get(tuple)` and 
`std::get(variant)` in mind, both of which are linear in nature. Now that 
you raise this, I can see the described problem and will implement said feature 
(maybe there exists a world where `std::apply` can use this too?). Since we're 
doing this, it may also be worth checking that `T1s...` are unique in `T2s...`, 
which I'd intended to be a separate feature.

Can we get away with modifying `__type_pack_element` so that we don't need to 
introduce a new keyword, or should we have two for each?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952

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

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D147717#4395204 , @Fznamznon wrote:

> In D147717#4393019 , @cor3ntin 
> wrote:
>
>> I think we should make sure to land this for clang 17. The rate of consteval 
>> bugs is no greater than that of any other feature at this point.
>
> There is https://github.com/llvm/llvm-project/issues/62886 which seems quite 
> ugly, but has a patch on review.
> Otherwise I agree, I'm not seeing more consteval bugs than any others.
> @aaron.ballman , @erichkeane wdyt about landing it?

The worst-case scenario is that we have to disable the status again. I can't 
give an estimate on when I'll finish, so this is fine to me based on available 
data.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147717

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

[PATCH] D151833: Respect "-fdiagnostics-absolute-paths" on emit include location

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D151833#4396399 , @charmitro wrote:

> In D151833#4396244 , @cjdb wrote:
>
>> Can the commit message have a description please? It's unclear to me why 
>> this is necessary (although I'm sure there's a good reason).
>
> Thanks! Done, also check https://github.com/llvm/llvm-project/issues/63026

Nice, thanks! Would you mind adding `Fixes #63026.` somewhere in the commit 
message? That will auto-close the issue upon merge.


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

https://reviews.llvm.org/D151833

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

[PATCH] D151575: [clang][diagnostics] Always show include stacks on errors

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added subscribers: shafik, erichkeane, aaron.ballman.
cjdb added a comment.

@aaron.ballman @erichkeane @shafik any reservations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151575

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

[PATCH] D151833: Respect "-fdiagnostics-absolute-paths" on emit include location

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Can the commit message have a description please? It's unclear to me why this 
is necessary (although I'm sure there's a good reason).


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

https://reviews.llvm.org/D151833

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

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5620
 
+  if (Kind == clang::TT_TypePackIndex)
+return EvaluateIntegralTypeTrait(*this, Kind, KWLoc, Args, RParenLoc,

erichkeane wrote:
> I realize this is the 1st, but this seems like it is going to be a 
> maintenance pain.  Can you at least split this into a static-function of 
> `IsIntegralTypeTrait` that we can use later with table-gen if this gets out 
> of hand?
Done, but in D152034. It's a change that should happen independently of this 
one IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952

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

[PATCH] D152034: [clang][NFC] refactors value type traits so we can have more than bools

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: erichkeane, dblaikie.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Since all the type traits up until now have had Boolean vaules, we've
always been able to assume that the expressions are `bool`. This is
about to change (D151952  introduces a trait 
that returns `size_t`), so
we need to restructure the code so it doesn't become unwieldy.

This is achieved by giving traits a designated "return" type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152034

Files:
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5378,9 +5378,14 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
-static bool evaluateTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
-  ArrayRef Args,
-  SourceLocation RParenLoc) {
+static bool EvaluateBooleanTypeTrait(Sema , TypeTrait Kind,
+ SourceLocation KWLoc,
+ ArrayRef Args,
+ SourceLocation RParenLoc,
+ bool IsDependent) {
+  if (IsDependent)
+return false;
+
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
@@ -5548,12 +5553,19 @@
   return true;
 }
 
+enum class TypeTraitReturnType {
+  Bool,
+};
+
+static TypeTraitReturnType GetReturnType(TypeTrait Kind) {
+  return TypeTraitReturnType::Bool;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
   if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
 return ExprError();
-  QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
*this, Kind, KWLoc, Args[0]->getType()))
@@ -5569,12 +5581,17 @@
 }
   }
 
-  bool Result = false;
-  if (!Dependent)
-Result = evaluateTypeTrait(*this, Kind, KWLoc, Args, RParenLoc);
-
-  return TypeTraitExpr::Create(Context, ResultType, KWLoc, Kind, Args,
-   RParenLoc, Result);
+  switch (GetReturnType(Kind)) {
+  case TypeTraitReturnType::Bool: {
+bool Result = EvaluateBooleanTypeTrait(*this, Kind, KWLoc, Args, RParenLoc,
+   Dependent);
+return TypeTraitExpr::Create(Context, Context.getLogicalOperationType(),
+ KWLoc, Kind, Args, RParenLoc, Result);
+  }
+  default:
+llvm_unreachable("reached the end of BuildTypeTrait because the type "
+ "trait's type is unaccounted for");
+  }
 }
 
 ExprResult Sema::ActOnTypeTrait(TypeTrait Kind, SourceLocation KWLoc,


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5378,9 +5378,14 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
-static bool evaluateTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
-  ArrayRef Args,
-  SourceLocation RParenLoc) {
+static bool EvaluateBooleanTypeTrait(Sema , TypeTrait Kind,
+ SourceLocation KWLoc,
+ ArrayRef Args,
+ SourceLocation RParenLoc,
+ bool IsDependent) {
+  if (IsDependent)
+return false;
+
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
@@ -5548,12 +5553,19 @@
   return true;
 }
 
+enum class TypeTraitReturnType {
+  Bool,
+};
+
+static TypeTraitReturnType GetReturnType(TypeTrait Kind) {
+  return TypeTraitReturnType::Bool;
+}
+
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
   if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
 return ExprError();
-  QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
*this, Kind, KWLoc, Args[0]->getType()))
@@ -5569,12 +5581,17 @@
 }
   }
 
-  bool Result = false;
-  if (!Dependent)
-Result = evaluateTypeTrait(*this, Kind, KWLoc, Args, 

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:796-802
+/// If this expression is not value-dependent, this stores the value.
+unsigned Value : 8;
 
 /// The number of arguments to this type trait. According to [implimits]
 /// 8 bits would be enough, but we require (and test for) at least 16 bits
 /// to mirror FunctionType.
+unsigned NumArgs : 16;

erichkeane wrote:
> dblaikie wrote:
> > cjdb wrote:
> > > These numbers feel very low for how this modification is using them. 
> > > Perhaps they should both be bumped to 32?
> > clang does, I think, have specific implementation limits for this sort of 
> > thing - 
> > 
> > Well, maybe we don't actually enforce a limit on number of template 
> > arguments... https://godbolt.org/z/accncYson compiles successfully, and has 
> > 2^18 parameters, beyond the 2^16 suggested here.
> > 
> > But maybe 2^16 is just what we test for/somewhat guarantee.
> > 
> > But if the `Value` is going to be the index into the args, shouldn't it be 
> > the same size as `NumArgs`, at least? (& the comment says 16, so 16 for 
> > both Value and NumArgs would seem suitably symmetric, and no larger than 
> > the current situation - since it'd just be splitting NumArgs in half, while 
> > still meeting the comment's suggestion/requirements?)
> > 
> > An assert, at least, when populating NumArgs to ensure it's no larger might 
> > not hurt... (which might be reachable from code/not checked prior - so it 
> > could be a hard compilation error, I guess, but I wouldn't feel super 
> > strongly about it)
> Can you explain the math here of how you chose to change this to 16?  I see 
> that you removed 7 bits, but took away 16.  I'm not sure what NumExprBits is 
> doing though, I got lost a little in that, so if you can calculate that to 
> make sure this needs to be done, it would be appreciated.
> 
> Additionally, a static_assert after this to ensure the size isn't changing 
> would also be appreciated.
> 
> But if the Value is going to be the index into the args, shouldn't it be the 
> same size as NumArgs, at least? (& the comment says 16, so 16 for both Value 
> and NumArgs would seem suitably symmetric, and no larger than the current 
> situation - since it'd just be splitting NumArgs in half, while still meeting 
> the comment's suggestion/requirements?)

Someone independently confirmed that you can have 200k types in a template, so 
we should probably be able to count at least that high (or alternatively, we 
should possibly consider not allowing more than 2^16 template parameters).

> Can you explain the math here of how you chose to change this to 16? I see 
> that you removed 7 bits, but took away 16. I'm not sure what NumExprBits is 
> doing though, I got lost a little in that, so if you can calculate that to 
> make sure this needs to be done, it would be appreciated.

The value of `NumArgs` hasn't changed: I've just codified it. I can't work out 
why we need `NumExprBits`, but it's apparently required padding (when I removed 
it, a bunch of tests failed). Its value is 18, courtesy of clangd in VS Code.

> Additionally, a static_assert after this to ensure the size isn't changing 
> would also be appreciated.

There's a static_assert in one of the `Stmt` constructors, which doesn't want 
more than eight bytes (we apparently need 16 if we're to change this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952

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

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:796-802
+/// If this expression is not value-dependent, this stores the value.
+unsigned Value : 8;
 
 /// The number of arguments to this type trait. According to [implimits]
 /// 8 bits would be enough, but we require (and test for) at least 16 bits
 /// to mirror FunctionType.
+unsigned NumArgs : 16;

These numbers feel very low for how this modification is using them. Perhaps 
they should both be bumped to 32?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952

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

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 527681.
cjdb added a comment.

fixes some code I wrote when debugging


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type_pack_index.cpp

Index: clang/test/SemaCXX/type_pack_index.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type_pack_index.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+using size_t = decltype(sizeof(0));
+
+static_assert(__type_pack_index(int, int) == 0, "");
+static_assert(__type_pack_index(long, long, int) == 0, "");
+static_assert(__type_pack_index(int, long, int) == 1, "");
+
+// This will be used by parameter packs most of the time, so we should check they
+// work too.
+template
+constexpr size_t mock_get()
+{
+  return __type_pack_index(T, Ts...);
+}
+
+static_assert(mock_get() == 0, "");
+static_assert(mock_get() == 1, "");
+static_assert(mock_get() == 2, "");
+
+// Types mentioned twice return the first occurrence
+// -
+enum E {};
+enum class EC : int {};
+struct S;
+union U;
+static_assert(__type_pack_index(S, U, S, S) == 1, "");
+static_assert(__type_pack_index(U, int, long, EC, U, double, __int128, S, EC, U) == 3, "");
+
+template
+union UT;
+static_assert(__type_pack_index(UT, int, long, EC, U, double, __int128, S, EC, U, UT, UT, UT) == 10, "");
+
+// Qualifiers
+// --
+static_assert(__type_pack_index(int const, int const, int) == 0, "");
+static_assert(__type_pack_index(int, int const, int) == 1, "");
+
+static_assert(__type_pack_index(int volatile, int volatile, int) == 0, "");
+static_assert(__type_pack_index(int, int volatile, int) == 1, "");
+
+static_assert(__type_pack_index(int const volatile, int const volatile, int) == 0, "");
+static_assert(__type_pack_index(int, int const volatile, int) == 1, "");
+
+static_assert(__type_pack_index(int&, int const, int, int&) == 2, "");
+static_assert(__type_pack_index(int const&, int const, int const&) == 1, "");
+static_assert(__type_pack_index(int volatile&, int volatile, int const&, int&, int volatile&) == 3, "");
+static_assert(__type_pack_index(int const volatile&, int volatile, int const&, int&, int volatile&, int const volatile&) == 4, "");
+
+static_assert(__type_pack_index(int&&, int const, int, int&, int&&, int const&&) == 3, "");
+static_assert(__type_pack_index(int const&&, int const, int const&, int const&&) == 2, "");
+static_assert(__type_pack_index(int volatile&&, int volatile, int const&, int&, int volatile&, int volatile&&) == 4, "");
+static_assert(__type_pack_index(int const volatile&&, int volatile, int const&, int&, int volatile&, int volatile&&, int const volatile&&) == 5, "");
+
+static_assert(__type_pack_index(int* __restrict, int*, int * __restrict) == 1, "");
+
+// Aliases
+// ---
+using Int = int;
+typedef long Long;
+static_assert(__type_pack_index(int, Int, Long) == 0, "");
+static_assert(__type_pack_index(Int, Int, Long) == 0, "");
+static_assert(__type_pack_index(long, Int, Long) == 1, "");
+static_assert(__type_pack_index(Long, Int, Long) == 1, "");
+
+// Error handling
+// --
+//
+// Not enough arguments
+// 
+static_assert(__type_pack_index(), "");// expected-error{{expected a type}}
+static_assert(__type_pack_index(int), ""); // expected-error{{type trait requires 2 or more arguments; have 1 argument}}
+
+// Type not found
+// --
+static_assert(__type_pack_index(int, long long, double));
+// expected-error@-1{{'__type_pack_index' couldn't find type 'int'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5378,9 +5378,9 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
-static bool evaluateTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
-  ArrayRef Args,
-  SourceLocation RParenLoc) {
+static bool EvaluateBooleanTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
+ ArrayRef Args,
+ SourceLocation RParenLoc) {
   if (Kind <= UTT_Last)
 return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
@@ -5548,6 +5548,54 @@
   return true;
 }
 
+static ExprResult EvaluateIntegralTypeTrait(Sema , TypeTrait Kind,
+   

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie.
Herald added subscribers: arphaman, martong.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Similarly to `__type_pack_element`, C++ doesn't offer a way to extract a
the index of the first occurrence of a type in a parameter pack. This
means that we need to build up context for something that the compiler
already has, and then compute the value in a much less efficient manner
than the  compiler.

Clang can compute this information without needing to use extra
resources.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151952

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type_pack_index.cpp

Index: clang/test/SemaCXX/type_pack_index.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type_pack_index.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+using size_t = decltype(sizeof(0));
+
+static_assert(__type_pack_index(int, int) == 0, "");
+static_assert(__type_pack_index(long, long, int) == 0, "");
+static_assert(__type_pack_index(int, long, int) == 1, "");
+
+// This will be used by parameter packs most of the time, so we should check they
+// work too.
+template
+constexpr size_t mock_get()
+{
+  return __type_pack_index(T, Ts...);
+}
+
+static_assert(mock_get() == 0, "");
+static_assert(mock_get() == 1, "");
+static_assert(mock_get() == 2, "");
+
+// Types mentioned twice return the first occurrence
+// -
+enum E {};
+enum class EC : int {};
+struct S;
+union U;
+static_assert(__type_pack_index(S, U, S, S) == 1, "");
+static_assert(__type_pack_index(U, int, long, EC, U, double, __int128, S, EC, U) == 3, "");
+
+template
+union UT;
+static_assert(__type_pack_index(UT, int, long, EC, U, double, __int128, S, EC, U, UT, UT, UT) == 10, "");
+
+// Qualifiers
+// --
+static_assert(__type_pack_index(int const, int const, int) == 0, "");
+static_assert(__type_pack_index(int, int const, int) == 1, "");
+
+static_assert(__type_pack_index(int volatile, int volatile, int) == 0, "");
+static_assert(__type_pack_index(int, int volatile, int) == 1, "");
+
+static_assert(__type_pack_index(int const volatile, int const volatile, int) == 0, "");
+static_assert(__type_pack_index(int, int const volatile, int) == 1, "");
+
+static_assert(__type_pack_index(int&, int const, int, int&) == 2, "");
+static_assert(__type_pack_index(int const&, int const, int const&) == 1, "");
+static_assert(__type_pack_index(int volatile&, int volatile, int const&, int&, int volatile&) == 3, "");
+static_assert(__type_pack_index(int const volatile&, int volatile, int const&, int&, int volatile&, int const volatile&) == 4, "");
+
+static_assert(__type_pack_index(int&&, int const, int, int&, int&&, int const&&) == 3, "");
+static_assert(__type_pack_index(int const&&, int const, int const&, int const&&) == 2, "");
+static_assert(__type_pack_index(int volatile&&, int volatile, int const&, int&, int volatile&, int volatile&&) == 4, "");
+static_assert(__type_pack_index(int const volatile&&, int volatile, int const&, int&, int volatile&, int volatile&&, int const volatile&&) == 5, "");
+
+static_assert(__type_pack_index(int* __restrict, int*, int * __restrict) == 1, "");
+
+// Aliases
+// ---
+using Int = int;
+typedef long Long;
+static_assert(__type_pack_index(int, Int, Long) == 0, "");
+static_assert(__type_pack_index(Int, Int, Long) == 0, "");
+static_assert(__type_pack_index(long, Int, Long) == 1, "");
+static_assert(__type_pack_index(Long, Int, Long) == 1, "");
+
+// Error handling
+// --
+//
+// Not enough arguments
+// 
+static_assert(__type_pack_index(), "");// expected-error{{expected a type}}
+static_assert(__type_pack_index(int), ""); // expected-error{{type trait requires 2 or more arguments; have 1 argument}}
+
+// Type not found
+// --
+static_assert(__type_pack_index(int, long long, double));
+// expected-error@-1{{'__type_pack_index' couldn't find type 'int'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5378,9 +5378,10 @@
 static bool EvaluateBinaryTypeTrait(Sema , TypeTrait BTT, QualType LhsT,
 QualType RhsT, SourceLocation KeyLoc);
 
-static bool evaluateTypeTrait(Sema , TypeTrait Kind, SourceLocation KWLoc,
- 

[PATCH] D151575: [clang][diagnostics] Always show include stacks on errors

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Thanks for working on this. Would you mind adding more context to the commit 
description please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151575

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

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

[Christopher Di Bella via Phabricator via cfe-commits]

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

Thanks for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151162

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

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D147717#4312066 , @Fznamznon wrote:

> In D147717#4275410 , @cjdb wrote:
>
>> I'm gonna get started on this today!
>
> @cjdb , how is this going? I've seen 
> https://github.com/llvm/llvm-project/issues/62224, but no more issues with 
> consteval label.

I haven't had time to invest in this recently, as I've been gearing up for an 
intern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147717

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

[PATCH] D149492: [clang] makes built-in traits match their stdlib counterparts' names

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Looks like I need to special-case `__is_null_pointer` as a function for 
libstdc++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149492

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

[PATCH] D149618: [clang][NFC] documents recent type trait primitives

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: aaron.ballman, erichkeane, shafik.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Depends on D149492 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149618

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1398,6 +1398,9 @@
 ``__X(...)`` has the same semantics and constraints as the corresponding
 ``std::X_t<...>`` or ``std::X_v<...>`` type trait.
 
+* ``__add_lvalue_reference(type)`` (C++)
+* ``__add_pointer(type)`` (C++)
+* ``__add_rvalue_reference(type)`` (C++)
 * ``__array_rank(type)`` (Embarcadero):
   Returns the number of levels of array in the type ``type``:
   ``0`` if ``type`` is not an array type, and
@@ -1409,6 +1412,7 @@
   Returns whether a class can be passed in registers under the current
   ABI. This type can only be applied to unqualified class types.
   This is not a portable type trait.
+* ``__decay(type)`` (C++)
 * ``__has_nothrow_assign`` (GNU, Microsoft, Embarcadero):
   Deprecated, use ``__is_nothrow_assignable`` instead.
 * ``__has_nothrow_move_assign`` (GNU, Microsoft):
@@ -1517,12 +1521,23 @@
   enumeration types if the underlying type was unsigned.
 * ``__is_void`` (C++, Embarcadero)
 * ``__is_volatile`` (C++, Embarcadero)
+* ``__make_signed(type)`` (C++)
+* ``__make_unsigned(type)`` (C++)
 * ``__reference_binds_to_temporary(T, U)`` (Clang):  Determines whether a
   reference of type ``T`` bound to an expression of type ``U`` would bind to a
   materialized temporary object. If ``T`` is not a reference type the result
   is false. Note this trait will also return false when the initialization of
   ``T`` from ``U`` is ill-formed.
-* ``__underlying_type`` (C++, GNU, Microsoft)
+* ``__remove_all_extents(type)`` (C++)
+* ``__remove_const(type)`` (C++)
+* ``__remove_cv(type)`` (C++)
+* ``__remove_cvref(type)`` (C++)
+* ``__remove_extent(type)`` (C++)
+* ``__remove_pointer(type)``(C++)
+* ``__remove_reference(type)`` (C++)
+* ``__remove_restrict(type)`` (C++)
+* ``__remove_volatile(type)`` (C++)
+* ``__underlying_type(type)`` (C++, GNU, Microsoft)
 
 In addition, the following expression traits are supported:
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1398,6 +1398,9 @@
 ``__X(...)`` has the same semantics and constraints as the corresponding
 ``std::X_t<...>`` or ``std::X_v<...>`` type trait.
 
+* ``__add_lvalue_reference(type)`` (C++)
+* ``__add_pointer(type)`` (C++)
+* ``__add_rvalue_reference(type)`` (C++)
 * ``__array_rank(type)`` (Embarcadero):
   Returns the number of levels of array in the type ``type``:
   ``0`` if ``type`` is not an array type, and
@@ -1409,6 +1412,7 @@
   Returns whether a class can be passed in registers under the current
   ABI. This type can only be applied to unqualified class types.
   This is not a portable type trait.
+* ``__decay(type)`` (C++)
 * ``__has_nothrow_assign`` (GNU, Microsoft, Embarcadero):
   Deprecated, use ``__is_nothrow_assignable`` instead.
 * ``__has_nothrow_move_assign`` (GNU, Microsoft):
@@ -1517,12 +1521,23 @@
   enumeration types if the underlying type was unsigned.
 * ``__is_void`` (C++, Embarcadero)
 * ``__is_volatile`` (C++, Embarcadero)
+* ``__make_signed(type)`` (C++)
+* ``__make_unsigned(type)`` (C++)
 * ``__reference_binds_to_temporary(T, U)`` (Clang):  Determines whether a
   reference of type ``T`` bound to an expression of type ``U`` would bind to a
   materialized temporary object. If ``T`` is not a reference type the result
   is false. Note this trait will also return false when the initialization of
   ``T`` from ``U`` is ill-formed.
-* ``__underlying_type`` (C++, GNU, Microsoft)
+* ``__remove_all_extents(type)`` (C++)
+* ``__remove_const(type)`` (C++)
+* ``__remove_cv(type)`` (C++)
+* ``__remove_cvref(type)`` (C++)
+* ``__remove_extent(type)`` (C++)
+* ``__remove_pointer(type)``(C++)
+* ``__remove_reference(type)`` (C++)
+* ``__remove_restrict(type)`` (C++)
+* ``__remove_volatile(type)`` (C++)
+* ``__underlying_type(type)`` (C++, GNU, Microsoft)
 
 In addition, the following expression traits are supported:
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129951: adds `__disable_adl` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Ping (moving my pings from Thursday afternoon to Monday mornings)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D149492: [clang] makes built-in traits match their stdlib counterparts' names

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: rsmith, aaron.ballman.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The original names apparently broke convention by not making them the
same as the C++ type trait names, so we're creating aliases for them.
It's unclear whether or not the existing names can be deprecated and
removed in the future.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149492

Files:
  clang/include/clang/Basic/TokenKinds.def
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -1073,6 +1073,38 @@
   static_assert(!__is_nullptr(StructWithMembers), "");
   static_assert(!__is_nullptr(int StructWithMembers::*), "");
   static_assert(!__is_nullptr(void(StructWithMembers::*)()), "");
+
+  static_assert(__is_null_pointer(decltype(nullptr)), "");
+  static_assert(!__is_null_pointer(void *), "");
+  static_assert(!__is_null_pointer(cvoid *), "");
+  static_assert(!__is_null_pointer(cvoid *), "");
+  static_assert(!__is_null_pointer(char *), "");
+  static_assert(!__is_null_pointer(int *), "");
+  static_assert(!__is_null_pointer(int **), "");
+  static_assert(!__is_null_pointer(ClassType *), "");
+  static_assert(!__is_null_pointer(Derives *), "");
+  static_assert(!__is_null_pointer(Enum *), "");
+  static_assert(!__is_null_pointer(IntArNB *), "");
+  static_assert(!__is_null_pointer(Union *), "");
+  static_assert(!__is_null_pointer(UnionAr *), "");
+  static_assert(!__is_null_pointer(StructWithMembers *), "");
+  static_assert(!__is_null_pointer(void (*)()), "");
+
+  static_assert(!__is_null_pointer(void), "");
+  static_assert(!__is_null_pointer(cvoid), "");
+  static_assert(!__is_null_pointer(cvoid), "");
+  static_assert(!__is_null_pointer(char), "");
+  static_assert(!__is_null_pointer(int), "");
+  static_assert(!__is_null_pointer(int), "");
+  static_assert(!__is_null_pointer(ClassType), "");
+  static_assert(!__is_null_pointer(Derives), "");
+  static_assert(!__is_null_pointer(Enum), "");
+  static_assert(!__is_null_pointer(IntArNB), "");
+  static_assert(!__is_null_pointer(Union), "");
+  static_assert(!__is_null_pointer(UnionAr), "");
+  static_assert(!__is_null_pointer(StructWithMembers), "");
+  static_assert(!__is_null_pointer(int StructWithMembers::*), "");
+  static_assert(!__is_null_pointer(void(StructWithMembers::*)()), "");
 }
 
 void is_member_object_pointer()
@@ -3534,6 +3566,7 @@
 }
 
 template  using remove_reference_t = __remove_reference_t(T);
+template  using remove_reference = __remove_reference(T);
 
 void check_remove_reference() {
   static_assert(__is_same(remove_reference_t, void), "");
@@ -3568,6 +3601,39 @@
   static_assert(__is_same(remove_reference_t, int S::*const volatile), "");
   static_assert(__is_same(remove_reference_t, int(S::*const volatile)()), "");
   static_assert(__is_same(remove_reference_t, int(S::*const volatile)() &), "");
+
+  static_assert(__is_same(remove_reference, void), "");
+  static_assert(__is_same(remove_reference, const volatile void), "");
+  static_assert(__is_same(remove_reference, int), "");
+  static_assert(__is_same(remove_reference, const int), "");
+  static_assert(__is_same(remove_reference, volatile int), "");
+  static_assert(__is_same(remove_reference, const volatile int), "");
+  static_assert(__is_same(remove_reference, int *), "");
+  static_assert(__is_same(remove_reference, int *const volatile), "");
+  static_assert(__is_same(remove_reference, int const *const volatile), "");
+  static_assert(__is_same(remove_reference, int), "");
+  static_assert(__is_same(remove_reference, int const volatile), "");
+  static_assert(__is_same(remove_reference, int), "");
+  static_assert(__is_same(remove_reference, int const volatile), "");
+  static_assert(__is_same(remove_reference, int()), "");
+  static_assert(__is_same(remove_reference, int (*const volatile)()), "");
+  static_assert(__is_same(remove_reference, int()), "");
+
+  static_assert(__is_same(remove_reference, S), "");
+  static_assert(__is_same(remove_reference, S), "");
+  static_assert(__is_same(remove_reference, S), "");
+  static_assert(__is_same(remove_reference, const S), "");
+  static_assert(__is_same(remove_reference, const S), "");
+  static_assert(__is_same(remove_reference, const S), "");
+  static_assert(__is_same(remove_reference, volatile S), "");
+  static_assert(__is_same(remove_reference, volatile S), "");
+  static_assert(__is_same(remove_reference, volatile S), "");
+  static_assert(__is_same(remove_reference, const volatile S), "");
+  static_assert(__is_same(remove_reference, const volatile S), "");
+  static_assert(__is_same(remove_reference, const volatile S), "");
+  static_assert(__is_same(remove_reference, int S::*const volatile), "");
+  

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:51-52
+  Diag->getDiags()->getDiagnosticIDs()->getStableName(Diag->getID()).str();
+  std::replace(StableName.begin(), StableName.end(), '_', '.');
+  SarifRule Rule = SarifRule::create().setRuleId(StableName);
 

Endill wrote:
> vaibhav.y wrote:
> > cjdb wrote:
> > > denik wrote:
> > > > §3.5.4 says that the hierarchical strings are separated by "/".
> > > > 
> > > > But more generally, are diagnostic names really fall under 
> > > > "hierarchical" definition?
> > > > Words between underscores should be treated as components. And $3.27.5 
> > > > says that the leading components have to be the identifier of the rule.
> > > > In some cases they look like valid components, e.g. `err_access`, 
> > > > `err_access_dtor`, `err_access_dtor_exception`.
> > > > But in cases like `err_cannot_open_file` neither of the leading 
> > > > components exists.
> > > > 
> > > > Theoretically we could use groups as the leading component for warnings 
> > > > for example. For errors the leading components are probably not even 
> > > > necessary, since if I understood correctly they are needed to suppress 
> > > > subsets of violations on the SARIF consumer side.
> > > > Or we just could keep the names with underscores as is. WDYT?
> > > I think in light of what you've said, changing back to underscores is 
> > > probably best.
> > > But more generally, are diagnostic names really fall under "hierarchical" 
> > > definition?
> > 
> > I have the same concern, but this is okay for a first pass as a "flat 
> > hierarchy" :)
> > 
> > If we want a deeper structure, we'll need some extra metadata in 
> > `DiagnosticSemaKinds.td`'s `Error<...>`  to add cluster names as a follow 
> > up to this.
> > 
> > WDYT about something like `clang/visibility/err_access` or 
> > `clang/syntax/err_stmtexpr_file_scope`.
> > 
> > Alternatively: we could draw from the c++ standard structure: 
> > https://eel.is/c++draft/basic.def.odr#term.odr.use and say the error code 
> > for an ODR violation could be `clang/basic/def/odr`, again I'm unsure how 
> > well this meshes with clang's diagnostic model.
> > Alternatively: we could draw from the c++ standard structure: 
> > https://eel.is/c++draft/basic.def.odr#term.odr.use and say the error code 
> > for an ODR violation could be clang/basic/def/odr, again I'm unsure how 
> > well this meshes with clang's diagnostic model.
> 
> The only reliable thing there are stable clause names like 
> `[namespace.udecl]`, because there are precedents of them being rearranged in 
> the table of contents ([[ 
> https://github.com/cplusplus/draft/commit/982a456f176ca00409c6e514af932051dce2485f
>  | 1 ]], [[ 
> https://github.com/cplusplus/draft/commit/3c580cd204fde95a21de1830ace75d14d429f845
>  | 2 ]]). We've got bitten by that in C++ conformance tests, which use table 
> of contents for directory hierarchy.
> WDYT about something like `clang/visibility/err_access` or 
> `clang/syntax/err_stmtexpr_file_scope`.

I think this might work!

> > Alternatively: we could draw from the c++ standard structure: 
> > https://eel.is/c++draft/basic.def.odr#term.odr.use and say the error code 
> > for an ODR violation could be clang/basic/def/odr, again I'm unsure how 
> > well this meshes with clang's diagnostic model.
> 
> The only reliable thing there are stable clause names like 
> `[namespace.udecl]`, because there are precedents of them being rearranged in 
> the table of contents ([[ 
> https://github.com/cplusplus/draft/commit/982a456f176ca00409c6e514af932051dce2485f
>  | 1 ]], [[ 
> https://github.com/cplusplus/draft/commit/3c580cd204fde95a21de1830ace75d14d429f845
>  | 2 ]]). We've got bitten by that in C++ conformance tests, which use table 
> of contents for directory hierarchy.

Right. Given that section names aren't actually stable, relying on those would 
be brittle at best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146654

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

[PATCH] D129951: adds `__disable_adl` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Gentle ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12
   foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh 
no foo}}
+ // expected-note@* {{In function 'baz'}}
   if (x())

aaron.ballman wrote:
> nickdesaulniers wrote:
> > cjdb wrote:
> > > aaron.ballman wrote:
> > > > nickdesaulniers wrote:
> > > > > nickdesaulniers wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Instead of allowing this note to appear anywhere in the file, I 
> > > > > > > think it's better to use "bookmark" comments. e.g.,
> > > > > > > ```
> > > > > > > void baz() { // #baz_defn
> > > > > > > }
> > > > > > > 
> > > > > > > void foo() {
> > > > > > >   baz(); // expected-note@#baz_defn {{whatever note text}}
> > > > > > > }
> > > > > > > ```
> > > > > > > so that we're testing where the diagnostic is emitted. (This is 
> > > > > > > especially important given that the changes are about location 
> > > > > > > tracking.)
> > > > > > oh, that's new (to me)! will do
> > > > > It looks like the "bookmarks" don't work because the notes do not 
> > > > > line+col info. They follow the warning/error diagnostic which does, 
> > > > > on the bottom most call site.
> > > > > 
> > > > > The warning is supposed to be emitted on the callsite, not the 
> > > > > definition.
> > > > I still don't think this is reasonable for test coverage because this 
> > > > means we'll accept the note *anywhere* in the file. This makes it much 
> > > > too easy to regress the behavior accidentally (e.g., accidentally emit 
> > > > all the notes on line 1 of the file). I think we need *some* way to 
> > > > nail down where these notes are emitted in the test. However, I might 
> > > > be asking you to solve "please track location information through the 
> > > > backend" for that, so perhaps this isn't a reasonable request?
> > > > 
> > > > Also, CC @cjdb for awareness of how this kind of diagnostic is produced 
> > > > (Chris is working on an idea for how we emit diagnostics so we get 
> > > > better structured information from them, so knowing about this novel 
> > > > approach might impact that idea).
> > > Very interesting, thanks for the heads up! Cross-phase diagnostics are 
> > > definitely something I hadn't considered, and it **does** impact the 
> > > design (but not in a derailing way).
> > I think we're back at "please track location information through the 
> > backend".
> > 
> > That is the tradeoff with this approach; not measurably regressing 
> > performance when these attributes aren't used in source in exchange for 
> > Note diagnostics that lack precise line+col info, but in practice provide 
> > enough info for the user to understand what's going wrong where.
> > 
> > Your call if the tradeoff is worth it.
> Here's my thinking, please correct any misunderstandings I have:
> 
> * This functionality is primarily for use with `_FORTIFY_SOURCE` (but not 
> solely for that purpose), and that's an important security feature used by 
> glibc and the Linux kernel.
> * Security of C and C++ source code is Very Important, especially now that 
> various gov't agencies [1][2] are suggesting to not use C or C++ for new 
> projects specifically because the security posture of the languages and their 
> tools.
> * Therefore, the ergonomics of this functionality are quite important for the 
> intended use cases and the ecosystem as a whole.
> * Emitting only the function name but without location information does not 
> give a good user experience, especially in circumstances where the function 
> has multiple overloads, because it pushes the burden onto the programmer to 
> figure out which functions are actually involved in the call chain. This 
> issue is also present in C because of support for 
> `__attribute__((overloadable))` and is not just a C++ concern.
> * The compile-time performance costs of tracking this location information 
> are roughly 1%, and there are no runtime or binary size overhead costs unless 
> other functionality is enabled (such as emitting debug info).
> * We can determine whether we need to enable this location tracking while 
> building the AST when we see one of these two attributes being used, so we 
> could enable this functionality selectively without burdening the user with 
> enabling it manually via a flag. (We could still consider giving the user a 
> flag to never track this even in the presence of the attributes if a user had 
> a compelling use case for it.)
> 
> If this is all reasonably accurate, I think it's worth seriously considering 
> accepting the costs. I don't want to increase our compile time overhead, but 
> at the same time, if ~1% is a "make or break" amount of compile-time 
> performance to lose, we should be addressing *that* issue rather than 
> hamstringing a user-facing diagnostic feature related to security. It's hard 
> to argue "that CVE was totally worth it because we 

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12
   foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh 
no foo}}
+ // expected-note@* {{In function 'baz'}}
   if (x())

aaron.ballman wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > aaron.ballman wrote:
> > > > Instead of allowing this note to appear anywhere in the file, I think 
> > > > it's better to use "bookmark" comments. e.g.,
> > > > ```
> > > > void baz() { // #baz_defn
> > > > }
> > > > 
> > > > void foo() {
> > > >   baz(); // expected-note@#baz_defn {{whatever note text}}
> > > > }
> > > > ```
> > > > so that we're testing where the diagnostic is emitted. (This is 
> > > > especially important given that the changes are about location 
> > > > tracking.)
> > > oh, that's new (to me)! will do
> > It looks like the "bookmarks" don't work because the notes do not line+col 
> > info. They follow the warning/error diagnostic which does, on the bottom 
> > most call site.
> > 
> > The warning is supposed to be emitted on the callsite, not the definition.
> I still don't think this is reasonable for test coverage because this means 
> we'll accept the note *anywhere* in the file. This makes it much too easy to 
> regress the behavior accidentally (e.g., accidentally emit all the notes on 
> line 1 of the file). I think we need *some* way to nail down where these 
> notes are emitted in the test. However, I might be asking you to solve 
> "please track location information through the backend" for that, so perhaps 
> this isn't a reasonable request?
> 
> Also, CC @cjdb for awareness of how this kind of diagnostic is produced 
> (Chris is working on an idea for how we emit diagnostics so we get better 
> structured information from them, so knowing about this novel approach might 
> impact that idea).
Very interesting, thanks for the heads up! Cross-phase diagnostics are 
definitely something I hadn't considered, and it **does** impact the design 
(but not in a derailing way).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D147844#4293988 , @dblaikie wrote:

> In D147844#4293743 , @cjdb wrote:
>
>> In D147844#4293693 , @dblaikie 
>> wrote:
>>
 I think some of the cases are ambiguous while others are not.
>>>
>>> Data would be good to have - if this assessment is true, we'd expect this 
>>> to bear out in terms of bug finding, yeah? (that the cases you find to be 
>>> ambiguous turn up as real bugs with some significant frequency in a 
>>> codebase?)
>>
>> I disagree that there's a need for bugs to add this warning. Reading 
>> ambiguous-but-correct code isn't going to be buggy, but it is going to cause 
>> readability issues for any reviewers or maintainers.
>
> That's generally been the bar we use for evaluating warnings - does it find 
> bugs. Especially because if it doesn't, it's unlikely to be turned on on 
> large pre-existing codebases owing to the cost of cleaning them up with 
> limited value in terms of improving readability but not finding any bugs. (& 
> goes hand-in-hand with the general policy of not adding off-by-default 
> warnings, because they don't get used much and come at a cost to clang's 
> codebase (& some (death-by-a-thousand-cuts) cost to compile time performance, 
> even when the warning is disabled))
>
> Readability improvements that don't cross the threshold to be the cause of a 
> significant number of bugs are moreso the domain of clang-tidy, not clang 
> warnings.

Fair enough, I often mix up which project a warning belongs to, so thanks for 
the reminder :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D147844#4293693 , @dblaikie wrote:

>> I think some of the cases are ambiguous while others are not.
>
> Data would be good to have - if this assessment is true, we'd expect this to 
> bear out in terms of bug finding, yeah? (that the cases you find to be 
> ambiguous turn up as real bugs with some significant frequency in a codebase?)

I disagree that there's a need for bugs to add this warning. Reading 
ambiguous-but-correct code isn't going to be buggy, but it is going to cause 
readability issues for any reviewers or maintainers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I think this is a good diagnostic to add: it improves readability and 
eliminates ambiguities. My only request is that if there isn't already a FixIt 
hint, one be added, please.




Comment at: clang/test/Sema/parentheses.c:94
 
   (void)(x + y > 0 ? 1 : 2); // no warning
   (void)(x + (y > 0) ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '+'}} expected-note 2{{place parentheses}}

I think this should also warn.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148835: [clang] removes trailing whitespace

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D148835#4288151 , @erichkeane 
wrote:

> In D148835#4288148 , @cjdb wrote:
>
>> In D148835#4286924 , 
>> @aaron.ballman wrote:
>>
>>> In D148835#4286905 , @erichkeane 
>>> wrote:
>>>
 In D148835#4284871 , @cjdb wrote:

> I've had some very good input about why this probably shouldn't go ahead: 
> git history erasure :')

 While that is a common criticism, if we're going to do this at all, we 
 should do it in a single patch, as we can use --ignore-rev: 
 https://akrabat.com/ignoring-revisions-with-git-blame/

 This is a pretty common thing to do, and I don't think it should prevent 
 us from doing this, which I think is the 'greater good' here.  The 
 alternative is to continue having a bunch of unrelated patches piece-meal 
 'erase' git history as people accidentally fix these.
>>>
>>> We already ignore blame revisions like this today: 
>>> https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs
>>
>> Oh cool, I had no idea this was possible :)
>>
>>> However, I'd ask that we hold off on this change unless we have some 
>>> tooling in place that rejects new additions of trailing whitespace, 
>>> otherwise we're going to end up in this exact same situation again in 
>>> another week or two. When CI starts failing on patches that insert *new* 
>>> trailing whitespace, then I'm all for this change because we can fix it 
>>> once and hopefully keep it fixed.
>>
>> Since Clang doesn't have pre-commit CI, how can we meaningfully progress 
>> here?
>
> Phab at least has Pre-commit CI, i would expect that to be sufficient? It 
> would at least cut down accidential whitespace by orders of magnitude.

Right, but Phab's precommit CI doesn't seem to be a blocker for merging, and 
often has false negatives, so I expect stuff to slip through :/

In D148835#4288248 , @sbc100 wrote:

> In D148835#4288151 , @erichkeane 
> wrote:
>
>> In D148835#4288148 , @cjdb wrote:
>>
>>> In D148835#4286924 , 
>>> @aaron.ballman wrote:
>>>
 In D148835#4286905 , @erichkeane 
 wrote:

> In D148835#4284871 , @cjdb 
> wrote:
>
>> I've had some very good input about why this probably shouldn't go 
>> ahead: git history erasure :')
>
> While that is a common criticism, if we're going to do this at all, we 
> should do it in a single patch, as we can use --ignore-rev: 
> https://akrabat.com/ignoring-revisions-with-git-blame/
>
> This is a pretty common thing to do, and I don't think it should prevent 
> us from doing this, which I think is the 'greater good' here.  The 
> alternative is to continue having a bunch of unrelated patches piece-meal 
> 'erase' git history as people accidentally fix these.

 We already ignore blame revisions like this today: 
 https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs
>>>
>>> Oh cool, I had no idea this was possible :)
>>>
 However, I'd ask that we hold off on this change unless we have some 
 tooling in place that rejects new additions of trailing whitespace, 
 otherwise we're going to end up in this exact same situation again in 
 another week or two. When CI starts failing on patches that insert *new* 
 trailing whitespace, then I'm all for this change because we can fix it 
 once and hopefully keep it fixed.
>>>
>>> Since Clang doesn't have pre-commit CI, how can we meaningfully progress 
>>> here?
>>
>> Phab at least has Pre-commit CI, i would expect that to be sufficient? It 
>> would at least cut down accidential whitespace by orders of magnitude.
>
> Yes, isn't this already covered by the existing use of `clang-format` on 
> upload to phabricator (`arc diff`) ?At least for me it always gives me 
> warnings if I try to upload anything that doesn't match clang-format.

There are folks who have historically uploaded using the web UI specifically to 
ignore clang-format's suggestions, and `arc diff --nolint` will skip the 
formatting stage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D129835: [clang] adds a discardable attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Yeah, in retrospect, I agree (and may need to clean up a few iterator types 
now, grr).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D148835: [clang] removes trailing whitespace

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D148835#4286924 , @aaron.ballman 
wrote:

> In D148835#4286905 , @erichkeane 
> wrote:
>
>> In D148835#4284871 , @cjdb wrote:
>>
>>> I've had some very good input about why this probably shouldn't go ahead: 
>>> git history erasure :')
>>
>> While that is a common criticism, if we're going to do this at all, we 
>> should do it in a single patch, as we can use --ignore-rev: 
>> https://akrabat.com/ignoring-revisions-with-git-blame/
>>
>> This is a pretty common thing to do, and I don't think it should prevent us 
>> from doing this, which I think is the 'greater good' here.  The alternative 
>> is to continue having a bunch of unrelated patches piece-meal 'erase' git 
>> history as people accidentally fix these.
>
> We already ignore blame revisions like this today: 
> https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs

Oh cool, I had no idea this was possible :)

> However, I'd ask that we hold off on this change unless we have some tooling 
> in place that rejects new additions of trailing whitespace, otherwise we're 
> going to end up in this exact same situation again in another week or two. 
> When CI starts failing on patches that insert *new* trailing whitespace, then 
> I'm all for this change because we can fix it once and hopefully keep it 
> fixed.

Since Clang doesn't have pre-commit CI, how can we meaningfully progress here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D129835: [clang] adds a discardable attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

>> With [[discardable]] one just needs to push/pop at the extremes of a file 
>> (and if a future version of module maps supports global pragmas for a 
>> module, there too, but that's a discussion that requires a design doc).
>
> I understood that, I just don't think that's a good thing. This is basically 
> an attribute that says "I know we said we wanted everything here to be 
> nodiscard, but JUST KIDDING not this one!" which is not a very clean approach 
> to writing headers.

@aaron.ballman Ugh, I've finally come up with a good use-case for 
`[[discardable]]`.

  class [[nodiscard]] iterator {
  public:
// usual iterator stuff
  
[[clang::discardable]] iterator operator++(int);
[[clang::discardable]] iterator operator--(int);
  };

I hate it, but the alternative is to decorate everything else with 
`[[nodiscard]]` just to facilitate these two operators. It's a compelling 
use-case, but I'm not sure if it's compelling enough on its own. WDYT?
(I personally think that those two should be nodiscard, but `i++` is pervasive 
enough that it might never be practical to correct.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

@aaron.ballman are you okay with this being merged now, provided that it's off 
by default? Apologies for letting this one fall through the cracks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

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

[PATCH] D140125: [clang] splits diagnostic message into summary and reason

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

Abandoning since we're going down a different design path now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140125

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

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.
Herald added a subscriber: PiotrZSL.
Herald added a project: clang-format.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

Abandoning since we're heading down a different design path now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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

[PATCH] D135338: [clang] adds move-assignable type-trait builtins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

Abandoning, since this doesn't have consensus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135338

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

[PATCH] D135240: [clang] adds move-constructible type-trait builtins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

Abandoning, since this doesn't have consensus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135240

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

[PATCH] D135239: [clang] adds copy-assignable type-trait builtins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

Abandoning, since this doesn't have consensus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135239

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

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

Abandoning, since this doesn't have consensus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135238

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

[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens in declarations

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.
Herald added a project: All.

I think there was a clang-tidy patch that handled this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107292

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

[PATCH] D129951: adds `__disable_adl` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756
+  if (FunctionDecl *F = D->getAsFunction();
+  F->isOverloadedOperator() || F->isCXXClassMember()) {
+S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators)

cor3ntin wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > cjdb wrote:
> > > > > rsmith wrote:
> > > > > > Maybe also global `operator new` / `operator delete`?
> > > > > How does ADL even work in with the global namespace? Should it be 
> > > > > invalid to apply here?
> > > > Should this be `isCXXInstanceMember()` instead? Or do we intend to 
> > > > disallow this on static member functions?
> > > Static member functions would be qualified, and not involve ADL. This 
> > > seems correct.
> > Hmmm, I think they're *normally* qualified, but if you call the static 
> > function from within a member function, it doesn't have to be qualified.
> nvm, they can be found by adl from within member functions.
Do we have a use-case for this? I'd rather start with global functions and add 
static member functions if a genuine need arises.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D129951: adds `__disable_adl` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 515497.
cjdb marked 6 inline comments as done.
cjdb added a comment.
Herald added subscribers: aheejin, dschuff.

- undoes whitespace changes as requested
- documents feature

I think the only thing left is to address the global new/delete and static 
member function discussion, which should be quickly implementable!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/disable-adl.cpp

Index: clang/test/SemaCXX/disable-adl.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/disable-adl.cpp
@@ -0,0 +1,179 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++20
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++2b
+
+namespace NS1 {
+  struct S1 {};
+  S1 inhibited(S1); // expected-note 2 {{candidate function}}
+
+  namespace NNS1 {
+struct S2 {};
+__disable_adl void hidden(S2);   // expected-note{{declared here}}
+__disable_adl int inhibited(S1); // expected-note 4 {{candidate function}}
+  }
+}
+
+namespace NS2 {
+  __disable_adl void inhibited(NS1::S1); // expected-note 2 {{candidate function}}
+}
+
+void test_functions() {
+  hidden(NS1::NNS1::S2{}); // expected-error{{use of undeclared identifier 'hidden'; did you mean 'NS1::NNS1::hidden'?}}
+  {
+NS1::S1 x = inhibited(NS1::S1{}); // no error
+  }
+  {
+using namespace NS1::NNS1;
+int x = inhibited(NS1::S1{}); // no error
+
+using namespace NS1;
+S1 y = inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+  }
+  {
+using NS1::NNS1::inhibited;
+int x = inhibited(NS1::S1{}); // no error
+
+using NS1::inhibited;
+NS1::S1 y = inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+  }
+  {
+using namespace NS2;
+inhibited(NS1::S1{}); // no error
+
+using namespace NS1::NNS1;
+inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+  }
+  {
+using NS2::inhibited;
+inhibited(NS1::S1{}); // no error
+
+using NS1::NNS1::inhibited;
+inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+  }
+}
+
+namespace NS1 {
+  template
+  S1 inhibited_template(T); // expected-note 2 {{candidate function}}
+
+  namespace NNS1 {
+template
+__disable_adl void hidden_template(T); // expected-note{{declared here}}
+
+template
+__disable_adl int inhibited_template(T); // expected-note 4 {{candidate function}}
+  }
+}
+
+namespace NS2 {
+  template
+  __disable_adl int inhibited_template(T); // expected-note 2 {{candidate function}}
+}
+
+void test_function_templates() {
+  hidden_template(NS1::NNS1::S2{}); // expected-error{{use of undeclared identifier 'hidden_template'; did you mean 'NS1::NNS1::hidden_template'?}}
+
+  {
+NS1::S1 x = inhibited_template(NS1::S1{}); // no error
+  }
+  {
+using namespace NS1::NNS1;
+int x = inhibited_template(NS1::S1{}); // no error
+
+using namespace NS1;
+S1 y = inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+  }
+  {
+using NS1::NNS1::inhibited_template;
+int x = inhibited_template(NS1::S1{}); // no error
+
+using NS1::inhibited_template;
+NS1::S1 y = inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+  }
+  {
+using namespace NS2;
+inhibited_template(NS1::S1{}); // no error
+
+using namespace NS1::NNS1;
+inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+  }
+  {
+using NS2::inhibited_template;
+inhibited_template(NS1::S1{}); // no error
+
+using NS1::NNS1::inhibited_template;
+inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+  }
+}
+
+namespace NS1 {
+  S1 inhibited_mixed(S1);
+
+  namespace NNS1 {
+template
+__disable_adl int inhibited_mixed(T);
+  }
+}
+
+void test_mixed() {
+  using namespace NS1::NNS1;
+  int x = inhibited_mixed(NS1::S1{}); // no error
+}
+
+// Should be covered by the hidden functions checks, but just to be sure.
+void test_NNS1_hidden() {
+  {
+NS1::S1 a = inhibited(NS1::S1{});
+NS1::S1 b = inhibited_template(NS1::S1{});
+NS1::S1 c = inhibited_mixed(NS1::S1{});
+  }
+  {
+using namespace NS1;
+NS1::S1 a = inhibited(NS1::S1{});
+NS1::S1 b = inhibited_template(NS1::S1{});
+NS1::S1 c = inhibited_mixed(NS1::S1{});
+  }
+}
+
+namespace NS1 {
+  namespace NNS1 {
+__disable_adl void operator-(S2); // 

[PATCH] D148835: [clang] removes trailing whitespace

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

I've had some very good input about why this probably shouldn't go ahead: git 
history erasure :')


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D148835: [clang] removes trailing whitespace

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: aaron.ballman, shafik, erichkeane, dblaikie.
Herald added subscribers: luke, steakhal, bzcheeseman, kosarev, pmatos, asb, 
ormris, frasercrmck, jdoerfert, martong, luismarques, apazos, sameer.abuasal, 
pengfei, s.egerton, Jim, jocewei, PkmX, arphaman, the_o, brucehoult, 
MartinMosbeck, rogfer01, steven_wu, atanasyan, edward-jones, zzheng, jrtc27, 
niosHD, sabuasal, simoncook, johnrusso, rbar, kbarton, hiraditya, krytarowski, 
whisperity, sbc100, jvesely, nemanjai, dschuff.
Herald added a reviewer: paulkirth.
Herald added a reviewer: NoQ.
Herald added a reviewer: ributzka.
Herald added a project: All.
Herald added a comment.
cjdb requested review of this revision.
Herald added subscribers: cfe-commits, jplehr, pcwang-thead, sstefan1, MaskRay, 
aheejin.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: dang.
Herald added a project: clang.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to ClangFormatStyleOptions.rst but not a change 
to clang/include/clang/Format/Format.h

ClangFormatStyleOptions.rst is auto generated from Format.h via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Lots of files have trailing whitespace characters, which get removed by
many text editors upon save. I've run `sed -i 's/\s*$//g'` on all files
in `clang` except for clang-format, and manually reverted anything that
causes `ninja check-clang check-clang-utils` to fail. This ensures that
commits of substance don't accrue bogus diffs and commentary requesting
they be reverted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148835

Files:
  clang/ModuleInfo.txt
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ClangLinkerWrapper.rst
  clang/docs/ClangOffloadPackager.rst
  clang/docs/ClangTransformerTutorial.rst
  clang/docs/DebuggingCoroutines.rst
  clang/docs/ExternalClangExamples.rst
  clang/docs/JSONCompilationDatabase.rst
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/StandardCPlusPlusModules.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/docs/tools/dump_ast_matchers.py
  clang/examples/PrintFunctionNames/PrintFunctionNames.cpp
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/Analysis/CallGraph.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Sema/CMakeLists.txt
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/test/ARCMT/Common.h
  clang/test/ARCMT/Inputs/module.map
  clang/test/ARCMT/atautorelease-check.m
  clang/test/ARCMT/checking.m
  clang/test/ARCMT/cxx-checking.mm
  clang/test/ARCMT/designated-init-in-header/file2.m.in
  clang/test/ARCMT/designated-init-in-header/file2.m.in.result
  clang/test/ARCMT/no-canceling-bridge-to-bridge-cast.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/ARCMT/objcmt-atomic-property.m
  clang/test/ARCMT/objcmt-atomic-property.m.result
  clang/test/ARCMT/objcmt-boxing.m
  clang/test/ARCMT/objcmt-boxing.m.result
  clang/test/ARCMT/objcmt-migrate-all.m
  clang/test/ARCMT/objcmt-migrate-all.m.result
  clang/test/ARCMT/objcmt-ns-macros.m
  clang/test/ARCMT/objcmt-ns-macros.m.result
  clang/test/ARCMT/objcmt-ns-nonatomic-iosonly.m
  clang/test/ARCMT/objcmt-ns-nonatomic-iosonly.m.result
  clang/test/ARCMT/objcmt-ns-returns-inner-pointer.m
  clang/test/ARCMT/objcmt-ns-returns-inner-pointer.m.result
  clang/test/ARCMT/objcmt-numeric-literals.m
  clang/test/ARCMT/objcmt-numeric-literals.m.result
  clang/test/ARCMT/objcmt-property-dot-syntax.m
  clang/test/ARCMT/objcmt-property-dot-syntax.m.result
  clang/test/ARCMT/objcmt-property.m
  clang/test/ARCMT/objcmt-property.m.result
  clang/test/ARCMT/objcmt-protocol-conformance.m
  

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG33e21610f9cd: [clang] makes 
`__is_trivially_equality_comparable` available as a struct (authored by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148677

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp


Index: clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify 
-std=gnu++20 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify 
-std=gnu++2b -fms-extensions -Wno-microsoft %s
+
+template 
+struct Same {
+  static constexpr auto value = __is_same(T, U);
+};
+
+template 
+struct __is_trivially_equality_comparable { // expected-warning{{keyword 
'__is_trivially_equality_comparable' will be made available as an identifier 
for the remainder of the translation unit}}
+  using type = T;
+};
+
+using A = Same<__is_trivially_equality_comparable::type, 
__is_trivially_equality_comparable::type>;
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,


Index: clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++2b -fms-extensions -Wno-microsoft %s
+
+template 
+struct Same {
+  static constexpr auto value = __is_same(T, U);
+};
+
+template 
+struct __is_trivially_equality_comparable { // expected-warning{{keyword '__is_trivially_equality_comparable' will be made available as an identifier for the remainder of the translation unit}}
+  using type = T;
+};
+
+using A = Same<__is_trivially_equality_comparable::type, __is_trivially_equality_comparable::type>;
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

This patch is causing breakages downstream because it didn't have do the struct 
dance, so I've added D148677 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 514798.
cjdb added a comment.

uploads the right test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148677

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp


Index: clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify 
-std=gnu++20 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify 
-std=gnu++2b -fms-extensions -Wno-microsoft %s
+
+template 
+struct Same {
+  static constexpr auto value = __is_same(T, U);
+};
+
+template 
+struct __is_trivially_equality_comparable { // expected-warning{{keyword 
'__is_trivially_equality_comparable' will be made available as an identifier 
for the remainder of the translation unit}}
+  using type = T;
+};
+
+using A = Same<__is_trivially_equality_comparable::type, 
__is_trivially_equality_comparable::type>;
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,


Index: clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/libcxx_is_trivially_equality_comparable_hack.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fms-extensions -Wno-microsoft %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++2b -fms-extensions -Wno-microsoft %s
+
+template 
+struct Same {
+  static constexpr auto value = __is_same(T, U);
+};
+
+template 
+struct __is_trivially_equality_comparable { // expected-warning{{keyword '__is_trivially_equality_comparable' will be made available as an identifier for the remainder of the translation unit}}
+  using type = T;
+};
+
+using A = Same<__is_trivially_equality_comparable::type, __is_trivially_equality_comparable::type>;
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Since this was originally a name in library, it needs an escape hatch
for versions of Clang that are slightly out-of-sync with libc++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148677

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/SemaCXX/move-not-noexcept.cpp


Index: clang/test/SemaCXX/move-not-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/move-not-noexcept.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+struct TrivialMoveOps {};
+
+struct DefaultedMoveCtor {
+  DefaultedMoveCtor(DefaultedMoveCtor&&) = default;
+};
+
+struct DefaultedMoveAssign {
+  DefaultedMoveAssign& operator=(DefaultedMoveAssign&&) = default;
+};
+
+struct UnknownNoexceptCtor {
+  UnknownNoexceptCtor(UnknownNoexceptCtor&&);
+  // expected-warning@-1{{move constructor for 'UnknownNoexceptCtor' is not 
noexcept, which can cause performance issues; use 'noexcept(false)' if this is 
intentional}}
+};
+
+struct UnknownNoexceptAssign {
+  UnknownNoexceptAssign& operator=(UnknownNoexceptAssign&&);
+  // expected-warning@-1{{move assignment operator for 'UnknownNoexceptAssign' 
is not noexcept, which can cause performance issues; use 'noexcept(false)' if 
this is intentional}}
+};
+
+struct NoexceptSpecifierCtor {
+  NoexceptSpecifierCtor(NoexceptSpecifierCtor&&) noexcept;
+};
+
+struct NoexceptSpecifierAssign {
+  NoexceptSpecifierAssign& operator=(NoexceptSpecifierAssign&&) noexcept;
+};
+
+struct NoexceptTrueSpecifierCtor {
+  NoexceptTrueSpecifierCtor(NoexceptTrueSpecifierCtor&&) noexcept(true);
+};
+
+struct NoexceptTrueSpecifierAssign {
+  NoexceptTrueSpecifierAssign& operator=(NoexceptTrueSpecifierAssign&&) 
noexcept(true);
+};
+
+struct NoexceptFalseSpecifierCtor {
+  NoexceptFalseSpecifierCtor(NoexceptFalseSpecifierCtor&&) noexcept(false);
+};
+
+struct NoexceptFalseSpecifierAssign {
+  NoexceptFalseSpecifierAssign& operator=(NoexceptFalseSpecifierAssign&&) 
noexcept(false);
+};
+
+template
+struct NoexceptDependentSpecifierCtor {
+  NoexceptDependentSpecifierCtor(NoexceptDependentSpecifierCtor&&) 
noexcept(__is_integral(T));
+};
+
+template
+struct NoexceptDependentSpecifierAssign {
+  NoexceptDependentSpecifierAssign& 
operator=(NoexceptDependentSpecifierAssign&&) noexcept(__is_integral(T));
+};
+
+struct ThrowParensSpecifierCtor {
+  ThrowParensSpecifierCtor(ThrowParensSpecifierCtor&&) throw();
+};
+
+struct ThrowParensSpecifierAssign {
+  ThrowParensSpecifierAssign& operator=(ThrowParensSpecifierAssign&&) throw();
+};
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -1622,6 +1622,7 @@
   tok::kw___is_signed,
   tok::kw___is_standard_layout,
   tok::kw___is_trivial,
+  tok::kw___is_trivially_equality_comparable,
   tok::kw___is_trivially_assignable,
   tok::kw___is_trivially_constructible,
   tok::kw___is_trivially_copyable,


Index: clang/test/SemaCXX/move-not-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/move-not-noexcept.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+struct TrivialMoveOps {};
+
+struct DefaultedMoveCtor {
+  DefaultedMoveCtor(DefaultedMoveCtor&&) = default;
+};
+
+struct DefaultedMoveAssign {
+  DefaultedMoveAssign& operator=(DefaultedMoveAssign&&) = default;
+};
+
+struct UnknownNoexceptCtor {
+  UnknownNoexceptCtor(UnknownNoexceptCtor&&);
+  // expected-warning@-1{{move constructor for 'UnknownNoexceptCtor' is not noexcept, which can cause performance issues; use 'noexcept(false)' if this is intentional}}
+};
+
+struct UnknownNoexceptAssign {
+  UnknownNoexceptAssign& operator=(UnknownNoexceptAssign&&);
+  // expected-warning@-1{{move assignment operator for 'UnknownNoexceptAssign' is not noexcept, which can cause performance issues; use 'noexcept(false)' if this is intentional}}
+};
+
+struct NoexceptSpecifierCtor {
+  NoexceptSpecifierCtor(NoexceptSpecifierCtor&&) noexcept;
+};
+
+struct NoexceptSpecifierAssign {
+  NoexceptSpecifierAssign& operator=(NoexceptSpecifierAssign&&) noexcept;
+};
+
+struct NoexceptTrueSpecifierCtor {
+  NoexceptTrueSpecifierCtor(NoexceptTrueSpecifierCtor&&) noexcept(true);
+};
+
+struct NoexceptTrueSpecifierAssign {
+  NoexceptTrueSpecifierAssign& operator=(NoexceptTrueSpecifierAssign&&) noexcept(true);
+};
+
+struct NoexceptFalseSpecifierCtor {
+  NoexceptFalseSpecifierCtor(NoexceptFalseSpecifierCtor&&) noexcept(false);
+};
+
+struct NoexceptFalseSpecifierAssign {
+  NoexceptFalseSpecifierAssign& 

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I'm gonna get started on this today!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147717

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

[PATCH] D147888: Update declaration message of extern linkage

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I think we should fundamentally rethink this entire category of diagnostic. 
Rather than having a static diagnostic per offence stating //what// happened, 
we should instead have a single diagnostic that captures both what happened, 
why it's bad, and how to fix it. Something along the lines of

  error: 'x' has been declared with incompatible linkage specifiers (static and 
extern); please pick exactly one
extern int x;
^~
  note: previous definition here
static int x;
^~

It'd also be more robust from an engineering perspective, since it means that 
we won't need to add new diagnostic permutations every time a new linkage 
specifier is added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147888

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

[PATCH] D147904: [Clang] Fix cast to BigIntType in hasUniqueObjectRepresentations

[Christopher Di Bella via Phabricator via cfe-commits]

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

LGTM, thanks for such a quick fix on a weekend :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147904

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

[PATCH] D147875: [clang][Diagnostics] WIP: Show line numbers when printing code snippets

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

I have no opinion on this addition. If folks find it useful in GCC, happy to 
see it added.




Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1139
+return 7;
+  return 0;
+}

I'd prefer this to be an assert rather than returning a value. We should never 
be able to return from here.


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

https://reviews.llvm.org/D147875

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

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D147717#4248989 , @erichkeane 
wrote:

> @cjdb has been running some GDB test suites against our compiler: I am 
> wondering if we could ask him to try the consteval ones too before we set 
> this?

A lot of that work is manual because I need to interpret LIT messages to know 
wether something is a config error (e.g. missing `// expected-no-diagnostics`), 
a difference in opinion between compilers (e.g. GCC says it's an error and 
Clang says it's a note, reporting diagnostics on different lines, etc.), or 
actually an error in Clang.

I won't have time to check this week, but perhaps next week if the test set 
isn't too large (failing that, I'll at least try to get you `// 
expected-no-diagnostics` for improved confidence). Does this timeframe seem 
acceptable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147717

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

[PATCH] D147288: [clang][NFC] updates cxx_status for P2113R0

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb abandoned this revision.
cjdb added a comment.

In D147288#4237413 , @royjacobson 
wrote:

> There was some discussion of this last year in this review: 
> https://reviews.llvm.org/D128750
>
> It's such an edge case that I don't think we should lose sleep about it 
> until/unless the committee finds time to clarify the issue.

I'm okay with this conclusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147288

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

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Thanks so much for seeing this through; I'm unusually looking forward to 
rebuilding LLVM this weekend!




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:82
+
+.. option:: OverloadedOperators
+

This is a great solution for ranges pipelines, which I've struggled to work out 
a good automated policy on for years.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144522

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