Re: [lldb-dev] [llvm-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Martin Storsjö via lldb-dev]

Hi Christian,

On Tue, 4 May 2021, Christian Kühnel wrote:


Having your own, custom Herald rules is always superior to general rules for
a project. They are naturally targeted towards your use cases. However I
wanted to offer a proper email integration for all users without having to
write their own rules. So the idea was to offer a "similar enough"
alternative for the XXX-commits mailing lists. 

I just checked your rules [1] and you add yourself to the list of
subscribers for certain revisions. For these notifications you should be on
the "TO" section of the email, right?


No; for revisions where I'm only listed as a subscriber, I'm in CC, for 
revisions where I'm reviewer or author, I'm in the TO field.



 The emails going through the project [1] are sent as CC to me. There is a
ton of header attributes that could be used for filtering:

  X-Phabricator-Cc:  

  X-Herald-Rules: <74>, <368>, <665>, <667>, <671>, 700>, <576>,
  <615>, <770> 

  X-Phabricator-Stamps: actor(@bruno) application(Differential)
  author(@bruno) herald(H74) herald(H368) herald(H576)
  herald(H615) herald(H665) herald(H667) herald(H671) herald(H700)
  herald(H770) monogram(D99434) object-type(DREV)
  phid(PHID-DREV-6ivftbt7xso57bvmy2br) reviewer(@aralisza)
  reviewer(@delcypher) reviewer(@dvyukov) reviewer(@kubamracek)
  reviewer(@vitalybuka) reviewer(@yln)
  revision-status(needs-review) subscriber(@hoy) subscriber(@jfb)
  subscriber(@kubamracek) subscriber(@llvm-commits)
  subscriber(@lxfind) subscriber(@modimo) subscriber(@rjmccall)
  subscriber(@t.p.northover) subscriber(@wenlei) tag(#llvm)
  via(web)


Do you think this is good enough for filtering?


Hmm, maybe. The issue is pretty much the reverse - if I'm listed as 
explicit subscriber but also subscribe to the "project", those mails would 
still have the same tags.


So is there a way to distinguish mails where I'm an explicit tagged 
subscriber (and thus in CC, also e.g. for reviews where I've taken part in 
discussion) and I'm also getting them via the project subscription (so the 
mail does have all the tags for subscription delivery)? Or would I be 
getting two mails for that situation, once for the project subscription 
and once for personal subscription to the individual review?


// Martin
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Aaron Ballman via lldb-dev]

On Mon, May 3, 2021 at 1:24 PM Krzysztof Parzyszek via cfe-dev
 wrote:
>
> Statement:
>
> Our current code review policy states[1]:
>
> “Code reviews are conducted, in order of preference, on our web-based 
> code-review tool (see Code Reviews with Phabricator), by email on the 
> relevant project’s commit mailing list, on the project’s development list, or 
> on the bug tracker.”
>
> This proposal is to limit code reviews only to Phabricator.  This would apply 
> to all projects in the LLVM monorepo.  With the change in effect, the amended 
> policy would read:
>
> “Code reviews are conducted on our web-based code-review tool (see Code 
> Reviews with Phabricator).”

Personally, I am in favor of this policy for initiating code reviews,
but am opposed to it for post-commit feedback. The problem, as I see
it, is that not every change *gets* code review via Phab and the email
lists are the only place on which to provide that post-commit
feedback. This largely comes up in two ways: NFC changes and changes
made by code owners in the area of the compiler which they own. We
still need *some* mechanism by which to provide them post-commit
feedback. Currently, the way we provide that is frequently via an
email reply to the commit message on the *-commits list so that the
issue can be seen by both the patch author and the community at large.

> Current situation:
>
> In a recent llvm-dev thread[2], Christian Kühnel pointed out that pre-commit 
> code reviews rarely originate via an email (most are started on Phabricator), 
> although, as others pointed out, email responses to an ongoing review are not 
> uncommon.  (That thread also contains examples of mishaps related to the 
> email-Phabricator interactions, or email handling itself.)
> I don’t have specific information about post-commit reviews.  It seems like 
> the most common form is an email reply to the auto-generated commit message, 
> although (in my personal experience), “raising a concern” in the commit on 
> Phabricator or commenting in the pre-commit review is usually sufficient to 
> get author’s attention.
> We have Phabricator patches that automatically apply email comments to the 
> Phabricator reviews, although reportedly this functionality is not fully 
> reliable[3,4].  This can cause review comments to be lost in the email 
> traffic.
>
>
>
> Benefits:
>
> Single way of doing code reviews: code reviews are a key part of the 
> development process, and having one way of performing them would make the 
> process clearer and unambiguous.
> Review authors and reviewers would only need to monitor one source of 
> comments without the fear that a review comment may end up overlooked.
> Local Phabricator extensions would no longer be needed.
>
>
>
> Concerns:
>
> For post-commit reviews, the commenter would need to find either the original 
> review, or the Phabricator commit (e.g. 
> https://reviews.llvm.org/rG06234f758e19).  Those are communicated (perhaps 
> ironically) via email, which implies that those automatic emails should 
> remain in place.

The Phab commit message does not have any subscribers though, and so
my understanding is that comments on that Phab interface are not
communicated out to the community as a whole, which means the
community may miss out on important post-commit-review information
like general awareness of the problem, workarounds people can use
until the author corrects something, alternative ideas on how to fix
the issue, etc. Or do I misunderstand the way Phab works in this
workflow?

Also, "the commenter would need to find the Phabricator commit"
concerns me -- adding extra burden on the people providing feedback
means that *some* amount of those people will struggle enough to
simply not provide that feedback. Responding to an email is about as
low as I think we can set that bar, so the current approach has better
ergonomics for giving feedback. When I look at an existing NFC commit
email 
(https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20210503/368413.html),
I don't see any direct link back to the Phabricator commit, so I have
to know to get the hash and try using that with an
https://reviews.llvm.org/rG in front of it. None of the existing links
in the email get me to where I'd need to go to add my review feedback,
but hitting the Reply button in my mail client will work. Adding a
third link and telling people "click on the link in the email" means
they've got a 50/50 shot of getting the right link because one of the
links goes to GitHub where you can also add comments.

> The current policy has been in place for a long time and it’s expected that 
> some people will continue using email for reviews out of habit or due to lack 
> of awareness of the policy change.
> Because of the larger variety, email clients may offer better accessibility 
> options than web browsers.
>
>
>
> Potential future direction:
>
> This section presents a potential future evolution of the review process.  
> Christian has condu

Re: [lldb-dev] [llvm-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Krzysztof Parzyszek via lldb-dev]

I'm opposed to separating the pre- and post-commit reviews.  One of the goals 
of this proposal is to have the entire review history in one place, and using a 
combination of email and Phabricator would prevent that.  If I want to find out 
why a commit has been reverted, I have to search the post-commit emails to see 
what happened.  I guess one could argue that pre- and post-commit reviews could 
happen on different pages (Dxxx vs rGxxx), but, in my view, that is still 
better than emails.  The two concerns I have about post-commit reviews on 
Phabricator are that

  1.  People will keep doing it via email, because there is no mechanism (short 
of making the -commits lists read-only) that would actively inform them about 
the change in policy.
  2.  It takes a bit of manual work to get to the rG page for the commit, and 
if the commit message doesn't include the link to the pre-commit review, the 
corresponding D page may be hard to locate.

Regarding point (2), I'm hoping that something can be done in Phabricator to 
make the post-commit reviews easier: it may be as simple as sending the commit 
message to a list of subscribers.


--
Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development

From: Philip Reames 
Sent: Monday, May 3, 2021 5:08 PM
To: Krzysztof Parzyszek ; llvm-dev 

Cc: clangd-...@lists.llvm.org; openmp-...@lists.llvm.org; 
lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; libcxx-...@lists.llvm.org; 
flang-...@lists.llvm.org; parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [llvm-dev] [RFC] Deprecate email code reviews in favor of 
Phabricator


In my view, this email is really too different topics.  Given that, my response 
is split into two parts.

First, should we make phabricator our default for code review?  I am not 
opposed to this.  I don't particular support it either, but I would not spend 
time arguing against it.  I would suggest that we re-frame the proposal to 
distinguish precommit and post commit review - with only the former moving to 
phabricator.  I have not seen post-commit done successfully on phabricator to 
date in any wide spread manner.

Second, should we consider retiring llvm-commits and the other mailing lists?  
My gut response is a flat out NO  What we have works.  I am highly 
reluctant to run the risk of breaking our existing processes - which for all 
their problems mostly work - for the, to me, seemingly very minimal value 
obtained by moving away from email discussion.  Post commit review in email 
works.  I strongly suspect that if you try to change that, you will either 
simply drive out post commit review discussion (bad idea!) or discussions will 
move to private email exchanges (bad idea!).  I'm open to being convinced here, 
but the burden of proof is high.  The risk we'd be talking about with such a 
transition is immense.

Philip
On 5/3/2021 10:24 AM, Krzysztof Parzyszek via llvm-dev wrote:



Statement:

Our current code review policy states[1]:

"Code reviews are conducted, in order of preference, on our web-based 
code-review tool (see Code Reviews with Phabricator), by email on the relevant 
project's commit mailing list, on the project's development list, or on the bug 
tracker."

This proposal is to limit code reviews only to Phabricator.  This would apply 
to all projects in the LLVM monorepo.  With the change in effect, the amended 
policy would read:

"Code reviews are conducted on our web-based code-review tool (see Code Reviews 
with Phabricator)."



Current situation:

  1.  In a recent llvm-dev thread[2], Christian Kühnel pointed out that 
pre-commit code reviews rarely originate via an email (most are started on 
Phabricator), although, as others pointed out, email responses to an ongoing 
review are not uncommon.  (That thread also contains examples of mishaps 
related to the email-Phabricator interactions, or email handling itself.)
  2.  I don't have specific information about post-commit reviews.  It seems 
like the most common form is an email reply to the auto-generated commit 
message, although (in my personal experience), "raising a concern" in the 
commit on Phabricator or commenting in the pre-commit review is usually 
sufficient to get author's attention.
  3.  We have Phabricator patches that automatically apply email comments to 
the Phabricator reviews, although reportedly this functionality is not fully 
reliable[3,4].  This can cause review comments to be lost in the email traffic.



Benefits:

  1.  Single way of doing code reviews: code reviews are a key part of the 
development process, and having one way of performing them would make the 
process clearer and unambiguous.
  2.  Review authors and reviewers would only need to monitor one source of 
comments without the fear that a review comment may end up overlooked.
  3.  Local Phabricator extensions would no longer be needed.



Concerns:

  1.  For post-commit reviews, the commenter would need to find either the 
or

Re: [lldb-dev] [cfe-dev] [llvm-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[via lldb-dev]

I’m not hearing any particular objection to moving to Phabricator-only for 
reviews.  The trend has been in that direction for years anyway.  The barriers 
for one-time contributors are (a) registration, and (b) unfamiliarity with a 
user-hostile web UI.  Regarding registration, it’s either subscribing to 
llvm-commits or to Phab; and IIUC, Phab understands github IDs, which makes 
that barrier pretty low.  Regarding the web UI, we have (or did have, 
admittedly I haven’t looked lately) step-by-step instructions in the docs for 
how to navigate Phab successfully, which mitigates (b) as much as we can.

I am in the camp of, why mess with llvm-commits?  The commit emails that Phab 
sends out already have the commit URL in them right at the top, which looks 
like it goes to a page where comments can be added (I haven’t actually tried 
adding a comment).  If those posted comments go to llvm-commits and the author, 
then I think the only necessary step to turn off email post-commit review is to 
make llvm-commits read-only except for Phabricator itself.
We don’t need the ability to have email replies on llvm-commits be recorded in 
Phab, if we can almost as easily make comments on Phab that go to llvm-commits. 
 Possibly I am overlooking some problem that abandoning llvm-commits entirely 
is going to solve?
--paulr

From: cfe-dev  On Behalf Of Christian Kühnel 
via cfe-dev
Sent: Tuesday, May 4, 2021 9:15 AM
To: Aaron Ballman 
Cc: llvm-dev ; clangd-...@lists.llvm.org; 
openmp-...@lists.llvm.org; lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; 
libcxx-...@lists.llvm.org; flang-...@lists.llvm.org; 
parallel_libs-...@lists.llvm.org
Subject: Re: [cfe-dev] [llvm-dev] [RFC] Deprecate email code reviews in favor 
of Phabricator

Hi Aaron,

> “Code reviews are conducted on our web-based code-review tool (see Code 
> Reviews with Phabricator).”

Personally, I am in favor of this policy for initiating code reviews,
but am opposed to it for post-commit feedback. The problem, as I see
it, is that not every change *gets* code review via Phab and the email
lists are the only place on which to provide that post-commit
feedback.

You can set up notifications on commits in Phabricator. Phabricator adds the 
user "llvm-commits" as subscriber to certain reviews: 
https://reviews.llvm.org/H615
We could do the same thing for commits...

So you can reply to any commit via the web UI (or email notification) and the 
author gets notified as well. One example that worked for me:
 
https://reviews.llvm.org/rG7f9717b922d421c30f889034488563c67076aca1

The Phab commit message does not have any subscribers though, and so
my understanding is that comments on that Phab interface are not
communicated out to the community as a whole, which means the
community may miss out on important post-commit-review information
like general awareness of the problem, workarounds people can use
until the author corrects something, alternative ideas on how to fix
the issue, etc. Or do I misunderstand the way Phab works in this
workflow?

We can add automatically subscribers to commits as well if we want to.
One random example where a subscriber was added to a commit via a Herald rule:
https://reviews.llvm.org/rG93537fabcee8fcfa3316d7abd5e935f7fe9b468f

Also, "the commenter would need to find the Phabricator commit"
concerns me -- adding extra burden on the people providing feedback
means that *some* amount of those people will struggle enough to
simply not provide that feedback. Responding to an email is about as
low as I think we can set that bar, so the current approach has better
ergonomics for giving feedback.

If we set up the notifications via Phabricator, you can reply via email. These 
contain a link at the bottom that will take you directly to the commit page in 
Phabricator. Not sure why we don't have these on the mailing list...

Tangential complaint -- our use of Herald causes some pain for me as a
list moderator because we've reached the point where Herald
automatically adds so many subscribers to a review that it gets marked
as spam for every email that is generated for the review.

So this would be a reason to replace the XXX-commits mailing lists with 
something else...

Given how often Phabricator goes down (which is not super often, but
often enough that people know it happens), I am deeply uncomfortable
with the idea of shutting down the current *-commits mailing lists
because of the chance for data loss. Personally, I think the *-com

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Krzysztof Parzyszek via lldb-dev]

You're right that doing post-commit reviews on Phabricator is not 
seamless---the rG link is not included anywhere.  Hopefully that could be fixed 
with some Phabricator configuration tweaks, like sending the commit email to 
the -commits list.

I'm not sure if there is a general fix for the spam issue.  I have had this 
problem as well with the existing lists, so it's not limited to Phabricator.  
In our spam filter I have whitelisted enough emails to avoid the unnecessary 
filtering almost completely, but it is an issue nevertheless.


-- 
Krzysztof Parzyszek  kparz...@quicinc.com   AI tools development

-Original Message-
From: Aaron Ballman  
Sent: Tuesday, May 4, 2021 6:24 AM
To: Krzysztof Parzyszek 
Cc: llvm-dev ; clangd-...@lists.llvm.org; 
openmp-...@lists.llvm.org; lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; 
libcxx-...@lists.llvm.org; flang-...@lists.llvm.org; 
parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [cfe-dev] [RFC] Deprecate email code reviews in favor of 
Phabricator

On Mon, May 3, 2021 at 1:24 PM Krzysztof Parzyszek via cfe-dev 
 wrote:
>
> Statement:
>
> Our current code review policy states[1]:
>
> “Code reviews are conducted, in order of preference, on our web-based 
> code-review tool (see Code Reviews with Phabricator), by email on the 
> relevant project’s commit mailing list, on the project’s development list, or 
> on the bug tracker.”
>
> This proposal is to limit code reviews only to Phabricator.  This would apply 
> to all projects in the LLVM monorepo.  With the change in effect, the amended 
> policy would read:
>
> “Code reviews are conducted on our web-based code-review tool (see Code 
> Reviews with Phabricator).”

Personally, I am in favor of this policy for initiating code reviews, but am 
opposed to it for post-commit feedback. The problem, as I see it, is that not 
every change *gets* code review via Phab and the email lists are the only place 
on which to provide that post-commit feedback. This largely comes up in two 
ways: NFC changes and changes made by code owners in the area of the compiler 
which they own. We still need *some* mechanism by which to provide them 
post-commit feedback. Currently, the way we provide that is frequently via an 
email reply to the commit message on the *-commits list so that the issue can 
be seen by both the patch author and the community at large.

> Current situation:
>
> In a recent llvm-dev thread[2], Christian Kühnel pointed out that 
> pre-commit code reviews rarely originate via an email (most are started on 
> Phabricator), although, as others pointed out, email responses to an ongoing 
> review are not uncommon.  (That thread also contains examples of mishaps 
> related to the email-Phabricator interactions, or email handling itself.) I 
> don’t have specific information about post-commit reviews.  It seems like the 
> most common form is an email reply to the auto-generated commit message, 
> although (in my personal experience), “raising a concern” in the commit on 
> Phabricator or commenting in the pre-commit review is usually sufficient to 
> get author’s attention.
> We have Phabricator patches that automatically apply email comments to the 
> Phabricator reviews, although reportedly this functionality is not fully 
> reliable[3,4].  This can cause review comments to be lost in the email 
> traffic.
>
>
>
> Benefits:
>
> Single way of doing code reviews: code reviews are a key part of the 
> development process, and having one way of performing them would make the 
> process clearer and unambiguous.
> Review authors and reviewers would only need to monitor one source of 
> comments without the fear that a review comment may end up overlooked.
> Local Phabricator extensions would no longer be needed.
>
>
>
> Concerns:
>
> For post-commit reviews, the commenter would need to find either the original 
> review, or the Phabricator commit (e.g. 
> https://reviews.llvm.org/rG06234f758e19).  Those are communicated (perhaps 
> ironically) via email, which implies that those automatic emails should 
> remain in place.

The Phab commit message does not have any subscribers though, and so my 
understanding is that comments on that Phab interface are not communicated out 
to the community as a whole, which means the community may miss out on 
important post-commit-review information like general awareness of the problem, 
workarounds people can use until the author corrects something, alternative 
ideas on how to fix the issue, etc. Or do I misunderstand the way Phab works in 
this workflow?

Also, "the commenter would need to find the Phabricator commit"
concerns me -- adding extra burden on the people providing feedback means that 
*some* amount of those people will struggle enough to simply not provide that 
feedback. Responding to an email is about as low as I think we can set that 
bar, so the current approach has better ergonomics for giving feedback. When I 
look at an existing NFC commit email 
(https:

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[via lldb-dev]

> You're right that doing post-commit reviews on Phabricator is not
> seamless---the rG link is not included anywhere.  Hopefully that could be
> fixed with some Phabricator configuration tweaks, like sending the commit
> email to the -commits list.

The commit email has a URL: link, e.g. this recent one (which has no
Dn review):

URL: 
https://github.com/llvm/llvm-project/commit/b04148f77713c92ee57b33b7b858ad18ce8d78e3

Does that take you to a different place than the rG link would?
Seems like they ought to go to the same place.
--paulr

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Aaron Ballman via lldb-dev]

On Tue, May 4, 2021 at 9:56 AM  wrote:
>
> > You're right that doing post-commit reviews on Phabricator is not
> > seamless---the rG link is not included anywhere.  Hopefully that could be
> > fixed with some Phabricator configuration tweaks, like sending the commit
> > email to the -commits list.
>
> The commit email has a URL: link, e.g. this recent one (which has no
> Dn review):
>
> URL: 
> https://github.com/llvm/llvm-project/commit/b04148f77713c92ee57b33b7b858ad18ce8d78e3
>
> Does that take you to a different place than the rG link would?
> Seems like they ought to go to the same place.

That sends you to the GitHub commit for the change. The corresponding
rG link would be:

https://reviews.llvm.org/rGb04148f77713c92ee57b33b7b858ad18ce8d78e3

~Aaron
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[James Henderson via lldb-dev]

The github URL is not the "rG" one being referred to here. If you wanted to
do a post-commit review on the commit, you'd go to
https://reviews.llvm.org/rGb04148f77713c92ee57b33b7b858ad18ce8d78e3, which
is a part of Phabricator. You can comment on this page, much in the same
way as you would a Dxx review.

On Tue, 4 May 2021 at 14:56, via lldb-dev  wrote:

> > You're right that doing post-commit reviews on Phabricator is not
> > seamless---the rG link is not included anywhere.  Hopefully that could be
> > fixed with some Phabricator configuration tweaks, like sending the commit
> > email to the -commits list.
>
> The commit email has a URL: link, e.g. this recent one (which has no
> Dn review):
>
> URL:
> https://github.com/llvm/llvm-project/commit/b04148f77713c92ee57b33b7b858ad18ce8d78e3
>
> Does that take you to a different place than the rG link would?
> Seems like they ought to go to the same place.
> --paulr
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[via lldb-dev]

> > The commit email has a URL: link, e.g. this recent one (which has no
> > Dn review):
> >
> > URL: 
> > https://github.com/llvm/llvm-project/commit/b04148f77713c92ee57b33b7b858ad18ce8d78e3
> >
> > Does that take you to a different place than the rG link would?
> > Seems like they ought to go to the same place.
> 
> That sends you to the GitHub commit for the change

Doh!  Of course the commit message is from github not Phab, and
github doesn't know the Phab URL.  We'd have to have Phab send a
commit email instead, and insert the rG URL.  That's a bit more
complicated, yep.
--paulr
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] small Editline wrapper cleanup req for feedback

[Greg Clayton via lldb-dev]

As long as the solution matches "EditLine *" (C struct type from edit line 
library) to back to the C++ instance of "Editline" (lower case ell in "line" 
from LLDB). It should be easy to do with a template.

I am fine with any new solution that makes it easier to add new commands. I 
would rather have a templated function argument over a template argument if 
possible. I am thinking of something like:

createEditLineCommandDescriptor(“Command1", “Command1 help", &Foo::Foo1);
createEditLineCommandDescriptor(“Command2", “Command2 help", &Foo::Bar);

as I find it more readable.

Greg

> On Apr 30, 2021, at 9:35 PM, Neal Sidhwaney via lldb-dev 
>  wrote:
> 
> Some comments in 
> https://reviews.llvm.org/rGfd89af6880f33ead708abe2f7d88ecb687d4e0d2 
>  
> prompted me to look more into potential simplifications of our EditLine 
> wrapper and I wanted to run this by anyone who is interested before making 
> the changes.
> 
> Right now we set a bunch of callbacks in libedit that are captureless lambdas 
> implicitly converted to C function pointers.  The lambdas look up an instance 
> of our Editline class and invoke member functions.  The boilerplate that 
> could be generated by templates is something like the following:
> 
> class Foo { // Imagine this is our Editline class that wraps libedit
> public:
>   unsigned char Foo1(int ch) {  // These are member functions invoked by 
> lambdas we pass to libedit
> return 'a';
>   }
>   unsigned char Bar(int ch) {
> return 'b';
>   }
>   unsigned char Baz(int ch) {
> return 'c';
>   }
> };
> 
> typedef unsigned char (*elFnPtr)(EditLine*, int);  // Signature of callbacks 
> libedit takes (note Edit__L__ine is libedit, and Edit__l__ine is our wrapper)
> typedef unsigned char (Foo::*FooMemPtr)(int ch);   // Signature of member 
> functions invoked
> 
> template
> tuple createEditLineCommandDescriptor(const 
> char* command, const char* helpText) {
>   return make_tuple(command, helpText, [] (EditLine*, int ch) {
> cout << ch;
> Foo foo;
> ((&foo)->*callback)('a');
> return (unsigned char) ch;
>   });
> }
> 
> auto editlineCommands = {
>   createEditLineCommandDescriptor<&Foo::Foo1>(“Command1", “Command1 help"),
>   createEditLineCommandDescriptor<&Foo::Bar>(“Command2", “Command2 help")
> };
> 
> for (auto editlineCommand : editLineCommands) {
>  // call into libedit to add editlineCommand, e.g.:
>  el_set(EL_ADDFN, editlineCommand.get<0>(), editLineCommand.get<1>(), 
> editLineCommand.get<2>());
> }
> 
> The pointer to member function is a type parameter because otherwise the 
> compiler complains about the lambda needing to capture it, in which case we 
> could not pass it to libedit.
> 
> I also plan to look into the wchar_t/char preprocessor logic that the 
> original comment brought up but then I got distracted by shiny template stuff 
> ;-)
> 
> Thanks!
> 
> Neal
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev