Revert "export IProblem"

Authored by brauch on Oct 16 2016, 2:28 PM.

Description

Revert "export IProblem"

Actually, let's not put this into 5.0, it breaks binary compatibility.
This reverts commit e4fb596442fe7cb1f5a96c377a08c0aee917b05c.

Details

Committed
brauchOct 16 2016, 2:28 PM
Parents
R33:e4fb596442fe: export IProblem
Branches
Unknown
Tags
Unknown
Reverts
R33:e4fb596442fe: export IProblem
rjvbb added a subscriber: rjvbb.Oct 16 2016, 2:40 PM

Actually, let's not put this into 5.0, it breaks binary compatibility.

Have there never been commits in the 5.0 kdevplatform branch that require a KDevelop rebuild?

So if this fix is not going to show up in a 5.0.x release version, please consider making at least the Q_ASSERT in ClangProblem::allFixits a regular one that also triggers in release/non-debug builds. (And keep in mind that users don't get any explanation about the qFatal() reason in regular setups on Mac: the error message just disappears.)

As far as I'm aware we avoid that. If you change the binary interface of IProblem, you have to rebuild:

  • kdevelop
  • kdev-python
  • kdev-php
  • kdev-ruby

If you forget any, you will get weird crashes.

I'm not sure what you hope to achieve for your users with changing the assert. It will just crash either way. Why not just build your mac version from master ...?

rjvbb added a comment.Oct 16 2016, 3:03 PM

I'm not so much concerned about the MacPorts ports I maintain because I can add whatever patches I deem required. But others will want to use the (5.0.x) release versions to build themselves, or maybe there are even official binaries to download.

I'd still argue that anything that's dynamic can go wrong for various reasons, and if it's not a crucial feature a well-written application will be designed to handle such failures dynamically (here it only means the user doesn't get a popup about the syntax error: the error is still flagged visually and explained in the code browser). For unexpected errors that arise because of benign reasons it would be more elegant to put up a dialog and then exit cleanly, for instance.

If the KDev team has different ideas about that, it's cleaner and safer to do an explicit abort instead of letting the code crash because of a null pointer dereference. It won't make difference for the user in the best case, but it'll make it easier to triage any tickets that might be opened because of the crashing.

And maybe Qt's logging system can be tweaked so that qFatal() puts up a dialog once the application is set up properly? I've patched Qt myself so I get a dialog that shows the app name, the message passed to qFatal() and gives me the choice between calling abort() or exit(). A priori I don't get the suggestion about dunking the cache when I restart a KDevelop session after taking the exit() option.