arichardson added a comment. In D72829#1846157 <https://reviews.llvm.org/D72829#1846157>, @serge-sans-paille wrote:
> @arichardson doc updated, any other advice? No comments on the code just wondering if the documentation should be expanded. I just came across this review by chance and it was not clear to me what "Enable semantic interposition" means. I had to read the code and look the GCC documentation+llvm mailing list posts to understand it. As this is user-facing documentation I feel like there should be a slightly longer explaning what the option does. For comparison here is the GCC command line flag documentation -fsemantic-interposition Some object formats, like ELF, allow interposing of symbols by the dynamic linker. This means that for symbols exported from the DSO, the compiler cannot perform interprocedural propagation, inlining and other optimizations in anticipation that the function or variable in question may change. While this feature is useful, for example, to rewrite memory allocation functions by a debugging implementation, it is expensive in the terms of code quality. With -fno-semantic-interposition the compiler assumes that if interposition happens for functions the overwriting function will have precisely the same semantics (and side effects). Similarly if interposition happens for variables, the constructor of the variable will be the same. The flag has no effect for functions explicitly declared inline (where it is never allowed for interposition to change semantics) and for symbols explicitly declared weak. Other than this, the change looks good to me. ================ Comment at: clang/docs/ClangCommandLineReference.rst:907 + +.. option:: -fsemantic-interposition, -fno-semantic-interposition + ---------------- arichardson wrote: > This looks like it should be reordered? Sorry I mean that it looks like all other entries are `.. option::` followed by the description but here it's the other way around. It seems like this change will make the documention of -fsanitize-* be `Enable semantic interposition`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72829/new/ https://reviews.llvm.org/D72829 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
