On Tue, Apr 23, 2019 at 11:36 PM Petr Hosek via Phabricator < revi...@reviews.llvm.org> wrote:
> phosek added inline comments. > > > ================ > Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:122 > + > +/// \p Flag must be a flag accepted by the driver with its leading '-' > removed, > +// otherwise '-print-multi-lib' will not emit them correctly. > ---------------- > jroelofs wrote: > > Can we enforce this precondition with an assert? The > '-'-prefix-not-there part is easy, but what about the "it's a driver flag" > part? > We could, but we would need to pass in the reference to `Driver` so we can > use `getOpts().ParseOneArg` to parse the flag. However, we're not doing > that in `Multilib` either at the moment even though [the comment says it > has to be a valid flag]( > https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Multilib.h#L80). > I'd prefer to keep this as a pure move and then add the additional checking > both here and to `Multilib` as a separate change, would that be fine with > you? Sure, that works. Jon > > > Repository: > rC Clang > > CHANGES SINCE LAST ACTION > https://reviews.llvm.org/D61040/new/ > > https://reviews.llvm.org/D61040 > > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
