Tests are marked as failing by the pre-merge checks? Otherwise LG after you get these to work as expected!
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Today
It seems that using is*Value and the introduction of getDecltypeForParenthesizedExpr could be two separate changes.
.
Consider PermuteOperands.
I'm about to land D100122 instead - https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29445 is waiting.
We can revisit this issue if fixing UndefValue is necessary.
Would you explain what this patch is for?
Fix up the other switch statement that I somehow missed.
Put everything back the way it was, and poke buildkite one last time. (This time it should succeed.)
Removed the dependency as it has been closed.
The hard tab and UTF-8 character é are reported here: https://buildkite.com/llvm-project/libcxx-ci/builds/2608#8ae5209d-9f5d-459c-ac7a-271e93ae70c7
Introduce three intentional errors for testing purposes: hard tab, Unicode, and cyclic dependency. Poke buildkite.
I thought the Beyonce rule was: if you like a world without unicode characters, you need to put a CI test on it.
Poke buildkite, having removed invisible Unicode characters from the headers.
Thank you all for your comments! But before proceeding, I want to ask a question.
It seems we have another approach for debug mode tests that was added in D59166, which I didn't notice before but which looks much better than we have now (except the facts that it is more complicated, has more requirements, and may suffer from the same problem because most tests don't check for the exact assertion just the fact that an assertion happened). So, the question is why the approach from D59166 didn't replace other (I mean the tests which this patch about) debug mode tests? Was the reason in the requirements? Should it finally become the only approach for debug mode tests or it is supposed to coexist with others?
Updated, addressed review feedback.
@mehdi_amini Thanks for the quick review. I added some tests to/from LLVM IR and addressed other comments.
Address review comment
Ready for review.
This ended up being a complete rewrite, and it's kinda ugly.
In D100703#2696809, @Quuxplusone wrote:Update: <type_traits> and maybe some other headers contain U+200B ZERO-WIDTH SPACE (attn: @cjdb). I'll just remove those characters. However, it does indicate that this will be a good check (at least once I improve the error message it gives ;)) because it'll incidentally help keep libc++ ASCII-friendly.
The C++ draft has some of these lints:
https://github.com/cplusplus/draft/blob/master/tools/check-source.sh#L25-L37
Should the commit message have NFC in it somewhere or does [doc] imply this?
Also please update the title to include a tag like for this one [Docs].
LGTM, But also adding @aqjune as a second reviewer.
Update: <type_traits> and maybe some other headers contain U+200B ZERO-WIDTH SPACE (attn: @cjdb). I'll just remove those characters. However, it does indicate that this will be a good check (at least once I improve the error message it gives ;)) because it'll incidentally help keep libc++ ASCII-friendly.
It should be ready to go?
For my future reference: What is the sequence you go through to ensure that no target output files change? Do you just build with the current system, then build with the revision and check that no output file dates changed?
Seems the Windows build fails, can you look why the XFAIL doesn't work.
Have caller create the SmallVector and pass by reference. This is much more common in the code base.
Thanks, @Mordante for the tips, I remember from next time :)
I did some debugging on why DeclRefExpr evaluated not to be a Function pointer. Here is what I got.
I made the following clang change to dump out the pointer to the emited value ('&foo'):
Also handle the outgoing case. Changes f32 return promotion from broken, to different broken
I agree with Mehdi and River. it seems like the passmgr should be a client of the more general shared thing.
In D100705#2696708, @xgupta wrote:Nice work @jnyfah!
@Mordante Review is already accepted by @curdeius in https://reviews.llvm.org/D100696. And actually, I see your message after committing the patch.
In D100691#2696632, @Paul-C-Anagnostopoulos wrote:I presume this passes all the TableGen tests.
How else did you test it? Knowing this may help me in the future.
- add checking CGDebugInfo pointer. In my original patch but got lost in the revision.
Rebased after landing the post-order iterator fixes in bbf01f96b5ccc1dcb4d1d47cb55292c27c698dbb.
Poke buildkite (investigating)
@jnyfah Would you please update this revision by submitting a new diff using update diff after making changes as suggested by @Meinersbur.
@jnyfah you may close this revision. changes are committed by https://reviews.llvm.org/rG21bef4e11e48d5d4bff7a23babbd420e86dd420a.
Ping. Could somebody review this, please?
Nice work @jnyfah!
htps://buildkite.com/llvm-project/libcxx-ci/builds/2595#c3483aa2-87b6-44fd-870e-4ce28b115ccc shows build errors. I assume these aren't the intended errors.
I would like to see how a build failure for this script is intended to look at the CI.
Thanks for your contribution. Next time, please provide a patch with more context.
The instructions can be found at https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.
LGTM, but please wait for an approval of the libc++ group.