From my understanding the purpose of VersionNameMap is to map an OS version number to a human readable name (e.g. High Sierra) and vice-versa. Looking at operations to map a version number to a name I count at least 8 ways to do this: Let P be a PlatformInfo object. VersionNameMap().to_name(...) VersionNameMap().to_name(..., platform="...") VersionNameMap(P).to_name(...) VersionNameMap(P).to_name(..., platform="...") VersionNameMap.map().to_name(...) VersionNameMap.map().to_name(..., platform="...") VersionNameMap.map(P).to_name(...) VersionNameMap.map(P).to_name(..., platform="...") And quickly glancing through the code these invocation may give different results regardless of whether both P and platform="..." are in agreement depending on the presence of Apple Additions.
Some of these invocations are only exist because Python doesn't allow a constructor to be hidden. It is always the case, especially for Apple's ports, that the VersionNameMap.map(...) function should be used, specifically because of apple_additions. In some of the other cases, you're explicitly overriding the default PlatformInfo object. Given the fact that you shouldn't be using the default constructor, we can definitely eliminate 1 and 2. There is also an argument to be made that we shouldn't define a default PlatformInfo object if the user does not provide us with one. That would eliminate 5 and 6. However, we definitely need to keep the ability to override the default platform in to_name. As mentioned in <https://bugs.webkit.org/show_bug.cgi?id=185386>, we rely on this in iOS where we don't have an iOS PlatformInfo object when we need to extract version information.
Created attachment 339775 [details] Patch
I've uploaded a potential improvement which eliminates 1 and 2, and indicates through a comment that 3 and 4 should be avoided. There is another way to clarify this code, although it would require more synchronization with apple_additions. We could have apple_additions update the underlying mapping of the VersionNameMap in the constructor. This would totally eliminate the map(...) function, leaving 1-4 but eliminating 5-8 explicitly.
Created attachment 340091 [details] Patch
Comment on attachment 340091 [details] Patch This patch combines what will eventually be 3 changes (2 associated with this bug and one associated with <https://bugs.webkit.org/show_bug.cgi?id=185386>). I wanted to first ensure that the direction of this patch satisfies Dan's concerns about the clarity of VersionNameMap before moving forward.