[Polly][PATCH] Use reference to clarify pointer can never be null

Tobias Grosser tobias at grosser.es
Sat Apr 18 08:57:16 PDT 2015


On 04/18/2015 04:59 PM, Johannes Doerfert wrote:
> On 04/18, Tobias Grosser wrote:
>> On 04/18/2015 03:58 PM, Johannes Doerfert wrote:
>>>> -const ScopDetection::BoxedLoopsSetTy *
>>>> +const ScopDetection::BoxedLoopsSetTy &
>>>>   ScopDetection::getBoxedLoops(const Region *R) const {
>>>> -  auto BLMIt = BoxedLoopsMap.find(R);
>>>> -  if (BLMIt == BoxedLoopsMap.end())
>>>> -    return nullptr;
>>>> -  return &BLMIt->second;
>>>> +  return BoxedLoopsMap.find(R)->second;
>>>>   }
>>> Are you sure the access of ->second is actually legal if find returns
>>> BoxedLoopsMap.end() ?
>>
>> My understanding is that we will have such a map for each region that we
>> detect. Hence, .find(R) should always be successful.
> If called with a region that run through ScopDetect it will be
> successful. However, if for whatever reason a valid region that was
> skipped or ignored by the ScopDetection is passed to that method it
> will most certainly crash.

I now added an explicit assert to check this.

> I do not see the benefit on the reference but only the possible problem
> of an undocumented precondition that cannot even be checked.

When looking through the code, I wondered if nullptr will ever be 
returned in a valid execution. As this seems not the case, I wanted to 
clarify this for future people who may ask themselves the same question.

I attached an updated patch.

Best,
Tobias

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Use-reference-instead-of-a-pointer.patch
Type: text/x-patch
Size: 5291 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150418/341369e0/attachment.bin>


More information about the llvm-commits mailing list