Page MenuHomePhabricator

seiya (Seiya Nuta)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 30 2019, 8:08 PM (28 w, 1 d)

Recent Activity

Wed, Oct 9

seiya added inline comments to D66282: [llvm-objcopy][MachO] Implement --remove-section.
Wed, Oct 9, 12:20 AM · Restricted Project

Mon, Oct 7

seiya added a comment to D65541: [llvm-objcopy][MachO] Implement --only-section.

Friendly ping: I'd like someone who is familiar with Mach-O to review this patch.

Mon, Oct 7, 8:31 PM · Restricted Project
seiya updated the diff for D66282: [llvm-objcopy][MachO] Implement --remove-section.

Rebased.

Mon, Oct 7, 8:30 PM · Restricted Project
seiya updated the diff for D66281: [llvm-objcopy][MachO] Implement --strip-all.

Rebased. No changes intended.

Mon, Oct 7, 8:27 PM · Restricted Project

Thu, Oct 3

seiya updated the diff for D66280: [llvm-objcopy][MachO] Support indirect symbol table.
  • Added a comment.
Thu, Oct 3, 5:20 PM · Restricted Project

Tue, Oct 1

seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Addressed a review comment.

Tue, Oct 1, 7:28 AM · Restricted Project

Mon, Sep 30

seiya added inline comments to D65541: [llvm-objcopy][MachO] Implement --only-section.
Mon, Sep 30, 8:18 PM · Restricted Project
seiya updated the diff for D66280: [llvm-objcopy][MachO] Support indirect symbol table.

clang-formatted

Mon, Sep 30, 8:14 PM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Addressed review comments.

Mon, Sep 30, 8:13 PM · Restricted Project

Sun, Sep 29

seiya updated the diff for D66280: [llvm-objcopy][MachO] Support indirect symbol table.

Addressed a review comment.

Sun, Sep 29, 8:13 PM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Add MachOConfig.cpp to isolate command-line validation logic from MachOObjCopy.cpp.

Sun, Sep 29, 8:11 PM · Restricted Project

Tue, Sep 24

seiya committed rGc83eefcfda7c: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC. (authored by seiya).
[llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC.
Tue, Sep 24, 2:41 AM
seiya committed rL372712: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
[llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC.
Tue, Sep 24, 2:37 AM
seiya closed D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Tue, Sep 24, 2:37 AM · Restricted Project

Fri, Sep 20

seiya added a comment to D67689: [llvm-objcopy] Add support for --gap-fill and --pad-to options.

Commented regarding tests and nits. Could you update docs/CommandGuide/llvm-objcopy.rst as well?

Fri, Sep 20, 12:04 AM · Restricted Project

Thu, Sep 19

seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
  • Addressed a review comment.
Thu, Sep 19, 11:23 PM · Restricted Project

Wed, Sep 18

seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
  • Added a comment for parseELFConfig.
Wed, Sep 18, 2:54 AM · Restricted Project
seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Wed, Sep 18, 2:49 AM · Restricted Project

Tue, Sep 17

seiya added a comment to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

Thanks you for feedbacks. Putting it all together, the @alexshap's suggestion (D67139#inline-607235) looks desirable to me because:

Tue, Sep 17, 2:49 AM · Restricted Project

Sun, Sep 15

seiya added a comment to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

What about having ELFcopyConfig inherit from CopyConfig?

How about this design? I believe moving the CopyConfig is not a big cost.

Sun, Sep 15, 6:07 PM · Restricted Project

Sep 13 2019

seiya added inline comments to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Sep 13 2019, 4:04 AM · Restricted Project
seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

Pass only ELFCopyConfig instead of passing both CopyConfig and ELFCopyConfig.

Sep 13 2019, 2:24 AM · Restricted Project

Sep 12 2019

seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

Adopt @jhenderson's idea D67139#1657802.

Sep 12 2019, 3:15 AM · Restricted Project

Sep 10 2019

seiya added inline comments to D67060: [Support] Support restoring colors in WithColor.
Sep 10 2019, 2:28 AM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.
  • Removed an unrelated change from the diff.
Sep 10 2019, 2:28 AM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

This patch looks good, but I'd add a test for the new code. Is there any place in LLVM where the current color is saved and restored? If there's such code, you can rewrite the code to see if your code would work as expected.

Sep 10 2019, 2:24 AM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.

Addressed review comments

Sep 10 2019, 2:13 AM · Restricted Project

Sep 9 2019

seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.

Addressed review comments.

Sep 9 2019, 7:11 PM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.
  • Abandon WithColorContext: save the previous color settings in WithColor instead.
  • Update the description.
Sep 9 2019, 7:24 AM · Restricted Project

Sep 5 2019

seiya added a comment to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

I've got another suggestion: have separate COFFConfig/ELFConfig etc classes that only contain the interpreted versions and lazily construct them prior to calling the corresponding code path. CopyConfig would contain the raw unparsed strings (perhaps rename them to "RawSymbolsToAdd" etc). Perhaps a higher-level Config class could exist just to wrap them and pass them to executeObjcopyOnBinary etc.

Rough outline:

CopyConfig.h:

class CopyConfig {
  std::vector<StringRef> RawSymbolsToAdd;
  ...
};

class ELFConfig {
  std::vector<NewSymbolInfo> SymbolsToAdd;
  ...
};

// etc for COFF, Mach-O...

llvm-objcopy.cpp:

struct Configurations {
  CopyConfig Common;
  Optional<ELFConfig> ELF;
  ...
}

...

static Error executeObjcopyOnBinary(Configurations &Cfgs, ...) {
  if (/*isElf*/...) {
    if (!Cfgs.ELF)
     Cfgs.ELF.emplace(Cfgs.Common);
    return elf::executeObjcopyOnBinary(Cfgs.Common, Cfgs.ELF, ...)
  }
  ...
}

LGTM. I'll implement the idea.

Sep 5 2019, 8:03 PM · Restricted Project

Sep 4 2019

seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

I don't think there's a way to get the current color from the terminal, so you should modify changeColor so that the function remembers the color given to the function.

Sep 4 2019, 2:37 AM · Restricted Project

Sep 3 2019

seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

This looks better, but I don't think we need the concept of context. If a function takes a context object, it is usually expected that you can interlace function calls with different context objects freely. Say, you have context object A and B, then you literally have two contexts, and they don't interfere with each other. However, that's not the case for the terminal color. The current color of text output on console is naturally and fundamentally a global state associated to each terminal, and therefore there is only one context. So, I don't think that allowing multiple contexts captures the real concept correctly.

I believe the following scheme models captures the reality better:

  1. Add a new method, getColor, to raw_fd_stream, which returns the current color set by changeColor before
  2. Make a change to WithColor so that it remembers the current color (obtained by calling getColor) before changing color
  3. In the destructor, WithColor restores the previous color
Sep 3 2019, 11:57 PM · Restricted Project
seiya created D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Sep 3 2019, 7:24 PM · Restricted Project
seiya added a comment to D66281: [llvm-objcopy][MachO] Implement --strip-all.

@seyia, sorry about the delays with code review, does this particular diff have any unreviewed dependencies ?

Sep 3 2019, 7:04 PM · Restricted Project
seiya added inline comments to D67060: [Support] Support restoring colors in WithColor.
Sep 3 2019, 6:52 PM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

Did you find any existing usage of WithColor that depends on the behavior of resetting the color? If not, you don't have to care too much about keeping the compatibility, as that is not really an intended feature as far as I understand. You probably can just change the behavior of WithColor so that it restores to the previous color instead of resseting.

Sep 3 2019, 6:34 PM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.

Moved NestableWithColor features into WithColor.

Sep 3 2019, 6:27 PM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

Instead of adding a new class, do you think you can change the behavior of WithColor so that it restores the original colors instead of resetting it?

This would be my preference too, if it's viable. I don't think WithColor has been around that long, so I suspect it may be straightforward to identify existing usage patterns, and I'd be surprised if any expect restoration to default.

Sep 3 2019, 12:55 AM · Restricted Project

Sep 2 2019

seiya added reviewers for D67060: [Support] Support restoring colors in WithColor: jhenderson, rupprecht, ruiu, MaskRay.

Added reviewers who may be familiar with this part.

Sep 2 2019, 12:48 AM · Restricted Project
seiya updated the summary of D67060: [Support] Support restoring colors in WithColor.
Sep 2 2019, 12:44 AM · Restricted Project
seiya updated the summary of D67060: [Support] Support restoring colors in WithColor.
Sep 2 2019, 12:44 AM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

This patch does not include tests but it will be used in D65191.

Sep 2 2019, 12:27 AM · Restricted Project
seiya created D67060: [Support] Support restoring colors in WithColor.
Sep 2 2019, 12:24 AM · Restricted Project

Aug 27 2019

seiya added a comment to D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.

Test cases for the new code?

I forgot to add NFC in the title. This patch does not introduce new features thus we don't need new tests for it I think.

Aug 27 2019, 8:38 PM · Restricted Project
seiya retitled D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing from [llvm-objcopy] Accept MachO formats in commad-line parsing. NFC. to [llvm-objcopy] Accept MachO formats in commad-line parsing.
Aug 27 2019, 8:38 PM · Restricted Project
seiya added a comment to D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.

Test cases for the new code?

Aug 27 2019, 8:33 PM · Restricted Project
seiya retitled D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing from [llvm-objcopy] Accept MachO formats in commad-line parsing to [llvm-objcopy] Accept MachO formats in commad-line parsing. NFC..
Aug 27 2019, 8:31 PM · Restricted Project

Aug 26 2019

seiya added inline comments to D65191: [llvm-objdump] Implement highlighting.
Aug 26 2019, 7:57 PM · Restricted Project
seiya updated the diff for D66409: [llvm-objcopy][MachO] Implement -Obinary.

Addressed a review comment.

Aug 26 2019, 7:57 PM · Restricted Project
seiya added inline comments to D66409: [llvm-objcopy][MachO] Implement -Obinary.
Aug 26 2019, 7:57 PM · Restricted Project
seiya updated the diff for D66409: [llvm-objcopy][MachO] Implement -Obinary.

Addressed review comments.

Aug 26 2019, 7:57 PM · Restricted Project
seiya added inline comments to D66409: [llvm-objcopy][MachO] Implement -Obinary.
Aug 26 2019, 7:57 PM · Restricted Project
seiya added inline comments to D66517: [llvm-objdump] - Remove one overload of reportError. NFCI..
Aug 26 2019, 3:14 AM · Restricted Project
seiya added inline comments to D66517: [llvm-objdump] - Remove one overload of reportError. NFCI..
Aug 26 2019, 2:33 AM · Restricted Project
seiya planned changes to D65541: [llvm-objcopy][MachO] Implement --only-section.

I'll submit another patch to refactor CopyConfig as described in https://reviews.llvm.org/D65541#inline-594247

Aug 26 2019, 1:36 AM · Restricted Project
seiya added inline comments to D66407: [llvm-objcopy][MachO] Implement -Ibinary.
Aug 26 2019, 1:34 AM · Restricted Project
seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.

Addressed review comments and added test for arm64.

Aug 26 2019, 1:27 AM · Restricted Project
seiya added inline comments to D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.
Aug 26 2019, 1:22 AM · Restricted Project
seiya updated the diff for D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.

Addressed review comments.

Aug 26 2019, 1:22 AM · Restricted Project

Aug 22 2019

seiya added inline comments to D65541: [llvm-objcopy][MachO] Implement --only-section.
Aug 22 2019, 10:51 PM · Restricted Project
seiya added inline comments to D65541: [llvm-objcopy][MachO] Implement --only-section.
Aug 22 2019, 10:20 PM · Restricted Project
seiya accepted D66432: [llvm-objcopy][NFC] Refactor symbol/section matching.
Aug 22 2019, 1:12 AM · Restricted Project

Aug 20 2019

seiya updated the diff for D66408: [llvm-objcopy][MachO] Implement --dump-section.

CommandGuide: Removed remove "the" before "<section>".

Aug 20 2019, 6:25 AM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.

CommandGuide: Removed remove "the" before "<section>".

Aug 20 2019, 6:24 AM · Restricted Project
seiya updated the diff for D66282: [llvm-objcopy][MachO] Implement --remove-section.

CommandGuide: Removed remove "the" before "<section>".

Aug 20 2019, 6:24 AM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Updated CommandGuide.

Aug 20 2019, 6:24 AM · Restricted Project
seiya added inline comments to D66408: [llvm-objcopy][MachO] Implement --dump-section.
Aug 20 2019, 6:23 AM · Restricted Project
seiya updated the diff for D66408: [llvm-objcopy][MachO] Implement --dump-section.

Addressed review comments.

Aug 20 2019, 6:23 AM · Restricted Project
seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.

Addressed review comments.

Aug 20 2019, 6:23 AM · Restricted Project
seiya updated the diff for D66408: [llvm-objcopy][MachO] Implement --dump-section.

Update CommandGuide.

Aug 20 2019, 6:10 AM · Restricted Project
seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.

Update CommandGuide.

Aug 20 2019, 6:08 AM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.

Update CommandGuide.

Aug 20 2019, 6:08 AM · Restricted Project
seiya updated the diff for D66282: [llvm-objcopy][MachO] Implement --remove-section.

Update CommandGuide.

Aug 20 2019, 6:08 AM · Restricted Project
seiya added inline comments to D65190: [X86] X86ATTInstPrinter: replace markup with startMarkup/endMarkup.
Aug 20 2019, 5:59 AM · Restricted Project
seiya updated the diff for D65190: [X86] X86ATTInstPrinter: replace markup with startMarkup/endMarkup.

Clang-formatted.

Aug 20 2019, 5:59 AM · Restricted Project
seiya updated the diff for D65191: [llvm-objdump] Implement highlighting.

clang-formatted

Aug 20 2019, 5:59 AM · Restricted Project
seiya added inline comments to D65191: [llvm-objdump] Implement highlighting.
Aug 20 2019, 5:59 AM · Restricted Project
seiya updated the diff for D65191: [llvm-objdump] Implement highlighting.
Aug 20 2019, 5:51 AM · Restricted Project
seiya committed rGb8dcc193890c: [yaml2obj/obj2yaml][MachO] Fix a test failure in big endian hosts (authored by seiya).
[yaml2obj/obj2yaml][MachO] Fix a test failure in big endian hosts
Aug 20 2019, 3:00 AM
seiya committed rL369360: [yaml2obj/obj2yaml][MachO] Fix a test failure in big endian hosts.
[yaml2obj/obj2yaml][MachO] Fix a test failure in big endian hosts
Aug 20 2019, 2:57 AM
seiya committed rG522377494b3d: [yaml2obj/obj2yaml][MachO] Allow setting custom section data (authored by seiya).
[yaml2obj/obj2yaml][MachO] Allow setting custom section data
Aug 20 2019, 1:49 AM
seiya committed rL369348: [yaml2obj/obj2yaml][MachO] Allow setting custom section data.
[yaml2obj/obj2yaml][MachO] Allow setting custom section data
Aug 20 2019, 1:48 AM
seiya closed D65799: [yaml2obj/obj2yaml][MachO] Allow setting custom section data.
Aug 20 2019, 1:48 AM · Restricted Project
seiya committed rG36848ff8dfb3: [llvm-objcopy][MachO] Fix method names. NFC. (authored by seiya).
[llvm-objcopy][MachO] Fix method names. NFC.
Aug 20 2019, 1:37 AM
seiya committed rL369346: [llvm-objcopy][MachO] Fix method names. NFC..
[llvm-objcopy][MachO] Fix method names. NFC.
Aug 20 2019, 1:36 AM
seiya closed D65540: [llvm-objcopy][MachO] Fix method names. NFC..
Aug 20 2019, 1:36 AM · Restricted Project

Aug 19 2019

seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.

Added a newline at the end of file.

Aug 19 2019, 3:27 PM · Restricted Project
seiya removed a child revision for D66283: [llvm-objcopy][MachO] Implement --add-section: D66407: [llvm-objcopy][MachO] Implement -Ibinary.
Aug 19 2019, 3:27 PM · Restricted Project
seiya added a child revision for D66283: [llvm-objcopy][MachO] Implement --add-section: D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.
Aug 19 2019, 3:27 PM · Restricted Project
seiya added a parent revision for D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing: D66283: [llvm-objcopy][MachO] Implement --add-section.
Aug 19 2019, 3:27 PM · Restricted Project
seiya edited parent revisions for D66407: [llvm-objcopy][MachO] Implement -Ibinary, added: 1; removed: 1.
Aug 19 2019, 3:27 PM · Restricted Project
seiya added a child revision for D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing: D66407: [llvm-objcopy][MachO] Implement -Ibinary.
Aug 19 2019, 3:27 PM · Restricted Project
seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.

Moved changes to CopyConfig to D66449.

Aug 19 2019, 3:27 PM · Restricted Project
Herald added a reviewer for D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing: rupprecht.
Aug 19 2019, 3:21 PM · Restricted Project
seiya updated the diff for D66409: [llvm-objcopy][MachO] Implement -Obinary.

Fixed review comments.

Aug 19 2019, 2:47 PM · Restricted Project
seiya updated the diff for D66408: [llvm-objcopy][MachO] Implement --dump-section.

Fixed review comments.

Aug 19 2019, 2:46 PM · Restricted Project
seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.
  • No such file or directory -> {{[Nn]}}o such file or directory
Aug 19 2019, 2:46 PM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.

Fixed a review comment.

Aug 19 2019, 2:46 PM · Restricted Project
seiya updated the diff for D66282: [llvm-objcopy][MachO] Implement --remove-section.

Address review comments.

Aug 19 2019, 2:46 PM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Fix a review comment.

Aug 19 2019, 2:43 PM · Restricted Project