- User Since
- Feb 2 2020, 12:55 PM (163 w, 2 d)
Whether the assertion itself in getCurLambda is still pertinent,
despite no test depending on it does require more analysis.
Mon, Mar 20
Fri, Mar 17
Do we need a release note for this?
Thu, Mar 16
LGTM. Triple checking with @hubert.reinterpretcast for conformance but I'm pretty sure that's correct.
Sat, Mar 11
Fri, Mar 10
In terms of the encoding question I was asking, that's information we'll have to figure out (CC @tahonermann and @cor3ntin for text encoding question). My guess (which needs verification) is that we convert the file name from the system encoding to UTF-8 internally, and this builtin will likely return UTF-8 as a result. If that's correct, I think that behavior is reasonable, but I've CCed some experts who can tell me all the things I forgot to consider.
Thu, Mar 9
Wed, Mar 8
Tue, Mar 7
Mon, Mar 6
I think this make sense.
Maybe we should add a comment quoting [expr.const] p15.1 in both places though.
Thu, Mar 2
Wed, Mar 1
This expands to
Tue, Feb 28
@shafik Can you let me know if you are happy with the changes i made to address your feedback? Thanks!
Mon, Feb 27
Remove the #error / #warning tests
Address Aaron's comments
This has been sitting in the queue for a while, sorry about that.
I think this makes sense in its current iteration, with the warning always on. Have you tried to build a large project like chrome with it?
Fri, Feb 24
Given this has not made progress in a while, I think we should try to implement https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2280r4.html instead of this approach, as I'm not sure if that's strictly conforming, and a more holistic approach is likely to lead to better results.
What do people think?
Sorry it took me a while to reply to you. I think you convinced me this is fine as-is! Thanks
@aaron.ballman I think we should close this change as it's superseeded by https://reviews.llvm.org/D144218
@rupprecht Great to hear! Sorry again for the disruption
@shafik Thanks for the review!
Address Shafik's comments
Thu, Feb 23
I played with different diagnostics, we already specialize the
literal case, and I didn't find that not saying a static_assert
failed improved things.
Instead, printing, in addition of that diagnostic, an instantiation
stack provides additional useful information.
(The stack is only printed for the literal bool/integer case)
Release notes + formatting
Address Aaron's comments
Wed, Feb 22
@aaron.ballman is the plan to backport that to clang 16? I think it probably should.
We ought to try to improve the situation for clang 17
Tue, Feb 21
This looks good to me.
Comments are allowed anywhere between token, anywhere whitespace would be.
This makes sense to me as is. The check ClassDecl->hasTrivialDefaultConstructor() && !ClassDecl->hasNonTrivialDefaultConstructor() can only be different from status quo in C++20.
Changing isTrivial would be a lot more involved change that should not be in scope of this PR
Feb 20 2023
Thanks for working on this issue.
I don't feel confident commenting on this patch's approach (I think @erichkeane is your best bet), but the small typo does make a pretty big difference here!
Can you add a test to check __has_cpp_attribute(assume) ?
This is an impressive amount of work. I think it makes sense!
Thanks a lot for doing that work.
I only have a few nits after a first review of this.
Feb 18 2023
- Reword Release note
- Restore dependent instantiation tests using Richard's suggestion
- Add additional test to check that diagnostics for static_assert are emitted once per instantiation.
Feb 17 2023
- Simplify how we check we are in a dependent context
- Fix typos
Forgot to run clang-format
Feb 16 2023
Fix GH60518 by not trying to capture parameters declared in the current
I wonder if a better fix would be change ResultChar to be a 64 bits integer, although I think it might be over killed (ie, i'm not aware of a good use case for wanting to store values that large in a 32 bits wide char, so this looks fine.
Thanks for the fix :)
Feb 13 2023
Thanks Tom, I agree https://github.com/llvm/llvm-project/commit/756395f61b90e30c9087b5efa8b4809ab03aff6e
Feb 12 2023
Feb 9 2023
Thanks for doing this Aaron.
Did you look in libc++ if they used it anywhere? (I assume they don't)
Feb 4 2023
My question is, why do we need to mess up with scopes in that way outside of parsing (there are only a couple places where we do that at the moment, and they are dummy scopes which only exist to balance some push and pop, afaict they serve no other purpose).
As i said, I have no objection to this at all, it clearly fixes the issue and we should land it! But the fact we have to do this in the first place *may* be a sign that there is a deeper issue, one that we clearly should not try to address as part of this.
Feb 2 2023
Feb 1 2023
LGTM except the duplicated comment
Jan 31 2023
@aaron.ballman Thanks for the review! I hope we won't find further issue with the paper this time :)
Jan 30 2023
- rename ActOnLambdaIntroducer
- Fix a crash in transformedLocalDecl
Jan 26 2023
Jan 23 2023
Jan 16 2023
Yes please! However the warning looks correct to me in that case. A
constructs x which constructs A etc.
Jan 13 2023
Jan 9 2023
- Address Aaron's comments (except renaming ActOnLambdaIntroducer as we pounder on it)
- Delete an aditional outdated comment
Symplify printing a NamedDecl in moved code per Aaron's comment