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

Analyzer parity with linker warnings #2273

Open
17 of 18 tasks
Tracked by #101149
agocke opened this issue Sep 14, 2021 · 2 comments
Open
17 of 18 tasks
Tracked by #101149

Analyzer parity with linker warnings #2273

agocke opened this issue Sep 14, 2021 · 2 comments

Comments

@agocke
Copy link
Member

agocke commented Sep 14, 2021

In short, unless the analysis requires IL analysis, the linker and the analyzer should produce the same warnings, and the analyzer should produce a strict subset of the total linker warnings.

At the high level, we need to confirm this as best we can using some complex repos. We should turn on the analyzer and verify that no more warnings are produced and that similar value is provided by the analyzer as by the linker. Repos:

  • dotnet/runtime
  • dotnet/aspnetcore

See #2580 for issues that additionally block integration of the feature branch.

I think these are the analyzers we are currently missing, together with the related warnings:

  • XmlAnalyzer: IL2001, IL2002, IL2007-IL2025, IL2029-IL2031, IL2038-IL2040, IL2044, IL2045, IL2048, IL2049, IL2051-IL2054

There are some that fit into analyzers we already have:
- [ ] IL2027: Multiple instances of a Requires* attribute (fits into Requires*Analyzer),

  • @tlakollo noted: The only way we generate IL2027 is when we have RUC specified in an xml file and RUC specified in the member, so I think is a warning that I would put in the xml analyzer side.
  • IL2041: DAM is not allowed on methods (fits into DAMAnalyzer),
    - [ ] IL2042: Cannot determine backing field of a property (fits into DAMAnalyzer),
  • IL2043: DAM on property conflicts with the same attribute on its accessor method (fits into DAMAnalyzer),
  • IL2050: Pinvoke method declares a parameter with COM marshalling (we must add an analyzer for this, although I don't see anything particularly difficult for supporting this warning),
    - [ ] IL2056: Annotated a property which has its backing field already annotated (fits into DAMAnalyzer)

Finally, there are issues where we know the linker behavior needs to be fixed, and the analyzer work should be done in parallel:
- [ ] #2158

Compiler generated backing fields
- [ ] #2628

Arrays
- [ ] foreach over arrays is complicated and analyzer doesn't see the array access -> the iterator variable is evaluated as TopValue which means it won't cause any warnings. Unlike linker which will see it as unannotated value which will cause warnings.

@agocke
Copy link
Member Author

agocke commented Sep 15, 2021

fyi @sbomer @mateoatr @tlakollo Just copied this over from your email, looks really good

@sbomer
Copy link
Member

sbomer commented Sep 29, 2021

Copying @mateoatr's notes about analyzer parity for warnings IL2001 through IL2060 (feel free to adjust):

edit: removing them here since they are copied in the original issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants