setModelData crashes program

[JonB]

@JNBarchan said in setModelData crashes program:

And always check the return result of a qobject_cast<>, for safety, certainly while developing! :)

In my nice (non-Qt) C# background, this kind of casting is built into the language. For qobject_cast<> we write:

ComboBox cb = editor as CombBox;

where the as operator returns null if editor cannot be cast to ComboBox at runtime. We use this if editor might not be a ComboBox.

However, if editor must be/is intended to be a ComboBox, we write:

ComboBox cb = (ComboBox) editor;

where the direct cast throws a runtime Exception if editor is not a ComboBox (i.e. as would return null). This saves a lot of code/accidents.

It would be nice if Qt offered a "variant" of qobject_cast<> in C++ which checked and automatically threw an Exception if the return result is NULL/0....?

[VRonin]

you just need to add Q_ASSERT(whatComboEditor); to have that

[JonB]

@VRonin said in setModelData crashes program:

you just need to add Q_ASSERT(whatComboEditor); to have that

Ah, OK. It's just that you have to remember to write that every time after each qobject_cast<>, it would be nice to have them combined in one statement. I suppose you would write:

Q_ASSERT(whatComboEditor = qobject_cast<>)

but is Q_ASSERT code removed in non-debug code and/or does it evaluate its argument more than once (assuming it's a macro)?

I would like a qobject_assert_cast<> :)

EDIT: Yes, I see you cannot use Q_ASSERT(whatComboEditor = qobject_cast<>) in non-debug as the code gets removed.

[SGaist]

That's not specific to Q_ASSERT, it's the same for the standard C++ assert. Always test the result of a function and not the function itself.

[VRonin]

If you want to use it in code that should still be present in release mode you can use Q_ASSUME

[JonB]

@VRonin said in setModelData crashes program:

If you want to use it in code that should still be present in release mode you can use Q_ASSUME

Um, I don't think so are you sure: http://doc.qt.io/qt-5/qtglobal.html#Q_ASSUME states it's a compile-time hint, and you get "undefined behaviour" if the condition is false. We need a runtime check of the return result of qobject_cast<>.

EDIT: Unless the docs are misleading, and the true implementation is via https://git.merproject.org/faenil/qtbase/commit/3b0ed624351441a2d7be45cf9582fd36955ae860 , which uses Q_ASSERT_X?

[VRonin]

Q_ASSUME in debug mode is like Q_ASSERT in release mode, if whatComboEditor is null the program would crash anyway so you can't do much worse.

The undefined behaviour comes from when you use the return value: if(Q_ASSUME(false)) is undefined behaviour. if it just wraps an operation you do not check anyway, Q_ASSUME does nothing to your compiled binary in release mode

[JonB]

@VRonin
That wasn't quite the point, though. I was talking about writing the single-line statement (if Qt won't offer a qboject_assert_cast<>):

Q_ASSERT(whatComboEditor = qobject_cast<>)

versus

Q_ASSUME(whatComboEditor = qobject_cast<>)

The point being that it does the assignment. So we do need the expression to evaluate, just we don't need/care to do the "assertion" part of testing the result in release build. This would not work for Q_ASSERT as (I believe) it's a macro which produces nothing (no code at all) in release build. That was when you suggested Q_ASSUME.

[VRonin]

• Q_ASSERT(statement);
  • Debug: if(!statement) triggerAnAssertion();
  • Release: ;
• Q_ASSUME(statement);
  • Debug: if(!statement) triggerAnAssertion();
  • Release: statement;

Is the difference clearer now?

[gabor53]

@VRonin
I added

 Q_ASSERT(whatComboEditor);

but it behaves the same.

[VRonin]

@VRonin said in setModelData crashes program:

Sounds like a model bug. What is fixModel?

[gabor53]

@VRonin

QSqlTableModel* fixModel = new QSqlTableModel(this);
  fixModel->setTable ("Items");
  fixModel->setEditStrategy (QSqlTableModel::OnRowChange);
  fixModel->setSort (2, Qt::DescendingOrder);

  fixModel->select ();

[JonB]

@VRonin

Is the difference clearer now?

Yep! :) Tx.

[gabor53]

@VRonin
Any way to go around the problem?
Would it be possible to just redraw the row in which the field was changed?