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

Add a guarding code which prevents putting Compilation into ISG pipeline. #71629

Open
lsoft opened this issue Jan 13, 2024 · 3 comments
Open

Comments

@lsoft
Copy link

lsoft commented Jan 13, 2024

Due to widespread problem with having Compilation inside of ISG pipeline:

I would suggest to add into ISG pipeline builder a special checks (guard) for incorrect types and etc. If checks fired, then exception has been fired and the ISG must be disabled.

In general, any ISG developer should see a bug during development and well before publishing the ISG. But for ISGs that are already published, a forced shutdown will have benefits in the long run.

If a "general" guard (against all incompatible types) is not possible, a special guard against Compilation will be useful too.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 13, 2024
@jaredpar
Copy link
Member

I would suggest to add into ISG pipeline builder a special checks (guard) for incorrect types and etc

I'm unsure what you mean by this. Particularly unsure what incorrect types mean here. Can you try elaborating a bit?

@lsoft
Copy link
Author

lsoft commented Jan 20, 2024

@jaredpar

I'm not an expert in ISG building, so I may say something stupid, but let's take a look to the following code snippet:

IncrementalValuesProvider<MyClass> myClasses = context.SyntaxProvider.CreateSyntaxProvider(...);

...

public class MyClass
{
     public string FoundClassName;
}

MyClass is not overrides GetHashCode and Equals and will be checked by reference. If I understand the ISG pipeline correctly, it means caching will be powerless for this step of ISG pipeline. So, is it possible for types like IncrementalValuesProvider<T> detect if T do not override (implement) GetHashCode and Equals?

I understand there is no silver bullet to exclude every "incorrect" uses of ISG pipelines, but any additional guard (even against overridden GetHashCode + Equals, or against predefined forbidden types like Compilation) will be good for ecosystem.

did I answer to your question?

@jaredpar
Copy link
Member

Essentially warning when an incremental generator is not using equitable types in value providers. That is something we have discussed a few times. Usually though our emphasis is on record types used here which are not properly equitable (generally because they included an array field).

This is very much on our mind for future SG areas.

@jaredpar jaredpar added this to the .NET 9 milestone Jan 25, 2024
@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Jan 25, 2024
@jaredpar jaredpar modified the milestones: .NET 9, Backlog Jul 11, 2024
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

3 participants