TextChanged triggered with syntaxhighlighter->rehighlight()

Vogtinator

Sorry for the doublepost, but now I'm getting another problem.
The destructor of QSyntaxHighlighter seems to trigger the textChanged() signal, but the QPlainTextEdit has already been destroyed (by "delete ui" in the MainWindow destructor) and it results in a call to __cxa_pure_virtual which terminates the program!
Is this a bug?
I know I can circumvent it by destroying them in the right order, but shouldn't the QObject system already handle such things correctly for me?

SGaist

This might indeed be a bug since the ownership of the highlighter should have been taken or do you set its parent later ?

Vogtinator

The parent is QPlainTextEditor::document and it doesn't even get to highlightBlock(text). My workaround is
@ ui->codeEdit->blockSignals(true);
delete highlighter;
delete ui;@

which works 100%. Simplest solution would be not to trigger textChanged() in the destructor or not by the QSyntaxHighlighter at all.

SGaist

The way I see it:

highlighter is destroyed

document must be "cleaned" from highlighting

documentChanged is emitted

Vogtinator

Here, a complete stacktrace:
The MainWindow deletes the QSyntaxHighlighter, which triggers textChanged(), which calls QSyntaxHighlighter::rehighlight(), which seems to be pure virtual as being destroyed.
@0 raise /lib64/libc.so.6
1 abort /lib64/libc.so.6
2 __gnu_cxx::__verbose_terminate_handler()
3 ??
4 std::terminate()
5 __cxa_pure_virtual
6 ?? /usr/lib64/libQtGui.so.4
7 ?? /usr/lib64/libQtGui.so.4
8 QSyntaxHighlighter::rehighlight()
9 SteveHighlighter::resetHighlight
10 MainWindow::textChanged mainwindow.cpp 187
11 MainWindow::qt_static_metacall
12 QMetaObject::activate(QObject*, QMetaObject const*, int, void**)
13 QMetaObject::activate(QObject*, QMetaObject const*, int, void**)
14 ??
15 QTextControl::qt_metacall(QMetaObject::Call, int, void**)
16 QMetaObject::activate(QObject*, QMetaObject const*, int, void**)
17 ??
18 QSyntaxHighlighter::setDocument(QTextDocument*)
19 QSyntaxHighlighter::~QSyntaxHighlighter()
20 SteveHighlighter::~SteveHighlighter stevehighlighter.h
21 SteveHighlighter::~SteveHighlighter stevehighlighter.h
22 MainWindow::~MainWindow mainwindow.cpp
23 main@

SGaist

Just thought about something... Why do you need to call rehighlight after resetErrorHighlighting ? Shouldn't that cleanup also do the highlighting ?

Also, could you share the code of the highlighter ? I just tried with a minimal highlighter and everything goes well

Vogtinator

bq. Just thought about something… Why do you need to call rehighlight after resetErrorHighlighting ? Shouldn’t that cleanup also do the highlighting ?

Yes, and it does it with rehighlight :-)

@#include "stevehighlighter.h"

SteveHighlighter::SteveHighlighter(QPlainTextEdit *editor)
: QSyntaxHighlighter(editor->document()), parent(editor)
{
QTextCharFormat format;

 format.setForeground(QColor(0, 128, 0));
 format.setFontWeight(QFont::Bold);
 setFormat(TOK_KEYWORD, format);

 format.setForeground(QColor(192, 16, 112));
 format.setFontWeight(QFont::Bold);
 setFormat(TOK_CONDITION, format);

 format.setForeground(QColor(128, 0, 0));
 format.setFontWeight(QFont::Bold);
 setFormat(TOK_INSTRUCTION, format);

}

void SteveHighlighter::setFormat(Token what, const QTextCharFormat &format)
{
this->format[what] = format;
rehighlight();
}

void SteveHighlighter::resetHighlight()
{
rehighlight();
}@

Header:
@#ifndef STEVEHIGHLIGHTER_H
#define STEVEHIGHLIGHTER_H

#include <QPlainTextEdit>
#include <QSyntaxHighlighter>

enum Token {
TOK_KEYWORD = 0,
TOK_CONDITION = 1,
TOK_INSTRUCTION = 2
};

class SteveHighlighter : public QSyntaxHighlighter
{
Q_OBJECT

public:
SteveHighlighter(QPlainTextEdit *editor);
void highlightBlock(const QString &text) override {};
void highlight(int line, const QTextCharFormat &format, const QString &what = "") {};
void resetHighlight();
void setFormat(Token what, const QTextCharFormat &format);

private:
int highlight_line;
QString highlight_str;
QPlainTextEdit *parent;
QTextCharFormat format[TOK_INSTRUCTION + 1];
};
#endif // STEVEHIGHLIGHTER_H@

I removed some irrelevant parts, but I also noticed after removal of the constructor code, it no longer crashes. I didn't expect that.

SGaist

The thing is, you are creating an infinite loop calling rehighlight in the slot triggered by textChanged, you should rather do everything in one pass (doesn't mean that you can't split the highlighting part in several functions)
or delay the highlighting a bit.

One thing to try to change: pass the text edit directly as parent (third constructor)

Vogtinator

bq. The thing is, you are creating an infinite loop calling rehighlight in the slot triggered by textChanged, you should rather do everything in one pass (doesn’t mean that you can’t split the highlighting part in several functions) or delay the highlighting a bit.

But that's not the cause of the crash.

bq. One thing to try to change: pass the text edit directly as parent (third constructor)

Tried that. Doesn't work, QPlainTextEdit != QTextEdit. QPlainTextEdit gets implicitly converted to QObject.

SGaist

Indeed... But why are you keeping a pointer to the parent ? You don't have any destructor and IIRC the default generated should delete all the member so deleting the parent variable which hold a pointer to the QPlainTextEdit which is the parent (indirectly) of the highlighter might be a problem...

Sorry I mixed the two classes

Vogtinator

I'm 100% sure a default constructor doesn't delete any objects a member pointer points to, only the pointer itself. A manual-specified destructor always runs before destroying the non-static members.

SGaist

You're right ! The object pointed to are not destroyed by the default constructor.

There must be something else... What did you removed from the constructor that avoided the crash ?

Vogtinator

The three
@ QTextCharFormat format;
format.setForeground(QColor(0, 128, 0));
format.setFontWeight(QFont::Bold);
setFormat(TOK_KEYWORD, format);@

blocks. Without them, it works. Might be because there's a similiar function setFormat but with different arguments in QSyntaxHighlighter. But then, it should also crash without these lines.
I could narrow it down further to rehighlight() in setFormat, but I really don't know why.

SGaist

Strange... Can't reproduce it on OS X

By the way, I would recommend using a QHash, you'll be more flexible if you add new Tokens