search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.13k stars 4.04k forks source link

Break points in source generated files don't bind. Source code is different from original version #74603

Open vsfeedback opened 4 months ago

vsfeedback commented 4 months ago

This issue has been moved from a ticket on Developer Community.

[severity:It's more difficult to complete my work] In my SG I'm producing a list of using directives. I'm sorting the string list using stringList.OrderBy(s => s). On my machine this work just fine, but some colleges reported that they couldn't put break points in the generated files with the error message that source files would differ from the original. Comparing the code shown in VS and the code from the compiled assembly reveled that in deed the order was different.

I was able to work around the problem by changing my SG to use stringList.OrderBy(s => s, StringComparer.Ordinal). However I expect the IDE to always show the code that actually gets compiled and for break points in the generated code to just work.

I'm guessing the issue comes down to the IDE using the wrong culture while generating the code, i.e. not the one that is used during compilation. Im using a custom culture on my machine while most of my colleges use the standard German culture. We're on VS 17.10.4.

Original Comments

Lisa Gao (CSI Interfusion Inc) [MSFT] on 7/21/2024, 07:10 PM:

(private comment, text removed)

Carsten Dierig on 7/22/2024, 02:50 AM:

(private comment, text removed)

Feedback Bot on 7/22/2024, 06:08 PM:

(private comment, text removed)

Feedback Bot on 7/23/2024, 07:51 AM:

(private comment, text removed)

Original Solutions

(no solutions)

dotnet-issue-labeler[bot] commented 4 months ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

dotnet-issue-labeler[bot] commented 4 months ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

CyrusNajmabadi commented 4 months ago

@chsienki @jaredpar @jasonmalinowski There's a lot to unpack here. First, what is our recomendation on how people should write generators that are affected by ordering. My feeling (and def disagree if you think differently) is that things should be written to be stable/deterministic across culture. So part of me feels like we should be advising that people use things like .Ordinal for ordering htings.

If that's the case, we can partially chalk this up to a generator not written with best practices.

However, if that's not the case (or is the case, but we want to make the experience better) we possibly want the IDE and compiler to agree on culture when running generators. So my question would be: How does the compiler determine what culture to run under? We possibly want to try to do the same in the IDE. I say 'possibly' as i imagine that we'd have to set the culture for our entire OOP process. And that might have significant impact in other ways (like diagnostic messages and the like). That said, maybe that's an argument for unifyign these so that your analyzer messages from the build match those from OOP.

Anyways, three major questions:

  1. what is best practices for the generator author? Have the generator be culture-dependent, or culture-independent? Chris/Jared to answer.
  2. how does the compiler server setup its culture/locale? Chris/Jared to answer.
  3. should we make OOP match the compiler server? Cyrus/Jason to investigate.
chsienki commented 4 months ago

I think yes, we should be advising customers to write generators in a culture independent way if possible.

Cultures are going to vary across usages of the generator, especially when you consider things like CI, and not using an invariant culture is introducing a source of non-determinism which you can't easily test for.

I'll wait for @jaredpar to chime in on the compiler server question, because he knows the most about it, but AFAIK it's just run under the default culture.

jasonmalinowski commented 4 months ago

I believe there are thread-local ways that a culture can be set on a specific thread; that way we could set some well-known culture before we're going to invoke a generator/analyzer, and then switch it back once we're done. For example, see the code we have here and the documentation discussing CurrentCulture here.

jaredpar commented 4 months ago

I think yes, we should be advising customers to write generators in a culture independent way if possible.

Generators are an extension of the compiler and need to match its principals in many cases. This is one of them. The output of the compiler is independent of the current culture and generators need to match that behavior.

what is best practices for the generator author? Have the generator be culture-dependent, or culture-independent? Chris/Jared to answer.

Generators need to be culture independent in their outputs. That is true even for items like string localization. The compiler does support localizing error messages but that is not based on machine culture. Instead it's based on the compiler option for how messages are localized.

If that's the case, we can partially chalk this up to a generator not written with best practices.

Agree this seems like a generator authoring error.

I'll wait for @jaredpar to chime in on the compiler server question, because he knows the most about it, but AFAIK it's just run under the default culture.

Nothing special is done for the compiler server. It just inherits settings from the machine (just as csc would).