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

fix: only emit must_use hint when wdf function has return type #122

Merged
merged 45 commits into from
Apr 24, 2024

Conversation

wmmc88
Copy link
Collaborator

@wmmc88 wmmc88 commented Mar 7, 2024

Previously, the macro would emit must_use regardless of the return type WDF function being called, including ones that returned (). Now, the macro will parse the generated bindings to determine if there is a return type, before emitting must_use. this removes the need for ugly code blocks to avoid compiler warnings like:

let [()] = [macros::call_unsafe_wdf_function_binding!(
                WdfSpinLockAcquire,
                self.wdf_spin_lock
            )];

After this change:

macros::call_unsafe_wdf_function_binding!(WdfSpinLockAcquire, self.wdf_spin_lock);

This also stabilizes the must_use functionality so that its available without the nightly feature enabled. cargo bloat was used to validate that the new implementation with the nested function does not increase code size in release mode.

To make the macro more unit-testable and maintainable, its logic was broken into 4 distinct parts that are individually unit-testable:

  1. Parse macro inputs into Inputs
  2. Based off of Inputs, generate new AST fragments in DerivedASTFragments
  3. Use the DerivedASTFragments to create the major blocks of the macro generates in IntermediateOutputASTFragments
  4. Piece all the IntermediateOutputASTFragments into the tokens output by the macro

Currently blocked on:

@wmmc88 wmmc88 requested review from hamzamust, rajavenms and a team March 7, 2024 01:34
@wmmc88 wmmc88 self-assigned this Mar 7, 2024
@wmmc88 wmmc88 requested review from rajeshbg and removed request for rajavenms March 19, 2024 21:41
@wmmc88 wmmc88 force-pushed the better-macro-must-use branch 3 times, most recently from 1b95387 to 7c6aa49 Compare March 29, 2024 22:57
Makefile.toml Outdated Show resolved Hide resolved
Makefile.toml Outdated Show resolved Hide resolved
@wmmc88 wmmc88 marked this pull request as ready for review April 3, 2024 23:09
@wmmc88 wmmc88 requested a review from ayodejiige April 3, 2024 23:10
@wmmc88 wmmc88 force-pushed the better-macro-must-use branch from 1bc40b9 to 66d1dea Compare April 5, 2024 19:53
@wmmc88 wmmc88 added this pull request to the merge queue Apr 24, 2024
Merged via the queue into microsoft:main with commit 5d52a43 Apr 24, 2024
49 checks passed
@wmmc88 wmmc88 deleted the better-macro-must-use branch April 24, 2024 22:59
@wmmc88 wmmc88 mentioned this pull request Sep 27, 2024
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.

3 participants