Skip to content

C#: Enable nullability on Semmle.Extraction.CIL.Driver#4114

Merged
tamasvajk merged 2 commits intogithub:mainfrom
tamasvajk:feature/nullability-cil-driver
Aug 25, 2020
Merged

C#: Enable nullability on Semmle.Extraction.CIL.Driver#4114
tamasvajk merged 2 commits intogithub:mainfrom
tamasvajk:feature/nullability-cil-driver

Conversation

@tamasvajk
Copy link
Contributor

No description provided.

@tamasvajk tamasvajk added the C# label Aug 20, 2020
@tamasvajk tamasvajk requested a review from a team August 20, 2020 14:53
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactoring of ExtractorOptions!


int IEqualityComparer<AssemblyName>.GetHashCode(AssemblyName obj) =>
obj.Name.GetHashCode() + 7 * obj.Version.GetHashCode();
(obj.Name, obj.Version).GetHashCode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, hadn't thought of that way to combine hash codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another new way of computing hashcodes: HashCode.Combine(). I'm not sure which is the preferred way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the latter does not create the intermediate tuple object.

// a deliberately malformed assembly.
// In this case, we just skip the extraction of this assembly.
isAssembly = false;
throw new InvalidAssemblyException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps document the method and mention that it may throw an InvalidAssemblyException.

options.Threads = System.Environment.ProcessorCount;
options.PDB = true;
options.TrapCompression = TrapWriter.CompressionMode.Gzip;
var options = new ExtractorOptions(args);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to introduce options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I remove the whole ParseCommandLine method and replace the callers with the a constructor call? We don't really need a factory method, do we?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea.

@tamasvajk tamasvajk merged commit 74db25d into github:main Aug 25, 2020
luchua-bc pushed a commit to luchua-bc/ql that referenced this pull request Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants