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 optional feature to share objectives between nodes #2754

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

BAGUVIX456
Copy link
Contributor

fixes issue #1917

@BAGUVIX456
Copy link
Contributor Author

Do tell me if I have overlooked anything. Also, is this okay as a compile-time option?

@tokatoka
Copy link
Member

tokatoka commented Dec 8, 2024

Also, is this okay as a compile-time option?

yeah

@domenukk domenukk requested a review from addisoncrump December 9, 2024 11:17
@tokatoka tokatoka marked this pull request as draft December 10, 2024 09:45
@BAGUVIX456
Copy link
Contributor Author

Does LibAFL have a preferred way to tackle conditional trait bounds based on compile-time features? I am having to add optional trait bounds by dividing implementations based on features, and is causing massive code duplication

@tokatoka
Copy link
Member

The implementation of MaybeHasClientPerfMonitor could answer your question

@BAGUVIX456
Copy link
Contributor Author

@tokatoka is this issue fixed? Should I stop working on this issue?

@tokatoka
Copy link
Member

no i closed because you have this PR

@domenukk
Copy link
Member

I suggest we leave issues open until they are actually fixed in main

@BAGUVIX456 BAGUVIX456 marked this pull request as ready for review December 15, 2024 18:48
@BAGUVIX456 BAGUVIX456 requested a review from tokatoka December 15, 2024 18:49
@BAGUVIX456 BAGUVIX456 marked this pull request as draft December 15, 2024 19:27
@BAGUVIX456
Copy link
Contributor Author

@tokatoka could you explain what are you trying to do in PRs #2761 and #2745? i am having trouble fixing conflicts

@BAGUVIX456 BAGUVIX456 marked this pull request as ready for review December 16, 2024 22:12
@BAGUVIX456
Copy link
Contributor Author

Any idea why msrv is failing? cargo hack worked fine for me locally

@Mrmaxmeier
Copy link
Contributor

Mrmaxmeier commented Dec 17, 2024

Any idea why msrv is failing? cargo hack worked fine for me locally

Seems like the home crate updated their MSRV without a major version bump so building with fresh/up-to-date crates leads to a MSRV mismatch

}
}

#[cfg(feature = "share_objectives")]
Copy link
Member

Choose a reason for hiding this comment

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

do you need to do like this?
you can just add to the previous impl LlmpEventConverter

Copy link
Member

Choose a reason for hiding this comment

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

I would even prefer to do this with a runtime flag, the overhead is minimal since there are not too many objectives found. Then the code is cleaner I think(?)

@BAGUVIX456
Copy link
Contributor Author

Should I try implementing this in some other way?

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.

4 participants