mstorsjo added a comment. In D129277#3636567 <https://reviews.llvm.org/D129277#3636567>, @aaron.ballman wrote:
> Thanks for catching this! Is it really an NFC change though (it seems like it > would change some of the diagnostic behavior and the list of suggested > predefines)? Can you add test coverage for the change? TBH I haven’t tried to follow exactly where this case would matter in the current state of affairs - the function is called in three places, and maybe the individual roles of the parameters currently only make a difference in the other callers. As it didn’t break any tests I presumed it’s NFC. Alternatively I can keep this change folded into D126676 <https://reviews.llvm.org/D126676> (although I’m not sure if the parameter change will end up visible in the final form of that patch though, so maybe I I might end up dropping the change from there too if it’s not testable there either?). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129277/new/ https://reviews.llvm.org/D129277 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
