Skip to content

Code Comment Inconsistency #454

@YichiZhang0613

Description

@YichiZhang0613

In gltf-main/gltf-json/src/root.rs, the comment "Panics if the vector has [u32::MAX] or more elements" indicates function will panic when vector's length is u32::MAX.

 /// Panics if the vector has [`u32::MAX`] or more elements, in which case an `Index` cannot be
    /// created.
    pub fn push(vec: &mut Vec<T>, value: T) -> Index<T> {
        let len = vec.len();
        let Ok(index): Result<u32, _> = len.try_into() else {
            panic!(
                "glTF vector of {ty} has {len} elements, which exceeds the Index limit",
                ty = std::any::type_name::<T>(),
            );
        };

        vec.push(value);
        Index::new(index)
    }

But following test doesn't panic, which means function will not panic when vector's length is u32::MAX as comment indicates, it actually will panic when vector's length is u32::MAX + 1.

#[test]
fn test(){
    let some_object = "hello";
    let mut vec = Vec::new();
    for _ in 0..u32::MAX {
        Index::push(&mut vec, some_object);
    }
    assert_eq!(Index::push(&mut vec, some_object), Index::new(u32::MAX as u32));
}

The same situation occurs on

/// # Panics
    ///
    /// Panics if there are already [`u32::MAX`] or more elements of this type,
    /// in which case an `Index` cannot be created.
    #[track_caller]
    pub fn push<T>(&mut self, value: T) -> Index<T>
    where
        Self: AsMut<Vec<T>>,
    {
        Index::push(self.as_mut(), value)
    }

So maybe the 2 comments above should be modified and following tests should be added

#[test]
#[should_panic]
fn index_push_exceeds_limit(){
    let some_object = "hello";
    let mut vec = vec!["hello";u32::MAX as usize + 1];
    Index::push(&mut vec, some_object);
}

#[test]
fn index_push_reach_limit(){
    let some_object = "hello";
    let mut vec = Vec::new();
    for _ in 0..u32::MAX {
        Index::push(&mut vec, some_object);
    }
    assert_eq!(Index::push(&mut vec, some_object), Index::new(u32::MAX as u32));
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions