[PATCH] D73307: Unique Names for Functions with Internal Linkage

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135
+    llvm::MD5 Md5;
+    Md5.update(getModule().getSourceFileName());
+    llvm::MD5::MD5Result R;
----------------
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73307/new/

https://reviews.llvm.org/D73307