Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed.
Let me double down -- just because we let some select people know about an option, I still don't think it should be user facing. I'm strongly against the philosophy that a core-modifying decision, such as configuring a state split should be presented on the default interface. Having "smart users" isn't an excuse to that either -- the last thing I'd like to see broadcasted is that you need to be a genius to use or configure a tool that is suppose to serve you, not the other way around. ================ Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:620-624 + "AggressiveEraseModeling", + "Enables exploration of the past-the-end branch for the " + "return value of the erase() method of containers.", + "false", + Released> ---------------- NoQ wrote: > Szelethus wrote: > > Hmm. The description isn't really user-friendly, imo, but the option > > doesn't sound too user-facing either. I don't think this is an option to be > > tinkered with. I think we should make this hidden. > > > > Also, even for developers, I find this a bitconfusing at first. Do we not > > enable that by default? Why not? What do we have to gain/lose? > What is the rule that the user needs to follow in order to avoid the warning? > "Always check the return value of `erase()` before use"? If so, a good > description would be along the lines of "Warn when the return value of > `erase()` is not checked before use; compare that value to end() in order to > suppress the warning" (or something more specific). Please mark this hidden. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77150/new/ https://reviews.llvm.org/D77150 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
