[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 8 09:19:50 PST 2017

sammccall added a comment.

In https://reviews.llvm.org/D39799#919468, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D39799#919415, @sammccall wrote:
> > Currently the working directory is always the parent of `compile_flags.txt` as you suggest.
> >  Maybe this isn't great though - sometimes it might be important that the WD is the parent of the source file?
> Parent of the source file seems rather inconvenient most of the time. Even when reading the contents of `compile_flags.txt` one would have to consider it the contexts of every source file.

Understood, this is the best behavior I think. I'm just wondering whether it's a good enough heuristic to be silently applied.
e.g. IIUC, things like `#include "sibling.h"` won't look for the h file next to the cc file?

> I'm looking at CompilationDatabase.cpp:321 <https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/lib/Tooling/CompilationDatabase.cpp;317699$321> and don't see the `CompileCommand::Directory` being set at all. Maybe I'm missing something.

`Result` is initialized with a copy of `CompilerCommands`, whose element is initialized with the directory passed to the constructor.

> A few ideas for tests:
> 1. Test relative include paths (`-Irelpath/to/include`) work.

Definitely will do this.

> 2. What happens if both `compile_commands.json` and `compile_flags.txt` are present? Should we test and document it? I'd expect the first one to take priority and the second one to come into play only if the input file is not found in the first one.

Ideally yes... i'm not sure this is so easy with the registry mechanism though :-(
They'd have to be present at the same level, so I'm not sure this is a huge deal.


More information about the cfe-commits mailing list