[PATCH] D129277: [clang] [Serialization] Fix swapped PPOpts/ExistingPPOpts parameters. NFC.

Martin Storsjö via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 7 11:29:06 PDT 2022


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



More information about the cfe-commits mailing list