This is an archive of the discontinued LLVM Phabricator instance.

Use an allow list on reserved macro identifiers
ClosedPublic

Authored by serge-sans-paille on May 10 2021, 7:10 AM.

Details

Summary

The allow list is based on man feature_test_macros

This fixes https://bugs.llvm.org/show_bug.cgi?id=50248

Diff Detail

Event Timeline

serge-sans-paille requested review of this revision.May 10 2021, 7:10 AM
serge-sans-paille created this revision.

There is a number of other macros that are meant to be user-defined,
see e.g. https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html

Also, cfe-commits not subscribed

Adding cfe-commits.

Some additional ones to allow from MSDN: _CRT_SECURE_NO_WARNINGS, _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES, _CRT_NONSTDC_NO_WARNINGS, (https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160).

clang/lib/Lex/PPDirectives.cpp
125
aaron.ballman added inline comments.May 10 2021, 8:18 AM
clang/lib/Lex/PPDirectives.cpp
125

Can you add a comment that this list is required to remain in alphabetical order (due to the use of binary_search) and list the identifiers in long-form rather than columns of three (this makes it easier for folks inserting new elements into the list)?

Minor nits as suggested by reviewers + extend the list.

This revision is now accepted and ready to land.May 12 2021, 12:00 PM
This revision was landed with ongoing or failed builds.May 13 2021, 12:24 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2021, 12:24 AM