[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/96023
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

https://github.com/cor3ntin commented:

Thanks Matheus.
I left a few nitpicky comments.
The design looks good to me I think but it's not a trivial change so Ill want 
to review it more and have other people look at it too.

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -139,7 +139,7 @@ static TemplateDeductionResult 
DeduceTemplateArgumentsByTypeMatch(
 SmallVectorImpl &Deduced, unsigned TDF,
 bool PartialOrdering = false, bool DeducedFromArrayBound = false);
 
-enum class PackFold { ParameterToArgument, ArgumentToParameter };
+enum class PackFold { ParameterToArgument, ArgumentToParameter, Both };

cor3ntin wrote:

Preexisting but this could really use a comment

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -2513,49 +2545,76 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 TemplateDeductionInfo &Info,
 SmallVectorImpl &Deduced,
 bool NumberOfArgumentsMustMatch, PackFold PackFold) {
-  if (PackFold == PackFold::ArgumentToParameter)
-std::swap(Ps, As);
+  bool FoldPackParameter = PackFold == PackFold::ParameterToArgument ||
+   PackFold == PackFold::Both,
+   FoldPackArgument = PackFold == PackFold::ArgumentToParameter ||
+  PackFold == PackFold::Both;
+
   // C++0x [temp.deduct.type]p9:
   //   If the template argument list of P contains a pack expansion that is not
   //   the last template argument, the entire template argument list is a
   //   non-deduced context.
-  if (hasPackExpansionBeforeEnd(Ps))
+  if (FoldPackParameter && hasPackExpansionBeforeEnd(Ps))
+return TemplateDeductionResult::Success;
+
+  if (FoldPackArgument && hasPackExpansionBeforeEnd(As))
 return TemplateDeductionResult::Success;
 
   // C++0x [temp.deduct.type]p9:
   //   If P has a form that contains  or , then each argument Pi of the
   //   respective template argument list P is compared with the corresponding
   //   argument Ai of the corresponding template argument list of A.
-  unsigned ArgIdx = 0, ParamIdx = 0;
-  for (; hasTemplateArgumentForDeduction(Ps, ParamIdx); ++ParamIdx) {
-const TemplateArgument &P = Ps[ParamIdx];
-if (!P.isPackExpansion()) {
+  for (unsigned ArgIdx = 0, ParamIdx = 0; /**/; /**/) {
+if (!hasTemplateArgumentForDeduction(Ps, ParamIdx))
+  return !FoldPackParameter && NumberOfArgumentsMustMatch &&
+ hasTemplateArgumentForDeduction(As, ArgIdx) &&
+ !As[ArgIdx].isPackExpansion()
+ ? TemplateDeductionResult::MiscellaneousDeductionFailure
+ : TemplateDeductionResult::Success;
+
+if (!Ps[ParamIdx].isPackExpansion()) {
   // The simple case: deduce template arguments by matching Pi and Ai.
 
   // Check whether we have enough arguments.
   if (!hasTemplateArgumentForDeduction(As, ArgIdx))
-return NumberOfArgumentsMustMatch
+return !FoldPackArgument && NumberOfArgumentsMustMatch
? TemplateDeductionResult::MiscellaneousDeductionFailure
: TemplateDeductionResult::Success;
 
-  // C++1z [temp.deduct.type]p9:
-  //   During partial ordering, if Ai was originally a pack expansion [and]
-  //   Pi is not a pack expansion, template argument deduction fails.
-  if (As[ArgIdx].isPackExpansion())
-return TemplateDeductionResult::MiscellaneousDeductionFailure;
+  if (As[ArgIdx].isPackExpansion()) {
+// C++1z [temp.deduct.type]p9:
+//   During partial ordering, if Ai was originally a pack expansion
+//   [and] Pi is not a pack expansion, template argument deduction
+//   fails.
+if (!FoldPackArgument)
+  return TemplateDeductionResult::MiscellaneousDeductionFailure;
+
+for (TemplateArgument Pattern = As[ArgIdx].getPackExpansionPattern();
+ /**/;
+ /**/) {

cor3ntin wrote:

Formatting is super weird. We might as well use while loop here (and above)

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -9934,6 +9935,9 @@ class Sema final : public SemaBase {
 
   /// We are instantiating a type alias template declaration.
   TypeAliasTemplateInstantiation,
+
+  /// We are performing partial ordering for template template parameters.
+  PartialOrderTTP,

cor3ntin wrote:

```suggestion
  PartialOrderingTTP,
```

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -3351,14 +3416,18 @@ Sema::DeduceTemplateArgumentsFromType(TemplateDecl *TD, 
QualType FromType,
   if (Inst.isInvalid())
 return TemplateDeductionResult::InstantiationDepth;
 
-  if (Trap.hasErrorOccurred())
-return TemplateDeductionResult::SubstitutionFailure;
-
   TemplateDeductionResult Result;
   runWithSufficientStackSpace(Info.getLocation(), [&] {
 Result = ::FinishTemplateArgumentDeduction(*this, TD, Deduced, Info);
   });
-  return Result;
+
+  if (Result != TemplateDeductionResult::Success)
+return Result;
+
+  if (Trap.hasErrorOccurred())
+return TemplateDeductionResult::SubstitutionFailure;
+
+  return TemplateDeductionResult::Success;

cor3ntin wrote:

Ditto

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -6369,27 +6451,88 @@ bool 
Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   //   be inverted between Ps and As. On non-deduced context, matching needs to
   //   happen both ways, according to [temp.arg.template]p3, but this is
   //   currently implemented as a special case elsewhere.
-  if (::DeduceTemplateArguments(*this, A, AArgs, PArgs, Info, Deduced,
-/*NumberOfArgumentsMustMatch=*/false,
-IsDeduced ? PackFold::ArgumentToParameter
-  : PackFold::ParameterToArgument) !=
-  TemplateDeductionResult::Success)
+  switch (TemplateDeductionResult Result = ::DeduceTemplateArguments(
+  *this, A, AArgs, PArgs, Info, Deduced,
+  /*NumberOfArgumentsMustMatch=*/false,
+  IsDeduced ? PackFold::ArgumentToParameter : PackFold::Both)) {
+  case clang::TemplateDeductionResult::Success:
+break;
+
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+Diag(AArg->getLocation(), diag::err_template_param_list_different_arity)
+<< (A->size() > P->size()) << /*isTemplateTemplateParameter=*/true
+<< SourceRange(A->getTemplateLoc(), P->getRAngleLoc());
+return false;

cor3ntin wrote:

So now i think that either
 - We should use `IncompletePack`, or
 - Introduce a new enumerator

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -3186,20 +3239,36 @@ static TemplateDeductionResult 
FinishTemplateArgumentDeduction(
 
   // Check that we produced the correct argument list.
   TemplateParameterList *TemplateParams = Template->getTemplateParameters();
+  auto notSame = [&](unsigned I, const TemplateArgument &P,
+ const TemplateArgument &A) {
+if (isSameTemplateArg(S.Context, P, A, PartialOrdering,
+  /*PackExpansionMatchesPack=*/true))
+  return false;
+Info.Param = makeTemplateParameter(TemplateParams->getParam(I));
+Info.FirstArg = P;
+Info.SecondArg = A;
+return true;
+  };

cor3ntin wrote:

I'm not a fan of the double negation here

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -6369,27 +6451,88 @@ bool 
Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   //   be inverted between Ps and As. On non-deduced context, matching needs to
   //   happen both ways, according to [temp.arg.template]p3, but this is
   //   currently implemented as a special case elsewhere.
-  if (::DeduceTemplateArguments(*this, A, AArgs, PArgs, Info, Deduced,
-/*NumberOfArgumentsMustMatch=*/false,
-IsDeduced ? PackFold::ArgumentToParameter
-  : PackFold::ParameterToArgument) !=
-  TemplateDeductionResult::Success)
+  switch (TemplateDeductionResult Result = ::DeduceTemplateArguments(
+  *this, A, AArgs, PArgs, Info, Deduced,
+  /*NumberOfArgumentsMustMatch=*/false,
+  IsDeduced ? PackFold::ArgumentToParameter : PackFold::Both)) {
+  case clang::TemplateDeductionResult::Success:
+break;
+
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+Diag(AArg->getLocation(), diag::err_template_param_list_different_arity)
+<< (A->size() > P->size()) << /*isTemplateTemplateParameter=*/true
+<< SourceRange(A->getTemplateLoc(), P->getRAngleLoc());
+return false;
+  case TemplateDeductionResult::NonDeducedMismatch:
+Diag(AArg->getLocation(), diag::err_non_deduced_mismatch)
+<< Info.FirstArg << Info.SecondArg;
+return false;
+  case TemplateDeductionResult::Inconsistent:
+Diag(getAsNamedDecl(Info.Param)->getLocation(),
+ diag::err_inconsistent_deduction)
+<< Info.FirstArg << Info.SecondArg;
+return false;
+  case TemplateDeductionResult::AlreadyDiagnosed:
 return false;
 
+  // None of these should happen for a plain deduction.
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::InstantiationDepth:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::IncompletePack:
+  case TemplateDeductionResult::Underqualified:
+  case TemplateDeductionResult::SubstitutionFailure:
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::InvalidExplicitArguments:
+  case TemplateDeductionResult::NonDependentConversionFailure:
+  case TemplateDeductionResult::ConstraintsNotSatisfied:
+  case TemplateDeductionResult::CUDATargetMismatch:
+llvm_unreachable("Unexpected Result");
+  }
+
   SmallVector DeducedArgs(Deduced.begin(), Deduced.end());
-  Sema::InstantiatingTemplate Inst(*this, Info.getLocation(), AArg, 
DeducedArgs,
-   Info);
-  if (Inst.isInvalid())
-return false;
 
-  bool AtLeastAsSpecialized;
+  TemplateDeductionResult TDK;
   runWithSufficientStackSpace(Info.getLocation(), [&] {
-AtLeastAsSpecialized =
-::FinishTemplateArgumentDeduction(
-*this, AArg, /*IsPartialOrdering=*/true, PArgs, Deduced, Info) ==
-TemplateDeductionResult::Success;
+TDK = ::FinishTemplateArgumentDeduction(
+*this, AArg, /*IsPartialOrdering=*/true, PArgs, Deduced, Info);
   });
-  return AtLeastAsSpecialized;
+  switch (TDK) {
+  case TemplateDeductionResult::Success:
+return true;
+
+  // It doesn't seem possible to get a non-deduced mismatch when partial
+  // ordering TTPs.
+  case TemplateDeductionResult::NonDeducedMismatch:
+llvm_unreachable("Unexpected NonDeducedMismatch");
+
+  // Substitution failures should have already been diagnosed.
+  case TemplateDeductionResult::AlreadyDiagnosed:
+  case TemplateDeductionResult::SubstitutionFailure:
+  case TemplateDeductionResult::InstantiationDepth:
+return false;
+
+  // None of these should happen when just converting deduced arguments.
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::IncompletePack:
+  case TemplateDeductionResult::Inconsistent:
+  case TemplateDeductionResult::Underqualified:
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::InvalidExplicitArguments:
+  case TemplateDeductionResult::NonDependentConversionFailure:
+  case TemplateDeductionResult::ConstraintsNotSatisfied:
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+  case TemplateDeductionResult::CUDATargetMismatch:
+llvm_unreachable("Unexpected Result");

cor3ntin wrote:

Ditto

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -2513,49 +2545,76 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 TemplateDeductionInfo &Info,
 SmallVectorImpl &Deduced,
 bool NumberOfArgumentsMustMatch, PackFold PackFold) {
-  if (PackFold == PackFold::ArgumentToParameter)
-std::swap(Ps, As);
+  bool FoldPackParameter = PackFold == PackFold::ParameterToArgument ||
+   PackFold == PackFold::Both,
+   FoldPackArgument = PackFold == PackFold::ArgumentToParameter ||
+  PackFold == PackFold::Both;
+
   // C++0x [temp.deduct.type]p9:
   //   If the template argument list of P contains a pack expansion that is not
   //   the last template argument, the entire template argument list is a
   //   non-deduced context.
-  if (hasPackExpansionBeforeEnd(Ps))
+  if (FoldPackParameter && hasPackExpansionBeforeEnd(Ps))
+return TemplateDeductionResult::Success;
+
+  if (FoldPackArgument && hasPackExpansionBeforeEnd(As))
 return TemplateDeductionResult::Success;
 
   // C++0x [temp.deduct.type]p9:
   //   If P has a form that contains  or , then each argument Pi of the
   //   respective template argument list P is compared with the corresponding
   //   argument Ai of the corresponding template argument list of A.
-  unsigned ArgIdx = 0, ParamIdx = 0;
-  for (; hasTemplateArgumentForDeduction(Ps, ParamIdx); ++ParamIdx) {
-const TemplateArgument &P = Ps[ParamIdx];
-if (!P.isPackExpansion()) {
+  for (unsigned ArgIdx = 0, ParamIdx = 0; /**/; /**/) {
+if (!hasTemplateArgumentForDeduction(Ps, ParamIdx))
+  return !FoldPackParameter && NumberOfArgumentsMustMatch &&
+ hasTemplateArgumentForDeduction(As, ArgIdx) &&
+ !As[ArgIdx].isPackExpansion()
+ ? TemplateDeductionResult::MiscellaneousDeductionFailure

cor3ntin wrote:

Should that be `IncompletePack` (rather than `MiscellaneousDeductionFailure`)?

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -2513,49 +2545,76 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 TemplateDeductionInfo &Info,
 SmallVectorImpl &Deduced,
 bool NumberOfArgumentsMustMatch, PackFold PackFold) {
-  if (PackFold == PackFold::ArgumentToParameter)
-std::swap(Ps, As);
+  bool FoldPackParameter = PackFold == PackFold::ParameterToArgument ||
+   PackFold == PackFold::Both,
+   FoldPackArgument = PackFold == PackFold::ArgumentToParameter ||
+  PackFold == PackFold::Both;
+
   // C++0x [temp.deduct.type]p9:
   //   If the template argument list of P contains a pack expansion that is not
   //   the last template argument, the entire template argument list is a
   //   non-deduced context.
-  if (hasPackExpansionBeforeEnd(Ps))
+  if (FoldPackParameter && hasPackExpansionBeforeEnd(Ps))
+return TemplateDeductionResult::Success;
+
+  if (FoldPackArgument && hasPackExpansionBeforeEnd(As))
 return TemplateDeductionResult::Success;
 
   // C++0x [temp.deduct.type]p9:
   //   If P has a form that contains  or , then each argument Pi of the
   //   respective template argument list P is compared with the corresponding
   //   argument Ai of the corresponding template argument list of A.
-  unsigned ArgIdx = 0, ParamIdx = 0;
-  for (; hasTemplateArgumentForDeduction(Ps, ParamIdx); ++ParamIdx) {
-const TemplateArgument &P = Ps[ParamIdx];
-if (!P.isPackExpansion()) {
+  for (unsigned ArgIdx = 0, ParamIdx = 0; /**/; /**/) {
+if (!hasTemplateArgumentForDeduction(Ps, ParamIdx))
+  return !FoldPackParameter && NumberOfArgumentsMustMatch &&
+ hasTemplateArgumentForDeduction(As, ArgIdx) &&
+ !As[ArgIdx].isPackExpansion()
+ ? TemplateDeductionResult::MiscellaneousDeductionFailure
+ : TemplateDeductionResult::Success;
+
+if (!Ps[ParamIdx].isPackExpansion()) {
   // The simple case: deduce template arguments by matching Pi and Ai.
 
   // Check whether we have enough arguments.
   if (!hasTemplateArgumentForDeduction(As, ArgIdx))
-return NumberOfArgumentsMustMatch
+return !FoldPackArgument && NumberOfArgumentsMustMatch
? TemplateDeductionResult::MiscellaneousDeductionFailure

cor3ntin wrote:

Should that be `IncompletePack` (rather than `MiscellaneousDeductionFailure`)?

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -3284,16 +3345,20 @@ DeduceTemplateArguments(Sema &S, T *Partial,
   if (Inst.isInvalid())
 return TemplateDeductionResult::InstantiationDepth;
 
-  if (Trap.hasErrorOccurred())
-return TemplateDeductionResult::SubstitutionFailure;
-
   TemplateDeductionResult Result;
   S.runWithSufficientStackSpace(Info.getLocation(), [&] {
 Result = ::FinishTemplateArgumentDeduction(S, Partial,
/*IsPartialOrdering=*/false,
TemplateArgs, Deduced, Info);
   });
-  return Result;
+
+  if (Result != TemplateDeductionResult::Success)
+return Result;
+
+  if (Trap.hasErrorOccurred())
+return TemplateDeductionResult::SubstitutionFailure;
+
+  return TemplateDeductionResult::Success;

cor3ntin wrote:

can that be simplified to 

```cpp
if (Trap.hasErrorOccurred())
return TemplateDeductionResult::SubstitutionFailure;
return Result;
```

?

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -6038,14 +6107,23 @@ static bool isAtLeastAsSpecializedAs(Sema &S, QualType 
T1, QualType T2,
 return false;
 
   const auto *TST1 = cast(T1);
-  bool AtLeastAsSpecialized;
+
+  Sema::SFINAETrap Trap(S);
+
+  TemplateDeductionResult Result;
   S.runWithSufficientStackSpace(Info.getLocation(), [&] {
-AtLeastAsSpecialized =
-FinishTemplateArgumentDeduction(
-S, P2, /*IsPartialOrdering=*/true, TST1->template_arguments(),
-Deduced, Info) == TemplateDeductionResult::Success;
+Result = ::FinishTemplateArgumentDeduction(
+S, P2, /*IsPartialOrdering=*/true, TST1->template_arguments(), Deduced,
+Info);
   });
-  return AtLeastAsSpecialized;
+
+  if (Result != TemplateDeductionResult::Success)
+return false;
+
+  if (Trap.hasErrorOccurred())
+return false;
+
+  return true;

cor3ntin wrote:

Ditto

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[via llvm-branch-commits]

@@ -8513,64 +8513,46 @@ bool 
Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
   << Template;
   }
 
+  if (!getLangOpts().RelaxedTemplateTemplateArgs)
+return !TemplateParameterListsAreEqual(
+Template->getTemplateParameters(), Params, /*Complain=*/true,
+TPL_TemplateTemplateArgumentMatch, Arg.getLocation());
+
   // C++1z [temp.arg.template]p3: (DR 150)
   //   A template-argument matches a template template-parameter P when P
   //   is at least as specialized as the template-argument A.
-  if (getLangOpts().RelaxedTemplateTemplateArgs) {
-// Quick check for the common case:
-//   If P contains a parameter pack, then A [...] matches P if each of A's
-//   template parameters matches the corresponding template parameter in
-//   the template-parameter-list of P.
-if (TemplateParameterListsAreEqual(
-Template->getTemplateParameters(), Params, false,
-TPL_TemplateTemplateArgumentMatch, Arg.getLocation()) &&
-// If the argument has no associated constraints, then the parameter is
-// definitely at least as specialized as the argument.
-// Otherwise - we need a more thorough check.
-!Template->hasAssociatedConstraints())
-  return false;
-
-if (isTemplateTemplateParameterAtLeastAsSpecializedAs(
-Params, Template, DefaultArgs, Arg.getLocation(), IsDeduced)) {
-  // P2113
-  // C++20[temp.func.order]p2
-  //   [...] If both deductions succeed, the partial ordering selects the
-  // more constrained template (if one exists) as determined below.
-  SmallVector ParamsAC, TemplateAC;
-  Params->getAssociatedConstraints(ParamsAC);
-  // C++2a[temp.arg.template]p3
-  //   [...] In this comparison, if P is unconstrained, the constraints on 
A
-  //   are not considered.
-  if (ParamsAC.empty())
-return false;
+  if (!isTemplateTemplateParameterAtLeastAsSpecializedAs(
+  Params, Param, Template, DefaultArgs, Arg.getLocation(), IsDeduced))
+return true;
+  // P2113
+  // C++20[temp.func.order]p2
+  //   [...] If both deductions succeed, the partial ordering selects the
+  // more constrained template (if one exists) as determined below.
+  SmallVector ParamsAC, TemplateAC;
+  Params->getAssociatedConstraints(ParamsAC);
+  // C++2a[temp.arg.template]p3

cor3ntin wrote:

```suggestion
  // C++20[temp.arg.template]p3
```

preexisting but might as well fix it

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Vlad Serebrennikov via llvm-branch-commits]

https://github.com/Endilll commented:

`Sema.h` changes look good.

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

[llvm-branch-commits] [SPARC][IAS] Enable `ParseForAllFeatures` in `MatchOperandParserImpl` (PR #96021)

[Sergei Barannikov via llvm-branch-commits]

@@ -11,7 +11,7 @@
 ! V9: unknown membar tag
 membar #BadTag
 
-! V8: instruction requires a CPU feature not currently enabled
+! V8: unexpected token
 ! V9: invalid membar mask number
 membar -127

s-barannikov wrote:

There are many places where ParseStatus is not handled correctly.
In this case, it is
```
if (!parseOperand(Operands, Name).isSuccess()) {
  SMLoc Loc = getLexer().getLoc();
  return Error(Loc, "unexpected token");
}
```
It should propagate the error returned by parseOperand instead of issuing 
another one.
Something like:
```
ParseStatus R = parseOperand(Operands, Name).isSuccess();
if (R.isFailure())
  return true;
if (R.isNoMatch())
  return TokError( "unexpected token");
// success, proceed do the next operand
```


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

[llvm-branch-commits] [SPARC][IAS] Enable `ParseForAllFeatures` in `MatchOperandParserImpl` (PR #96021)

[Sergei Barannikov via llvm-branch-commits]

https://github.com/s-barannikov edited 
https://github.com/llvm/llvm-project/pull/96021
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [SPARC][IAS] Enable `ParseForAllFeatures` in `MatchOperandParserImpl` (PR #96021)

[Sergei Barannikov via llvm-branch-commits]

https://github.com/s-barannikov edited 
https://github.com/llvm/llvm-project/pull/96021
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [SPARC][IAS] Enable `ParseForAllFeatures` in `MatchOperandParserImpl` (PR #96021)

[Sergei Barannikov via llvm-branch-commits]

https://github.com/s-barannikov edited 
https://github.com/llvm/llvm-project/pull/96021
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] AMDGPU: Handle legal v2bf16 atomicrmw fadd for gfx12 (PR #95930)

[Jay Foad via llvm-branch-commits]

@@ -1735,8 +1737,11 @@ defm : SIBufferAtomicPat<"SIbuffer_atomic_dec", i64, 
"BUFFER_ATOMIC_DEC_X2">;
 let OtherPredicates = [HasAtomicCSubNoRtnInsts] in
 defm : SIBufferAtomicPat<"SIbuffer_atomic_csub", i32, "BUFFER_ATOMIC_CSUB", 
["noret"]>;
 
-let SubtargetPredicate = isGFX12Plus in {
+let SubtargetPredicate = HasAtomicBufferPkAddBF16Inst in {
   defm : SIBufferAtomicPat_Common<"SIbuffer_atomic_fadd", v2bf16, 
"BUFFER_ATOMIC_PK_ADD_BF16_VBUFFER">;

jayfoad wrote:

VBUFFER is a new encoding in GFX12 which replaces the old MTBUF and MUBUF 
encodings. We have different pseudos for VBUFFER (which should only be selected 
on GFX12+) and MTBUF/MUBUF (which should only be selected pre-GFX12).

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

[llvm-branch-commits] [SPARC][IAS] Rework ASI tag matching in prep for `ParseForAllFeatures` (PR #96020)

[Sergei Barannikov via llvm-branch-commits]

@@ -1085,13 +1085,24 @@ ParseStatus SparcAsmParser::parseASITag(OperandVector 
&Operands) {
   SMLoc E = Parser.getTok().getEndLoc();
   int64_t ASIVal = 0;
 
-  if (is64Bit() && (getLexer().getKind() == AsmToken::Hash)) {
+  switch (getLexer().getKind()) {
+  case AsmToken::LParen:
+  case AsmToken::Integer:
+  case AsmToken::Identifier:
+  case AsmToken::Plus:
+  case AsmToken::Minus:
+  case AsmToken::Tilde:

s-barannikov wrote:

I'd suggest to extract these cases into a helper function that returns true if 
the token can be a start of expression.

Also, optionally, add a `parseExpression` method that returns NoMatch if the 
first token can't start an expression and Success/Failure if it can and 
expression parsing succeeded/failed.


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

[llvm-branch-commits] [SPARC][IAS] Enable `ParseForAllFeatures` in `MatchOperandParserImpl` (PR #96021)

[Sergei Barannikov via llvm-branch-commits]

@@ -11,7 +11,7 @@
 ! V9: unknown membar tag
 membar #BadTag
 
-! V8: instruction requires a CPU feature not currently enabled
+! V8: unexpected token
 ! V9: invalid membar mask number
 membar -127

s-barannikov wrote:

It appears changing this line reveals other bugs. It may be worth making this 
change separately, before enabling ParseForAllFeatures.


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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

@@ -2513,49 +2545,76 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 TemplateDeductionInfo &Info,
 SmallVectorImpl &Deduced,
 bool NumberOfArgumentsMustMatch, PackFold PackFold) {
-  if (PackFold == PackFold::ArgumentToParameter)
-std::swap(Ps, As);
+  bool FoldPackParameter = PackFold == PackFold::ParameterToArgument ||
+   PackFold == PackFold::Both,
+   FoldPackArgument = PackFold == PackFold::ArgumentToParameter ||
+  PackFold == PackFold::Both;
+
   // C++0x [temp.deduct.type]p9:
   //   If the template argument list of P contains a pack expansion that is not
   //   the last template argument, the entire template argument list is a
   //   non-deduced context.
-  if (hasPackExpansionBeforeEnd(Ps))
+  if (FoldPackParameter && hasPackExpansionBeforeEnd(Ps))
+return TemplateDeductionResult::Success;
+
+  if (FoldPackArgument && hasPackExpansionBeforeEnd(As))
 return TemplateDeductionResult::Success;
 
   // C++0x [temp.deduct.type]p9:
   //   If P has a form that contains  or , then each argument Pi of the
   //   respective template argument list P is compared with the corresponding
   //   argument Ai of the corresponding template argument list of A.
-  unsigned ArgIdx = 0, ParamIdx = 0;
-  for (; hasTemplateArgumentForDeduction(Ps, ParamIdx); ++ParamIdx) {
-const TemplateArgument &P = Ps[ParamIdx];
-if (!P.isPackExpansion()) {
+  for (unsigned ArgIdx = 0, ParamIdx = 0; /**/; /**/) {
+if (!hasTemplateArgumentForDeduction(Ps, ParamIdx))
+  return !FoldPackParameter && NumberOfArgumentsMustMatch &&
+ hasTemplateArgumentForDeduction(As, ArgIdx) &&
+ !As[ArgIdx].isPackExpansion()
+ ? TemplateDeductionResult::MiscellaneousDeductionFailure
+ : TemplateDeductionResult::Success;
+
+if (!Ps[ParamIdx].isPackExpansion()) {
   // The simple case: deduce template arguments by matching Pi and Ai.
 
   // Check whether we have enough arguments.
   if (!hasTemplateArgumentForDeduction(As, ArgIdx))
-return NumberOfArgumentsMustMatch
+return !FoldPackArgument && NumberOfArgumentsMustMatch
? TemplateDeductionResult::MiscellaneousDeductionFailure

mizvekov wrote:

The pre-existing error we have used for this is 
`MiscellaneousDeductionFailure`. `IncompletePack` means something else, it's 
used when we have an already expanded pack and we failed to deduce all elements 
of it.

This is a separate thing we have to fix, basically get rid of the 
`MiscellaneousDeductionFailure`, create new representations for all the 
separate things it was used for, and remove the generic `bad deduction` 
diagnostic.

But this would touch tests far outside of the scope of this patch, so I think 
it's best left for a separate PR.

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

@@ -2513,49 +2545,76 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 TemplateDeductionInfo &Info,
 SmallVectorImpl &Deduced,
 bool NumberOfArgumentsMustMatch, PackFold PackFold) {
-  if (PackFold == PackFold::ArgumentToParameter)
-std::swap(Ps, As);
+  bool FoldPackParameter = PackFold == PackFold::ParameterToArgument ||
+   PackFold == PackFold::Both,
+   FoldPackArgument = PackFold == PackFold::ArgumentToParameter ||
+  PackFold == PackFold::Both;
+
   // C++0x [temp.deduct.type]p9:
   //   If the template argument list of P contains a pack expansion that is not
   //   the last template argument, the entire template argument list is a
   //   non-deduced context.
-  if (hasPackExpansionBeforeEnd(Ps))
+  if (FoldPackParameter && hasPackExpansionBeforeEnd(Ps))
+return TemplateDeductionResult::Success;
+
+  if (FoldPackArgument && hasPackExpansionBeforeEnd(As))
 return TemplateDeductionResult::Success;
 
   // C++0x [temp.deduct.type]p9:
   //   If P has a form that contains  or , then each argument Pi of the
   //   respective template argument list P is compared with the corresponding
   //   argument Ai of the corresponding template argument list of A.
-  unsigned ArgIdx = 0, ParamIdx = 0;
-  for (; hasTemplateArgumentForDeduction(Ps, ParamIdx); ++ParamIdx) {
-const TemplateArgument &P = Ps[ParamIdx];
-if (!P.isPackExpansion()) {
+  for (unsigned ArgIdx = 0, ParamIdx = 0; /**/; /**/) {
+if (!hasTemplateArgumentForDeduction(Ps, ParamIdx))
+  return !FoldPackParameter && NumberOfArgumentsMustMatch &&
+ hasTemplateArgumentForDeduction(As, ArgIdx) &&
+ !As[ArgIdx].isPackExpansion()
+ ? TemplateDeductionResult::MiscellaneousDeductionFailure

mizvekov wrote:

As above

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

@@ -3284,16 +3345,20 @@ DeduceTemplateArguments(Sema &S, T *Partial,
   if (Inst.isInvalid())
 return TemplateDeductionResult::InstantiationDepth;
 
-  if (Trap.hasErrorOccurred())
-return TemplateDeductionResult::SubstitutionFailure;
-
   TemplateDeductionResult Result;
   S.runWithSufficientStackSpace(Info.getLocation(), [&] {
 Result = ::FinishTemplateArgumentDeduction(S, Partial,
/*IsPartialOrdering=*/false,
TemplateArgs, Deduced, Info);
   });
-  return Result;
+
+  if (Result != TemplateDeductionResult::Success)
+return Result;
+
+  if (Trap.hasErrorOccurred())
+return TemplateDeductionResult::SubstitutionFailure;
+
+  return TemplateDeductionResult::Success;

mizvekov wrote:

It would change the priority we give to these errors, and I think there is a 
better chance the previous error would have had more context and been more 
helpful.

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

@@ -6369,27 +6451,88 @@ bool 
Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   //   be inverted between Ps and As. On non-deduced context, matching needs to
   //   happen both ways, according to [temp.arg.template]p3, but this is
   //   currently implemented as a special case elsewhere.
-  if (::DeduceTemplateArguments(*this, A, AArgs, PArgs, Info, Deduced,
-/*NumberOfArgumentsMustMatch=*/false,
-IsDeduced ? PackFold::ArgumentToParameter
-  : PackFold::ParameterToArgument) !=
-  TemplateDeductionResult::Success)
+  switch (TemplateDeductionResult Result = ::DeduceTemplateArguments(
+  *this, A, AArgs, PArgs, Info, Deduced,
+  /*NumberOfArgumentsMustMatch=*/false,
+  IsDeduced ? PackFold::ArgumentToParameter : PackFold::Both)) {
+  case clang::TemplateDeductionResult::Success:
+break;
+
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+Diag(AArg->getLocation(), diag::err_template_param_list_different_arity)
+<< (A->size() > P->size()) << /*isTemplateTemplateParameter=*/true
+<< SourceRange(A->getTemplateLoc(), P->getRAngleLoc());
+return false;

mizvekov wrote:

The right thing would be to introduce a new enumerator, but I think this patch 
keeps the status quo while being large enough already.

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

@@ -6369,27 +6451,88 @@ bool 
Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   //   be inverted between Ps and As. On non-deduced context, matching needs to
   //   happen both ways, according to [temp.arg.template]p3, but this is
   //   currently implemented as a special case elsewhere.
-  if (::DeduceTemplateArguments(*this, A, AArgs, PArgs, Info, Deduced,
-/*NumberOfArgumentsMustMatch=*/false,
-IsDeduced ? PackFold::ArgumentToParameter
-  : PackFold::ParameterToArgument) !=
-  TemplateDeductionResult::Success)
+  switch (TemplateDeductionResult Result = ::DeduceTemplateArguments(
+  *this, A, AArgs, PArgs, Info, Deduced,
+  /*NumberOfArgumentsMustMatch=*/false,
+  IsDeduced ? PackFold::ArgumentToParameter : PackFold::Both)) {
+  case clang::TemplateDeductionResult::Success:
+break;
+
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+Diag(AArg->getLocation(), diag::err_template_param_list_different_arity)
+<< (A->size() > P->size()) << /*isTemplateTemplateParameter=*/true
+<< SourceRange(A->getTemplateLoc(), P->getRAngleLoc());
+return false;
+  case TemplateDeductionResult::NonDeducedMismatch:
+Diag(AArg->getLocation(), diag::err_non_deduced_mismatch)
+<< Info.FirstArg << Info.SecondArg;
+return false;
+  case TemplateDeductionResult::Inconsistent:
+Diag(getAsNamedDecl(Info.Param)->getLocation(),
+ diag::err_inconsistent_deduction)
+<< Info.FirstArg << Info.SecondArg;
+return false;
+  case TemplateDeductionResult::AlreadyDiagnosed:
 return false;
 
+  // None of these should happen for a plain deduction.
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::InstantiationDepth:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::IncompletePack:
+  case TemplateDeductionResult::Underqualified:
+  case TemplateDeductionResult::SubstitutionFailure:
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::InvalidExplicitArguments:
+  case TemplateDeductionResult::NonDependentConversionFailure:
+  case TemplateDeductionResult::ConstraintsNotSatisfied:
+  case TemplateDeductionResult::CUDATargetMismatch:

mizvekov wrote:

Adding default would suppress the warning you get when one of the enumerators 
is missing a case. I think this is an immensely useful warning, it helps you 
not forget to look for and do something about each place the enumerator could 
be used.

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

[llvm-branch-commits] [clang] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

@@ -6369,27 +6451,88 @@ bool 
Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   //   be inverted between Ps and As. On non-deduced context, matching needs to
   //   happen both ways, according to [temp.arg.template]p3, but this is
   //   currently implemented as a special case elsewhere.
-  if (::DeduceTemplateArguments(*this, A, AArgs, PArgs, Info, Deduced,
-/*NumberOfArgumentsMustMatch=*/false,
-IsDeduced ? PackFold::ArgumentToParameter
-  : PackFold::ParameterToArgument) !=
-  TemplateDeductionResult::Success)
+  switch (TemplateDeductionResult Result = ::DeduceTemplateArguments(
+  *this, A, AArgs, PArgs, Info, Deduced,
+  /*NumberOfArgumentsMustMatch=*/false,
+  IsDeduced ? PackFold::ArgumentToParameter : PackFold::Both)) {
+  case clang::TemplateDeductionResult::Success:
+break;
+
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+Diag(AArg->getLocation(), diag::err_template_param_list_different_arity)
+<< (A->size() > P->size()) << /*isTemplateTemplateParameter=*/true
+<< SourceRange(A->getTemplateLoc(), P->getRAngleLoc());
+return false;
+  case TemplateDeductionResult::NonDeducedMismatch:
+Diag(AArg->getLocation(), diag::err_non_deduced_mismatch)
+<< Info.FirstArg << Info.SecondArg;
+return false;
+  case TemplateDeductionResult::Inconsistent:
+Diag(getAsNamedDecl(Info.Param)->getLocation(),
+ diag::err_inconsistent_deduction)
+<< Info.FirstArg << Info.SecondArg;
+return false;
+  case TemplateDeductionResult::AlreadyDiagnosed:
 return false;
 
+  // None of these should happen for a plain deduction.
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::InstantiationDepth:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::IncompletePack:
+  case TemplateDeductionResult::Underqualified:
+  case TemplateDeductionResult::SubstitutionFailure:
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::InvalidExplicitArguments:
+  case TemplateDeductionResult::NonDependentConversionFailure:
+  case TemplateDeductionResult::ConstraintsNotSatisfied:
+  case TemplateDeductionResult::CUDATargetMismatch:

mizvekov wrote:

I think the sheer amount of enumerators here is an indication of this growing 
without careful design.

We could probably refactor this and collapse a lot of these cases into fewer 
cases, and such.

I think it would also be helpful to not have one TemplateDeductionResult to 
rule them all, when some times you are dealing with the subset of the problem 
and a lot of these cases don't apply at all.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

https://github.com/ilya-biryukov edited 
https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

https://github.com/ilya-biryukov commented:

Thanks for the change! I have done a round of review and left a few suggestion 
and also have a bunch of questions. I am sorry if some of them may look too 
obvious, I did try to dig into the code where I could, but Clang's 
serialization is complicated and some things are easier researched through a 
conversation than through looking at code.  Please bear with me, I promise to 
get better in the upcoming reviews.

Here are a few extra question that came to mind too.

Question 1: Do we have estimates on how much bigger the PCMs become? Did you 
observer any negative performance impact?

I would expect `TypeID`s to be much more common than decls, and the 
corresponding size increases to be more significant. We've already lost ~6% 
from DeclID change, I am slightly worried the types are going to be a bigger 
hit.

Question 2: could you explain why we the PCM in your example was changing 
before? Was there some base offset inherited from the PCM files we depended on?

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -7650,6 +7647,16 @@ ModuleFile *ASTReader::getOwningModuleFile(GlobalDeclID 
ID) const {
   return &getModuleManager()[ModuleFileIndex - 1];
 }
 
+ModuleFile *ASTReader::getOwningModuleFile(TypeID ID) const {
+  if (ID < NUM_PREDEF_TYPE_IDS)
+return nullptr;
+
+  uint64_t ModuleFileIndex = ID >> 32;

ilya-biryukov wrote:

NIT: maybe we could define helper functions to extract the `ModuleFileIndex`, 
type index and qualifiers from `TypeID`?

It would help the readability a bit (it's not too hard to remember how to do 
these operations, but it's a bit of code duplication and definitely makes 
reading a bit more involved)

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict

ilya-biryukov wrote:

```suggestion
///   three bits are used to store the const/volatile/restrict
```

NIT: indent by 2 spaces to align the second line of the bullet item with its 
first letters on the first line.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -7100,14 +7084,25 @@ TypeSourceInfo *ASTRecordReader::readTypeSourceInfo() {
   return TInfo;
 }
 
+std::pair
+ASTReader::translateTypeIDToIndex(serialization::TypeID ID) const {
+  unsigned Index =
+  (ID & llvm::maskTrailingOnes(32)) >> Qualifiers::FastWidth;
+
+  ModuleFile *OwningModuleFile = getOwningModuleFile(ID);
+  assert(OwningModuleFile &&
+ "untranslated type ID or local type ID shouldn't be in TypesLoaded");
+  return {OwningModuleFile, OwningModuleFile->BaseTypeIndex + Index};
+}
+
 QualType ASTReader::GetType(TypeID ID) {
   assert(ContextObj && "reading type with no AST context");
   ASTContext &Context = *ContextObj;
 
   unsigned FastQuals = ID & Qualifiers::FastMask;
-  unsigned Index = ID >> Qualifiers::FastWidth;
 
-  if (Index < NUM_PREDEF_TYPE_IDS) {
+  if (uint64_t Index = ID >> Qualifiers::FastWidth;

ilya-biryukov wrote:

Higher bits still represent the module file index here, right?
Could you clarify why it's correct to compare with `NUM_PREDEF_TYPE_IDS` 
without first extracting those bits?

Are the higher bits of predefined types always `0`?
(If this was done on top of results `translateTypeIDToIndex`, it would be very 
clear that the code is correct, not sure if there are reasons to postpone this 
call).

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -3262,17 +3262,18 @@ void ASTWriter::WritePragmaDiagnosticMappings(const 
DiagnosticsEngine &Diag,
 /// Write the representation of a type to the AST stream.
 void ASTWriter::WriteType(QualType T) {
   TypeIdx &IdxRef = TypeIdxs[T];
-  if (IdxRef.getIndex() == 0) // we haven't seen this type before.
+  if (IdxRef.getValue() == 0) // we haven't seen this type before.

ilya-biryukov wrote:

NIT: For the purpose of improving readability, I wonder if there is a better 
name than `getValue`? Would something like `getSerializedValue()` or 
`getGlobalIndex()` be appropriate here?

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.
+  unsigned ModuleFileIndex = StoredIdx.getModuleFileIndex();
+  if (ModuleFileIndex == 0 && StoredIdx.getValue())
+return;
+
+  // Otherwise, keep the highest ID since the module file comes later has
+  // higher module file indexes.
+  if (Idx.getValue() >= StoredIdx.getValue())

ilya-biryukov wrote:

```suggestion 
  if (Idx.getModuleFileIndex() >= StoredIdx.getModuleFileIndex())
```
This seems to align better with what the comment for the code are saying. If 
this does not work, I might be missing something.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}

ilya-biryukov wrote:

Could we explicitly pass 0 to the ModuleFileIndex instead? It's very uncommon 
to have defaulted parameters at the start of the parameter list rather than at 
the end.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to

ilya-biryukov wrote:

```suggestion
  // keep the entry from a later module so that we can properly write it out to
```

NIT: the "just writing" sounds weird and there is a reference to the fact it's 
being written later in the line anyway.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.

ilya-biryukov wrote:

Could you explain why we need this special case now and we didn't need it 
before the change?
What has changed? Do we have tests for it?

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding

ilya-biryukov wrote:

```suggestion
/// - the next 29 bits provide a type index in the corresponding

NIT: maybe update the wording? Upon the first read I thought it's referring to 
the 29 highest bit and only after reading the next bullet item I have realized 
it's referring to the 29 bits after the initial 3 lower bits.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -12,44 +12,44 @@
 // RUN: -fmodule-file=Module=%t/Module.pcm -emit-llvm -o - | FileCheck 
%t/Object.cppm
 
 // Test again with reduced BMI.
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: split-file %s %t
+// RUNX: rm -rf %t

ilya-biryukov wrote:

are these leftovers from previous versions of the patch that need to be 
reverted?

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Ilya Biryukov via llvm-branch-commits]

@@ -7392,27 +7388,28 @@ QualType ASTReader::GetType(TypeID ID) {
   return TypesLoaded[Index].withFastQualifiers(FastQuals);
 }
 
-QualType ASTReader::getLocalType(ModuleFile &F, unsigned LocalID) {
+QualType ASTReader::getLocalType(ModuleFile &F, TypeID LocalID) {

ilya-biryukov wrote:

Could we clarify what `LocalID` means here in a comment somewhere?
IIUC, the `ModuleFileIndex` there is an index into `F.TransitiveImports` rather 
than into a "global" module manager.

Previously, we had distinction between `unsigned` in parameters to this 
function and `TypeID` as a return type.
Maybe we should keep this here and accept `uint64_t` to avoid confusing the 
common global module file index and a local file index?

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

[llvm-branch-commits] [clang] [libcxx] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/96023

>From 7c84cd61f2c1bf7ec30d77aaf6b1a87cccf96d2f Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 17 Jun 2024 21:39:08 -0300
Subject: [PATCH] [clang] Finish implementation of P0522

This finishes the clang implementation of P0522, getting rid
of the fallback to the old, pre-P0522 rules.

Before this patch, when partial ordering template template parameters,
we would perform, in order:
* If the old rules would match, we would accept it. Otherwise, don't
  generate diagnostics yet.
* If the new rules would match, just accept it. Otherwise, don't
  generate any diagnostics yet again.
* Apply the old rules again, this time with diagnostics.

This situation was far from ideal, as we would sometimes:
* Accept some things we shouldn't.
* Reject some things we shouldn't.
* Only diagnose rejection in terms of the old rules.

With this patch, we apply the P0522 rules throughout.

This needed to extend template argument deduction in order
to accept the historial rule for TTP matching pack parameter to non-pack
arguments.
This change also makes us accept some combinations of historical and P0522
allowances we wouldn't before.

It also fixes a bunch of bugs that were documented in the test suite,
which I am not sure there are issues already created for them.

This causes a lot of changes to the way these failures are diagnosed,
with related test suite churn.

The problem here is that the old rules were very simple and
non-recursive, making it easy to provide customized diagnostics,
and to keep them consistent with each other.

The new rules are a lot more complex and rely on template argument
deduction, substitutions, and they are recursive.

The approach taken here is to mostly rely on existing diagnostics,
and create a new instantiation context that keeps track of this context.

So for example when a substitution failure occurs, we use the error
produced there unmodified, and just attach notes to it explaining
that it occurred in the context of partial ordering this template
argument against that template parameter.

This diverges from the old diagnostics, which would lead with an
error pointing to the template argument, explain the problem
in subsequent notes, and produce a final note pointing to the parameter.
---
 clang/docs/ReleaseNotes.rst   |   9 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   7 +
 clang/include/clang/Sema/Sema.h   |  14 +-
 clang/lib/Frontend/FrontendActions.cpp|   2 +
 clang/lib/Sema/SemaTemplate.cpp   |  94 ++---
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 341 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  15 +
 .../temp/temp.arg/temp.arg.template/p3-0x.cpp |  31 +-
 clang/test/CXX/temp/temp.param/p12.cpp|  21 +-
 clang/test/Modules/cxx-templates.cpp  |  15 +-
 clang/test/SemaCXX/make_integer_seq.cpp   |   5 +-
 clang/test/SemaTemplate/cwg2398.cpp   |  30 +-
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |  45 +--
 clang/test/SemaTemplate/temp_arg_template.cpp |  38 +-
 .../SemaTemplate/temp_arg_template_p0522.cpp  |  70 ++--
 .../Templight/templight-empty-entries-fix.cpp |  12 +
 .../templight-prior-template-arg.cpp  |  33 +-
 .../type_traits/is_specialization.verify.cpp  |   2 +-
 18 files changed, 504 insertions(+), 280 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7112d1f889fef..abe535f55fb2a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -176,6 +176,8 @@ C++17 Feature Support
   the values produced by GCC, so these macros should not be used from header
   files because they may not be stable across multiple TUs (the values may vary
   based on compiler version as well as CPU tuning). #GH60174
+- The implementation of the relaxed template template argument matching rules 
is
+  more complete and reliable, and should provide more accurate diagnostics.
 
 C++14 Feature Support
 ^
@@ -589,6 +591,10 @@ Improvements to Clang's diagnostics
 - Clang no longer emits a "declared here" note for a builtin function that has 
no declaration in source.
   Fixes #GH93369.
 
+- Clang now properly explains the reason a template template argument failed to
+  match a template template parameter, in terms of the C++17 relaxed matching 
rules
+  instead of the old ones.
+
 Improvements to Clang's time-trace
 --
 
@@ -887,7 +893,8 @@ Bug Fixes to C++ Support
   between the addresses of two labels (a GNU extension) to a pointer within a 
constant expression. (#GH95366).
 - Fix immediate escalation bugs in the presence of dependent call arguments. 
(#GH94935)
 - Clang now diagnoses explicit specializations with storage class specifiers 
in all contexts.
-
+- Fixes to several issues in partial ordering of template template parameters, 
which
+  were documented in the test suite

[llvm-branch-commits] [clang] [libcxx] [clang] Finish implementation of P0522 (PR #96023)

[Matheus Izvekov via llvm-branch-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/96023

>From dd761ef307502c69dd6fdbf2fa56275f56e8cb6a Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 17 Jun 2024 21:39:08 -0300
Subject: [PATCH] [clang] Finish implementation of P0522

This finishes the clang implementation of P0522, getting rid
of the fallback to the old, pre-P0522 rules.

Before this patch, when partial ordering template template parameters,
we would perform, in order:
* If the old rules would match, we would accept it. Otherwise, don't
  generate diagnostics yet.
* If the new rules would match, just accept it. Otherwise, don't
  generate any diagnostics yet again.
* Apply the old rules again, this time with diagnostics.

This situation was far from ideal, as we would sometimes:
* Accept some things we shouldn't.
* Reject some things we shouldn't.
* Only diagnose rejection in terms of the old rules.

With this patch, we apply the P0522 rules throughout.

This needed to extend template argument deduction in order
to accept the historial rule for TTP matching pack parameter to non-pack
arguments.
This change also makes us accept some combinations of historical and P0522
allowances we wouldn't before.

It also fixes a bunch of bugs that were documented in the test suite,
which I am not sure there are issues already created for them.

This causes a lot of changes to the way these failures are diagnosed,
with related test suite churn.

The problem here is that the old rules were very simple and
non-recursive, making it easy to provide customized diagnostics,
and to keep them consistent with each other.

The new rules are a lot more complex and rely on template argument
deduction, substitutions, and they are recursive.

The approach taken here is to mostly rely on existing diagnostics,
and create a new instantiation context that keeps track of this context.

So for example when a substitution failure occurs, we use the error
produced there unmodified, and just attach notes to it explaining
that it occurred in the context of partial ordering this template
argument against that template parameter.

This diverges from the old diagnostics, which would lead with an
error pointing to the template argument, explain the problem
in subsequent notes, and produce a final note pointing to the parameter.
---
 clang/docs/ReleaseNotes.rst   |   9 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   7 +
 clang/include/clang/Sema/Sema.h   |  14 +-
 clang/lib/Frontend/FrontendActions.cpp|   2 +
 clang/lib/Sema/SemaTemplate.cpp   |  94 ++---
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 341 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  15 +
 .../temp/temp.arg/temp.arg.template/p3-0x.cpp |  31 +-
 clang/test/CXX/temp/temp.param/p12.cpp|  21 +-
 clang/test/Modules/cxx-templates.cpp  |  15 +-
 clang/test/SemaCXX/make_integer_seq.cpp   |   5 +-
 clang/test/SemaTemplate/cwg2398.cpp   |  30 +-
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |  45 +--
 clang/test/SemaTemplate/temp_arg_template.cpp |  38 +-
 .../SemaTemplate/temp_arg_template_p0522.cpp  |  70 ++--
 .../Templight/templight-empty-entries-fix.cpp |  12 +
 .../templight-prior-template-arg.cpp  |  33 +-
 .../type_traits/is_specialization.verify.cpp  |   2 +-
 18 files changed, 504 insertions(+), 280 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7112d1f889fef..abe535f55fb2a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -176,6 +176,8 @@ C++17 Feature Support
   the values produced by GCC, so these macros should not be used from header
   files because they may not be stable across multiple TUs (the values may vary
   based on compiler version as well as CPU tuning). #GH60174
+- The implementation of the relaxed template template argument matching rules 
is
+  more complete and reliable, and should provide more accurate diagnostics.
 
 C++14 Feature Support
 ^
@@ -589,6 +591,10 @@ Improvements to Clang's diagnostics
 - Clang no longer emits a "declared here" note for a builtin function that has 
no declaration in source.
   Fixes #GH93369.
 
+- Clang now properly explains the reason a template template argument failed to
+  match a template template parameter, in terms of the C++17 relaxed matching 
rules
+  instead of the old ones.
+
 Improvements to Clang's time-trace
 --
 
@@ -887,7 +893,8 @@ Bug Fixes to C++ Support
   between the addresses of two labels (a GNU extension) to a pointer within a 
constant expression. (#GH95366).
 - Fix immediate escalation bugs in the presence of dependent call arguments. 
(#GH94935)
 - Clang now diagnoses explicit specializations with storage class specifiers 
in all contexts.
-
+- Fixes to several issues in partial ordering of template template parameters, 
which
+  were documented in the test suite

[llvm-branch-commits] [clang] [libcxx] [clang] Finish implementation of P0522 (PR #96023)

[Louis Dionne via llvm-branch-commits]

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

libcxx/ nit LGTM.

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

[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

[Louis Dionne via llvm-branch-commits]

https://github.com/ldionne created 
https://github.com/llvm/llvm-project/pull/96116

None

>From 4044e7c930381e5e070c7131c5b14a3dfd373259 Mon Sep 17 00:00:00 2001
From: Louis Dionne 
Date: Wed, 19 Jun 2024 16:50:07 -0400
Subject: [PATCH] [libc++] Add release note for #95264

---
 libcxx/docs/ReleaseNotes/18.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 7ea13e6943dd4..3e19e7c33f6af 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -328,6 +328,15 @@ ABI Affecting Changes
   done to fix `#70494 `_ 
and the vendor communication is handled
   in `#70820 `_.
 
+- LLVM 18.1.8 Fixed an issue that caused ``std::string`` to pass an incorrect 
size to ``allocator_traits::deallocate``
+  when deallocating memory. The impact is different depending on a few factors:
+  - Users who don't use a custom allocator in ``std::string`` and don't enable 
sized deallocation (which is
+off by default in Clang 18) will not be affected. This is expected to be 
the vast majority of users.
+  - Users who don't use a custom allocator in ``std::string`` but are enabling 
sized deallocation (e.g. with
+``-fsized-deallocation``) will notice that ``operator delete(void*, 
size_t)`` is now being passed the correct
+size. This likely has no impact if they were not customizing ``operator 
delete``.
+  - Users who use a custom allocator in ``std::string`` will notice that they 
now get passed the correct allocation
+size upon deallocation.
 
 Build System Changes
 

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

[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

[Louis Dionne via llvm-branch-commits]

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/96116
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)


Changes



---
Full diff: https://github.com/llvm/llvm-project/pull/96116.diff


1 Files Affected:

- (modified) libcxx/docs/ReleaseNotes/18.rst (+9) 


``diff
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 7ea13e6943dd4..3e19e7c33f6af 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -328,6 +328,15 @@ ABI Affecting Changes
   done to fix `#70494 `_ 
and the vendor communication is handled
   in `#70820 `_.
 
+- LLVM 18.1.8 Fixed an issue that caused ``std::string`` to pass an incorrect 
size to ``allocator_traits::deallocate``
+  when deallocating memory. The impact is different depending on a few factors:
+  - Users who don't use a custom allocator in ``std::string`` and don't enable 
sized deallocation (which is
+off by default in Clang 18) will not be affected. This is expected to be 
the vast majority of users.
+  - Users who don't use a custom allocator in ``std::string`` but are enabling 
sized deallocation (e.g. with
+``-fsized-deallocation``) will notice that ``operator delete(void*, 
size_t)`` is now being passed the correct
+size. This likely has no impact if they were not customizing ``operator 
delete``.
+  - Users who use a custom allocator in ``std::string`` will notice that they 
now get passed the correct allocation
+size upon deallocation.
 
 Build System Changes
 

``




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

[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)

[Daniil Kovalev via llvm-branch-commits]

@@ -354,6 +354,23 @@ Given that ``signedPointer`` matches the layout for signed 
pointers signed with
 the given key, extract the raw pointer from it.  This operation does not trap
 and cannot fail, even if the pointer is not validly signed.
 
+``ptrauth_sign_constant``
+^
+
+.. code-block:: c
+
+  ptrauth_sign_constant(pointer, key, discriminator)
+
+Return a signed pointer for a constant address in a manner which guarantees
+a non-attackable sequence.
+
+``pointer`` must be a constant expression of pointer type which evaluates to
+a non-null pointer.  The result will have the same type as ``discriminator``.

kovdan01 wrote:

Could you please reply to the previous comment in this thread? It was resolved, 
but the issue with the docs does not seem to be fixed. Please let me know if I 
miss smth and there is no issue - but, at least, we have different info in rst 
docs and in ptrauth.h header (the first one says that the result has the same 
type as `discriminator`, the second one - that the result's type is the same as 
the type of original value). So, things should at least be consistent. Ideally, 
we can just use the exact same text for description in both places, but I'm 
happy with different descriptions if they just match each other.

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

[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)

[Daniil Kovalev via llvm-branch-commits]

@@ -58,6 +58,35 @@ void test_string_discriminator(const char *str) {
 }
 
 
+void test_sign_constant(int *dp, int (*fp)(int)) {
+  __builtin_ptrauth_sign_constant(&dv, VALID_DATA_KEY); // expected-error 
{{too few arguments}}
+  __builtin_ptrauth_sign_constant(&dv, VALID_DATA_KEY, &dv, &dv); // 
expected-error {{too many arguments}}
+
+  __builtin_ptrauth_sign_constant(mismatched_type, VALID_DATA_KEY, 0); // 
expected-error {{signed value must have pointer type; type here is 'struct A'}}
+  __builtin_ptrauth_sign_constant(&dv, mismatched_type, 0); // expected-error 
{{passing 'struct A' to parameter of incompatible type 'int'}}
+  __builtin_ptrauth_sign_constant(&dv, VALID_DATA_KEY, mismatched_type); // 
expected-error {{extra discriminator must have pointer or integer type; type 
here is 'struct A'}}
+
+  (void) __builtin_ptrauth_sign_constant(NULL, VALID_DATA_KEY, &dv); // 
expected-error {{argument to ptrauth_sign_constant must refer to a global 
variable or function}}

kovdan01 wrote:

Thanks for explanation. I suppose it's OK to leave the way null sign/auth is 
handled "as is" for this PR and ship changes (if needed) as future patches.

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

[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)

[Daniil Kovalev via llvm-branch-commits]

@@ -354,6 +354,23 @@ Given that ``signedPointer`` matches the layout for signed 
pointers signed with
 the given key, extract the raw pointer from it.  This operation does not trap
 and cannot fail, even if the pointer is not validly signed.
 
+``ptrauth_sign_constant``
+^
+
+.. code-block:: c
+
+  ptrauth_sign_constant(pointer, key, discriminator)
+
+Return a signed pointer for a constant address in a manner which guarantees
+a non-attackable sequence.

kovdan01 wrote:

Thanks, now I see that. Are you going to submit documentation changes all at 
once as a separate PR? If it's not too time-consuming, I'd prefer having all 
related terms defined as a part of this PR (particularly, at least the part 
"Attackable code sequences"), but I'm OK with submitting documentation changes 
separately.

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

[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)

[Daniil Kovalev via llvm-branch-commits]

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

LGTM with a couple of documentation-related comments

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

[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)

[Daniil Kovalev via llvm-branch-commits]

https://github.com/kovdan01 edited 
https://github.com/llvm/llvm-project/pull/93904
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [clang] Implement pointer authentication for C++ virtual functions, v-tables, and VTTs (PR #94056)

[Anton Korobeynikov via llvm-branch-commits]

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

Thanks!

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> Thanks for the change! I have done a round of review and left a few 
> suggestion and also have a bunch of questions. I am sorry if some of them may 
> look too obvious, I did try to dig into the code where I could, but Clang's 
> serialization is complicated and some things are easier researched through a 
> conversation than through looking at code. Please bear with me, I promise to 
> get better in the upcoming reviews.

You're welcome.

> 
> Here are a few extra question that came to mind too.
> 
> Question 1: Do we have estimates on how much bigger the PCMs become? Did you 
> observer any negative performance impact?
> 
> I would expect `TypeID`s to be much more common than decls, and the 
> corresponding size increases to be more significant. We've already lost ~6% 
> from DeclID change, I am slightly worried the types are going to be a bigger 
> hit.

I did and my local results shows, I see less than 1% change with this patch. 
This fits my understanding somehow. Since the `TypeID` is much less common than 
`DeclID`. This matches my experience in coding. The `DeclID` patch is the 
hardest one and the `TypeID` patch is the easiest one.

> 
> Question 2: could you explain why we the PCM in your example was changing 
> before? Was there some base offset inherited from the PCM files we depended 
> on?

Yes. In the higher level, it should be easy to understand. There is a new type 
in `m-partA.v1.cppm`. And in the low level, when we write a module file, we 
would record the type offset for the module files we depend on: 
https://github.com/llvm/llvm-project/blob/ad79a14c9e5ec4a369eed4adf567c22cc029863f/clang/lib/Serialization/ASTWriter.cpp#L5454

And this is exactly what the series patch want to eliminate.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -7100,14 +7084,25 @@ TypeSourceInfo *ASTRecordReader::readTypeSourceInfo() {
   return TInfo;
 }
 
+std::pair
+ASTReader::translateTypeIDToIndex(serialization::TypeID ID) const {
+  unsigned Index =
+  (ID & llvm::maskTrailingOnes(32)) >> Qualifiers::FastWidth;
+
+  ModuleFile *OwningModuleFile = getOwningModuleFile(ID);
+  assert(OwningModuleFile &&
+ "untranslated type ID or local type ID shouldn't be in TypesLoaded");
+  return {OwningModuleFile, OwningModuleFile->BaseTypeIndex + Index};
+}
+
 QualType ASTReader::GetType(TypeID ID) {
   assert(ContextObj && "reading type with no AST context");
   ASTContext &Context = *ContextObj;
 
   unsigned FastQuals = ID & Qualifiers::FastMask;
-  unsigned Index = ID >> Qualifiers::FastWidth;
 
-  if (Index < NUM_PREDEF_TYPE_IDS) {
+  if (uint64_t Index = ID >> Qualifiers::FastWidth;

ChuanqiXu9 wrote:

> Are the higher bits of predefined types always 0?

Yes. Technically, the predefined ones (including decls, identifiers and types) 
doesn't belong to any modules. So their module file index  is always 0.

I will try to update a comment for this.

> (If this was done on top of results translateTypeIDToIndex, it would be very 
> clear that the code is correct, not sure if there are reasons to postpone 
> this call).

The `Index` in `translateTypeIDToIndex` means index in the 
`ASTReader::TypesLoaded` array. And these predefined ones, they are predefined, 
not loaded, shouldn't be in   `ASTReader::TypesLoaded`. Then I call 
`translateTypeIDToIndex` after dealing with the predefined ones.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}

ChuanqiXu9 wrote:

Done.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.

ChuanqiXu9 wrote:

Since the current being written module file is the latest module. The reason 
why we don't need it previously is that, 
previously the type ID of the current being written module file is always 
larger than types from other module files.

For example, if A import B and B has 30 types, then previously, the type ID for 
the first type in A will be 31 + `NUM_PREDEFINED_TYPES`. This is also one of 
the motivation of the series patch. We don't hope the encoding of the current 
module file being affected by imported modules as much as possible.

So now, for the above example, the first type ID in A will be `<0, 1 + 
NUM_PREDEFINED_TYPES>` and type ID in B may be `<1, [0, 30]>`. So we can't 
compare them easily.

>  Do we have tests for it?

Yes, if we remove this line, there are failing tests.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.
+  unsigned ModuleFileIndex = StoredIdx.getModuleFileIndex();
+  if (ModuleFileIndex == 0 && StoredIdx.getValue())
+return;
+
+  // Otherwise, keep the highest ID since the module file comes later has
+  // higher module file indexes.
+  if (Idx.getValue() >= StoredIdx.getValue())

ChuanqiXu9 wrote:

Yes, done.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -12,44 +12,44 @@
 // RUN: -fmodule-file=Module=%t/Module.pcm -emit-llvm -o - | FileCheck 
%t/Object.cppm
 
 // Test again with reduced BMI.
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: split-file %s %t
+// RUNX: rm -rf %t

ChuanqiXu9 wrote:

Oh, sorry. This is something left during debugging. I will remove it.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -7392,27 +7388,28 @@ QualType ASTReader::GetType(TypeID ID) {
   return TypesLoaded[Index].withFastQualifiers(FastQuals);
 }
 
-QualType ASTReader::getLocalType(ModuleFile &F, unsigned LocalID) {
+QualType ASTReader::getLocalType(ModuleFile &F, TypeID LocalID) {

ChuanqiXu9 wrote:

> Could we clarify what LocalID means here in a comment somewhere?

In ASTReader.h, there is a single line comment: `Resolve a local type ID within 
a given AST file into a type.`. I add a new line to explain it as "A local type 
ID is only meaningful with the corresponding module file. See the 
implementation of getGlobalTypeID for details.".

> IIUC, the ModuleFileIndex there is an index into F.TransitiveImports rather 
> than into a "global" module manager.

Yes, this is correct for local ID. But I feel it might be odd to put this in 
the comment. I feel, it might be good to know, we can use 
`ASTReader::getGlobalTypeID` to convert a `` pair to a 
`GlobalTypeID`. And the concrete process is the implementation details to 
GlobalTypeID`.

> Previously, we had distinction between unsigned in parameters to this 
> function and TypeID as a return type.
Maybe we should keep this here and accept uint64_t to avoid confusing the 
common global module file index and a local file index?

I don't feel it better. `uint64_t` doesn't provide any type information. To be 
best, maybe we can do something like for `LocalDeclID` and `GlobalDeclID`. I 
only did that for DeclID since I feel the DeclID is the most complicated and 
the TypeID are much more simpler. I think we can this later if we feel it is 
necessary.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92511

>From 57cfb2b7791666022ee46201b5126ac610c19bdd Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH 1/2] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 03bac9be2bf3d..13ee1e4724f1e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {}
+
+  uint32_t getModuleFileIndex() const { return ModuleFileIndex; }
 
-  uint32_t getIndex() const { return Idx; }
+  uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; }
 
   TypeID asTypeID(unsigned FastQuals) const {
 if (Idx == uint32_t(-1))
   return TypeID(-1);
 
-return (Idx << Qualifiers::FastWidth) | FastQuals;
+unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals;
+return ((uint64_t)ModuleFileIndex << 32) | Index;
   }
 
   static TypeIdx fromTypeID(TypeID ID) {
 if (ID == TypeID(-1))
   return TypeIdx(-1);
 
-return TypeIdx(ID >> Qualifiers::FastWidth);
+return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >>
+ Qualifiers::FastWidth);
   }
 };
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 3f38a08f0da3a..f4180984eaad3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -490,14 +490,6 @@ class ASTReader
   /// ID = (I + 1) << FastQual::Width has already been loaded
   llvm::PagedVector TypesLoaded;
 
-  using GlobalTypeMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global type IDs to the module in which the
-  /// type resides along with the offset that should be added to the
-  /// global type ID to produce a local ID.
-  GlobalTypeMapType GlobalTypeMap;
-
   /// Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1423,8 +1415,8 @@ class ASTReader
 RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {}
   };
 
-  QualType readTypeRecord(unsigned Index);
-  RecordLocation TypeCursorForIndex(unsigned Index);
+  QualType readTypeRecord(serialization::TypeID ID);
+  RecordLocation TypeCursorForIndex(serialization::TypeID ID);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(GlobalDe

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92511

>From 57cfb2b7791666022ee46201b5126ac610c19bdd Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH 1/2] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 03bac9be2bf3d..13ee1e4724f1e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {}
+
+  uint32_t getModuleFileIndex() const { return ModuleFileIndex; }
 
-  uint32_t getIndex() const { return Idx; }
+  uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; }
 
   TypeID asTypeID(unsigned FastQuals) const {
 if (Idx == uint32_t(-1))
   return TypeID(-1);
 
-return (Idx << Qualifiers::FastWidth) | FastQuals;
+unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals;
+return ((uint64_t)ModuleFileIndex << 32) | Index;
   }
 
   static TypeIdx fromTypeID(TypeID ID) {
 if (ID == TypeID(-1))
   return TypeIdx(-1);
 
-return TypeIdx(ID >> Qualifiers::FastWidth);
+return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >>
+ Qualifiers::FastWidth);
   }
 };
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 3f38a08f0da3a..f4180984eaad3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -490,14 +490,6 @@ class ASTReader
   /// ID = (I + 1) << FastQual::Width has already been loaded
   llvm::PagedVector TypesLoaded;
 
-  using GlobalTypeMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global type IDs to the module in which the
-  /// type resides along with the offset that should be added to the
-  /// global type ID to produce a local ID.
-  GlobalTypeMapType GlobalTypeMap;
-
   /// Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1423,8 +1415,8 @@ class ASTReader
 RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {}
   };
 
-  QualType readTypeRecord(unsigned Index);
-  RecordLocation TypeCursorForIndex(unsigned Index);
+  QualType readTypeRecord(serialization::TypeID ID);
+  RecordLocation TypeCursorForIndex(serialization::TypeID ID);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(GlobalDe

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to

ChuanqiXu9 wrote:

Done

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -7650,6 +7647,16 @@ ModuleFile *ASTReader::getOwningModuleFile(GlobalDeclID 
ID) const {
   return &getModuleManager()[ModuleFileIndex - 1];
 }
 
+ModuleFile *ASTReader::getOwningModuleFile(TypeID ID) const {
+  if (ID < NUM_PREDEF_TYPE_IDS)
+return nullptr;
+
+  uint64_t ModuleFileIndex = ID >> 32;

ChuanqiXu9 wrote:

Done

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -3262,17 +3262,18 @@ void ASTWriter::WritePragmaDiagnosticMappings(const 
DiagnosticsEngine &Diag,
 /// Write the representation of a type to the AST stream.
 void ASTWriter::WriteType(QualType T) {
   TypeIdx &IdxRef = TypeIdxs[T];
-  if (IdxRef.getIndex() == 0) // we haven't seen this type before.
+  if (IdxRef.getValue() == 0) // we haven't seen this type before.

ChuanqiXu9 wrote:

I feel it might not be needed. Since the type `TypeIdx` shows the information 
that this is index from serialization to me.

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict

ChuanqiXu9 wrote:

Done

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

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding

ChuanqiXu9 wrote:

Done

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

[llvm-branch-commits] [clang] [clang] Implement pointer authentication for C++ virtual functions, v-tables, and VTTs (PR #94056)

[Oliver Hunt via llvm-branch-commits]

@@ -296,3 +296,21 @@ 
ConstantAggregateBuilderBase::finishStruct(llvm::StructType *ty) {
   buffer.erase(buffer.begin() + Begin, buffer.end());
   return constant;
 }
+

ojhunt wrote:

@asl Updating to resolve conflict required bringing this function in that was 
previously in Akira's work

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

[llvm-branch-commits] [nsan] Fix style issue (PR #96142)

[Fangrui Song via llvm-branch-commits]

https://github.com/MaskRay created 
https://github.com/llvm/llvm-project/pull/96142

The initial check-in of compiler-rt/lib/nsan #94322 has a lot of style
issues. Fix them before the history becomes more useful.



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

[llvm-branch-commits] [nsan] Fix style issue (PR #96142)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Fangrui Song (MaskRay)


Changes

The initial check-in of compiler-rt/lib/nsan #94322 has a lot of style
issues. Fix them before the history becomes more useful.


---

Patch is 54.71 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/96142.diff


8 Files Affected:

- (modified) compiler-rt/lib/nsan/CMakeLists.txt (+5-5) 
- (renamed) compiler-rt/lib/nsan/nsan.cpp (+133-141) 
- (modified) compiler-rt/lib/nsan/nsan.h (+3-3) 
- (renamed) compiler-rt/lib/nsan/nsan_flags.cpp (+6-7) 
- (removed) compiler-rt/lib/nsan/nsan_interceptors.cc (-364) 
- (added) compiler-rt/lib/nsan/nsan_interceptors.cpp (+362) 
- (renamed) compiler-rt/lib/nsan/nsan_stats.cpp (+1-1) 
- (renamed) compiler-rt/lib/nsan/nsan_suppressions.cpp (+26-26) 


``diff
diff --git a/compiler-rt/lib/nsan/CMakeLists.txt 
b/compiler-rt/lib/nsan/CMakeLists.txt
index ae94c96d60727..36c7b2b9dfada 100644
--- a/compiler-rt/lib/nsan/CMakeLists.txt
+++ b/compiler-rt/lib/nsan/CMakeLists.txt
@@ -3,11 +3,11 @@ add_compiler_rt_component(nsan)
 include_directories(..)
 
 set(NSAN_SOURCES
-  nsan.cc
-  nsan_flags.cc
-  nsan_interceptors.cc
-  nsan_stats.cc
-  nsan_suppressions.cc
+  nsan.cpp
+  nsan_flags.cpp
+  nsan_interceptors.cpp
+  nsan_stats.cpp
+  nsan_suppressions.cpp
 )
 
 set(NSAN_HEADERS
diff --git a/compiler-rt/lib/nsan/nsan.cc b/compiler-rt/lib/nsan/nsan.cpp
similarity index 78%
rename from compiler-rt/lib/nsan/nsan.cc
rename to compiler-rt/lib/nsan/nsan.cpp
index d0d29ddfba0e3..ece1130f73d14 100644
--- a/compiler-rt/lib/nsan/nsan.cc
+++ b/compiler-rt/lib/nsan/nsan.cpp
@@ -82,22 +82,22 @@ const char FTInfo::kTypePattern[sizeof(float)];
 const char FTInfo::kTypePattern[sizeof(double)];
 const char FTInfo::kTypePattern[sizeof(long double)];
 
-// Helper for __nsan_dump_shadow_mem: Reads the value at address `Ptr`,
+// Helper for __nsan_dump_shadow_mem: Reads the value at address `ptr`,
 // identified by its type id.
-template  __float128 readShadowInternal(const u8 *Ptr) {
+template  __float128 readShadowInternal(const u8 *ptr) {
   ShadowFT Shadow;
-  __builtin_memcpy(&Shadow, Ptr, sizeof(Shadow));
+  __builtin_memcpy(&Shadow, ptr, sizeof(Shadow));
   return Shadow;
 }
 
-__float128 readShadow(const u8 *Ptr, const char ShadowTypeId) {
+__float128 readShadow(const u8 *ptr, const char ShadowTypeId) {
   switch (ShadowTypeId) {
   case 'd':
-return readShadowInternal(Ptr);
+return readShadowInternal(ptr);
   case 'l':
-return readShadowInternal(Ptr);
+return readShadowInternal(ptr);
   case 'q':
-return readShadowInternal<__float128>(Ptr);
+return readShadowInternal<__float128>(ptr);
   default:
 return 0.0;
   }
@@ -120,30 +120,30 @@ struct PrintBuffer {
 template  struct FTPrinter {};
 
 template <> struct FTPrinter {
-  static PrintBuffer dec(double Value) {
-PrintBuffer Result;
-snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20f", Value);
-return Result;
+  static PrintBuffer dec(double value) {
+PrintBuffer result;
+snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20f", value);
+return result;
   }
-  static PrintBuffer hex(double Value) {
-PrintBuffer Result;
-snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20a", Value);
-return Result;
+  static PrintBuffer hex(double value) {
+PrintBuffer result;
+snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20a", value);
+return result;
   }
 };
 
 template <> struct FTPrinter : FTPrinter {};
 
 template <> struct FTPrinter {
-  static PrintBuffer dec(long double Value) {
-PrintBuffer Result;
-snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20Lf", Value);
-return Result;
+  static PrintBuffer dec(long double value) {
+PrintBuffer result;
+snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20Lf", value);
+return result;
   }
-  static PrintBuffer hex(long double Value) {
-PrintBuffer Result;
-snprintf(Result.Buffer, sizeof(Result.Buffer) - 1, "%.20La", Value);
-return Result;
+  static PrintBuffer hex(long double value) {
+PrintBuffer result;
+snprintf(result.Buffer, sizeof(result.Buffer) - 1, "%.20La", value);
+return result;
   }
 };
 
@@ -151,15 +151,15 @@ template <> struct FTPrinter {
 template <> struct FTPrinter<__float128> : FTPrinter {};
 
 // This is a template so that there are no implicit conversions.
-template  inline FT ftAbs(FT V);
+template  inline FT ftAbs(FT v);
 
-template <> inline long double ftAbs(long double V) { return fabsl(V); }
-template <> inline double ftAbs(double V) { return fabs(V); }
+template <> inline long double ftAbs(long double v) { return fabsl(v); }
+template <> inline double ftAbs(double v) { return fabs(v); }
 
 // We don't care about nans.
 // std::abs(__float128) code is suboptimal and generates a function call to
 // __getf2().
-template  inline FT ftAbs(FT V) { return V >= FT{0} ? V : -V; }
+tem