Skip to content

[3.0] Replace NameTrimmer with equivalent IdentifySharedPrefixes mod#2557

Draft
Exanite wants to merge 78 commits intodotnet:develop/3.0from
Exanite:feature/identify-shared-prefixes-mod
Draft

[3.0] Replace NameTrimmer with equivalent IdentifySharedPrefixes mod#2557
Exanite wants to merge 78 commits intodotnet:develop/3.0from
Exanite:feature/identify-shared-prefixes-mod

Conversation

@Exanite
Copy link
Copy Markdown
Member

@Exanite Exanite commented Mar 31, 2026

Summary of the PR

(WIP)

This PR chains off of #2555 and focuses on rewriting the NameTrimmer class as a new IdentifySharedPrefixes mod with equivalent functionality. The goal is to keep all outputs the same while removing edge cases and generally just making the code easier to understand.

Note: Part of the reason this PR chains off of #2555 is because #2555 actually needs some of the changes here to fully work correctly. #2555 can be merged as is, but one unit test has to be commented out and a non-critical feature will be missing.

Related issues, Discord discussions, or proposals

This is the conversation in January when this change was briefly discussed: https://discord.com/channels/521092042781229087/587346162802229298/1457864604951777441

Further Comments

Tasks

  • Replace LenientUnderscore with NameSplitter-based implementation
  • Add a IdentifySharedPrefixes mod that identifies shared prefixes and uses the name affix system to annotate those prefixes.
    • Ideally rewrite to avoid the "3 pass" logic. I have no idea how it works still.
      • Decision: Currently keeping it mostly the same since it works and handles all cases. It's also very fast already.
    • Ideally avoid using Humanizer and use the new NameSplitter code for consistency (especially since we have thorough test coverage for this code)
  • Migrate the old test cases over from the NameTrimmer tests
    • Comments after implementing: These seem to provide fairly good edge case coverage along with the other tests I migrated from PrettifyNamesTests. Since the new IdentifySharedPrefixes mod ended up being an almost direct copy of the core trimming code from NameTrimmer, I decided to not add too many new tests.
  • Decide what to do about IdentifiesSharedPrefix_WhenPrefixesDeclared.
    • This tests for a really annoying edge case that isn't very likely to show up in real world bindings, but is an important case to consider.
    • Issue: Underscores often break things.
      • The test pre-declares GL as a prefix for GL_PIXEL_COUNT_NV and GL_PIXEL_COUNT_AVAILABLE_NV to hide it from the shared prefix identification process.
      • Stripping affixes leads to _PIXEL_COUNT_NV and _PIXEL_COUNT_AVAILABLE_NV.
      • Apparently this leads to _ being the shared prefix. This is definitely a bug since I expect _PIXEL_COUNT or PIXEL_COUNT.
      • Ideally, prefixes are declared without leading/trailing underscores and underscores are ignored when stripping affixes. Eg: GL and PIXEL_COUNT should be the final affixes.
      • Hmm, there is a problem here though... affixes are supposed to be added back verbatim if not trimmed, meaning that we will get GLPIXEL_COUNT, which is definitely not great behavior.
      • Essentially, this requires us to allow each affix to be prettified before joining them together. This is also doable, but arguably unnecessary (remember this is a theoretical edge case that hasn't shown up yet)?
    • Decision: This is purely theoretical and not worth handling right now.
      • If we introduce new prefixes, then we're modifying the codebase. We can also modify the codebase to handle these cases as needed at that time.
      • Additionally, Silk 2 never needed anything like this.
  • Change INameProcessor to be an interface private to PrettifyNames
    • We can change this later if required, but I currently don't see a point to exposing it. There are plenty of ways to configure the name transformation process already through ways that are more declarative and configurable by the user.
  • Remove shared prefix trimming code from PrettifyNames
    • Visitor
    • Prettify-only pipeline
  • Rewrite rest of trimming+prettify pipeline as sequential INameProcessors
    • This should include the name conflict resolution code, since that's currently done as a step right after the name processors run.
    • This will act as the main and only code path. Currently we have two code paths (which technically was 3 before I removed another one during [3.0] Improve handling of nested struct names and affixes #2555).
    • Remove the name container isolation and just provide all names together. If name processors need the isolation, we still provide the container names and container members separately and can implement the old behavior as needed, but restricting access to all names is unnecessary and can actually lead to incorrect behavior.
  • Regenerate all bindings using Windows and ensure no changes to output

Exanite added 30 commits March 24, 2026 05:59
…ld used by other bindings

The default acronym threshold was changed during #29, which was merged as part of dotnet#2503.
Considering we decided to follow Microsoft's Framework Design Guidelines (acronym threshold of 2) for the bindings and rest of the API, might as well be consistent here.
This lets us handle prefixing and prettification separately, which notably is important if we add prefixes after prettification.
We want to prefix the final name, not the intermediate name in this case.
This no longer makes sense to keep and enabling features by baseline version seems fiddly.
If we need to toggle features for newer versions, we can explicitly add a boolean config option.
Kinda a cop out decision, but it keeps thing simple (and thus maintainable) and implementing it fully seems overkill for what we need.
This is because we no longer output the separating underscore in ExtractNestedTyping
…nvention

Note that the goal is to eventually remove the name overrides for the `EFXEAXREVERBPROPERTIESflLateReverbPan` and `-Delegate` cases entirely.
This is because these are theoretically possible to handle automatically and the reason it doesn't work is due to an edge case interaction with the name override system.

See the "Tasks" section here for more info: dotnet#2555
Exanite added 30 commits April 1, 2026 15:05
This is because 2D isn't trimmed properly by my new name splitter, but I decided that this case was not worth handling in favor of keeping the trimming code simple.

See my thoughts here: https://discord.com/channels/521092042781229087/1485943448069738608/1489457083857506336
See dotnet#2557 for full reasoning, but the short is that this edge case is purely theoretical and can be handled later on when actually necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant