Page MenuHomePhabricator

amccarth (Adrian McCarthy)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2015, 9:26 AM (356 w, 5 d)

Recent Activity

Sep 21 2021

amccarth accepted D110172: [lldb/win] Default to native PDB reader when building with LLVM_ENABLE_DIA_SDK=NO.

LGTM

Sep 21 2021, 8:54 AM · Restricted Project

Sep 16 2021

amccarth added a comment to D109834: [lldb/win] Improve check-lldb-shell with LLVM_ENABLE_DIA_SDK=NO.

I'm trying to understand why I've never seen the problem that this patch is trying to address.

Sep 16 2021, 2:33 PM · Restricted Project, Restricted Project
amccarth added a comment to D109832: [lldb/win] Fix TestIRMemoryMapWindows.test when running tests in git bash.

LGTM.

Sep 16 2021, 2:12 PM · Restricted Project

Sep 13 2021

amccarth accepted D109634: [LLD] Remove global state in lld/COFF.

Thanks for the fixes. LGTM.

Sep 13 2021, 10:44 AM · Restricted Project

Sep 10 2021

amccarth added a comment to D109634: [LLD] Remove global state in lld/COFF.

Minor suggestions, but otherwise LGTM. I assume you've coordinated with @aganea to ensure you aren't duplicating effort.

Sep 10 2021, 3:43 PM · Restricted Project

Aug 31 2021

amccarth added a comment to D109030: [LLD][COFF] Clean paths in PDB even when /pdbsourcepath isn't used.

I'm mildly surprised the make_absolute call doesn't remove unnecessary dotted paths. I guess that's related to the weird schism between sys::fs and sys::path.

Aug 31 2021, 4:10 PM · Restricted Project

Aug 27 2021

amccarth added a comment to D108850: [LLD] Remove global state in lldCommon.

This is a larger change than I anticipated. I was surprised by the need to have the pointer be thread local (and then ensure that the thread local pointers for new threads are set up, too). If I'm understanding correctly, the idea is that a single program could have n threads doing n links at the same time. Is that right? I'm not arguing against that. I'm just trying to make sure I understand why I was surprised.

Aug 27 2021, 4:26 PM · Restricted Project, Restricted Project, lld

Aug 24 2021

amccarth accepted D108513: [docs] Update Getting Started with Visual Studio guide.

LGTM.

Aug 24 2021, 4:05 PM · Restricted Project

Aug 23 2021

amccarth added a comment to D108513: [docs] Update Getting Started with Visual Studio guide.

I also thank you for working to improve this documentation. I mostly agree the other reviewer. I've only added a couple inline comments to raise questions. I wouldn't hold this back for either of those.

Aug 23 2021, 2:47 PM · Restricted Project

Aug 20 2021

amccarth added inline comments to D108444: [docs] Clarify how to run cmake and llvm-lit with Visual Studio addressing PR45978.
Aug 20 2021, 11:08 AM · Restricted Project

Aug 12 2021

amccarth added inline comments to D105561: [libc] Capture floating point encoding and arrange it sequentially in memory.
Aug 12 2021, 2:45 PM · Restricted Project

Aug 11 2021

amccarth added inline comments to D107654: [Flang] Fix error messages on Windows..
Aug 11 2021, 8:53 AM · Restricted Project

Aug 10 2021

amccarth added a comment to D107654: [Flang] Fix error messages on Windows..

Thanks for making the change more local.

Aug 10 2021, 1:48 PM · Restricted Project

Aug 6 2021

amccarth added a comment to D107557: [OptTable] Refine how `printHelp` treats empty help texts.

Hmm, that's a good question. Most of the options that are listed aren't even hooked up in llvm-cvtres (and the tool it impersonates, MS cvtres.exe, also just lists the options by name without any help text in its help printing).

I guess we could add a help text for the options that we have implemented at least, and then omit the other ones. Or do @thakis or @amccarth have a strong opinion on it?

Aug 6 2021, 9:10 AM · Restricted Project

Aug 4 2021

amccarth accepted D107263: [llvm-rc] Allow specifying language with a leading 0x prefix.

I was a little thrown by llvm-windres versus llvm-rc. I hadn't heard of llvm-windres before, and my web search came up with zero hits (literally!). But now I understand.

Aug 4 2021, 5:00 PM · Restricted Project

Jul 23 2021

amccarth accepted D106598: [llvm-rc] Allow dashes as part of resource name strings.

I see. I didn't realize llvm-rc used an actual separate preprocessor step. (RC has a crude one built-in that doesn't quite match any of the standards.)

Jul 23 2021, 10:03 AM · Restricted Project
amccarth added a comment to D106598: [llvm-rc] Allow dashes as part of resource name strings.

Cool. I'd never seen anyone use strings that don't look like a C or C++ identifier as a resource identifier, but it should work given that FindResource just uses the string as an opaque key.

Jul 23 2021, 9:28 AM · Restricted Project

Jul 8 2021

amccarth accepted D105621: [llvm-rc] Make commas in user data structs optional.

LGTM

Jul 8 2021, 4:12 PM · Restricted Project
amccarth added a comment to D105621: [llvm-rc] Make commas in user data structs optional.

Two small requests, then LGTM.

Jul 8 2021, 9:03 AM · Restricted Project

Jul 2 2021

amccarth added a comment to D104987: [libcxx] Use GetSystemTimePreciseAsFileTime() if available.

tl;dr: I understand the desire to avoid even tiny synchronization costs when trying to read a high-precision timer, but I'm uncomfortable with the data race. I suggest initializing the static function pointer under the locking provided by the compiler.

The simplest option would be to initialize it where it's defined. I would move the code that finds the address to a separate function, say GetGetSysTimeProcAddress, and then simplify this code to:

static GetSysTime_t getSysTime_p = GetGetSysTimeProcAddress();
getSysTime_p(&ft);

Oh, that's great, I had no idea that this actually ends up synchronized. That's very neat.

Jul 2 2021, 4:45 PM · Restricted Project
amccarth added a comment to D104987: [libcxx] Use GetSystemTimePreciseAsFileTime() if available.

tl;dr: I understand the desire to avoid even tiny synchronization costs when trying to read a high-precision timer, but I'm uncomfortable with the data race. I suggest initializing the static function pointer under the locking provided by the compiler.

Jul 2 2021, 9:57 AM · Restricted Project

Jun 25 2021

amccarth requested review of D104954: Implement AddSymbols method for SymbolFileNativePDB.
Jun 25 2021, 2:54 PM
amccarth accepted D104589: [llvm-rc] Don't rewrite the arch in the default triple unless necessary.

LGTM. Thanks.

Jun 25 2021, 10:00 AM · Restricted Project

Jun 22 2021

amccarth added a comment to D104589: [llvm-rc] Don't rewrite the arch in the default triple unless necessary.

The idea looks fine. Just a couple inline questions.

Jun 22 2021, 10:32 AM · Restricted Project

Jun 17 2021

amccarth added a comment to D88666: DirectoryWatcher: add an implementation for Windows.

One thing we've run into in the past is that running some of these tests on a drive other than the OS drive can cause weird failure. I am not sure if that may be the case here as well.

Jun 17 2021, 3:28 PM · Restricted Project

Jun 11 2021

amccarth added a comment to D88666: DirectoryWatcher: add an implementation for Windows.

This patch was reverted a while back because a couple DirectoryWatcher tests routinely timed out on build bots even though they work when run in isolation.

Jun 11 2021, 4:50 PM · Restricted Project
amccarth accepted D103089: [Debug-Info][CodeView] Fix GUID string generation for MSVC generated objects..

Thanks for the changes. This LGTM now.

Jun 11 2021, 3:01 PM · Restricted Project, debug-info

Jun 9 2021

amccarth added a comment to D102736: Fix tmp files being left on Windows builds..

Nice catch Reid.

Jun 9 2021, 3:32 PM · Restricted Project, Restricted Project

Jun 7 2021

amccarth accepted D103806: [SystemZ][z/OS] Pass OpenFlags when creating tmp files.

LGTM

Jun 7 2021, 1:52 PM · Restricted Project, Restricted Project

Jun 3 2021

amccarth added a comment to D87732: [Support] Provide sys::path::guess_style.
In D87732#2298861, @rnk wrote:
In D87732#2296076, @rnk wrote:

Separately, I would like LLVM to move in the direction of standardizing on forward slashes in internal representations and data structures.

If we move that direction, we'll have to re-think VFS again. I'll also lobby hard to make sure that we show the native representation in compiler output, like diagnostic messages.

Yes, we might have to adjust VFS usage, but most of the important users are in-tree, so this is doable, not something unfixable,

Jun 3 2021, 2:53 PM · Restricted Project

Jun 1 2021

amccarth added a comment to D103089: [Debug-Info][CodeView] Fix GUID string generation for MSVC generated objects..

I'm fine focusing on this part to get the bug fixed. We can consider looking at accepting upper and lower case letters in the future.

Jun 1 2021, 12:16 PM · Restricted Project, debug-info
amccarth accepted D103370: WindowsSupport.h: do not depend on private config header.

LGTM.

Jun 1 2021, 11:40 AM · Restricted Project

May 27 2021

amccarth accepted D102736: Fix tmp files being left on Windows builds..

Yeah, this is good. My remaining comments are all speculations about how to improve this further in the future, but they aren't directly applicable to the goal here.

May 27 2021, 8:15 AM · Restricted Project, Restricted Project

May 25 2021

amccarth added a comment to D103089: [Debug-Info][CodeView] Fix GUID string generation for MSVC generated objects..

This sounds like an appropriate place to apply Postel's Law: Be conservative in what you do, be liberal in what you accept from others. I'm be happy to always generate uppercase hex digits when formatting a GUID to registry format, but maybe we should also consider accepting upper or lowercase when reading a formatted GUID.

May 25 2021, 9:32 AM · Restricted Project, debug-info

May 24 2021

amccarth added a comment to D102684: [LLD] Allow disabling the early exit codepath as a build configuration.

While exiting from a non-main thread is tricky, things also do hang when exiting from the main thread, when bypassing constructors. The fatal issue there is that we have a number of threads still running, that can keep various locks. When exiting, all other threads are stopped where they are, and we run the destructors for DLLs.

May 24 2021, 12:04 PM · Restricted Project

May 21 2021

amccarth added a comment to D102684: [LLD] Allow disabling the early exit codepath as a build configuration.

I looked into this in some detail back in the day and am mainly responsible for how this thread pool currently functions. I primarily focused on static linking with the MSVC static runtime, as that's what we use downstream and were seeing issues with, but also included MinGW, GNU and libc++. To be honest, I can't remember if I tried LLVM_LINK_LLVM_DYLIB, there are just too many possible configurations! The various compilers/runtimes have changed since I last looked at this code, so it's not surprising that there may be issues depending on your configuration.

From past experience, I think it has always been "dangerous" on Windows to exit without waiting/terminating any running threads. If this can't be done, then a "hard" exit, e.g. via ExitProcess(), is usually the only reliable way to finish....

May 21 2021, 2:58 PM · Restricted Project
amccarth added a comment to D102736: Fix tmp files being left on Windows builds..

Cool! This is getting much simpler. My remaining comments are mostly musings and you can tell me to buzz off.

May 21 2021, 9:37 AM · Restricted Project, Restricted Project

May 19 2021

amccarth requested changes to D102736: Fix tmp files being left on Windows builds..

Sorry, forgot to Request Changes.

May 19 2021, 12:08 PM · Restricted Project, Restricted Project
amccarth added a comment to D102736: Fix tmp files being left on Windows builds..

At some point, the duplicate handle must be closed. I don't see that happening. I've added an inline comment where I think it should be done.

May 19 2021, 12:07 PM · Restricted Project, Restricted Project

May 18 2021

amccarth added a comment to D102736: Fix tmp files being left on Windows builds..

I'll have to dig into my archived email tomorrow.

May 18 2021, 5:15 PM · Restricted Project, Restricted Project

May 13 2021

amccarth added a comment to D102402: LineEditor: Add a bare-bones readline-based implementation.

When this gets further along, maybe we can get LLDB to use it instead of EditLine.

May 13 2021, 11:19 AM · Restricted Project

May 12 2021

amccarth added inline comments to D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths..
May 12 2021, 2:27 PM · Restricted Project

May 11 2021

amccarth accepted D102208: Remove Windows editline from LLDB.

LGTM. Dead code should go. If somebody wants to own a Windows port of EditLine, they can re-instate it and put some tests on it.

May 11 2021, 3:02 PM · Restricted Project, Restricted Project
amccarth updated subscribers of D102208: Remove Windows editline from LLDB.

It looks like this EditLine port was added before I joined this project. @zturner may have worked on it, but I don't know/remember.

May 11 2021, 9:17 AM · Restricted Project, Restricted Project
amccarth added a comment to D101479: [Driver] Support libc++ in MSVC.

Not sure if we want the desicion between static and shared libc++ be coupled with /MT and /MD, as one can quite plausibly want to use e.g. a static libc++ with /MD.

I don't understand this. When would someone want to use /MD and not get the DLL version of the run-time libraries?

Whether one wants to link against the CRT statically or dynamically, and libc++ statically or dynamically, are two entirely separate things. I would e.g. expect that Chrome is built with a statically linked libc++ but linked against the dynamic CRT.

May 11 2021, 8:35 AM · Restricted Project

May 10 2021

amccarth added a comment to D101479: [Driver] Support libc++ in MSVC.

Not sure if we want the desicion between static and shared libc++ be coupled with /MT and /MD, as one can quite plausibly want to use e.g. a static libc++ with /MD.

May 10 2021, 9:40 AM · Restricted Project

May 5 2021

amccarth accepted D101433: Added a faster method to clone llvm project [DOCS].

LGTM. Thanks!

May 5 2021, 8:02 AM · Restricted Project

Apr 29 2021

amccarth added inline comments to D101433: Added a faster method to clone llvm project [DOCS].
Apr 29 2021, 8:40 AM · Restricted Project

Apr 28 2021

amccarth added inline comments to D101433: Added a faster method to clone llvm project [DOCS].
Apr 28 2021, 9:37 AM · Restricted Project

Apr 27 2021

amccarth added a comment to D101378: [llvm, clang] Remove stdlib includes from .h files without `std::`.

A drive-by look.

Apr 27 2021, 9:56 AM · Restricted Project, Restricted Project

Apr 26 2021

amccarth accepted D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc.

There's a lot going on here, but I don't see anything wrong. Thanks for the completeness of the tests and the comments, as that helps a lot in understanding what's going on here.

Apr 26 2021, 10:49 AM · Restricted Project, Restricted Project

Apr 21 2021

amccarth added a comment to D100833: [llvm-cvtres] Reduce the set of dependencies of llvm-cvtres. NFC..

Wow. Excellent. Thank you!

Apr 21 2021, 8:58 AM · Restricted Project

Apr 20 2021

amccarth committed rG6e77a67171e6: Fix clang Visual Studio build instructions (authored by Loghorn).
Fix clang Visual Studio build instructions
Apr 20 2021, 11:18 AM
amccarth closed D68321: Fix clang Visual Studio build instructions.
Apr 20 2021, 11:18 AM · Restricted Project
amccarth added a comment to D68321: Fix clang Visual Studio build instructions.

I'll merge it for you.

Apr 20 2021, 10:57 AM · Restricted Project

Apr 19 2021

amccarth added a comment to D100755: [llvm-rc] [3/4] Run clang to preprocess input files.

Ah, I get it. Thanks for the clarification.

Apr 19 2021, 2:52 PM · Restricted Project, Restricted Project
amccarth added a comment to D100755: [llvm-rc] [3/4] Run clang to preprocess input files.

I'm not quite clear on the motivation. It's hard for me to imagine the resource compiler being useful if you cannot use the C preprocessor. I guess for simple tests you don't need it, is that worth adding an option?

Apr 19 2021, 2:10 PM · Restricted Project, Restricted Project
amccarth accepted D100753: [llvm-rc] [1/4] Simplify Opts.td to avoid repetition. NFC..
Apr 19 2021, 1:51 PM · Restricted Project

Apr 16 2021

amccarth added a comment to D100659: [ADT] Don't include <algorithm> in iterator.h.

Perhaps somebody confused <utility> std::move with <algorithm> std::move.

Apr 16 2021, 9:21 AM · Restricted Project

Apr 14 2021

amccarth accepted D100488: [SystemZ][z/OS] Add IsText Argument to GetFile and GetFileOrSTDIN.

Personally, I'm not a fan of boolean function parameters because of the inline comments necessary to make the call site understandable. But it appears to be consistent with LLVM Coding Standards and other APIs, so this looks right to me.

Apr 14 2021, 12:03 PM · Restricted Project, Restricted Project

Apr 12 2021

amccarth added a comment to D99580: [CLANG] [DebugInfo] Convert File name to native format.

Sorry, I think I've lost track of some context while I was on vacation. I don't understand why several of the tests are now unsupported on Windows. Some of those seem like important tests.

Apr 12 2021, 12:03 PM · Restricted Project, Restricted Project

Apr 1 2021

amccarth added a comment to D99580: [CLANG] [DebugInfo] Convert File name to native format.

It looks like the code change is for everyone, but the new test is specific to mingw.

For Linux like platform it does not create problem because input file path is already in POSIX style, so having a test case will not test the change because even without the change it will pass.

Apr 1 2021, 9:01 AM · Restricted Project, Restricted Project

Mar 30 2021

amccarth added a comment to D99580: [CLANG] [DebugInfo] Convert File name to native format.

The previous discussions (that I participated in) were centered around the redirecting virtual filesystem, which creates paths in hybrid style and there is no "correct" way to make those native without taking breaking changes and making it less useful for writing platform-agnostic tests. But it's not clear that's relevant here.

Mar 30 2021, 2:46 PM · Restricted Project, Restricted Project
amccarth added a comment to D99426: [SystemZ][z/OS][Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text.

I like the composability enabled by making OF_Text and OF_CRLF independent. I wonder, though, if there's a chokepoint where we could/should assert if the caller uses OF_CRLF without OF_Text, which would be suspicious.

Mar 30 2021, 9:39 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
amccarth accepted D99241: [libcxx] [test] Don't add dirs from the LIB env var to PATH.

LGTM.

Mar 30 2021, 8:49 AM · Restricted Project

Mar 29 2021

amccarth added a comment to D99241: [libcxx] [test] Don't add dirs from the LIB env var to PATH.

Adding everything in LIB to PATH is not a normal thing to do in Windows, so I would support this patch to remove that hack. If we knew what the original problem was, we could probably find a better solution.

Mar 29 2021, 1:41 PM · Restricted Project

Mar 25 2021

amccarth added a comment to D99357: [Support][Windows] Make sure only executables are found by sys::findProgramByName.

Just some drive-by comments.

Mar 25 2021, 3:55 PM · Restricted Project
amccarth added a comment to D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS.

When changing an IO stream's mode from binary to text breaks only Windows, it's likely due to the fact that Windows uses CR+LF as the newline indicator. Not only are the CRs sometimes unexpected, they can also throw off code that tries to seek to a computed file position.

Mar 25 2021, 10:46 AM · Restricted Project, Restricted Project

Mar 24 2021

amccarth accepted D99212: [Support] Fix Windows 7 path handling.

This looks even better.

Mar 24 2021, 9:11 AM · Restricted Project
amccarth accepted D99200: [SystemZ][z/OS] JSON file should be text files.

LGTM.

Mar 24 2021, 8:33 AM · Restricted Project

Mar 23 2021

amccarth accepted D99213: [libcxx] Avoid pulling in xlocinfo.h in public headers.

LGTM.

Mar 23 2021, 1:57 PM · Restricted Project
amccarth accepted D99212: [Support] Fix Windows 7 path handling.

I like the solution. Has anyone actually tried it on Windows 7? I ask because some of the observations noted in second hand descriptions of the problem didn't fully add up. I think we understand the root problem, but I'm not 100% certain.

Mar 23 2021, 1:54 PM · Restricted Project

Mar 17 2021

amccarth added a comment to D98529: [lldb] Strip pointer authentication codes from aarch64 pc..

Minidumps should have the registers in the processor context. It seems LLDB knows about TCR_ELn for n > 0. Maybe TCR_EL0 is special?

Mar 17 2021, 8:47 AM · Restricted Project

Mar 16 2021

amccarth added a comment to D98529: [lldb] Strip pointer authentication codes from aarch64 pc..

This workaround seems consistent with other overrides of FixCodeAddress, my only concern is about the assumption of the number of bits that need to be preserved/stripped.

Mar 16 2021, 11:18 AM · Restricted Project

Mar 5 2021

amccarth added a comment to D97538: [libcxx] [test] Ifdef out tests that rely on perms::none on directories for triggering errors.

Tagging @curdeius and @amccarth for input and ideas
If a process running as root can iterate over a directory even after chmod 000, I don't think changing the owner would help. Dunno about windows - on the API level we operate so far, permissions only consist of a boolean readonly flag, not much more. On some other level, windows filesystems have full ACLs for controlling access though.

Mar 5 2021, 3:34 PM · Restricted Project

Mar 4 2021

amccarth added a comment to D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS.

From a Windows point of view, this LGTM.

Mar 4 2021, 3:20 PM · Restricted Project, Restricted Project

Feb 26 2021

amccarth accepted D97539: [libcxx] Implement semaphores for windows.

Looks good enough to me. (But I'm also not a libc++ owner.)

Feb 26 2021, 4:04 PM · Restricted Project

Feb 25 2021

amccarth added a comment to D97437: Rewrite MSVC toolchain discovery with VFS.

LGTM.

Feb 25 2021, 1:31 PM · Restricted Project

Feb 24 2021

amccarth added a comment to D88220: [C++20] P1825R0: More implicit moves.

The option is used in the wild, but not as widely as I first believed. We've already fixed a couple projects that were using it. I think the Release Note is the right solution. Thanks for doing that. I withdraw my suggestion to allow and ignore.

Feb 24 2021, 8:33 AM · Restricted Project
amccarth accepted D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11.

LGTM.

Feb 24 2021, 8:29 AM · Restricted Project

Feb 19 2021

amccarth resigned from D96840: [LLDB] [docs] Update the list of supported architectures on Windows.
Feb 19 2021, 9:24 AM · Restricted Project
amccarth added a comment to D96840: [LLDB] [docs] Update the list of supported architectures on Windows.

I'm in no position to evaluate the functionality of lldb on Windows. I've spent the last few months trying to understand why 900+ tests spontaneously started failing on Windows.

Feb 19 2021, 9:24 AM · Restricted Project

Feb 18 2021

amccarth abandoned D96971: Allow but ignore `-Wreturn-std-move-in-c++11`.

Fair enough. I'm continuing the whack-a-mole game for the Chromium third-parties.

Feb 18 2021, 3:14 PM
amccarth requested review of D96971: Allow but ignore `-Wreturn-std-move-in-c++11`.
Feb 18 2021, 9:39 AM
amccarth added a comment to D88220: [C++20] P1825R0: More implicit moves.

The removal of -Wreturn-std-move-in-c++11 breaks a few projects. These projects are specifying the option, which then triggers an unknown-option warning and thus -Wx halts the build.

Feb 18 2021, 9:08 AM · Restricted Project

Feb 16 2021

amccarth added a comment to D95494: Support: Add mapped_file_region::sync(), equivalent to msync.

I think we'd just want it to sync as much say destruction a file stream or destruction a mapped_file_region.

Feb 16 2021, 3:55 PM · Restricted Project

Feb 12 2021

amccarth added a comment to D95494: Support: Add mapped_file_region::sync(), equivalent to msync.
In D95494#2534572, @rnk wrote:

@amccarth, can you confirm that these are the right APIs to use here? Do we need both FlushViewOfFile and FlushFileBuffers?

Feb 12 2021, 2:13 PM · Restricted Project

Jan 29 2021

amccarth accepted D91171: [16/N] [libcxx] Implement the permissions function for windows.

LGTM

Jan 29 2021, 3:38 PM · Restricted Project
amccarth accepted D91170: [15/N] [libcxx] Implement the canonical function for windows.

The changes to provide realpath on Windows looks right. Since it's relying on a system call, it would be difficult to create tests for the \\?\ and UNC cases. I'm assuming the more mainstream cases will be tested by existing tests. So LGTM.

Jan 29 2021, 10:45 AM · Restricted Project

Jan 22 2021

amccarth added a comment to D70701: Fix more VFS tests on Windows.

BTW, I hope I didn't come across as overly negative in my previous
response. I'd love to see the situation improved. I just don't know what
that would look like.

Jan 22 2021, 10:03 AM · Restricted Project, Restricted Project

Jan 21 2021

amccarth updated subscribers of D70701: Fix more VFS tests on Windows.

I didn't design VFS. I just fixed a bunch of portability bugs that
prevented us from running most of the VFS tests on Windows. If I were
designing it, I (hope) I wouldn't have done it this way. But the horse is
already out of the barn.

Jan 21 2021, 5:36 PM · Restricted Project, Restricted Project

Jan 13 2021

amccarth accepted D92419: [Support] On Windows, take the affinity mask into account.

Thanks for the changes!

Jan 13 2021, 3:59 PM · Restricted Project

Jan 11 2021

amccarth accepted D91176: [20/N] [libcxx] Implement parsing of root_name for paths on windows.
Jan 11 2021, 2:00 PM · Restricted Project

Jan 8 2021

amccarth added a comment to D91176: [20/N] [libcxx] Implement parsing of root_name for paths on windows.

This looks good to me, in that I believe it will correctly parse typical native Windows path names. Only tests will tell, but I understand that there are many patches necessary to enable the tests.

Jan 8 2021, 5:10 PM · Restricted Project
amccarth accepted D91853: [compiler-rt] [sanitizer] Silence -Wframe-larger-than= for a few windows functions with large stack buffers.

LGTM, but please take the suggested edit to update a documentation URL.

Jan 8 2021, 12:11 PM · Restricted Project
amccarth accepted D91178: [22/N] [libcxx] Implement append and operator/ properly for windows.

LGTM.

Jan 8 2021, 11:44 AM · Restricted Project
amccarth accepted D91171: [16/N] [libcxx] Implement the permissions function for windows.

LGTM.

Jan 8 2021, 11:43 AM · Restricted Project

Jan 5 2021

amccarth added a comment to D91852: [compiler-rt] [windows] Add UNUSED attributes on variables/functions only used for 64 bit targets.

Ping @amccarth, do you want to follow up on the discussion before I land this?

Jan 5 2021, 1:56 PM · Restricted Project

Dec 17 2020

amccarth accepted D93456: [libcxx] Avoid overflows in the windows __libcpp_steady_clock_now().

This looks correct and straightforward, so LGTM.

Dec 17 2020, 9:08 AM · Restricted Project

Dec 11 2020

amccarth accepted D91172: [17/N] [libcxx] Implement the read_symlink function for windows.

LGTM.

Dec 11 2020, 2:36 PM · Restricted Project