You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2025/01/12 05:34:31 UTC
[PR] Remove unnecessary NoInlining, #931 [lucenenet]
paulirwin opened a new pull request, #1099:
URL: https://github.com/apache/lucenenet/pull/1099
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/main/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [X] You've included unit or integration tests for your change, where applicable.
- [X] You've included inline docs for your change, where applicable.
- [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Remove unnecessary use of `[MethodImpl(MethodImplOptions.NoInlining)]`
Fixes #931
## Description
I need to finish reviewing the methods mentioned in calls to `StackTraceHelper.DoesStackTraceContainMethod(string methodName)` (the overload without the class name parameter) to make sure there aren't any false positives there, but I've at least validated that those existing usages of NoInlining _seem_ like they might plausibly match their use in the tests.
This PR removes NoInlining from interface and abstract methods, where it has no effect because the attribute is not inherited. It also removes it from methods with empty bodies, because an empty body doesn't call anything, and thus those checks in the tests would never be true anyways. Finally, it removes it in a few other places that were not asserted in tests.
As a TODO, I left the SystemConsole attributes there, even though those are suspect. We had discussed possibly being able to remove that class anyways, but I'll defer that for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove unnecessary NoInlining, #931 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1099:
URL: https://github.com/apache/lucenenet/pull/1099#issuecomment-2585807978
I'll go ahead and split out the remaining investigation work into its own PR, and merge this one as it's a nice seam.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove unnecessary NoInlining, #931 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1099:
URL: https://github.com/apache/lucenenet/pull/1099
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove unnecessary NoInlining, #931 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on PR #1099:
URL: https://github.com/apache/lucenenet/pull/1099#issuecomment-2585639989
Oh, I didn't notice it is still a draft. Feel free to re-request a review when the PR is ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org