Use command line checksumAlgorithm in generator driver by RikkiGibson · Pull Request #81934 · dotnet/roslyn

RikkiGibson · 2026-01-08T23:37:49Z

Fixes #63039
18.3 backport: #81954

I think the SourceProductionContext.ChecksumAlgorithm, GeneratorDriverOptions.ChecksumAlgorithm, etc. we might want to make public in the future. Though, it seems like it should only really matter for scenarios where we are going to emit a PDB.

RikkiGibson · 2026-01-08T23:43:38Z

src/Compilers/Core/Portable/SourceGeneration/IncrementalContexts.cs

+            hintName,
+            ChecksumAlgorithm == SourceHashAlgorithm.None || ChecksumAlgorithm == sourceText.ChecksumAlgorithm
+                ? sourceText
+                : SourceText.From(sourceText.ToString(), encoding: sourceText.Encoding, checksumAlgorithm: ChecksumAlgorithm));


I'm unsure what to do in this case. The generator has a method it can call, passing a SourceText it already created. We want to override that and essentially ignore the ChecksumAlgorithm they specified, when we have our own that we want to use.

@tmat do you think this is a case we need to handle? What if we made it so our checksumAlgorithm is considered, when the AddSource(string, string) overload is used?

I don't think this method of making a derived text is fine. Maybe I can make a new SourceText subtype, which delegates to the underlying text, except it uses a different ChecksumAlgorithm. I'll see if that works out. #Resolved

It's not too difficult to write a class SourceTextWithAlgorithm, wrapping the original SourceText, but, it's not obvious to me which virtual methods to override and delegate to the underlying text. We wouldn't want to inadvertently create a derived text which drops the new checksum algorithm, but, we also wouldn't want to create some perf issue when the text is used.

I don't see the point with a user providing a checksum algorithm. I think we just support SourceText to have a simple text abstraction that isn't a contiguous sequence (like System.String). We should just wrap/copy, passing along the original data. But overriding with our checksum algorithm imo.

I've now made it so that when the ChecksumAlgorithm of the text they gave us, differs from what we want to use, then we just wrap theirs.

RikkiGibson · 2026-01-09T00:11:24Z

src/Compilers/Core/Portable/SourceGeneration/GeneratorContexts.cs

        /// <param name="hintName">An identifier that can be used to reference this source text, must be unique within this generator</param>
        /// <param name="source">The source code to add to the compilation</param>
-        public void AddSource(string hintName, string source) => AddSource(hintName, SourceText.From(source, Encoding.UTF8));
+        public void AddSource(string hintName, string source) => AddSource(hintName, SourceText.From(source, Encoding.UTF8, checksumAlgorithm: _checksumAlgorithm == SourceHashAlgorithm.None ? SourceHashAlgorithm.Sha1 : _checksumAlgorithm));


Note that in the None case, this is just using the default value for this parameter, which we were using implicitly before.

Is there a reason for using Sha1 here other than it's what we did before? If so I would suggest that we add SourceHashAlgorithm.Latest and use it here. Generally the rule for deprecated crypto is that we can't default to it unless it was required due to compatibility, file format specification requirements, etc ...

Consider that using SHA1 here as a default means you will run into problems on say Redhat. Or anywhere where openssl has a locked down implementation that throws on SHA1 usage.

That seems reasonable. Were you thinking of SourceHashAlgorithms.Default? Or is there a reason to introduce a distinct ‘Latest’ value in addition to that?

It feels like perhaps the SourceText.From method which had SHA1 as a default value, should also change to SourceHashAlgorithms.Default, but, that change may have more of a ripple effect.

Changed to use SourceHashAlgorithms.Default when the host doesn't provide an algorithm. Note that the command line compiler will always provide an algorithm, because it has a default value for CommandLineArguments.ChecksumAlgorithm, which is the same as SourceHashAlgorithms.Default.

src/Compilers/Core/Portable/SourceGeneration/GeneratorContexts.cs

jcouv

LGTM Thanks (commit 2) with a suggestion to consider

jcouv

LGTM Thanks (commit 3)

AlekseyTs · 2026-01-09T16:10:37Z

It looks like we have no tests on VB side. #Closed

AlekseyTs · 2026-01-09T16:10:57Z

Done with review pass (commit 4) #Closed

jaredpar · 2026-01-09T16:35:13Z

src/Compilers/Core/Portable/SourceGeneration/GeneratorContexts.cs

        /// <param name="hintName">An identifier that can be used to reference this source text, must be unique within this generator</param>
        /// <param name="source">The source code to add to the compilation</param>
-        public void AddSource(string hintName, string source) => AddSource(hintName, SourceText.From(source, Encoding.UTF8));
+        public void AddSource(string hintName, string source) => AddSource(hintName, SourceText.From(source, Encoding.UTF8, checksumAlgorithm: _checksumAlgorithm == SourceHashAlgorithm.None ? SourceHashAlgorithm.Sha1 : _checksumAlgorithm));


Is there a reason for using Sha1 here other than it's what we did before? If so I would suggest that we add SourceHashAlgorithm.Latest and use it here. Generally the rule for deprecated crypto is that we can't default to it unless it was required due to compatibility, file format specification requirements, etc ...

Consider that using SHA1 here as a default means you will run into problems on say Redhat. Or anywhere where openssl has a locked down implementation that throws on SHA1 usage.

jaredpar · 2026-01-09T16:37:04Z

src/Compilers/Core/Portable/SourceGeneration/GeneratorDriverOptions.cs

+        /// <summary>
+        /// When specified, overrides the <see cref="SourceText.ChecksumAlgorithm"/> given in <see cref="SourceProductionContext.AddSource(string, SourceText)"/>.
+        /// </summary>
+        internal SourceHashAlgorithm ChecksumAlgorithm { get; init; }


Is the use case for this testing or is there a product reason to override this value from the compiler option?

There's a product reason for this. The reason is not to override the value from the compiler option. Rather, it is to override the ChecksumAlgorithm from the SourceText that the generator created, and use the ChecksumAlgorithm given from the compiler option instead.

AlekseyTs · 2026-01-09T17:20:11Z

Would it make sense to simply throw on an input that doesn't use proper algorithm? #Closed

jaredpar · 2026-01-09T17:59:39Z

Would it make sense to simply throw on an input that doesn't use proper algorithm?

I was going to suggest that but it seems that value is coming from an init property hence no good place to put validation. If this is a test only property then I'd agree we sohuld throw / assert on bad algorithm vs. doing a fallback atthe point it's used.

Or maybe we just add an internal ctor and do validation it the constructor.

RikkiGibson · 2026-01-09T18:07:08Z

Would it make sense to simply throw on an input that doesn't use proper algorithm?

I am not sure about this. The problem is that if we ship the change, and the user starts getting that exception, they would have to either change the /checksumAlgorithm they pass to compiler, or go to the authors of generators which are not passing along the right value, and ask them to make a change to start doing so. Both of those would be problematic for users, possibly blocking adoption of the new compiler.

Generator authors will also need a clearly documented way to get the checksum to use with AddSource(string, SourceText). Today they can probably discover the right algorithm by digging through the SourceTexts of the non-generated files, but, perhaps a simpler public API which just exposes the algorithm which should be used when they create SourceTexts, would be justified.

src/Compilers/Core/Portable/Text/SourceTextWithAlgorithm.cs

RikkiGibson · 2026-01-09T21:47:09Z

It looks like we have no tests on VB side.

Still working on addressing this, I am going to just port the tests over to CommandLineTests.vb

RikkiGibson · 2026-01-10T03:05:43Z

/backport to release/dev18.3

github-actions · 2026-01-10T03:05:51Z

Started backporting to release/dev18.3 (link to workflow run)

jaredpar · 2026-01-10T16:37:15Z

The problem is that if we ship the change, and the user starts getting that exception, they would have to either change the /checksumAlgorithm they pass to compiler, or go to the authors of generators which are not passing along the right value, and ask them to make a change to start doing so

Not quite following this. The compiler already errors when you pass /checksumAlgorithm:none. Are you talking about it coming from a diff place than the command line?

RikkiGibson · 2026-01-10T17:16:55Z

Consider a command line build scenario like the following.

We pass /checksumAlgorithm:SHA256 to the compiler.
We pass /analyzer:MySourceGenerator.dll to the compiler, which contains some third-party source generator. The generator calls SourceProductionContext.AddSource("myFile", SourceText.From("class C {}")), and the default value SourceHashAlgorithms.SHA1 is passed to SourceText.From.

With the solution in this PR, the compiler will see that the text produced by the generator doesn't have the desired checksum algorithm. So it will call sourceText.WithChecksumAlgorithm(SourceHashAlgorithms.SHA256) (using the value from command line arguments) and produce a derived text which uses the desired algorithm, and include that in the compilation instead.

If we just threw when the source text coming from the generator used the wrong algorithm, then, generators which worked yesterday could start throwing today in people's builds due to checksum algorithm mismatch.

src/Compilers/Core/Portable/Text/SourceTextWithAlgorithm.cs

AlekseyTs

LGTM (commit 7), assuming that SourceTextWithAlgorithm.cs will be removed before merging

jcouv

LGTM Thanks (commit 7) modulo unnecessary file removal

RikkiGibson · 2026-01-12T19:52:40Z

/backport to release/dev18.3

github-actions · 2026-01-12T19:52:48Z

Started backporting to release/dev18.3 (link to workflow run)

jaredpar · 2026-01-12T20:40:07Z

If we just threw when the source text coming from the generator used the wrong algorithm, then, generators which worked yesterday could start throwing today in people's builds due to checksum algorithm mismatch.

Note: in terms of validation, I'm specifically referring to GeneratorContext.ChecksumAlgorithm. Essentially what do we do when that has an invalid value.

src/Compilers/Core/Portable/SourceGeneration/GeneratorContexts.cs

jaredpar · 2026-01-12T20:42:56Z

src/Compilers/Core/Portable/Text/SourceText.cs

+
+        internal SourceText WithChecksumAlgorithm(SourceHashAlgorithm checksumAlgorithm)
+        {
+            if (checksumAlgorithm == SourceHashAlgorithm.None || checksumAlgorithm == ChecksumAlgorithm)


Can you help me understand this? If this has SHA256 and the caller says WithChecksumAlgorithm(SourceHashAlgorithm.None) the return is SHA256? The caller specified a value and it wasn't used. That doesn't seem correct.

This part is more dubious. It's convenient for this method's semantics to be: use this checksum algorithm instead, if it actually is one. That is exercised in the case that the host didn't set GeneratorDriverOptions.ChecksumAlgorithm.

We can push the == None check out to the caller instead if that seems more obviously correct. Then calling WithChecksumAlgorithm(None) would throw an exception.

Note also that this is an internal API. So we can adjust its behavior in a follow-up.

Is there any other With style API where we ignore the passed in value?

Probably not, and I think that renaming the API to something like WithChecksumAlgorithmIfAny, in a follow up, may potentially be an acceptable resolution.

RikkiGibson · 2026-01-12T22:16:45Z

Note: in terms of validation, I'm specifically referring to GeneratorContext.ChecksumAlgorithm. Essentially what do we do when that has an invalid value.

@jaredpar If that property had an invalid value, then we would throw in the SourceText constructor later on.

Doing an earlier validation on the property value would be reasonable, but, it feels like it could be done in a follow up, if that's OK, so that we can get the important part of this change into 10.0.200.

Use command line checksumAlgorithm in generator driver

6a7df53

RikkiGibson requested a review from a team as a code owner January 8, 2026 23:37

github-actions bot added the Area-Compilers label Jan 8, 2026

RikkiGibson commented Jan 8, 2026

View reviewed changes

Use SourceTextWithAlgorithm

0d33ee6

RikkiGibson commented Jan 9, 2026

View reviewed changes

jcouv reviewed Jan 9, 2026

View reviewed changes

src/Compilers/Core/Portable/SourceGeneration/GeneratorContexts.cs Outdated Show resolved Hide resolved

jcouv approved these changes Jan 9, 2026

View reviewed changes

jcouv self-assigned this Jan 9, 2026

Simplify

0aaefd0

RikkiGibson requested a review from tmat January 9, 2026 00:27

jcouv approved these changes Jan 9, 2026

View reviewed changes

Fix formatting

09eb652

jaredpar reviewed Jan 9, 2026

View reviewed changes

tmat reviewed Jan 9, 2026

View reviewed changes

src/Compilers/Core/Portable/Text/SourceTextWithAlgorithm.cs Outdated Show resolved Hide resolved

Address feedback

a963f3a

RikkiGibson and others added 2 commits January 9, 2026 17:16

Fix test

4cc55d7

Add VB tests

0f2e8bd

RikkiGibson requested review from a team, jcouv and tmat January 10, 2026 01:58

github-actions bot mentioned this pull request Jan 10, 2026

[release/dev18.3] Use command line checksumAlgorithm in generator driver #81954

Merged

2 tasks

AlekseyTs reviewed Jan 12, 2026

View reviewed changes

src/Compilers/Core/Portable/Text/SourceTextWithAlgorithm.cs Outdated Show resolved Hide resolved

AlekseyTs approved these changes Jan 12, 2026

View reviewed changes

jcouv approved these changes Jan 12, 2026

View reviewed changes

Delete src/Compilers/Core/Portable/Text/SourceTextWithAlgorithm.cs

8df8603

RikkiGibson enabled auto-merge (squash) January 12, 2026 19:22

RikkiGibson disabled auto-merge January 12, 2026 19:50

jaredpar reviewed Jan 12, 2026

View reviewed changes

RikkiGibson merged commit 11129e9 into dotnet:main Jan 12, 2026
25 checks passed

dotnet-policy-service bot added this to the Next milestone Jan 12, 2026

RikkiGibson deleted the Issue63039 branch January 13, 2026 01:27

RikkiGibson mentioned this pull request Jan 13, 2026

Address remaining feedback on "Use command line checksumAlgorithm in generator driver" #81992

Merged

dotnet-bot mentioned this pull request Jan 14, 2026

[Automated] PRs inserted in VS build main-11413.139 #81999

Closed

dotnet-maestro bot mentioned this pull request Jan 14, 2026

[main] Source code updates from dotnet/roslyn dotnet/dotnet#4187

Merged

davidwengier modified the milestones: Next, 18.4 Jan 27, 2026

Conversation

RikkiGibson commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson Jan 8, 2026 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaredpar commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson commented Jan 9, 2026

Uh oh!

Uh oh!

RikkiGibson commented Jan 9, 2026

Uh oh!

RikkiGibson commented Jan 10, 2026

Uh oh!

github-actions bot commented Jan 10, 2026

Uh oh!

jaredpar commented Jan 10, 2026

Uh oh!

RikkiGibson commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AlekseyTs left a comment

Choose a reason for hiding this comment

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

RikkiGibson commented Jan 12, 2026

Uh oh!

github-actions bot commented Jan 12, 2026

Uh oh!

jaredpar commented Jan 12, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

RikkiGibson commented Jan 8, 2026 •

edited

Loading

RikkiGibson Jan 8, 2026 •

edited by jcouv

Loading

RikkiGibson Jan 9, 2026 •

edited

Loading

AlekseyTs commented Jan 9, 2026 •

edited

Loading

AlekseyTs commented Jan 9, 2026 •

edited

Loading

AlekseyTs commented Jan 9, 2026 •

edited

Loading

jaredpar commented Jan 9, 2026 •

edited

Loading

RikkiGibson commented Jan 10, 2026 •

edited

Loading

RikkiGibson Jan 12, 2026 •

edited

Loading