[PATCH] D103356: Reset all options in cl::ResetCommandLineParser()

[Sam McCall via Phabricator via llvm-commits]

sammccall accepted this revision.
sammccall added a comment.

Sorry for the delay. I'm not really familiar with the code, but this seems right (not sure if it's bulletproof, but surely not worse).



================
Comment at: llvm/lib/Support/CommandLine.cpp:1328
       O.second->reset();
+    for (Option *O : SC->PositionalOpts)
+      O->reset();
----------------
As I understand these will typically be in OptionsMap too, unless ArgStr happens to be empty.
So we're possibly resetting the same options multiple times, and that's OK because it's cheap and idempotent.

This all seems fine but I'd leave a comment at the top like "Reset all options at least once, it's okay if some get reset multiple times".

(An alternative is to track which ones have been reset, or add a member containing this, but it doesn't seem worth it)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103356/new/

https://reviews.llvm.org/D103356