hubert.reinterpretcast added inline comments.
================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135 + llvm::MD5 Md5; + Md5.update(getModule().getSourceFileName()); + llvm::MD5::MD5Result R; ---------------- tmsriram wrote: > hubert.reinterpretcast wrote: > > davidxl wrote: > > > rnk wrote: > > > > davidxl wrote: > > > > > Source filenames are not guaranteed to be unique, or it does contain > > > > > the path as well? > > > > It appears to contain the path as the compiler receives it on the > > > > command line. However, it is possible to design a build where this > > > > string is not unique: > > > > ``` > > > > cd foo > > > > clang -c name_conflict.c > > > > cd ../bar > > > > clang -c name_conflict.c > > > > clang foo/name_conflict.o bar/name_conflict.o > > > > ``` > > > > > > > > I don't think we should try to handle this case, but we should document > > > > the limitation somewhere. Some build systems do operate changing the > > > > working directory as they go. > > > > > > > > Can you add some to the clang/docs/UsersManual.rst file? I'd expect it > > > > to show up here: > > > > https://clang.llvm.org/docs/UsersManual.html#command-line-options > > > > > > > > You can elaborate that the unique names are calculated using the source > > > > path passed to the compiler, and in a build system where that is not > > > > unique, the function names may not be unique. > > > > > > > > --- > > > > > > > > I have also used this construct in the past for building single-file > > > > ABI compatibility test cases: > > > > > > > > ``` > > > > // foo.cpp > > > > #ifdef PART1 > > > > // ... > > > > #elif defined(PART2) > > > > // ... > > > > #endif > > > > > > > > $ cc -c foo.cpp -DPART1 > > > > $ cc -c foo.cpp -DPART2 > > > > ``` > > > yes, the first example was the scenario I meant. I agree name conflict > > > due that case should be really rare. If yes, we can always use content > > > based uniqueid -- by appending llvm::getUniqueModuleId -- but that is > > > probably overkill. > > > yes, the first example was the scenario I meant. I agree name conflict > > > due that case should be really rare. > > Except for projects where it is the rule and not the exception. One pattern > > where this occurs is when each subdirectory implements one class and there > > are multiple classes that implement the same interface. In each directory, > > the implementation of each class is separated into files by grouping > > methods of the class. The set of filenames in one such directory may well > > be the same as the set in another. > I am not sure much can be done here other than try using getUniqueModuleId. > Just to be clear, even if the file names are the same, the actual conflict > happens only when the "make " is done from the leaf directory. Doing: > > cc a/name_conflict.cpp -c > cc b/name_conflict.cpp -c > > should still produce unique names. Understood. I do happen to have been working on a project that does the whole `$(MAKE) -C a/` thing with such a project layout. Content based hashing is unfortunate for other reasons: The hash would be unstable during the course of maintenance on the codebase. I guess the limitation is okay for the intended use case of the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
