Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llvm-project Mini-Update #4348

Merged
merged 21 commits into from
Jan 30, 2024

Conversation

StephanTLavavej
Copy link
Member

I started by adding a script check_libcxx_paths.py to make LLVM updates easier. Then I figured I'd see how easy it was, and it only took me a day instead of a month.

llvm/llvm-project#79793 is still in flight, but the affected test is blocked by a suspected MSVC compiler bug with constexpr, so we don't need to wait.

@StephanTLavavej StephanTLavavej added the test Related to test code label Jan 29, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner January 29, 2024 12:39
@cpplearner
Copy link
Contributor

llvm/llvm-project#79793 is still in flight, but the affected test is blocked by a suspected MSVC compiler bug with constexpr, so we don't need to wait.

It's probably the same bug as DevCom-10439137 "Discarded id-expression causes unnecessary reading".

@StephanTLavavej
Copy link
Member Author

Yep, I agree! Great catch, I didn't know about that bug report. I've updated the entry.

I also noted in VSO-1869865 (the internal mirror of DevCom-10439137) that the ICE I just reduced, VSO-1948221 "x86chk ICE with constexpr type_info: Assertion failed: isIndirection()", is likely related - it involved constexpr and a (void) cast, which I found curious at the time.

@StephanTLavavej StephanTLavavej self-assigned this Jan 30, 2024
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Copy link
Member Author

I had to push an additional commit because new tests appeared that require Unix headers, and the MSVC-internal test harness doesn't understand this.

@cpplearner
Copy link
Contributor

Should UNSUPPORTED: !has-unix-headers be added to magic_comments.txt? I don't understand what this file does, but apparently it's used by the MSVC-internal test harness.

@StephanTLavavej
Copy link
Member Author

@cpplearner That's a good idea. magic_comments.txt is indeed used by the MSVC-internal test harness - it's an extremely primitive mechanism that I wrote a while ago (later adapted by Casey) where if a test mentions any of those "magic comments", it's skipped. This imitates the real test harness's implementation of "feature" detection.

We already have // REQUIRES: has-unix-headers there; arguably upstream should be changed to say that, avoiding the current double negation. (The comment matching we do is also whole-line, as I recall.)

Since the PR and its mirror have passed checks, I'll explore doing this the next time I update LLVM. Thanks!

@StephanTLavavej StephanTLavavej merged commit 202e382 into microsoft:main Jan 30, 2024
35 checks passed
@StephanTLavavej StephanTLavavej deleted the smol-libcxx-update branch January 30, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Related to test code
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants