Re: r297975 - Use arg_begin() instead of getArgumentList().begin(), the argument list is an implementation detail

[David Blaikie via cfe-commits]

On Mon, Mar 20, 2017 at 8:59 AM Reid Kleckner  wrote:

> I came across llvm/docs/HistoricalNotes/2002-06-25-MegaPatchInfo.txt,
> which has this:
> """
> * The Function class now has helper functions for accessing the Arguments
> list.
>   Instead of having to go through getArgumentList for simple things like
>   iterator over the arguments, now the a*() methods can be used to access
> them.
> """
>
> So, it seems like there was a desire to move away from using
> getArgumentList() all the way back in 2002.
>
> I removed getArgumentList() in a follow-up change anyway, so it's not a
> public member anymore.
>

Ah, great - haven't got to my llvm-commits catch up yet, I'll keep an eye
out for it. Thanks!


>
> On Mon, Mar 20, 2017 at 8:54 AM, David Blaikie  wrote:
>
>
>
> On Thu, Mar 16, 2017 at 12:07 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: rnk
> Date: Thu Mar 16 13:55:46 2017
> New Revision: 297975
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297975=rev
> Log:
> Use arg_begin() instead of getArgumentList().begin(), the argument list is
> an implementation detail
>
>
> Seems like a somewhat strange justification, given that getArgumentList
> looks like a public member and probably used pervasively before LLVM moved
> towards more iterator-centric interfaces, etc.
>
> What do you mean by 'an implementation detail' in this context/what
> motivated this change? (curious what I'm missing)
>
> - Dave
>
>
>
> Modified:
> cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=297975=297974=297975=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Mar 16 13:55:46 2017
> @@ -3780,9 +3780,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFun
>// Emit initial values for private copies (if any).
>llvm::Value *TaskPrivatesMap = nullptr;
>auto *TaskPrivatesMapTy =
> -
> std::next(cast(TaskFunction)->getArgumentList().begin(),
> -3)
> -  ->getType();
> +  std::next(cast(TaskFunction)->arg_begin(),
> 3)->getType();
>if (!Privates.empty()) {
>  auto FI = std::next(KmpTaskTWithPrivatesQTyRD->field_begin());
>  TaskPrivatesMap = emitTaskPrivateMappingFunction(
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r297975 - Use arg_begin() instead of getArgumentList().begin(), the argument list is an implementation detail

[Reid Kleckner via cfe-commits]

I came across llvm/docs/HistoricalNotes/2002-06-25-MegaPatchInfo.txt, which
has this:
"""
* The Function class now has helper functions for accessing the Arguments
list.
  Instead of having to go through getArgumentList for simple things like
  iterator over the arguments, now the a*() methods can be used to access
them.
"""

So, it seems like there was a desire to move away from using
getArgumentList() all the way back in 2002.

I removed getArgumentList() in a follow-up change anyway, so it's not a
public member anymore.

On Mon, Mar 20, 2017 at 8:54 AM, David Blaikie  wrote:

>
>
> On Thu, Mar 16, 2017 at 12:07 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rnk
>> Date: Thu Mar 16 13:55:46 2017
>> New Revision: 297975
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=297975=rev
>> Log:
>> Use arg_begin() instead of getArgumentList().begin(), the argument list
>> is an implementation detail
>>
>
> Seems like a somewhat strange justification, given that getArgumentList
> looks like a public member and probably used pervasively before LLVM moved
> towards more iterator-centric interfaces, etc.
>
> What do you mean by 'an implementation detail' in this context/what
> motivated this change? (curious what I'm missing)
>
> - Dave
>
>
>>
>> Modified:
>> cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
>> CGOpenMPRuntime.cpp?rev=297975=297974=297975=diff
>> 
>> ==
>> --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Mar 16 13:55:46 2017
>> @@ -3780,9 +3780,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFun
>>// Emit initial values for private copies (if any).
>>llvm::Value *TaskPrivatesMap = nullptr;
>>auto *TaskPrivatesMapTy =
>> -  std::next(cast(TaskFunction)->
>> getArgumentList().begin(),
>> -3)
>> -  ->getType();
>> +  std::next(cast(TaskFunction)->arg_begin(),
>> 3)->getType();
>>if (!Privates.empty()) {
>>  auto FI = std::next(KmpTaskTWithPrivatesQTyRD->field_begin());
>>  TaskPrivatesMap = emitTaskPrivateMappingFunction(
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r297975 - Use arg_begin() instead of getArgumentList().begin(), the argument list is an implementation detail

[David Blaikie via cfe-commits]

On Thu, Mar 16, 2017 at 12:07 PM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Thu Mar 16 13:55:46 2017
> New Revision: 297975
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297975=rev
> Log:
> Use arg_begin() instead of getArgumentList().begin(), the argument list is
> an implementation detail
>

Seems like a somewhat strange justification, given that getArgumentList
looks like a public member and probably used pervasively before LLVM moved
towards more iterator-centric interfaces, etc.

What do you mean by 'an implementation detail' in this context/what
motivated this change? (curious what I'm missing)

- Dave


>
> Modified:
> cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=297975=297974=297975=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Mar 16 13:55:46 2017
> @@ -3780,9 +3780,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFun
>// Emit initial values for private copies (if any).
>llvm::Value *TaskPrivatesMap = nullptr;
>auto *TaskPrivatesMapTy =
> -
> std::next(cast(TaskFunction)->getArgumentList().begin(),
> -3)
> -  ->getType();
> +  std::next(cast(TaskFunction)->arg_begin(),
> 3)->getType();
>if (!Privates.empty()) {
>  auto FI = std::next(KmpTaskTWithPrivatesQTyRD->field_begin());
>  TaskPrivatesMap = emitTaskPrivateMappingFunction(
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r297975 - Use arg_begin() instead of getArgumentList().begin(), the argument list is an implementation detail

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Mar 16 13:55:46 2017
New Revision: 297975

URL: http://llvm.org/viewvc/llvm-project?rev=297975=rev
Log:
Use arg_begin() instead of getArgumentList().begin(), the argument list is an 
implementation detail

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=297975=297974=297975=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Mar 16 13:55:46 2017
@@ -3780,9 +3780,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFun
   // Emit initial values for private copies (if any).
   llvm::Value *TaskPrivatesMap = nullptr;
   auto *TaskPrivatesMapTy =
-  std::next(cast(TaskFunction)->getArgumentList().begin(),
-3)
-  ->getType();
+  std::next(cast(TaskFunction)->arg_begin(), 3)->getType();
   if (!Privates.empty()) {
 auto FI = std::next(KmpTaskTWithPrivatesQTyRD->field_begin());
 TaskPrivatesMap = emitTaskPrivateMappingFunction(


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits