[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

Mon Feb 8 06:57:07 PST 2021

echristo added a reviewer: vitalybuka.
echristo added a comment.

In D96203#2548495 <https://reviews.llvm.org/D96203#2548495>, @aaron.ballman wrote:

> In D96203#2548471 <https://reviews.llvm.org/D96203#2548471>, @mibintc wrote:
>
>> In D96203#2546856 <https://reviews.llvm.org/D96203#2546856>, @aaron.ballman wrote:
>>
>>> Thank you for working on this!
>>>
>>>> This changes the option names that include substring blacklist to blocklist.
>>>
>>> I think this change works in some places, but in other places we say things like "blocklisted" which feels a bit awkward. I don't super love the name `blocklist`, but I don't super hate it either. All the alternative names I come up with aren't really great either, like `UnsanitiziedEntities` or `IgnoredObjects`. Maybe someone will have a better idea here than me, but I do think the current name is an improvement over the old name.
>>
>> There is a pre-existing patch https://reviews.llvm.org/D82244 using block and allow, that's why I chose blocklist. Although going in cold I would prefer to use allow and deny, and block seems to me a little like cheating (too similar to black).
>
> That was my concern as well -- I can easily imagine someone seeing "blocklisted" and thinking it was a typo for "blacklisted" and undoing the change to comments with an NFC change.
>
>> However I can "disagree and commit" (cf. https://en.wikipedia.org/wiki/Disagree_and_commit) with this choice of wording. I can search out the awkward Blocklisted and come up with different phrasing in the prose, but I think names in the program should use blocklist so that they can be easily connected to the option name for future maintainers of llvm.  I'll check this.
>
> I agree that the frontend option names and the internal variable names need to connect to one another, so if we pick a new term, it should be used consistently. I'd be mildly happier with a better term than blocklist, but I can live with that name too.

FWIW I would prefer denylist as well. (Also uses of whitelist should be allowlist, but also incremental :)

-eric

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96203