Skip to content

Revert "Better AST value for nodes." #477

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.

Sign up for GitHub

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

Jump to bottom
Closed
mcserep wants to merge 4 commits into from
Closed

Revert "Better AST value for nodes." #477

mcserep wants to merge 4 commits into from

Conversation

@mcserep
Copy link
Collaborator

@mcserep mcserep commented Oct 13, 2020

#425 resulted in critical error for the parser component, as CodeCompass cannot even parse itself (see #471).
Implementing USR (#429) instead of mangled names will probably fix this issue (as it eliminates the erroneous getMangledName() method completely), but that PR also fails for segmentation fault currently.

As the master branch contains such a serious issue for several weeks now and the immediate solution cannot be seen yet, I suggest to revert #425, as it introduced the bug in the first place.

@mcserep mcserep added Kind: Bug ⚠️ Kind: Critical 💥 Plugin: C++ Issues related to the parsing and presentation of C++ projects. labels Oct 13, 2020
@zporky
Copy link
Collaborator

zporky commented Oct 13, 2020

I agree to revert this change but only after testing that the reverted code is working fine (and CodeCompass can compile themselves).
We still have to understand the actual chain of events leading to the crash.

@mcserep
Copy link
Collaborator Author

mcserep commented Oct 14, 2020

I agree to revert this change but only after testing that the reverted code is working fine (and CodeCompass can compile themselves).

For me CodeCompass can parse itself with this modification reverted.
In the CI this test fails unfortunately, because we run out of disk space. (All system dependencies + compiled CodeCompass + parse > 24GB as it seems like). I will try to find a solution for this, as it would be nice to see the CI pass before merging this PR.

@whisperity
Copy link
Contributor

whisperity commented Oct 14, 2020

> 24 GiB

Wait.. what... HOW?!
Did we start parsing the LLVM dependencies, too?

@mcserep
Copy link
Collaborator Author

mcserep commented Oct 14, 2020

> 24 GiB

Wait.. what... HOW?!
Did we start parsing the LLVM dependencies, too?

On second check we do not have 24GB free space, because that only applies to the minimal environment and we use generic.
https://docs.travis-ci.com/user/common-build-problems/#im-running-out-of-disk-space-in-my-build

I will try out the minimal image maybe to gain more initial free space.

@mcserep mcserep force-pushed the revert-425-better_astvalue branch 2 times, most recently from 493a1d6 to 97ae8bd Compare October 14, 2020 23:04
@mcserep
CI: Switch to minimal image.
3478644 
Only install required Boost components.
Extend Xenial and Bionic PostgreSQL build with postgresql server install.
Build and install CodeCompass in $HOME instead of $TRAVIS_BUILD_DIR, so the build and install directories won't be parsed on the self-parse test.
Run CodeCompass self-parse test on number of available CPU cores.
Replace the relative include path of Functions.cmake in webgui/InstallGUI.cmake with an absolute, so it works when the build directory is not in the source directory.
@mcserep mcserep force-pushed the revert-425-better_astvalue branch from dd601a3 to 3478644 Compare October 15, 2020 07:21
@mcserep
Copy link
Collaborator Author

mcserep commented Oct 15, 2020

With the minimal image there is no issue, the CodeCompass self-parse test passes.
So in conclusion, the 24GB space is enough @whisperity. It turns out the generic image contained quite a lot of preinstalled software (see #479), thus the available free space was more restricted.

@mcserep mcserep mentioned this pull request Oct 15, 2020
Merged
@mcserep
Copy link
Collaborator Author

mcserep commented Oct 15, 2020

Original issue #471 solved by #429 , so closing this. Improvements on CI have been moved to separate PR (#479).

@mcserep mcserep closed this Oct 15, 2020
@mcserep mcserep deleted the revert-425-better_astvalue branch October 15, 2020 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Kind: Bug ⚠️ Kind: Critical 💥 Plugin: C++ Issues related to the parsing and presentation of C++ projects.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mcserep @zporky @whisperity