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

Use static abstract interface methods in SymbolicRegexMatcher #61631

Closed
wants to merge 1 commit into from

Conversation

stephentoub
Copy link
Member

@tannergooding, @davidwrighton, any reason not to start relying on static abstract interface methods?

cc: @joperezr, @olsaarik

@ghost
Copy link

ghost commented Nov 15, 2021

Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

@tannergooding, @davidwrighton, any reason not to start relying on static abstract interface methods?

cc: @joperezr, @olsaarik

Author: stephentoub
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: -

@tannergooding
Copy link
Member

@stephentoub, still waiting on some Mono and Crossgen2 fixes before we can: #60905 (comment)

CC. @lambdageek, @akoeplinger

@tannergooding tannergooding added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 15, 2021
@stephentoub
Copy link
Member Author

Thanks, Tanner. Presumably CI will fall over with this then? 😄

@stephentoub
Copy link
Member Author

still waiting on some Mono and Crossgen2 fixes before we can

From @lambdageek's comments on the other issue, it seems like this isn't a problem as it relates to this PR (which doesn't try to take the address of such a method). What about crossgen2? CI passed; does that mean we're not crossgen'ing these assemblies as part of this build, or that any such use will cause crossgen to fail and we simply then always JIT that assembly, or something else?

@tannergooding
Copy link
Member

What about crossgen2

@jkotas ?

@jkotas
Copy link
Member

jkotas commented Nov 17, 2021

does that mean we're not crossgen'ing these assemblies as part of this build,

Right, libraries CI tests are not crossgening anything under libraries.

I expect that we will see outer loop crossgen legs failing at least. We may see other failure modes as well, not sure.

I think we really need to finish the crossgen support before we start using this feature in libraries.

@stephentoub
Copy link
Member Author

I think we really need to finish the crossgen support before we start using this feature in libraries.

That's fine. Do we know when that will happen? If soon, I'll leave this open. Otherwise, I'll close it.

@stephentoub
Copy link
Member Author

/azp list

@stephentoub
Copy link
Member Author

/azp run runtime-coreclr crossgen2 outerloop

@dotnet dotnet deleted a comment from azure-pipelines bot Nov 17, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@joperezr joperezr self-assigned this Nov 29, 2021
@joperezr joperezr added the needs-author-action An issue or pull request that requires more info or actions from the author. label Nov 29, 2021
@stephentoub
Copy link
Member Author

This isn't actionable right now. Will revisit once crossgen supports it.

@stephentoub stephentoub deleted the srmstaticinterface branch January 9, 2022 03:02
@ghost ghost locked as resolved and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.RegularExpressions needs-author-action An issue or pull request that requires more info or actions from the author. NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants