[PATCH] D49890: Clang-Tidy Export Problem

Ahmad Nouralizadeh via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 14 17:00:58 PDT 2018


TheAhmad added a comment.

In https://reviews.llvm.org/D49890#1182556, @alexfh wrote:

> Could you describe the specific problem you're solving and provide an example? As mentioned by others, a test would be very welcome as well.


Sorry for so much delay,  @alexfh. I didn't see your comment. I will describe in detail:
I wanted to do a source to source transformation on `MPlayer-1.3.0` source code. The transformation may require modification of many files and possibly repeated modifications in the headers files included in multiple `.c` files. Therefore, the changes should be serialized for each translation unit and stored in a `YAML` file. At the end, `clang-apply-replacements` will be called and transform the entire source code.
The problem is that `clang-tidy` expects a limited format for the compilation database. This is the format typically used when the build system generating the compilation database is `CMAKE`.  But `MPlayer` uses `Makefile`. Therefore, I had to use an external database generator, `Bear`.  In this case, the contents of the `YAML` files are OK. But it is not what is expected by `clang-tidy`.  `clang-tidy` requires every file path to be absolute, even the header files.
The problem (i.e., using relative paths) only arises when the fixes are `exported`. Not when they are applied `in-place`. I reused some of the code for the in-place case and did some modifications to it. The code is OK, at least for my case with `MPlayer`. A small change is still needed to support `merge conflicts` which can be brought from the `in-place fix` stuff. It seems that at the end the commanlities of the two cases should be put in a function. Then this function can be called from both places (i.e., the `in-place fix` and the `export fix`).
I am new to `Clang` and do not know what is needed for tests. I am looking forward to your reply.
Regards.



================
Comment at: ClangTidy.cpp:614
+  vfs::FileSystem &FileSystem = *Files->getVirtualFileSystem();
+  auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
+  if (!InitialWorkingDir)
----------------
JonasToth wrote:
> TheAhmad wrote:
> > Eugene.Zelenko wrote:
> > > Type is not obvious, so please don't use auto.
> > Hi, Eugene. Why line 352 uses auto?
> He means line 615 (`InitialWorkingDir`). The type of the variable can not be deduced from reading the code.
> 
> The rule is, to write the type once. E.g. `llvm::make_unique<MyType>(args)` makes it clear, that the type is `MyType`, so you can use `auto` for the variable.
> This is not the case for `InitialWorkingDir`.
Right. I agree. So `line 352` should not use `auto` either.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49890





More information about the cfe-commits mailing list