We can merge the two scans into one. This can reduce the time complexity from 2O(n) to O(n).
This seems like an unusual usage of big-O notation. Always saw the meaningful pieces inside the O, and in general I thought O(n) =~ O(xn) for all x, but especially if x is small.
The point made by Chao Li, that by using a loop guard we avoid performing expression evaluation, seems like a reasonable and sufficient non-data supported reason for having the code structured this way. I'd at least want a more data-driven reason to try and change it in such a simple way against the read/write boundary that presently exists.
I was pondering whether contain_volatile_functions could/should be taught to also detect whether every expression it sees resolves to a constant...
contain_volatile_functions((Node *) rte->values_lists, &has_non_constants)
But got a little ways down the call stack and hit the "walker" macro and decided to stop...
Also, add a brace for better/more consistent style in the attached patch.
Should try and avoid oh-by-the-way fixes like this in behavior patches.
This specific one also doesn't seem warranted. Or, at least, the original style is indeed de facto acceptable; if-blocks containing a single executing statement do not get braces but rely on indentation alone. I don't think that every other if-block in the region being multi-statement supports an exception.
David J.
