Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LLVMPassManagerBuilderRef Extensions #114

Merged
merged 2 commits into from Aug 15, 2019
Merged

Conversation

talyian
Copy link
Contributor

@talyian talyian commented Aug 14, 2019

Add some missing methods on LLVMPassManagerBuilderRef

@msftclas
Copy link

msftclas commented Aug 14, 2019

CLA assistant check
All CLA requirements met.

@EgorBo
Copy link
Member

EgorBo commented Aug 14, 2019

btw, does LlvmSharp support the new Pass manager?

@tannergooding
Copy link
Member

LLVMSharp just wraps all the APIs publicly exposed by the underlying LLVM 8.0.0 header files.

AFAIK, all the bindings are declared correctly so they should work if they are exposed.

@@ -13,5 +13,32 @@ public partial struct LLVMPassManagerBuilderRef : IEquatable<LLVMPassManagerBuil
public bool Equals(LLVMPassManagerBuilderRef other) => Pointer == other.Pointer;

public override int GetHashCode() => Pointer.GetHashCode();

public void Dispose() {
Copy link
Member

Choose a reason for hiding this comment

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

nit: This repo uses allman style braces (new line before the brace) as per the .editorconfig file:
https://github.com/microsoft/LLVMSharp/blob/master/.editorconfig#L254

If you are using an IDE or editor with a compatible .editorconfig extension and a supported Roslyn plugin (VS has one built-in and VS Code has one you can install), then the document should be automatically formatted with the correct rules as you are working.

@tannergooding
Copy link
Member

Overall LGTM.

Just a nit where the coding style deviates from the .editorconfig.

@tannergooding tannergooding merged commit 96cd847 into dotnet:master Aug 15, 2019
@tannergooding
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants