On Sun, Jul 27, 2025 at 12:53 AM Tender Wang <tndrw...@gmail.com> wrote:
> > 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.
