preservim / tagbar

Vim plugin that displays tags in a window, ordered by scope
https://preservim.github.io/tagbar
Other
6.1k stars 485 forks source link

bug: c++ `override` virtual functions aren't shown in tagbar #831

Closed aerosayan closed 1 year ago

aerosayan commented 1 year ago

Hello, I absolutely love this plugin!

Found a somewhat important bug. In c++, virtual functions that use overridde aren't being recognized as functions. override is quite important for showing that it overrides a function of the base class. It's very important for understanding someone else's code, or even our code. qtcreator uses an italic font to denote overriden virtual functions, but using override specifier is recommended by c++ committee.

PROOF : In the attached picture, function ReadGrid is not being recognized, but ReadCellDataset is. There are more than 9 more overriden functions in this file, and none of them are being shown in tagbar.

I know it's kind of painful to keep up with c++'s syntax, but would really appreciate fixing this.

Thanks!

bug

raven42 commented 1 year ago

Have you checked if universal ctags prints a value for the ReadGrid() override function? The tagbar tags are all based on what ctags outputs. So if ctags does not have the function listed, then tagbar doesn't know anything about it.

To check this you can run the following (at least I think... I'm not on my development computer right now, so I can't verify the command is exactly correct):

ctags  -f - --format=2 --excmd=pattern --fields=nksSafet --sort=no --append=no --language-force=c++ --c++-kinds=hdpgetncsufmv <file>
aerosayan commented 1 year ago

Thanks for your help.

I have exuberant ctags installed, and maybe that's why it's not recognizing the code.

$ ctags --version
Exuberant Ctags 5.9~svn20110310, Copyright (C) 1996-2009 Darren Hiebert
  Addresses: <dhiebert@users.sourceforge.net>, http://ctags.sourceforge.net
  Optional compiled features: +wildcards, +regex

I'll report back after I try to install universal ctags from this: https://github.com/universal-ctags/ctags

Do you need portion of my code to verify if it works for you?

Here's some functions and data members from the struct...

It's kind of long, but that might make it easy to verify if it works for you ...

#pragma once

template<typename TypeConfig>
struct SU2ASCIIGridFileReader : public GridFileReaderInterface<TypeConfig>
{
private:
    //  Setup local types
    using Index = typename TypeConfig::IndexType;
    using Float = typename TypeConfig::FloatType;
    using String = typename TypeConfig::StringType;
    using ErrorCode = Index;

public:
    //  Grid data
    Vector<Array<TypeConfig,Index,5>> mCells;    // Node indices that make up each cell
    Vector<Array<TypeConfig,Float,2>> mNodes;    // Node co-ordinates

    virtual ErrorCode ReadGrid(const String filepath) override
    {
        //  Open file
        this->mFilepath = filepath;
        std::ifstream ifile(this->Filepath());
        if(!ifile.is_open())
        {
            std::cerr
                << "ERROR: File could not be opened" << std::endl
                << "Filepath that was tried to be opened: " << this->Filepath() << std::endl
                ;
            exit(1);
        }

        std::cout << "Reading grid filepath: " << filepath << std::endl;

        //  Read grid data
        oxAssertErrorCode(this->JumpToCellDataset(ifile));
        oxAssertErrorCode(this->ReadCellDataset(ifile));
        oxAssertErrorCode(this->JumpToNodeDataset(ifile));
        oxAssertErrorCode(this->ReadNodeDataset(ifile));
        oxAssertErrorCode(this->JumpToMarkerDataset(ifile));
        oxAssertErrorCode(this->ReadMarkerDataset(ifile));

        return 0;
    }

    virtual ErrorCode ReadCellDataset(std::ifstream & ifile) override
    {
        //  Go to first line of the file.
        ifile.clear();
        ifile.seekg(0,std::ios::beg);

        //  Skip all lines before cell dataset
        for(Index i=0; i<this->CellDatasetLineNum()-1; ++i)
        {
            ifile.ignore(4096, '\n');
        }

        //   Read the number of cells
        {    // local scope
        String s;
        ifile >> s; // Read the dataset name, discard it
        ifile >> this->mNumCells; // Read the number of cells
        }    // local scope

        //  Allocate memory
        auto & cells = this->mCells;
        cells.resize(this->NumCells());

        //  Read node indices that make up the cells
        for(Index i=0; i<this->NumCells(); ++i)
        {
            //  Cell data
            //  i1    : Cell type
            //  i2-i5 : Node indices that make the cell
            //  i6    : Cell index, i.e index location of this cell in the cell list
            Index i1, i2, i3, i4, i5, i6;

            //  Read cell type
            ifile >> i1;

            if(i1 == 5)
            {
                //  Read 4 cell values only for this triangle cell
                ifile >> i2 >> i3 >> i4 >> i6;

                //  Prevent access out of bounds
                assert(i6>=0 and i6<this->NumCells());

                //  Store cell data
                cells[i6][0] = i2;
                cells[i6][1] = i3;
                cells[i6][2] = i4;
                cells[i6][3] = 0;
                cells[i6][4] = 3;
            }
            else
            if(i1 == 9)
            {
                //  Read 5 cell values only for this quadrilateral cell
                ifile >> i2 >> i3 >> i4 >> i5 >> i6;

                //  Prevent access out of bounds
                assert(i6>=0 and i6<this->NumCells());

                //  Store cell data
                cells[i6][0] = i2;
                cells[i6][1] = i3;
                cells[i6][2] = i4;
                cells[i6][3] = i5;
                cells[i6][4] = 4;
            }
            else
            {
                std::cerr
                    << "ERROR: Cell type not supported" << std::endl
                    << "Cell type read was: " << i1 << std::endl
                    ;
                exit(1);
            }
        }
        return 0;
    }

    virtual ErrorCode ReadNodeDataset(std::ifstream & ifile) override
    {
        return 0;
    }

    virtual ErrorCode ReadMarkerDataset(std::ifstream & ifile) override
    {
        return 0;
    }

    virtual ErrorCode JumpToCellDataset(std::ifstream & ifile) override
    {
        this->mCellDatasetLineNum = JumpToDataset(ifile, "NELEM");
        std::cout << "Grid file's cell dataset was found on line number: " << this->CellDatasetLineNum() <<std::endl;
        return 0;
    }

    virtual ErrorCode JumpToNodeDataset(std::ifstream & ifile) override
    {
        this->mNodeDatasetLineNum = JumpToDataset(ifile, "NPOIN");
        std::cout << "Grid file's node dataset was found on line number: " << this->NodeDatasetLineNum() <<std::endl;
        return 0;
    }

    virtual ErrorCode JumpToMarkerDataset(std::ifstream & ifile) override
    {
        this->mMarkerDatasetLineNum = JumpToDataset(ifile, "NMARK");
        std::cout << "Grid file's marker dataset was found on line number: " << this->MarkerDatasetLineNum() <<std::endl;
        return 0;
    }

protected:
    //  Start our search from the first line, and find the dataset section of given name.
    virtual Index JumpToDataset(std::ifstream & ifile, const String datasetName)
    {
        // Data
        Index lineNum=1;    // Current line number being searched
        String line;        // Temporary string to store each line
        bool hasFoundDataset = false; // Have we found what we're searching?

        //  Go to first line of the file.
        ifile.clear();
        ifile.seekg(0,std::ios::beg);

        //  Search until we find the dataset or reach end of file
        while((hasFoundDataset == false) and std::getline(ifile, line))
        {
            if(line.find(datasetName)!=String::npos)
            {
                hasFoundDataset = true;
            }
            else
            {
                lineNum++;
            }
        }

        //  Sanity check
        if(hasFoundDataset == false)
        {
            std::cerr
                << "ERROR: Dataset was not found." << std::endl
                << "Dataset searched was: " << datasetName << std::endl
                << "File searched was: " << this->Filepath() << std::endl
                ;
            exit(1);
        }

        return lineNum;
    }

};
alerque commented 1 year ago

At this point exhuberant ctags variants are completely archaic and we've been only supporting the universal ctags variant for a while. We haven't ripped out all the old support yet but that may happen at some point since it's a constant source of confusion and bug reports. Encourage your distro and any other relevant projects to switch ;-) If uctags outputs something we're not parsing feel free to report back. Also know they are pretty open to contributions if there is something they are not parsing, and after they do we can get support added here.

aerosayan commented 1 year ago

A few updates from me, because others will also face similar problems in future too. Sot this might be helpful in future.

To be clear, my problem isn't solved. These are just the steps I took.

I installed a new version of ctags from snap, and updated the alternatives list for ctags. Still didn't work.

My guess is, tagbar probably directly calls /usr/bin/ctags or /bin/ctags or something in /usr/local/bin/ctags which is not where my snap binary is stored i.e /snap/bin/ctags

Not sure how to proceed. Adding the snap binary location to tagbar might help, but I'm not sure.

On my side, replacing /usr/bin/ctags with /snap/bin/ctags might work, and that's what I'll try next.

Detailed Steps Taken:

Ubuntu repo has older universal-tags compiled in 2019, which I installed but doesn't solve the problem.

$ ctags --version
Universal Ctags 0.0.0, Copyright (C) 2015 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: Jan  6 2019, 23:23:29
  URL: https://ctags.io/
  Optional compiled features: +wildcards, +regex, +iconv, +option-directory, +xpath, +json, +interactive, +sandbox, +yaml

Well, I tried to install ctags from snap.

$ sudo snap install universal-ctags

But ctags was not using this new version from snap, so we have to register it as an alternative for ctags, and use it. SOURCE : https://askubuntu.com/questions/1304472/update-alternatives-list-editor-does-not-show-editors-installed-via-snap

$ sudo update-alternatives --install /usr/bin/ctags ctags /snap/bin/ctags 1111
$ sudo update-alternatives --config ctags                                     
There are 5 choices for the alternative ctags (providing /usr/bin/ctags).

  Selection    Path                      Priority   Status
------------------------------------------------------------
  0            /snap/bin/ctags            1111      auto mode
  1            /snap/bin/ctags            1111      manual mode
  2            /usr/bin/ctags-exuberant   30        manual mode
  3            /usr/bin/ctags-universal   30        manual mode
  4            /usr/bin/ctags.emacs       28        manual mode
* 5            /usr/bin/ctags28           27        manual mode

Press <enter> to keep the current choice[*], or type selection number: 1

Well, now ctags, is using the new version from snap that's compiled on 2022.

$ ctags --version
Universal Ctags 5.9.0(ac824e67), Copyright (C) 2015-2022 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: Jul 20 2022, 06:16:34
  URL: https://ctags.io/
  Optional compiled features: +wildcards, +regex, +gnulib_regex, +iconv, +option-directory, +xpath, +json, +interactive, +yaml, +packcc, +optscript

But, the problem is still there. I can't see the overriden functions.

These are the results of where and which.

$ where ctags
/usr/bin/ctags
/bin/ctags
/snap/bin/ctags
/snap/bin/ctags

$ which ctags
/usr/bin/ctags
aerosayan commented 1 year ago
$ realpath /usr/bin/ctags
/usr/bin/snap

Surprisingly /usr/bin/ctags now links to /usr/bin/snap, which kind of makes sense since snap will execute ctags in a container. Might be useful information, in debugging the issue, or supporting snap in future.

alerque commented 1 year ago

You should be able to set g:tagbar_ctags_bin to specify the path you want to run.

let g:tagbar_ctags_bin = "/snap/bin/ctags"
aerosayan commented 1 year ago

@alerque Thanks!

Setting let g:tagbar_ctags_bin = "/usr/bin/ctags-universal" fixed the bug for me!

Surprisingly it was the 2019 compiled version.

$ /usr/bin/ctags-universal --version
Universal Ctags 0.0.0, Copyright (C) 2015 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: Jan  6 2019, 23:23:29
  URL: https://ctags.io/
  Optional compiled features: +wildcards, +regex, +iconv, +option-directory, +xpath, +json, +interactive, +sandbox, +yaml

Using /snap/bin/ctags actually caused an error which just blinks for less than one-tenth of a second and disappears, It might be interesting for the dev team since snap has the latest version of ctags and others might want to use it!

Success screenshot:

Even the lambdas and type-names are visible. Which is based.

success

alerque commented 1 year ago

We know picking the right ctags is a fiasco, but I don't think there is much we can do about it. Distros package this in all sorts of ways including miss-labeling packages (both ways!) and the binaries themselves don't even always properly identify themselves with --version. Containerized packaging like flatpack and snap have made this even harder to navigate.

We've gotten so many support requests along these lines I thought of even not having ANY path out of the box and requiring all users enter a hard coded full path—but that would unfairly penalize the few distros that just do the right thing and ship sane fresh packages.

If you can think of where in the docs you looked and would have expected to see more about this earlier on we can maybe improve the UX that way. Otherwise it seems this is resolved because this plugin is parsing the info just fine if you provide a working ctags.