Re: [patch 2/N] std::regex refactoring - sub _Executor for lookahead
On Mon, Apr 28, 2014 at 8:45 AM, Jonathan Wakely jwak...@redhat.com wrote: Is there any reason this object is created on the heap? Say, _Executor's size is so huge and a uncommon user gets a stack overflow by keep invoking this function? -- Regards, Tim Shen
Re: [patch 2/N] std::regex refactoring - sub _Executor for lookahead
On 28/04/14 10:18 -0400, Tim Shen wrote: On Mon, Apr 28, 2014 at 8:45 AM, Jonathan Wakely jwak...@redhat.com wrote: Is there any reason this object is created on the heap? Say, _Executor's size is so huge and a uncommon user gets a stack overflow by keep invoking this function? Ah yes, I didn't think of that. But the size of _Executor is fixed, isn't it? If it has a huge number of states or matches those will be on the heap anyway, in vectors/arrays. It could be huge if instantiated with a huge iterator type, as it stores three members of the iterator type, but I don't think users should be too surprised if they overflow the stack with freakishly large iterators :-) Am I still missing something? (I don't have a preference for whether to change this, but if we keep it on the heap we should add a comment, or I'll keep forgetting the rationale and try to change it again!)
Re: [patch 2/N] std::regex refactoring - sub _Executor for lookahead
On Mon, Apr 28, 2014 at 10:55 AM, Jonathan Wakely jwak...@redhat.com wrote: Ah yes, I didn't think of that. But the size of _Executor is fixed, isn't it? If it has a huge number of states or matches those will be on the heap anyway, in vectors/arrays. It could be huge if instantiated with a huge iterator type, as it stores three members of the iterator type, but I don't think users should be too surprised if they overflow the stack with freakishly large iterators :-) Am I still missing something? (I don't have a preference for whether to change this, but if we keep it on the heap we should add a comment, or I'll keep forgetting the rationale and try to change it again!) Either way is OK, in fact. Let's just keep the code simple by applying this patch. I can't imagine one could use nested lookahead. :) -- Regards, Tim Shen