[PATCH] D91649: [AA] Split up LocationSize::unknown()

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 19 13:42:52 PST 2020


asbirlea added a comment.

In D91649#2401902 <https://reviews.llvm.org/D91649#2401902>, @nikic wrote:

> In D91649#2401252 <https://reviews.llvm.org/D91649#2401252>, @asbirlea wrote:
>
>> - `LocationSize::unknownMaybeNegative()` seems somewhat ambiguous in isolation; unless I see the other name or have the context from this patch. I'm either keep `LocationSize::unknown` or make it even more verbose `LocationSize::unknownPositiveOrNegative()`.
>
> I'd like to not reuse the existing `unknown()` name, to force all users (including out of tree users) to make a decision one way or another. Not particularly attached to the names though. We could also drop the "unknown" prefix and make them `LocationSize::nonNegative()` and `LocationSize::positiveOrNegative()` for example.
>
> Generally I'm not really fond of "negative" sizes as a concept, it just seemed like the most concise way to explain it. It would be more accurate to say that the size is always positive, but there may be a negative offset. So maybe `LocationSize::nonNegative()`, `LocationSize::withNonNegativeOffset()` and `LocationSize::withAnyOffset()`, where the first two are aliases but can be used according to desired semantics. Can't say I really like this either though.
>
> Or maybe `LocationSize::afterPointer()` and `LocationSize::beforeOrAfterPointer()`.

Either of the options: `LocationSize::withNonNegativeOffset()` and `LocationSize::withAnyOffset()`, or LocationSize::afterPointer()` and `LocationSize::beforeOrAfterPointer()` are better IMO.
I don't have strong opinions either way as long as they're reasonably self explanatory, and these meet that criteria.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91649/new/

https://reviews.llvm.org/D91649



More information about the llvm-commits mailing list