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

Stdlib linked list #491

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ChetanKarwa
Copy link

This PR is to submit the first draft of the linked list module for stdlib.
This is based upon #68 by Milan, #463 by me.

I have been working on this module as a part of my GSoC Project.
All my work was regularly updated on my personal repository that I have been sharing with everyone in my weekly report.
Link to my repository (here).
A bit detailed project report is available (here).
List of all the weekly Blogs - (here)

The module was tested in two compilers, intel oneAPI and gfortran.

@awvwgk awvwgk added topic: container (Abstract) data structures and containers reviewers needed This patch requires extra eyes labels Aug 21, 2021
@zoziha zoziha mentioned this pull request Aug 26, 2021
7 tasks
@awvwgk
Copy link
Member

awvwgk commented Sep 13, 2021

How should we review this patch best? While there is an implementation I don't find a specification, examples or tests in this PR.

@arjenmarkus
Copy link
Member

arjenmarkus commented Sep 15, 2021 via email

@jvdp1 jvdp1 added the waiting for OP This patch requires action from the OP label Nov 10, 2021
@awvwgk
Copy link
Member

awvwgk commented Dec 19, 2021

Any update on this feature?

@arjenmarkus
Copy link
Member

arjenmarkus commented Dec 20, 2021 via email

Copy link
Contributor

@zoziha zoziha left a comment

Choose a reason for hiding this comment

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

This linked list is very good 👍 and can store almost any data type, I leave a little suggestion based on my limited knowledge, maybe not quite right :)

deallocate(current_node)
this_child_list%num_nodes = this_child_list%num_nodes - 1
end do

Copy link
Contributor

@zoziha zoziha Mar 2, 2022

Choose a reason for hiding this comment

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

Suggested change
nullify (this_child_list%head, this_child_list%tail)

It seems that the head and tail of the linked list need to be blanked here, otherwise an error will be reported when a new push node is pushed again.

current_node = this_node
next_node => current_node%next
do
deallocate(current_node)
Copy link
Contributor

@zoziha zoziha Mar 2, 2022

Choose a reason for hiding this comment

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

Suggested change
deallocate(current_node)
call current_node%clear()
deallocate(current_node)

It seems that the destruction of the current node content should be added here.


if (index == node_index) then
! Return the pointer to item stored at specified index
return_item => current_node%item
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a routine can be abstracted here, allowing the user to customize the way to get the contents of the linked list?

...
generic :: get => get_node_at_index, &
                  get_node_at_index_user
procedure, private :: get_node_at_index, &
                      get_node_at_index_user
...
abstract interface
        subroutine get_value(this_item, return_item)
            class(*), intent(in) :: this_item
            class(*), intent(out) :: return_item
        end subroutine get_value
end interface
...
subroutine get_node_at_index_user(this_list, node_index, func, return_item)
        class(child_list), intent(inout) :: this_list
        integer, intent(in) :: node_index
        procedure(get_value) :: func
        class(*), intent(out) :: return_item
        type(node), pointer :: curr_node
        integer index

        curr_node => this_list%head
        index = 1
        do while (associated(curr_node))
            if (index == node_index) then
                call func(curr_node%item, return_item)
                nullify (curr_node)
                return
            end if

            curr_node => curr_node%next
            index = index + 1
        end do
        nullify (curr_node)

end subroutine get_node_at_index_user
...

The user can use it like this:

program main

    use linked_list_m, only: child_list, rk, ik
    implicit none
    real(rk) :: x
    integer(ik) :: n

    type(this_child) :: list
    print *, list%size()

    call list%push(1.0_rk)
    call list%push(2_ik)

    call list%get(1, get_value, x)
    call list%get(2, get_value, n)

    print *, x, n, list%size()

contains

    subroutine get_value(this_item, return_item)
        class(*), intent(in) :: this_item
        class(*), intent(out) :: return_item

        select type (this_item)
        type is (real(rk))
            select type (return_item)
            type is (real(rk))
                return_item = this_item
            class default
                print *, '*<ERROR>* get_value: type mismatch'
            end select
        type is (integer(ik))
            select type (return_item)
            type is (integer(ik))
                return_item = this_item
            class default
                print *, '*<ERROR>* get_value: type mismatch'
            end select
        class default
            print *, "*<ERROR>* get_value: invalid type"
        end select

    end subroutine get_value

end program main


end do
nullify(current_node)
nullify(return_item)

This comment was marked as abuse.

Comment on lines +54 to +57
class(*), intent(in), optional :: new_item

! allocating new_item to the new node's item
allocate(new_node%item, source=new_item)
Copy link
Contributor

Choose a reason for hiding this comment

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

source uses the optional argument new_item here, there may be memory access errors.

@14NGiestas 14NGiestas linked an issue Jul 5, 2022 that may be closed by this pull request
@awvwgk
Copy link
Member

awvwgk commented Jul 30, 2022

I guess this PR has become stale. Since we already have a decent implementation here, should we try to update the PR, address the review comments and get it merged?

@jvdp1
Copy link
Member

jvdp1 commented Jul 31, 2022

I guess this PR has become stale. Since we already have a decent implementation here, should we try to update the PR, address the review comments and get it merged?

Maybe @arjenmarkus / @chetan has some news/updates. Specs and tests are mainly missing.

@milancurcic milancurcic mentioned this pull request Oct 18, 2022
@arjenmarkus
Copy link
Member

I have (finally, sigh ;)) started setting up a test program. The documentation will follow with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewers needed This patch requires extra eyes topic: container (Abstract) data structures and containers waiting for OP This patch requires action from the OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linked list
5 participants