r264071 - StaticAnalyzer: Avoid an unintentional copy

Justin Bogner via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 29 16:59:03 PDT 2016


David Blaikie <dblaikie at gmail.com> writes:
> On Tue, Mar 22, 2016 at 10:50 AM, Justin Bogner via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: bogner
>> Date: Tue Mar 22 12:50:05 2016
>> New Revision: 264071
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=264071&view=rev
>> Log:
>> StaticAnalyzer: Avoid an unintentional copy
>>
>
> Just to be clear - it sounds like a copy happens here regardless. It's just
> best to be explicit so readers aren't confused about that. (sounds like the
> iterator produces results by value (probably non-standard for an iterator
> to do that, but probably not uncommon) & so taking a reference to it does
> reference lifetime extension, etc) Now we're being explicit aobut the
> pointer being copied, rather than implying that we refer to a pointer kept
> inside the sequence/container.

Right. The copy was actually only of the pointer here, so my commit
message was kind of confusing. It should've been more like "Use a
`FieldDecl *` rather than a `FieldDecl *&`".

>
>>
>> The range here isn't over references, so using `auto &` here incites a
>> copy. Switching to `auto *` would do, but we might as well list an
>> explicit type for clarity.
>>
>> Found by -Wrange-loop-analysis.
>>
>> Modified:
>>     cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp?rev=264071&r1=264070&r2=264071&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Tue Mar 22
>> 12:50:05 2016
>> @@ -168,7 +168,7 @@ public:
>>                                          const ASTRecordLayout &RL) {
>>      CharUnits PaddingSum;
>>      CharUnits Offset =
>> ASTContext.toCharUnitsFromBits(RL.getFieldOffset(0));
>> -    for (const auto &FD : RD->fields()) {
>> +    for (const FieldDecl *FD : RD->fields()) {
>>        // This checker only cares about the padded size of the
>>        // field, and not the data size. If the field is a record
>>        // with tail padding, then we won't put that number in our
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>


More information about the cfe-commits mailing list