Skip to content

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

Open
Sergio0694 wants to merge 12 commits intomainfrom
dev/embedded-types
Open

Emit '[Embedded]' attribute for all polyfills#127
Sergio0694 wants to merge 12 commits intomainfrom
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.

@cremor
Copy link
Copy Markdown

cremor commented Sep 1, 2025

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
Copy link
Copy Markdown

cremor commented Sep 4, 2025

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
Copy link
Copy Markdown

jviau commented Sep 11, 2025

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
Copy link
Copy Markdown

ns88ns commented Apr 1, 2026

@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.

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