Split the Copilot instruction files by activity

.github/instructions/conventions.instructions.md

@@ -0,0 +1,57 @@
+---
+applyTo: "src/**"
+---
+
+# General Conventions (all source areas)
+
+Code conventions for any change under `src/`, applied when authoring and when reviewing. Also
+apply the language file for the code in question (`csharp`, `native`), `tests` for test changes,
+and any matching area file (`core-runtime`, `jit`, `system-net-*`, `extensions-*`, `compression`,
+`cdac`). Where a more specific file conflicts with a general one, the more specific file wins.
+
+Pull-request process — scope, benchmark evidence, API approval, backport — is in
+[`copilot-instructions.md`](/.github/copilot-instructions.md). Build and test workflow is in the
+`build-and-test` skill. Review-only criteria are in `.github/skills/code-review/pr-assessment.md`.
+
+## Change Scope & Justification
+
+- **Prefer the simplest solution that works.** The burden of proof is on the more complex approach. Unnecessary abstraction, extra indirection, and elaborate solutions for marginal gains are a cost, not a feature.
+- **Justify each addition.** New code, APIs, abstractions, and flags create a permanent maintenance obligation. If an addition can be avoided without sacrificing correctness or meaningful capability, avoid it.
+- **Fix root cause, not symptoms or workarounds.** Investigate and fix the root cause rather than adding workarounds or suppressing warnings. Revert broken commits before layering fixes.
+- **Don't bundle unrelated changes.** Keep each change to a single concern: no drive-by refactoring, no whitespace noise, no build artifacts. Large refactorings and mechanical renames belong in their own change, separate from logic changes.
+
+## Consistency with Codebase Patterns
+
+### Code Reuse & Deduplication
+
+- **Extract duplicated logic into shared helper methods.** Fix improvements inside shared helpers so all callers benefit.
+- **Move shared code to shared files, not duplicated across runtimes.** When identical code exists across CoreCLR and NativeAOT, move it to the shared partition (using `#if !MONO` if needed).
+- **Use existing APIs instead of creating parallel ones.** Before introducing new types, enums, or helpers, check if existing ones serve the same purpose. Fix existing utilities rather than introducing duplicates.
+- **Delete dead code and unused declarations aggressively.** Remove dead code, unnecessary wrappers, obsolete fields, and unused variables when encountered or when the only caller changes. Also remove helper methods, enum values, function declarations, and resx strings left unused by a removal.
+
+### Established Conventions
+
+- **Store error strings in `.resx`, not inline code.** Reference via the `SR` class. When removing code that uses a resx string, delete the unused string entry.
+- **Preserve existing alphabetical ordering in modified lists.** When a PR adds or reorders entries in an alphabetized list—especially items within a `.csproj` item group, such as `Compile`, `ProjectReference`, and `PackageReference`—verify that the changed entries preserve the surrounding order. Flag only ordering regressions introduced by the PR; do not require unrelated cleanup of pre-existing unsorted entries. This also applies to lists of areas, configuration entries, resx entries, entrypoint/export lists, and ref source members.
+- **Don't modify auto-generated files or `eng/common` manually.** Change the generator or source definition instead. Files in `eng/common` are synced from dotnet/arcade.
+- **Use `DOTNET_` prefix for environment variables, not `COMPlus_`.** New runtime environment variables must use `DOTNET_` exclusively.
+- **Match existing style in modified files.** The existing style in a file takes precedence over general guidelines. Do not change existing code for style alone.
+
+### Runtime-Specific Patterns
+
+- **Consider NativeAOT parity for runtime changes.** When changing CoreCLR behavior, verify whether the same change is needed for NativeAOT. Note: Mono and CoreCLR native code conventions differ significantly — do not assume they share the same rules.
+- **Keep interpreter behavior consistent with the regular JIT.** Follow the same patterns, naming, error codes (`CORJIT_BADCODE`), and macros (`NO_WAY`). Use `FEATURE_INTERPRETER` guards.
+- **Source generators: no file locks, diagnostics from analyzers only.** Generators should bypass invalid state gracefully. A separate analyzer should produce diagnostics.
+- **Ref assembly conventions.** No `using` directives (fully qualify types), empty method bodies or `throw null`, genapi-style formatting, alphabetical member order. TFM-specific APIs go in separate files.
+
+## Documentation & Comments
+
+- **Comments should explain why, not restate code.** Delete comments like `// Get the types` that just duplicate the code in English. Don't include historical context about why code changed.
+- **Delete or update obsolete comments when corresponding code changes.** Stale comments describing old behavior are worse than no comments. Update them when you touch the relevant code; leave unrelated stale comments to a dedicated cleanup pass.
+- **Track deferred work with GitHub issues and searchable TODOs.** Reference a tracking issue in TODO comments with a consistent prefix (e.g., `TODO-Async:`). Remove ancient TODOs that will never be addressed.
+- **Don't duplicate comments on interface implementations.** Documentation comments belong on the interface definition. Implementations should use `<inheritdoc/>` to avoid divergence.
+- **Add XML doc comments on all new public APIs.** These seed the official API documentation on learn.microsoft.com. Properties should start with "Gets the ..." or "Gets or sets the ...". Do not add XML docs to test code.
+- **Use SHA-specific or commit-based links in documentation.** Don't use branch-relative links that break when files move.
+- **Reference specs and authoritative sources in implementation code.** When parsing signatures and metadata, cite the relevant spec section (e.g., ECMA-335). Link to relevant RFCs, papers, or repo-specific documentation (such as the ECMA-335 augments maintained in this repo). This applies broadly, not just to ECMA-335.
+- **Use established terminology in user-facing text.** Do not expose internal type names, private field names, or codenames like "Roslyn" in public docs or error messages.
+- **Retain copyright headers and license information.** All C# and C++ source files must include the standard license header, including test files. When porting from other projects, retain original copyright and update THIRD-PARTY-NOTICES.TXT.

.github/instructions/review-core-runtime.instructions.md → .github/instructions/core-runtime.instructions.md

@@ -2,15 +2,10 @@
 applyTo: "src/coreclr/**,src/native/corehost/**"
 ---
 
-# Code Review -- Core runtime
+# Core runtime
 
-Rules for reviewing CoreCLR and native host changes. Also apply `review-all-src`, the language
-file (`review-csharp` or `review-native`), `review-all-tests` for test changes, and `jit` for
-JIT changes.
-
-These are review criteria. During code authoring or local experimentation, treat PR-level gates
-such as motivation, benchmark evidence, and issue prerequisites as preparation guidance for a
-ready-for-review PR, not as reasons to block exploratory work unless the user asks for review.
+Conventions for CoreCLR and native host changes. Also apply `conventions`, the language file
+(`csharp` or `native`), `tests` for test changes, and `jit` for JIT changes.
 
 ## Correctness & Safety
 
@@ -20,7 +15,3 @@ ready-for-review PR, not as reasons to block exploratory work unless the user as
 ## Performance & Allocations
 
 - **Avoid LINQ and records in low-level compiler codebases.** In CG2/ILC and AOT tools, use direct loops instead of LINQ and readonly structs instead of records. Use concrete types over interfaces in private code.
-
-## PR Prerequisites
-
-- **Start core component changes with an issue.** Changes to host, VM, or JIT should start with a GitHub issue describing the problem and motivation before submitting a PR.

.github/instructions/review-csharp.instructions.md → .github/instructions/csharp.instructions.md

@@ -2,16 +2,11 @@
 applyTo: "**/*.cs"
 ---
 
-# Code Review -- C# (managed code)
+# C# (managed code)
 
-Rules for reviewing C# changes across `src/`. Also apply `review-all-src` (all changes),
-`review-all-tests` (test files), and any matching area file (`review-core-runtime`, `jit`,
-`system-net-*`, `extensions-*`, `compression`, `cdac`). Native runtime code is covered by
-`review-native`.
-
-These are review criteria. During code authoring or local experimentation, treat PR-level gates
-such as motivation, benchmark evidence, and issue prerequisites as preparation guidance for a
-ready-for-review PR, not as reasons to block exploratory work unless the user asks for review.
+Conventions for C# changes across `src/`. Also apply `conventions` (all changes), `tests` (test
+files), and any matching area file (`core-runtime`, `jit`, `system-net-*`, `extensions-*`,
+`compression`, `cdac`). Native runtime code is covered by `native`.
 
 ## Correctness & Safety
 
@@ -22,7 +17,7 @@ ready-for-review PR, not as reasons to block exploratory work unless the user as
 - **Include actionable details in exception messages.** Use `nameof` for parameter names. Include the unsupported type or unexpected value. Never throw empty exceptions.
 - **Initialize output parameters in all code paths.** When a method has `out` parameters or pointer outputs (`bytesWritten`, `numLocals`), ensure they are initialized to a defined value in all error paths.
 - **Use `ThrowIf` helpers over manual checks.** Use `ArgumentOutOfRangeException.ThrowIfNegative`, `ObjectDisposedException.ThrowIf`, etc. instead of manual if-then-throw patterns.
-- **Challenge exception swallowing that masks unexpected errors.** When a PR adds try/catch blocks that silently discard exceptions (`catch { continue; }`, `catch { return null; }`), question whether the exception represents a truly expected, recoverable condition or an unexpected error signaling a deeper problem (race conditions, memory corruption, build environment issues). Silently catching exceptions that "shouldn't happen" hides root causes and makes debugging harder. The default disposition should be to let unexpected exceptions propagate or fail fast so the real issue gets investigated.
+- **Don't swallow exceptions that mask unexpected errors.** Before adding a try/catch that silently discards exceptions (`catch { continue; }`, `catch { return null; }`), establish that the exception is a truly expected, recoverable condition rather than an unexpected error signaling a deeper problem (race conditions, memory corruption, build environment issues). Silently catching exceptions that "shouldn't happen" hides root causes and makes debugging harder. Let unexpected exceptions propagate or fail fast so the real issue gets investigated.
 
 ### Thread Safety
 
@@ -38,22 +33,17 @@ ready-for-review PR, not as reasons to block exploratory work unless the user as
 
 ### Correctness Patterns
 
-- **Fix root cause, not symptoms or workarounds.** Investigate and fix the root cause rather than adding workarounds or suppressing warnings. Revert broken commits before layering fixes.
-- **Prefer safe code over unsafe micro-optimizations.** Do not introduce `Unsafe.As`, `Unsafe.AsRef`, or raw pointers without demonstrable performance need. Prefer Span-based APIs. If performance is the issue, prefer fixing the JIT.
+- **Prefer safe code over unsafe micro-optimizations.** Do not introduce `Unsafe.As`, `Unsafe.AsRef`, or raw pointers without demonstrable performance need. Prefer Span-based APIs. If performance is the issue, prefer fixing the JIT. Never convert safe code to unsafe as a side effect of an unrelated functional change.
 - **Use `Unsafe.BitCast` for same-size type punning between blittable types.** Prefer `Unsafe.BitCast<TFrom, TTo>` over `Unsafe.As<TFrom, TTo>` for type punning between unmanaged value types of the same size. For common cases, prefer safe alternatives (e.g., `BitConverter.SingleToInt32Bits` for `float`→`int`).
-- **Scope creep: don't bundle cleanup into unrelated changes.** When the focus is a functional change, don't also convert safe code to unsafe or refactor for micro-optimizations. Keep those in separate PRs.
-- **Delete dead code and unnecessary wrappers.** Remove dead code, unnecessary wrappers, obsolete fields, and unused variables when encountered or when the only caller changes.
 - **Handle `SafeHandle.IsInvalid` before `Dispose`.** Check `IsInvalid` (not null) on returned SafeHandles. Get the exception before calling `Dispose`, since Dispose might clear the error state.
-- **Seal classes when `Equals` uses exact type matching.** If a class implements `Equals` with `GetType()` comparison, flag this as a potential bug if the class is unsealed — the solution is usually to seal the class, but don't automatically recommend sealing as the fix. Raise it as a warning for the author to evaluate.
+- **Seal classes when `Equals` uses exact type matching.** If a class implements `Equals` with `GetType()` comparison, treat this as a potential bug when the class is unsealed. Sealing is usually the right fix, but not always — evaluate rather than applying it reflexively.
 - **Use `Environment.ProcessPath` and `AppContext.BaseDirectory`.** Use these instead of `Process.GetCurrentProcess().MainModule?.FileName` and `Assembly.Location` for NativeAOT/single-file compatibility.
 - **File name casing must match csproj references exactly.** Linux is case-sensitive. New source files must be listed in the `.csproj` if other files in that folder are explicitly listed.
-- **Backport targeted fixes, not refactorings.** When backporting to servicing branches, create small targeted fixes. Backporting large refactorings introduces unnecessary risk.
 
 ## Performance & Allocations
 
 ### Measurement & Evidence
 
-- **Performance changes require benchmark evidence.** Include BenchmarkDotNet results before merging. Prefer local BenchmarkDotNet runs first, especially for experimental/iterative work. EgorBot runs on an individual's personal account and is not billed like Copilot usage — only recommend it when explicitly requested, or for a final cross-architecture (x64/arm64) confirmation that cannot be reproduced locally.
 - **Justify binary size increases with real-world measurements.** Changes that increase binary size require measured wall-clock improvements on real-world apps, not just instruction counts.
 - **Avoid premature optimization with object pools and caches.** Do not introduce global caches or object pools without evidence they are needed. Prefer making the underlying operation faster.
 
@@ -86,8 +76,8 @@ ready-for-review PR, not as reasons to block exploratory work unless the user as
 
 ## API Design & Contracts
 
-- **New public APIs require approved proposals before PR submission.** All new API surface must go through API review. PRs adding unapproved APIs will be closed. The implementation should match what was approved, though it is explicitly allowed to defer portions of an approved API to incremental follow-up PRs, and an implementor may opt to exclude specific API members for technical reasons without needing re-approval unless the exclusion significantly impacts the design. When new public API surface is detected, the API approval verification procedure (`.github/skills/code-review/api-approval-check.md`) is executed to enforce this rule.
-- **Use `internal` for new APIs pending API review.** If the API is needed immediately for implementation, mark it `internal` and file a review request separately.
+- **Implementation must match the approved API shape.** Deferring portions of an approved API to incremental follow-up PRs is explicitly allowed, as is excluding specific members for technical reasons — unless the exclusion significantly impacts the design.
+- **Use `internal` for new APIs pending API review.** If the API is needed immediately for implementation, mark it `internal` and file a review request separately. This governs code being submitted — an `api-proposal` prototype branch keeps the surface public so ref source generation can extract it.
 - **Parameter names must match between ref and src.** Renaming a public API parameter (including case changes) is a breaking change affecting named arguments and late-bound scenarios.
 - **Align exception types and validation order across platforms.** Validate arguments first (`ArgumentNullException`, then `ArgumentException`), then `PNSE`, then `ObjectDisposedException`, then perform the operation. Throw the same exception types on all platforms.
 - **`Try` APIs should return `false` only for the common expected failure.** Throw for everything else (corruption, permissions, invalid arguments). Try methods must always throw on invalid arguments.

.github/instructions/review-native.instructions.md → .github/instructions/native.instructions.md

@@ -2,16 +2,11 @@
 applyTo: "**/*.c,**/*.cc,**/*.cpp,**/*.cxx,**/*.h,**/*.hpp,**/*.inc,**/*.S,**/*.s,**/*.asm"
 ---
 
-# Code Review -- Native code (C/C++/asm) & interop
+# Native code (C/C++/asm) & interop
 
-Rules for reviewing native runtime code (CoreCLR VM, JIT, `src/native`, Mono native). Also apply
-`review-all-src`, `review-all-tests` for test changes, and `review-core-runtime` for CoreCLR and
-native host changes. For JIT specifics see `jit`; for networking interop see
-`system-net-interop`.
-
-These are review criteria. During code authoring or local experimentation, treat PR-level gates
-such as motivation, benchmark evidence, and issue prerequisites as preparation guidance for a
-ready-for-review PR, not as reasons to block exploratory work unless the user asks for review.
+Conventions for native runtime code (CoreCLR VM, JIT, `src/native`, Mono native). Also apply
+`conventions`, `tests` for test changes, and `core-runtime` for CoreCLR and native host changes.
+For JIT specifics see `jit`; for networking interop see `system-net-interop`.
 
 ## Correctness & Safety