Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: regexp: add Regexp.ProgramSize method similar to C++ RE2::ProgramSize #39413

Open
liangyuanpeng opened this issue Jun 5, 2020 · 9 comments
Labels
Projects
Milestone

Comments

@liangyuanpeng
Copy link

@liangyuanpeng liangyuanpeng commented Jun 5, 2020

What version of Go are you using (go version)?

go version go1.14.1 linux/amd64

Does this issue reproduce with the latest release?

Yes, checked https://pkg.go.dev/regexp?tab=doc

What operating system and processor architecture are you using (go env)?

go env amd64
$ go env

What did you do?

Checked https://pkg.go.dev/regexp?tab=doc for ProgramSize() function

What did you expect to see?

What did you see instead?

Have not function of ProgramSize()

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 5, 2020

I see that in re2 there is a method RE2::ProgramSize. I assume that this issue is a request for a similar method for regexp.Regexp. This seems simple enough to implement, though I don't know how meaningful the value is. Turning this issue into a proposal.

@ianlancetaylor ianlancetaylor changed the title Regexp have not ProgramSize() function ---(re2) proposal: regexp: add Regexp.ProgramSize method similar to C++ RE2::ProgramSize Jun 5, 2020
@gopherbot gopherbot added this to the Proposal milestone Jun 5, 2020
@gopherbot gopherbot added the Proposal label Jun 5, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Jun 5, 2020
@liangyuanpeng
Copy link
Author

@liangyuanpeng liangyuanpeng commented Jun 6, 2020

@ianlancetaylor Yes, this is what I want to say, thank you for your modification to the issue.

In my case, i need rough estimate of how complex a compiled regex is to evaluate.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 10, 2020

This seems fine. In general we're not going to add everything RE2 has to package regexp, but understanding the size of the compiled regexp is generally useful.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 17, 2020

One potential problem, on a week's reflection, is that today the regexp.Regexp package makes no mention in its API of how the search is implemented. There is always an underlying program today, but in the future maybe certain special cases won't have one. What then?

I wonder if it makes more sense for callers that need to know regexp program size to use regexp/syntax directly.

The original report does not say why this is useful, just that it was expected in the API.

@liangyuanpeng, what use did you have in mind for the program size?

@liangyuanpeng
Copy link
Author

@liangyuanpeng liangyuanpeng commented Jun 23, 2020

@rsc

Sorry for my late reply.

Just used of regex match and replace in the proxy from http request . and we need check the Regular expression complexity in application started.

One example is the (Envoy)(https://www.envoyproxy.io/docs/envoy/latest/api-v3/type/matcher/v3/regex.proto#envoy-v3-api-msg-type-matcher-v3-regexmatcher) . Envoy use c++ re2 implementation it and we use Golang to do same thing.

If i didn't make it clear, remind me please!

@junyer
Copy link
Contributor

@junyer junyer commented Jun 24, 2020

I talked to Envoy folks about this last year. [1][2] I see now that their documentation says "a rough estimate of how complex a compiled regex is to evaluate", which is wrong. The program size is a crude "cost" signal, not a complexity metric. Having it is better than not having it, but one must understand that it's only slightly more informative than the length of the pattern string.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 24, 2020

It might make sense to just use regexp/syntax directly to calculate program length. You can do that today, and it avoids adding any specific mention of implementation to package regexp.

@rsc
Copy link
Contributor

@rsc rsc commented Jul 8, 2020

Based on the discussion above, especially @junyer's comment, this sounds like a likely decline.
(If the functionality is needed in specific cases, use regexp/syntax directly.)

@rsc rsc moved this from Active to Likely Decline in Proposals Jul 8, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Jul 15, 2020

No change in consensus, so declined.

@rsc rsc moved this from Likely Decline to Declined in Proposals Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.