[PATCH] D52524: Add -Wpoison-system-directories warning

manojgupta added a comment. An earlier version did check for library directories [1]. I am not exactly sure why was it removed, maybe it didn't work. So if anyone is willing to test that, please apply the diff and try. [1] Diff https://reviews.llvm.org/D52524?id=215958 CHANGES SINCE LAST ACTI

[PATCH] D52524: Add -Wpoison-system-directories warning

aaron.ballman added a comment. In D52524#2721073 , @sbc100 wrote: > Would it make sense to have this apply to system library paths too? i.e. > `-L/usr/local/lib`. > > We have a bespoke mechanism in emscripten right now that is a bit more > flexible but

[PATCH] D52524: Add -Wpoison-system-directories warning

sbc100 added a comment. Would it make sense to have this apply to system library paths too? i.e. `-L/usr/local/lib`. We have a bespoke mechanism in emscripten right now that is a bit more flexible but I'd love to simple replace it with `-Wpoison-system-directories`. The name "system director

[PATCH] D52524: Add -Wpoison-system-directories warning

manojgupta closed this revision. manojgupta added a comment. Submitted as https://reviews.llvm.org/rL371785. Thanks for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 ___ cfe-commits mail

[PATCH] D52524: Add -Wpoison-system-directories warning

denik marked 2 inline comments as done. denik added inline comments. Comment at: clang/lib/Frontend/InitHeaderSearch.cpp:141-143 + if (HasSysroot) { +if (MappedPathStr.startswith("/usr/include") || +MappedPathStr.startswith("/usr/local/include")) { a

[PATCH] D52524: Add -Wpoison-system-directories warning

denik updated this revision to Diff 219924. denik added a comment. Combined two if into one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/lib/Frontend/InitHeaderSearch.cpp

[PATCH] D52524: Add -Wpoison-system-directories warning

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor nit, this LGTM. However, I'm not the most familiar with how cross-compiling works in the first place, so I may be the wrong one to approve this. ===

[PATCH] D52524: Add -Wpoison-system-directories warning

denik added a comment. Ping @aaron.ballman , please verify the change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D52524: Add -Wpoison-system-directories warning

denik added a comment. Hi Aaron, Thank you for your review. I updated the diff with suggested changes. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1072 +// cross-compiling. +def PoisonSystemDirectories : DiagGroup<"poison-system-directories">; +

[PATCH] D52524: Add -Wpoison-system-directories warning

denik updated this revision to Diff 217746. denik marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/lib/Frontend/InitHeaderSearch.cpp clang/test/

[PATCH] D52524: Add -Wpoison-system-directories warning

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1072 +// cross-compiling. +def PoisonSystemDirectories : DiagGroup<"poison-system-directories">; + Do you envision more warnings being added to this group? If not, I wo

[PATCH] D52524: Add -Wpoison-system-directories warning

denik updated this revision to Diff 216021. denik added a comment. Removed check for libraries. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/include/clang/Basic/DiagnosticGro