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

[compute] Apply fold expressions #14677

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ragmani
Copy link
Contributor

@ragmani ragmani commented Feb 14, 2025

This commit applies fold expressions.

ONE-DCO-1.0-Signed-off-by: ragmani [email protected]

This commit applies fold expressions.

ONE-DCO-1.0-Signed-off-by: ragmani <[email protected]>
@ragmani
Copy link
Contributor Author

ragmani commented Feb 14, 2025

With Fold Expressions

Pros:

Conciseness: Eliminates boilerplate code such as explicit loops or temporary arrays, resulting in more succinct code.

Declarative Style: Clearly expresses that a condition must hold for all elements, improving readability for those familiar with the construct.

Compile-Time Evaluation: When applicable, the fold expression is evaluated at compile time, potentially reducing runtime overhead.

Cons:

Steep Learning Curve: Developers unfamiliar with C++17 fold expressions may find the syntax confusing or non-intuitive.

Debugging Challenges: Errors or unexpected behavior within fold expressions can be harder to trace compared to traditional loops.

Limited Flexibility: For complex operations that require different handling for each element, a fold expression may be less adaptable than an explicit loop.


Without Fold Expressions (Traditional Loop)

Pros:

Familiarity: More developers are accustomed to traditional loops, which can lead to easier understanding and maintenance.

Easier Debugging: Iterative code is often simpler to step through and debug, as each iteration is explicit.

Flexibility: Allows for more complex logic per iteration, offering greater control when different processing is needed for each element.

Cons:

Verbosity: Requires more boilerplate code, such as loop constructs and possibly temporary containers, which can clutter the code.

Imperative Style: The intent of "all elements must satisfy a condition" is less declaratively expressed compared to a fold expression.

Potential for Errors: More explicit code may lead to mistakes (e.g., off-by-one errors) that are less likely in a concise fold expression.

};

// Apply the lambda to each check shape and combine with &&
return (match(check_shapes) && ...);
}

struct UNUSED_ALL
Copy link
Contributor Author

@ragmani ragmani Feb 14, 2025

Choose a reason for hiding this comment

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

The structure UNUSED_ALL can be replaced with (void(check_shapes), ...);. But There are no overhead difference, readability, and conciseness.

@ragmani ragmani requested a review from a team February 14, 2025 04:23
@ragmani ragmani added the PR/ready for review It is ready to review. Please review it. label Feb 14, 2025
Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants