nickdesaulniers added inline comments.
================ Comment at: clang/include/clang/Basic/Builtins.def:513 BUILTIN(__builtin_printf, "icC*.", "Fp:0:") +BUILTIN(__builtin_putchar, "ii", "F") +BUILTIN(__builtin_puts, "icC*", "nF") ---------------- rsmith wrote: > aaron.ballman wrote: > > Should we also add a builtin for `putc()` (I know that's often a macro, so > > I'm not certain if it applies)? > Yes, GCC has a `__builtin_putc`, so it'd make sense for us to support that > too. Curious, `putc` isn't documented at https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html, which is what I was using. It looks like `putc` was a part of ANSI C, so I'm not sure what else might be missing from my implementation. Let me see if I can find a more complete list of C90 functions to verify. ================ Comment at: clang/lib/Lex/PPMacroExpansion.cpp:348 // C++ Standing Document Extensions. - if (LangOpts.CPlusPlus) + if (getLangOpts().CPlusPlus) Ident__has_cpp_attribute = ---------------- rsmith wrote: > The changes to this file appear to be independent of the rest of the patch. > Please go ahead and land this cleanup separately. done: 663f4f7edc2476231fa5bfc04519d5fd51a10112 ================ Comment at: clang/test/CXX/over/over.oper/over.literal/p7.cpp:10 -void puts(const char *); +int puts(const char *); static inline void operator "" _puts(const char *c) { ---------------- rsmith wrote: > Doesn't this also need to be `extern "C"` to be recognized as the builtin? Interesting, if I change this back to `void`, the test passes for me. I'm not sure why it ever failed then. Will drop this hunk from the diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86508/new/ https://reviews.llvm.org/D86508 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
