Skip to content

Emit '[Embedded]' attribute for all polyfills#127

Merged
Sergio0694 merged 12 commits into
mainfrom
dev/embedded-types
May 25, 2026
Merged

Emit '[Embedded]' attribute for all polyfills#127
Sergio0694 merged 12 commits into
mainfrom
dev/embedded-types

Conversation

@Sergio0694

Copy link
Copy Markdown
Owner

Closes #50, #103

Description

This PR updates the generator to also emit the [Embedded] attribute for polyfills, to avoid conflicts for internal types.

Comment thread src/PolySharp.SourceGenerators/PolyfillsGenerator.Polyfills.cs Outdated
@cremor

cremor commented Sep 1, 2025

Copy link
Copy Markdown

Would this work if you don't have the latest SDK or VS installed? Because according to this you couldn't declare the EmbeddedAttribute yourself if the compiler would have created one prior to VS 17.13. See also dotnet/roslyn#76523

Also, just to have it documented here too: @MarkKharitonov found a problem with this solution, see #50 (comment)

@Sergio0694

Copy link
Copy Markdown
Owner Author

@cremor yeah this will work fine, I'm still using Roslyn 4.3.0, and not that new API that was added 🙂

@cremor

cremor commented Sep 4, 2025

Copy link
Copy Markdown

I didn't mean the new API. I meant that quote from the PR dotnet/roslyn#76523:

Today, [...] if we need to generate it, we error if the user defines one.

I understand this as:

If you use a compiler version before VS 17.13
and
you define EmbeddedAttribute yourself (e.g. by using PolySharp with this new changes)
and
you have something in your code that would lead Roslyn to emit the EmbeddedAttribute itself

then you would get an error.

I haven't tested this. I just wanted to raise awareness.

@jviau

jviau commented Sep 11, 2025

Copy link
Copy Markdown

Small suggestion. Recent Roslyn APIs have an convenience API .AddEmbeddedAttributeDefinition(), which will emit the EmbeddedAttribute type for you. As opposed to including it directly like you are in this PR. If you make including of the [Embedded] marker on all the types opt-in via an MSBuild variable, then you can also opt in having the EmbeddedAttribute at all via conditionally calling that API.

You could re-create this helper if you are not on the newer Roslyn packages: https://github.com/dotnet/roslyn/blob/51d2ed9e47364b57024f7715c2e2b2e385fe1454/src/Compilers/Core/Portable/SourceGeneration/IncrementalContexts.cs#L144

Introduces a new MSBuild property 'PolySharpUseEmbeddedAttributeForGeneratedTypes' and corresponding generation option to control whether the [Embedded] attribute is emitted and applied to generated types. Refactors the generator to emit the EmbeddedAttribute only when this option is enabled, improving flexibility and reducing unnecessary code emission.
Document and expose the new PolySharpUseEmbeddedAttributeForGeneratedTypes property in both READMEs and PolySharp.targets. This option allows generated polyfill types to be marked with the [Embedded] attribute.
Removed IsPublicAccessibilityRequired from GeneratedType and moved accessibility and embedded attribute handling to SyntaxFixupType flags. Updated PolyfillsGenerator to use new flags for source adjustments, improving flexibility and maintainability of syntax fixup logic.
Introduces a new overload of GetBoolMSBuildProperty in AnalyzerConfigOptionsProviderExtensions to allow specifying a default value if the property is missing or empty. Updates PolyfillsGenerator to use this overload for UseEmbeddedAttributeForGeneratedTypes, defaulting to true.
@Youssef1313

Copy link
Copy Markdown
Contributor

@Sergio0694 Is there anything remaining here to get this completed?

@ns88ns

ns88ns commented Apr 1, 2026

Copy link
Copy Markdown

@Sergio0694 - [reposted from #50]

Seems the solution is still problematic.

Although it resolves #50 and #103, it introduces another problem:
I tested the dev/embedded-types and what I found:

The same setup: one unit-test project consumes internals from several other library projects in the same solution.

All projects use PolySharp, the unit-test project also uses it (otherwise, what's the sense).
All projects are configured with:

  • PolySharpUsePublicAccessibilityForGeneratedTypes: false
  • PolySharpIncludeRuntimeSupportedAttributes: true
  • PolySharpUseEmbeddedAttributeForGeneratedTypes: true

The library projects have the [assembly: InternalsVisibleTo()]

Polyfills are working well in the lib projects themselves, but aren't generated in the unit-test project. Instead, it produces:

  • CS0122: 'DynamicallyAccessedMemberTypes' is inaccessible due to its protection level (once per per dependency);

In addition, if any polyfills are used in the unit-test project:

  • CS0246: The type or namespace name '...' could not be found (are you missing a using directive or an assembly reference?)
  • CS0518: Predefined type '...' is not defined or imported
  • CS0656: Missing compiler required member '.....ctor'

Please, find the minimal repro solution attached.
_test_02.zip

The archive contains the solution test_02.slnx for VS2022 with 3 projects targeted to .net481.
All 3 projects use the PolySharp. The t.test.csproj exposes the problem on build.

To reproduce the issue:

  1. unpack the solution;
  2. change the PackageReference Include="PolySharp" in the Directory.Build.props to use manually built dev/embedded-types;
  3. build the solution with any predefined configuration. The build fails on the t.test.csproj;

Regards.

@Sergio0694

Copy link
Copy Markdown
Owner Author

@ns88ns that should be fixed by #134.

@Youssef1313 just me finding some time to review and merge everything 😄

@Sergio0694 Sergio0694 merged commit 5b844c2 into main May 25, 2026
9 checks passed
@Sergio0694 Sergio0694 deleted the dev/embedded-types branch May 25, 2026 17:54
@Sergio0694 Sergio0694 restored the dev/embedded-types branch May 25, 2026 17:55
@Sergio0694 Sergio0694 deleted the dev/embedded-types branch May 25, 2026 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InternalsVisibleTo from two other projects leads to not being able to use PolySharp

5 participants