Fix #12861 Hang in valueFlowCondition() with huge array

[clock999]

I tested the fix based on 4617bc2.

Use the cppcheck to check the below code which is submitted on the ticket #12861.

#define ROW A, A, A, A, A, A, A, A,
#define ROW8 ROW ROW ROW ROW ROW ROW ROW ROW
#define ROW64 ROW8 ROW8 ROW8 ROW8 ROW8 ROW8 ROW8 ROW8
#define ROW512 ROW64 ROW64 ROW64 ROW64 ROW64 ROW64 ROW64 ROW64
void f() {
	static const char A = 'a';
	const char a[] = {
		ROW512	ROW512 	ROW512	ROW512
	};
}

With the fix, the overall time is 1.52582s.
Without the fix, the time is 25.9674s.

I also tested the testrunner for running the whole of the unit tests. There is some performance improment with the fix, but not remarkable.

[chrchr-github]

Please add a test in test/cli/performance_test.py.

[firewave]

Please add a test in test/cli/performance_test.py.

The test already exists and it is the one failing with this change applied. So somehow this actually makes things worse.

[clock999]

Yes, I will check it again. If I can't fix it, I will ignore this PR.

[clock999]

I updated the commit. For the test case submitted by the ticket #12861, the performance is improved a lot, at least the consumed time can be reduced to less than 2 seconds. For other test cases, I don't have the exact testing data, but I think this can be helpful for the performance.

[chrchr-github]

Is there a reason not to implement caching in the regular astTop() function (i.e. why add astFinalTop())?

[clock999]

Hi CHR, the process of creating the AST tree is a little complicated for me currently, and I can't figure out the details, that is also why I didn't modify the createAst(). As I understand, during TokenList::createAst(), the astTop() is used. But at that time, the createAst() has not been finished, so the top is a temporary one, which can not be cached as the final top. So we need two versions of the function, one is for creating ast, and another with the cache function is for the usage after the ast is created.
Maybe we can replaced the astTop with a new function name for createAst without cache function. And we keep astTop() added the cache that may be better for no confusion. Or we can add a param to astTop(bool iscache).

[chrchr-github]

I think this sounds reasonable: astTop(bool iscache). And please add a comment explaining the parameter.

[firewave]

This seems unnecessary.

[danmar]

remove the "pls" :-)

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud