Disallow readonly instance fields in InlineArray structs

[VSadov]

Making the element field of an InlineArray struct should be forbidden as it currently has no meaning.
The restriction is required at very least to be able to attach some meaning to this scenario in the future.

UNDONE:

• Will add a test once Fix up InlineArray tests #90192 PR is merged.

[VSadov]

CC: @lambdageek - similar change may be needed in Mono.

[lambdageek]

@VSadov thanks for the heads up.

Try adding this

 							break;
 						}
 					}
+					if ((ftype->attrs & FIELD_ATTRIBUTE_INIT_ONLY) != 0) {
+						mono_class_set_type_load_failure (klass, "InlineArrayAttribute cannot be applied to a type with readonly instance fields.");
+						break;
+					}
 				}
 
 				/* FIXME (LAMESPEC): should we also change the min alignment according to pack? */

Right here:

runtime/src/mono/mono/metadata/class-init.c

Lines 2303 to 2307 in a67d5f1

	}
	}
	}
	/* FIXME (LAMESPEC): should we also change the min alignment according to pack? */

/cc @kotlarmilos

[jkotas]

I know we have discussed this over email. I agree that this makes sense to block in Roslyn, for now at least. Looking at this again, I am not sure about blocking it in the runtime.

There are many other type shapes that the runtime type loader allows and Roslyn disallows. It feels random to block this particular one. What should be our policy where the type loader matches the same type restrictions as Roslyn vs. where the type loader is less strict than Roslyn?

So far, our general type loader policy has been that the type loader only enforces invariants that are important for making the runtime work reliably. Readonly InlineArray does not harm the runtime in any way, so why block it?

[jkotas]

cc @jaredpar @AlekseyTs

[VSadov]

Readonly InlineArray does not harm the runtime in any way, so why block it?

I can be easily convinced either way. My initial suggestion was also to do nothing on the runtime side, but the last discussion ended with "we should forbid this to not infringe on possible future use".

Since indexing and other accessors are done in Roslyn, forbidding only on the Roslyn side might be sufficient.

@jaredpar @AlekseyTs

[kotlarmilos]

@VSadov If we decide blocking this scenario in class loader, for Mono try using deferred type load failures callback that should allow AOT compilation to generate the code and raise exceptions during the runtime.

if ((ftype->attrs & FIELD_ATTRIBUTE_INIT_ONLY) != 0) {
    if (mono_get_runtime_callbacks ()->mono_class_set_deferred_type_load_failure_callback) {
        if (mono_get_runtime_callbacks ()->mono_class_set_deferred_type_load_failure_callback (klass, "InlineArrayAttribute cannot be applied to a type with readonly instance fields."))
            break;
        else
            ; // failure occured during AOT compilation, continue execution
    } else {
        mono_class_set_type_load_failure (klass, "InlineArrayAttribute cannot be applied to a type with readonly instance fields.");
        break;
    }
}

/cc: @lambdageek

[AlekseyTs]

Compiler simply blocks declarations of readonly element fields in source, no verification when we load type from metadata. I guess it is fine for runtime to not verify this aspect on type load, but, It still should be specified as illegal in runtime spec, I think.

[VSadov]

I think we can close this.

[VSadov]

Added an item related to this to the documentation tracking issue for InlineArray - #91012