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

Remove unfinished features like debug info handling #26

Closed
langston-barrett opened this issue Oct 5, 2022 · 1 comment
Closed

Remove unfinished features like debug info handling #26

langston-barrett opened this issue Oct 5, 2022 · 1 comment
Assignees

Comments

@langston-barrett
Copy link
Collaborator

When beginning work on #12, I realized that I would have to update parts of the fact generator that deal with LLVM debug information. This seems tedious and unnecessary, given that I don't think the analysis really depends in an essential way on the content of the debug information. Indeed, notes in the code indicate that the current support for debug info is still incomplete:

//UNFINISHED (needs debuginfo)

Similarly, there's a bunch of code related to C++ support that appears unfinished, either because it was never completed as part of cclyzer, or because it wasn't completely ported over from LogicBlox to Souffle:

// secondary_superclass(Supertype, Type) <-

I propose to remove unfinished code from cclyzer++. While it is a shame to remove what might be valuable work, it's hard to imagine how this work could provide value without considerable amounts of reverse-engineering. There are no tests and very limited commentary describing what the code is supposed to do, which makes it hard to have any confidence that it's doing it correctly. In the meantime, like all code, it makes the project overall more costly to maintain, slows down builds, and makes it harder to understand the complete project.

@langston-barrett
Copy link
Collaborator Author

The debug info handling is removed. The C++ stuff doesn't involve code in the FactGenerator, so it won't slow down upgrading to more recent versions of LLVM. Instead, #83 took care of taking it out of the default analysis builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant