https://github.com/AaronBallman closed
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jimmy-zx wrote:
Ping. Can one of you press the merge button?
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
linux4life798 wrote:
> @linux4life798 Can you review the last update?
This change LGTM (I don't actually have reviewer rights).
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.
linux4life798 wrote:
> Added flush before calling `writeMainFileToStdOut`.
I think that's reasonable.
> Personally I didn't find this function particularly useful: I was hoping to
> perform rewriting without creating new files, but seems like capturing stdout
> from a shared library requires
Endilll wrote:
@linux4life798 Can you review the last update?
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jimmy-zx wrote:
Added flush before calling `writeMainFileToStdOut`.
Personally I didn't find this function particularly useful: I was hoping to
perform rewriting without creating new files, but seems like capturing stdout
from a shared library requires some complex operation (`dup2` stuff).
`
https://github.com/jimmy-zx updated
https://github.com/llvm/llvm-project/pull/77269
>From a5379ca876d9531d48b37b9ad9c864db98c7dcd6 Mon Sep 17 00:00:00 2001
From: Jimmy Z <51149050+jimmy...@users.noreply.github.com>
Date: Mon, 8 Jan 2024 04:36:27 +
Subject: [PATCH 1/5] [libclang/python] Expos
Endilll wrote:
@linux4life798 Do you mind filing issues for the points you highlighted? Feel
free to skip this step if you (or someone else) plan to submit PRs that address
them.
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing
linux4life798 wrote:
I don't think this needs to block this change, but another concern I have about
this API is that it is writing to stdout from C++ intermixed with Python
writing to its own buffered stdout. This means that the user may see
inconsistent output. This is somewhat a deficiency
linux4life798 wrote:
> @linux4life798 Nice catch! I see that so far only CompilationDatabase and
> CompletionChunk expose functions in camelCase. It would be nice to change
> them, too, but those are decade-old APIs that I think we promise stability
> for, so it might not be possible.
Yep. Of
Endilll wrote:
@linux4life798 Nice catch! I see that so far only `CompilationDatabase` and
`CompletionChunk` expose functions in camelCase. It would be nice to change
them, too, but those are decade-old APIs that _I think_ we promise stability
for, so it might not be possible.
https://github.
linux4life798 wrote:
Thanks!
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jimmy-zx wrote:
@linux4life798 Thanks for pointing out! I've updated the naming to conform
snake_case.
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
https://github.com/jimmy-zx updated
https://github.com/llvm/llvm-project/pull/77269
>From a5379ca876d9531d48b37b9ad9c864db98c7dcd6 Mon Sep 17 00:00:00 2001
From: Jimmy Z <51149050+jimmy...@users.noreply.github.com>
Date: Mon, 8 Jan 2024 04:36:27 +
Subject: [PATCH 1/4] [libclang/python] Expos
linux4life798 wrote:
Hi all, sorry for jumping in late. Minor nit here.
It looks like this change is introducing camelCase methods, but the majority of
the existing Python file uses snake_case, even though the underlying libclang
interface is camelCase. For consistency, can we make these inter
jimmy-zx wrote:
Just updated the description a bit and I believe it's all set for merging.
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
https://github.com/jimmy-zx edited
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Endilll approved this pull request.
LGTM
PR description is going to become a commit message after merging. Let me know
when it's ready.
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.l
jimmy-zx wrote:
Both tests are just invoking the functionalities of the rewriter on a sample
code. I think additional tests for libclang does not provide extra coverage (as
it is just the front-end). Thus I mirrored the tests back to the Python binding
in 7a4ebf9c8c9ffa29d7f7f863d9a19bfe534323
https://github.com/Endilll edited
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Endilll wrote:
> I have reviewed the tests for libclang and it appears that there are already
> tests for the rewriter, which are more extensive than the ones I wrote.
> Therefore, I have decided to mirror the tests from libclang in the Python
> binding. Please let me know if this approach is
jimmy-zx wrote:
> Looks good overall. It's good that you wrote your own tests, but it would
> also be nice to mirror tests in `clang/unittests/libclang/LibclangTest.cpp`
> which test the same API. This way we can identify issues in binding layer
> itself (when C++ tests pass, but Python tests
https://github.com/jimmy-zx updated
https://github.com/llvm/llvm-project/pull/77269
>From a5379ca876d9531d48b37b9ad9c864db98c7dcd6 Mon Sep 17 00:00:00 2001
From: Jimmy Z <51149050+jimmy...@users.noreply.github.com>
Date: Mon, 8 Jan 2024 04:36:27 +
Subject: [PATCH 1/3] [libclang/python] Expos
https://github.com/Endilll commented:
Looks good overall.
It's good that you wrote your own tests, but it would also be nice to mirror
tests in `clang/unittests/libclang/LibclangTest.cpp` which test the same API.
This way we can identify issues in binding layer itself (when C++ tests pass,
but
https://github.com/jimmy-zx updated
https://github.com/llvm/llvm-project/pull/77269
>From a5379ca876d9531d48b37b9ad9c864db98c7dcd6 Mon Sep 17 00:00:00 2001
From: Jimmy Z <51149050+jimmy...@users.noreply.github.com>
Date: Mon, 8 Jan 2024 04:36:27 +
Subject: [PATCH 1/2] [libclang/python] Expos
https://github.com/AaronBallman commented:
The changes look reasonable to me as far as they go, but I don't have much
expertise in Python. I added @Endilll to hopefully help give it a second set of
eyes.
You should add a release note (clang/docs/ReleaseNotes.rst) about the new
bindings so oth
jimmy-zx wrote:
Pinging @AaronBallman and @jyknight for review. (sorry if I chose the wrong
people)
https://github.com/llvm/llvm-project/pull/77269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Jimmy Z (jimmy-zx)
Changes
Exposes `CXRewriter` API to the python binding as in
https://github.com/llvm/llvm-project/commit/69e5abb57b70570cf04671a93246e5e624023650.
---
Full diff: https://github.com/llvm/llvm-project/pull/77269.diff
2
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you, it i
https://github.com/jimmy-zx created
https://github.com/llvm/llvm-project/pull/77269
Exposes `CXRewriter` API to the python binding as in
https://github.com/llvm/llvm-project/commit/69e5abb57b70570cf04671a93246e5e624023650.
>From a5379ca876d9531d48b37b9ad9c864db98c7dcd6 Mon Sep 17 00:00:00 2001
30 matches
Mail list logo
