[education/labplot] /: Fixed the regression in the parsing logic of parametric expressions.

Alexander Semke <[email protected]>
Newsgroups gmane.comp.kde.cvs
Message-ID <[email protected]>
Git commit c533d1d4791f04b1aeb49d1f3b36d3cb9698bfcc by Alexander Semke.
Committed on 19/07/2026 at 10:18.
Pushed by sgerlach into branch 'master'.

Fixed the regression in the parsing logic of parametric expressions.

Use two separate parser instances for the x- and y-expressions -  the "high performance" mode
of the parser records, on its very first parse() call, which symbols are actually used and restricts all
subsequent parse() calls to only those symbols.  since xexpr and yexpr are generally different
expressions that may use different sets of symbols/functions (e.g.  xexpr = "1+sin(t)",  
yexpr = "cos(t)*(1+sin(t))"), sharing a single parser between them would incorrectly reject symbols
in yexpr that don't happen to appear in xexpr (and vice versa).

M  +26   -13   src/backend/gsl/ExpressionParser.cpp
M  +96   -0    tests/backend/gsl/ExpressionParserTest.cpp
M  +5    -0    tests/backend/gsl/ExpressionParserTest.h

https://invent.kde.org/education/labplot/-/commit/c533d1d4791f04b1aeb49d1f3b36d3cb9698bfcc

diff --git a/src/backend/gsl/ExpressionParser.cpp b/src/backend/gsl/ExpressionParser.cpp
index 6902e5fcf6..723928a9ac 100644
--- a/src/backend/gsl/ExpressionParser.cpp
+++ b/src/backend/gsl/ExpressionParser.cpp
@@ -849,25 +849,38 @@ bool ExpressionParser::tryEvaluateParametric(const QString& xexpr,
 	const Range<double> range{min, max};
 	const double step = range.stepSize(count);
 
-	Parser parser(true);
-	ParserLastErrorMessage lock(parser, m_lastErrorMessage);
+	// use two separate parser instances for the x- and y-expressions:
+	// the "high performance" mode of the parser records, on its very first parse() call,
+	// which symbols are actually used and restricts all subsequent parse() calls to only
+	// those symbols. since xexpr and yexpr are generally different expressions that may use
+	// different sets of symbols/functions (e.g. xexpr = "1+sin(t)", yexpr = "cos(t)*(1+sin(t))"),
+	// sharing a single parser between them would incorrectly reject symbols in yexpr that
+	// don't happen to appear in xexpr (and vice versa).
+	Parser xParser(true);
+	Parser yParser(true);
 
 	const auto numberLocale = QLocale();
 	for (int i = 0; i < count; i++) {
-		parser.assign_symbol("t", range.start() + step * i);
-		parser.assign_symbol("i", i + 1);
-
-		double x = parser.parse(qPrintable(xexpr), qPrintable(numberLocale.name()));
-		if (parser.parseErrors() > 0) // try default locale if failing
-			x = parser.parse(qPrintable(xexpr), "en_US");
-		if (parser.parseErrors() > 0)
+		xParser.assign_symbol("t", range.start() + step * i);
+		xParser.assign_symbol("i", i + 1);
+		yParser.assign_symbol("t", range.start() + step * i);
+		yParser.assign_symbol("i", i + 1);
+
+		double x = xParser.parse(qPrintable(xexpr), qPrintable(numberLocale.name()));
+		if (xParser.parseErrors() > 0) // try default locale if failing
+			x = xParser.parse(qPrintable(xexpr), "en_US");
+		if (xParser.parseErrors() > 0) {
+			m_lastErrorMessage = QString::fromStdString(xParser.lastErrorMessage());
 			return false;
+		}
 
-		double y = parser.parse(qPrintable(yexpr), qPrintable(numberLocale.name()));
-		if (parser.parseErrors() > 0) // try default locale if failing
-			y = parser.parse(qPrintable(yexpr), "en_US");
-		if (parser.parseErrors() > 0)
+		double y = yParser.parse(qPrintable(yexpr), qPrintable(numberLocale.name()));
+		if (yParser.parseErrors() > 0) // try default locale if failing
+			y = yParser.parse(qPrintable(yexpr), "en_US");
+		if (yParser.parseErrors() > 0) {
+			m_lastErrorMessage = QString::fromStdString(yParser.lastErrorMessage());
 			return false;
+		}
 
 		if (std::isnan(x))
 			WARN(Q_FUNC_INFO << ", WARNING: X expression " << STDSTRING(xexpr) << " evaluated @ " << range.start() + step * i << " is NAN")
diff --git a/tests/backend/gsl/ExpressionParserTest.cpp b/tests/backend/gsl/ExpressionParserTest.cpp
index e0c6866e2c..f2e722f65c 100644
--- a/tests/backend/gsl/ExpressionParserTest.cpp
+++ b/tests/backend/gsl/ExpressionParserTest.cpp
@@ -1085,4 +1085,100 @@ void ExpressionParserTest::testPolarSpiral() {
 		VALUES_EQUAL(sqrt(xVector.at(i) * xVector.at(i) + yVector.at(i) * yVector.at(i)), (i + 1) / 100.);
 }
 
+// r(phi) = 1 + sin(phi), uses both "sin" and the constant/index symbols so the used-symbols
+// cache of the underlying high-performance parser needs to keep working across the whole range.
+void ExpressionParserTest::testPolarCardioid() {
+	auto* parser = ExpressionParser::getInstance();
+	constexpr auto numElements = 100;
+	const double min = 0.0;
+	const double max = 6.28;
+	const double step = (max - min) / (numElements - 1);
+
+	QVector<double> xVector(numElements);
+	QVector<double> yVector(numElements);
+
+	QVERIFY(parser->tryEvaluatePolar(QStringLiteral("1+sin(phi)"), QStringLiteral("0.0"), QStringLiteral("6.28"), numElements, &xVector, &yVector));
+
+	QCOMPARE(xVector.size(), numElements);
+	QCOMPARE(yVector.size(), numElements);
+
+	for (int i = 0; i < numElements; i++) {
+		const double phi = min + step * i;
+		const double r = 1. + sin(phi);
+		VALUES_EQUAL(xVector.at(i), r * cos(phi));
+		VALUES_EQUAL(yVector.at(i), r * sin(phi));
+	}
+}
+
+// Regression test for the min/max/count based tryEvaluateCartesian() overload (used e.g. for
+// simple function plots), separate from the vars/xVectors based overload tested above.
+void ExpressionParserTest::testEvaluateCartesianRange() {
+	auto* parser = ExpressionParser::getInstance();
+	constexpr auto numElements = 5;
+	QVector<double> xVector(numElements);
+	QVector<double> yVector(numElements);
+
+	QVERIFY(parser->tryEvaluateCartesian(QStringLiteral("x^2"), QStringLiteral("0"), QStringLiteral("4"), numElements, &xVector, &yVector));
+
+	QCOMPARE(xVector.size(), numElements);
+	QCOMPARE(yVector.size(), numElements);
+
+	const QVector<double> xRef({0., 1., 2., 3., 4.});
+	const QVector<double> yRef({0., 1., 4., 9., 16.});
+	COMPARE_DOUBLE_VECTORS(xVector, xRef);
+	COMPARE_DOUBLE_VECTORS(yVector, yRef);
+}
+
+void ExpressionParserTest::testEvaluateParametricCircle() {
+	auto* parser = ExpressionParser::getInstance();
+	constexpr auto numElements = 100;
+	QVector<double> xVector(numElements);
+	QVector<double> yVector(numElements);
+
+	QVERIFY(parser->tryEvaluateParametric(QStringLiteral("cos(t)"),
+										  QStringLiteral("sin(t)"),
+										  QStringLiteral("0.0"),
+										  QStringLiteral("6.28"),
+										  numElements,
+										  &xVector,
+										  &yVector));
+
+	QCOMPARE(xVector.size(), numElements);
+	QCOMPARE(yVector.size(), numElements);
+
+	for (int i = 0; i < numElements; i++)
+		VALUES_EQUAL(sqrt(xVector.at(i) * xVector.at(i) + yVector.at(i) * yVector.at(i)), 1.);
+}
+
+// Regression test: expr1 and expr2 use different sets of symbols/functions ("sin" only vs.
+// "sin" and "cos"). Since tryEvaluateParametric() parses both expressions internally, each one
+// must resolve its own symbols correctly regardless of what the other expression used.
+void ExpressionParserTest::testEvaluateParametricMismatchedSymbols() {
+	auto* parser = ExpressionParser::getInstance();
+	constexpr auto numElements = 100;
+	const double min = 0.0;
+	const double max = 6.28;
+	const double step = (max - min) / (numElements - 1);
+
+	QVector<double> xVector(numElements);
+	QVector<double> yVector(numElements);
+
+	QVERIFY(parser->tryEvaluateParametric(QStringLiteral("1+sin(t)"),
+										  QStringLiteral("cos(t)*(1+sin(t))"),
+										  QStringLiteral("0.0"),
+										  QStringLiteral("6.28"),
+										  numElements,
+										  &xVector,
+										  &yVector));
+
+	QCOMPARE(xVector.size(), numElements);
+	QCOMPARE(yVector.size(), numElements);
+
+	for (int i = 0; i < numElements; i++) {
+		const double t = min + step * i;
+		VALUES_EQUAL(xVector.at(i), 1. + sin(t));
+		VALUES_EQUAL(yVector.at(i), cos(t) * (1. + sin(t)));
+	}
+}
+
 QTEST_MAIN(ExpressionParserTest)
diff --git a/tests/backend/gsl/ExpressionParserTest.h b/tests/backend/gsl/ExpressionParserTest.h
index 3fcccdff24..8966ca1588 100644
--- a/tests/backend/gsl/ExpressionParserTest.h
+++ b/tests/backend/gsl/ExpressionParserTest.h
@@ -87,6 +87,11 @@ private Q_SLOTS:
 
 	void testPolarCircle();
 	void testPolarSpiral();
+	void testPolarCardioid();
+
+	void testEvaluateCartesianRange();
+	void testEvaluateParametricCircle();
+	void testEvaluateParametricMismatchedSymbols();
 };
 
 #endif // EXPRESSIONPARSERTEST_H
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.