Skip to content

Commit 12ccb9c

Browse files
committed
Add more comments on enum vendor suffix trimming
(cherry picked from commit 933e1bc)
1 parent e6c339b commit 12ccb9c

File tree

1 file changed

+52
-27
lines changed

1 file changed

+52
-27
lines changed

sources/SilkTouch/SilkTouch/Mods/MixKhronosData.cs

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -203,15 +203,29 @@ public record Configuration
203203
/// if the enum type contains members that don't match the exclusive vendor.
204204
/// </summary>
205205
/// <remarks>
206-
/// For context, OpenGL has a problem where an enum group starts out as ARB but never gets promoted, and then contains other vendor enums or even core enums.
207-
/// For example, BufferUsageARB contains StreamDraw, which is a core enum. Enabling this option will cause BufferUsageARB to be trimmed as BufferUsage.
206+
/// For context, OpenGL has a problem where an enum group starts out as ARB but never gets promoted,
207+
/// and then contains other vendor enums or even core enums. OpenAL is similar. Vulkan does not have this problem.
208208
/// </remarks>
209+
/// <example>
210+
/// <c>BufferUsageARB</c> in OpenGL is an ARB suffixed enum that contains <c>GL_STREAM_DRAW</c> which is a core enum.
211+
/// In this case, ARB is the exclusive vendor suffix, but it is contradicted by the existence of a non-suffixed enum member.
212+
/// This implies that <c>BufferUsageARB</c> was incorrectly promoted and that we should remove its vendor suffix.
213+
/// Enabling this option will trim <c>BufferUsageARB</c> as <c>BufferUsage</c>.
214+
/// </example>
209215
public bool TrimEnumTypeNonExclusiveVendors { get; init; } = false;
210216

211217
/// <summary>
212218
/// Whether enum members should have their vendor suffixes trimmed
213219
/// if they share a vendor suffix with the containing type.
214220
/// </summary>
221+
/// <example>
222+
/// <c>VkPresentModeKHR</c> in Vulkan is a KHR suffixed enum that contains <c>VK_PRESENT_MODE_IMMEDIATE_KHR</c>.
223+
/// The KHR suffix is useful in native code because it is not always immediately clear which enum group the enum member belongs to.
224+
/// However, in C#, enum members are always part of an enum type, and as such, we can assume that if an enum member belongs
225+
/// to a KHR enum type, then the non-suffixed enum member is also a KHR enum member.
226+
/// <para/>
227+
/// Enabling this option will trim <c>VK_PRESENT_MODE_IMMEDIATE_KHR</c> as <c>VK_PRESENT_MODE_IMMEDIATE</c>.
228+
/// </example>
215229
public bool TrimEnumMemberImpliedVendors { get; init; } = false;
216230
}
217231

@@ -1919,14 +1933,16 @@ public override SyntaxNode VisitFieldDeclaration(FieldDeclarationSyntax node)
19191933
public override SyntaxNode VisitDelegateDeclaration(DelegateDeclarationSyntax node) =>
19201934
node.WithAttributeLists(ProcessAndGetNewAttributes(node.AttributeLists, node.Identifier, false));
19211935

1936+
// Special case for enums since this code needs information about
1937+
// the enum type name and its member names at the same time
1938+
// in order to properly trim the type name and member name
19221939
public override SyntaxNode VisitEnumDeclaration(EnumDeclarationSyntax node)
19231940
{
1924-
// Special case for enums since this code needs information about the enum type and its members at the same time.
1925-
19261941
var typeName = node.AttributeLists.GetNativeNameOrDefault(node.Identifier);
19271942
var groupInfo = job.Groups.GetValueOrDefault(typeName);
19281943

19291944
var typeVendor = job.Vendors.FirstOrDefault(typeName.EndsWith);
1945+
var hasTypeSuffix = typeVendor != null;
19301946
var vendorFromTypeNameOrder = config.VendorSuffixOrder;
19311947

19321948
// Figure out the enum's exclusive vendor
@@ -1938,37 +1954,21 @@ public override SyntaxNode VisitEnumDeclaration(EnumDeclarationSyntax node)
19381954
exclusiveVendor = null;
19391955
}
19401956

1941-
// Check if the enum contains unsuffixed members
1942-
var containsUnsuffixed = node.Members.Any(member =>
1943-
{
1944-
var memberName = member.AttributeLists.GetNativeNameOrDefault(member.Identifier);
1945-
if (job.Vendors.FirstOrDefault(memberName.EndsWith) == null)
1946-
{
1947-
return true;
1948-
}
1949-
1950-
return false;
1951-
});
1952-
1953-
var isSafeToTrimMembers = !containsUnsuffixed;
1954-
1957+
// See config option for more info and examples on what this does
19551958
if (config.TrimEnumTypeNonExclusiveVendors && typeVendor != null)
19561959
{
19571960
var shouldTrimType = typeVendor != exclusiveVendor;
19581961

19591962
// Check if there are other versions of the enum (this includes the core variant and other vendor variants)
19601963
var isSafeToTrimType = job.Groups.Count(x => x.Key.StartsWith(typeName[..^typeVendor.Length])) <= 1;
19611964

1962-
if (shouldTrimType)
1965+
if (shouldTrimType && isSafeToTrimType)
19631966
{
1964-
if (isSafeToTrimType)
1965-
{
1966-
// Remove the exclusive vendor from the enum name since it is wrong and it is safe to do so
1967-
vendorFromTypeNameOrder = -1;
1967+
// Remove the exclusive vendor since it isn't actually exclusive
1968+
vendorFromTypeNameOrder = -1;
19681969

1969-
// Type suffix has been removed
1970-
isSafeToTrimMembers = false;
1971-
}
1970+
// Type suffix has been removed
1971+
hasTypeSuffix = false;
19721972
}
19731973
}
19741974

@@ -1978,8 +1978,33 @@ public override SyntaxNode VisitEnumDeclaration(EnumDeclarationSyntax node)
19781978
node = node.WithAttributeLists(node.AttributeLists.AddNameSuffix(typeVendor, vendorFromTypeNameOrder));
19791979
}
19801980

1981+
// Check if the enum contains unsuffixed members
1982+
var containsUnsuffixedMembers = node.Members.Any(member =>
1983+
{
1984+
var memberName = member.AttributeLists.GetNativeNameOrDefault(member.Identifier);
1985+
if (job.Vendors.FirstOrDefault(memberName.EndsWith) == null)
1986+
{
1987+
return true;
1988+
}
1989+
1990+
return false;
1991+
});
1992+
1993+
// We should not trim member suffixes if the enum type already contains unsuffixed members
1994+
// We also should not trim member suffixes if the type suffix was removed
1995+
//
1996+
// For example (direct conflict with existing):
1997+
// ConvolutionBorderMode in OpenGL contains GL_REDUCE and GL_REDUCE_EXT.
1998+
// Trimming EXT from GL_REDUCE_EXT will conflict with GL_REDUCE.
1999+
//
2000+
// Another example (possible confusion between core and non-core):
2001+
// ContextRequest in OpenAL contains ALC_FALSE and ALC_DONT_CARE_SOFT. One is core and one is non-core.
2002+
// If we trim SOFT from ALC_DONT_CARE_SOFT, it is not immediately obvious that the enum members have different promotion statuses.
2003+
var canTrimImpliedVendors = hasTypeSuffix && !containsUnsuffixedMembers;
2004+
19812005
// Trim the enum members if needed
1982-
if (config.TrimEnumMemberImpliedVendors && typeVendor != null && isSafeToTrimMembers)
2006+
// See config option for more info and examples on what this does
2007+
if (config.TrimEnumMemberImpliedVendors && typeVendor != null && canTrimImpliedVendors)
19832008
{
19842009
node = node.WithMembers([
19852010
..node.Members.Select(member => {

0 commit comments

Comments
 (0)