[clang] [clang] Implement P2582R1: CTAD from inherited constructors (PR #98788)

2024-07-18 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov commented:

Just a couple of nits, I will do a more comprehensive review later.

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


[clang] [clang] Implement P2582R1: CTAD from inherited constructors (PR #98788)

2024-07-18 Thread Matheus Izvekov via cfe-commits

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


[clang] [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (PR #86265)

2024-07-18 Thread Matheus Izvekov via cfe-commits


@@ -14636,6 +14645,20 @@ TreeTransform::TransformLambdaExpr(LambdaExpr 
*E) {
 /*IsInstantiation*/ true);
   SavedContext.pop();
 
+  // Parts other than the capture e.g. the lambda body might still contain a
+  // pattern that an outer fold expression would expand.
+  //
+  // We don't have a way to propagate up the ContainsUnexpandedParameterPack
+  // flag from a Stmt, so we have to revisit the lambda.
+  if (!LSICopy.ContainsUnexpandedParameterPack) {
+llvm::SmallVector UnexpandedPacks;
+getSema().collectUnexpandedParameterPacksFromLambda(NewCallOperator,
+UnexpandedPacks);
+// FIXME: Should we call Sema::DiagnoseUnexpandedParameterPacks() instead?
+// Unfortunately, that requires the LambdaScopeInfo to exist, which has 
been
+// removed by ActOnFinishFunctionBody().
+LSICopy.ContainsUnexpandedParameterPack = !UnexpandedPacks.empty();
+  }

mizvekov wrote:

I am not sure I understand why we would need to call 
`Sema::DiagnoseUnexpandedParameterPacks` here.

I think we should be able to diagnose all unexpanded parameter packs during the 
initial lambda parsing, so what gives?

Also, I am not sure what you mean by leaving it as an NFC.

Regardless, you have a copy of the LSI here, so what would stop you from 
pushing a new Lambda Scope and copying this LSI back into it? Something like:

```C++
  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
  *LSI = LSICopy;
  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
```

This is obviously an ugly hack, but I don't think the solution is to simply 
teach `ActOnFinishFunctionBody` not to pop the function scope.

See this FIXME for a potentially more comprehensive fix: 
https://github.com/llvm/llvm-project/blob/77ac07444d32668d5826ef27c24180fb10425213/clang/lib/Sema/TreeTransform.h#L14648

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


[clang] [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (PR #86265)

2024-07-18 Thread Matheus Izvekov via cfe-commits


@@ -23,3 +23,104 @@ namespace PR41576 {
   }
   static_assert(f(3, 4) == 6); // expected-note {{instantiation}}
 }
+
+namespace PR85667 {
+
+template 
+struct identity {
+  using type = T;
+};
+
+template  void f() {
+
+  static_assert([](Is... x) {
+return ([I(x)] {
+  return I;
+}() + ...);
+  }(1, 2) == 3);
+
+  static_assert([](Is... x) {
+return ([](auto y = Is()) { return y + 1; } + ...);
+  }(0, 0, 0) == 3);
+
+  []() {
+return ([]() noexcept(Is()) { return 0; }() + ...);
+  }.template operator()();
+
+  static_assert(__is_same(decltype([]() {
+return ([]() -> decltype(Is()) { return {}; }(),
+...);
+  }.template operator()()),
+  char));
+
+  []() {
+return ([]() -> decltype(Is()) { return Ts(); }() + ...);
+// expected-error@-1 {{unexpanded parameter pack 'Ts'}}
+  }.template operator()();
+
+  // Note that GCC and EDG reject this case currently.
+  // GCC says the fold expression "has no unexpanded parameter packs", while
+  // EDG says the constraint is not allowed on a non-template function.
+  // MSVC is happy with it.
+  []() {
+([]()
+   requires(Is())
+ {},
+ ...);
+  }.template operator()();
+
+  // https://github.com/llvm/llvm-project/issues/56852
+  [](Is...) {
+([] {
+  using T = identity::type;
+}(), ...);
+  }(1, 2);
+
+  [](auto ...y) {
+([y] { }(), ...);
+  }();
+
+  [](auto ...x) {
+([&](auto ...y) {
+  ([x..., y] { }(), ...);
+})(1);
+  }(2, 'b');
+
+#if 0
+  // https://github.com/llvm/llvm-project/issues/18873
+  [](auto ...x) { // #1
+([&](auto ...y) {  // #2
+  ([x, y] { }(), ...); // #3
+})(1, 'a');  // #4
+  }(2, 'b');  // #5
+

mizvekov wrote:

I am not opposed to adding disabled crash repros as tests, for future fixes, 
but these should be clearly marked with a FIXME.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-18 Thread Matheus Izvekov via cfe-commits


@@ -6308,11 +6308,11 @@ TypeResult Sema::ActOnTypeName(Declarator &D) {
 CheckExtraCXXDefaultArguments(D);
   }
 
-  if (const AutoType *AutoT = T->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
-
+  if (const AutoType *AutoT = T->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();
+CheckConstrainedAuto(AutoT,
+ Loc ? Loc.getConceptNameLoc() : SourceLocation());
+  }

mizvekov wrote:

But my question was, what cases are we handling with this check in 
`ActOnTypeName`?
I thought 'auto' couldn't appear here.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-18 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

LGTM, Thanks!

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-18 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-18 Thread Matheus Izvekov via cfe-commits


@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
-  if (const AutoType *AutoT = R->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
+  if (const AutoType *AutoT = R->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();

mizvekov wrote:

Yeah, we don't need to have both TypeSourceInfo and TypeLoc in clang. I am not 
sure the history of how the situation arose, but we can pretty much consolidate 
on TypeLoc at some point.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-17 Thread Matheus Izvekov via cfe-commits


@@ -6308,11 +6308,11 @@ TypeResult Sema::ActOnTypeName(Declarator &D) {
 CheckExtraCXXDefaultArguments(D);
   }
 
-  if (const AutoType *AutoT = T->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
-
+  if (const AutoType *AutoT = T->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();
+CheckConstrainedAuto(AutoT,
+ Loc ? Loc.getConceptNameLoc() : SourceLocation());
+  }

mizvekov wrote:

Ah, is this purely for the error cases?

Ie in my example above, you will still get the error, but is that for 
suppressing the deprecated warning in case that auto was constrained and 
happened to be the only use of that concept?

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-17 Thread Matheus Izvekov via cfe-commits


@@ -6308,11 +6308,11 @@ TypeResult Sema::ActOnTypeName(Declarator &D) {
 CheckExtraCXXDefaultArguments(D);
   }
 
-  if (const AutoType *AutoT = T->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
-
+  if (const AutoType *AutoT = T->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();
+CheckConstrainedAuto(AutoT,
+ Loc ? Loc.getConceptNameLoc() : SourceLocation());
+  }

mizvekov wrote:

These still have the same issue, but I don't understand why we need this check 
here.

We obviously can't write stuff like
```C++
using X = auto;
```

So I am missing in what case undeduced auto can appear here, except the weird 
deleted function error recovery issue.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-17 Thread Matheus Izvekov via cfe-commits


@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
-  if (const AutoType *AutoT = R->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
+  if (const AutoType *AutoT = R->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();

mizvekov wrote:

But here we are not just asking for the location of constrained auto.

A TypeLoc is not just a SourceLocation, it's a QualType *with* source locations.

The reason you are starting with a TypeLoc is that you are also interested in 
the source location.

This is a bit confusing because in a lot of places we pass both a QualType and 
a TypeLoc, which is mostly redundant. That might induce folks into thinking 
that a TypeLoc is just source locations on the side, which is not true.

But presently, they are not always redundant though. There are some cases we 
store both separately, and apply some transformations, like type deduction, 
only to the QualType, and leave the TypeLoc alone.
I think this is not ideal though, and seems to be mostly due to tech debt.

As an aside, if all you need to do is mark the concepts as used, you might as 
well do that earlier when the AutoType is formed, for example around the calls 
to `ASTContext::getAutoType` in `SemaType.cpp` / `ConvertDeclSpecToType`.


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


[clang] Fix assertion failure during conversion function overload resolution. (PR #98671)

2024-07-16 Thread Matheus Izvekov via cfe-commits

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

LGTM as well

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


[clang] [clang] Add deprecation warning for `-Ofast` driver option (PR #98736)

2024-07-16 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [clang] Add deprecation warning for `-Ofast` driver option (PR #98736)

2024-07-15 Thread Matheus Izvekov via cfe-commits


@@ -442,6 +442,9 @@ def warn_drv_deprecated_arg : Warning<
 def warn_drv_deprecated_arg_no_relaxed_template_template_args : Warning<
   "argument '-fno-relaxed-template-template-args' is deprecated">,
   InGroup;
+def warn_drv_deprecated_arg_ofast : Warning<
+  "argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'">,

mizvekov wrote:

I think in this diagnostic it's more important to inform the user that he 
should replace `-Ofast` with `-O3 -ffast-math` in order to just circumvent the 
deprecation and keep the current behavior, and this should be clear. Throwing 
the option of using just `O3` as equally valid is a bit misleading, but we can, 
as a note, inform the user that it's possible he made the wrong choice in the 
first place, and advise a switch to `O3`.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-15 Thread Matheus Izvekov via cfe-commits


@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
-  if (const AutoType *AutoT = R->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
+  if (const AutoType *AutoT = R->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();

mizvekov wrote:

Regarding the error recovery issue, in `SemaOverload.cpp`:

In `FixOverloadedFunctionReference`, for both `UnresolvedLookupExpr` and 
`UnresolvedMemberExpr`, we must avoid building a `DeclRefExpr` which has a 
function type returning undeduced auto.

Presently this should only happen for calls to deleted functions returning a 
deduced type, so should only happen when it's called from 
`FinishOverloadedCallExpr`, in the `OR_Deleted` case.

You can replace just the `auto` in the return type with `int`, keeping pointers 
and references in case of `auto *` or `auto &`. That can be done using 
`Sema::SubstAutoType`.

Eventually when we implement some kind of `ErrorTy`, for improving error 
recovery, we can change all such uses of 'int', using that instead.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-15 Thread Matheus Izvekov via cfe-commits


@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
-  if (const AutoType *AutoT = R->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
+  if (const AutoType *AutoT = R->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();

mizvekov wrote:

Sure, but let me get back first to the other problem in this area:

I think that assumption I pointed out is wrong, we should not be using 
`getAs` here, as that will not dig through ReferenceType and 
PointerType, and the following example will not be diagnosed: 
https://godbolt.org/z/jzEaYPq4r

The `getContainedAutoTypeLoc` is what is a good fit for that, and what we 
should be using throughout. It will dig through the things needed for finding 
the AutoType, like pointers, references, and some of the necessary type sugar 
which can appear in source code when writing a deduced type. It does not dig 
through the things it doesn't need to, like `decltype`, because you can't write 
a deduced type with that.

So something like:
```suggestion
  if (AutoTypeLoc TL = TInfo->getTypeLoc().getContainedAutoTypeLoc()) {
const AutoType *AT = TL.getTypePtr();
```

Should work, and it will not get confused by the error recovery issue, which 
can be addressed separately.

Nit: We conventionally use `TL` abbreviation for TypeLoc, and `Loc` is 
conventional for SourceLocation.

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


[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-15 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

> That's a pretty substantial policy change to propose, and this probably isn't 
> the place to propose/discuss it. If that's your intent, probably best to take 
> that up on discord.
> 

I am not proposing a policy change. I believe the current policy is aimed at 
giving an escape hatch for projects which there is basically one or two active 
developers. I am pointing out that I believe we don't need for that escape 
hatch to apply to any parts of clang currently.

> (FWIW, check some of the recent modules changes @ChuanqiXu9 has been working 
> on to see that reviewer bandwidth here is pretty thin (& my experience in 
> LLVM in general, including clang, is that reviewer bandwidth is pretty thin - 
> though it is something we should address & I do think it might be time to 
> change LLVM's post-commit review policy, but I think it'll be a substantial 
> amount of work))

If you feel bandwidth for modules is pretty thin, I put myself available as a 
reviewer, so feel free to ping me.
I may not have a lot of time available for fully reviewing big patches, but I 
can certainly help with the smaller patches such as this one.



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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-12 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov requested changes to this pull request.

See previous comments, I think this is primarily an error recovery issue on the 
decltype, we shouldn't let it escape an undeduced auto, as that would confuse 
further analysis.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-12 Thread Matheus Izvekov via cfe-commits


@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
-  if (const AutoType *AutoT = R->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
+  if (const AutoType *AutoT = R->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();

mizvekov wrote:

Oh, looking at the reproducer, I see the problem:

Our variable is getting the type of an undeduced AutoType through that decltype 
calling a deleted function with auto type.

This is confusing our semantic analysis for the variable, as now we think the 
variable was declared as auto, as getAs will look through all sugar nodes, 
'decltype' included.

Since `getContainedAutoTypeLoc()` doesn't look through decltype, you get a 
crash.

But we shouldn't be getting an 'auto' here in the first place.

The correct fix would be to make that decltype have some error type, for error 
recovery purposes.

Since we don't implement a generic unknown error type, 'int' would still be 
better.

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


[clang] [clang] Fix crash in concept deprecation (PR #98622)

2024-07-12 Thread Matheus Izvekov via cfe-commits


@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
-  if (const AutoType *AutoT = R->getAs())
-CheckConstrainedAuto(
-AutoT,
-TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc());
+  if (const AutoType *AutoT = R->getAs()) {
+AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc();

mizvekov wrote:

These two lines are analyzing the type in very different ways, so there seems 
to be further bugs here.

`Type::getAs` will simply desugar a type until it finds a node with 
type 'AutoType'.

`getContainedAutoTypeLoc()` will dig through a certain list of type nodes until 
it finds an 'AutoType'.
This list includes non-sugar nodes as well.

Let's assume for now we really want `R->getAs()` in the if condition 
here, which would not find the AutoType in `auto *`. This seems wrong, as these 
can be constrained as well. But we can handle that as a separate issue.

So to hit your bug, you are finding an AutoType through getAs, but not finding 
it through getContainedAutoTypeLoc().

My first suspicion would be that the type contains a sugar node not handled by 
`GetContainedAutoTypeLocVisitor`, in `AST/TypeLoc.cpp`.

Can you check that?

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


[clang] [Clang][NFCI] Remove records of unsatisfied atomic expressions in ConstraintSatisfaction (PR #98654)

2024-07-12 Thread Matheus Izvekov via cfe-commits

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

LGTM, I agree that we currently don't use it, so we might as well remove it.

But the overall situation is all wrong, we shouldn't be storing fully formed 
diagnostic strings in the AST, we should be storing them as PartialDiagnostics.

But making that right requires bigger changes.

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


[clang] [clang] Prevent dangling StringRefs (PR #98699)

2024-07-12 Thread Matheus Izvekov via cfe-commits

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

LGTM.

If it does fix the mentioned issue, please add a release note.

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


[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-12 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

> Clarifying a couple of things here

I think we need the distinction between simple NFC, for trivial, 
non-controversial and non-surprising changes like some renames, small 
whitespace / formatting change, documentation changes, and complex NFC, which 
requires some thinking and has more risk of not actually being NFC.

So in practical terms, I propose a complex NFC change should only be excused of 
changes to functional testing, nothing else.

Also, I think we presently have sufficient review bandwidth in clang proper, 
even including the area of modules, so that any non-trivial change can be 
reviewed, and I also propose that we abstain from trying to directly commit 
those.



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


[clang] [clang-tools-extra] [Clang][AST] Move NamespaceDecl bits to DeclContext (PR #98567)

2024-07-12 Thread Matheus Izvekov via cfe-commits

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

LGTM, as a clean up, and I see no notable performance implications.

Is there any follow up work for this?

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


[clang] [Clang][Parse] Fix ambiguity with nested-name-specifiers that may declarative (PR #96364)

2024-07-11 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-11 Thread Matheus Izvekov via cfe-commits

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

LGTM, sans:
* missing newline at end of test file
* A similar test for template template parameter.

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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-11 Thread Matheus Izvekov via cfe-commits


@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform {
 // Find all template parameters that appear in the given DeducedArgs.
 // Return the indices of the template parameters in the TemplateParams.
 SmallVector TemplateParamsReferencedInTemplateArgumentList(
-ArrayRef TemplateParams,
+const TemplateParameterList* TemplateParamsList,
 ArrayRef DeducedArgs) {
   struct TemplateParamsReferencedFinder
   : public RecursiveASTVisitor {
+const TemplateParameterList* TemplateParamList;
 llvm::DenseSet TemplateParams;
 llvm::DenseSet ReferencedTemplateParams;
 
-TemplateParamsReferencedFinder(ArrayRef TemplateParams)
-: TemplateParams(TemplateParams.begin(), TemplateParams.end()) {}
+TemplateParamsReferencedFinder(
+const TemplateParameterList *TemplateParamList)
+: TemplateParamList(TemplateParamList),
+  TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) 
{
+}
 
 bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) {
-  MarkAppeared(TTP->getDecl());
+  // We use the index and depth to retrieve the corresponding template
+  // parameter from the parameter list.
+  // Note that Clang may not preserve type sugar during template argument
+  // deduction. In such cases, the TTP is a canonical 
TemplateTypeParamType,
+  // which only retains its index and depth information.
+  if (TTP->getDepth() == TemplateParamList->getDepth() &&

mizvekov wrote:

The NTTP case is trickier to test, as we don't canonicalize expressions the 
same way we do types and templates,
but in some situations, you could have an NTTP which was uniqued pointing to a 
specific parameter, but this is misleading and you should disregard that and 
look at only the depth and index.

But I would expect template template parameters to behave similarly as type 
parameters here, and it should be just as straightforward to test, so I think 
it would b e worth a try.

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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-11 Thread Matheus Izvekov via cfe-commits


@@ -99,3 +99,28 @@ BFoo b2(1.0, 2.0);
 // CHECK-NEXT: | | |-ParmVarDecl {{.*}} 'type-parameter-0-0'
 // CHECK-NEXT: | | `-ParmVarDecl {{.*}} 'type-parameter-0-0'
 // CHECK-NEXT: | `-CXXDeductionGuideDecl {{.*}} implicit used  'auto (double, double) -> Foo' implicit_instantiation
+
+namespace GH90209 {
+template 
+struct List {
+  List(int);
+};
+
+template 
+struct TemplatedClass {
+  TemplatedClass(T1);
+};
+
+template 
+TemplatedClass(T1) -> TemplatedClass>;
+
+template 
+using ATemplatedClass = TemplatedClass>;
+
+ATemplatedClass test(1);
+// Verify that we have a correct template parameter list for the deduction 
guide.
+//
+// CHECK:  FunctionTemplateDecl {{.*}} 
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2
+// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
+} // namespace GH90209

mizvekov wrote:

The GitHub editor still shows the missing newline at end of file marker.

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


[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-10 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

The reproducer turned out to be pretty simple:
```C++
export module a;
module :private;
static void f() {}
void g() {
  f();
}
```

Compiles without that patch, otherwise produces:
```
error: no matching function for call to 'f'
```



> Oh, sorry, I took another look at the commit and it looks the change makes it 
> not a NFC change is this line: 
> [99873b3#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514](https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514)
> 
> This shouldn't be in that commit but in this commit. It is not intentional. I 
> guess we can't observe that if we put that in this PR too. And that change 
> looks not bad. So maybe it makes something already bad to show up.

Even without that change, the other changes are too complex, and there are 
other suspicious things you wouldn't expect in an NFC commit which doesn't call 
this out specifically, like the removal of that whole block with the FIXME 
included.

I think the appropriate tag for such commits would be NFCI, and should still 
require PR and review.

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


[clang] [Clang][Sema] Handle class member access expressions with valid nested-name-specifiers that become invalid after lookup (PR #98167)

2024-07-09 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

2024-07-09 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

FYI, the commit 99873b35da7ecb905143c8a6b8deca4d4416f1a9, which lists this PR 
as a motivator, causes breakage building a project, which I am still looking 
for a reduced test case.

The commit is not NFC despite what is says, just by cursory inspection of the 
change.

Please avoid adding NFC tag to such commits in the future and create a pull 
request.

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


[clang] [Clang] Fix the order of addInstantiatedParameters in LambdaScopeForCallOperatorInstantiationRAII (PR #97215)

2024-07-08 Thread Matheus Izvekov via cfe-commits

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

LGTM

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


[clang] [Sema] Fix crash in Sema::FindInstantiatedDecl (PR #96509)

2024-07-08 Thread Matheus Izvekov via cfe-commits
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To: 



@@ -6300,7 +6300,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, 
NamedDecl *D,
   getTrivialTemplateArgumentLoc(UnpackedArg, QualType(), Loc));
   }
   QualType T = CheckTemplateIdType(TemplateName(TD), Loc, Args);
-  if (T.isNull())
+  if (T.isNull() || T->containsErrors())

mizvekov wrote:

I don't understand the logic of this change: If there is no reason to make 
`CheckTemplateIdType` fail for more cases, why make this change then?

Usually in error recovery, we want to carry the error information as much as we 
can, where practical.
In this case, there is the same implementation complexity either way.
This change went in the opposite direction: We are bailing out earlier for more 
cases, and we have no reason for it.

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


[clang] [Clang][Index] Add support for dependent class scope explicit specializations of function templates to USRGenerator (PR #98027)

2024-07-08 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-08 Thread Matheus Izvekov via cfe-commits


@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform {
 // Find all template parameters that appear in the given DeducedArgs.
 // Return the indices of the template parameters in the TemplateParams.
 SmallVector TemplateParamsReferencedInTemplateArgumentList(
-ArrayRef TemplateParams,
+const TemplateParameterList* TemplateParamsList,

mizvekov wrote:

The template parameters themselves should also have a depth, not just the index.
So you don't need to change everything to pass a TemplateParameterList instead 
of the array as before.

In fact, TemplateParameterList will just return the depth of its first 
parameter.

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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-08 Thread Matheus Izvekov via cfe-commits


@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform {
 // Find all template parameters that appear in the given DeducedArgs.
 // Return the indices of the template parameters in the TemplateParams.
 SmallVector TemplateParamsReferencedInTemplateArgumentList(
-ArrayRef TemplateParams,
+const TemplateParameterList* TemplateParamsList,
 ArrayRef DeducedArgs) {
   struct TemplateParamsReferencedFinder
   : public RecursiveASTVisitor {
+const TemplateParameterList* TemplateParamList;
 llvm::DenseSet TemplateParams;
 llvm::DenseSet ReferencedTemplateParams;
 
-TemplateParamsReferencedFinder(ArrayRef TemplateParams)
-: TemplateParams(TemplateParams.begin(), TemplateParams.end()) {}
+TemplateParamsReferencedFinder(
+const TemplateParameterList *TemplateParamList)
+: TemplateParamList(TemplateParamList),
+  TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) 
{
+}
 
 bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) {
-  MarkAppeared(TTP->getDecl());
+  // We use the index and depth to retrieve the corresponding template
+  // parameter from the parameter list.
+  // Note that Clang may not preserve type sugar during template argument
+  // deduction. In such cases, the TTP is a canonical 
TemplateTypeParamType,
+  // which only retains its index and depth information.

mizvekov wrote:

I would remove this section of the comment, as I think this sort of implies it 
would be ok to use the type sugar if it were available, but if we rely on this 
for correct compilation of the program, it doesn't make sense to call it type 
sugar anymore.
```suggestion
```

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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-08 Thread Matheus Izvekov via cfe-commits


@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform {
 // Find all template parameters that appear in the given DeducedArgs.
 // Return the indices of the template parameters in the TemplateParams.
 SmallVector TemplateParamsReferencedInTemplateArgumentList(
-ArrayRef TemplateParams,
+const TemplateParameterList* TemplateParamsList,
 ArrayRef DeducedArgs) {
   struct TemplateParamsReferencedFinder
   : public RecursiveASTVisitor {
+const TemplateParameterList* TemplateParamList;
 llvm::DenseSet TemplateParams;
 llvm::DenseSet ReferencedTemplateParams;
 
-TemplateParamsReferencedFinder(ArrayRef TemplateParams)
-: TemplateParams(TemplateParams.begin(), TemplateParams.end()) {}
+TemplateParamsReferencedFinder(
+const TemplateParameterList *TemplateParamList)
+: TemplateParamList(TemplateParamList),
+  TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) 
{
+}
 
 bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) {
-  MarkAppeared(TTP->getDecl());
+  // We use the index and depth to retrieve the corresponding template
+  // parameter from the parameter list.
+  // Note that Clang may not preserve type sugar during template argument
+  // deduction. In such cases, the TTP is a canonical 
TemplateTypeParamType,
+  // which only retains its index and depth information.
+  if (TTP->getDepth() == TemplateParamList->getDepth() &&

mizvekov wrote:

Shouldn't this issue come up for the other kind of template parameters as well, 
besides type parameters?

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


[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)

2024-07-08 Thread Matheus Izvekov via cfe-commits


@@ -99,3 +99,28 @@ BFoo b2(1.0, 2.0);
 // CHECK-NEXT: | | |-ParmVarDecl {{.*}} 'type-parameter-0-0'
 // CHECK-NEXT: | | `-ParmVarDecl {{.*}} 'type-parameter-0-0'
 // CHECK-NEXT: | `-CXXDeductionGuideDecl {{.*}} implicit used  'auto (double, double) -> Foo' implicit_instantiation
+
+namespace GH90209 {
+template 
+struct List {
+  List(int);
+};
+
+template 
+struct TemplatedClass {
+  TemplatedClass(T1);
+};
+
+template 
+TemplatedClass(T1) -> TemplatedClass>;
+
+template 
+using ATemplatedClass = TemplatedClass>;
+
+ATemplatedClass test(1);
+// Verify that we have a correct template parameter list for the deduction 
guide.
+//
+// CHECK:  FunctionTemplateDecl {{.*}} 
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2
+// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
+} // namespace GH90209

mizvekov wrote:

Minor nit: there is now a missing newline at end of file

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


[clang] [clang] CTAD alias: fix transformation for require-clause expr Part2. (PR #93533)

2024-07-04 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [Clang][Sema] Fix crash when rebuilding MemberExprs with invalid object expressions (PR #97455)

2024-07-03 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [clang] Diagnose use of deprecated template alias (PR #97619)

2024-07-03 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

For reference, see https://reviews.llvm.org/D136533 for similar expansion of 
diagnostics for uses of declarations, and how that lead to trouble in MacOS 
land.

I'd suggest building libc++ on MacOS, to be sure this won't cause regressions 
and a future revert.

I have a MacOS machine now, so I should be able to try to revive that patch at 
some point as well.

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


[clang] [Clang][Sema] Fix crash when rebuilding MemberExprs with invalid object expressions (PR #97455)

2024-07-03 Thread Matheus Izvekov via cfe-commits


@@ -2896,6 +2896,9 @@ class TreeTransform {
 SS.Adopt(QualifierLoc);
 
 Base = BaseResult.get();
+if (Base->containsErrors())
+  return ExprError();

mizvekov wrote:

I am still traveling back from St Louis, so I can't double check this:

Why is this point the farthest we can go if Base contains errors?

It seems we can build a reference expression over it from the cases above, but 
why not below?

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


[clang] [Clang][Sema] Correctly transform dependent operands of overloaded binary operator& (PR #97596)

2024-07-03 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [Clang][Sema] Treat explicit specializations of static data member templates declared without 'static' as static data members when diagnosing uses of 'auto' (PR #97425)

2024-07-02 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [Clang][Sema] qualifier should be transformed (PR #94725)

2024-06-22 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

> I mean we can transform `QualifiedTemplateName` to `DependentTemplateName` 
> and this is what this patch does.

But that goes against the natural flow of template instantiation. We can't go 
back from non-dependent into dependent.
We can't transform a regular TemplateName back into a DependentTemplateName, 
just as we can't go back to DependentTemplateSpecializationType from a 
TemplateSpecializationType.

That's why I find this so confusing...

> If we build a `DependentTemplateName` instead of `QualifiedTemplateName` 
> earlier would affect the code in partial specialization like:
> 
> ```c++
> template
> struct A::B
> ```

Can you elaborate on that? That still seems like the right path to me, you 
probably found another problem, that happens some times.

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


[clang] [Clang][Sema] qualifier should be transformed (PR #94725)

2024-06-22 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

> @mizvekov After looking into the code, I think we should transform qualifier 
> in TST. Consider the following code:

So the NNS is already dependent, that's great.

I still don't understand why you are saying we didn't or can't build a 
DependentTemplateSpecializationType instead.

Yes, the template name needs to stay a DependentTemplateName for that.

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-19 Thread Matheus Izvekov via cfe-commits




mizvekov wrote:

After we fork for clang-20, we can entirely remove 
`frelaxed-template-template-args`, and so won't need to worry about testing 
that non-conforming mode. Would it be reasonable to postpone that until then?

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


[clang] [clang] Change style of superseded issues on C++ DR status page (PR #96051)

2024-06-19 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-18 Thread Matheus Izvekov via cfe-commits

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

>From b238961ba3a174d7dc211caf36ff8fd6c8429a76 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |  11 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  63 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 153 +---
 clang/lib/AST/ASTDiagnostic.cpp   |  45 +++--
 clang/lib/AST/ASTImporter.cpp |  15 ++
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/Decl.cpp|   3 +-
 clang/lib/AST/ItaniumMangle.cpp   |  20 ++-
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 163 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/AST/TypePrinter.cpp |   3 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   9 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 158 +
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  62 +--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 28 files changed, 629 insertions(+), 257 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 #undef TEMPLATE_KIND
 }
 llvm_unreachable("Unhandled NameKind enum");
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a366f1331c2d3..e6d16af2495fe 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1120,6 +1120,7 @@ class CollectExtraHighlightings
 case TemplateName::SubstTemplateTemplateParm:
 case TemplateName::SubstTemplateTemplateParmPack:
 case TemplateName::UsingTemplate:
+case TemplateName::DeducedTemplate:
   // Names that could be resolved to a TemplateDecl are handled elsewhere.
   break;
 }
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index de86cb5e9d7fc..837bcacbc0bfc 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::ContextualFoldingSet
 SubstTemplateTemplateParmPacks;
+  mutable llvm::ContextualFoldingSet
+  DeducedTemplates;
 
   mutable llvm::ContextualFoldingSet
   Arr

[clang] [Clang] Instantiate local constexpr functions eagerly (PR #95660)

2024-06-15 Thread Matheus Izvekov via cfe-commits

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

LGTM

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


[clang] [Clang] Instantiate local constexpr functions eagerly (PR #95660)

2024-06-15 Thread Matheus Izvekov via cfe-commits

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


[clang] [Clang] Instantiate local constexpr functions eagerly (PR #95660)

2024-06-15 Thread Matheus Izvekov via cfe-commits


@@ -18112,7 +18112,8 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, 
FunctionDecl *Func,
 
 if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
 Func->isConstexpr()) {
-  if (isa(Func->getDeclContext()) &&
+  if (!Func->isConstexpr() &&

mizvekov wrote:

Nit: Slight simplification, you might as well move this branch after the `else 
if (Func->isConstexpr())` below instead of testing it's negation.

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-13 Thread Matheus Izvekov via cfe-commits


@@ -9219,7 +9222,8 @@ class Sema final : public SemaBase {
   /// \returns true if an error occurred, false otherwise.
   bool CheckTemplateArgumentList(
   TemplateDecl *Template, SourceLocation TemplateLoc,
-  TemplateArgumentListInfo &TemplateArgs, bool PartialTemplateArgs,
+  TemplateArgumentListInfo &TemplateArgs,

mizvekov wrote:

Yeah. One issue I have often had with these functions with large amount of both 
defaulted and non-defaulted parameters, is that you would want to extend it by 
changing the signature, then arguments would match parameters incorrectly, but 
this would not cause a hard error on all of the call sites.

I could have easily added DefaultArgs as defaulted empty here, but chose not to 
due to this reason.

Besides that, overloading functions with such huge numbers of parameters 
creates some confusion as well.
I'd slightly prefer if we avoided that, but don't have strong enough feelings 
to go on a crusade against it.

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


[clang] [Clang][Sema] qualifier should be transformed (PR #94725)

2024-06-12 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov requested changes to this pull request.

Needs changes as discussed.

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


[clang] [Clang][Sema] qualifier should be transformed (PR #94725)

2024-06-12 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

I think this is missing one detail: We now have the same qualifier in two 
places: The elaborated type node attached to the TST, and in the name of the 
TST itself.

While this is not ideal situation, I think it makes sense to just drop the 
TemplateName qualifier in the transform for now, so we don't keep transforming 
it twice, until we make the situation consistent again. Right now the NNS in 
the TST name is not printed, its only purpose there is to carry the template 
keyword information, which shouldn't be affected here.

The fact that this change somehow affects program semantics is still unexpected 
and unexplained.

As I said on the other PR, after we have built a TST, per current AST design, 
we shouldn't need the nested name specifier anymore beyond type sugar.

So either we built this TST too early, as in it should have stayed as a 
DependentTemplateSpecializationType longer or some such, or there is something 
that needs to be rectified in the AST design, but I can't imagine what.

If I had to take a guess, there is something wrong in the dependency 
computation for the NNS. Have you checked that?

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


[clang] Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit (PR #94515)

2024-06-12 Thread Matheus Izvekov via cfe-commits


@@ -2402,10 +2405,9 @@ Expr *VarDecl::getInit() {
 
   auto *Eval = getEvaluatedStmt();
 
-  return cast_if_present(
-  Eval->Value.isOffset()
-  ? Eval->Value.get(getASTContext().getExternalSource())
-  : Eval->Value.get(nullptr));
+  return cast(Eval->Value.isOffset()
+? Eval->Value.get(getASTContext().getExternalSource())
+: Eval->Value.get(nullptr));

mizvekov wrote:

Small nit: It seems this could move the conditional into the argument for 
`Eval->Value.get`.

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


[clang] Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit (PR #94515)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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

This is much better, thanks!

LGTM

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


[clang] Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit (PR #94515)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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


[clang] [clang] Don't print extra space when dumping template names (PR #95213)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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

I think what I tried to do here is generally consistent: The convention on the 
Text node dumper is to add a space at the beginning of the field you want to 
append.

I think the true issue here is the label: It breaks convention by adding a 
space after the colon.

I think it would be better to fix that, but I don't know the impact.

Since right now this field is only added after the label, this makes little 
practical difference, so I leave it up to you, either way is fine.

Thanks for the fix!

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

FYI https://github.com/itanium-cxx-abi/cxx-abi/issues/184 is the tracking issue 
for the mangling rules we need here.
We will probably end up with something quite different than what I coded here 
so far.

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits


@@ -9219,7 +9222,8 @@ class Sema final : public SemaBase {
   /// \returns true if an error occurred, false otherwise.
   bool CheckTemplateArgumentList(
   TemplateDecl *Template, SourceLocation TemplateLoc,
-  TemplateArgumentListInfo &TemplateArgs, bool PartialTemplateArgs,
+  TemplateArgumentListInfo &TemplateArgs,

mizvekov wrote:

I am not seeing a worthwhile tradeoff.

Just the function signature is hugely complicated, and would need to be 
duplicated.
The implementation would be mostly the same, with a small block which would be 
omitted in one of the implementations.

What did you have in mind?

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits


@@ -9434,6 +9505,32 @@ ASTContext::getSubstTemplateTemplateParmPack(const 
TemplateArgument &ArgPack,
   return TemplateName(Subst);
 }
 
+/// Retrieve the template name that represents a template name
+/// deduced from a specialization.
+TemplateName
+ASTContext::getDeducedTemplateName(TemplateName Underlying,
+   DefaultArguments DefaultArgs) const {
+  if (!DefaultArgs)
+return Underlying;
+
+  auto &Self = const_cast(*this);

mizvekov wrote:

Actually in this case the relevant hash tables were already declared mutable, 
but there were a couple of profile functions not marked const. Fixed.

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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

>From f05e8590c7fae599d0658829949fa907942e83f2 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |  11 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  63 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 153 +---
 clang/lib/AST/ASTDiagnostic.cpp   |  45 +++--
 clang/lib/AST/ASTImporter.cpp |  15 ++
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/Decl.cpp|   3 +-
 clang/lib/AST/ItaniumMangle.cpp   |  20 ++-
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 163 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/AST/TypePrinter.cpp |   3 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   9 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 158 +
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  62 +--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 28 files changed, 629 insertions(+), 257 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 #undef TEMPLATE_KIND
 }
 llvm_unreachable("Unhandled NameKind enum");
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a366f1331c2d3..e6d16af2495fe 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1120,6 +1120,7 @@ class CollectExtraHighlightings
 case TemplateName::SubstTemplateTemplateParm:
 case TemplateName::SubstTemplateTemplateParmPack:
 case TemplateName::UsingTemplate:
+case TemplateName::DeducedTemplate:
   // Names that could be resolved to a TemplateDecl are handled elsewhere.
   break;
 }
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index f1f20fca477a4..c9376dc02fcfc 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::ContextualFoldingSet
 SubstTemplateTemplateParmPacks;
+  mutable llvm::ContextualFoldingSet
+  DeducedTemplates;
 
   mutable llvm::ContextualFoldingSet
   Arr

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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


[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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

>From 8bd63f109c2bc1888b4d8dbd5e880900bbb4cef7 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Wed, 12 Jun 2024 00:42:48 -0300
Subject: [PATCH] [clang] fix broken canonicalization of
 DeducedTemplateSpecializationType

This reverts the functional elements of commit 
3e78fa860235431323aaf08c8fa922d75a7cfffa
and redoes it, by fixing the true root cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical
name would result in inconsistent preservation of as-written information
in the AST.

The true problem in #61317 is that we would not consider the methods
with requirements expression which contain DTSTs as the same
in relation to merging of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing
to different redeclarations of the same class template, but since
canonicalization for them was broken, their canonical types would not
match either.

---

No changelog entry because #61317 was already claimed as fixed in
previous release.
---
 clang/include/clang/AST/ASTContext.h   |  7 +++
 clang/include/clang/AST/TemplateName.h |  4 +-
 clang/include/clang/AST/Type.h | 11 ++--
 clang/lib/AST/ASTContext.cpp   | 25 ++---
 clang/lib/AST/TemplateName.cpp |  9 
 clang/unittests/AST/CMakeLists.txt |  1 +
 clang/unittests/AST/ProfilingTest.cpp  | 73 ++
 7 files changed, 107 insertions(+), 23 deletions(-)
 create mode 100644 clang/unittests/AST/ProfilingTest.cpp

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index a1d1d1c51cd41..53ece996769a8 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase {
 QualType DeducedType,
 bool IsDependent) const;
 
+private:
+  QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
+QualType DeducedType,
+bool IsDependent,
+QualType Canon) const;
+
+public:
   /// Return the unique reference to the type for the specified TagDecl
   /// (struct/union/class/enum) decl.
   QualType getTagDeclType(const TagDecl *Decl) const;
diff --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index 988a55acd2252..24a7fde76195d 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -346,7 +346,9 @@ class TemplateName {
   /// error.
   void dump() const;
 
-  void Profile(llvm::FoldingSetNodeID &ID);
+  void Profile(llvm::FoldingSetNodeID &ID) {
+ID.AddPointer(Storage.getOpaqueValue());
+  }
 
   /// Retrieve the template name as a void pointer.
   void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9eb3f6c09e3d3..fab233b62d8d1 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
 
   DeducedTemplateSpecializationType(TemplateName Template,
 QualType DeducedAsType,
-bool IsDeducedAsDependent)
+bool IsDeducedAsDependent, QualType Canon)
   : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
 toTypeDependence(Template.getDependence()) |
 (IsDeducedAsDependent
  ? TypeDependence::DependentInstantiation
  : TypeDependence::None),
-DeducedAsType.isNull() ? QualType(this, 0)
-   : DeducedAsType.getCanonicalType()),
+Canon),
 Template(Template) {}
 
 public:
   /// Retrieve the name of the template that we are deducing.
   TemplateName getTemplateName() const { return Template;}
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
+  void Profile(llvm::FoldingSetNodeID &ID) const {
 Profile(ID, getTemplateName(), getDeducedType(), isDependentType());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-QualType CanonicalType =
-Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
-ID.AddPointer(CanonicalType.getAsOpaquePtr());
+Deduced.Profile(ID);
 ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index bf74e56a14799..34aa399fda2f8 

[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits


@@ -9434,6 +9505,32 @@ ASTContext::getSubstTemplateTemplateParmPack(const 
TemplateArgument &ArgPack,
   return TemplateName(Subst);
 }
 
+/// Retrieve the template name that represents a template name
+/// deduced from a specialization.
+TemplateName
+ASTContext::getDeducedTemplateName(TemplateName Underlying,
+   DefaultArguments DefaultArgs) const {
+  if (!DefaultArgs)
+return Underlying;
+
+  auto &Self = const_cast(*this);

mizvekov wrote:

This is done throughout the ASTContext and is not novel here.

We wish to be able to create a new node from a const ASTContext reference.

These creation functions mostly behave const-ish, they just use hash tables in 
the ASTContext for caching / identity purposes.

The other alternative is to declare these hash tables as mutable, though this 
would necessitate a large noisy change to update everything at once.

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits


@@ -6371,6 +6376,70 @@ ASTContext::getCanonicalTemplateName(const TemplateName 
&Name) const {
 canonArgPack, subst->getAssociatedDecl()->getCanonicalDecl(),
 subst->getFinal(), subst->getIndex());
   }
+  case TemplateName::DeducedTemplate: {
+assert(IgnoreDeduced == false);
+DeducedTemplateStorage *DTS = Name.getAsDeducedTemplateName();
+DefaultArguments DefArgs = DTS->getDefaultArguments();
+TemplateName Underlying = DTS->getUnderlying();
+
+bool NonCanonical = false;
+TemplateName CanonUnderlying =
+getCanonicalTemplateName(Underlying, /*IgnoreDeduced=*/true);
+NonCanonical |= CanonUnderlying != Underlying;
+auto CanonArgs =
+getCanonicalTemplateArguments(*this, DefArgs.Args, NonCanonical);
+{
+  unsigned NumArgs = CanonArgs.size() - 1;
+  auto handleParamDefArg = [&](const TemplateArgument &ParamDefArg,
+   unsigned I) {
+auto CanonParamDefArg = getCanonicalTemplateArgument(ParamDefArg);
+TemplateArgument &CanonDefArg = CanonArgs[I];
+if (CanonDefArg.structurallyEquals(CanonParamDefArg))
+  return;
+if (I == NumArgs)
+  CanonArgs.pop_back();
+NonCanonical = true;
+  };
+  auto handleParam = [&](auto *TP, int I) -> bool {
+if (!TP->hasDefaultArgument())
+  return true;
+handleParamDefArg(TP->getDefaultArgument().getArgument(), I);
+return false;
+  };
+
+  ArrayRef Params = CanonUnderlying.getAsTemplateDecl()
+ ->getTemplateParameters()
+ ->asArray();
+  assert(CanonArgs.size() <= Params.size());
+  for (int I = NumArgs; I >= 0; --I) {

mizvekov wrote:

The code is correct, but the variable name is not right. Should have been 
`LastArgIndex` or some such.

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-12 Thread Matheus Izvekov via cfe-commits


@@ -6371,6 +6376,70 @@ ASTContext::getCanonicalTemplateName(const TemplateName 
&Name) const {
 canonArgPack, subst->getAssociatedDecl()->getCanonicalDecl(),
 subst->getFinal(), subst->getIndex());
   }
+  case TemplateName::DeducedTemplate: {
+assert(IgnoreDeduced == false);
+DeducedTemplateStorage *DTS = Name.getAsDeducedTemplateName();
+DefaultArguments DefArgs = DTS->getDefaultArguments();
+TemplateName Underlying = DTS->getUnderlying();

mizvekov wrote:

They are pointer sized, it's the same as a QualType, and we never take those by 
reference either.

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


[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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

>From dadb9244bee22bc303af154b47f527b973940b40 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Wed, 12 Jun 2024 00:42:48 -0300
Subject: [PATCH] [clang] fix broken canonicalization of
 DeducedTemplateSpecializationType

This reverts the functional elements of commit 
3e78fa860235431323aaf08c8fa922d75a7cfffa
and redoes it, by fixing the true root cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical
name would result in inconsistent preservation of as-written information
in the AST.

The true problem in #61317 is that we would not consider the methods
with requirements expression which contain DTSTs as the same
in relation to merging of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing
to different redeclarations of the same class template, but since
canonicalization for them was broken, their canonical types would not
match either.

---

No changelog entry because #61317 was already claimed as fixed in
previous release.
---
 clang/include/clang/AST/ASTContext.h   |  7 +++
 clang/include/clang/AST/TemplateName.h |  4 +-
 clang/include/clang/AST/Type.h | 11 ++--
 clang/lib/AST/ASTContext.cpp   | 25 ++---
 clang/lib/AST/TemplateName.cpp |  9 
 clang/unittests/AST/CMakeLists.txt |  1 +
 clang/unittests/AST/ProfilingTest.cpp  | 75 ++
 7 files changed, 109 insertions(+), 23 deletions(-)
 create mode 100644 clang/unittests/AST/ProfilingTest.cpp

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index a1d1d1c51cd41..53ece996769a8 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase {
 QualType DeducedType,
 bool IsDependent) const;
 
+private:
+  QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
+QualType DeducedType,
+bool IsDependent,
+QualType Canon) const;
+
+public:
   /// Return the unique reference to the type for the specified TagDecl
   /// (struct/union/class/enum) decl.
   QualType getTagDeclType(const TagDecl *Decl) const;
diff --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index 988a55acd2252..24a7fde76195d 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -346,7 +346,9 @@ class TemplateName {
   /// error.
   void dump() const;
 
-  void Profile(llvm::FoldingSetNodeID &ID);
+  void Profile(llvm::FoldingSetNodeID &ID) {
+ID.AddPointer(Storage.getOpaqueValue());
+  }
 
   /// Retrieve the template name as a void pointer.
   void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9eb3f6c09e3d3..fab233b62d8d1 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
 
   DeducedTemplateSpecializationType(TemplateName Template,
 QualType DeducedAsType,
-bool IsDeducedAsDependent)
+bool IsDeducedAsDependent, QualType Canon)
   : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
 toTypeDependence(Template.getDependence()) |
 (IsDeducedAsDependent
  ? TypeDependence::DependentInstantiation
  : TypeDependence::None),
-DeducedAsType.isNull() ? QualType(this, 0)
-   : DeducedAsType.getCanonicalType()),
+Canon),
 Template(Template) {}
 
 public:
   /// Retrieve the name of the template that we are deducing.
   TemplateName getTemplateName() const { return Template;}
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
+  void Profile(llvm::FoldingSetNodeID &ID) const {
 Profile(ID, getTemplateName(), getDeducedType(), isDependentType());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-QualType CanonicalType =
-Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
-ID.AddPointer(CanonicalType.getAsOpaquePtr());
+Deduced.Profile(ID);
 ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index bf74e56a14799..34aa399fda2f8 

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

I don't think regression tests are the right level here, we risk creating an 
unstable test.

I have added a new unittest module for Profiling tests. I hope some day we will 
have time to add one test case for each property of each AST node.

Though I still think the first line of defense here should have been code 
review.

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


[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

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

>From 5c4fa3ce2ce23fdaf877b71b2775244d15a149d3 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Wed, 12 Jun 2024 00:42:48 -0300
Subject: [PATCH] [clang] fix broken canonicalization of
 DeducedTemplateSpecializationType

This reverts the functional elements of commit 
3e78fa860235431323aaf08c8fa922d75a7cfffa
and redoes it, by fixing the true root cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical
name would result in inconsistent preservation of as-written information
in the AST.

The true problem in #61317 is that we would not consider the methods
with requirements expression which contain DTSTs as the same
in relation to merging of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing
to different redeclarations of the same class template, but since
canonicalization for them was broken, their canonical types would not
match either.

---

No changelog entry because #61317 was already claimed as fixed in
previous release.
---
 clang/include/clang/AST/ASTContext.h   |  4 ++
 clang/include/clang/AST/TemplateName.h |  4 +-
 clang/include/clang/AST/Type.h | 11 ++--
 clang/lib/AST/ASTContext.cpp   | 25 ++---
 clang/lib/AST/TemplateName.cpp |  9 
 clang/unittests/AST/CMakeLists.txt |  1 +
 clang/unittests/AST/ProfilingTest.cpp  | 75 ++
 7 files changed, 106 insertions(+), 23 deletions(-)
 create mode 100644 clang/unittests/AST/ProfilingTest.cpp

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index a1d1d1c51cd41..5985887000d44 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1770,6 +1770,10 @@ class ASTContext : public RefCountedBase {
   QualType getDeducedTemplateSpecializationType(TemplateName Template,
 QualType DeducedType,
 bool IsDependent) const;
+  QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
+QualType DeducedType,
+bool IsDependent,
+QualType Canon) const;
 
   /// Return the unique reference to the type for the specified TagDecl
   /// (struct/union/class/enum) decl.
diff --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index 988a55acd2252..24a7fde76195d 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -346,7 +346,9 @@ class TemplateName {
   /// error.
   void dump() const;
 
-  void Profile(llvm::FoldingSetNodeID &ID);
+  void Profile(llvm::FoldingSetNodeID &ID) {
+ID.AddPointer(Storage.getOpaqueValue());
+  }
 
   /// Retrieve the template name as a void pointer.
   void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9eb3f6c09e3d3..fab233b62d8d1 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
 
   DeducedTemplateSpecializationType(TemplateName Template,
 QualType DeducedAsType,
-bool IsDeducedAsDependent)
+bool IsDeducedAsDependent, QualType Canon)
   : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
 toTypeDependence(Template.getDependence()) |
 (IsDeducedAsDependent
  ? TypeDependence::DependentInstantiation
  : TypeDependence::None),
-DeducedAsType.isNull() ? QualType(this, 0)
-   : DeducedAsType.getCanonicalType()),
+Canon),
 Template(Template) {}
 
 public:
   /// Retrieve the name of the template that we are deducing.
   TemplateName getTemplateName() const { return Template;}
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
+  void Profile(llvm::FoldingSetNodeID &ID) const {
 Profile(ID, getTemplateName(), getDeducedType(), isDependentType());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-QualType CanonicalType =
-Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
-ID.AddPointer(CanonicalType.getAsOpaquePtr());
+Deduced.Profile(ID);
 ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index bf74e56a14799..34aa399fda2f8 10064

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:


> Or, are you saying it is too hard to get reduced?
> 
> 

I don't have a reduced test case. It's not impossible to reduce. Though we 
usually do a poor job of preserving TemplateNames in other places, which makes 
this harder to test. I have patches on my pipeline dealing with this though.

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


[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

It still fixes the original bug report though.

Otherwise adding a test which directly tests all observable effects of the 
profiling fix would be a bit arbitrary, as we don't have any unit tests for the 
gazillion ways this could go wrong for the other AST nodes.

This usually gets caught incidentally in diagnostics for unrelated tests. In 
this case that incidental test was in libc++.

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


[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

2024-06-12 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/95202

This reverts the functional elements of commit 
3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root 
cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical name 
would result in inconsistent preservation of as-written information in the AST.

The true problem in #61317 is that we would not consider the methods with 
requirements expression which contain DTSTs as the same in relation to merging 
of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing to 
different redeclarations of the same class template, but since canonicalization 
for them was broken, their canonical types would not match either.

---

No changelog entry because #61317 was already claimed as fixed in previous 
release.

>From 71a827e8ff6dcc47f1fd434a19070e6f841ee27a Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Wed, 12 Jun 2024 00:42:48 -0300
Subject: [PATCH] [clang] fix broken canonicalization of
 DeducedTemplateSpecializationType

This reverts the functional elements of commit 
3e78fa860235431323aaf08c8fa922d75a7cfffa
and redoes it, by fixing the true root cause of #61317.

A TemplateName can be non-canonical; profiling it based on the canonical
name would result in inconsistent preservation of as-written information
in the AST.

The true problem in #61317 is that we would not consider the methods
with requirements expression which contain DTSTs as the same
in relation to merging of declarations when importing modules.

The expressions would never match because they contained DTSTs pointing
to different redeclarations of the same class template, but since
canonicalization for them was broken, their canonical types would not
match either.

---

No changelog entry because #61317 was already claimed as fixed in
previous release.
---
 clang/include/clang/AST/ASTContext.h   |  4 
 clang/include/clang/AST/TemplateName.h |  4 +++-
 clang/include/clang/AST/Type.h |  9 +++--
 clang/lib/AST/ASTContext.cpp   | 25 +++--
 clang/lib/AST/TemplateName.cpp |  9 -
 5 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index a1d1d1c51cd41..5985887000d44 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1770,6 +1770,10 @@ class ASTContext : public RefCountedBase {
   QualType getDeducedTemplateSpecializationType(TemplateName Template,
 QualType DeducedType,
 bool IsDependent) const;
+  QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template,
+QualType DeducedType,
+bool IsDependent,
+QualType Canon) const;
 
   /// Return the unique reference to the type for the specified TagDecl
   /// (struct/union/class/enum) decl.
diff --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index 988a55acd2252..24a7fde76195d 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -346,7 +346,9 @@ class TemplateName {
   /// error.
   void dump() const;
 
-  void Profile(llvm::FoldingSetNodeID &ID);
+  void Profile(llvm::FoldingSetNodeID &ID) {
+ID.AddPointer(Storage.getOpaqueValue());
+  }
 
   /// Retrieve the template name as a void pointer.
   void *getAsVoidPointer() const { return Storage.getOpaqueValue(); }
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 9eb3f6c09e3d3..f557697da74c6 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -6050,14 +6050,13 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
 
   DeducedTemplateSpecializationType(TemplateName Template,
 QualType DeducedAsType,
-bool IsDeducedAsDependent)
+bool IsDeducedAsDependent, QualType Canon)
   : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
 toTypeDependence(Template.getDependence()) |
 (IsDeducedAsDependent
  ? TypeDependence::DependentInstantiation
  : TypeDependence::None),
-DeducedAsType.isNull() ? QualType(this, 0)
-   : DeducedAsType.getCanonicalType()),
+Canon),
 Template(Template) {}
 
 public:
@@ -6071,9 +6070,7 @@ class DeducedTemplateSpecializationType : public 
DeducedType,
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName

[clang] [Clang] Initialize AtLeastAsSpecialized to prevent undefined behavior in Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs() (PR #95195)

2024-06-11 Thread Matheus Izvekov via cfe-commits


@@ -6447,7 +6447,7 @@ bool 
Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   if (Inst.isInvalid())
 return false;
 
-  bool AtLeastAsSpecialized;
+  bool AtLeastAsSpecialized = false;
   runWithSufficientStackSpace(Info.getLocation(), [&] {

mizvekov wrote:

runWithSufficientStackSpace is a small helper used to prevent stack exhaustion.

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


[clang] [Clang] Initialize AtLeastAsSpecialized to prevent undefined behavior in Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs() (PR #95195)

2024-06-11 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

There is no potential UB, this is a false positive: this lambda will always be 
executed before runWithSufficientStackSpace returns.

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


[clang] [StructuralEquivalence] improve NTTP and CXXDependentScopeMemberExpr comparison (PR #95190)

2024-06-11 Thread Matheus Izvekov via cfe-commits

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

LGTM, Thanks!

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


[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-11 Thread Matheus Izvekov via cfe-commits

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

>From d12c7d50b67cd669f09b3701ccf34154876786c9 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |   8 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  59 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 129 --
 clang/lib/AST/ASTDiagnostic.cpp   |  24 +--
 clang/lib/AST/ASTImporter.cpp |  15 ++
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/ItaniumMangle.cpp   |  11 ++
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 157 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 128 --
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  60 +--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 25 files changed, 564 insertions(+), 208 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 #undef TEMPLATE_KIND
 }
 llvm_unreachable("Unhandled NameKind enum");
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a366f1331c2d3..e6d16af2495fe 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1120,6 +1120,7 @@ class CollectExtraHighlightings
 case TemplateName::SubstTemplateTemplateParm:
 case TemplateName::SubstTemplateTemplateParmPack:
 case TemplateName::UsingTemplate:
+case TemplateName::DeducedTemplate:
   // Names that could be resolved to a TemplateDecl are handled elsewhere.
   break;
 }
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 8bce4812f0d48..8818314de9364 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::ContextualFoldingSet
 SubstTemplateTemplateParmPacks;
+  mutable llvm::ContextualFoldingSet
+  DeducedTemplates;
 
   mutable llvm::ContextualFoldingSet
   ArrayParameterTypes;
@@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase {
 unsigned Index,

[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-10 Thread Matheus Izvekov via cfe-commits

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

>From 68791782b7a1a0eafa98950f6e03aa1585be5223 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |   8 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  59 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 129 --
 clang/lib/AST/ASTDiagnostic.cpp   |  24 +--
 clang/lib/AST/ASTImporter.cpp |  92 +-
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/ItaniumMangle.cpp   |  11 ++
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 157 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 128 --
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  60 +--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 25 files changed, 594 insertions(+), 255 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 #undef TEMPLATE_KIND
 }
 llvm_unreachable("Unhandled NameKind enum");
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a366f1331c2d3..e6d16af2495fe 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1120,6 +1120,7 @@ class CollectExtraHighlightings
 case TemplateName::SubstTemplateTemplateParm:
 case TemplateName::SubstTemplateTemplateParmPack:
 case TemplateName::UsingTemplate:
+case TemplateName::DeducedTemplate:
   // Names that could be resolved to a TemplateDecl are handled elsewhere.
   break;
 }
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 8bce4812f0d48..8818314de9364 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::ContextualFoldingSet
 SubstTemplateTemplateParmPacks;
+  mutable llvm::ContextualFoldingSet
+  DeducedTemplates;
 
   mutable llvm::ContextualFoldingSet
   ArrayParameterTypes;
@@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase {
 unsigned Index,

[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-10 Thread Matheus Izvekov via cfe-commits

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

>From ea98dec85a9817eb4e35ce97389433e4a5b9676d Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |   8 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  59 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 129 --
 clang/lib/AST/ASTDiagnostic.cpp   |  24 +--
 clang/lib/AST/ASTImporter.cpp |  92 +-
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/ItaniumMangle.cpp   |  11 ++
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 157 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 134 ---
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  60 +--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 25 files changed, 600 insertions(+), 255 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 #undef TEMPLATE_KIND
 }
 llvm_unreachable("Unhandled NameKind enum");
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a366f1331c2d3..e6d16af2495fe 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1120,6 +1120,7 @@ class CollectExtraHighlightings
 case TemplateName::SubstTemplateTemplateParm:
 case TemplateName::SubstTemplateTemplateParmPack:
 case TemplateName::UsingTemplate:
+case TemplateName::DeducedTemplate:
   // Names that could be resolved to a TemplateDecl are handled elsewhere.
   break;
 }
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 8bce4812f0d48..8818314de9364 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::ContextualFoldingSet
 SubstTemplateTemplateParmPacks;
+  mutable llvm::ContextualFoldingSet
+  DeducedTemplates;
 
   mutable llvm::ContextualFoldingSet
   ArrayParameterTypes;
@@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase {
 unsigned Index,
   

[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-10 Thread Matheus Izvekov via cfe-commits

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

>From 015a05707caad5d39909bc68a5371161de464d9d Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |   8 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  59 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 129 --
 clang/lib/AST/ASTDiagnostic.cpp   |  24 +--
 clang/lib/AST/ASTImporter.cpp |  92 +-
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/ItaniumMangle.cpp   |  11 ++
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 157 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 134 ---
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  44 +++--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 25 files changed, 584 insertions(+), 255 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 #undef TEMPLATE_KIND
 }
 llvm_unreachable("Unhandled NameKind enum");
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a366f1331c2d3..e6d16af2495fe 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -1120,6 +1120,7 @@ class CollectExtraHighlightings
 case TemplateName::SubstTemplateTemplateParm:
 case TemplateName::SubstTemplateTemplateParmPack:
 case TemplateName::UsingTemplate:
+case TemplateName::DeducedTemplate:
   // Names that could be resolved to a TemplateDecl are handled elsewhere.
   break;
 }
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 8bce4812f0d48..8818314de9364 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase {
   mutable llvm::ContextualFoldingSet
 SubstTemplateTemplateParmPacks;
+  mutable llvm::ContextualFoldingSet
+  DeducedTemplates;
 
   mutable llvm::ContextualFoldingSet
   ArrayParameterTypes;
@@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase {
 unsigned Index,
 

[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)

2024-06-10 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/94981

This extends default argument deduction to cover class templates as well, and 
also applies outside of partial ordering, adding to the provisional wording 
introduced in https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how template template 
parameters are partially ordered, and should reduce the negative impact of 
enabling `-frelaxed-template-template-args` by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch 
restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering, the following code 
becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f' deduced from 
`A`, this implements an additional mangling rule.

---

Since this changes provisional implementation of CWG2398 which has not been 
released yet, and already contains a changelog entry, we don't provide a 
changelog entry here.

>From 28bc152e72d458e91aee90d46763eea04091ebe2 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Mon, 20 May 2024 01:15:03 -0300
Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class
 templates

This extends default argument deduction to cover class templates as
well, and also applies outside of partial ordering, adding to the
provisional wording introduced in 
https://github.com/llvm/llvm-project/pull/89807.

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

Given the following example:
```C++
template  struct A;
template  struct B;

template  class TT1, class T5> struct B>;   // #1
template   struct B>; // #2

template struct B>;
```
Prior to P0522, `#2` was picked. Afterwards, this became ambiguous.
This patch restores the pre-P0522 behavior, `#2` is picked again.

As the consequences are not restricted to partial ordering,
the following code becomes valid:
```C++
template struct A {};
A v;
template class TT> void f(TT);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }
```

Also, since 'f' deduced from `A` is different from 'f'
deduced from `A`, this implements an additional mangling
rule.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang-tools-extra/clangd/DumpAST.cpp  |   1 +
 .../clangd/SemanticHighlighting.cpp   |   1 +
 clang/include/clang/AST/ASTContext.h  |   8 +-
 clang/include/clang/AST/ASTImporter.h |   5 +
 clang/include/clang/AST/DependenceFlags.h |   5 +
 clang/include/clang/AST/PropertiesBase.td |  17 ++
 clang/include/clang/AST/TemplateName.h|  59 ++-
 clang/include/clang/Sema/Sema.h   |  10 +-
 clang/lib/AST/ASTContext.cpp  | 129 --
 clang/lib/AST/ASTDiagnostic.cpp   |  24 +--
 clang/lib/AST/ASTImporter.cpp |  92 +-
 clang/lib/AST/ASTStructuralEquivalence.cpp|   3 +
 clang/lib/AST/ItaniumMangle.cpp   |  11 ++
 clang/lib/AST/ODRHash.cpp |   1 +
 clang/lib/AST/TemplateName.cpp| 157 ++
 clang/lib/AST/TextNodeDumper.cpp  |  12 ++
 clang/lib/AST/Type.cpp|   3 +-
 clang/lib/Sema/SemaTemplate.cpp   |  63 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 133 ---
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  24 +--
 .../CXX/temp/temp.decls/temp.alias/p2.cpp |   5 +-
 clang/test/CodeGenCXX/mangle-cwg2398.cpp  |  11 ++
 clang/test/SemaTemplate/cwg2398.cpp   |  43 +++--
 clang/tools/libclang/CIndex.cpp   |   3 +
 clang/unittests/AST/ASTImporterTest.cpp   |  17 ++
 25 files changed, 582 insertions(+), 255 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp

diff --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9a525efb938e8..e605f82e91fe4 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor 
{
   TEMPLATE_KIND(SubstTemplateTemplateParm);
   TEMPLATE_KIND(SubstTemplateTemplateParmPack);
   TEMPLATE_KIND(UsingTemplate);
+  TEMPLATE_KIND(DeducedTemplate);
 

[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)

2024-06-10 Thread Matheus Izvekov via cfe-commits

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


[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)

2024-06-10 Thread Matheus Izvekov via cfe-commits

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

>From f918649a9208c2250873eb63641e106478371b43 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sat, 8 Jun 2024 11:07:27 -0300
Subject: [PATCH] [clang] improve TemplateArgument and TemplateName dump
 methods

These will work as AST Text node dumpers, as usual, instead of AST
printers.

Note that for now, the TemplateName dumper is using the TemplateArgument
dumper through an implicit conversion.
This can be fixed in a later pass.
---
 clang/include/clang/AST/TemplateBase.h |  2 +-
 clang/include/clang/AST/TemplateName.h |  2 +-
 clang/lib/AST/ASTDumper.cpp| 34 ++
 clang/lib/AST/TemplateBase.cpp |  9 ---
 clang/lib/AST/TemplateName.cpp | 11 -
 5 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/TemplateBase.h 
b/clang/include/clang/AST/TemplateBase.h
index fea2c8ccfee67..0eaa4b0eedb35 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -459,7 +459,7 @@ class TemplateArgument {
  bool IncludeType) const;
 
   /// Debugging aid that dumps the template argument.
-  void dump(raw_ostream &Out) const;
+  void dump(raw_ostream &Out, const ASTContext &Context) const;
 
   /// Debugging aid that dumps the template argument to standard error.
   void dump() const;
diff --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index 489fccb2ef74d..988a55acd2252 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -340,7 +340,7 @@ class TemplateName {
  Qualified Qual = Qualified::AsWritten) const;
 
   /// Debugging aid that dumps the template name.
-  void dump(raw_ostream &OS) const;
+  void dump(raw_ostream &OS, const ASTContext &Context) const;
 
   /// Debugging aid that dumps the template name to standard
   /// error.
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index c8973fdeda352..f0603880c32dd 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -360,3 +360,37 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream 
&OS) const {
   ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
   P.Visit(this);
 }
+
+//===--===//
+// TemplateName method implementations
+//===--===//
+
+// FIXME: These are actually using the TemplateArgument dumper, through
+// an implicit conversion. The dump will claim this is a template argument,
+// which is misleading.
+
+LLVM_DUMP_METHOD void TemplateName::dump() const {
+  ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false);
+  Dumper.Visit(*this);
+}
+
+LLVM_DUMP_METHOD void TemplateName::dump(llvm::raw_ostream &OS,
+ const ASTContext &Context) const {
+  ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors());
+  Dumper.Visit(*this);
+}
+
+//===--===//
+// TemplateArgument method implementations
+//===--===//
+
+LLVM_DUMP_METHOD void TemplateArgument::dump() const {
+  ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false);
+  Dumper.Visit(*this);
+}
+
+LLVM_DUMP_METHOD void TemplateArgument::dump(llvm::raw_ostream &OS,
+ const ASTContext &Context) const {
+  ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors());
+  Dumper.Visit(*this);
+}
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 46f7b79b272ef..2e6839e948d9d 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -577,15 +577,6 @@ void TemplateArgument::print(const PrintingPolicy &Policy, 
raw_ostream &Out,
   }
 }
 
-void TemplateArgument::dump(raw_ostream &Out) const {
-  LangOptions LO; // FIXME! see also TemplateName::dump().
-  LO.CPlusPlus = true;
-  LO.Bool = true;
-  print(PrintingPolicy(LO), Out, /*IncludeType*/ true);
-}
-
-LLVM_DUMP_METHOD void TemplateArgument::dump() const { dump(llvm::errs()); }
-
 
//===--===//
 // TemplateArgumentLoc Implementation
 
//===--===//
diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp
index 4fc25cb34803e..11544dbb56e31 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -360,14 +360,3 @@ const StreamingDiagnostic &clang::operator<<(const 
StreamingDiagnostic &DB,
   OS.flush();
   return DB << NameStr;
 }
-
-void TemplateName::dump(raw_ostream &OS) const {
-  LangOptions LO;  // FIXME!
-  LO.CPlusPlus = true;
-  LO.Bool = true;
-  print(OS, Pri

[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)

2024-06-10 Thread Matheus Izvekov via cfe-commits


@@ -360,3 +360,34 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream 
&OS) const {
   ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
   P.Visit(this);
 }
+
+//===--===//
+// TemplateName method implementations
+//===--===//
+
+// FIXME: These are using the TemplateArgument dumper.

mizvekov wrote:

Well you are going to hit dump on a TemplateName, and it's going to dump a 
TemplateArgument of template kind, which is misleading and incorrect, but still 
helpful in the narrow context this is a debugging aid not actually used in 
production.

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


[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)

2024-06-09 Thread Matheus Izvekov via cfe-commits

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


[clang] [clang] improve TemplateArgument and TemplateName dump methods (PR #94905)

2024-06-09 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/94905

These will work as AST Text node dumpers, as usual, instead of AST printers.

Note that for now, the TemplateName dumper is using the TemplateArgument dumper 
through an implicit conversion.
This can be fixed in a later pass.

>From fb159ba1974e9311818892950c301f590bbe6360 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sat, 8 Jun 2024 11:07:27 -0300
Subject: [PATCH] [clang] improve TemplateArgument and TemplateName dump
 methods

These will work as AST Text node dumpers, as usual, instead of AST
printers.

Note that for now, the TemplateName dumper is using the TemplateArgument
dumper through an implicit conversion.
This can be fixed in a later pass.
---
 clang/include/clang/AST/TemplateBase.h |  2 +-
 clang/include/clang/AST/TemplateName.h |  2 +-
 clang/lib/AST/ASTDumper.cpp| 31 ++
 clang/lib/AST/TemplateBase.cpp |  9 
 clang/lib/AST/TemplateName.cpp | 11 -
 5 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/TemplateBase.h 
b/clang/include/clang/AST/TemplateBase.h
index fea2c8ccfee67..0eaa4b0eedb35 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -459,7 +459,7 @@ class TemplateArgument {
  bool IncludeType) const;
 
   /// Debugging aid that dumps the template argument.
-  void dump(raw_ostream &Out) const;
+  void dump(raw_ostream &Out, const ASTContext &Context) const;
 
   /// Debugging aid that dumps the template argument to standard error.
   void dump() const;
diff --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index 489fccb2ef74d..988a55acd2252 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -340,7 +340,7 @@ class TemplateName {
  Qualified Qual = Qualified::AsWritten) const;
 
   /// Debugging aid that dumps the template name.
-  void dump(raw_ostream &OS) const;
+  void dump(raw_ostream &OS, const ASTContext &Context) const;
 
   /// Debugging aid that dumps the template name to standard
   /// error.
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index c8973fdeda352..869527241f2c8 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -360,3 +360,34 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream 
&OS) const {
   ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
   P.Visit(this);
 }
+
+//===--===//
+// TemplateName method implementations
+//===--===//
+
+// FIXME: These are using the TemplateArgument dumper.
+LLVM_DUMP_METHOD void TemplateName::dump() const {
+  ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false);
+  Dumper.Visit(*this);
+}
+
+LLVM_DUMP_METHOD void TemplateName::dump(llvm::raw_ostream &OS,
+ const ASTContext &Context) const {
+  ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors());
+  Dumper.Visit(*this);
+}
+
+//===--===//
+// TemplateArgument method implementations
+//===--===//
+
+LLVM_DUMP_METHOD void TemplateArgument::dump() const {
+  ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false);
+  Dumper.Visit(*this);
+}
+
+LLVM_DUMP_METHOD void TemplateArgument::dump(llvm::raw_ostream &OS,
+ const ASTContext &Context) const {
+  ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors());
+  Dumper.Visit(*this);
+}
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 46f7b79b272ef..2e6839e948d9d 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -577,15 +577,6 @@ void TemplateArgument::print(const PrintingPolicy &Policy, 
raw_ostream &Out,
   }
 }
 
-void TemplateArgument::dump(raw_ostream &Out) const {
-  LangOptions LO; // FIXME! see also TemplateName::dump().
-  LO.CPlusPlus = true;
-  LO.Bool = true;
-  print(PrintingPolicy(LO), Out, /*IncludeType*/ true);
-}
-
-LLVM_DUMP_METHOD void TemplateArgument::dump() const { dump(llvm::errs()); }
-
 
//===--===//
 // TemplateArgumentLoc Implementation
 
//===--===//
diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp
index 4fc25cb34803e..11544dbb56e31 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -360,14 +360,3 @@ const StreamingDiagnostic &clang::operator<<(const 
StreamingDiagnostic &DB,
   OS.flush();
   return DB << NameStr;
 }
-
-void TemplateName::dump(raw_ost

[clang] e090bac - [clang] NFC: add new cwg2398 tests

2024-06-09 Thread Matheus Izvekov via cfe-commits

Author: Matheus Izvekov
Date: 2024-06-09T13:44:45-03:00
New Revision: e090bac638e56ff9db87e622cdf925f2b99dfc30

URL: 
https://github.com/llvm/llvm-project/commit/e090bac638e56ff9db87e622cdf925f2b99dfc30
DIFF: 
https://github.com/llvm/llvm-project/commit/e090bac638e56ff9db87e622cdf925f2b99dfc30.diff

LOG: [clang] NFC: add new cwg2398 tests

Added: 


Modified: 
clang/test/SemaTemplate/cwg2398.cpp

Removed: 




diff  --git a/clang/test/SemaTemplate/cwg2398.cpp 
b/clang/test/SemaTemplate/cwg2398.cpp
index f7f69e9d4268a..7675d4287cb88 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -200,8 +200,120 @@ namespace consistency {
 template struct A, B, B>;
 // new-error@-1 {{ambiguous partial specializations}}
   } // namespace t2
+  namespace t3 {
+template struct A;
+
+template class TT1,
+ class T1, class T2, class T3, class T4>
+struct A, TT1, typename nondeduced>::type> 
{};
+// new-note@-1 {{partial specialization matches}}
+
+template class UU1,
+ class U1, class U2>
+struct A, UU1, typename nondeduced>::type>;
+// new-note@-1 {{partial specialization matches}}
+
+template struct A, B, B>;
+// new-error@-1 {{ambiguous partial specializations}}
+  } // namespace t3
+  namespace t4 {
+template struct A;
+
+template class TT1,
+ class T1, class T2, class T3, class T4>
+struct A, TT1, typename nondeduced>::type> 
{};
+// new-note@-1 {{partial specialization matches}}
+
+template class UU1,
+ class U1, class U2>
+struct A, UU1, typename nondeduced>::type>;
+// new-note@-1 {{partial specialization matches}}
+
+template struct A, B, B>;
+// new-error@-1 {{ambiguous partial specializations}}
+  } // namespace t4
+  namespace t5 {
+template struct A;
+
+template class TT1,
+ class T1, class T2, class T3, class T4>
+struct A, TT1> {};
+// new-note@-1 {{partial specialization matches}}
+
+template class UU1,
+ class U1, class U2>
+struct A, UU1>;
+// new-note@-1 {{partial specialization matches}}
+
+template struct A, B>;
+// new-error@-1 {{ambiguous partial specializations}}
+  } // namespace t5
+  namespace t6 {
+template struct A;
+
+template class TT1,
+ class T1, class T2, class T3>
+struct A, TT1> {};
+// new-note@-1 {{partial specialization matches}}
+
+template class UU1,
+ class U1, class U2>
+struct A, UU1>;
+// new-note@-1 {{partial specialization matches}}
+
+template struct A, B>;
+// new-error@-1 {{ambiguous partial specializations}}
+  } // namespace t6
 } // namespace consistency
 
+namespace classes {
+  namespace canon {
+template struct A {};
+
+template class TT> auto f(TT a) { return a; }
+// old-note@-1 2{{template template argument has 
diff erent template parameters}}
+// new-note@-2 2{{substitution failure: too few template arguments}}
+
+A v1;
+A v2;
+
+using X = decltype(f(v1));
+// expected-error@-1 {{no matching function for call}}
+
+using X = decltype(f(v2));
+// expected-error@-1 {{no matching function for call}}
+  } // namespace canon
+  namespace expr {
+template  struct A {
+  static constexpr auto val = E1;
+};
+template  class TT> void f(TT v) {
+  // old-note@-1 {{template template argument has 
diff erent template parameters}}
+  // new-note@-2 {{substitution failure: too few template arguments}}
+  static_assert(v.val == 3);
+};
+void test() {
+  f(A());
+  // expected-error@-1 {{no matching function for call}}
+}
+  } // namespace expr
+  namespace packs {
+template  struct A {
+  static constexpr auto val = sizeof...(T2s);
+};
+
+template  class TT> void f(TT v) {
+  // old-note@-1 {{template template argument has 
diff erent template parameters}}
+  // new-note@-2 {{deduced type 'A<[...], (no argument), (no argument), 
(no argument)>' of 1st parameter does not match adjusted type 'A<[...], void, 
void, void>' of argument [with TT = A]}}
+  static_assert(v.val == 3);
+};
+void test() {
+  f(A());
+  // expected-error@-1 {{no matching function for call}}
+}
+  } // namespace packs
+} // namespace classes
+
 namespace regression1 {
   template  struct map {};
   template  class foo {};



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


[clang] [llvm] Enable LLDB tests in Linux pre-merge CI (PR #94208)

2024-06-08 Thread Matheus Izvekov via cfe-commits

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


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


[clang] [llvm] Enable LLDB tests in Linux pre-merge CI (PR #94208)

2024-06-07 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

@Endilll This looks good, thanks!

How will this affect test capacity? Right now, the Linux bots are lagging, 
while the Windows bot is breezing through.

This is the opposite of the usual. Are we under-provisioned on Linux CI 
resources? How much worse will it get when we add this extra workload?

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


[clang] [clang] always use resolved arguments for default argument deduction (PR #94756)

2024-06-07 Thread Matheus Izvekov via cfe-commits

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


[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)

2024-06-07 Thread Matheus Izvekov via cfe-commits


@@ -390,29 +390,37 @@ bool Sema::isAcceptableNestedNameSpecifier(const 
NamedDecl *SD,
 /// (e.g., Base::), perform name lookup for that identifier as a
 /// nested-name-specifier within the given scope, and return the result of that
 /// name lookup.
-NamedDecl *Sema::FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS) 
{
-  if (!S || !NNS)
-return nullptr;
+bool Sema::LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS,
+   UnresolvedSetImpl &R) {
+  if (!S)
+return false;
 
   while (NNS->getPrefix())
 NNS = NNS->getPrefix();
 
-  if (NNS->getKind() != NestedNameSpecifier::Identifier)
-return nullptr;
-
-  LookupResult Found(*this, NNS->getAsIdentifier(), SourceLocation(),
- LookupNestedNameSpecifierName);
+  // FIXME: This is a rather nasty hack! Ideally we should get the results

mizvekov wrote:

I don't think this is a hack per se, I think this is just a consequence of not 
having a special NNS kind for this situation, and representing it with a DTST.

The alternative that I see is to implement a new NNS prefix which is composed 
by an identifier followed by template arguments.

It can still be represented internally with the type, if that's cheaper, but 
having a special accessor, and making `NNS->getAsIdentifier()` work for it, 
would be nicer.

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


[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)

2024-06-07 Thread Matheus Izvekov via cfe-commits


@@ -548,6 +575,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S, 
NestedNameSpecInfo &IdInfo,
 // Perform unqualified name lookup in the current scope.
 LookupName(Found, S);
   }
+#endif

mizvekov wrote:

A left-over.

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


[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)

2024-06-07 Thread Matheus Izvekov via cfe-commits

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

LGTM save a few minor cleanups needed.

I think this patch has an incredible amount of noise due to amount of cosmetic 
changes caused by reformatting, specially due to changing parameters and such.

This is fine for now and I am not suggesting you go back and do it for this 
patch, but I think it would have been helpful to pre-clang-format the file, and 
offload more of the minor changes into a previous patch.

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


[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)

2024-06-07 Thread Matheus Izvekov via cfe-commits


@@ -2923,10 +2920,9 @@ class TreeTransform {
 }
 
 return getSema().BuildMemberReferenceExpr(Base, BaseType, OpLoc, isArrow,
-  SS, TemplateKWLoc,
-  FirstQualifierInScope,
-  R, ExplicitTemplateArgs,
-  /*S*/nullptr);
+  SS, TemplateKWLoc, R,
+  ExplicitTemplateArgs,
+  /*S*/ nullptr);

mizvekov wrote:

```suggestion
  /*S=*/nullptr);
```

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


[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)

2024-06-07 Thread Matheus Izvekov via cfe-commits

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


[clang] [clang] always use resolved arguments for default argument deduction (PR #94756)

2024-06-07 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

@kadircet @yozhu FYI this fixes the problem you reported.

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


[clang] [clang] always use resolved arguments for default argument deduction (PR #94756)

2024-06-07 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/94756

This fixes a regression introduced with the changes in 
https://github.com/llvm/llvm-project/pull/93433 around preservation of 
TemplateName sugar in template type deduction.

Since the argument side TST is non-canonical, we have to extract the arguments 
from it's canonical type.
This was done for the deduction of the TST arguments, but we missed it for the 
default arguments used in the deduction of the TST name.

>From df6049fd91c95e341dd80a61e5bd173ce5837131 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Fri, 7 Jun 2024 10:32:12 -0300
Subject: [PATCH] [clang] always use resolved arguments for default argument
 deduction

This fixes a regression introduced with the changes in
https://github.com/llvm/llvm-project/pull/93433 around preservation
of TemplateName sugar in template type deduction.

Since the argument side TST is non-canonical, we have to extract the
arguments from it's canonical type.
This was done for the deduction of the TST arguments, but we missed it
for the default arguments used in the deduction of the TST name.
---
 clang/lib/Sema/SemaTemplateDeduction.cpp | 13 ++---
 clang/test/SemaTemplate/cwg2398.cpp  | 16 
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 1011db2d2830d..befeb38e1fe5b 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -712,13 +712,6 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 if (const auto *TD = TNA.getAsTemplateDecl(); TD && TD->isTypeAlias())
   return TemplateDeductionResult::Success;
 
-// Perform template argument deduction for the template name.
-if (auto Result =
-DeduceTemplateArguments(S, TemplateParams, TNP, TNA, Info,
-SA->template_arguments(), Deduced);
-Result != TemplateDeductionResult::Success)
-  return Result;
-
 // FIXME: To preserve sugar, the TST needs to carry sugared resolved
 // arguments.
 ArrayRef AResolved =
@@ -726,6 +719,12 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList 
*TemplateParams,
 ->castAs()
 ->template_arguments();
 
+// Perform template argument deduction for the template name.
+if (auto Result = DeduceTemplateArguments(S, TemplateParams, TNP, TNA, 
Info,
+  AResolved, Deduced);
+Result != TemplateDeductionResult::Success)
+  return Result;
+
 // Perform template argument deduction on each template
 // argument. Ignore any missing/extra arguments, since they could be
 // filled in by default arguments.
diff --git a/clang/test/SemaTemplate/cwg2398.cpp 
b/clang/test/SemaTemplate/cwg2398.cpp
index 45e74cce3a98c..f7f69e9d4268a 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -201,3 +201,19 @@ namespace consistency {
 // new-error@-1 {{ambiguous partial specializations}}
   } // namespace t2
 } // namespace consistency
+
+namespace regression1 {
+  template  struct map {};
+  template  class foo {};
+
+  template  class MapType, typename Value>
+  Value bar(MapType map);
+
+  template  class MapType, typename Value>
+  Value bar(MapType> map);
+
+  void aux() {
+map> input;
+bar(input);
+  }
+} // namespace regression1

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


[clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)

2024-06-07 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

> so after this patch, clang seems to be crashing on:

Thanks for the reproducer. Yeah this is about default argument deduction, which 
doesn't involve default arguments as written in source.

I confirm the crash and it's indeed caused by changes in this patch. The fix is 
very simple and I will be posting a patch shortly.



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


[clang] [Clang][Sema][TemplateDeduction] Skip pack expansion type at the end of default template argument list if unneeded (PR #94659)

2024-06-06 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov requested changes to this pull request.

Please include test case.

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


<    1   2   3   4   5   6   7   8   9   10   >