Page MenuHomePhabricator

nealsid (Neal)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 28 2018, 5:03 PM (168 w, 3 d)

Recent Activity

Aug 18 2021

nealsid added a comment to D106035: Remove conditional compilation for WCHAR support in libedit.

Sorry, I haven't forgotten about this. Haven't had time to sit down for a proper debugging session this or last week. I definitely still plan to update and resend for review. Thanks!

Aug 18 2021, 6:04 PM · Restricted Project

Aug 5 2021

nealsid added a comment to D106035: Remove conditional compilation for WCHAR support in libedit.

Yikes - issue count starting to slowly increase :) I've reverted the patch, and sorry again about that.

Aug 5 2021, 3:01 AM · Restricted Project
nealsid added a comment to D106035: Remove conditional compilation for WCHAR support in libedit.

:) Shoot. I definitely want to revert ASAP rather than keep the tree in a bad state. Just curious, since there are timeout messages in the log, would you be able to try rerunning just that one? I see this, too: 0: b"error: '\xe1\x88\xb4' is not a valid command. So I'm guessing the timeout is a red herring.

Aug 5 2021, 2:14 AM · Restricted Project
nealsid added a comment to D106035: Remove conditional compilation for WCHAR support in libedit.

The release branch was recently made, so we can land this right now. Just give me a ping when I should merge this :)

Aug 5 2021, 12:33 AM · Restricted Project

Aug 4 2021

nealsid updated the diff for D106035: Remove conditional compilation for WCHAR support in libedit.

Update against HEAD; tested on Debian Buster and Monterey

Aug 4 2021, 11:43 PM · Restricted Project

Aug 1 2021

nealsid updated the diff for D106035: Remove conditional compilation for WCHAR support in libedit.

Update against HEAD (I still need to do a bit more testing but wanted to get the buildbot results in the meantime)

Aug 1 2021, 4:08 PM · Restricted Project

Jul 18 2021

nealsid added a comment to D106035: Remove conditional compilation for WCHAR support in libedit.

I actually expected after the RFC that we would remove all the non-wchar code, but this seems also fine. I think this LGTM in general, but I feel a bit nervous about touching stuff that depends so much on OS/environment. What OS/environment did you test this patch on? Would be nice to have this tested on a few setups before landing.

This was my mistake, sorry. I originally went the route in this patch and ran into some errors testing, so I switched to what I detailed in the RFC. But then I found the problem (I was using narrow chars for the GetCharacter callback when that actually isn't supported). Overall I think it is best to use narrow char and string rather than wide-char and wstring because that's more consistent with the rest of LLVM.

I actually like this approach even better, so I'm glad this turned out the way it did!

Regarding platforms, I tested on OS X Big Sur and Monterey, and Debian Linux inside a VM. Jan was able to build on Fedora. I'm happy to test on more platforms - FreeBSD, NetBSD perhaps?

I think this is more than enough to test on before landing so this LGTM. FWIW, we do have a release branching in about 10 days or so, so you might want to wait with landing this just directly after that branch was made so that this spends a bit more time on ToT where we can find issues before going into a release. (I assume you don't care whether this makes it into the current release or not, but please correct me if i'm wrong).

Jul 18 2021, 4:03 PM · Restricted Project

Jul 16 2021

nealsid updated the diff for D106035: Remove conditional compilation for WCHAR support in libedit.

Reran clang-format and removed extra braces.

Jul 16 2021, 11:16 PM · Restricted Project
nealsid added a comment to D106035: Remove conditional compilation for WCHAR support in libedit.

I actually expected after the RFC that we would remove all the non-wchar code, but this seems also fine. I think this LGTM in general, but I feel a bit nervous about touching stuff that depends so much on OS/environment. What OS/environment did you test this patch on? Would be nice to have this tested on a few setups before landing.

Jul 16 2021, 11:15 PM · Restricted Project

Jul 14 2021

nealsid updated the summary of D106035: Remove conditional compilation for WCHAR support in libedit.
Jul 14 2021, 8:53 PM · Restricted Project
nealsid requested review of D106035: Remove conditional compilation for WCHAR support in libedit.
Jul 14 2021, 8:52 PM · Restricted Project

Jun 27 2021

nealsid abandoned D104437: Add test for functions with extended characters..

Not needed with existing test coverage.

Jun 27 2021, 9:28 PM · Restricted Project
nealsid added a comment to D104437: Add test for functions with extended characters..

Agreed, it looks like there's plenty of test coverage for this scenario, so I'll revert this patch - sorry about that. I'm working on removing the wchar/char preprocessor logic in Editline.cpp so I'll verify the existing coverage before sending that for review. Thanks.

Jun 27 2021, 9:27 PM · Restricted Project

Jun 21 2021

nealsid added a comment to D104437: Add test for functions with extended characters..

Thanks to both of you for this! I’m traveling for a few days but will be able to take a look towards the end of the week.

Jun 21 2021, 11:24 PM · Restricted Project

Jun 17 2021

nealsid added a comment to D104437: Add test for functions with extended characters..

Ah, my grep/find skills clearly failed me :)

Jun 17 2021, 1:25 PM · Restricted Project

Jun 16 2021

nealsid updated the diff for D104437: Add test for functions with extended characters..

Add newline.

Jun 16 2021, 8:18 PM · Restricted Project
nealsid requested review of D104437: Add test for functions with extended characters..
Jun 16 2021, 8:17 PM · Restricted Project

May 18 2021

nealsid added a comment to D102260: Update MSVC version number in preprocessor check.

Awesome. If you or someone could land it for me, that would be appreciated. Thanks!

May 18 2021, 1:51 AM · Restricted Project
nealsid updated the diff for D102260: Update MSVC version number in preprocessor check.
May 18 2021, 1:14 AM · Restricted Project

May 16 2021

nealsid added a comment to D102260: Update MSVC version number in preprocessor check.

Sorry, this got a little complicated. On my previous install I had VS 2019 16.5, which ran into the compile error. When I reinstalled, 16.0, 16.6, and 16.9 worked, but 16.4/16.5 did not. Sounds like a super tiny regression for only 2 minor updates. As far as the actual issue, I could not fully understand it, but it seems to have something to do with applying parameter pack arguments while also using default values for std::map template parameters.

May 16 2021, 5:21 PM · Restricted Project

May 13 2021

nealsid added a comment to D102260: Update MSVC version number in preprocessor check.

Ah, my Windows Boot Camp partition seems to have bit it, so it may be a couple days before I can test & update this, sorry. Please hold off.

May 13 2021, 2:59 PM · Restricted Project

May 12 2021

nealsid added a comment to D102260: Update MSVC version number in preprocessor check.

Whoops, I did not realize my VS was out of date - appreciate the link. Let me try building with the latest updates, perhaps a later version works.

May 12 2021, 10:57 AM · Restricted Project

May 11 2021

nealsid added a comment to D102208: Remove Windows editline from LLDB.

Thanks, all. If there are no more comments, could someone land it for me? I don't have commit access.

May 11 2021, 4:08 PM · Restricted Project, Restricted Project
nealsid added a comment to D102208: Remove Windows editline from LLDB.

I ran LLDB tests from a clean tree and with my change and the failures seemed to be the same, but there is a large amount of them. I also did manual testing with the changes. Build is still green.

May 11 2021, 11:28 AM · Restricted Project, Restricted Project
nealsid updated the summary of D102208: Remove Windows editline from LLDB.
May 11 2021, 11:03 AM · Restricted Project, Restricted Project
nealsid updated the diff for D102208: Remove Windows editline from LLDB.

Updated the diff after moving an unrelated change into another revision.

May 11 2021, 11:01 AM · Restricted Project, Restricted Project
nealsid requested review of D102260: Update MSVC version number in preprocessor check.
May 11 2021, 10:54 AM · Restricted Project

May 10 2021

nealsid requested review of D102208: Remove Windows editline from LLDB.
May 10 2021, 7:25 PM · Restricted Project, Restricted Project

Apr 29 2021

nealsid added a comment to D101250: Wrap edit line configuration calls into helper functions.

Friendly ping - let me know if I misunderstood the comments or if anything else is needed! Thanks again.

Apr 29 2021, 12:36 PM · Restricted Project

Apr 26 2021

nealsid added a comment to D101250: Wrap edit line configuration calls into helper functions.

LGTM. I wonder if we would want to wrap this in a macro to get rid of the EditLineConstString duplication while keeping the type safety.

Apr 26 2021, 9:17 PM · Restricted Project

Apr 25 2021

nealsid requested review of D101250: Wrap edit line configuration calls into helper functions.
Apr 25 2021, 1:25 AM · Restricted Project

Apr 20 2021

nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Small update of patch.

Apr 20 2021, 5:21 PM · Restricted Project

Apr 13 2021

nealsid added a comment to D98153: Some FormatEntity.cpp cleanup and unit testing.

Thanks for the review, and apologies for the delay in updating the diffs - seems like I haven't even had an hour to sit down at my dev machine for the past couple weeks, much less actually do any coding :) I don't have commit access, so if you could do that, that would be much appreciated.

Apr 13 2021, 10:27 PM · Restricted Project
nealsid added inline comments to D98153: Some FormatEntity.cpp cleanup and unit testing.
Apr 13 2021, 10:24 PM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Update type in for loop.

Apr 13 2021, 10:13 PM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Fixed comment.

Apr 13 2021, 9:53 PM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

CR feedback

Apr 13 2021, 9:50 PM · Restricted Project

Mar 26 2021

nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Thanks for the review - addressed your comments here (I had to rely on Doxygen picking up the type reference automatically because I couldn't get the \see syntax to work correctly, though)

Mar 26 2021, 1:20 AM · Restricted Project

Mar 15 2021

nealsid added a comment to D98153: Some FormatEntity.cpp cleanup and unit testing.

Thanks for cleaning this up! Are the if (!sc) ... stuff are missing nullptr checks that affect a normal LLDB session (not sure if we can ever have no SymbolContext) or is this just for the unit test?

Anyway, I have some small inline comments but otherwise this looks pretty good to me.

Mar 15 2021, 10:57 PM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Rebased on HEAD

Mar 15 2021, 10:49 PM · Restricted Project

Mar 10 2021

nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Fix regression introduced by removing return statement in case where we were able to format a function name.

Mar 10 2021, 8:26 PM · Restricted Project
nealsid planned changes to D98153: Some FormatEntity.cpp cleanup and unit testing.
Mar 10 2021, 8:12 PM · Restricted Project

Mar 9 2021

nealsid added a comment to D98153: Some FormatEntity.cpp cleanup and unit testing.

Thanks for cleaning this up! Are the if (!sc) ... stuff are missing nullptr checks that affect a normal LLDB session (not sure if we can ever have no SymbolContext) or is this just for the unit test?

Anyway, I have some small inline comments but otherwise this looks pretty good to me.

Mar 9 2021, 11:51 AM · Restricted Project
nealsid added inline comments to D98153: Some FormatEntity.cpp cleanup and unit testing.
Mar 9 2021, 2:48 AM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.
Mar 9 2021, 2:44 AM · Restricted Project

Mar 8 2021

nealsid added a comment to D98153: Some FormatEntity.cpp cleanup and unit testing.

Thanks - all the comments sound good. I'll upload a new patch.

Mar 8 2021, 5:09 PM · Restricted Project

Mar 7 2021

nealsid updated the summary of D98153: Some FormatEntity.cpp cleanup and unit testing.
Mar 7 2021, 6:30 PM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Reuploading patch after I incorrectly set project repo.

Mar 7 2021, 6:29 PM · Restricted Project
nealsid changed the repository for D98153: Some FormatEntity.cpp cleanup and unit testing from rLLDB LLDB to rG LLVM Github Monorepo.
Mar 7 2021, 6:25 PM · Restricted Project
nealsid updated the diff for D98153: Some FormatEntity.cpp cleanup and unit testing.

Rebased on HEAD

Mar 7 2021, 5:44 PM · Restricted Project
nealsid requested review of D98153: Some FormatEntity.cpp cleanup and unit testing.
Mar 7 2021, 1:27 PM · Restricted Project

Mar 1 2021

nealsid added a comment to D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Removed comment on #include line and organized the #includes to match coding guidelines better (I wasn't sure about whether the project uses blank lines in between #includes from different subproject like lldb/clang/llvm so I didn't make the change more broadly)

Looks good. Let me know if you don't have commit access and need me to land this for you.

Mar 1 2021, 7:02 PM · Restricted Project
nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Removed comment on #include line and organized the #includes to match coding guidelines better (I wasn't sure about whether the project uses blank lines in between #includes from different subproject like lldb/clang/llvm so I didn't make the change more broadly)

Mar 1 2021, 4:22 PM · Restricted Project

Feb 27 2021

nealsid added a project to D50299: Migrate to llvm::unique_function instead of static member functions for callbacks: Restricted Project.
Feb 27 2021, 11:33 PM · Restricted Project
nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Updated diff with run of clang-format and added a check for function pointer validity. Thanks.

Feb 27 2021, 11:33 PM · Restricted Project

Feb 26 2021

nealsid added a comment to D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Thanks Neal. Can you run clang-format again and upload the diff with context (git diff -U9999). Overall this looks pretty good.

Feb 26 2021, 10:10 PM · Restricted Project

Feb 25 2021

nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

I'm updating this very old patch that I forgot about in 2018. This one removes the use of batons in the edit line callbacks, replaces typedefs with using declarations, and uses unique_function instead of callbacks.
Thanks,
Neal

Feb 25 2021, 4:50 PM · Restricted Project

Aug 31 2018

nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Ran clang-format

Aug 31 2018, 10:30 AM · Restricted Project
nealsid retitled D50299: Migrate to llvm::unique_function instead of static member functions for callbacks from Migrate to std::function instead of static member functions for callbacks to Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 31 2018, 9:11 AM · Restricted Project
nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Fix 80 char line length violations

Aug 31 2018, 9:11 AM · Restricted Project
nealsid updated the summary of D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 31 2018, 9:00 AM · Restricted Project
nealsid updated the summary of D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 31 2018, 9:00 AM · Restricted Project

Aug 6 2018

nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 6 2018, 2:25 PM · Restricted Project
nealsid added a comment to D49963: Preliminary patch to support prompt interpolation.

This will be very cool. Biggest issues to watch out for is to make sure it doesn't return incorrect values when running for things like the thread count. If you switch to use the "m_debugger.GetCommandInterpreter().GetExecutionContext()" it should solve this by making sure the frame and thread are not filled in when the process is running. It might also be racy. For example if you say "continue", hopefully the process will be resumed by the time the prompt is asked to refresh itself. Since we get events asynchronously this might be a problem.

Aug 6 2018, 12:21 PM
nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 6 2018, 9:27 AM · Restricted Project
nealsid added inline comments to D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 6 2018, 9:24 AM · Restricted Project
nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Addressed code review feedback to move to llvm::unique_function instead of function pointers for callbacks and type aliases instead of typedefs.

Aug 6 2018, 9:23 AM · Restricted Project

Aug 5 2018

nealsid added inline comments to D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 5 2018, 3:31 PM · Restricted Project

Aug 4 2018

nealsid planned changes to D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Thanks for the comments! Will address and resend for review.

Aug 4 2018, 10:02 PM · Restricted Project
nealsid updated the diff for D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.

Fix unit test build

Aug 4 2018, 3:39 PM · Restricted Project
nealsid created D50299: Migrate to llvm::unique_function instead of static member functions for callbacks.
Aug 4 2018, 3:31 PM · Restricted Project

Aug 3 2018

nealsid planned changes to D49963: Preliminary patch to support prompt interpolation.
Aug 3 2018, 1:49 PM
nealsid accepted D49963: Preliminary patch to support prompt interpolation.
Aug 3 2018, 1:47 PM

Jul 28 2018

nealsid updated subscribers of D49963: Preliminary patch to support prompt interpolation.
Jul 28 2018, 9:53 PM
nealsid created D49963: Preliminary patch to support prompt interpolation.
Jul 28 2018, 9:50 PM