Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Interceptors open issues for C# 13 #7835
Interceptors open issues for C# 13 #7835
Changes from 3 commits
773b58d
5cbf767
3542f3d
e03b1b1
d17f6fb
5f7f69e
bfff8d8
23fcba7
208b4e6
e6e6a7d
66f39b8
292a8bd
e30a998
bdab7e9
6b276f9
6c5831b
6da3e45
f9f442e
05a2bf9
3c62594
abd391f
e395798
22dbc9d
da785d4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is assuming that as a generator author my
AddSource
calls look like this:If so, can we make it implicit so that I can just write:
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.
Yes my intent here matches what you're saying. AddSource will always take a hintName, not an absolute path. Also, it's feeling unclear to me now if
GetFilePath()
API is worth it, if we also have to addGetInterceptsFilePath()
or similar API anyway.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.
Some feedback I got includes:
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.
Another potential is we could both
InterceptsLocation
is opaqueEssentially tell customers
That would require using the API I suggested above but taking a
SyntaxNode
(thus has line / column + path) instead ofSyntaxTree
.I'm not saying we should do this but it is an item to consider.
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.
Should a section on dotnet/roslyn#68218 be included somewhere in the doc?
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.
Yes, and it will also be useful to get a sense of how urgent it is to get it done in C# 13.
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.
BTW, it feels like the real kicker here is not just that we want generics support (through something more sophisticated like a unification approach), but that we actually want the interceptor to be able to have a more specific signature than the original method. e.g. in this case where in order for type parameters to propagate through the system, the interceptor parameter needs to be
Func<TypeParam>
even though the original parameter type isDelegate
.