[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

No problem!

It looks like this example is salvageable, nothing is stopping us from just 
applying the same rules when deducing a template template parameter against 
other kinds of templates.

This shouldn't stop you from cleaning up the code, whatever rules we come up 
here are for salvaging old code, not necessarily to entomb these as examples of 
good practice.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-17 Thread via cfe-commits

joanahalili wrote:

Thank you for the quick response to this! 
I have come up with a small repro to illustrate the breakages we are 
encountering https://godbolt.org/z/rM518enr4. 
We have a number of cases similar to this that we are now working to fix.
Thanks!

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-16 Thread via cfe-commits

cor3ntin wrote:

@joanahalili 
> We are seeing a widespread breakage due to this commit and could use having 
> the flag -fno-relaxed-template-template-args un-deprecated for a while. This 
> would help do a cleanup on our end.

Can you provide more information? How widespread are we talking about? You have 
example of common breakages?
Thanks

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

@joanahalili This is now merged in main: 
https://github.com/llvm/llvm-project/pull/92324

You can pass `-Wno-deprecated-no-relaxed-template-template-args` to disable the 
deprecation warning for `-fno-relaxed-template-template-args` specifically, 
without affecting other warnings.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-15 Thread Erich Keane via cfe-commits

erichkeane wrote:

Great, I agree that is the right way forward.  A more complicated flag like 
that might be cool someday, but not something we need today.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> I think I prefer pretty fine-grained ones TBH, it makes our deprecation 
> warnings more valuable. In a perfect world, it would change every release of 
> the compiler so that folks would be frequently reminded of it, but it isn't a 
> perfect world :)

I meant something fine grained, but generic, like 
`-Wno-deprecated-flag=-fno-relaxed-template-template-args`.
We have something similar for generic warnings.
That would probably take a little longer to get everyone onboard the design, 
and may lack other motivators presently.
 
> Is doing so much of a task? I would expect it to be like other diagnostics 
> and take roughly the same amount of time as reverting the 
> negative-spelling-deprecation. I don't see the request to remove the 
> deprecation (it IS just a warning afterall!) to be particularly motivating 
> until release time (it IS just a warning afterall!), so I'd think it would a 
> better spending of time to implement what Richard suggested.

Right, it takes about the same amount of time, so we may as well implement the 
simple thing, and argue about something else later if someone sees motivation.


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-15 Thread Erich Keane via cfe-commits

erichkeane wrote:

> > Would it be reasonable to add a 
> > `-Wno-deprecated-relaxed-template-template-args` flag (or something like 
> > that) for this specific deprecation?
> 
> I had similar idea, but what about instead implementing something generic to 
> ignore deprecation of any driver flag?

I think I prefer pretty fine-grained ones TBH, it makes our deprecation 
warnings more valuable.  In a perfect world, it would change every release of 
the compiler so that folks would be frequently reminded of it, but it isn't a 
perfect world :) 

> Does it sound good for everyone that we revert the deprecation of the 
> negative spelling of the flag for a while, until we come up with a patch for 
> a new flag which helps ignore these deprecations?

Is doing so much of a task? I would expect it to be like other diagnostics and 
take roughly the same amount of time as reverting the 
negative-spelling-deprecation.  I don't see the request to remove the 
deprecation (it IS just a warning afterall!) to be particularly motivating 
until release time (it IS just a warning afterall!), so I'd think it would a 
better spending of time to implement what Richard suggested.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

Regarding @joanahalili 's post

Does it sound good for everyone that we revert the deprecation of the positive 
spelling of the flag for a while, until we come up with a patch for a new flag 
which helps ignore these deprecations?



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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> Would it be reasonable to add a 
> `-Wno-deprecated-relaxed-template-template-args` flag (or something like 
> that) for this specific deprecation?

I had similar idea, but what about instead implementing something generic to 
ignore deprecation of any driver flag?

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-15 Thread Richard Smith via cfe-commits

zygoloid wrote:

> The immediate deprecation causes a few issues

On the one hand: waiting to deprecate something that we know we're going to 
deprecate usually doesn't help anyone. We delay getting the message out to our 
users, we sometimes forget to do it for the next release, and at best it means 
it takes an extra release cycle to deliver whatever kind of benefit we were 
aiming for. So I think that we should generally start issuing deprecation 
warnings as soon as the new thing is ready and we know that we plan to remove 
the old thing.

On the other hand: a warning without a dedicated warning flag is, for some of 
our users, the same thing as an error. And, for some of our users, a 
deprecation error is the same thing as removing the feature immediately. So 
that doesn't work in this case :)

Would it be reasonable to add a 
`-Wno-deprecated-relaxed-template-template-args` flag (or something like that) 
for this specific deprecation?

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-15 Thread via cfe-commits

joanahalili wrote:

Hello @ mizvekov

We are seeing a widespread breakage due to this commit and could use having the 
flag `-fno-relaxed-template-template-args` un-deprecated for a while.  This 
would help do a cleanup on our end.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-14 Thread Sam McCall via cfe-commits

sam-mccall wrote:

The immediate deprecation causes a few issues:

- mechanical: we build with `-Wall -Werror -Wno-deprecated-declarations 
-Wno-deprecated-other-stuff` in part to catch driver misuse and fix it early. 
However this warning is not actionable, so now we need `-Wno-deprecated` which 
has undesirable effects and is also just changing unreasonably many things at 
once.
- mixed messages: there was a lot of confusion that this commit made the flag 
both required and deprecated. This isn't usual practice.
- scary with unclear expectations: deprecation warning means we need to migrate 
now, but we can't (not our code, authors probably won't take this seriously 
until clang-19). So our toolchain could break any day now?
- compatibility: there's no non-deprecated argv that results in the same 
behavior before and after this commit. What is a build system supposed to do if 
it can't version-lock clang? (Google actually *does* version-lock clang, this 
is only tangentially relevant to us because of clang-tidy, clangd etc. More 
relevant to others)
 
I think a good alternative would be: fix behavior + change default now in 
clang-19 cycle, deprecate `-fno-relaxed` flag in clang-20 cycle, remove 
`-fno-relaxed` and deprecate `-frelaxed` in clang-21 cycle. I believe this is 
more in line with what we've done in the past. Removing `-fno-relaxed` support 
is the real win here, and as this breaks deployed code doing that before 
clang-21 is probably unrealistic anyway.

---

(I forgot to mention earlier - a consequence of the crashes is that clang-tidy 
pipelines, clangd etc started crashing, which was disruptive to a lot of 
people. For clang per se it's not terribly severe to crash after emitting 
diagnostics, but other tools need to be able to consume real-world code and 
keep running, whether it's valid or not. In that sense this was more severe 
than a usual crash-on-invalid)

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-13 Thread Sam McCall via cfe-commits

sam-mccall wrote:

All makes sense to me. 

I'd point out that the only revert I was asking for was asking for was of the 
deprecation of the flag to restore the old behavior, and *optionally* the 
default flip. 

The combination of {prominent oss library, accepted by clang-18 and gcc, now 
ICE, flag to fix is deprecated} doesn't seem tenable even if the code is 
invalid, so I don't think that resolving it hinged on reproducer quality.

Others requested a broader revert. I suspect this is fix-forward being bad for 
tree stability, and revert + partial-reapply being better in this respect. 
Which... well, tradeoffs.

In any case I think we lost some nuance here, I'll try to be more helpful in 
future!

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

The fix was committed, and we just reverted the revert, so default is back to 
`-frelaxed-template-template-args`.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> @mizvekov Thank you! With that patch, clang not only doesn't crash on stdexec 
> with `-frelaxed-template-template-args`, but in fact accepts the code.

Thanks! The crash is still there and is pre-existing, but it's a 
'crash-on-invalid' issue, which is lower priority.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-13 Thread Erich Keane via cfe-commits

erichkeane wrote:

> I'm sorry that I wasn't able to more usefully reduce the failure cases. When 
> such regressions show up, we usually don't have any meaningful context on the 
> code. For our own code, we have guidelines to try to limit complexity which 
> makes reduction more tractable, but third-party libraries are harder.
> 
> For publicly-available code, it's not clear to me how much of the burden 
> should fall on people that identify the problem. I want to do as much of this 
> work as I can, it's difficult to balance the urgency of providing some 
> reproducer (it gets hard to push for a fix if we wait a week for a good 
> reproducers), the quality of reduction, and other work/deadlines. (As 
> mentioned, the timing was difficult this time as this landed just before a 
> holiday).

I agree here for the most part.  BUT our problem was that until we saw the 
reproducer from the bug report, it wasn't clear that this was a regression that 
required a revert.  I think that if someone is going to request a revert that 
they have to show that we have a regression, and not 'the patch doing what it 
was supposed to'.  I'll note that ~4 of us spent HOURS trying to figure this 
out as well, so we ALSO need to balance the work/deadlines of those trying to 
determine whether it is worth a revert.

For example, on this patch and a few recent ones, we've had a handful of folks 
bringing 'regressions' to us that were just the intent of the patch.  It was 
not clear in this case that the regression shown here wasn't just another case 
of "we diagnose something we didn't before, but that is because we wanted to".  
Because it was a "crash-after-invalid", it wasn't particularly serious, 
particularly as it was a crash that happened with this flag before.

This is VERY different from "diagnoses improperly" (deserves a revert) and 
"crashes on valid" (also deserves a revert).  

In this case we actually DIDN'T get to either of the two above, but the minimal 
reproducer let a few of us spend a few hours figuring out that this is a C++ 
Core issue, depending on your interpretation of a core issue this is either 
valid or invalid code.  

Within a few hours of determining this was the case, we reverted to leave the 
'previous' behavior until the standard is clarified.  In the end, your 
reproducer may very well still 'fail' (if WG21 decides that way), and wouldn't 
be a regression when we make that change.

TLDR; the request for a reduction was because it was associated with requests 
for a revert in a case it wasn't clear met the barrier for revert.  

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-13 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> For publicly-available code, it's not clear to me how much of the burden 
> should fall on people that identify the problem. I want to do as much of this 
> work as I can, it's difficult to balance the urgency of providing some 
> reproducer (it gets hard to push for a fix if we wait a week for a good 
> reproducers), the quality of reduction, and other work/deadlines. (As 
> mentioned, the timing was difficult this time as this landed just before a 
> holiday).

(Not talking about the revert policy, but just about reduced reproducers.)

I don't think we have (or want) hard-and-fast rules for how much of the burden 
falls on folks who find the issue -- we want to encourage people to report 
issues even if they don't have the time or ability to reduce the problem, but 
we also don't want to overload the community with issues that need significant 
reduction work. My suggestion is: initially, provide *anything* that 
demonstrates the issue so that folks are aware a problem exists, but with an 
understanding that the harder it is to reproduce (or the larger the reproducer 
is), the less likely it is the issue will be addressed in a timely manner. So 
if something is of critical importance to you, then the bar is higher for you 
to provide as much help as possible in getting to the point we can fix the 
issue on a timeline that works for you. But it's definitely a balancing act and 
the community should be willing to pitch in to help with reduction and mitigate 
fallout as much as possible (as what seems to have happened in this case).

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-13 Thread Sam McCall via cfe-commits

sam-mccall wrote:

I'm sorry that I wasn't able to more usefully reduce the failure cases. When 
such regressions show up, we usually don't have any meaningful context on the 
code. For our own code, we have guidelines to try to limit complexity which 
makes reduction more tractable, but third-party libraries are harder.

For publicly-available code, it's not clear to me how much of the burden should 
fall on people that identify the problem.
I want to do as much of this work as I can, it's difficult to balance the 
urgency of providing some reproducer (it gets hard to push for a fix if we wait 
a week for a good reproducers), the quality of reduction, and other 
work/deadlines. (As mentioned, the timing was difficult this time as this 
landed just before a holiday).

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-13 Thread Sam McCall via cfe-commits

sam-mccall wrote:

@mizvekov Thank you! With that patch, clang not only doesn't crash on stdexec 
with `-frelaxed-template-template-args`, but in fact accepts the code.


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

@sam-mccall @bgra8 @ericniebler I believe this MR should fix your issues: 
https://github.com/llvm/llvm-project/pull/91833

Can you double check?

You might consider applying https://github.com/llvm/llvm-project/pull/91837,
since that is stacked on that and will revert the default back to enabling 
relaxed matching by default. Otherwise, you have to compile with 
`-frelaxed-template-template-args`.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread via cfe-commits

cor3ntin wrote:

We operated a partial revert here 
https://github.com/llvm/llvm-project/pull/91811
That should fix the issues in trunk while we investigate the regression more 
thoroughly 

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

By the way, creduce/cvise won't help much here unless the interestingness test 
accounts for 'works on GCC'.

Otherwise, It'd be trivial to conjure some snippet of code that works before 
P0522, but breaks afterward as intended.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Erich Keane via cfe-commits

erichkeane wrote:

>I am repeating myself here, but the crash happens after a bunch of errors: 
>it's not significant, we have evidence this sort of crash is associated with 
>error recovery.

I missed this, thanks for clarifying (godbolt link scrolled and all I saw was 
warnings!).  A crash-on-invalid is very much not grounds for a revert.

Does someone have reproducer where this ISN'T the case? 

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

I am repeating myself here, but the crash happens after a bunch of errors: it's 
not significant, we have evidence this sort of crash is associated with error 
recovery.

This patch implements a standard mandated breaking change, and this 'stdexec' 
is user code.
Without evidence that stdexec does something that should be accepted within 
reason, I don't think that's grounds for revert.

The simplest thing to look for here: Does this project already workaround the 
changes as they were implemented in GCC?
If so, this could be just a matter of changing a preprocessor test.

Here is what I think would be a good evidence that something is wrong with this 
patch: A snippet of code, extracted from 'stdexec', that works on GCC, works on 
official clang, but doesn't after this patch.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

To be clear, we're asking for a reproducer with normal names first and 
foremost. I'm thankful for @bgra8 help running `creduce` over what they had, 
but we wouldn't had this particular conversation if they also provided full 
original reproducer that we could run `creduce` on with the settings we see fit.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Erich Keane via cfe-commits

erichkeane wrote:

> Can we start with either reverting the whole commit or at least removing the 
> flag deprecation and returning the old default 
> (`-fno-relaxed-template-template-args`)? I think, @bgra8 and @sam-mccall can 
> try to provide a reduced test case without renaming next week (it's sort of a 
> [long 
> weekend](https://www.thelocal.de/20220307/german-word-of-the-day-der-bruckentag)
>  in Germany with some folks not working), but not having a _convenient_ 
> reproducer is not a good reason to keep the ToT Clang in a broken state.

I'd like to give @mizvekov a chance to fix this and it isn't 'work hours' in 
his time zone yet.  I'd like to give him time to estimate a fix before we do 
the code churn of a revert.  I'm also not convinced that a break in the stdexec 
project raises to the "ToT is broken" definition that Chromium/a few others 
cause.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> but not having a convenient reproducer is not a good reason to keep the ToT 
> Clang in a broken state

As someone who worked on a different reduction of Sam's reproducer yesterday, 
and spent whopping 8 hours of work time to get 
https://github.com/llvm/llvm-project/issues/91566#issuecomment-2102739179, I 
disagree with your framing. I don't find it acceptable to claim that you 
provided an actionable reproducer, when it's 400 lines of dense code that can't 
be reduced automatically, and have half of the names stripped.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread via cfe-commits

alexfh wrote:

Can we start with either reverting the whole commit or at least removing the 
flag deprecation and returning the old default 
(`-fno-relaxed-template-template-args`)? I think, @bgra8 and @sam-mccall can 
try to provide a reduced test case without renaming next week (it's sort of a 
[long 
weekend](https://www.thelocal.de/20220307/german-word-of-the-day-der-bruckentag)
 in Germany with some folks not working), but not having a *convenient* 
reproducer is not a good reason to keep the ToT Clang in a broken state.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Erich Keane via cfe-commits

erichkeane wrote:

> If you're using `creduce`, this set of arguments disables all renaming 
> passes: `--remove-pass pass_clang rename-fun --remove-pass pass_clang 
> rename-param --remove-pass pass_clang rename-var --remove-pass pass_clang 
> rename-class --remove-pass pass_clang rename-cxx-method --remove-pass 
> pass_clex rename-toks`

This 100% needs to happen ASAP.  @sam-mccall or @bgra8 : Is this something you 
can provide?

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Erich Keane via cfe-commits

erichkeane wrote:

I agree with @sam-mccall : I think "crashes on some widely-used real-world 
code" means we should revert `B` until we can make that case work.  Yes, it is 
a pre-existing issue, however it means that we aren't "ready" to change the 
flag default.

However, we DEFINITELY need a useful reproducer, and I'm OK holding off on a 
revert if we can get a fix in 'the next few days' timeframe.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread via cfe-commits

cor3ntin wrote:

Having a clean reduction to decide whether the code is supposed to compile or 
not is indeed a good first step.

Clang 18 was not conforming and people might have relied on it.

There is a  -fno-relaxed-template-template-args that can be used as a 
workaround for now

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

It'd also be nice if someone can share a reproducer that crashes on trunk but 
not on 18.1.0 without names reduced to 1-2 letters. Definitely would speed up 
the work towards the fix.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread via cfe-commits

cor3ntin wrote:

I don't think there is motivation to revert anything at this point (we should 
avoid churn), however I do agree this is a regression that we should fix 
quickly @mizvekov.

(Even if it's not a new bug, it is a newly exposed bug, the pain for users is 
the same)

If the fix takes a long time to manifest then we should consider more 
aggressive mitigations.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-10 Thread Sam McCall via cfe-commits

sam-mccall wrote:

This commit did three things: A) changed the implementation, B) changed the 
flag default, and C) deprecated the flag.

Since clang now crashes on widely-used, real-world code, can we at least revert 
C, and ideally B until the crashes are fixed?

(It would also have been helpful to separate A and B, automated bisection can't 
tell the difference between the two, and generally we'd deal with crashes in 
default config by reverting the whole commit)

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> @mizvekov I have a [reduced test 
> case](https://github.com/llvm/llvm-project/files/15261978/repro.zip) for the 
> crash @sam-mccall reported.
> 
> Clang does not crash before but crashes at this revision.

Thanks. I confirm it crashes, but it crashes on clang 18.1.4 as well, if one 
just passes `-frelaxed-template-template-args`.
So most likely this is the same problem, we exposed it by default, but wasn't 
caused by changes in this patch per se.

The program became invalid with the change. The crash is most likely secondary, 
caused by bad error recovery.

The former is the most interesting thing here, we can try to see if it's one of 
the cases where we can solve with additional wording, but the reduction process 
might have mangled this.

Is it possible the original code already had workaround for 
template-template-args with GCC, but the workaround was otherwise GCC specific, 
not using the feature testing macro?


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-09 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

If you're using `creduce`, this set of arguments disables all renaming passes: 
`--remove-pass pass_clang rename-fun --remove-pass pass_clang rename-param 
--remove-pass pass_clang rename-var --remove-pass pass_clang rename-class 
--remove-pass pass_clang rename-cxx-method --remove-pass pass_clex rename-toks`

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-09 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

@bgra8 Your reduction has names replaced by the tool. This is quite hard to 
work with. Can you reduce again, but with renaming passes disabled?
I uploaded your reproducer to CE: https://godbolt.org/z/9PK1oPoPW

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-09 Thread via cfe-commits

bgra8 wrote:

> > Here's a preprocessed file: 
> > [repro.zip](https://github.com/llvm/llvm-project/files/15250584/repro.zip)
> > I tried to reduce, and got rid of most of the test code and some of the 
> > stdexec code, but there's still a lot left. I hit the end of my timebox on 
> > that. Maybe creduce can do better.
> 
> That crashes with released clang 18.1.4 as well, same stack trace it seems.
> 
> ```
> Homebrew clang version 18.1.4
> Target: arm64-apple-darwin23.4.0
> ```
> 
> Is it possible you may have reduced into a different bug? It's always helpful 
> in that case to keep testing with known good compiler while reducing.

@mizvekov I have a [reduced test 
case](https://github.com/llvm/llvm-project/files/15261978/repro.zip) for the 
crash @sam-mccall reported.

Clang does not crash before but crashes at this revision.

Compilation command:
```
$ clang -std=gnu++20 -fsyntax-only -c repro.cc
```



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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-09 Thread via cfe-commits

eaeltsin wrote:

> > Thanks, guarding the second specialization with the feature test macro 
> > works.
> > I will try to reduce the test case tomorrow, if you still need this.
> 
> Thanks. If it's not too much work for you, that would be great. Otherwise, I 
> think a pretty good guess can be made from reading the source code, but I may 
> not be able to try until next week. If that happens and I still don't manage 
> to reproduce it, I will ask again.

Here is my reduction - https://godbolt.org/z/Mrcafn8h6


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> Here's a preprocessed file: 
> [repro.zip](https://github.com/llvm/llvm-project/files/15250584/repro.zip)
> 
> I tried to reduce, and got rid of most of the test code and some of the 
> stdexec code, but there's still a lot left. I hit the end of my timebox on 
> that. Maybe creduce can do better.

That crashes with released clang 18.1.4 as well, same stack trace it seems.
```
Homebrew clang version 18.1.4
Target: arm64-apple-darwin23.4.0
```

Is it possible you may have reduced into a different bug?
It's always helpful in that case to keep testing with known good compiler while 
reducing.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-08 Thread Sam McCall via cfe-commits

sam-mccall wrote:

Here's a preprocessed file: 
[repro.zip](https://github.com/llvm/llvm-project/files/15250584/repro.zip)

I tried to reduce, and got rid of most of the test code and some of the stdexec 
code, but there's still a lot left.
I hit the end of my timebox on that. Maybe creduce can do better.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-08 Thread via cfe-commits

cor3ntin wrote:

@sam-mccall Thanks! do you have a reduction? or at least a preprocessed source 
file we could reduce?

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-08 Thread Sam McCall via cfe-commits

sam-mccall wrote:

This patch introduced a crash on code that clang previously accepted (I'm not 
sure whether the code is correct).

The code is 
https://github.com/nvidia/stdexec/tree/467f4a68ee04f3bb4c35e7a5dd13a3419da160cb,
 building `test/stdexec/algos/adaptors/test_stopped_as_optional.cpp` crashes.


Errors prior to crash

```
third_party/stdexec/test/stdexec/algos/adaptors/test_stopped_as_optional.cpp:38:19:
 error: static assertion failed
   38 | static_assert(ex::sender_in);
  |   ^~~
third_party/stdexec/test/stdexec/algos/adaptors/test_stopped_as_optional.cpp:38:19:
 note: because 'ex::sender_in' evaluated to false
./third_party/stdexec/include/stdexec/execution.hpp:824:9: note: because 
'decltype(__impl, 
stdexec::__env::empty_env>()())' (aka 
'stdexec::_ERROR_, 
stdexec::_WITH_META_FUNCTION_T_, 
stdexec::_WITH_TYPES_, 
stdexec::__env::empty_env, 
stdexec::__q,
 stdexec::__q>>') does not satisfy 
'__valid_completion_signatures':
  824 | get_completion_signatures((_Sender&&) __sndr, (_Env&&) __env)
  | ^
./third_party/stdexec/include/stdexec/execution.hpp:273:5: note: because 
'stdexec::_ERROR_, 
stdexec::_WITH_META_FUNCTION_T_, 
stdexec::_WITH_TYPES_, 
stdexec::__env::empty_env, 
stdexec::__q,
 stdexec::__q>>' does not satisfy '__ok'
  273 | __ok<_Completions> && __is_instance_of<_Completions, 
completion_signatures>;
  | ^
./third_party/stdexec/include/stdexec/__detail/__meta.hpp:243:18: note: because 
'__same_as<__ok_t<_ERROR_<_BAD_SUBSTITUTION_<__mstring<76UL>{"The specified 
meta-function could not be evaluated with the types provided."}>, 
_WITH_META_FUNCTION_T_, 
_WITH_TYPES_<__sexpr<(lambda at 
./third_party/stdexec/include/stdexec/__detail/__basic_sender.hpp:594:18)>, 
empty_env, 
__q, 
__q > > >, __msuccess>' evaluated to 
false
  243 |   concept __ok = __same_as<__ok_t<_Arg>, __msuccess>;
  |  ^
./third_party/stdexec/include/stdexec/__detail/__concepts.hpp:51:23: note: 
because '__is_same(stdexec::_ERROR_, 
stdexec::_WITH_META_FUNCTION_T_, 
stdexec::_WITH_TYPES_, 
stdexec::__env::empty_env, 
stdexec::__q,
 stdexec::__q > >, int)' evaluated to 
false
   51 |   concept __same_as = __is_same(_Ap, _Bp);
  |   ^
third_party/stdexec/test/stdexec/algos/adaptors/test_stopped_as_optional.cpp:45:15:
 error: no matching function for call to object of type 'const 
__connect::connect_t'
   45 | auto op = ex::connect(std::move(snd), 
expect_value_receiver{std::optional{}});
  |   ^~~
./third_party/stdexec/include/stdexec/execution.hpp:1471:12: note: candidate 
template ignored: constraints not satisfied [with _Sender = 
__libcpp_remove_reference_t<__sexpr<(lambda at 
./third_party/stdexec/include/stdexec/__detail/__basic_sender.hpp:594:18)> &>, 
_Receiver = expect_value_receiver>]
 1471 |   auto operator()(_Sender&& __sndr, _Receiver&& __rcvr) const
  |^
./third_party/stdexec/include/stdexec/execution.hpp:1468:18: note: because 
'__connectable_with_tag_invoke, 
(anonymous namespace)::expect_value_receiver > >' evaluated to false
 1468 | requires __connectable_with_tag_invoke<_Sender, _Receiver>
  |  ^
./third_party/stdexec/include/stdexec/execution.hpp:1415:7: note: because 
'__connectable_with_tag_invoke_<__tfx_sender<__sexpr<(lambda at 
./third_party/stdexec/include/stdexec/__detail/__basic_sender.hpp:594:18)>, 
expect_value_receiver > >, (anonymous 
namespace)::expect_value_receiver 
> >' evaluated to false
 1415 |   __connectable_with_tag_invoke_<__tfx_sender<_Sender, _Receiver>, 
_Receiver>;
  |   ^
./third_party/stdexec/include/stdexec/execution.hpp:1409:7: note: because 
'sender_in, 
env_of_t > > >' evaluated to 
false
 1409 |   sender_in<_Sender, env_of_t<_Receiver>> && //
  |   ^
./third_party/stdexec/include/stdexec/execution.hpp:824:9: note: because 
'decltype(__impl, 
stdexec::__env::empty_env>()())' (aka 
'_ERROR_{"The 
given type cannot be used as a[...]"}>, 
stdexec::__error__::_WITH_SENDER_>>>, 
stdexec::__error__::_WITH_ENVIRONMENT_>') does not 
satisfy '__valid_completion_signatures':
  824 | get_completion_signatures((_Sender&&) __sndr, (_Env&&) __env)
  | ^
./third_party/stdexec/include/stdexec/execution.hpp:273:5: note: because 
'stdexec::_ERROR_, 
stdexec::__error__::_WITH_SENDER_>>>, 
stdexec::__error__::_WITH_ENVIRONMENT_>' does not 
satisfy '__ok'
  273 | __ok<_Completions> && __is_instance_of<_Completions, 
completion_signatures>;
  | ^
./third_party/stdexec/include/stdexec/__detail/__meta.hpp:243:18: note: because 
'__same_as<__ok_t<_ERROR_<_UNRECOGNIZED_SENDER_TYPE_<__mstring<134UL>{"The 
given type cannot be used as a sender with the given environment because the 
attempt to compute the completion signatures failed."}>, 
_WITH_SENDER_<__sexpr > > >, _WITH_ENVIRONMENT_ > >, 
__msuccess>' evaluated to false
 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> Thanks, guarding the second specialization with the feature test macro works.
> 
> 
> 
> I will try to reduce the test case tomorrow, if you still need this.
> 
> 

Thanks. If it's not too much work for you, that would be great. Otherwise, I 
think a pretty good guess can be made from reading the source code, but I may 
not be able to try until next week. If that happens and I still don't manage to 
reproduce it, I will ask again.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-07 Thread via cfe-commits

eaeltsin wrote:

Thanks, guarding the second specialization with the feature test macro works.

I will try to reduce the test case tomorrow, if you still need this.


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

Oh I see the code already includes workaround for GCC vs non-GCC. It's possible 
in this case you may replace the workaround with a check for the feature 
testing macro.

But if this is a new ambiguity not covered by any of the cases I am tracking, 
it could still be worthwhile to isolate it. It could be we can cover it with 
new provisional wording.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> Hi, is there a way to make a compile-time check for this feature?

Yes, this is exposed by a standard feature testing macro: 
https://en.cppreference.com/w/cpp/feature_test#cpp_template_template_args

> 
> Looking at
> 

Thanks for reporting this. A few questions:
* Does https://github.com/llvm/llvm-project/pull/90820 help here?
* Does it reproduce with GCC, using `-std=c++17` and above?
* Are you able to reduce this test case?


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-07 Thread via cfe-commits

eaeltsin wrote:

Hi, is there a way to make a compile-time check for this feature?

Looking at 
1. 
https://github.com/xtensor-stack/xtensor/blob/master/include/xtensor/xutils.hpp#L1029
2. 
https://github.com/xtensor-stack/xtensor/blob/master/include/xtensor/xstorage.hpp#L1415

After this commit, 2 becomes ambiguous. If 2 is deleted, the code doesn't 
compile before this commit.

Or am I wrong and this code has another problem?

Output after the commit:
```
n file included from xtensor/xexpression_holder.hpp:17:
In file included from xtensor/xarray.hpp:19:
In file included from xtensor/xbuffer_adaptor.hpp:21:
In file included from xtensor/xstorage.hpp:23:
In file included from xtensor/xtensor_simd.hpp:17:
ERROR: xtensor/xutils.hpp:899:31 ambiguous partial specializations of 
'rebind_container>'
  899 | using type = typename rebind_container::type;
  |   ^
xtensor/xutils.hpp:927:5 in instantiation of template class 
'xt::get_strides_type>' requested here
  927 | using get_strides_t = typename get_strides_type::type;
  | ^
xtensor/xarray.hpp:53:30 in instantiation of template type alias 
'get_strides_t' requested here
   53 | using strides_type = get_strides_t;
  |  ^
xtensor/xcontainer.hpp:36:43 in instantiation of template class 
'xt::xcontainer_inner_types, 
xt::layout_type::row_major, xt::svector>>' requested here
   36 | using inner_shape_type = typename 
xcontainer_inner_types::inner_shape_type;
  |   ^
xtensor/xarray.hpp:64:11 in instantiation of template class 
'xt::xcontainer_iterable_types, 
xt::layout_type::row_major, xt::svector>>' requested here
   64 | : xcontainer_iterable_types>
  |   ^
xtensor/xiterable.hpp:43:43 in instantiation of template class 
'xt::xiterable_inner_types, 
xt::layout_type::row_major, xt::svector>>' requested here
   43 | using inner_shape_type = typename 
iterable_types::inner_shape_type;
  |   ^
xtensor/xiterable.hpp:151:30 (skipping 1 context in backtrace; use 
-ftemplate-backtrace-limit=0 to see all)
  151 | class xiterable : public xconst_iterable
  |  ^
xtensor/xiterable.hpp:311:42 in instantiation of template class 
'xt::xiterable, 
xt::layout_type::row_major, xt::svector>>' requested here
  311 | class xcontiguous_iterable : private xiterable
  |  ^
xtensor/xcontainer.hpp:71:31 in instantiation of template class 
'xt::xcontiguous_iterable, 
xt::layout_type::row_major, xt::svector>>' requested here
   71 | class xcontainer : public xcontiguous_iterable,
  |   ^
xtensor/xcontainer.hpp:260:39 in instantiation of template class 
'xt::xcontainer, 
xt::layout_type::row_major, xt::svector>>' requested here
  260 | class xstrided_container : public xcontainer
  |   ^
xtensor/xarray.hpp:82:37 in instantiation of template class 
'xt::xstrided_container, 
xt::layout_type::row_major, xt::svector>>' requested here
   82 | class xarray_container : public 
xstrided_container>,
  | ^
xtensor/xexpression_holder.hpp:192:32 in instantiation of template class 
'xt::xarray_container, xt::layout_type::row_major, 
xt::svector>' requested here
  192 | xt::xarray empty_arr;
  |^
xtensor/xutils.hpp:886:12 partial specialization matches [with X = long, C = 
xt::svector, T = unsigned long, N = 4]
  886 | struct rebind_container>
  |^
xtensor/xstorage.hpp:1415:12 partial specialization matches [with X = long, T = 
unsigned long, N = 4, A = std::allocator, B = true]
 1415 | struct rebind_container>
  |^
```

Output before the commit if the second specialization is deleted:
```
In file included from xtensor/test/test_extended_broadcast_view.cpp:15:
In file included from xtensor/xarray.hpp:19:
In file included from xtensor/xbuffer_adaptor.hpp:21:
In file included from xtensor/xstorage.hpp:23:
In file included from xtensor/xtensor_simd.hpp:17:
xtensor/xutils.hpp:899:31: error: implicit instantiation of undefined template 
'xt::rebind_container>'
  899 | using type = typename rebind_container::type;
  |   ^
xtensor/xutils.hpp:927:5: note: in instantiation of template class 
'xt::get_strides_type>' requested here
  927 | using get_strides_t = typename get_strides_type::type;
  | ^
xtensor/xarray.hpp:53:30: note: in instantiation of template type alias 
'get_strides_t' requested here
   53 | using strides_type = get_strides_t;
  |  ^
xtensor/xcontainer.hpp:36:43: note: in instantiation of template class 
'xt::xcontainer_inner_types, 
xt::layout_type::row_major, xt::svector>>' requested here
   36 |

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-02 Thread via cfe-commits


@@ -1133,8 +1133,8 @@ C++17 implementation status
 
 
   Matching template template parameters to compatible arguments
-  https://wg21.link/p0522r0;>P0522R0
-  Partial (10)
+  https://wg21.link/p0522r0;>P0522R0 (DR)
+  Clang 19 (10)

cor3ntin wrote:

This should have been "unreleased", not "full".
Sorry I missed that during review

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-01 Thread Matheus Izvekov via cfe-commits

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-30 Thread Erich Keane via cfe-commits

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


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-30 Thread via cfe-commits

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

I'm reasonably convinced the remaining issues are somewhat orthogonal and can 
be addressed separately.
I think we should move forward with this (and see if it sticks) but please give 
@erichkeane @zygoloid @shafik a few days in case they have additional feedback

Thanks a  lot for working on this

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

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

>From 1756044e71d756f7102f962d0298627ede27871c Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Fixes https://github.com/llvm/llvm-project/issues/36505
---
 clang/docs/ReleaseNotes.rst   |  19 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 +-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +-
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/cwg2398.cpp   | 139 ++
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 +--
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 +--
 19 files changed, 317 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp
 create mode 100644 clang/test/SemaTemplate/cwg2398.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1390d6536b28c..bd7f96246fd407 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,17 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions
+  (#GH36505).
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++17 Feature Support
@@ -164,6 +180,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported : 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

mizvekov wrote:

> @mizvekov We have a bunch of related issues, could you look at them 
> https://github.com/llvm/llvm-project/issues?q=is%3Aissue+is%3Aopen+%22-frelaxed-template-template-args%22
>  ?

Removed a bunch of duplicates. 4 issues remaining:

1) #36505 Is the issue we are fixing in this patch.
2) #65843 Is unrelated.
3) #63281 Is not a bug
4) #62529 The bug is accepting it, although perhaps we could seek clarification 
if we should exclude template template parameters from 
[CWG1430](https://cplusplus.github.io/CWG/issues/1430.html)


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

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

>From 4ee58efa0f154b531dcc674b6f4fe084182aa803 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Fixes https://github.com/llvm/llvm-project/issues/36505
---
 clang/docs/ReleaseNotes.rst   |  19 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 ++-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +++-
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/cwg2398.cpp   | 115 ++
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 ++-
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 ++-
 19 files changed, 293 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp
 create mode 100644 clang/test/SemaTemplate/cwg2398.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1390d6536b28c..bd7f96246fd407 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,17 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions
+  (#GH36505).
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++17 Feature Support
@@ -164,6 +180,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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


@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())

mizvekov wrote:

I see.

The code I posted was an attempt to construct such an example, where a pack 
doesn't need to be defaulted as-written for the proposed changes in the rules 
to have an effect. Except that example hits the wall at something else first, 
and I haven't fully investigated that. I haven't excluded the possibility that 
such an example could be constructed yet.

So when I proposed that provisional wording, I didn't exclude packs from the 
equation. I don't see a reason to, as without excluding them, the rules stay 
simpler and more general and should still work.

If we want to exclude them for implementation efficiency reasons, we could, as 
presumably it could still work as-if.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread via cfe-commits

cor3ntin wrote:

In particular #49185, #63281 and #62529 seem worth looking into

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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


@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())
+  return A;
+auto *R = TemplateTypeParmDecl::Create(
+S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+T->getDepth(), T->getIndex(), T->getIdentifier(),
+T->wasDeclaredWithTypename(), /*ParameterPack=*/false,
+T->hasTypeConstraint());
+R->setDefaultArgument(
+S.Context.getTrivialTypeSourceInfo(Default.getAsType()));
+if (R->hasTypeConstraint()) {
+  auto *C = R->getTypeConstraint();
+  R->setTypeConstraint(C->getConceptReference(),
+   C->getImmediatelyDeclaredConstraint());

mizvekov wrote:

It's the same tests I linked in the other thread: 
https://github.com/llvm/llvm-project/blob/15f02723d49be9a828fbf072966a225babd60457/clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp#L48

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread via cfe-commits

cor3ntin wrote:

@mizvekov We have a bunch of related issues, could you look at them
https://github.com/llvm/llvm-project/issues?q=is%3Aissue+is%3Aopen+%22-frelaxed-template-template-args%22
 ?

(and add tests + "Fixes #` to the commit message, as well as mentioning all 
the fixed issues in the release notes)

Thanks!

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread via cfe-commits

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread via cfe-commits


@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())

cor3ntin wrote:

My question was more whether we want to deduce packs at all.

I think the new behavior make sense, in that it is consistent with 
https://eel.is/c++draft/temp.deduct#partial-11 (
I suspect because both candidates are valid, this tie breaker gets triggered)

We might change that in the future, but now packs can't be defaulted, so I 
struggle to come up to think of a case where considering the pack would change 
the order of partial ordering (and not trailing packs can't be deduced)

here are more tests which I think are consistent https://godbolt.org/z/5Ke3TGfK6


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread via cfe-commits


@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())
+  return A;
+auto *R = TemplateTypeParmDecl::Create(
+S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+T->getDepth(), T->getIndex(), T->getIdentifier(),
+T->wasDeclaredWithTypename(), /*ParameterPack=*/false,
+T->hasTypeConstraint());
+R->setDefaultArgument(
+S.Context.getTrivialTypeSourceInfo(Default.getAsType()));
+if (R->hasTypeConstraint()) {
+  auto *C = R->getTypeConstraint();
+  R->setTypeConstraint(C->getConceptReference(),
+   C->getImmediatelyDeclaredConstraint());

cor3ntin wrote:

Can you point me to where these tests are?

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

Endilll wrote:

> Since the core issue is not resolved by WG21, I don't want to claim to be 
> resolving it here with this patch.

It'd be far from the first time we're testing an issue that is still open 
(https://github.com/llvm/llvm-project/blob/bd53c7cce418fe7f3e171859d4718df15d03dc2b/clang/test/CXX/drs/dr24xx.cpp#L62),
 but the fact that 2398 filing lacks any possible resolution that we can be 
testing against, it complicates the matter.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

mizvekov wrote:

Thanks.

Since the core issue is not resolved by WG21, I don't want to claim to be 
resolving it here with this patch.

And while we don't remove the non-conforming flag, we should still test it, and 
it's just much more economical and error proof that we reuse the same tests.


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

Endilll wrote:

Given that you're fixing CWG2398, you should be adding a test here to update 
https://clang.llvm.org/cxx_dr_status.html. This corner of our test suite lives 
by its own rules to some extent, so you should stay consistent with 700 tests 
we already have.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

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

>From 43f813d0a1a87b6cad9b859237489778f4f2945f Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Closes #55894
---
 clang/docs/ReleaseNotes.rst   |  18 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 ++-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +++-
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/cwg2398.cpp   | 115 ++
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 ++-
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 ++-
 19 files changed, 292 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp
 create mode 100644 clang/test/SemaTemplate/cwg2398.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5e5d3a2e6ea36..f525bdd73010cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,16 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions.
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++20 Feature Support
@@ -152,6 +167,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new

mizvekov wrote:

While it's true this is a DR, I just don't think for this particular case it's 
worth the cost of testing every single mode, as there are no particularly 
interesting interactions there.
Moot point as I will be moving it away anyway.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

mizvekov wrote:

Okay, I think I misunderstood then what `CXX/drs` was about, I should probably 
move it elsewhere.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new

Endilll wrote:

Why only C++23 is tested? DR tests should test all language modes.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits


@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

Endilll wrote:

We don't test non-conforming modes of operation in DR test suite.
This should be tested elsewhere, e.g. in SemaTemplate tests.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-04-26 Thread Vlad Serebrennikov via cfe-commits

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


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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

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

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