Enhance match struct with the matched_string field

[ketkarameya]

Add the field matched_string to Match struct. This field will store the code snippet matched by the rule.

[lazaroclapp]

Makes sense to me.

That said, question: Are these substrings eager copies in memory or are they stored some other way? Any concern about memory usage ballooning due to multiple copies of substrings of the code under analysis? Specially under, say, a long chain of simplifications on the same huge boolean expression? (Seems unlikely, but still, one alternative is to keep just the start and end position and a pointer to the full source - or even the file, to be read on demand and cached - and then just constructing the substring when requested through a method of the match struct, no?)

[ketkarameya]

I totally agree to what u say. I think we should not do this.
I was leaning towards usability over performance. But i feel this will get really bad when we capture these large constructs like class or file.
With storing references comes lifetimes :|

[ketkarameya]

Another problem is that, we are using this Match struct for our python bindings too.
And pyo3 library doesnt support struct with lifetimes.

[ketkarameya]

We actually require this matched_string for debugging :|
But I guess, let's not do that!

[dvmarcilio]

In the case of edits/rewrites, I wonder what would happen if there is no matched string.
I may be misunderstanding, but we might not be able to get the original match content if rewrites (e.g., delete a function) happen.

[ketkarameya]

yes. And Diego makes a good point too. If we dont do this eagerly we will not be able to report it at all.

[ketkarameya]

I think I am gonna merge this. It might have performance implications (in memory) but I guess its alright. Neither do we have any benchmarks to empirically prove how bad it is. @lazaroclapp what do u think ? (I know u have already accepted the PR)

[lazaroclapp]

I think I am gonna merge this. It might have performance implications (in memory) but I guess its alright. Neither do we have any benchmarks to empirically prove how bad it is. @lazaroclapp what do u think ? (I know u have already accepted the PR)

I think this makes sense to me, given the complications involved in designing something that would work well with deletions (I am sure there is some involved copy-on-write way to handle it, but it feels like premature optimization). Separate from this PR, a good question is whether we have any way to measure internally Piranha's memory usage. But, also, I wouldn't worry too much about it unless we start seeing issues, since it's not like Piranha is running during a standard build or anything like that. It is very much an "off main pipeline" tool 🙂

[ketkarameya]

Thanks a lot. I will merge this PR.
Filed the issue #322 to track this.