nickdiego / compiledb

Tool for generating Clang's JSON Compilation Database files for make-based build systems.
GNU General Public License v3.0
1.38k stars 123 forks source link

Include '-f' flags from compiler line #42

Closed pamarcos closed 6 years ago

pamarcos commented 6 years ago

I'm not sure why we have a whitelist of flags we want to take from the compiler line, but there's a ton of flags we'll miss without the -f family (sanitizers, for instance).

I'd prefer if we just took all flags from the compiler line, but at least this fixes another use case.

nickdiego commented 6 years ago

I'm not sure why we have a whitelist of flags we want to take from the compiler line, but there's a ton of flags we'll miss without the -f family (sanitizers, for instance).

So, compiledb-generator started as a fork of YCM-generator, so it inherited most of its parsing logic. I must confess that I don't know why this whitelist regex logic is necessary.

I'd prefer if we just took all flags from the compiler line, but at least this fixes another use case. I agree with you about this, it would make the parsing far less complex, but we need to make sure if there isn't really some flag we would like to ignore. Also, I added some unit tests to make it possible to improve compiledb (mainly parsing logic) without casing regressions. Could you please add some tests for this change?

Thanks for your contribution.

pamarcos commented 6 years ago

The CI check is still pending, but I have no way of re-triggering it. Also, after your explanation of why there's a filtering for flags, I think it's way safer to remove that filtering since people can use a very wide variety of flags and all of them are needed to re-create a build.

pamarcos commented 6 years ago

Superseded by https://github.com/nickdiego/compiledb-generator/pull/46