Conversation
| } | ||
| } | ||
|
|
||
| if (!hasNativeTypeName && !(currentType.Contains("IUnknown**")) && !(currentType.Contains("IInspectable**"))) |
There was a problem hiding this comment.
The specific conditional check for the NativeTypeName IUnknown** and IInspectable** are for the only two specific occurrences where NativeTypeName gets added twice for its field. It's because the original NativeTypeName was not showing in the original attribute list, so this new NativeTypeName gets added, and then it becomes a duplicate. I'm not sure why this is happening, but for now I have this temporary fix so it won't get added.
|
Based on the diff there was only 1 API that was flagged as incorrect and fixed. How much confidence do we have that the logic is working and we've caught all issues? @kennykerr mentioned he's heard from customers hitting issues like this pretty consistently so I'd expect there would have been more broken APIs. Have we fixed all the biggest offenders manually at this point? |
It seems unlikely that this has caught all the issues, but I can't be certain. #1330 is about validating far more than just field/enum replacements. If you check the size and alignment of structs in C++ you can quickly eliminate all kinds of size and packing issues and determine whether this is in fact accurate. Short of doing that, I'd suggest going through the various size and alignment issues that have been previously reported and testing whether this solution would have caught them. |
I tested it with #1420, and my fix wasn't able to pick up on that bug. But the cause of the problem is still the same, it's not able to validate struct size because it's not scraping a NativeTypeName from those fields. I'm still looking into this now - I'm going to close and reopen this when it's ready again. |
|
Reopened the PR because I realized the PAN_ issue #1420 were not being picked up because there was no corresponding size of type BYTE. After adding that, it was able to properly give an error that the enum is an incorrect size, and I fixed it accordingly. Like Kenny suggested, I tested it out with other alignment issues that were reported. I tested it out with #1330, another struct enum issue, by removing the byte size and it was able to properly pick up that the enum was an incorrect size. |
4 participants
Fixes: #1330
Fixes: #1420
Problem: Originally, EnsureEnumSizeMatchesOriginalSize(), the method that is supposed to do the enum size validation, is not able to work as intended because the scraper did not emit a NativeTypeName, which is yields our problem since EnsureEnumSizeMatchesOriginalSize() assumes there will always be a NativeTypeName.
Solution: I added code that will add a NativeTypeName for every field that doesn't have it, with the current type being the new NativeTypeName.
This should be able to detect any incorrect enum sizes such as in #1325, where DEVMODE_COLOR, DEVMODE_TRUETYPE_OPTION, DEVMODE_COLLATE are defined as uint but should be ushort.