On Mar 2, 2013, at 11:18 AM, Rafael Espíndola <[email protected]> wrote:
>> It's not intentional - if it was we wouldn't have the assert there & >> this would just be an intended/valid codepath. >> >> If we have some way to take "we're in a state we don't expect to be >> in" (& importantly: haven't tested) & turn that in to a well defined >> crash/error reporting that seems to be the right thing to have here. > > Exactly. It gives the developers of the program using libclang the > opportunity to report you a bug with hopefully enough information to > fix it. This is similar to chromium's use of breakpad + the CHECK > macro (http://src.chromium.org/viewvc/chrome/trunk/src/base/location.h). > > When in function f you do > > if (!foo) { > assert(0 && ...); > return ...; > } > > You enter a slippery slope. The code calling this function will be in > an unexpected state and might itself crash, just in a harder to debug > way. This can lead to Why do you think that for this specific function, returning FileID() is valid for a return value. If a caller misbehaves because of it, then the caller should be fixed. > > f(); > if (did_f_ignore_a_bug) { > // ignore it too. > assert(0 && ...); > return; > } > > Note also that this change is in SourceManager.cpp. Could you at least > move it back in the stack to the caller in libclang? Not sure how this is possible, do you have specific source changes in mind ? > > Cheers, > Rafael _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
