[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -10136,6 +10136,9 @@ def err_vec_builtin_incompatible_vector : Error<
 def err_vsx_builtin_nonconstant_argument : Error<
   "argument %0 to %1 must be a 2-bit unsigned literal (i.e. 0, 1, 2 or 3)">;
 
+def err_vectorelements_non_vector : Error<
+ "'__builtin_vectorelements' argument must be a vector">;

erichkeane wrote:

Do we use similar language elsewhere?  "Must be a vector" is pretty confusing.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -5456,9 +5459,8 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
 /// TemplateArguments, followed by a QualType representing the
 /// non-canonical aliased type when the template is a type alias
 /// template.
-class alignas(8) TemplateSpecializationType
-: public Type,
-  public llvm::FoldingSetNode {
+class alignas(8) TemplateSpecializationType : public Type,

erichkeane wrote:

Unrelated change?  Or am I missing something?

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -3083,6 +3083,19 @@ ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
 E->getTypeOfArgument()->getPointeeType()))
 .getQuantity();
 return llvm::ConstantInt::get(CGF.SizeTy, Alignment);
+  } else if (E->getKind() == UETT_VectorElements) {
+// For scalable vectors, we don't know the size at compile time. We can use
+// @llvm.vscale to calculate it at runtime.
+if (E->getTypeOfArgument()->isSizelessVectorType()) {

erichkeane wrote:

Where do you handle non-sizeless vector types that make it here?   

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -0,0 +1,121 @@
+// RUN: %clang_cc1 -O1 -triple aarch64 -target-feature +neon %s -emit-llvm -o 
- | FileCheck --check-prefixes=CHECK,NEON %s

erichkeane wrote:

Add -disable-llvm-passes to all of these.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -10136,6 +10136,9 @@ def err_vec_builtin_incompatible_vector : Error<
 def err_vsx_builtin_nonconstant_argument : Error<
   "argument %0 to %1 must be a 2-bit unsigned literal (i.e. 0, 1, 2 or 3)">;
 
+def err_vectorelements_non_vector : Error<
+ "'__builtin_vectorelements' argument must be a vector">;

erichkeane wrote:

Do we use similar language elsewhere?  "Must be a vector" is pretty confusing.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -5456,9 +5459,8 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
 /// TemplateArguments, followed by a QualType representing the
 /// non-canonical aliased type when the template is a type alias
 /// template.
-class alignas(8) TemplateSpecializationType
-: public Type,
-  public llvm::FoldingSetNode {
+class alignas(8) TemplateSpecializationType : public Type,

erichkeane wrote:

Unrelated change?  Or am I missing something?

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

erichkeane wrote:

Also, needs a release note.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

erichkeane wrote:

Also, needs a release note.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Sema] Use original template pattern when declaring implicit deduction guides for nested template classes (PR #68379)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/68379
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

[Erich Keane via cfe-commits]

@@ -1299,8 +1299,9 @@ static bool checkTupleLikeDecomposition(Sema &S,
   //   in the associated namespaces.
   Expr *Get = UnresolvedLookupExpr::Create(
   S.Context, nullptr, NestedNameSpecifierLoc(), SourceLocation(),
-  DeclarationNameInfo(GetDN, Loc), /*RequiresADL*/true, &Args,
-  UnresolvedSetIterator(), UnresolvedSetIterator());
+  DeclarationNameInfo(GetDN, Loc), /*RequiresADL*/ true, &Args,
+  UnresolvedSetIterator(), UnresolvedSetIterator(),
+  /*KnownDependent*/ false);

erichkeane wrote:

```suggestion
  /*KnownDependent=*/ false);
```

https://github.com/llvm/llvm-project/pull/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

[Erich Keane via cfe-commits]

@@ -165,3 +165,18 @@ namespace BindingInStmtExpr {
   using U = decltype(num_bindings()); // expected-note {{previous}}
   using U = N<3>; // expected-error-re {{type alias redefinition with 
different types ('N<3>' vs {{.*}}N<2>}}
 }
+
+namespace PR65153 {
+struct A{};
+
+template 
+const A JoinStringViews = T;
+
+template 
+class Builder {
+public:
+static constexpr A Equal{};
+// no crash here
+static constexpr auto Val = JoinStringViews;
+};
+} // namespace PR65153

erichkeane wrote:

needs newline at end.

https://github.com/llvm/llvm-project/pull/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

This needs a release note, else I think it is OK.

https://github.com/llvm/llvm-project/pull/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Sema] Don't assert non-empty unexpanded packs following Colle… (PR #69224)

[Erich Keane via cfe-commits]

erichkeane wrote:

@cor3ntin : Lambda stuff, I think you're more familiar than I am with this.  
Mind taking a pass?

https://github.com/llvm/llvm-project/pull/69224
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -13595,6 +13595,15 @@ bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
 Info.Ctx.getOpenMPDefaultSimdAlign(E->getArgumentType()))
 .getQuantity(),
 E);
+  case UETT_VectorElements: {
+QualType Ty = E->getTypeOfArgument();
+// If the vector has a fixed size, we can determine the number of elements
+// at compile time.
+if (Ty->isVectorType())
+  return Success(Ty->castAs()->getNumElements(), E);
+
+return false;

erichkeane wrote:

Your understanding of how this works is correct, I missed you were handling 
sizeless vectors that way.  So above 'return false' there 
could be an `assert(Ty->isSizelessVectorType())` ?  

Note that we're not guaranteed to come to ExprConstant.cpp in the case where it 
is a sized-vector, so you likely need to make sure the CGExprScalar code 
handles sized types as well.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -13595,6 +13595,15 @@ bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
 Info.Ctx.getOpenMPDefaultSimdAlign(E->getArgumentType()))
 .getQuantity(),
 E);
+  case UETT_VectorElements: {
+QualType Ty = E->getTypeOfArgument();
+// If the vector has a fixed size, we can determine the number of elements
+// at compile time.
+if (Ty->isVectorType())
+  return Success(Ty->castAs()->getNumElements(), E);
+
+return false;

erichkeane wrote:

Your understanding of how this works is correct, I missed you were handling 
sizeless vectors that way.  So above 'return false' there 
could be an `assert(Ty->isSizelessVectorType())` ?  

Note that we're not guaranteed to come to ExprConstant.cpp in the case where it 
is a sized-vector, so you likely need to make sure the CGExprScalar code 
handles sized types as well.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -13595,6 +13595,15 @@ bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
 Info.Ctx.getOpenMPDefaultSimdAlign(E->getArgumentType()))
 .getQuantity(),
 E);
+  case UETT_VectorElements: {
+QualType Ty = E->getTypeOfArgument();
+// If the vector has a fixed size, we can determine the number of elements
+// at compile time.
+if (Ty->isVectorType())
+  return Success(Ty->castAs()->getNumElements(), E);
+
+return false;

erichkeane wrote:

Your understanding of how this works is correct, I missed you were handling 
sizeless vectors that way.  So above 'return false' there 
could be an `assert(Ty->isSizelessVectorType())` ?  

Note that we're not guaranteed to come to ExprConstant.cpp in the case where it 
is a sized-vector, so you likely need to make sure the CGExprScalar code 
handles sized types as well.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > Also, needs a release note.
> 
> How do I do this? I cannot seem to find documentation on the process...

See docs/ReleaseNotes.rst.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > Also, needs a release note.
> 
> How do I do this? I cannot seem to find documentation on the process...

See docs/ReleaseNotes.rst.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > Also, needs a release note.
> 
> How do I do this? I cannot seem to find documentation on the process...

See docs/ReleaseNotes.rst.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Sema] Use original template pattern when declaring implicit deduction guides for nested template classes (PR #68379)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/68379
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Support target attr specifying CPU (PR #68678)

[Erich Keane via cfe-commits]

@@ -560,11 +560,12 @@ ParsedTargetAttr TargetInfo::parseTargetAttr(StringRef 
Features) const {
 }
 
 // While we're here iterating check for a different target cpu.
-if (Feature.startswith("arch=")) {
+if (Feature.startswith("arch=") || Feature.startswith("cpu=")) {

erichkeane wrote:

We shouldn't be allowing 'cpu' in x86/x86-64 mode, as this is supposed to be 
compatible with GCC and I don't see value in it as a non-compatible extension.

https://github.com/llvm/llvm-project/pull/68678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Support target attr specifying CPU (PR #68678)

[Erich Keane via cfe-commits]

@@ -560,11 +560,12 @@ ParsedTargetAttr TargetInfo::parseTargetAttr(StringRef 
Features) const {
 }
 
 // While we're here iterating check for a different target cpu.
-if (Feature.startswith("arch=")) {
+if (Feature.startswith("arch=") || Feature.startswith("cpu=")) {

erichkeane wrote:

We shouldn't be allowing 'cpu' in x86/x86-64 mode, as this is supposed to be 
compatible with GCC and I don't see value in it as a non-compatible extension.

https://github.com/llvm/llvm-project/pull/68678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Support target attr specifying CPU (PR #68678)

[Erich Keane via cfe-commits]

@@ -2420,11 +2420,11 @@ command line.
 
 The current set of options correspond to the existing "subtarget features" for
 the target with or without a "-mno-" in front corresponding to the absence
-of the feature, as well as ``arch="CPU"`` which will change the default "CPU"
-for the function.
+of the feature, as well as ``arch="CPU"`` and ``cpu="CPU"`` which will change
+the default "CPU" for the function.

erichkeane wrote:

```suggestion
the default processor for the function.
```

https://github.com/llvm/llvm-project/pull/68678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Support target attr specifying CPU (PR #68678)

[Erich Keane via cfe-commits]

@@ -2420,11 +2420,11 @@ command line.
 
 The current set of options correspond to the existing "subtarget features" for
 the target with or without a "-mno-" in front corresponding to the absence
-of the feature, as well as ``arch="CPU"`` which will change the default "CPU"
-for the function.
+of the feature, as well as ``arch="CPU"`` and ``cpu="CPU"`` which will change
+the default "CPU" for the function.

erichkeane wrote:

```suggestion
the default processor for the function.
```

https://github.com/llvm/llvm-project/pull/68678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

https://github.com/erichkeane created 
https://github.com/llvm/llvm-project/pull/69244

Out of line class template declaration specializations aren't created at the 
time they have their template arguments checked, so we previously weren't doing 
any amount of work to substitute the constraints before comparison. This 
resulted in the out of line definition's difference in 'depth' causing the 
constraints to compare differently.

This patch corrects that.  Additionally, it handles ClassTemplateDecl when 
collecting template arguments.

Fixes: 61763

>From 021ec9d584dffc6852a777effc50843bd0facaa1 Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Mon, 16 Oct 2023 13:02:14 -0700
Subject: [PATCH] [CONCEPTS]Corrected comparison of constraints with out of
 line CTD

Out of line class template declaration specializations aren't created at the
time they have their template arguments checked, so we previously weren't doing
any amount of work to substitute the constraints before comparison.
This resulted in the out of line definition's difference in 'depth'
causing the constraints to compare differently.

This patch corrects that.  Additionally, it handles ClassTemplateDecl
when collecting template arguments.

Fixes: 61763
---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/include/clang/Sema/Sema.h   | 91 +++
 clang/include/clang/Sema/Template.h   |  4 +-
 clang/lib/Sema/SemaConcept.cpp| 39 
 clang/lib/Sema/SemaTemplate.cpp   | 22 +++--
 clang/lib/Sema/SemaTemplateDeduction.cpp  |  2 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 20 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  9 +-
 .../SemaTemplate/concepts-out-of-line-def.cpp | 37 
 9 files changed, 174 insertions(+), 54 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 68172d5317a13ba..e7ef8d219de2c41 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -359,6 +359,10 @@ Bug Fixes to C++ Support
   reference. Fixes:
   (`#64162 `_)
 
+- Clang now properly compares constraints on an out of line class template
+  declaration definition. Fixes:
+  (`#61763 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 712db0a3dd895d5..7541e087f4ec77f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3816,17 +3816,6 @@ class Sema final {
   // the purposes of [temp.friend] p9.
   bool FriendConstraintsDependOnEnclosingTemplate(const FunctionDecl *FD);
 
-  // Calculates whether two constraint expressions are equal irrespective of a
-  // difference in 'depth'. This takes a pair of optional 'NamedDecl's 'Old' 
and
-  // 'New', which are the "source" of the constraint, since this is necessary
-  // for figuring out the relative 'depth' of the constraint. The depth of the
-  // 'primary template' and the 'instantiated from' templates aren't 
necessarily
-  // the same, such as a case when one is a 'friend' defined in a class.
-  bool AreConstraintExpressionsEqual(const NamedDecl *Old,
- const Expr *OldConstr,
- const NamedDecl *New,
- const Expr *NewConstr);
-
   enum class AllowedExplicit {
 /// Allow no explicit functions to be used.
 None,
@@ -8590,8 +8579,48 @@ class Sema final {
 TPL_TemplateParamsEquivalent,
   };
 
+  // A struct to represent the 'new' declaration, which is either itself just
+  // the named decl, or the important information we need about it in order to
+  // do constraint comparisions.
+  class TemplateCompareNewDeclInfo {
+const NamedDecl *ND = nullptr;
+const DeclContext *DC = nullptr;
+const DeclContext *LexicalDC = nullptr;
+SourceLocation Loc;
+
+  public:
+TemplateCompareNewDeclInfo(const NamedDecl *ND) : ND(ND) {}
+TemplateCompareNewDeclInfo(const DeclContext *DeclCtx,
+   const DeclContext *LexicalDeclCtx,
+   SourceLocation Loc)
+
+: DC(DeclCtx), LexicalDC(LexicalDeclCtx), Loc(Loc) {
+  assert(DC && LexicalDC &&
+ "Constructor only for cases where we have the information to put "
+ "in here");
+}
+
+// If this was constructed with no information, we cannot do substitution
+// for constraint comparison, so make sure we can check that.
+bool isInvalid() const { return !ND && !DC; }
+
+const NamedDecl *getDecl() const { return ND; }
+
+bool ContainsDecl(const NamedDecl *ND) const { return this->ND == ND; }
+
+const DeclContext *getLexicalDeclContext() const {
+  return ND ? ND->getLexicalDeclContext() : LexicalDC;
+}

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/69244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

https://github.com/erichkeane updated 
https://github.com/llvm/llvm-project/pull/69244

>From 021ec9d584dffc6852a777effc50843bd0facaa1 Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Mon, 16 Oct 2023 13:02:14 -0700
Subject: [PATCH 1/2] [CONCEPTS]Corrected comparison of constraints with out of
 line CTD

Out of line class template declaration specializations aren't created at the
time they have their template arguments checked, so we previously weren't doing
any amount of work to substitute the constraints before comparison.
This resulted in the out of line definition's difference in 'depth'
causing the constraints to compare differently.

This patch corrects that.  Additionally, it handles ClassTemplateDecl
when collecting template arguments.

Fixes: 61763
---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/include/clang/Sema/Sema.h   | 91 +++
 clang/include/clang/Sema/Template.h   |  4 +-
 clang/lib/Sema/SemaConcept.cpp| 39 
 clang/lib/Sema/SemaTemplate.cpp   | 22 +++--
 clang/lib/Sema/SemaTemplateDeduction.cpp  |  2 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 20 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  9 +-
 .../SemaTemplate/concepts-out-of-line-def.cpp | 37 
 9 files changed, 174 insertions(+), 54 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 68172d5317a13ba..e7ef8d219de2c41 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -359,6 +359,10 @@ Bug Fixes to C++ Support
   reference. Fixes:
   (`#64162 `_)
 
+- Clang now properly compares constraints on an out of line class template
+  declaration definition. Fixes:
+  (`#61763 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 712db0a3dd895d5..7541e087f4ec77f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3816,17 +3816,6 @@ class Sema final {
   // the purposes of [temp.friend] p9.
   bool FriendConstraintsDependOnEnclosingTemplate(const FunctionDecl *FD);
 
-  // Calculates whether two constraint expressions are equal irrespective of a
-  // difference in 'depth'. This takes a pair of optional 'NamedDecl's 'Old' 
and
-  // 'New', which are the "source" of the constraint, since this is necessary
-  // for figuring out the relative 'depth' of the constraint. The depth of the
-  // 'primary template' and the 'instantiated from' templates aren't 
necessarily
-  // the same, such as a case when one is a 'friend' defined in a class.
-  bool AreConstraintExpressionsEqual(const NamedDecl *Old,
- const Expr *OldConstr,
- const NamedDecl *New,
- const Expr *NewConstr);
-
   enum class AllowedExplicit {
 /// Allow no explicit functions to be used.
 None,
@@ -8590,8 +8579,48 @@ class Sema final {
 TPL_TemplateParamsEquivalent,
   };
 
+  // A struct to represent the 'new' declaration, which is either itself just
+  // the named decl, or the important information we need about it in order to
+  // do constraint comparisions.
+  class TemplateCompareNewDeclInfo {
+const NamedDecl *ND = nullptr;
+const DeclContext *DC = nullptr;
+const DeclContext *LexicalDC = nullptr;
+SourceLocation Loc;
+
+  public:
+TemplateCompareNewDeclInfo(const NamedDecl *ND) : ND(ND) {}
+TemplateCompareNewDeclInfo(const DeclContext *DeclCtx,
+   const DeclContext *LexicalDeclCtx,
+   SourceLocation Loc)
+
+: DC(DeclCtx), LexicalDC(LexicalDeclCtx), Loc(Loc) {
+  assert(DC && LexicalDC &&
+ "Constructor only for cases where we have the information to put "
+ "in here");
+}
+
+// If this was constructed with no information, we cannot do substitution
+// for constraint comparison, so make sure we can check that.
+bool isInvalid() const { return !ND && !DC; }
+
+const NamedDecl *getDecl() const { return ND; }
+
+bool ContainsDecl(const NamedDecl *ND) const { return this->ND == ND; }
+
+const DeclContext *getLexicalDeclContext() const {
+  return ND ? ND->getLexicalDeclContext() : LexicalDC;
+}
+
+const DeclContext *getDeclContext() const {
+  return ND ? ND->getDeclContext() : DC;
+}
+
+SourceLocation getLocation() const { return ND ? ND->getLocation() : Loc; }
+  };
+
   bool TemplateParameterListsAreEqual(
-  const NamedDecl *NewInstFrom, TemplateParameterList *New,
+  const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList 
*New,
   const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
 

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

https://github.com/erichkeane updated 
https://github.com/llvm/llvm-project/pull/69244

>From 810d49553eac0e42697283b008810cdc37971dd0 Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Mon, 16 Oct 2023 13:02:14 -0700
Subject: [PATCH 1/2] [CONCEPTS]Corrected comparison of constraints with out of
 line CTD

Out of line class template declaration specializations aren't created at the
time they have their template arguments checked, so we previously weren't doing
any amount of work to substitute the constraints before comparison.
This resulted in the out of line definition's difference in 'depth'
causing the constraints to compare differently.

This patch corrects that.  Additionally, it handles ClassTemplateDecl
when collecting template arguments.

Fixes: 61763
---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/include/clang/Sema/Sema.h   | 91 +++
 clang/include/clang/Sema/Template.h   |  4 +-
 clang/lib/Sema/SemaConcept.cpp| 39 
 clang/lib/Sema/SemaTemplate.cpp   | 22 +++--
 clang/lib/Sema/SemaTemplateDeduction.cpp  |  2 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 20 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  9 +-
 .../SemaTemplate/concepts-out-of-line-def.cpp | 37 
 9 files changed, 174 insertions(+), 54 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ff66d2c272098c8..dbe9604f299190a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -518,6 +518,10 @@ Bug Fixes to C++ Support
   (`#46200 `_)
   (`#57812 `_)
 
+- Clang now properly compares constraints on an out of line class template
+  declaration definition. Fixes:
+  (`#61763 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 250ac33680cdbc7..71c43d2d2d2b319 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3809,17 +3809,6 @@ class Sema final {
   // the purposes of [temp.friend] p9.
   bool FriendConstraintsDependOnEnclosingTemplate(const FunctionDecl *FD);
 
-  // Calculates whether two constraint expressions are equal irrespective of a
-  // difference in 'depth'. This takes a pair of optional 'NamedDecl's 'Old' 
and
-  // 'New', which are the "source" of the constraint, since this is necessary
-  // for figuring out the relative 'depth' of the constraint. The depth of the
-  // 'primary template' and the 'instantiated from' templates aren't 
necessarily
-  // the same, such as a case when one is a 'friend' defined in a class.
-  bool AreConstraintExpressionsEqual(const NamedDecl *Old,
- const Expr *OldConstr,
- const NamedDecl *New,
- const Expr *NewConstr);
-
   enum class AllowedExplicit {
 /// Allow no explicit functions to be used.
 None,
@@ -8615,8 +8604,48 @@ class Sema final {
 TPL_TemplateParamsEquivalent,
   };
 
+  // A struct to represent the 'new' declaration, which is either itself just
+  // the named decl, or the important information we need about it in order to
+  // do constraint comparisions.
+  class TemplateCompareNewDeclInfo {
+const NamedDecl *ND = nullptr;
+const DeclContext *DC = nullptr;
+const DeclContext *LexicalDC = nullptr;
+SourceLocation Loc;
+
+  public:
+TemplateCompareNewDeclInfo(const NamedDecl *ND) : ND(ND) {}
+TemplateCompareNewDeclInfo(const DeclContext *DeclCtx,
+   const DeclContext *LexicalDeclCtx,
+   SourceLocation Loc)
+
+: DC(DeclCtx), LexicalDC(LexicalDeclCtx), Loc(Loc) {
+  assert(DC && LexicalDC &&
+ "Constructor only for cases where we have the information to put "
+ "in here");
+}
+
+// If this was constructed with no information, we cannot do substitution
+// for constraint comparison, so make sure we can check that.
+bool isInvalid() const { return !ND && !DC; }
+
+const NamedDecl *getDecl() const { return ND; }
+
+bool ContainsDecl(const NamedDecl *ND) const { return this->ND == ND; }
+
+const DeclContext *getLexicalDeclContext() const {
+  return ND ? ND->getLexicalDeclContext() : LexicalDC;
+}
+
+const DeclContext *getDeclContext() const {
+  return ND ? ND->getDeclContext() : DC;
+}
+
+SourceLocation getLocation() const { return ND ? ND->getLocation() : Loc; }
+  };
+
   bool TemplateParameterListsAreEqual(
-  const NamedDecl *NewInstFrom, TemplateParameterList *New,
+  const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList 
*New,
   const NamedDecl *OldInstFrom

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

erichkeane wrote:

I did a force-push with the hope that it'll get the code-formatter to start 
recognizing what is going on, it seems to be doing some wacky things?

https://github.com/llvm/llvm-project/pull/69244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

@@ -346,6 +351,11 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 CurDecl = Response::UseNextDecl(ND).NextDecl;
   }
 
+  if (!ND) {
+assert(DC);

erichkeane wrote:

Probably not necessary, I'll remove it.  I think I added that top assert later 
on.

https://github.com/llvm/llvm-project/pull/69244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

https://github.com/erichkeane updated 
https://github.com/llvm/llvm-project/pull/69244

>From 810d49553eac0e42697283b008810cdc37971dd0 Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Mon, 16 Oct 2023 13:02:14 -0700
Subject: [PATCH 1/3] [CONCEPTS]Corrected comparison of constraints with out of
 line CTD

Out of line class template declaration specializations aren't created at the
time they have their template arguments checked, so we previously weren't doing
any amount of work to substitute the constraints before comparison.
This resulted in the out of line definition's difference in 'depth'
causing the constraints to compare differently.

This patch corrects that.  Additionally, it handles ClassTemplateDecl
when collecting template arguments.

Fixes: 61763
---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/include/clang/Sema/Sema.h   | 91 +++
 clang/include/clang/Sema/Template.h   |  4 +-
 clang/lib/Sema/SemaConcept.cpp| 39 
 clang/lib/Sema/SemaTemplate.cpp   | 22 +++--
 clang/lib/Sema/SemaTemplateDeduction.cpp  |  2 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 20 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  9 +-
 .../SemaTemplate/concepts-out-of-line-def.cpp | 37 
 9 files changed, 174 insertions(+), 54 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ff66d2c272098c8..dbe9604f299190a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -518,6 +518,10 @@ Bug Fixes to C++ Support
   (`#46200 `_)
   (`#57812 `_)
 
+- Clang now properly compares constraints on an out of line class template
+  declaration definition. Fixes:
+  (`#61763 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 250ac33680cdbc7..71c43d2d2d2b319 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3809,17 +3809,6 @@ class Sema final {
   // the purposes of [temp.friend] p9.
   bool FriendConstraintsDependOnEnclosingTemplate(const FunctionDecl *FD);
 
-  // Calculates whether two constraint expressions are equal irrespective of a
-  // difference in 'depth'. This takes a pair of optional 'NamedDecl's 'Old' 
and
-  // 'New', which are the "source" of the constraint, since this is necessary
-  // for figuring out the relative 'depth' of the constraint. The depth of the
-  // 'primary template' and the 'instantiated from' templates aren't 
necessarily
-  // the same, such as a case when one is a 'friend' defined in a class.
-  bool AreConstraintExpressionsEqual(const NamedDecl *Old,
- const Expr *OldConstr,
- const NamedDecl *New,
- const Expr *NewConstr);
-
   enum class AllowedExplicit {
 /// Allow no explicit functions to be used.
 None,
@@ -8615,8 +8604,48 @@ class Sema final {
 TPL_TemplateParamsEquivalent,
   };
 
+  // A struct to represent the 'new' declaration, which is either itself just
+  // the named decl, or the important information we need about it in order to
+  // do constraint comparisions.
+  class TemplateCompareNewDeclInfo {
+const NamedDecl *ND = nullptr;
+const DeclContext *DC = nullptr;
+const DeclContext *LexicalDC = nullptr;
+SourceLocation Loc;
+
+  public:
+TemplateCompareNewDeclInfo(const NamedDecl *ND) : ND(ND) {}
+TemplateCompareNewDeclInfo(const DeclContext *DeclCtx,
+   const DeclContext *LexicalDeclCtx,
+   SourceLocation Loc)
+
+: DC(DeclCtx), LexicalDC(LexicalDeclCtx), Loc(Loc) {
+  assert(DC && LexicalDC &&
+ "Constructor only for cases where we have the information to put "
+ "in here");
+}
+
+// If this was constructed with no information, we cannot do substitution
+// for constraint comparison, so make sure we can check that.
+bool isInvalid() const { return !ND && !DC; }
+
+const NamedDecl *getDecl() const { return ND; }
+
+bool ContainsDecl(const NamedDecl *ND) const { return this->ND == ND; }
+
+const DeclContext *getLexicalDeclContext() const {
+  return ND ? ND->getLexicalDeclContext() : LexicalDC;
+}
+
+const DeclContext *getDeclContext() const {
+  return ND ? ND->getDeclContext() : DC;
+}
+
+SourceLocation getLocation() const { return ND ? ND->getLocation() : Loc; }
+  };
+
   bool TemplateParameterListsAreEqual(
-  const NamedDecl *NewInstFrom, TemplateParameterList *New,
+  const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList 
*New,
   const NamedDecl *OldInstFrom

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -3083,6 +3083,10 @@ ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
 E->getTypeOfArgument()->getPointeeType()))
 .getQuantity();
 return llvm::ConstantInt::get(CGF.SizeTy, Alignment);
+  } else if (E->getKind() == UETT_VectorElements) {
+auto *VecTy =
+dyn_cast(ConvertType(E->getTypeOfArgument()));

erichkeane wrote:

```suggestion
cast(ConvertType(E->getTypeOfArgument()));
```
You immediately dereference this anyway, and I think the semantics guarantees 
this is the case, correct?

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

2 more nits, else LGTM.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -3083,6 +3083,10 @@ ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
 E->getTypeOfArgument()->getPointeeType()))
 .getQuantity();
 return llvm::ConstantInt::get(CGF.SizeTy, Alignment);
+  } else if (E->getKind() == UETT_VectorElements) {
+auto *VecTy =
+dyn_cast(ConvertType(E->getTypeOfArgument()));

erichkeane wrote:

```suggestion
cast(ConvertType(E->getTypeOfArgument()));
```
You immediately dereference this anyway, and I think the semantics guarantees 
this is the case, correct?

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -3083,6 +3083,10 @@ ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
 E->getTypeOfArgument()->getPointeeType()))
 .getQuantity();
 return llvm::ConstantInt::get(CGF.SizeTy, Alignment);
+  } else if (E->getKind() == UETT_VectorElements) {
+auto *VecTy =
+dyn_cast(ConvertType(E->getTypeOfArgument()));

erichkeane wrote:

```suggestion
cast(ConvertType(E->getTypeOfArgument()));
```
You immediately dereference this anyway, and I think the semantics guarantees 
this is the case, correct?

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Check features of tune CPU against target CPU (PR #68861)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

1st pass through looks fine, but someone more knowledgable about powerPC and 
these compile options are likely better reviewers than I am.

https://github.com/llvm/llvm-project/pull/68861
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

I'm good with it if the pre-commit is, and no one comes out and really needs 
the mangling done in this patch.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

@@ -5126,6 +5126,14 @@ void CXXNameMangler::mangleExpression(const Expr *E, 
unsigned Arity,
   Diags.Report(DiagID);
   return;
 }
+case UETT_VectorElements: {

erichkeane wrote:

OK, SGTM at least, I just know this pattern of 'cannot yet mangle...' shows up 
primarily in the MicrosoftMangle, so figured it might need to be covered.  
There is some funny-business as to how much each mangles of expressions in 
template arguments.

This gets me thinking further though,the constexprness of this likely means you 
may need to mangle this.  I'm open if others are to letting it be done in a 
follow-up however.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Add __builtin_vectorelements to get number of elements in vector (PR #69010)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

I'm good with it if the pre-commit is, and no one comes out and really needs 
the mangling done in this patch.

https://github.com/llvm/llvm-project/pull/69010
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

erichkeane wrote:

My main two reviewers are away this week, and the review I got caught the 
'dumb' stuff I'm likely to miss anyway.  So I'm going to merge this to give it 
more time to bake, and count on RAC.

https://github.com/llvm/llvm-project/pull/69244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)

[Erich Keane via cfe-commits]

https://github.com/erichkeane closed 
https://github.com/llvm/llvm-project/pull/69244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::debug_info_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > > > Wouldn't it be better to go the other way around? i.e. have a 
> > > > `[[clang::compressed_bitfield]]` (or whatever) which influences the 
> > > > ABI, so it's possible to do stuff like
> > > > ```
> > > > [[clang::compressed_bitfield]] bool IsSomething : 1;
> > > > [[clang::compressed_bitfield]] MyEnum Whatever : 3;
> > > > [[clang::compressed_bitfield]] int MoreStuff : 4;
> > > > ```
> > > > 
> > > > 
> > > > which the current approach doesn't allow.
> > > 
> > > 
> > > The issue is that MSVC wouldn't know/recognize/implement this attribute - 
> > > so clang couldn't do that without breaking ABI compatibility with MSVC.
> > 
> > 
> > That's not important for everybody, and you could still `#if` around that 
> > code like
> > ```
> > #if __has_cpp_attribute(clang::compressed_bitfield)
> > #define BITFILED_T(ActualType, ABIType) 
> > static_assert(check_abi_compat()); 
> > [[clang::compressed_bitfield]] ActualType
> > #else
> > #define BITFIELD_T(ActualType, ABIType) ABIType
> > ...
> > ```
> > 
> > 
> > Sure, it's not super pretty, but it gets you there and allows you to do 
> > more stuff. We could also ask the MSVC folks whether there would be 
> > interest to add such an attribute, since there is clearly interest in 
> > having a good ABI and stronger typing (CC @StephanTLavavej @CaseyCarter).
> 
> I think that's a somewhat orthogonal feature to the one @Endilll is 
> proposing. While it might make sense to ask the MSVC and GCC folks if they'd 
> be willing to entertain a similar attribute, the implementation landscape is 
> much wider in C and those implementations may have similar ABI issues. If an 
> implementation ignores the `compressed_bitfield` attribute, you get potential 
> ABI issues; if an implementation ignores the `debug_info_type` attribute, you 
> get the default debugging experience but no change to codegen behavior. So I 
> think there may be room for both features.
> 
> We have a `preferred_name` attribute already for specifying "please display 
> this name instead of that name" in diagnostics; this seems somewhat similar 
> in nature in that it's "please display this type instead of that bit-field 
> type" in the debugger, so mayyybe we want to name this `preferred_type` 
> instead? If this could also be useful for diagnostics which mention types and 
> can be triggered with a bit-field, that might also be a reason to use a 
> different name (I'm not certain we have many/any such diagnostics today 
> though.) I'm not opposed to the current name, but more thinking that it might 
> be too specific and we may want a path to more generalization in the future.

While I agree that a 'pack-at-the-bit-level' type attribute might be valuable, 
I too see that as a 'different' problem.  There is plenty of code (even in 
Clang itself) where and Enum is being stored as a bitfield, and more 
interesting debugging information could be generated for it.  I don't mind the 
name, I think there IS a nice level of symmetry to `preferred_type`, which I 
think ALSO has the advantage of being useful (by name!) for non-debug 
diagnostics.

One thing I REALLY hate about our pattern of using bitfields for enums, is that 
it becomes pretty trivial to add an entry to an enum and forget to increase the 
size of the bitfield.  I think such a attribute could cause a diagnostic of 
`bitfield says it holds an ENUM type, but can't store all the values`, which 
would be INCREDIBLY valuable (not that I'm pushing that this patch IMPLEMENT 
that, but more that I'm saying such a name COULD do that, and probably should 
as well).

So in conclusion, I don't mind the name, but also prefer something more 
general.  

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::debug_info_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

@@ -107,6 +107,10 @@ def NonBitField : SubsetSubjectisBitField()}],
 "non-bit-field non-static data members">;
 
+def BitField : SubsetSubjectisBitField()}],
+ "bit-field non-static data members">;

erichkeane wrote:

I don't know how much this text helps here, the diagnostic specifying 'non 
static' seems a little silly.  The `NonBitField` one is important because it 
applies to non-static data members that ARE NOT bitfields.  But in this case, 
'must be a bitfield' also implies 'non static'.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::debug_info_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

@@ -7219,6 +7219,18 @@ its underlying representation to be a WebAssembly 
``funcref``.
   }];
 }
 
+def DebugInfoTypeDocumentation : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+This attribute allows to alter type of a bitfield in debug information.
+Such a need might arise when bitfield is intended to store an enumeration 
value,
+but has to be specified as having enumeration's underlying type, in order to
+facilitate compiler optimizations. But this also causes underlying type to be
+emitted in debug information, making it hard for debuggers to map bitfield's
+value back to enumeration. This attribute helps with this.
+  }];

erichkeane wrote:

This whole bit of documentation reads REALLY awkwardly and has a couple of 
sentences that are run-on or not-full sentences.  AS a strawman:

"
Attribute `[[]]` provides a mechanism to alter the type in 
debug information of a bitfield.  This is useful as it enables a better debug 
experience in cases where, for layout or other performance purposes, an 
enumeration value is best represented in a structure as a bitfield. By changing 
the type in the debug information, the layout is untouched, but debuggers are 
able to map the bitfield's values back to the enumeration values they represent.
"

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::debug_info_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

@@ -7219,6 +7219,18 @@ its underlying representation to be a WebAssembly 
``funcref``.
   }];
 }
 
+def DebugInfoTypeDocumentation : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+This attribute allows to alter type of a bitfield in debug information.
+Such a need might arise when bitfield is intended to store an enumeration 
value,
+but has to be specified as having enumeration's underlying type, in order to
+facilitate compiler optimizations. But this also causes underlying type to be
+emitted in debug information, making it hard for debuggers to map bitfield's
+value back to enumeration. This attribute helps with this.
+  }];

erichkeane wrote:

This seems better than mine FYI.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::debug_info_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > One thing I REALLY hate about our pattern of using bitfields for enums, is 
> > that it becomes pretty trivial to add an entry to an enum and forget to 
> > increase the size of the bitfield. I think such a attribute could cause a 
> > diagnostic of `bitfield says it holds an ENUM type, but can't store all the 
> > values`, which would be INCREDIBLY valuable (not that I'm pushing that this 
> > patch IMPLEMENT that, but more that I'm saying such a name COULD do that, 
> > and probably should as well).
> > So in conclusion, I don't mind the name, but also prefer something more 
> > general.
> 
> Couldn't we add a warning for that in general? Seems like a pretty useful one 
> to me.

Not without SOME sort of psychic-level analysis from Sema.  Without some level 
of mapping of "I mean for this to hold the values of this Enum", we don't have 
the information to do so.  We cannot take advantage of assignments (though a 
similar warning DOES exist in the 'range' checking) since it might be intended 
that the field does some level of 'conversion' between enums.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::debug_info_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

@@ -5910,6 +5910,30 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D,
   D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
 }
 
+static void handleDebugInfoTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!AL.hasParsedType()) {
+S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
+return;
+  }
+
+  TypeSourceInfo *ParmTSI = nullptr;
+  QualType type = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI);
+  assert(ParmTSI && "no type source info for attribute argument");
+
+  if (type->isEnumeralType()) {
+QualType BitfieldType = llvm::cast(D)->getType();
+QualType EnumUnderlyingType =
+type->getAs()->getDecl()->getIntegerType();
+if (EnumUnderlyingType != BitfieldType) {

erichkeane wrote:

> A few things -- I think we should look at the canonical type so that we don't 
> run into issues with typedefs. e.g.,
> 
> ```
> enum E { Zero, One };
> typedef int Foo;
> struct S {
>   [[clang::debug_info_type(E) Foo field : 1;
> };
> ```
> 
> where the enum's underlying type is `int` but the bit-field's type is a 
> typedef.
> 
> Another case we should consider would be whether we want to allow signed 
> unsigned mismatches, as in:
> 
> ```
> enum E { Invalid = -1, ValidValue, OtherValidValue, YetAnotherValidValue };
> struct S {
>   [[clang::debug_info_type(E) unsigned field : 3;
> };
> ```
> 
> and only prevent situations where the specified type is not an enumeration or 
> integral type.

This ALSO makes me wonder if forcing it to be an enum type is necessary.  I can 
see potential value of a "OpaqueValueOfStruct" storage type thing, that perhaps 
we should just 'trust' that the user is doing something sensible with it.

  I think in the signed/unsigned mismatch, we should allow it (thanks to the 
layout rules).  That is, consider:

```
enum SIGNED {Invalid = -1, Valid };
enum UNSIGNED {Valid, INVALID = 0xF };

struct S {
  [[attr_name(SIGNED)]]
  unsigned SignedThing : 4;
  [[attr_name(UNSIGNED)]]
  unsigned UnsignedThing : 4;
};
```
They HAVE to be the same type else they don't get packed into the same byte, 
but you have 1 enum that can represent a negative value.


https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

I think this looks right, but if @shafik would take an additional look, I'd 
appreciate it.

Additionally, this needs a release note.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

The release note needs work, and I'm not completely sure what you mean to say, 
if you try to clarify it, perhaps I can help correct the wording.  Else LGTM.

https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

@@ -396,6 +396,10 @@ Bug Fixes in This Version
   cannot be used with ``Release`` mode builds. (`#68237 
`_).
 - Fix crash in evaluating ``constexpr`` value for invalid template function.
   Fixes (`#68542 `_)
+- Clang will correctly evaluate ``noexcept`` expression with template in 
template
+  method of template record. Fixes

erichkeane wrote:

I'm not sure what you're trying to say here, but `method of template record` 
doesn't really mean anything to me/confuses me.  Can you try again?

https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

The release note needs work, and I'm not completely sure what you mean to say, 
if you try to clarify it, perhaps I can help correct the wording.  Else LGTM.

https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]Avoid to check created local variable multiple time when evaluating (PR #69106)

[Erich Keane via cfe-commits]

erichkeane wrote:

> I test example in branch llvmorg-16.0.6,ExprConstant.cpp also has this 
> assertion but it will not result in crash.So I think you should not remove 
> this assertion and try to find the reason `!Result.isAbsent()` Maybe its 
> valueKind changed somewhere(just my guess).

I agree here, we shouldn't be double-creating variables like this, removing the 
assert doesn't seem right to me.

https://github.com/llvm/llvm-project/pull/69106
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

>I meant warning on something like https://godbolt.org/z/Ma17xjjc5. That 
>doesn't seem like it should be that hard.

Yeah, that one wouldn't be too hard (and isn't a pattern I've ever seen TBH), 
but doesn't solve the problem at hand, which is that it is very common to need 
to use the same type for multiple enum values in bitfields, so folks use 
unsigned, thus the type name is lost.  

>@erichkeane I guess you'd be pretty happy if our enums were declared the 
>following way:

Not really?  That doesn't gain us the 'we must change where this is stored' 
situation like a `preferred_type` attribute would/could.  We MIGHT be able to 
get away with a comment on top of that with a list of places that needs to ALSO 
be changed, but we end up missing similar comments today anyway.  

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > > @erichkeane I guess you'd be pretty happy if our enums were declared the 
> > > following way:
> 
> > Not really? That doesn't gain us the 'we must change where this is stored' 
> > situation like a preferred_type attribute would/could. We MIGHT be able to 
> > get away with a comment on top of that with a list of places that needs to 
> > ALSO be changed, but we end up missing similar comments today anyway.
> 
> I meant `unsigned _BitInt(N)` underlying type to complement 
> `preferred_type`'s check that a bit-field is wide enough. Otherwise such a 
> check will have to incorporate heuristics to prevent false-positives for 
> cases like the following:
> 
> https://github.com/llvm/llvm-project/blob/180eae1f1e5a08595ed2278d93f01fb321284649/clang/include/clang/AST/DeclarationName.h#L174-L185
> 
> One might argue that this particular case might be resolved by transition to 
> `PointerIntPair`, which is true. It's just the only case I was able to find. 
> I have a recollection of an enum that ends with `Size` enumerator, possibly 
> TableGen'ed, but I can't find it.

We actually DO a similar kind of diagnostic in one of our assignments already I 
think, which figures out that a value we're storing into doesn't have the 
number of bits required for the 'highest' and 'lowest' value of an Enum (though 
I'm having trouble finding it ATM).  This obviously doesn't 'work' in the case 
of bitmasks, but to the point that it is 'quiet' instead of false-positive.  
I'm just suggesting that we can extend this warning to the declarations of 
bitfield that use this to specify an enumeration type.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

Ah, found it: https://godbolt.org/z/59sc87Y3Y

See how on the assignment to a bitfield we check to make sure the largest value 
of the enum will fit in the bitfield?  I'm saying I want us to do this EARLIER, 
on declaration with this attribute.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > See how on the assignment to a bitfield we check to make sure the largest 
> > value of the enum will fit in the bitfield? I'm saying I want us to do this 
> > EARLIER, on declaration with this attribute.
> 
> I totally do. Thank you for pointing out to `-Wbitfield-enum-conversion`! But 
> you should be able to see in my example above that we need a way to exclude 
> an enumerator from this check (in my example it's `[[clang::non_storable]]` 
> applied to `UncommonNameKindOffset`), otherwise we'll get false-positives.

I don't see why that is necessary?  We don't have anything like that today for 
`-Wbitfield-enum-conversion`?  I have a hard time seeing the value of it, even 
for  `-Wbitfield-enum-conversion`, when does someone have an enum value that 
they don't intend to ever be in the enum?  

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

>In my previous comments 
>(https://github.com/llvm/llvm-project/pull/69104#issuecomment-1771167758, 
>https://github.com/llvm/llvm-project/pull/69104#issuecomment-1771204043) I 
>provided an example of StoredNameKind enum that has enumerators 0 through 8 (9 
>total, 4 bits to store), but we store it in a bit-fields of width 3, because 
>the last enumerator is there to count other enumerators, and is used for 
>offset purposes. If we apply -Wbitfield-enum-conversion to it as-is, we get a 
>false-positive: https://godbolt.org/z/4xPs6qPTY.

I thought that was a contrived example, but I see where it is from now (note 
your example is the related but not-exactly-the-same 
`Wbitfield-constant-conversion`), which brings up an additional concern/thing 
to deal with for the `non_storable` attribute (that is, do we apply it to 
assignments? how about exact-assignments like you've done there?).

That is an odd/awkward pattern even so... I think there is still value to the 
warning as proposed, but I'm now not opposed to using `non_storable` as an 
escape hatch.




https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > which brings up an additional concern/thing to deal with for the 
> > non_storable attribute (that is, do we apply it to assignments? how about 
> > exact-assignments like you've done there?).
> 
> Conservative approach would be to issue diagnostics based on `non_storable` 
> only when LHS of assignment was checked taking it into account. In other 
> words, LHS is a bit-field with `preferred_type`, so `non_storable` enumerator 
> was excluded when computing how many bits are required to store the enum.

In that case, yes, we'd exclude it.  It is the storing issue we have today that 
gets funny.  So:

`my_bitfield = VarOfTypeEnumThatWontFitExceptForNonStorable // Don't diagnose, 
as we don't consider it`
`my_bitfield = EnumThatWontFitExceptForNonStorable::NonStorableItem; // Do we 
diagnose this?`

I'd lean towards "No" in the 2nd case, the person is clear in what they are 
doing there.  They should probably get the existing 
`Wbitfield-constant-conversion` still though.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

>>I'm still thinking my way through a non_storable attribute, but on its face, 
>>it seems like it could be overkill. I suspect (but haven't measured!) that 
>>there is way more code out there that maps enumerations to bit-fields that 
>>expect all members of the enumeration to be possible to store than there is 
>>code that has a sentinel value not expected to be used. I kind of wonder if 
>>the correct answer to those more exceptional situations is "don't use this 
>>attribute, you're not doing a straight mapping in this case." Would it 
>>perhaps make sense to handle the common path first, see what usage experience 
>>finds in the wild, and then consider non_storable in the future?

This is my feeling on it too, and I think the 'proper' way ahead.  I am 
unopposed to a `non_storable` attribute, but I'd want to see more uses where it 
would be needed. 

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > > There's some danger here. _BitInt is a C23 feature as are enumerations 
> > > with a fixed underlying type. Enumerations with a fixed underlying type 
> > > explicitly disallow using a bit-precise integer type as the underlying 
> > > type. See C23 6.7.2.2p4, which says in part, "For all the integer 
> > > constant expressions which make up the values of the enumeration 
> > > constants, there shall be a type capable of representing all the values 
> > > that is a standard or extended signed or unsigned integer type, or char.".
> > 
> > 
> > Thank you for pointing out to this! But it's not clear to me whether this 
> > was an explicit decision to not add bit-precise integers here, or just 
> > nobody cared enough to push for that. Because use case we have on our hands 
> > seems legit to me.
> 
> Explicit decision: 
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3030.htm#design-bit.precise.integers
>  -- we should add that diagnostic at some point. :-D
> 
> The biggest concern I had is that bit-precise integer types do not undergo 
> integer promotions, but enumerations do. So it's unclear what the 
> expectations are in this case. Should `enum Foo : unsigned _BitInt(8);` 
> promote to `int` like `unsigned char` would, or should it not promote like 
> `unsigned _BitInt(8)` would?

Ooof, yeah, this needs to be ill-formed, I'd never thought of that when 
implementing it.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> I filed an issue for this, automatically mentioned above.

Fixed!

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> I'm ignoring signed/unsigned mismatch as @erichkeane and @AaronBallman 
> suggested. The only outstanding aspect is the following diagnostic I added 
> today and haven't received feedback on:
> 
> ```c++
>   [[clang::preferred_type(bool)]] unsigned b4 : 1;
>   [[clang::preferred_type(bool)]] unsigned b5 : 2;
>   // expected-warning@-1 {{bit-field that holds a boolean value should have 
> width of 1 instead of 2}}
> ```

Yeah, I haven't had a good chance to do a huge deep dive on this today, and 
probably won't until later this week.  While I think that warning is accurate, 
I somewhat question the value of the 'bool' as working on this type (as, I'm 
not sure what it really means to put a non-enum here? What types ARE we 
allowing in this now?).  

I think the wording of the diagnostic is perhaps awkward, so we should bikeshed 
that as well, though I'm on the fence about the diagnostic existing at all.  On 
one hand, this would be such a special case, as typically 'too large' for the 
type isn't an issue for any reason.  On the other, the 'bool' case is perhaps 
uniquely an issue.

What about if the person prefers the type be 'char', but has it be a size of 9? 
 THIS is a case where it is reasonable (consider cases where you need to store 
the result of one of those calls to the C library that returns 'an int that is 
either a char or -1').

So I guess I'm really questioning how accurate/error-free we could make that 
diagnostic.  I'm leaning towards "too big of a bitfield should be ok".

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reland "[clang][Sema] Use original template pattern when declaring implicit deduction guides for nested template classes" (PR #69676)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/69676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

>As I mentioned in 
>https://github.com/llvm/llvm-project/pull/69104#discussion_r1365269451, I'm 
>not putting any restrictions on type parameter of the attribute, which makes 
>even more sense for more generic preferred_type.

>But I'm confused by the fact you are raising this question. In 
>https://github.com/llvm/llvm-project/pull/69104#discussion_r1364432624, you 
>wrote:

It was just a question, based on the motivation I was misguided and wanted to 
make sure I had the complete list.  I can see value to leaving it unrestricted.

>I'd like to state upfront that I'm diagnosing only booleans, because I see how 
>it can help maintaining Clang headers. Now, on CHAR_BIT == 8 platform, your 
>example results in 8 bits for value, and 1 bit of padding 
>(http://eel.is/c++draft/class.bit#1.sentence-6). I find such bit-field 
>somewhat strange (why can't padding be declared explicitly via unnamed 
>bit-field?), but maybe I haven't seen enough bit-fields in my life to 
>appreciate it.

No, I'm saying that there ARE cases where the user would want to do something 
like:
` unsigned SomeChar : 9;`

That is because in some interfaces in the C standard library will return an 
integer that is EITHER a CHAR or an error-code.  So if the user is intending to 
capture that, this would be a false positive.  Definitively motivated IMO more 
than a 2 bit `bool`.  But since you're limiting this to `bool`, I'm less 
concerned.

>I wonder if we should treat one-bit bit-fields as if they were bool 
>automatically (e.g., create this attribute implicitly in that case). How often 
>do we expect to see one-bit bit-fields that are arithmetic? I'm sure it 
>happens (to multiply against -1, 0, or 1 depending on the sign of the 
>bit-field, for example), but I expect it to be quite rare compared to 
>bool-like use.

I'd be tentative to do that initially, I think I'd prefer that be a future 
'enhancement' after we evaluate the fall-out of this.  The rest of this patch 
has some significant promise, and I'd like to not hold it up trying to evaluate 
that.


>However, will this actually work in practice in the debugger? If not, perhaps 
>we should limit to just integer and enumeration types for now, leaving the 
>extension for the future.

OOof, yeah, I was initially thinking about something like `Fancy`, but the 
tests I'd need to feel comfortable doing something like that would be pretty 
extensive.  Additionally, anything 'floating point' would be REALLY messy as a 
bitfield anyway.  I like the idea of limiting this to 
`isIntegralOrEnumerationType` (or, perhaps that and `isa` so that we 
support incomplete enum types, just with no diagnostics).

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix designated initializers inside templates (PR #69712)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/69712
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > However, will this actually work in practice in the debugger? If not, 
> > perhaps we should limit to just integer and enumeration types for now, 
> > leaving the extension for the future.
> 
> I composed an example of that:
> 
> ```c++
> struct A {
>   short a1;
>   short a2;
> };
> 
> struct B {
>   [[clang::preferred_type(A)]] unsigned b1 : 32 = 0x000F'000C;
> };
> 
> int main()
> {
> B b;
> return b.b1;
> }
> ```
> 
> Nightly build of LLDB from apt.llvm.org handles it just fine:
> 
> ```
> Process 2755547 stopped
> * thread #1, name = 'test-preferred-', stop reason = step in
> frame #0: 0x5148 test-preferred-type`main at test.cxx:13:14
>10   int main()
>11   {
>12   B b;
> -> 13   return b.b1;
>14   }
> (lldb) v -T
> (B) b = {
>   (A:32) b1 = {
> (short) a1 = 12
> (short) a2 = 15
>   }
> }
> ```
> 
> @erichkeane Would you still prefer restricting type argument to integral and 
> enumeration types?

I think that is the least dangerous, but the `requireCompleteType` isn't awful 
(and I can do a disagree & commit on).  I think it gets goofy when the sizes 
don't match with floating point/complex/etc, but I also see "you got what you 
asked for!" as being a reasonable defense to that.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] use `StringSet::insert(iter, iter)` instead for loop to insert (PR #69851)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

Ends up being completely NFC, new form of insert is just the same loop.

https://github.com/llvm/llvm-project/pull/69851
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Add clang::preferred_type attribute for bitfields (PR #69104)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

LGTM, i think we're in an acceptable way forward, particularly on diagnostics.

https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

Alright, looks like @shafik isn't going to review this, so I'll just have to 
accept it.  LGTM.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

@@ -455,6 +455,10 @@ Bug Fixes in This Version
   cannot be used with ``Release`` mode builds. (`#68237 
`_).
 - Fix crash in evaluating ``constexpr`` value for invalid template function.
   Fixes (`#68542 `_)
+- Clang will correctly evaluate ``noexcept`` expression for template functions
+  of template classes. Fixes
+  (`#68543 `_,
+  `#42496 `_)

erichkeane wrote:

Does this do anything for: https://github.com/llvm/llvm-project/issues/59827 ?

https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

LGTM, please update the release notes if it fixes that bug too.

https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]Transform uninstantiated ExceptionSpec in TemplateInstantiator (PR #68878)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

LGTM, please update the release notes if it fixes that bug too.

https://github.com/llvm/llvm-project/pull/68878
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C23] Use thread_local semantics (PR #70107)

[Erich Keane via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c23 -emit-llvm -o - %s | 
FileCheck %s
+
+// Ensure that thread_local and _Thread_local emit the same codegen. See
+// https://github.com/llvm/llvm-project/issues/70068 for details.
+
+void func(void) {
+  static thread_local int i = 12;
+  static _Thread_local int j = 13;
+
+  extern thread_local int k;
+  extern thread_local int l;
+
+  (void)k;
+  (void)l;
+}
+
+// CHECK:  @func.i = internal thread_local global i32 12, align 4
+// CHECK-NEXT: @func.j = internal thread_local global i32 13, align 4
+// CHECK-NEXT: @k = external thread_local global i32, align 4
+// CHECK-NEXT: @l = external thread_local global i32, align 4
+
+// CHECK:  define dso_local void @func()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %0 = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 
@k)

erichkeane wrote:

for the %0-3 you probably want to do wildcards, since these can sometimes end 
up with 'names' depending on your config.  The rest I think are all ok.

https://github.com/llvm/llvm-project/pull/70107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C23] Use thread_local semantics (PR #70107)

[Erich Keane via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c23 -emit-llvm -o - %s | 
FileCheck %s
+
+// Ensure that thread_local and _Thread_local emit the same codegen. See
+// https://github.com/llvm/llvm-project/issues/70068 for details.
+
+void func(void) {
+  static thread_local int i = 12;
+  static _Thread_local int j = 13;
+
+  extern thread_local int k;
+  extern thread_local int l;
+
+  (void)k;
+  (void)l;
+}
+
+// CHECK:  @func.i = internal thread_local global i32 12, align 4
+// CHECK-NEXT: @func.j = internal thread_local global i32 13, align 4
+// CHECK-NEXT: @k = external thread_local global i32, align 4
+// CHECK-NEXT: @l = external thread_local global i32, align 4
+
+// CHECK:  define dso_local void @func()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %[[K:.+]] = call align 4 ptr @llvm.threadlocal.address.p0(ptr 
align 4 @k)
+// CHECK-NEXT: %{{.+}} = load i32, ptr %[[K]], align 4

erichkeane wrote:

Probably not worth having anything up to or including the equals sign here, it 
doesn't really add anything.  Same on line 27.

https://github.com/llvm/llvm-project/pull/70107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C23] Use thread_local semantics (PR #70107)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/70107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang]improve diagnosing redefined defaulted constructor with different exception specs (PR #69688)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/69688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reland "[clang][Sema] Use original template pattern when declaring implicit deduction guides for nested template classes" (PR #73087)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/73087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenACC] Implement 'routine' construct parsing (PR #73143)

[Erich Keane via cfe-commits]

https://github.com/erichkeane created 
https://github.com/llvm/llvm-project/pull/73143

The 'routine' construct applies either to a function directly, or, when
provided a name, applies to the function named (and is visible in the
current scope). This patch implements the parsing for this.  The
identifier provided (or Id Expression) is required to be a valid,
declared identifier, though the semantic analysis portion of the Routine
directive will need to enforce it being a function/overload set.

>From c6b9245f7865a99cd5024fc62aca0f496bcce12b Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Tue, 21 Nov 2023 14:04:41 -0800
Subject: [PATCH] [OpenACC] Implement 'routine' construct parsing

The 'routine' construct applies either to a function directly, or, when
provided a name, applies to the function named (and is visible in the
current scope). This patch implements the parsing for this.  The
identifier provided (or Id Expression) is required to be a valid,
declared identifier, though the semantic analysis portion of the Routine
directive will need to enforce it being a function/overload set.
---
 .../clang/Basic/DiagnosticParseKinds.td   |  2 +
 clang/include/clang/Basic/OpenACCKinds.h  |  3 +-
 clang/include/clang/Parse/Parser.h|  5 ++
 clang/lib/Parse/ParseOpenACC.cpp  | 78 ---
 clang/test/ParserOpenACC/parse-constructs.c   | 42 ++
 clang/test/ParserOpenACC/parse-constructs.cpp | 46 +++
 6 files changed, 165 insertions(+), 11 deletions(-)
 create mode 100644 clang/test/ParserOpenACC/parse-constructs.cpp

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 54b5ba6e6414b2d..2fd7165a422859a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1364,6 +1364,8 @@ def warn_pragma_acc_unimplemented_clause_parsing
 def err_acc_invalid_directive
 : Error<"invalid OpenACC directive '%select{%1|%1 %2}0'">;
 def err_acc_missing_directive : Error<"expected OpenACC directive">;
+def err_acc_invalid_open_paren
+: Error<"expected clause-list or newline in OpenACC directive">;
 
 // OpenMP support.
 def warn_pragma_omp_ignored : Warning<
diff --git a/clang/include/clang/Basic/OpenACCKinds.h 
b/clang/include/clang/Basic/OpenACCKinds.h
index cf4bad9ce0cb9ff..1a5bf7e0e831ce3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -55,7 +55,8 @@ enum class OpenACCDirectiveKind {
   Update,
   // FIXME: wait construct.
 
-  // FIXME: routine construct.
+  // Procedure Calls in Compute Regions.
+  Routine,
 
   // Invalid.
   Invalid,
diff --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 465453826c0b982..ca29ce46873e8a4 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3532,9 +3532,14 @@ class Parser : public CodeCompletionHandler {
   /// Placeholder for now, should just ignore the directives after emitting a
   /// diagnostic. Eventually will be split into a few functions to parse
   /// different situations.
+public:
   DeclGroupPtrTy ParseOpenACCDirectiveDecl();
   StmtResult ParseOpenACCDirectiveStmt();
 
+private:
+  void ParseOpenACCDirective();
+  ExprResult ParseOpenACCRoutineName();
+
 private:
   
//======//
   // C++ 14: Templates [temp]
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 978a07ec82e4288..88c2fd68e075d65 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -46,6 +46,7 @@ OpenACCDirectiveKindEx getOpenACCDirectiveKind(StringRef 
Name) {
   .Case("host_data", OpenACCDirectiveKind::HostData)
   .Case("loop", OpenACCDirectiveKind::Loop)
   .Case("atomic", OpenACCDirectiveKind::Atomic)
+  .Case("routine", OpenACCDirectiveKind::Routine)
   .Case("declare", OpenACCDirectiveKind::Declare)
   .Case("init", OpenACCDirectiveKind::Init)
   .Case("shutdown", OpenACCDirectiveKind::Shutdown)
@@ -97,6 +98,8 @@ bool isOpenACCDirectiveKind(OpenACCDirectiveKind Kind, 
StringRef Tok) {
 
   case OpenACCDirectiveKind::Atomic:
 return Tok == "atomic";
+  case OpenACCDirectiveKind::Routine:
+return Tok == "routine";
   case OpenACCDirectiveKind::Declare:
 return Tok == "declare";
   case OpenACCDirectiveKind::Init:
@@ -232,24 +235,79 @@ void ParseOpenACCClauseList(Parser &P) {
 P.Diag(P.getCurToken(), 
diag::warn_pragma_acc_unimplemented_clause_parsing);
 }
 
-void ParseOpenACCDirective(Parser &P) {
-  OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(P);
+} // namespace
+
+// Routine has an optional paren-wrapped name of a function in the local scope.
+// We parse the name, emitting any diagnostics
+ExprResult Parser::ParseOpenACCRoutineName() {
+
+  ExprResult Res;
+  if (get

[clang] [OpenACC] Implement 'routine' construct parsing (PR #73143)

[Erich Keane via cfe-commits]

https://github.com/erichkeane updated 
https://github.com/llvm/llvm-project/pull/73143

>From c6b9245f7865a99cd5024fc62aca0f496bcce12b Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Tue, 21 Nov 2023 14:04:41 -0800
Subject: [PATCH 1/2] [OpenACC] Implement 'routine' construct parsing

The 'routine' construct applies either to a function directly, or, when
provided a name, applies to the function named (and is visible in the
current scope). This patch implements the parsing for this.  The
identifier provided (or Id Expression) is required to be a valid,
declared identifier, though the semantic analysis portion of the Routine
directive will need to enforce it being a function/overload set.
---
 .../clang/Basic/DiagnosticParseKinds.td   |  2 +
 clang/include/clang/Basic/OpenACCKinds.h  |  3 +-
 clang/include/clang/Parse/Parser.h|  5 ++
 clang/lib/Parse/ParseOpenACC.cpp  | 78 ---
 clang/test/ParserOpenACC/parse-constructs.c   | 42 ++
 clang/test/ParserOpenACC/parse-constructs.cpp | 46 +++
 6 files changed, 165 insertions(+), 11 deletions(-)
 create mode 100644 clang/test/ParserOpenACC/parse-constructs.cpp

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 54b5ba6e6414b2d..2fd7165a422859a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1364,6 +1364,8 @@ def warn_pragma_acc_unimplemented_clause_parsing
 def err_acc_invalid_directive
 : Error<"invalid OpenACC directive '%select{%1|%1 %2}0'">;
 def err_acc_missing_directive : Error<"expected OpenACC directive">;
+def err_acc_invalid_open_paren
+: Error<"expected clause-list or newline in OpenACC directive">;
 
 // OpenMP support.
 def warn_pragma_omp_ignored : Warning<
diff --git a/clang/include/clang/Basic/OpenACCKinds.h 
b/clang/include/clang/Basic/OpenACCKinds.h
index cf4bad9ce0cb9ff..1a5bf7e0e831ce3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -55,7 +55,8 @@ enum class OpenACCDirectiveKind {
   Update,
   // FIXME: wait construct.
 
-  // FIXME: routine construct.
+  // Procedure Calls in Compute Regions.
+  Routine,
 
   // Invalid.
   Invalid,
diff --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 465453826c0b982..ca29ce46873e8a4 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3532,9 +3532,14 @@ class Parser : public CodeCompletionHandler {
   /// Placeholder for now, should just ignore the directives after emitting a
   /// diagnostic. Eventually will be split into a few functions to parse
   /// different situations.
+public:
   DeclGroupPtrTy ParseOpenACCDirectiveDecl();
   StmtResult ParseOpenACCDirectiveStmt();
 
+private:
+  void ParseOpenACCDirective();
+  ExprResult ParseOpenACCRoutineName();
+
 private:
   
//======//
   // C++ 14: Templates [temp]
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 978a07ec82e4288..88c2fd68e075d65 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -46,6 +46,7 @@ OpenACCDirectiveKindEx getOpenACCDirectiveKind(StringRef 
Name) {
   .Case("host_data", OpenACCDirectiveKind::HostData)
   .Case("loop", OpenACCDirectiveKind::Loop)
   .Case("atomic", OpenACCDirectiveKind::Atomic)
+  .Case("routine", OpenACCDirectiveKind::Routine)
   .Case("declare", OpenACCDirectiveKind::Declare)
   .Case("init", OpenACCDirectiveKind::Init)
   .Case("shutdown", OpenACCDirectiveKind::Shutdown)
@@ -97,6 +98,8 @@ bool isOpenACCDirectiveKind(OpenACCDirectiveKind Kind, 
StringRef Tok) {
 
   case OpenACCDirectiveKind::Atomic:
 return Tok == "atomic";
+  case OpenACCDirectiveKind::Routine:
+return Tok == "routine";
   case OpenACCDirectiveKind::Declare:
 return Tok == "declare";
   case OpenACCDirectiveKind::Init:
@@ -232,24 +235,79 @@ void ParseOpenACCClauseList(Parser &P) {
 P.Diag(P.getCurToken(), 
diag::warn_pragma_acc_unimplemented_clause_parsing);
 }
 
-void ParseOpenACCDirective(Parser &P) {
-  OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(P);
+} // namespace
+
+// Routine has an optional paren-wrapped name of a function in the local scope.
+// We parse the name, emitting any diagnostics
+ExprResult Parser::ParseOpenACCRoutineName() {
+
+  ExprResult Res;
+  if (getLangOpts().CPlusPlus)
+Res = ParseCXXIdExpression(/*isAddressOfOperand=*/false);
+  else {
+// There isn't anything quite the same as ParseCXXIdExpression for C, so we
+// need to get the identifier, then call into Sema ourselves.
+
+if (expectIdentifier())
+  return ExprError();
+
+Token FuncName = getCurToken();
+UnqualifiedId Name;
+CXXScopeSpec ScopeSpec;
+SourceLoc

[clang] [OpenACC] Implement 'routine' construct parsing (PR #73143)

[Erich Keane via cfe-commits]

@@ -232,32 +235,87 @@ void ParseOpenACCClauseList(Parser &P) {
 P.Diag(P.getCurToken(), 
diag::warn_pragma_acc_unimplemented_clause_parsing);
 }
 
-void ParseOpenACCDirective(Parser &P) {
-  OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(P);
+} // namespace
+
+// Routine has an optional paren-wrapped name of a function in the local scope.
+// We parse the name, emitting any diagnostics
+ExprResult Parser::ParseOpenACCRoutineName() {
+
+  ExprResult Res;
+  if (getLangOpts().CPlusPlus)
+Res = ParseCXXIdExpression(/*isAddressOfOperand=*/false);
+  else {

erichkeane wrote:

Interestingly, when I was rewriting our brace policy a few years ago, nearly 
this exact case was one that people wanted to preserve, so the rule essentially 
became "once you start, use it for the rest, but not before you need to".  

I wasn't a huge proponent of it, and don't mind here, so I've added them.

https://github.com/llvm/llvm-project/pull/73143
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenACC] Implement 'routine' construct parsing (PR #73143)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/73143
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenACC] Implement 'routine' construct parsing (PR #73143)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/73143
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenACC] Implement 'routine' construct parsing (PR #73143)

[Erich Keane via cfe-commits]

https://github.com/erichkeane closed 
https://github.com/llvm/llvm-project/pull/73143
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [clang] report inlining decisions with -Wattribute-{warning|error} (PR #73552)

[Erich Keane via cfe-commits]

@@ -794,6 +796,13 @@ void BackendConsumer::DontCallDiagHandler(const 
DiagnosticInfoDontCall &D) {
   ? diag::err_fe_backend_error_attr
   : diag::warn_fe_backend_warning_attr)
   << llvm::demangle(D.getFunctionName()) << D.getNote();
+
+  SmallVector InliningDecisions = D.getInliningDecisions();

erichkeane wrote:

Could we use `StringRef` here instead?  It seems to make more sense to me.

https://github.com/llvm/llvm-project/pull/73552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang] report inlining decisions with -Wattribute-{warning|error} (PR #73552)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/73552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang] report inlining decisions with -Wattribute-{warning|error} (PR #73552)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

only a few comments on the CFE, most of the work seems LLVM related.

https://github.com/llvm/llvm-project/pull/73552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang] report inlining decisions with -Wattribute-{warning|error} (PR #73552)

[Erich Keane via cfe-commits]

@@ -794,6 +796,13 @@ void BackendConsumer::DontCallDiagHandler(const 
DiagnosticInfoDontCall &D) {
   ? diag::err_fe_backend_error_attr
   : diag::warn_fe_backend_warning_attr)
   << llvm::demangle(D.getFunctionName()) << D.getNote();
+
+  SmallVector InliningDecisions = D.getInliningDecisions();
+  InliningDecisions.push_back(D.getCaller().str());
+  for (auto [index, value] : llvm::enumerate(InliningDecisions))

erichkeane wrote:

```suggestion
  for (const auto &[index, value] : llvm::enumerate(InliningDecisions))
```

Seems we don't modify either, and `value` is a string, so requiring a copy of 
the pair is expensive.

https://github.com/llvm/llvm-project/pull/73552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang] report inlining decisions with -Wattribute-{warning|error} (PR #73552)

[Erich Keane via cfe-commits]

@@ -93,6 +93,8 @@ def err_fe_backend_error_attr :
 def warn_fe_backend_warning_attr :
   Warning<"call to '%0' declared with 'warning' attribute: %1">, BackendInfo,
   InGroup;
+def note_fe_backend_in : Note<"called by function '%0'">;

erichkeane wrote:

Is this the right file for this?  It seems weird to do codegen in a 'frontend 
kinds' diagnostics list, though I'm not convinced I know enough to ask this 
question well enough.

https://github.com/llvm/llvm-project/pull/73552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [clang] report inlining decisions with -Wattribute-{warning|error} (PR #73552)

[Erich Keane via cfe-commits]

@@ -93,6 +93,8 @@ def err_fe_backend_error_attr :
 def warn_fe_backend_warning_attr :
   Warning<"call to '%0' declared with 'warning' attribute: %1">, BackendInfo,
   InGroup;
+def note_fe_backend_in : Note<"called by function '%0'">;

erichkeane wrote:

IDK, that is why I asked.  It SOUNDS wrong, so I'm hopeful someone else can 
come along and tell us.

https://github.com/llvm/llvm-project/pull/73552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Move documentation about -verify from a header to public docs (PR #73694)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

Urgh, I finally set a bookmark on that page, so this is going to break that!

But yeah, this makes a lot more sense here, and I think ends up being easier to 
maintain.  It passes a quick proofread, so lgtm.

https://github.com/llvm/llvm-project/pull/73694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits