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

ILLink analyzer recognizes && and ||, but neither trimmer or ilc do #102830

Open
jtschuster opened this issue May 29, 2024 · 2 comments
Open

ILLink analyzer recognizes && and ||, but neither trimmer or ilc do #102830

jtschuster opened this issue May 29, 2024 · 2 comments
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Milestone

Comments

@jtschuster
Copy link
Member

The trim analyzer recognizes and understands && and || and will not warn for code that is guarded by a feature guard, but the trimmer and ilc don't understand it and will not eliminate the branch with problematic code and will warn if there is code that should be considered guarded.

This is expected according to the feature-checks docs: https://github.com/dotnet/runtime/blob/main/docs/design/tools/illink/feature-checks.md#boolean-andor

If neither tool recognizes the pattern and will warn, does it make sense for the analyzer to not warn and leave the user surprised at publish?

@jtschuster jtschuster added the area-Tools-ILLink .NET linker development as well as trimming analyzers label May 29, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 29, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

@sbomer
Copy link
Member

sbomer commented May 29, 2024

The analyzer behavior fell out of the control-flow graph representation. It's one of those cases where the analyzer would have to do extra work to not recognize that pattern, but the other tools would have to do extra work to recognize it. Personally I'm more inclined to teach the other tools to recognize it.

But I also agree that ideally the analyzer would match the other tools. I haven't looked into it yet, but if it's simple enough to make the analyzer not recognize the pattern I would be supportive.

@agocke agocke added this to the Future milestone Jun 20, 2024
@agocke agocke removed the untriaged New issue has not been triaged by the area owner label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
Status: No status
Development

No branches or pull requests

3 participants