spatel closed this revision. spatel added a comment. In D80584#2066319 <https://reviews.llvm.org/D80584#2066319>, @jdoerfert wrote:
> First, I think the warning is strictly a good thing. Thanks for keeping that > in! Sure - we've been missing that for a long time. > I don't think the options are really "complexity to the user" given a > sensible default (= the old value). If a user sees a warning that says: `Name > conflict, use --nameless-prefix=XXXXX, where XXXXX is a string not otherwise > used in your IR`, they will happily do so. The UTC_ARGS extension makes it > permanent and no one needs to worry about this until they see a warning > again. (If we can "hide" the option from the help message, I'm fine with that > too.) I'm still not convinced that's worth doing. I did change instnamer though: rGdd54432a0f5a <https://reviews.llvm.org/rGdd54432a0f5a6f042fa4d2db3094c6f02e5ad275> ...so between the warning and at least 1 of these not using 'tmp', I think we're good enough. > Adding a test is just something we should do as it is easy to accidentally > break functionality like the warning. Yep, I didn't remember that we had tests for scripts. So we have that now, and there's a clang test that wiggles on this too...although that seems like we should generalize that file to avoid killing all of the bots like I did. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80584/new/ https://reviews.llvm.org/D80584 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
