Chris Lattner via llvm-dev
2019-Jul-09 05:03 UTC
[llvm-dev] RFC: changing variable naming rules in LLVM codebase
This looks really great to me Rui, and I’m also pleased to see the positive comments on the review thread. Thank you for driving this forward! -Chris> On Jul 4, 2019, at 9:50 PM, Rui Ueyama <ruiu at google.com> wrote: > > Hi all, > > I wrote a tool <https://reviews.llvm.org/D64123> to batch-rename variable names so that they are in camelCase, and I applied the tool to lld subdirectory. You can see my change at https://reviews.llvm.org/D64121 <https://reviews.llvm.org/D64121>. If you have any comments, please reply. > > If people are happy about this change, I can do the same thing for other directories including LLVM itself and Clang. > > On Mon, Jun 10, 2019 at 6:34 PM Rui Ueyama <ruiu at google.com <mailto:ruiu at google.com>> wrote: > On Mon, Jun 10, 2019 at 6:27 PM Michael Platings <Michael.Platings at arm.com <mailto:Michael.Platings at arm.com>> wrote: > Hi Rui, > > > > As per the provisional plan [1] we’re still at step 1: improving git blame. The status of this is that there are some fairly mature patches in the Git project’s queue [2], and I’m hopeful that it will be accepted in something close to its current form. > > > > But if you can get started on steps 2 & 3 i.e. making forks available with the possible changes applied then that would be great. My hope is that once everyone can see what the options really look like then it will be easier to reach consensus. > > > Sure, I'll try to do that. I'll probably start with finding identifiers and typenames that will conflict after the naming scheme change and rename them so that they won't conflict. The number of such symbols would hopefully be small, and submitting such renaming changes wouldn't be distracting. After that, I think I can create a mechanical change to rename variables to see how it will look like. > > > Thanks, > > -Michael > > > > [1] https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan <https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan> > [2] https://public-inbox.org/git/20190515214503.77162-8-brho at google.com/T/ <https://public-inbox.org/git/20190515214503.77162-8-brho at google.com/T/> > > > From: Rui Ueyama <ruiu at google.com <mailto:ruiu at google.com>> > Sent: 07 June 2019 08:42 > To: Chris Lattner <clattner at nondot.org <mailto:clattner at nondot.org>> > Cc: Michael Platings <Michael.Platings at arm.com <mailto:Michael.Platings at arm.com>>; llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>; nd <nd at arm.com <mailto:nd at arm.com>> > Subject: Re: [llvm-dev] RFC: changing variable naming rules in LLVM codebase > > > > This thread is not active for a while, but I'm still interested in seeing a change. > > > > Reading through this thread, looks like we can agree that we want to change the local variable naming scheme so that they start with a lowercase letter. Besides that, there were discussions about lower_case, camelCase, m_ prefix, and each argument seems as convincing as others. I'm not sure what is the decision making process in a situation like this. > > > > I'd personally vote for changing local variables to start with a lowercase letter and keep other naming conventions as-is to make it a minimum change. > > > > As I stated before, I'm happy to make a change to lld to see how a naming convention change will look like, but in order to do that I need to get at least a rough consensus to do that. What is a way to proceed? > > > > On Sat, May 25, 2019 at 3:00 PM Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > Hi Michael, > > > > I’m still very interested in seeing a change here. If someone is interested in seeing a codebase using the proposed camelBack convention for variables names, the MLIR codebase is public <https://github.com/tensorflow/mlir> and uses it. > > > > If you’re curious to see what this looks like in practice, there are lots of examples in the codebase, here is an example .cpp file <https://github.com/tensorflow/mlir/blob/master/lib/Transforms/LoopUnrollAndJam.cpp>, here is another <https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp>, here is an example header <https://github.com/tensorflow/mlir/blob/master/include/mlir/IR/Location.h>. > > > > > > We are still working our way through open sourcing logistics (not all the code is out yet), but we would still like to contribute at least parts of this to LLVM if the project is interested. [[This is just an FYI, not itself a proposal yet - one will be coming when we’re ready.]] > > > > -Chris > > > > > > > On May 21, 2019, at 3:01 AM, Michael Platings via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > > > Hi folks, > > Git is on its way to learning how to ignore commits, allowing us to do variable renaming and other small refactorings without breaking git blame. > > It's like git-hyper-blame [1] but significantly more powerful as it uses fuzzy matching to match lines, including lines that may have been split or joined. > > A preview release of Git with this new feature is at: https://github.com/mplatings/git/releases/tag/ignore-rev <https://github.com/mplatings/git/releases/tag/ignore-rev> > > Some of you have told me that you already have to spend time running git blame multiple times to look past uninteresting commits so I'd love for you to give this feature a try and see if it helps you. Your feedback will be very valuable. > > Thanks, > -Michael > > [1] https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html <https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190708/aff7af9b/attachment.html>
Rui Ueyama via llvm-dev
2019-Jul-09 07:23 UTC
[llvm-dev] RFC: changing variable naming rules in LLVM codebase
Thanks, Chris. Looks like everybody is at least mildly comfortable with my variable name renaming change, so I'll to submit that change to lld subdirectory soon. If you have any objections, please let me know. Note that this is not the end of my effort but actually the beginning of it. As Chris said, I believe we should do this to the entire LLVM codebase. I'm planning to do that directory-by-directory basis. On Tue, Jul 9, 2019 at 2:03 PM Chris Lattner <clattner at nondot.org> wrote:> This looks really great to me Rui, and I’m also pleased to see the > positive comments on the review thread. Thank you for driving this forward! > > -Chris > > > On Jul 4, 2019, at 9:50 PM, Rui Ueyama <ruiu at google.com> wrote: > > Hi all, > > I wrote a tool <https://reviews.llvm.org/D64123> to batch-rename variable > names so that they are in camelCase, and I applied the tool to lld > subdirectory. You can see my change at https://reviews.llvm.org/D64121. > If you have any comments, please reply. > > If people are happy about this change, I can do the same thing for other > directories including LLVM itself and Clang. > > On Mon, Jun 10, 2019 at 6:34 PM Rui Ueyama <ruiu at google.com> wrote: > >> On Mon, Jun 10, 2019 at 6:27 PM Michael Platings < >> Michael.Platings at arm.com> wrote: >> >>> Hi Rui, >>> >>> >>> >>> As per the provisional plan [1] we’re still at step 1: improving git >>> blame. The status of this is that there are some fairly mature patches in >>> the Git project’s queue [2], and I’m hopeful that it will be accepted in >>> something close to its current form. >>> >>> >>> >>> But if you can get started on steps 2 & 3 i.e. making forks available >>> with the possible changes applied then that would be great. My hope is that >>> once everyone can see what the options really look like then it will be >>> easier to reach consensus. >>> >> >> Sure, I'll try to do that. I'll probably start with finding identifiers >> and typenames that will conflict after the naming scheme change and rename >> them so that they won't conflict. The number of such symbols would >> hopefully be small, and submitting such renaming changes wouldn't be >> distracting. After that, I think I can create a mechanical change to rename >> variables to see how it will look like. >> >> >>> >>> Thanks, >>> >>> -Michael >>> >>> >>> >>> [1] https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan >>> >>> [2] >>> https://public-inbox.org/git/20190515214503.77162-8-brho at google.com/T/ >>> >>> >>> >>> *From:* Rui Ueyama <ruiu at google.com> >>> *Sent:* 07 June 2019 08:42 >>> *To:* Chris Lattner <clattner at nondot.org> >>> *Cc:* Michael Platings <Michael.Platings at arm.com>; >>> llvm-dev at lists.llvm.org; nd <nd at arm.com> >>> *Subject:* Re: [llvm-dev] RFC: changing variable naming rules in LLVM >>> codebase >>> >>> >>> >>> This thread is not active for a while, but I'm still interested in >>> seeing a change. >>> >>> >>> >>> Reading through this thread, looks like we can agree that we want to >>> change the local variable naming scheme so that they start with a lowercase >>> letter. Besides that, there were discussions about lower_case, camelCase, >>> m_ prefix, and each argument seems as convincing as others. I'm not sure >>> what is the decision making process in a situation like this. >>> >>> >>> >>> I'd personally vote for changing local variables to start with a >>> lowercase letter and keep other naming conventions as-is to make it a >>> minimum change. >>> >>> >>> >>> As I stated before, I'm happy to make a change to lld to see how a >>> naming convention change will look like, but in order to do that I need to >>> get at least a rough consensus to do that. What is a way to proceed? >>> >>> >>> >>> On Sat, May 25, 2019 at 3:00 PM Chris Lattner via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>> Hi Michael, >>> >>> >>> >>> I’m still very interested in seeing a change here. If someone is >>> interested in seeing a codebase using the proposed camelBack convention for >>> variables names, the MLIR codebase is public >>> <https://github.com/tensorflow/mlir> and uses it. >>> >>> >>> >>> If you’re curious to see what this looks like in practice, there are >>> lots of examples in the codebase, here is an example .cpp file >>> <https://github.com/tensorflow/mlir/blob/master/lib/Transforms/LoopUnrollAndJam.cpp> >>> , here is another >>> <https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp>, >>> here is an example header >>> <https://github.com/tensorflow/mlir/blob/master/include/mlir/IR/Location.h> >>> . >>> >>> >>> >>> >>> >>> We are still working our way through open sourcing logistics (not all >>> the code is out yet), but we would still like to contribute at least parts >>> of this to LLVM if the project is interested. [[This is just an FYI, not >>> itself a proposal yet - one will be coming when we’re ready.]] >>> >>> >>> >>> -Chris >>> >>> >>> >>> >>> >>> On May 21, 2019, at 3:01 AM, Michael Platings via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>> >>> >>> Hi folks, >>> >>> Git is on its way to learning how to ignore commits, allowing us to do >>> variable renaming and other small refactorings without breaking git blame. >>> >>> It's like git-hyper-blame [1] but significantly more powerful as it uses >>> fuzzy matching to match lines, including lines that may have been split or >>> joined. >>> >>> A preview release of Git with this new feature is at: >>> https://github.com/mplatings/git/releases/tag/ignore-rev >>> >>> Some of you have told me that you already have to spend time running git >>> blame multiple times to look past uninteresting commits so I'd love for you >>> to give this feature a try and see if it helps you. Your feedback will be >>> very valuable. >>> >>> Thanks, >>> -Michael >>> >>> [1] >>> https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190709/309b4564/attachment.html>
James Henderson via llvm-dev
2019-Jul-09 10:16 UTC
[llvm-dev] RFC: changing variable naming rules in LLVM codebase
Hi Rui, Is your tool in a good state to be run on people's downstream repositories? It would be nice to be able to update variable names there too at the same time. James On Tue, 9 Jul 2019 at 08:24, Rui Ueyama via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Thanks, Chris. > > Looks like everybody is at least mildly comfortable with my variable name > renaming change, so I'll to submit that change to lld subdirectory soon. If > you have any objections, please let me know. Note that this is not the end > of my effort but actually the beginning of it. As Chris said, I believe we > should do this to the entire LLVM codebase. I'm planning to do that > directory-by-directory basis. > > On Tue, Jul 9, 2019 at 2:03 PM Chris Lattner <clattner at nondot.org> wrote: > >> This looks really great to me Rui, and I’m also pleased to see the >> positive comments on the review thread. Thank you for driving this forward! >> >> -Chris >> >> >> On Jul 4, 2019, at 9:50 PM, Rui Ueyama <ruiu at google.com> wrote: >> >> Hi all, >> >> I wrote a tool <https://reviews.llvm.org/D64123> to batch-rename >> variable names so that they are in camelCase, and I applied the tool to lld >> subdirectory. You can see my change at https://reviews.llvm.org/D64121. >> If you have any comments, please reply. >> >> If people are happy about this change, I can do the same thing for other >> directories including LLVM itself and Clang. >> >> On Mon, Jun 10, 2019 at 6:34 PM Rui Ueyama <ruiu at google.com> wrote: >> >>> On Mon, Jun 10, 2019 at 6:27 PM Michael Platings < >>> Michael.Platings at arm.com> wrote: >>> >>>> Hi Rui, >>>> >>>> >>>> >>>> As per the provisional plan [1] we’re still at step 1: improving git >>>> blame. The status of this is that there are some fairly mature patches in >>>> the Git project’s queue [2], and I’m hopeful that it will be accepted in >>>> something close to its current form. >>>> >>>> >>>> >>>> But if you can get started on steps 2 & 3 i.e. making forks available >>>> with the possible changes applied then that would be great. My hope is that >>>> once everyone can see what the options really look like then it will be >>>> easier to reach consensus. >>>> >>> >>> Sure, I'll try to do that. I'll probably start with finding identifiers >>> and typenames that will conflict after the naming scheme change and rename >>> them so that they won't conflict. The number of such symbols would >>> hopefully be small, and submitting such renaming changes wouldn't be >>> distracting. After that, I think I can create a mechanical change to rename >>> variables to see how it will look like. >>> >>> >>>> >>>> Thanks, >>>> >>>> -Michael >>>> >>>> >>>> >>>> [1] https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan >>>> >>>> [2] >>>> https://public-inbox.org/git/20190515214503.77162-8-brho at google.com/T/ >>>> >>>> >>>> >>>> *From:* Rui Ueyama <ruiu at google.com> >>>> *Sent:* 07 June 2019 08:42 >>>> *To:* Chris Lattner <clattner at nondot.org> >>>> *Cc:* Michael Platings <Michael.Platings at arm.com>; >>>> llvm-dev at lists.llvm.org; nd <nd at arm.com> >>>> *Subject:* Re: [llvm-dev] RFC: changing variable naming rules in LLVM >>>> codebase >>>> >>>> >>>> >>>> This thread is not active for a while, but I'm still interested in >>>> seeing a change. >>>> >>>> >>>> >>>> Reading through this thread, looks like we can agree that we want to >>>> change the local variable naming scheme so that they start with a lowercase >>>> letter. Besides that, there were discussions about lower_case, camelCase, >>>> m_ prefix, and each argument seems as convincing as others. I'm not sure >>>> what is the decision making process in a situation like this. >>>> >>>> >>>> >>>> I'd personally vote for changing local variables to start with a >>>> lowercase letter and keep other naming conventions as-is to make it a >>>> minimum change. >>>> >>>> >>>> >>>> As I stated before, I'm happy to make a change to lld to see how a >>>> naming convention change will look like, but in order to do that I need to >>>> get at least a rough consensus to do that. What is a way to proceed? >>>> >>>> >>>> >>>> On Sat, May 25, 2019 at 3:00 PM Chris Lattner via llvm-dev < >>>> llvm-dev at lists.llvm.org> wrote: >>>> >>>> Hi Michael, >>>> >>>> >>>> >>>> I’m still very interested in seeing a change here. If someone is >>>> interested in seeing a codebase using the proposed camelBack convention for >>>> variables names, the MLIR codebase is public >>>> <https://github.com/tensorflow/mlir> and uses it. >>>> >>>> >>>> >>>> If you’re curious to see what this looks like in practice, there are >>>> lots of examples in the codebase, here is an example .cpp file >>>> <https://github.com/tensorflow/mlir/blob/master/lib/Transforms/LoopUnrollAndJam.cpp> >>>> , here is another >>>> <https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp>, >>>> here is an example header >>>> <https://github.com/tensorflow/mlir/blob/master/include/mlir/IR/Location.h> >>>> . >>>> >>>> >>>> >>>> >>>> >>>> We are still working our way through open sourcing logistics (not all >>>> the code is out yet), but we would still like to contribute at least parts >>>> of this to LLVM if the project is interested. [[This is just an FYI, not >>>> itself a proposal yet - one will be coming when we’re ready.]] >>>> >>>> >>>> >>>> -Chris >>>> >>>> >>>> >>>> >>>> >>>> On May 21, 2019, at 3:01 AM, Michael Platings via llvm-dev < >>>> llvm-dev at lists.llvm.org> wrote: >>>> >>>> >>>> >>>> Hi folks, >>>> >>>> Git is on its way to learning how to ignore commits, allowing us to do >>>> variable renaming and other small refactorings without breaking git blame. >>>> >>>> It's like git-hyper-blame [1] but significantly more powerful as it >>>> uses fuzzy matching to match lines, including lines that may have been >>>> split or joined. >>>> >>>> A preview release of Git with this new feature is at: >>>> https://github.com/mplatings/git/releases/tag/ignore-rev >>>> >>>> Some of you have told me that you already have to spend time running >>>> git blame multiple times to look past uninteresting commits so I'd love for >>>> you to give this feature a try and see if it helps you. Your feedback will >>>> be very valuable. >>>> >>>> Thanks, >>>> -Michael >>>> >>>> [1] >>>> https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>> >>>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>> >> _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190709/74febf3d/attachment.html>
Joan Lluch via llvm-dev
2019-Jul-09 12:46 UTC
[llvm-dev] RFC: changing variable naming rules in LLVM codebase
Hi Rui, I’m totally positive on switching to camelCase convention. In fact I have been always uncomfortable with the current naming approach. My only suggestion/concern is that we should provide a transition path not only for the trunk code in the repository, but for eventual out-of-trunk code with implementations of custom architectures. I have currently a custom backend implemented on top of LLVM 9 and therefore this change will surely break my code. I assume that developers affected by this will be able to run the converting tools to fix their own code. John> On 9 Jul 2019, at 09:23, Rui Ueyama via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > Thanks, Chris. > > Looks like everybody is at least mildly comfortable with my variable name renaming change, so I'll to submit that change to lld subdirectory soon. If you have any objections, please let me know. Note that this is not the end of my effort but actually the beginning of it. As Chris said, I believe we should do this to the entire LLVM codebase. I'm planning to do that directory-by-directory basis. > >> On Tue, Jul 9, 2019 at 2:03 PM Chris Lattner <clattner at nondot.org> wrote: >> This looks really great to me Rui, and I’m also pleased to see the positive comments on the review thread. Thank you for driving this forward! >> >> -Chris >> >> >>> On Jul 4, 2019, at 9:50 PM, Rui Ueyama <ruiu at google.com> wrote: >>> >>> Hi all, >>> >>> I wrote a tool to batch-rename variable names so that they are in camelCase, and I applied the tool to lld subdirectory. You can see my change at https://reviews.llvm.org/D64121. If you have any comments, please reply. >>> >>> If people are happy about this change, I can do the same thing for other directories including LLVM itself and Clang. >>> >>>> On Mon, Jun 10, 2019 at 6:34 PM Rui Ueyama <ruiu at google.com> wrote: >>>>> On Mon, Jun 10, 2019 at 6:27 PM Michael Platings <Michael.Platings at arm.com> wrote: >>>> >>>>> Hi Rui, >>>>> >>>>> >>>>> >>>>> As per the provisional plan [1] we’re still at step 1: improving git blame. The status of this is that there are some fairly mature patches in the Git project’s queue [2], and I’m hopeful that it will be accepted in something close to its current form. >>>>> >>>>> >>>>> >>>>> But if you can get started on steps 2 & 3 i.e. making forks available with the possible changes applied then that would be great. My hope is that once everyone can see what the options really look like then it will be easier to reach consensus. >>>>> >>>> >>>> Sure, I'll try to do that. I'll probably start with finding identifiers and typenames that will conflict after the naming scheme change and rename them so that they won't conflict. The number of such symbols would hopefully be small, and submitting such renaming changes wouldn't be distracting. After that, I think I can create a mechanical change to rename variables to see how it will look like. >>>> >>>>> >>>>> Thanks, >>>>> >>>>> -Michael >>>>> >>>>> >>>>> >>>>> [1] https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan >>>>> >>>>> [2] https://public-inbox.org/git/20190515214503.77162-8-brho at google.com/T/ >>>>> >>>>> >>>>> >>>>> From: Rui Ueyama <ruiu at google.com> >>>>> Sent: 07 June 2019 08:42 >>>>> To: Chris Lattner <clattner at nondot.org> >>>>> Cc: Michael Platings <Michael.Platings at arm.com>; llvm-dev at lists.llvm.org; nd <nd at arm.com> >>>>> Subject: Re: [llvm-dev] RFC: changing variable naming rules in LLVM codebase >>>>> >>>>> >>>>> >>>>> This thread is not active for a while, but I'm still interested in seeing a change. >>>>> >>>>> >>>>> >>>>> Reading through this thread, looks like we can agree that we want to change the local variable naming scheme so that they start with a lowercase letter. Besides that, there were discussions about lower_case, camelCase, m_ prefix, and each argument seems as convincing as others. I'm not sure what is the decision making process in a situation like this. >>>>> >>>>> >>>>> >>>>> I'd personally vote for changing local variables to start with a lowercase letter and keep other naming conventions as-is to make it a minimum change. >>>>> >>>>> >>>>> >>>>> As I stated before, I'm happy to make a change to lld to see how a naming convention change will look like, but in order to do that I need to get at least a rough consensus to do that. What is a way to proceed? >>>>> >>>>> >>>>> >>>>> On Sat, May 25, 2019 at 3:00 PM Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>>>> >>>>> Hi Michael, >>>>> >>>>> >>>>> >>>>> I’m still very interested in seeing a change here. If someone is interested in seeing a codebase using the proposed camelBack convention for variables names, the MLIR codebase is public and uses it. >>>>> >>>>> >>>>> >>>>> If you’re curious to see what this looks like in practice, there are lots of examples in the codebase, here is an example .cpp file, here is another, here is an example header. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> We are still working our way through open sourcing logistics (not all the code is out yet), but we would still like to contribute at least parts of this to LLVM if the project is interested. [[This is just an FYI, not itself a proposal yet - one will be coming when we’re ready.]] >>>>> >>>>> >>>>> >>>>> -Chris >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> On May 21, 2019, at 3:01 AM, Michael Platings via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>>>> >>>>> >>>>> >>>>> Hi folks, >>>>> >>>>> Git is on its way to learning how to ignore commits, allowing us to do variable renaming and other small refactorings without breaking git blame. >>>>> >>>>> It's like git-hyper-blame [1] but significantly more powerful as it uses fuzzy matching to match lines, including lines that may have been split or joined. >>>>> >>>>> A preview release of Git with this new feature is at: https://github.com/mplatings/git/releases/tag/ignore-rev >>>>> >>>>> Some of you have told me that you already have to spend time running git blame multiple times to look past uninteresting commits so I'd love for you to give this feature a try and see if it helps you. Your feedback will be very valuable. >>>>> >>>>> Thanks, >>>>> -Michael >>>>> >>>>> [1] https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html >>>>> _______________________________________________ >>>>> LLVM Developers mailing list >>>>> llvm-dev at lists.llvm.org >>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> LLVM Developers mailing list >>>>> llvm-dev at lists.llvm.org >>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>> >> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190709/cea460a5/attachment.html>
Neil Nelson via llvm-dev
2019-Jul-09 15:48 UTC
[llvm-dev] RFC: changing variable naming rules in LLVM codebase
CamelCase is a style most commonly seen with those having a Microsoft orientation. It is uncommon for Linux code and is avoided by style requirement in the Boost libraries. I have coded CamelCase and otherwise. Perhaps changing the style of the original code merely on this point is a minor slam at those original authors who had a different perspective in mind. We may want to consider leaving the style as it is for a while to review how the current code-base is styled and why those authors preferred that style. Merely changing to CamelCase has a kind of 'Let's dress ourselves in Microsoft style", a point somewhat eagerly denied in the Linux community. Neil Nelson On 7/9/19 1:23 AM, Rui Ueyama via llvm-dev wrote:> Thanks, Chris. > > Looks like everybody is at least mildly comfortable with my variable > name renaming change, so I'll to submit that change to lld > subdirectory soon. If you have any objections, please let me know. > Note that this is not the end of my effort but actually the beginning > of it. As Chris said, I believe we should do this to the entire LLVM > codebase. I'm planning to do that directory-by-directory basis. > > On Tue, Jul 9, 2019 at 2:03 PM Chris Lattner <clattner at nondot.org > <mailto:clattner at nondot.org>> wrote: > > This looks really great to me Rui, and I’m also pleased to see the > positive comments on the review thread. Thank you for driving > this forward! > > -Chris > > >> On Jul 4, 2019, at 9:50 PM, Rui Ueyama <ruiu at google.com >> <mailto:ruiu at google.com>> wrote: >> >> Hi all, >> >> I wrote a tool <https://reviews.llvm.org/D64123> to batch-rename >> variable names so that they are in camelCase, and I applied the >> tool to lld subdirectory. You can see my change at >> https://reviews.llvm.org/D64121. If you have any comments, please >> reply. >> >> If people are happy about this change, I can do the same thing >> for other directories including LLVM itself and Clang. >> >> On Mon, Jun 10, 2019 at 6:34 PM Rui Ueyama <ruiu at google.com >> <mailto:ruiu at google.com>> wrote: >> >> On Mon, Jun 10, 2019 at 6:27 PM Michael Platings >> <Michael.Platings at arm.com <mailto:Michael.Platings at arm.com>> >> wrote: >> >> Hi Rui, >> >> As per the provisional plan [1] we’re still at step 1: >> improving git blame. The status of this is that there are >> some fairly mature patches in the Git project’s queue >> [2], and I’m hopeful that it will be accepted in >> something close to its current form. >> >> But if you can get started on steps 2 & 3 i.e. making >> forks available with the possible changes applied then >> that would be great. My hope is that once everyone can >> see what the options really look like then it will be >> easier to reach consensus. >> >> >> Sure, I'll try to do that. I'll probably start with finding >> identifiers and typenames that will conflict after the naming >> scheme change and rename them so that they won't conflict. >> The number of such symbols would hopefully be small, and >> submitting such renaming changes wouldn't be distracting. >> After that, I think I can create a mechanical change to >> rename variables to see how it will look like. >> >> >> Thanks, >> >> -Michael >> >> [1] >> https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan >> >> [2] >> https://public-inbox.org/git/20190515214503.77162-8-brho at google.com/T/ >> >> *From:*Rui Ueyama <ruiu at google.com <mailto:ruiu at google.com>> >> *Sent:* 07 June 2019 08:42 >> *To:* Chris Lattner <clattner at nondot.org >> <mailto:clattner at nondot.org>> >> *Cc:* Michael Platings <Michael.Platings at arm.com >> <mailto:Michael.Platings at arm.com>>; >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>; >> nd <nd at arm.com <mailto:nd at arm.com>> >> *Subject:* Re: [llvm-dev] RFC: changing variable naming >> rules in LLVM codebase >> >> This thread is not active for a while, but I'm still >> interested in seeing a change. >> >> Reading through this thread, looks like we can agree that >> we want to change the local variable naming scheme so >> that they start with a lowercase letter. Besides that, >> there were discussions about lower_case, camelCase, m_ >> prefix, and each argument seems as convincing as others. >> I'm not sure what is the decision making process in a >> situation like this. >> >> I'd personally vote for changing local variables to start >> with a lowercase letter and keep other naming >> conventions as-is to make it a minimum change. >> >> As I stated before, I'm happy to make a change to lld to >> see how a naming convention change will look like, but in >> order to do that I need to get at least a rough consensus >> to do that. What is a way to proceed? >> >> On Sat, May 25, 2019 at 3:00 PM Chris Lattner via >> llvm-dev <llvm-dev at lists.llvm.org >> <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> Hi Michael, >> >> I’m still very interested in seeing a change here. >> If someone is interested in seeing a codebase using >> the proposed camelBack convention for variables >> names, the MLIR codebase is public >> <https://github.com/tensorflow/mlir> and uses it. >> >> If you’re curious to see what this looks like in >> practice, there are lots of examples in the codebase, >> here is an example .cpp file >> <https://github.com/tensorflow/mlir/blob/master/lib/Transforms/LoopUnrollAndJam.cpp>, >> here is another >> <https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp>, >> here is an example header >> <https://github.com/tensorflow/mlir/blob/master/include/mlir/IR/Location.h>. >> >> We are still working our way through open sourcing >> logistics (not all the code is out yet), but we would >> still like to contribute at least parts of this to >> LLVM if the project is interested. [[This is just an >> FYI, not itself a proposal yet - one will be coming >> when we’re ready.]] >> >> -Chris >> >> >> >> On May 21, 2019, at 3:01 AM, Michael Platings via >> llvm-dev <llvm-dev at lists.llvm.org >> <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> Hi folks, >> >> Git is on its way to learning how to ignore >> commits, allowing us to do variable renaming and >> other small refactorings without breaking git blame. >> >> It's like git-hyper-blame [1] but significantly >> more powerful as it uses fuzzy matching to match >> lines, including lines that may have been split >> or joined. >> >> A preview release of Git with this new feature is >> at: >> https://github.com/mplatings/git/releases/tag/ignore-rev >> >> Some of you have told me that you already have to >> spend time running git blame multiple times to >> look past uninteresting commits so I'd love for >> you to give this feature a try and see if it >> helps you. Your feedback will be very valuable. >> >> Thanks, >> -Michael >> >> [1] >> https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> <mailto:llvm-dev at lists.llvm.org> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190709/8246ad1f/attachment-0001.html>