User Details
- User Since
- Jul 12 2018, 2:31 PM (245 w, 3 d)
Tue, Mar 21
It looks like we might have another similar bug: https://github.com/llvm/llvm-project/issues/61589
Mon, Mar 20
As I noted in the bug report not doing D.setInvalidType(); does fix this bug and seems harmless since the error diagnostic should prevent us from getting to codegen but it is not clear to me if this has other negative impacts. Reading your replies it is not obvious you looked at this path or not.
Adding clang-language-wg for more visibility
Please edit the summary to indicate this fixes: https://github.com/llvm/llvm-project/issues/61441
Sat, Mar 18
Thank you for submitting this fix.
Fri, Mar 17
- Switched to LexUnexpandedNonComment in more places
- Extended test suite to cover the new cases uncovered in the comments
I would have loved to test the case from https://github.com/llvm/llvm-project/issues/61335 directly but I think in order to do it nicely I need __builtin_memset to be usable in a constant expression context. I will add this to my todo list. I am open to other alternatives for testing this.
Thu, Mar 16
- Added release note
- Update CXX status
- Updating diagnostic wording
- Adding Reference to changes from p2448r2 to the comments
- Modified p3 tests to have an extension pass as well
Thank you for the fix.
Can we also add the test from the bug report as a regression test, it looks like the existing test are basically covering the same thing but I have seen stranger bugs so it would be good to explicitly cover it.
Can you add a summary, I realize folks can goto the release notes edit and see the bug report and then go look it up but at least a cursory summary is a little more reviewer friendly.
Wed, Mar 15
Is there a way to test this?
Tue, Mar 14
LGTM
- Switched to using auto in two if statements
- Added Release notes
Apologies for late review.
Mon, Mar 13
LGTM, I wlll let @royjacobson give the final approval after reviewing codegen tests.
Sun, Mar 12
Fri, Mar 10
Thu, Mar 9
Tue, Mar 7
Mon, Mar 6
This LGTM, I agree w/ @cor3ntin about quoting [expr.cont] to document this choice.
Besides the nitpick, LGTM
Thank you for fixing this so quickly.
Fri, Mar 3
Thu, Mar 2
I think these two bug reports: https://github.com/llvm/llvm-project/issues/46132 and https://github.com/llvm/llvm-project/issues/61118 may be related to this change.
Wed, Mar 1
LGTM
LGTM
Tue, Feb 28
LGTM, the modules failures is a previous known issue.
LGTM
Mon, Feb 27
Also please add a release note before landing the fix, thank you.
Thank you for this patch, it looks good.
Feb 24 2023
Feb 23 2023
Feb 21 2023
Feb 20 2023
Thank you for taking the time to submit a patch.
Feb 17 2023
- Add release note
Feb 16 2023
I think VisitCastExpr is the right place to start looking for what to do.
LGTM
So we are only checking global constructors b/c it is valid in a constant expression context to initialize a record and not initialize all their fields as long as we don't use any of those fields.
Feb 15 2023
Feb 14 2023
If I look at the clang docs for Wconversion I see it includes -Wshorten-64-to-32 which I believe this is a case of. I think maybe the warning needs a better warning for this case?
Ping
Feb 13 2023
LGTM
Feb 6 2023
WG21 is meeting all this week, so a bunch of folks who should take a look at this may not get around to it right away.
Feb 5 2023
It looks like https://github.com/llvm/llvm-project/issues/60543 is hitting your added llvm_unreachable("missing serialization code for annotation token"); is this expected.
Feb 2 2023
Feb 1 2023
Jan 31 2023
- Add release note.
Jan 30 2023
The build failures look unrelated to this change.
- Fix up release notes
- Add release note
Adding clang-language-wg for more visibility.