rjmccall added a comment. @rsmith This is a big enough architectural change that I'd appreciate your sign-off on the basic approach.
================ Comment at: clang/lib/Sema/SemaDecl.cpp:3759 unsigned BuiltinID; - if (Old->isImplicit() && (BuiltinID = Old->getBuiltinID())) { + bool RevertedBuiltin{Old->getIdentifier()->hasRevertedBuiltin()}; + if (Old->isImplicit() && ---------------- rjmccall wrote: > The old code didn't check this eagerly. The `Old->isImplicit()` check is > unlikely to fire; please make sure that we don't do any extra work unless it > does. Since BuiltinAttr is inherited, we should keep the isImplicit check, because we only want to use a special diagnostic when "redeclaring" the implicit builtin declaration. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:8880 + } + } + ---------------- Hmm. I'm concerned about not doing any sort of semantic compatibility check here before we assign the function special semantics. Have we really never done those in C++? If not, I wonder if we can reasonably make an implicit declaration but just make it hidden from lookup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits