Made type and value enums clone- and copy-able. #22
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22 +/- ##
==========================================
- Coverage 53.71% 53.17% -0.54%
==========================================
Files 35 37 +2
Lines 3742 3780 +38
==========================================
Hits 2010 2010
- Misses 1732 1770 +38
Continue to review full report at Codecov.
|
|
While we're at it, we should probably add Copy + Clone to the super traits? |
|
Mhm, such as the following snippet? pub trait AnyType: AsTypeRef + Debug + Clone + Copy { }In any case, I'd like to remove |
|
Yup, I think that should suffice for both macros |
|
Welp, that's not gonna be possible... There are two solutions:
Your thoughts? |
|
Generics are preferable to trait objects in almost all situations due to it being a compile time cost. The one case I can think of where trait objects are required is when we have input of mixed types (ie function parameters for one). We definitely can't get rid of those. So I guess we should just leave it out. Maybe later we can add a Trait <-> Enum mapping which allows us to convert between them, which would allow you to clone. But for now this should suffice. Please be sure to add |
src/values/traits.rs
Outdated
| @@ -22,6 +22,30 @@ macro_rules! trait_value_set { | |||
| ); | |||
| } | |||
|
|
|||
| /// Represents an aggregate value, built on top of other values. | |||
| pub trait AggregateValue: AnyValue { | |||
There was a problem hiding this comment.
I think this should be pub trait AggregateValue: BasicValue? Since AggregateValue is a subset of BasicValue?
There was a problem hiding this comment.
Oh yep, good catch. I'll fix this as soon as I can. No other problem to fix here?
There was a problem hiding this comment.
I'll give it one final look before I merge, but I did not see anything else
| /// Represents any LLVM type. | ||
| pub trait AnyType: AsTypeRef + Debug { | ||
| /// Returns an `AnyTypeEnum` that represents the current type. | ||
| fn as_any_type_enum(&self) -> AnyTypeEnum { |
There was a problem hiding this comment.
It's interesting to note that these look a little bit like associated types, though not exactly the same
There was a problem hiding this comment.
They kind of do, although not in a generic way
|
Thanks for the PR! |
Description
Added
#[derive(Clone, Copy)]toAnyValueEnum,AggregateValueEnum,BasicValueEnum,BasicMetadataValueEnum,BasicTypeEnumandAnyTypeEnum.Related Issue
Fixes #14.
Motivation and Context
Allows values and types (which are opaque pointers) to be copied and cloned at will.
How Has This Been Tested?
Existing tests have been run without a problem. The change is implemented by the Rust compiler, and shouldn't cause any issue.
Types of Changes
Required
Desired