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

set nil as the initial value of the struct reference type fields #51

Merged
merged 1 commit into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions autograd/gates_basic.v
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ pub fn (g &SubstractGate[T]) cache[T](mut result Variable[T], args ...CacheParam

pub struct MultiplyGate[T] {
pub:
a &Variable[T]
b &Variable[T]
a &Variable[T] = unsafe { nil }
b &Variable[T] = unsafe { nil }
}
Comment on lines 76 to 80
Copy link

Choose a reason for hiding this comment

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

The initialization of a and b to nil using unsafe blocks is a significant change that addresses potential issues with uninitialized memory. However, it is important to ensure that all code that creates instances of MultiplyGate properly initializes these fields before they are used. Failing to do so could lead to null pointer dereferences, which can cause crashes or other undefined behavior. It is also worth considering if there are safer ways to ensure initialization without resorting to unsafe blocks, such as providing default values or factory functions that guarantee proper initialization.


pub fn multiply_gate[T](a &Variable[T], b &Variable[T]) &MultiplyGate[T] {
Expand Down Expand Up @@ -119,8 +119,8 @@ pub fn (g &MultiplyGate[T]) cache[T](mut result Variable[T], args ...CacheParam)

pub struct DivideGate[T] {
pub:
a &Variable[T]
b &Variable[T]
a &Variable[T] = unsafe { nil }
b &Variable[T] = unsafe { nil }
}
Comment on lines 120 to 124
Copy link

Choose a reason for hiding this comment

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

Similar to the MultiplyGate, the DivideGate struct now initializes its a and b fields to nil within unsafe blocks. This change requires careful review to ensure that all instances of DivideGate are properly initialized before use. Additionally, the use of unsafe blocks should be justified and reviewed to ensure that the safety benefits outweigh the risks of bypassing the language's safety features.


pub fn divide_gate[T](a &Variable[T], b &Variable[T]) &DivideGate[T] {
Expand Down
4 changes: 2 additions & 2 deletions autograd/gates_blas.v
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import vtl.la

pub struct MatMulGate[T] {
pub:
a &Variable[T]
b &Variable[T]
a &Variable[T] = unsafe { nil }
b &Variable[T] = unsafe { nil }
}
Comment on lines 6 to 10
Copy link

Choose a reason for hiding this comment

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

The initialization of a and b fields in the MatMulGate struct to nil using unsafe blocks is a significant change. This change assumes that the fields will be properly initialized before they are used. It is crucial to ensure that all usages of MatMulGate are reviewed and updated to handle the possibility of nil values to prevent null pointer dereferences, which can lead to crashes or undefined behavior. Additionally, the use of unsafe blocks should be carefully documented, explaining why this approach is necessary and what precautions should be taken when working with these fields.


pub fn matmul_gate[T](a &Variable[T], b &Variable[T]) &MatMulGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion autograd/gates_exp.v
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import vtl

pub struct ExpGate[T] {
pub:
a &Variable[T]
a &Variable[T] = unsafe { nil }
}

pub fn exp_gate[T](a &Variable[T]) &ExpGate[T] {
Expand Down
4 changes: 2 additions & 2 deletions autograd/gates_pow.v
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import math
import vtl

pub struct PowGate[T] {
a &Variable[T]
b &Variable[T]
a &Variable[T] = unsafe { nil }
b &Variable[T] = unsafe { nil }
}

pub fn pow_gate[T](a &Variable[T], b &Variable[T]) &PowGate[T] {
Copy link

Choose a reason for hiding this comment

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

The pow_gate function signature has not been updated to reflect the changes in the PowGate struct. Since the fields a and b are now optional and initialized to nil, the function parameters a and b are no longer necessary for the struct initialization. If the intention is to keep the function signature as is and require the caller to provide a and b, then the struct should not initialize these fields to nil. If the fields are meant to be optional, the function signature should be updated accordingly, and the function body should handle the case where a and b might be nil.

Expand Down
6 changes: 3 additions & 3 deletions autograd/gates_trig.v
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import vtl

pub struct SinGate[T] {
pub:
a &Variable[T]
a &Variable[T] = unsafe { nil }
}

pub fn sin_gate[T](a &Variable[T]) &SinGate[T] {
Expand Down Expand Up @@ -37,7 +37,7 @@ pub fn (g &SinGate[T]) cache[T](mut result Variable[T], args ...CacheParam) ! {

pub struct CosGate[T] {
pub:
a &Variable[T]
a &Variable[T] = unsafe { nil }
}

pub fn cos_gate[T](a &Variable[T]) &CosGate[T] {
Expand Down Expand Up @@ -70,7 +70,7 @@ pub fn (g &CosGate[T]) cache[T](mut result Variable[T], args ...CacheParam) ! {

pub struct TanGate[T] {
pub:
a &Variable[T]
a &Variable[T] = unsafe { nil }
}

pub fn tan_gate[T](a &Variable[T]) &TanGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion autograd/node.v
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub mut:
// The variables that created this node
parents []&Variable[T]
// Wrapper around a Tensor, contains operation data
payload &Payload[T]
payload &Payload[T] = unsafe { nil }
// Debug use only, contains a name for a node
name string
}
Expand Down
2 changes: 1 addition & 1 deletion autograd/payload.v
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module autograd
pub struct Payload[T] {
pub:
// Contents of the paylod
variable &Variable[T]
variable &Variable[T] = unsafe { nil }
}
Comment on lines 7 to 11
Copy link

Choose a reason for hiding this comment

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

The change to initialize the variable field within the Payload struct to nil using an unsafe block is a significant alteration aimed at preventing the use of uninitialized memory. This is a good practice for safety, especially in a language like V that emphasizes memory safety. However, it is important to ensure that all code that relies on Payload is updated to handle the possibility of variable being nil. This change could potentially lead to runtime errors if the variable field is dereferenced without checking for nil. It is recommended to review all usages of Payload to ensure that proper nil checks or initializations are in place before dereferencing the variable field.


pub fn payload[T](variable &Variable[T]) &Payload[T] {
Expand Down
6 changes: 3 additions & 3 deletions autograd/variable.v
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ pub mut:
// The value of the Variable. This should not be edited outside
// of Variable operations, as other edits will not be tracked
// and will lead to incorrect results
value &vtl.Tensor[T]
value &vtl.Tensor[T] = unsafe { nil }
// The graph the variable is associated with. This is a reference,
// as a variable does not own its context
context &Context[T]
context &Context[T] = unsafe { nil }
// The gradient of the Variable. This is set as a reference to
// the value of a Variable unless `backprop` has been called, in
// which case all related Variables will have their gradient
// updated correctly
grad &vtl.Tensor[T]
grad &vtl.Tensor[T] = unsafe { nil }
// If set to true, this variable will track its operations,
// otherwise it will act similar to a vtl.Tensor, only calculating
// forward operations
Comment on lines 15 to 29
Copy link

Choose a reason for hiding this comment

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

The use of unsafe { nil } to initialize reference type fields is a critical change that addresses potential issues with uninitialized memory. However, it's important to ensure that all parts of the code that interact with these fields are now expecting and correctly handling nil values. This change could introduce nil pointer dereferences if the rest of the codebase is not updated accordingly. The team should verify that all usages of Variable fields are checked for nil before access and that appropriate error handling is in place.

Expand Down
8 changes: 4 additions & 4 deletions datasets/imdb.v
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ pub const (
// ImdbDataset is a dataset for sentiment analysis.
pub struct ImdbDataset {
pub:
train_features &vtl.Tensor[string]
train_labels &vtl.Tensor[int]
test_features &vtl.Tensor[string]
test_labels &vtl.Tensor[int]
train_features &vtl.Tensor[string] = unsafe { nil }
train_labels &vtl.Tensor[int] = unsafe { nil }
test_features &vtl.Tensor[string] = unsafe { nil }
test_labels &vtl.Tensor[int] = unsafe { nil }
}

// load_imdb_helper loads the IMDB dataset for a given split.
Expand Down
8 changes: 4 additions & 4 deletions datasets/mnist.v
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ pub const (
// MnistDataset is a dataset of MNIST handwritten digits.
pub struct MnistDataset {
pub:
train_features &vtl.Tensor[u8]
train_labels &vtl.Tensor[u8]
test_features &vtl.Tensor[u8]
test_labels &vtl.Tensor[u8]
train_features &vtl.Tensor[u8] = unsafe { nil }
train_labels &vtl.Tensor[u8] = unsafe { nil }
test_features &vtl.Tensor[u8] = unsafe { nil }
test_labels &vtl.Tensor[u8] = unsafe { nil }
}

// load_mnist_helper loads the MNIST dataset from the given filename.
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/activation/elu.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.nn.internal

pub struct EluGate[T] {
pub:
cache &vtl.Tensor[T]
cache &vtl.Tensor[T] = unsafe { nil }
}

pub fn elu_gate[T](cache &vtl.Tensor[T]) &EluGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/activation/leaky_relu.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.nn.internal

pub struct LeakyReluGate[T] {
pub:
cache &vtl.Tensor[T]
cache &vtl.Tensor[T] = unsafe { nil }
}

pub fn leaky_relu_gate[T](cache &vtl.Tensor[T]) &LeakyReluGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/activation/relu.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.nn.internal

pub struct ReLUGate[T] {
pub:
cache &vtl.Tensor[T]
cache &vtl.Tensor[T] = unsafe { nil }
}

pub fn relu_gate[T](cache &vtl.Tensor[T]) &ReLUGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/activation/sigmoid.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.nn.internal

pub struct SigmoidGate[T] {
pub:
cache &vtl.Tensor[T]
cache &vtl.Tensor[T] = unsafe { nil }
}

pub fn sigmoid_gate[T](cache &vtl.Tensor[T]) &SigmoidGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/layers/dropout.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.autograd
pub struct DropoutGate[T] {
pub:
prob f64
mask &vtl.Tensor[T]
mask &vtl.Tensor[T] = unsafe { nil }
}

pub fn dropout_gate[T](mask &vtl.Tensor[T], prob f64) &DropoutGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/layers/flatten.v
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import vtl.autograd

pub struct FlattenGate[T] {
pub:
input &autograd.Variable[T]
input &autograd.Variable[T] = unsafe { nil }
cached_shape []int
}

Expand Down
6 changes: 3 additions & 3 deletions nn/gates/layers/linear.v
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import vtl.stats

pub struct LinearGate[T] {
pub:
input &autograd.Variable[T]
weight &autograd.Variable[T]
bias &autograd.Variable[T]
input &autograd.Variable[T] = unsafe { nil }
weight &autograd.Variable[T] = unsafe { nil }
bias &autograd.Variable[T] = unsafe { nil }
}
Comment on lines 8 to 13
Copy link

Choose a reason for hiding this comment

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

The initialization of input, weight, and bias fields to nil using unsafe blocks is a significant change that could potentially lead to runtime errors if these fields are accessed before being properly initialized. It is crucial to ensure that all code paths that create instances of LinearGate properly initialize these fields before use. This change may require a thorough review of all usages of LinearGate to prevent null pointer dereferences.


pub fn linear_gate[T](input &autograd.Variable[T], weight &autograd.Variable[T], bias &autograd.Variable[T]) &LinearGate[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/gates/layers/maxpool.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.nn.internal

pub struct MaxPool2DGate[T] {
pub:
max_indices &vtl.Tensor[int]
max_indices &vtl.Tensor[int] = unsafe { nil }
kernel []int
shape []int
stride []int
Comment on lines 7 to 12
Copy link

Choose a reason for hiding this comment

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

The initialization of max_indices with unsafe { nil } is a significant change that ensures the field is explicitly set to a null pointer. This change is likely aimed at preventing the use of uninitialized memory, which can lead to undefined behavior or crashes. However, it is important to ensure that all code that interacts with MaxPool2DGate is updated to handle this new initialization pattern correctly. Any code that assumes max_indices to be non-null without checking could now potentially dereference a null pointer, leading to a crash. It is crucial to review all usages of MaxPool2DGate to ensure that they properly handle the possibility of max_indices being nil.

Expand Down
4 changes: 2 additions & 2 deletions nn/gates/loss/mse.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import vtl.nn.internal

pub struct MseGate[T] {
pub:
cache &autograd.Variable[T]
target &vtl.Tensor[T]
cache &autograd.Variable[T] = unsafe { nil }
target &vtl.Tensor[T] = unsafe { nil }
}

pub fn mse_gate[T](cache &autograd.Variable[T], target &vtl.Tensor[T]) &MseGate[T] {
Expand Down
4 changes: 2 additions & 2 deletions nn/gates/loss/sigmoid_cross_entropy.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import vtl.nn.internal

pub struct SigmoidCrossEntropyGate[T] {
pub:
cache &autograd.Variable[T]
target &vtl.Tensor[T]
cache &autograd.Variable[T] = unsafe { nil }
target &vtl.Tensor[T] = unsafe { nil }
}

pub fn sigmoid_cross_entropy_gate[T](cache &autograd.Variable[T], target &vtl.Tensor[T]) &SigmoidCrossEntropyGate[T] {
Expand Down
4 changes: 2 additions & 2 deletions nn/gates/loss/softmax_cross_entropy.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import vtl.nn.internal

pub struct SoftmaxCrossEntropyGate[T] {
pub:
cache &autograd.Variable[T]
target &vtl.Tensor[T]
cache &autograd.Variable[T] = unsafe { nil }
target &vtl.Tensor[T] = unsafe { nil }
Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

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

The initialization of cache and target fields to nil within the SoftmaxCrossEntropyGate struct is a critical change for preventing the use of uninitialized memory. However, it's important to ensure that all code paths that create instances of this struct properly initialize these fields before they are used. Failing to do so could lead to null pointer dereferences, which can cause runtime crashes. The use of unsafe blocks indicates that this is a deliberate bypass of the language's safety features, so extra caution should be taken when working with these fields.

}

pub fn softmax_cross_entropy_gate[T](cache &autograd.Variable[T], target &vtl.Tensor[T]) &SoftmaxCrossEntropyGate[T] {
Expand Down
4 changes: 2 additions & 2 deletions nn/layers/linear.v
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import vtl.nn.types

// LinearLayer is a layer that applies a linear transformation to its input.
pub struct LinearLayer[T] {
weights &autograd.Variable[T]
bias &autograd.Variable[T]
weights &autograd.Variable[T] = unsafe { nil }
bias &autograd.Variable[T] = unsafe { nil }
}

pub fn linear_layer[T](ctx &autograd.Context[T], input_dim int, output_dim int) types.Layer[T] {
Expand Down
2 changes: 1 addition & 1 deletion nn/models/sequential.v
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn init() {

pub struct Sequential[T] {
pub mut:
info &SequentialInfo[T]
info &SequentialInfo[T] = unsafe { nil }
}

// sequential creates a new sequential network with a new context.
Expand Down
2 changes: 1 addition & 1 deletion nn/models/sequential_info.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.nn.loss
import vtl.nn.types

pub struct SequentialInfo[T] {
ctx &autograd.Context[T]
ctx &autograd.Context[T] = unsafe { nil }
Copy link

Choose a reason for hiding this comment

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

The change to initialize ctx to nil using an unsafe block is a significant alteration to the default behavior of the SequentialInfo struct. This change should be carefully reviewed to ensure that all usages of SequentialInfo are updated to handle the possibility of ctx being nil. Additionally, since ctx is a reference to autograd.Context, it is crucial to ensure that there are no scenarios where a nil context could lead to a segmentation fault or other undefined behavior when dereferenced.

pub mut:
layers []types.Layer[T]
loss types.Loss
Expand Down
2 changes: 1 addition & 1 deletion src/iter.v
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub enum IteratorStrategy {
@[heap]
pub struct TensorIterator[T] {
pub:
tensor &Tensor[T]
tensor &Tensor[T] = unsafe { nil }
Copy link

Choose a reason for hiding this comment

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

The initialization of tensor to nil using an unsafe block is a significant change that addresses potential issues with uninitialized memory. However, it's important to ensure that all usages of TensorIterator throughout the codebase are updated to handle the possibility of tensor being nil. This change could lead to runtime errors if the tensor field is dereferenced without proper checks. It is recommended to add null checks or handle the nil case appropriately wherever TensorIterator is used.

next_handler IteratorStrategy
pub mut:
iteration int
Expand Down
2 changes: 1 addition & 1 deletion src/iter_axis.v
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module vtl
@[heap]
pub struct TensorAxisIterator[T] {
pub:
tensor &Tensor[T]
tensor &Tensor[T] = unsafe { nil }
pub mut:
shape []int
strides []int
Comment on lines 5 to 11
Copy link

Choose a reason for hiding this comment

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

The change to initialize the tensor field with unsafe { nil } is a significant one. It is important to ensure that all usages of TensorAxisIterator are reviewed to confirm that the tensor field is being properly initialized before use, as this change could potentially introduce null pointer dereferences if the field is accessed without proper initialization. This is a critical safety concern, especially in a language like V that emphasizes safety and simplicity.

Expand Down
2 changes: 1 addition & 1 deletion src/tensor.v
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub enum MemoryFormat {
@[heap]
pub struct Tensor[T] {
pub mut:
data &storage.CpuStorage[T]
data &storage.CpuStorage[T] = unsafe { nil }
memory MemoryFormat
size int
shape []int
Expand Down
2 changes: 1 addition & 1 deletion src/tensor_vcl_d_vcl.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import vtl.storage
@[heap]
pub struct VclTensor[T] {
pub mut:
data &storage.VclStorage[T]
data &storage.VclStorage[T] = unsafe { nil }
memory MemoryFormat
size int
shape []int
Comment on lines 6 to 12
Copy link

Choose a reason for hiding this comment

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

The initialization of data to nil using an unsafe block is a significant change that addresses potential issues with uninitialized memory. However, it is important to ensure that all code that interacts with VclTensor instances properly checks for nil before dereferencing data. Failing to do so could lead to null pointer dereference errors at runtime.

Expand Down
Loading