rjmccall added inline comments.
================ Comment at: clang/lib/Basic/Builtins.cpp:33 static constexpr Builtin::Info BuiltinInfo[] = { - {"not a builtin function", nullptr, nullptr, nullptr, HeaderDesc::NO_HEADER, +#define INTERESTING_IDENTIFIER(ID) \ + {#ID, nullptr, nullptr, nullptr, HeaderDesc::NO_HEADER, ALL_LANGUAGES}, ---------------- zahiraam wrote: > From your comment in Builtin.h: "You shouldn't muddle this into Builtin::ID." > This means this shouldn't happen here. It should be done in > IdentiferTable::AddKeywords as I had done previously? You shouldn't be adding entries for the interesting keywords to the builtins table, no. This table is indexed by Builtin::ID. I agree that `AddKeywords` seems like the right place to set up the interesting identifiers rather than `initializeBuiltins`. ================ Comment at: clang/lib/Basic/Builtins.cpp:138 + // Step #2: mark all target-independent builtins with their ID's. + for (unsigned i = FirstBuiltinID; i != Builtin::FirstTSBuiltin; ++i) if (builtinIsSupported(BuiltinInfo[i], LangOpts)) { ---------------- You shouldn't use FirstBuiltinID and FirstInterestingIdentifierID as the starting points here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146148/new/ https://reviews.llvm.org/D146148 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits