-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Invoke Created action from HotReloadException constructor #79790
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
Conversation
8cd8591 to
4b4910b
Compare
b718dca to
cc0f523
Compare
|
@dotnet/roslyn-compiler ptal |
src/Compilers/CSharp/Portable/Emitter/EditAndContinue/PEDeltaAssemblyBuilder.cs
Show resolved
Hide resolved
|
Done with review pass (commit 2) #Closed |
| new SynthesizedHotReloadExceptionConstructorSymbol(this, stringType, intType), | ||
| new SynthesizedFieldSymbol(this, intType, CodeFieldName, DeclarationModifiers.Public, isReadOnly: true, isStatic: false) | ||
| new SynthesizedFieldSymbol(this, intType, CodeFieldName, DeclarationModifiers.Public, isReadOnly: true, isStatic: false), | ||
| new SynthesizedFieldSymbol(this, actionOfTType.Construct(exceptionType), CreatedActionFieldName, DeclarationModifiers.Private, isReadOnly: false, isStatic: true) |
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.
actionOfTType.Construct(exceptionType), CreatedActionFieldName
Is it intentional that we don't care about constraints (in a scenario with a custom Action type)? We generally check constraints whenever we construct symbols, but we may have an exception for types that are expected to be defined in the BCL (in System namespace) where we may make certain assumptions. I'm not sure
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.
Discussed offline
...pilers/CSharp/Portable/Symbols/Synthesized/SynthesizedHotReloadExceptionConstructorSymbol.cs
Show resolved
Hide resolved
...pilers/CSharp/Portable/Symbols/Synthesized/SynthesizedHotReloadExceptionConstructorSymbol.cs
Show resolved
Hide resolved
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.
Done with review pass (commit 3)
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.
LGTM (commit 3)
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.
LGTM Thanks (commit 3)
|
/backport to release/dev18.0 |
|
Started backporting to release/dev18.0: https://github.com/dotnet/roslyn/actions/runs/18015545762 |
…ception constructor (#80463) Backport of #79790 to release/dev18.0 /cc @tmat ## Customer Impact ## Regression - [ ] Yes - [x] No [If yes, specify when the regression was introduced. Provide the PR or commit if known.] ## Testing [How was the fix verified? How was the issue missed previously? What tests were added?] ## Risk [High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.]
Emits field
private static Action<HotReloadException> Createdto synthesizedHotReloadExceptiontype and addsCreated?.Invoke(this)to its constructor.This allows Hot Reload agent injected in the application process to hook up a handler that notifies the host when a runtime rude edit occurs.
The host may decide to restart the application at that point (
HotReloadAutoRestartsetting is on).Contributes to dotnet/sdk#48515.