This is an archive of the discontinued LLVM Phabricator instance.

[NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/
ClosedPublic

Authored by ZarkoCA on Nov 4 2021, 11:58 AM.

Details

Summary

Replace filenames, variable names, check prefixes uses of blacklist with ignore list.

Diff Detail

Event Timeline

ZarkoCA requested review of this revision.Nov 4 2021, 11:58 AM
ZarkoCA created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 4 2021, 11:58 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jkorous requested changes to this revision.Nov 4 2021, 4:11 PM

Hi! Thank you for the clean-up :)

I feel there might be a bit of work still left. While renaming filenames and function names should be mostly inconsequential renaming command line options and sanitizer ignorelist content (and by that changing the syntax) most likely need accompanying changes in clang itself.

I noticed you have at least one other similar patch but I don't see the changes I'd expect there - please let me know if I'm just missing something!

Do the tests actually pass with this patch?

clang/test/CodeGen/address-safety-attr.cpp
9

Shouldn't we teach ASan about the new syntax of ignorelist as well fun:*IgnorelistedFunction*?

10

Shouldn't the change of the option name to -fsanitize-ignorelist in tests be accompanied with analogous change in driver and/or cc1 options definition?

clang/test/CodeGenCXX/cfi-ignorelist.cpp
3

We should use the verb "to ignorelist smth" (without the space) consistently.

This revision now requires changes to proceed.Nov 4 2021, 4:11 PM
lebedev.ri resigned from this revision.Nov 5 2021, 12:22 AM

I will not be part of this language policing.
-10 to any changes that break existing external interface (that is, it is not okay to just rename externally-visible command-line flags, config options, etc.)

Hi! Thank you for the clean-up :)

I feel there might be a bit of work still left. While renaming filenames and function names should be mostly inconsequential renaming command line options and sanitizer ignorelist content (and by that changing the syntax) most likely need accompanying changes in clang itself.

I noticed you have at least one other similar patch but I don't see the changes I'd expect there - please let me know if I'm just missing something!

Do the tests actually pass with this patch?

Thanks for the quick review. The tests do pass, it looks like there has been a patch that allowed the ignorelist options to be accepted by clang: https://reviews.llvm.org/D101832.

ZarkoCA requested review of this revision.Nov 11 2021, 2:05 PM
jkorous accepted this revision.Nov 15 2021, 1:56 PM

LGTM

clang/test/CodeGen/address-safety-attr.cpp
9

Nvm - that's just name of the test function below.

This revision is now accepted and ready to land.Nov 15 2021, 1:56 PM