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

Primary constructor properties have sequence points but breakpoints can't be placed #63299

Closed
Tracked by #68477
tmat opened this issue Aug 10, 2022 · 4 comments
Closed
Tracked by #68477

Comments

@tmat
Copy link
Member

tmat commented Aug 10, 2022

Currently the emitted metadata/pdb for primary constructors are incompatible with the implementation of several IDE features (breakpoint placement, EnC, code coverage):

  1. The synthesized constructor assigns parameters to backing fields directly.
    A sequence point is emitted for the entire constructor and additional sequence point is emitted for each backing field assignment.

Stepping into new C(1,2) makes 3 steps in the constructor:
Step 1:
image
Step 2:
image
Step 3:
image

If the record declaration has a body the entire declaration is active, which is odd:
image

  1. The accessors generated for properties specified in primary constructors have sequence points and are not marked by [CompilerGenerated] attribute. When debugging the debugger allows to step into to property accessor and the active statement (yellow span) is displayed over the parameter syntax (note: the user may have stepping into properties disabled:
    image).
    This causes code coverage tools to report uncovered code (see https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1567458).

  2. The IDE does not allow placing breakpoints on the syntax of the parameters.

I think we should consider:

  • Instead of assigning to the backing field the constructor emits call to the property setter.
    This would also fix https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1567458.
  • Make the constructor non-user code (remove all sequence points emitted to the constructor) and mark it [CompilerGenerated].
  • Allow placing breakpoint on any primary constructor parameter in the IDE. Two breakpoints would be placed: onto the getter and onto the setter.
  • Fix EnC to account for these breakpoints. Edits inserting whitespace into the primary constructor declaration, or code around the entire declaration are currently broken. E.g.
    image

Related:
dotnet/csharplang#5999

UPDATE

Current plan:

  1. remove sequence points from primary ctor body. Keep a single sequence point for implicit/explicit base initializer call.
  2. keep sequence point in getter accessor
  3. remove sequence point from init accessor (unnecessary since we have [1] and [4]) We keep the sequence points in both accessors. The debugger does not step into properties by default, so unless the user opts into doing so or places explicit breakpoint on the property these sequence points will have no effect.
  4. add sequence point to copy-ctor at the end

The span for the copy-ctor sequence point would cover the type name, but not parameters (since they are not "in-scope" within copy-ctor).

record class [|C<T>|](int X, int Y); // copy ctor span
record class [|C<T>(int X, int Y)|]; // primary ctor span

A breakpoint placed on the type name would bind to both copy and primary ctors.

  1. test coverage should ignore sequence points in compiler generated code
@tmat
Copy link
Member Author

tmat commented Aug 10, 2022

@davidwengier FYI

@RikkiGibson
Copy link
Contributor

What's the effect of placing CompilerGenerated on the constructor? It prevents the debugger from stepping in?

@tmat
Copy link
Member Author

tmat commented Jun 15, 2023

What's the effect of placing CompilerGenerated on the constructor? It prevents the debugger from stepping in?

CompilerGenerated has no effect on debugging.

@tmat tmat changed the title Primary constructor (properties) have sequence points but breakpoints can't be placed Primary constructor properties have sequence points but breakpoints can't be placed Jun 22, 2023
@tmat
Copy link
Member Author

tmat commented Jul 23, 2023

Fixed by #68765

@tmat tmat closed this as completed Jul 23, 2023
@tmat tmat added this to the 17.8 P1 milestone Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants