-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add initial regex source generator #59186
Conversation
Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions Issue DetailsThis adds the [RegexGenerator("abc")]
private static partial Regex Abc(); it will generate C# code to implement It includes a test project for the generator focused on testing the mechanics of the generator as well as integrating the source generator into the regex test suite, rolled out into many but not all of the tests so far. There are a bunch of follow-ups to this for while I'll be opening issues, including rolling this out through the rest of the tests (and finding ways to make it go faster, as it's currently significantly increasing the time for the regex test suite), fixing a bunch of TODOs (e.g. figuring out what to do about culture), and improving perf further (it's now really easy to see opportunities for improved code generation). Fixes #58880 cc: @danmoseley, @pgovind, @tannergooding, @eerhardt, @veanes, @olsaarik
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
@jaredpar, FYI. |
...pressions/tests/System.Text.RegularExpressions.Generators.Tests/RegexGeneratorParserTests.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCode.cs
Show resolved
Hide resolved
...ibraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCharClass.cs
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,318 @@ | |||
<?xml version="1.0" encoding="utf-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we not reuse the existing resx?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How? i.e. is there a mechanism for including an additional resx file and having them merged, or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant just use the existing one - of course not all the strings apply. Perhaps that would get complex given this one is localized.
No I don't know a way to include one in another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant just use the existing one
And add the generator specific strings into the existing one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's less wasteful than the duplication. Feel free to ignore.
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexNode.cs
Outdated
Show resolved
Hide resolved
Will actually be interesting now we can look at the C# generated for different regexes to more easily see how it works. |
7993493
to
771edaf
Compare
Will there be any warning if that library code (supposed to be language agnostic?) would be called NOT from Roslyn? |
src/libraries/System.Text.RegularExpressions/tests/Regex.Tests.Common.cs
Outdated
Show resolved
Hide resolved
I don't understand the question. |
src/libraries/System.Text.RegularExpressions/gen/DiagnosticDescriptors.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs
Outdated
Show resolved
Hide resolved
a26947e
to
d79f58d
Compare
src/libraries/System.Text.RegularExpressions/gen/DiagnosticDescriptors.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
Outdated
Show resolved
Hide resolved
d79f58d
to
8c9ef4f
Compare
Adds the new RegexGenerator attribute that's a signal to the regex generator to generate code for the specified regex.
Add a source generator for generating C# code for Regex. This is primarily a port of RegexCompiler.cs, generating C# code instead of MSIL.
Adds tests dedicated to the mechanics of the source generator, e.g. that appropriate diagnostics are issued for improper use of RegexGenerator.
Start integrating the source generator into the regex test suite, so that many existing tests also validate the generated code.
Changing the generator to not collect all regexes together means we don't need to reprocess/regenerate all regexes every time any one of them is changed.
To better test the appropriate ctor usage.
Also fixed one place where the IL we were generating wasn't as good as the reflection emit code.
Also clean up generated code in a few places to make it more readable / concise.
8c9ef4f
to
e153b3a
Compare
This adds the
[RegexGenerator(...)]
attribute and a new System.Text.RegularExpressions.Generator source generator for Regex. Given a method like:it will generate C# code to implement
Abc()
that returns aRegex
instance with code like you'd have gotten withRegexOptions.Compiled
, but generated at build time in C# rather than at run time with reflection emit in MSIL. The generator is largely a port of RegexCompiler.cs; ideally we wouldn't duplicate all of the emit logic, once for MSIL and once for C#, and we should subsequently look for ways to avoid this duplication. The generator includes source from System.Text.RegularExpressions, in particular in support of parsing and analyzing regexes.It includes a test project for the generator focused on testing the mechanics of the generator as well as integrating the source generator into the regex test suite, rolled out into many but not all of the tests so far.
There are a bunch of follow-ups to this for while I'll be opening issues, including rolling this out through the rest of the tests (and finding ways to make it go faster, as it's currently significantly increasing the time for the regex test suite), fixing a bunch of TODOs (e.g. figuring out what to do about culture), and improving perf further (it's now really easy to see opportunities for improved code generation).
Fixes #58880
Contributes to #44676
cc: @danmoseley, @pgovind, @tannergooding, @eerhardt, @veanes, @olsaarik