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

On Tue, Apr 27, 2021 at 8:01 AM Melanie Blower via Phabricator < revi...@reviews.llvm.org> wrote: > mibintc abandoned this revision. > mibintc added a comment. > > There was no resolution about what option name would be acceptable > Oh no, I'm sorry that you felt that way. I saw Vitaly's

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

mibintc abandoned this revision. mibintc added a comment. There was no resolution about what option name would be acceptable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96203/new/ https://reviews.llvm.org/D96203

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

vitalybuka requested changes to this revision. vitalybuka added a comment. This revision now requires changes to proceed. Is this abandoned patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96203/new/ https://reviews.llvm.org/D96203

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

To my taste "exclude" better represents meaning than "disallow". E.g. disallow can be interpreted as "make them fatal". We can fix 2a4317bfb319e for consistency later. -fsanitize-exclude-list= ? On Thu, 25 Feb 2021 at 09:54, Melanie Blower via Phabricator < revi...@reviews.llvm.org> wrote: >

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

mibintc added a comment. In D96203#2587942 , @echristo wrote: > Exclusion list? or exclude? In 2a4317bfb319e , (Fangrui Song 2020-06-19) changed option name

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

echristo added a comment. Exclusion list? or exclude? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96203/new/ https://reviews.llvm.org/D96203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

mibintc updated this revision to Diff 326412. mibintc added a comment. I rebased the patch and basically made no other changes. On the review there's a suggestion that 'blocklist' isn't the best word choice, do you have suggestions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

vitalybuka added a comment. Could you please rebase the patch onto D96974 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96203/new/ https://reviews.llvm.org/D96203 ___

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

mibintc added inline comments. Comment at: clang/include/clang/Basic/SanitizerBlacklist.h:20 #include #include vitalybuka wrote: > This file should go into a separate patch. It touches multiple files with > simple changes but unlikely will case rollback.

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

vitalybuka added inline comments. Comment at: clang/include/clang/Basic/SanitizerBlacklist.h:20 #include #include This file should go into a separate patch. It touches multiple files with simple changes but unlikely will case rollback. It's unrelated to

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

vitalybuka added a comment. In D96203#2559792 , @mibintc wrote: > In D96203#2559426 , @vitalybuka > wrote: > FWIW I would prefer denylist as well. (Also uses of whitelist should be allowlist, but also

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

mibintc added a comment. In D96203#2559426 , @vitalybuka wrote: >>> FWIW I would prefer denylist as well. (Also uses of whitelist should be >>> allowlist, but also incremental :) > > I feel like existing and proposed value do match the meaning of this

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

vitalybuka added a comment. >> FWIW I would prefer denylist as well. (Also uses of whitelist should be >> allowlist, but also incremental :) I feel like existing and proposed value do match the meaning of this list. maybe ignorelist, skiplist (can be confused with data structure?), or just

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

MaskRay added a comment. In D96203#2548529 , @echristo wrote: > In D96203#2548495 , @aaron.ballman > wrote: > >> In D96203#2548471 , @mibintc wrote: >> >>> In

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

echristo added a reviewer: vitalybuka. echristo added a comment. In D96203#2548495 , @aaron.ballman wrote: > In D96203#2548471 , @mibintc wrote: > >> In D96203#2546856 ,

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

aaron.ballman added a comment. In D96203#2548471 , @mibintc wrote: > In D96203#2546856 , @aaron.ballman > wrote: > >> Thank you for working on this! >> >>> This changes the option names that include substring

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

mibintc added a comment. In 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

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

aaron.ballman added a comment. 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

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

mibintc created this revision. mibintc added reviewers: Sanitizers, aaron.ballman, echristo, MaskRay. Herald added subscribers: dexonsmith, dang. Herald added a reviewer: jansvoboda11. mibintc requested review of this revision. Herald added a project: clang. This changes the option names that