Add ability for the analyzer to recognize const fields #2774
Conversation
This means the value of the fields is tracked as a const value instead of a field reference. This is to support some additional code patterns as well as align the behavior with the linker. Compiler will inline the const fields into the IL effectively removing the field refernce in these cases and linker only sees the constant. So ideally the analyzer should have a similar behavior.
| return true; | ||
| } else if (operation.Type?.SpecialType == SpecialType.System_Boolean && constantValue is bool boolConstantValue) { | ||
| constValue = new ConstIntValue (boolConstantValue ? 1 : 0); | ||
| return true; |
There was a problem hiding this comment.
Why only these types rather than any primitive that can be const?
There was a problem hiding this comment.
Because only these are recognized by the rest of trimming analysis. And also because we only have value system representation for these (for example ConstInstValue... we would need to introduce similar types for other values like DateTime, double, ...)
There was a problem hiding this comment.
I think we should be tracking other integer types though - for example:
class Program {
static void Main() {
uint i = 0u;
var t_arr = new Type[10];
t_arr[i] = typeof(C);
t_arr[i].GetMethods();
}
}
class C {
[RUC("RUC")]
public void R() {}
}Analyzer:
warning IL2065: Value passed to implicit 'this' parameter of method 'System.Type.GetMethods()' can not be statically determined and may not meet 'DynamicallyAccessedMembersAttribute' requirements.
Linker:
Trim analysis warning IL2026: Program.Main(): Using member 'C.R()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. RUC.
I think it would be fine to do as a separate change so we can unblock runtime though.
There was a problem hiding this comment.
I'll look into this in a followup change
| return true; | ||
| } else if (operation.Type?.SpecialType == SpecialType.System_Boolean && constantValue is bool boolConstantValue) { | ||
| constValue = new ConstIntValue (boolConstantValue ? 1 : 0); | ||
| return true; |
There was a problem hiding this comment.
I think we should be tracking other integer types though - for example:
class Program {
static void Main() {
uint i = 0u;
var t_arr = new Type[10];
t_arr[i] = typeof(C);
t_arr[i].GetMethods();
}
}
class C {
[RUC("RUC")]
public void R() {}
}Analyzer:
warning IL2065: Value passed to implicit 'this' parameter of method 'System.Type.GetMethods()' can not be statically determined and may not meet 'DynamicallyAccessedMembersAttribute' requirements.
Linker:
Trim analysis warning IL2026: Program.Main(): Using member 'C.R()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. RUC.
I think it would be fine to do as a separate change so we can unblock runtime though.
…#2774) This means the value of the fields is tracked as a const value instead of a field reference. This is to support some additional code patterns as well as align the behavior with the linker. Compiler will inline the const fields into the IL effectively removing the field refernce in these cases and linker only sees the constant. So ideally the analyzer should have a similar behavior. Commit migrated from dotnet/linker@05a3b65
3 participants
This means the value of the fields is tracked as a const value instead of a field reference.
This is to support some additional code patterns as well as align the behavior with the linker. Compiler will inline the const fields into the IL effectively removing the field reference in these cases and linker only sees the constant. So ideally the analyzer should have a similar behavior.
This is currently blocking runtime integration: dotnet/runtime#68650