[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-11-02 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov abandoned this revision. george.karpenkov added a comment. @dcoughlin OK, I guess you could theoretically come up with a scenario where an error in this code would lead to crashing-while-crashing which would obscure the original error. https://reviews.llvm.org/D38986

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-11-01 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added a comment. Personally, I don't think this is worth it and I find it unpleasant to add untestable code -- especially if that code is going to stick around in release builds. https://reviews.llvm.org/D38986 ___ cfe-commits mailing

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-11-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @dcoughlin @NoQ so can this be committed? https://reviews.llvm.org/D38986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > Does llvm_unreachable() guarantee that the string construction code is > completely removed from release builds? http://llvm.org/docs/CodingStandards.html#assert-liberally: > When assertions are disabled (i.e. in release builds), `llvm_unreachable` > becomes a hint to

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @dcoughlin > Is it when an end-user is running a build with assertions and can't provide a > reproducer but can provide the console output? Yes, or just for developer staring at the crash for the first time, or for the crashers in CI. > Does

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-20 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added a comment. What is the workflow where this is needed? Is it when an end-user is running a build with assertions and can't provide a reproducer but can provide the console output? Does llvm_unreachable() guarantee that the string construction code is completely removed from

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > I think it is much better when the assert failure tells the developer _what_ > value is failing, rather than saying "oops we are dead". yes of course, more informative assert messages is better. https://reviews.llvm.org/D38986

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-16 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun. @dcoughlin I'm curious whether you'd like such a change: in general, I think it is much better when the assert failure tells the developer _what_ value is failing, rather than saying "oops we are dead". I would