amccarth (Adrian McCarthy)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 26 2015, 9:26 AM (173 w, 3 d)

Recent Activity

Tue, May 15

amccarth accepted D46875: [llvm-rc] Add support for the optional CLASS statement for dialogs.
Tue, May 15, 11:22 AM
amccarth added inline comments to D46875: [llvm-rc] Add support for the optional CLASS statement for dialogs.
Tue, May 15, 8:57 AM

Mon, May 14

amccarth accepted D46816: [llvm-rc] Read the Planes/BitCount fields from BITMAPINFOHEADER for icons.
Mon, May 14, 2:48 PM
amccarth added inline comments to D46816: [llvm-rc] Read the Planes/BitCount fields from BITMAPINFOHEADER for icons.
Mon, May 14, 2:39 PM
amccarth accepted D46818: [llvm-rc] Add support for parsing memory flags.

Thanks for splitting the test.

Mon, May 14, 2:33 PM
amccarth added a comment to D46818: [llvm-rc] Add support for parsing memory flags.

I kinda wish the test was broken up a bit. The stringtable bundle logic is distinctly different than just applying the keywords to other resource types, so that would be a natural place to break this apart.

Mon, May 14, 9:22 AM
amccarth added inline comments to D46816: [llvm-rc] Read the Planes/BitCount fields from BITMAPINFOHEADER for icons.
Mon, May 14, 8:51 AM
amccarth accepted D46813: [llvm-rc] Add missing inputs for tag-icon-cursor.test..

Ah, I see you figured out the discrepancy in a follow-on patch (46816), so this LGTM.

Mon, May 14, 8:35 AM
amccarth added inline comments to D46813: [llvm-rc] Add missing inputs for tag-icon-cursor.test..
Mon, May 14, 8:28 AM

Wed, May 9

amccarth accepted D46506: [llvm-rc] Allow -1 for control IDs in old style dialogs with 16 bit fields .

LGTM, but consider inline suggestion.

Wed, May 9, 9:06 AM
amccarth accepted D46636: [llvm-rc] Add support for the RCDATA resource type.

The docs suggest there are differences between RCDATA and user-defined resource definitions, but the Microsoft rc.exe doesn't seem to follow the docs. For example, the docs say user-defined types can use an external file but that RCDATA cannot. In real life, the resource compiler will accept an RCDATA statement that does reference an external file. Also, the docs say RCDATA can have VERSION, CHARACTERISTICS, and LANGUAGE optional statements, but those don't seem to work because the keywords are mistaken for file names.

Wed, May 9, 8:36 AM
amccarth accepted D46579: [llvm-rc] Handle C preprocessor output.
Wed, May 9, 8:36 AM

Tue, May 8

amccarth added inline comments to D46579: [llvm-rc] Handle C preprocessor output.
Tue, May 8, 4:03 PM
amccarth accepted D46507: [llvm-rc] Add support for all missing dialog controls.

Thanks!

Tue, May 8, 1:13 PM
amccarth added inline comments to D46507: [llvm-rc] Add support for all missing dialog controls.
Tue, May 8, 11:07 AM
amccarth added inline comments to D46507: [llvm-rc] Add support for all missing dialog controls.
Tue, May 8, 10:35 AM

Mon, May 7

amccarth added inline comments to D46427: [PDB] Quote linker arguments containing spaces (mimic MSVC).
Mon, May 7, 3:54 PM · lld
amccarth accepted D46511: [llvm-rc] Don't strictly require quotes around external file names.

I'm happy. Thanks!

Mon, May 7, 2:21 PM
amccarth accepted D46509: [llvm-rc] Implement the BITMAP resource type.
Mon, May 7, 1:21 PM
amccarth accepted D46511: [llvm-rc] Don't strictly require quotes around external file names.
Mon, May 7, 11:48 AM
amccarth added inline comments to D46511: [llvm-rc] Don't strictly require quotes around external file names.
Mon, May 7, 11:38 AM
amccarth added inline comments to D46509: [llvm-rc] Implement the BITMAP resource type.
Mon, May 7, 11:28 AM
amccarth accepted D46508: [llvm-rc] Allow optional commas between the string table index and value.

As I recall, commas may be optionally allowed in several places besides string tables. We'll have to check that and, if necessary, address in a subsequent patch.

Mon, May 7, 11:23 AM
amccarth accepted D46507: [llvm-rc] Add support for all missing dialog controls.

Looks good. AFAICT we're now only missing PUSHBOX and CONTROL. If these don't present any additional difficulty, maybe better to just add them now for the sake of completeness?

Mon, May 7, 11:19 AM
amccarth accepted D46510: [llvm-rc] Exclude padding from sizes in versioninfo resources.
Mon, May 7, 11:16 AM
amccarth added inline comments to D46511: [llvm-rc] Don't strictly require quotes around external file names.
Mon, May 7, 11:07 AM

Wed, May 2

amccarth accepted D46358: [llvm-cvtres] Allow parameters preceded by '-' in addition to '/'.

I'm mildly concerned about the tests that give filenames with forward slashes. That doesn't work with a lot of Windows command lines, but apparently the tests here were already doing that.

Wed, May 2, 2:00 PM
amccarth accepted D46239: [llvm-rc] Default to writing the output next to the input, if no output is specified.

LGTM

Wed, May 2, 1:54 PM
amccarth accepted D46238: [llvm-rc] Add rudimentary support for codepages.

LGTM.

Wed, May 2, 11:42 AM

Mon, Apr 30

amccarth accepted D46292: Use the UUID from the minidump's CodeView Record for placeholder modules.

The big picture here is fine.

Mon, Apr 30, 3:52 PM · Restricted Project
amccarth added a comment to D46238: [llvm-rc] Add rudimentary support for codepages.

Looks good to me now, but I'm not an official reviewer.

Mon, Apr 30, 1:50 PM
amccarth added inline comments to D46238: [llvm-rc] Add rudimentary support for codepages.
Mon, Apr 30, 11:53 AM
amccarth added inline comments to D46238: [llvm-rc] Add rudimentary support for codepages.
Mon, Apr 30, 10:10 AM

Fri, Apr 27

amccarth added a comment to D46164: Support: assume `std::is_final` with MSVC.

This looks fine to me. I just dropped in to add some context on MSVC and __cplusplus: https://blogs.msdn.microsoft.com/vcblog/2018/04/09/msvc-now-correctly-reports-__cplusplus/

Fri, Apr 27, 7:50 AM

Apr 19 2018

amccarth committed rL330354: Fix narrowing warning by appending `f` to literal constant..
Fix narrowing warning by appending `f` to literal constant.
Apr 19 2018, 11:35 AM

Apr 18 2018

amccarth accepted D45700: Improve LLDB's handling of non-local minidumps.

I actually preferred the factory solution.

Apr 18 2018, 1:51 PM · Restricted Project

Apr 17 2018

amccarth accepted D45700: Improve LLDB's handling of non-local minidumps.

LGTM, but consider highlighting the side effect to target when the factory makes a Placeholder module.

Apr 17 2018, 4:49 PM · Restricted Project

Apr 16 2018

amccarth added a comment to D45700: Improve LLDB's handling of non-local minidumps.

Nice!

Apr 16 2018, 2:48 PM · Restricted Project
amccarth committed rL330135: Remove faulty assertion in llvm-pdbutil.
Remove faulty assertion in llvm-pdbutil
Apr 16 2018, 10:05 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Apr 16 2018, 10:05 AM

Apr 13 2018

amccarth added a comment to D45645: Remove faulty assertion in llvm-pdbutil.

I meant to mention that the instance of the empty class is reported as 8 bytes of padding (in a 64-bit build), in case that's relevant to anybody's understanding of the issue.

Apr 13 2018, 4:44 PM
amccarth created D45645: Remove faulty assertion in llvm-pdbutil.
Apr 13 2018, 4:41 PM

Apr 9 2018

amccarth accepted D45345: [annotated_builder] try harder to clean build directories.

Yes, by all means. My intent was not to block this but to point out that it might still suffer from an already existent problem.

Apr 9 2018, 4:07 PM
amccarth added a comment to D45345: [annotated_builder] try harder to clean build directories.

This looks like an improvement, but it still might not work on Windows, because there you're racing the file system--deleting a file on Windows is not an atomic, instantaneous operation. Tune in to https://www.youtube.com/watch?v=uhRWMGBjlO8 at 7:30 for details.

Apr 9 2018, 2:29 PM

Mar 22 2018

amccarth added inline comments to D44810: [PDB] Make LLD PDBs look a little more like Microsoft PDBs.
Mar 22 2018, 4:00 PM
amccarth added a comment to D44771: [lit] Test /dev/null support on Windows..
In D44771#1046003, @rnk wrote:

I have a feeling if you just say open("NUL", mode) you will create a file called "NUL" in the cwd. It's not something you can feed to generic filesystem APIs.

Mar 22 2018, 1:57 PM

Feb 26 2018

amccarth abandoned D43784: Un-XFAIL TestCallStdStringFunction.test for Windows.

Abandoning. This isn't really working on Windows. The breakpoint fails to set for reasons I don't understand yet.

Feb 26 2018, 4:44 PM
amccarth added a comment to D43784: Un-XFAIL TestCallStdStringFunction.test for Windows.

Nice catch. I'll take a closer look to see what exactly is happening on Windows.

Feb 26 2018, 2:34 PM
amccarth created D43784: Un-XFAIL TestCallStdStringFunction.test for Windows.
Feb 26 2018, 1:49 PM
amccarth committed rL326130: Partial fix for TestConflictingSymbol.py on Windows.
Partial fix for TestConflictingSymbol.py on Windows
Feb 26 2018, 1:27 PM
amccarth closed D43688: Partial fix for TestConflictingSymbol.py on Windows.
Feb 26 2018, 1:27 PM
amccarth committed rL326095: Fix tabs/spaces indentation problem in TestUnicodeSymbols.py.
Fix tabs/spaces indentation problem in TestUnicodeSymbols.py
Feb 26 2018, 7:57 AM
amccarth closed D43705: Fix tabs/spaces in TestUnicodeSymbols.py.
Feb 26 2018, 7:57 AM

Feb 23 2018

amccarth created D43705: Fix tabs/spaces in TestUnicodeSymbols.py.
Feb 23 2018, 5:20 PM
amccarth added inline comments to D43596: Replace HashStringUsingDJB with llvm::djbHash.
Feb 23 2018, 5:00 PM
amccarth added a comment to D43662: [Utility] Simplify and generalize the CleanUp helper, NFC.

Nit: As a single word, "cleanup" is a noun (or an adjective). As two words, "clean up" is a verb. Given that, I'd expect to see classes and objects with names like Cleanup or cleanup and functions to contain CleanUp. When I see an identifier like CleanUpTest, I expect it's a function that cleans up after a test, not a test of a class called CleanUp.

Feb 23 2018, 2:34 PM
amccarth added a comment to D43688: Partial fix for TestConflictingSymbol.py on Windows.

On non-windows systems we're expected to pick up the name from the symbol table. Is there such a thing on windows as well?

Given that constant is declared with visibility("hidden"), why would it end up in a symbol table? Windows DLLs have tables for items explicitly exported, but that's contrary to asking the symbol to be hidden.

visibility((hidden)) means it won't end up in the *exported* symbol table (.dynsym on linux). However, the compiler will still put it into the local symbol table (.symtab, which is not used by the linker or at runtime, but the debugger can use it to get more insight).

Feb 23 2018, 2:18 PM
amccarth added a comment to D43688: Partial fix for TestConflictingSymbol.py on Windows.

This test deliberately compiles without -g, so the variable should not be in the debug info.

Feb 23 2018, 11:42 AM
amccarth created D43688: Partial fix for TestConflictingSymbol.py on Windows.
Feb 23 2018, 11:13 AM
amccarth added a comment to D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows..

That's weird, because lots of lldb tests compile and link test binaries on Windows with -fuse-ld=lld (without the .exe). What makes you say the .exe is necessary?

Maybe he is using clang (not clang-cl) and want to use ld.lld.exe instead of lld-link?

Feb 23 2018, 7:52 AM · Restricted Project

Feb 22 2018

amccarth committed rL325838: Fix llvm-pdbutil to handle new built-in types.
Fix llvm-pdbutil to handle new built-in types
Feb 22 2018, 3:21 PM
amccarth closed D43646: Fix llvm-pdbutil to handle new built-in types.
Feb 22 2018, 3:21 PM
amccarth created D43646: Fix llvm-pdbutil to handle new built-in types.
Feb 22 2018, 3:06 PM
amccarth committed rL325836: Fix TestMoveNearest on Windows.
Fix TestMoveNearest on Windows
Feb 22 2018, 2:50 PM
amccarth committed rL325835: Fix TestSBData.py on Windows.
Fix TestSBData.py on Windows
Feb 22 2018, 2:50 PM
amccarth closed D43600: Fix TestMoveNearest on Windows.
Feb 22 2018, 2:50 PM
amccarth closed D43532: Fix TestSBData.py on Windows (which uses Python 3).
Feb 22 2018, 2:49 PM
amccarth added a comment to D43600: Fix TestMoveNearest on Windows.

call_foo1() is defined in foo.cpp, which is built into a DLL, and thus needs the __declspec(dllimport/dllexport) from the LLDB_TEST_API. foo.h defines the API for the DLL.

Feb 22 2018, 2:41 PM
amccarth added a comment to D43600: Fix TestMoveNearest on Windows.

Do we not need call_foo2 anymore?

Feb 22 2018, 2:13 PM
amccarth added a comment to D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows..

Right now, we have to add an .exe suffix when using this switch, like -fuse-ld=lld.exe.

Feb 22 2018, 10:35 AM · Restricted Project
amccarth added a comment to D43621: [Driver] Allow using a canonical form of '-fuse-ld=' when cross-compiling on Windows..

This must be new-ish code, since I've routinely used -fuse-ld=lld (without .exe) on Windows.

Feb 22 2018, 8:04 AM · Restricted Project

Feb 21 2018

amccarth abandoned D43599: FFix TestSBData.py on Windows (which uses Python 3).

Please ignore. I'm still trying to figure out arc.

Feb 21 2018, 4:21 PM
amccarth created D43600: Fix TestMoveNearest on Windows.
Feb 21 2018, 4:20 PM
amccarth created D43599: FFix TestSBData.py on Windows (which uses Python 3).
Feb 21 2018, 4:19 PM
amccarth committed rL325704: Fix TestBreakpointInGlobalConstructor for Windows.
Fix TestBreakpointInGlobalConstructor for Windows
Feb 21 2018, 10:10 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Feb 21 2018, 10:10 AM
amccarth updated the diff for D43419: Fix TestBreakpointInGlobalConstructor for Windows.

Per Pavel's suggestion, change special value to mean don't check the number of locations found.

Feb 21 2018, 9:05 AM
amccarth added a comment to D43419: Fix TestBreakpointInGlobalConstructor for Windows.

By unconditional, do you mean allowing any value for out_num_locations in these cases? I'm happy to do that, but I'm not sure if I've understood you correctly.

Feb 21 2018, 8:03 AM

Feb 20 2018

amccarth created D43532: Fix TestSBData.py on Windows (which uses Python 3).
Feb 20 2018, 3:18 PM
amccarth added a comment to D43419: Fix TestBreakpointInGlobalConstructor for Windows.

At some point we should introduce "platformCapacities" so that you could do:

if not test.getPlatformCapacities("preload-dylibs"):

so we are testing specific features not the whole platform.

Feb 20 2018, 12:45 PM
amccarth updated the diff for D43419: Fix TestBreakpointInGlobalConstructor for Windows.

Wrapped the modified docstring.

Feb 20 2018, 12:44 PM
amccarth updated the diff for D43419: Fix TestBreakpointInGlobalConstructor for Windows.

Added new special value to accommodate the fact that Windows may postpone resolving the breakpoints for DLLs that aren't yet loaded.

Feb 20 2018, 12:01 PM
amccarth added a comment to D43419: Fix TestBreakpointInGlobalConstructor for Windows.

I have reservations about making the debugger try to pre-locate the modules and their PDBs before those modules are actually loaded. For a few reasons.

Feb 20 2018, 9:37 AM

Feb 16 2018

amccarth added a comment to D43419: Fix TestBreakpointInGlobalConstructor for Windows.

OK, so my fix is too simplistic. I'll have to (1) make LLDB on Windows also attempt to pre-load the DLLs, (2) make the test use platform-specific expectations, or (3) use a looser expectation. I'll give it some thought over the weekend.

Feb 16 2018, 4:27 PM
amccarth created D43419: Fix TestBreakpointInGlobalConstructor for Windows.
Feb 16 2018, 3:53 PM
amccarth accepted D43326: [PDB] Fix emission of PDB string table.

LGTM.

Feb 16 2018, 11:36 AM

Feb 15 2018

amccarth added a comment to D43326: [PDB] Fix emission of PDB string table.

Excellent patch description! It's reassuring to see all the nagging questions explained. I like the test, too.

Feb 15 2018, 8:27 AM

Feb 14 2018

amccarth committed rL325188: Supply missing break in case statement..
Supply missing break in case statement.
Feb 14 2018, 3:19 PM
amccarth closed D43215: Supply missing break in case statement..
Feb 14 2018, 3:19 PM
amccarth added a comment to D43215: Supply missing break in case statement..

Any remaining concerns?

Feb 14 2018, 2:46 PM
amccarth updated the diff for D43215: Supply missing break in case statement..

Per Aaron's suggestion, I ran the tests with an unconditional return (and an assertion). There was no change in the test results, so I'm making this return unconditionally.

Feb 14 2018, 9:50 AM

Feb 12 2018

amccarth added a reviewer for D43215: Supply missing break in case statement.: zturner.
Feb 12 2018, 4:33 PM
amccarth created D43215: Supply missing break in case statement..
Feb 12 2018, 4:32 PM
amccarth committed rL324925: Remove dead code for handling DWARF pubnames.
Remove dead code for handling DWARF pubnames
Feb 12 2018, 11:21 AM
amccarth closed D43202: Remove dead code for handling DWARF pubnames.
Feb 12 2018, 11:20 AM
amccarth added reviewers for D43202: Remove dead code for handling DWARF pubnames: labath, jingham.
Feb 12 2018, 11:06 AM
amccarth created D43202: Remove dead code for handling DWARF pubnames.
Feb 12 2018, 11:03 AM

Feb 7 2018

amccarth added inline comments to D43046: [Windows] Set the console output page to UTF-8.
Feb 7 2018, 4:01 PM

Feb 5 2018

amccarth accepted D42592: [COFF] Add minimal support for /guard:cf.

LGTM.

Feb 5 2018, 4:57 PM
amccarth added inline comments to D42925: Call FlushFileBuffers on readwrite file mappings..
Feb 5 2018, 1:07 PM