[PATCH] D94169: [clang][driver] Restore the original help text for `-I`
andreil99 accepted this revision.
andreil99 added a comment.
This revision is now accepted and ready to land.
Thanks for suggesting `DocBrief`, Richard!
Looks good to me with a nit.
Comment at: clang/include/clang/Driver/Options.td:652
Flags<[CC1Option,CC1AsOption]>, MetaVarName<"">,
-HelpText<"Add directory to include search path. If there are multiple -I "
- "options, these directories are searched in the order they are "
- "given before the standard system directories are searched. "
- "If the same directory is in the SYSTEM include search paths, for
"
- "example if also specified with -isystem, the -I option will be "
- "ignored">;
+HelpText<"Add directory to include search path">,
+DocBrief<[{Add directory to include search path. For C++ inputs, if
How about `Add directory to the end of the list of include search paths` or
something similar? There is an order in which the directories are used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94169/new/
https://reviews.llvm.org/D94169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D94169: [clang][driver] Restore the original help text for `-I`
andreil99 added a comment. Post commit review is a normal practice in the LLVM community. This is not an excuse to revert somebody's patch per se, unless there are other serious reasons for the revert. The text does not describe “clang internals”, it describes the semantic of that flag with respect to other flags specifically to clang (I’m sure the text could be improved and open to suggestions). At the time when the original patch was done this td file was not shared with flang and such plans weren’t announced, it was quite the opposite, one of the flang driver RFCs specifically mentioned a separate *.td file, if memory serves me well. And I do apologize if I missed something, I didn’t follow flang discussions closely back in July/August of the last year. Now since you have started sharing, we need to find a good way to have front-end specifics in the documentation, which happens to be auto-generated from a shared td file. Stripping it down to only matching subset of things between multiple different front-ends would hurt documentation quality. Let’s discuss here of what could be done about that, or feel free to move the discussion somewhere else. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94169/new/ https://reviews.llvm.org/D94169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D94169: [clang][driver] Restore the original help text for `-I`
andreil99 requested changes to this revision. andreil99 added a comment. This revision now requires changes to proceed. I'm fine with having this text in the ClangCommandLineReference.rst only, and removing it from `clang -help`. However, reverting the patch would not do, as ClangCommandLineReference.rst is auto-generated from `clang/include/clang/Driver/Options.td`. Please feel free to propose a patch for this. The rationale for the change was multiple requests from our users, since this is important for them and is what they have to discover by experiments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94169/new/ https://reviews.llvm.org/D94169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71625: [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.
andreil99 accepted this revision. andreil99 added a comment. This revision is now accepted and ready to land. Thanks, Vlad! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71625/new/ https://reviews.llvm.org/D71625 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache
andreil99 accepted this revision. andreil99 added a comment. This revision is now accepted and ready to land. Thanks for catching and fixing this, Sergej! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70499/new/ https://reviews.llvm.org/D70499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D69651: [CMake] Add cross Windows to ARM Linux toolchain CMake cache file.
andreil99 accepted this revision. andreil99 added a comment. This revision is now accepted and ready to land. Thanks, Vlad! Looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69651/new/ https://reviews.llvm.org/D69651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
