aaron.ballman added a comment. In D91000#3770071 <https://reviews.llvm.org/D91000#3770071>, @whisperity wrote:
> Generally LGTM. Please revisit the documentation and let's fix the style, and > then we can land. > > In D91000#3197851 <https://reviews.llvm.org/D91000#3197851>, @aaron.ballman > wrote: > >> In terms of whether we should expose the check in C++: I think we shouldn't. >> [...] >> >> I think we should probably also not enable the check when the user compiles >> in C99 or earlier mode, because there is no Annex K available to provide >> replacement functions. > > @aaron.ballman I think the current version of the check satisfies these > conditions. It seems the check **will** run for every TU, but in case there > is no alternative the check could suggest, it will do nothing: > > if (!ReplacementFunctionName) > return; > > Is this good enough? This seems more future-proof than juggling the > `LangOpts` instance in yet another member function. My concern with that approach was that we pay the full expense of doing the matches only get get into the `check()` function to bail out on all the Annex K functions, but now that there are replacements outside of Annex K, I don't see a way around paying that expense, so I think my concern has been addressed as well as it could have been. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91000/new/ https://reviews.llvm.org/D91000 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
