C#: Stub generator improvements. #10003
Conversation
michaelnebel
added
the
no-change-note-required
michaelnebel
force-pushed
the
csharp/stubsenum
branch
from
1287fba to
e33981e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -513,12 +513,30 @@ private string stubClassName(Type t) { | |||
| stubClassName(element), "," order by i | |||
| ) + ")" | |||
| else | |||
| if t instanceof ValueOrRefType | |||
| if t instanceof FunctionPointerType | |||
There was a problem hiding this comment.
You can remove this if, and instead replace the matching else with or not t instanceof FunctionPointerType and
There was a problem hiding this comment.
Sure - we can do it like that instead - I just continued the pattern that was already there.
However, it might make things slightly more complicated, when adding new cases 😄
csharp/ql/src/Stubs/Stubs.qll
Outdated
| stubQualifiedNamePrefix(t) + t.getUndecoratedName() + | ||
| stubGenericArguments(t) | ||
| else result = "<error>" | ||
| exists(CallingConvention callconvention, string calltext | |
There was a problem hiding this comment.
Perhaps add FunctionPointerType fpt, assign fpt = this, and replace the three t.(FunctionPointerType) with fpt.
There was a problem hiding this comment.
Yes, I thought about that as well, but decided against it because last time I did it, the review comment recommend to do it the other way around 😄 But on the other hand, here we already have an exists, so it will make it nicer 👍
There was a problem hiding this comment.
It makes sense to introduce a new variable (either with exists or any) when there are multiple identical casts, as in this case.
There was a problem hiding this comment.
Great - that is the rule of 👍 I needed 😸
| @@ -134,6 +134,34 @@ public class Nested : Class9 | |||
|
|
|||
| public Class9.Nested NestedInstance { get; } = new Class9.Nested(1); | |||
| } | |||
|
|
|||
| public enum Enum1 | |||
There was a problem hiding this comment.
Add a test case with non-int enum?
There was a problem hiding this comment.
Oh, definitely yes!
and also a testcase for function pointers!
michaelnebel
force-pushed
the
csharp/stubsenum
branch
from
30c52e3 to
eb90579
Compare
In this PR we introduce the following improvements to the the stub generator.
This brings the stub generator in state where we almost can generate stubs for the ASP.NET Core and .NET Core frameworks without making manual changes to the generated code.
The only part that is now missing are some
unsafekeywords (whether this is due to extraction errors or issues with the stub generator is unknown).Furthermore, the subs for ASP.NET Core and .NET Core has been regenerated based on the newest version of the stub generator.
Closes https://github.com/github/codeql-csharp-team/issues/260