C#: Extract the metadata handle#590
Conversation
|
This pull request introduces 1 alert when merging 74945bd into 044dcfb - view on LGTM.com new alerts:
Comment posted by LGTM.com |
hvitved
left a comment
There was a problem hiding this comment.
A very nice addition, and this should certainly make data flow analysis faster/more accurate.
| internal static Tuple locations_default(ISourceLocation label, IFile file, int startLine, int startCol, int endLine, int endCol) => | ||
| new Tuple("locations_default", label, file, startLine, startCol, endLine, endCol); | ||
|
|
||
| internal static Tuple metadata_handle(IEntity entity, Assembly assembly, int handleValue) => |
There was a problem hiding this comment.
How about changing the type of handleValue to Handle, and then call handleValue.GetHashCode() here rather than at all the call sites?
There was a problem hiding this comment.
I didn't do that because I didn't want to put population logic into the Tuples methods.
| @callable | @value_or_ref_type | @void_type; | ||
|
|
||
| #keyset[entity, location] | ||
| metadata_handle(int entity : @metadata_entity ref, int location: @assembly ref, int handle: int ref) |
There was a problem hiding this comment.
Is #keyset[location, handle] also valid?
There was a problem hiding this comment.
No, because there are 1-2 entities for each (location,handle) - the CIL one and the C# one.
| } | ||
|
|
||
| query predicate tooManyMatchingHandles(MetadataEntity e) { | ||
| count(MetadataEntity e2 | e.matchesHandle(e2))>2 |
| count(MetadataEntity e2 | e.matchesHandle(e2))>2 | ||
| } | ||
|
|
||
| query predicate missingCil(Element e) { |
There was a problem hiding this comment.
Change type to MetadataEntity and remove infix cast below
There was a problem hiding this comment.
Then I would need an infix cast in a different place - I think on the fromLibrary.
| get | ||
| { | ||
| var propertyInfo = symbol.GetType().GetProperty("Handle", | ||
| System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.GetProperty); |
| } | ||
| } | ||
|
|
||
| return new Handle(); // A nil handle |
There was a problem hiding this comment.
Change the return type to Handle? and use null here instead? Then the check !handle.IsNil above will be handle.HasValue, which is more standard.
| { | ||
| return (MethodDefinitionHandle)value; | ||
| } | ||
| else if(value is TypeDefinitionHandle) |
…ns.GetToken(). C#: Make path IDs consistent.
74945bd to
d687dd9
Compare
| { | ||
| var value = propertyInfo.GetValue(symbol); | ||
|
|
||
| if (value is MethodDefinitionHandle) |
There was a problem hiding this comment.
Use pattern matching to avoid cast, and same in the other cases below.
This is a fix for the rather unsatisfactory
getLabel()solution we use when matching C# and CIL types/methods etc. Instead, populate ametadata_handle/3extensional which records the metadata handle (a unique integer in an assembly) for each extracted entity in both the CIL and C# extractors.The pair
(assembly, handle)can be used to uniquely and efficiently identify the corresponding C#/CIL method.The dbstats are needed to ensure the suitable join order (handle,assembly).
Note that #29460 on Semmle/code is needed to bump the db and submodule.