[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-10-05 Thread Ben Langmuir via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG074fcec1eabf: [clang][deps] Canonicalize module map path (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://review

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-10-05 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 465544. benlangmuir added a comment. Rebased after landing https://reviews.llvm.org/D135220, which should hopefully fix the Windows test failure (it fixed the path for me locally on Windows, but I had some python-related issues that prevented me from run

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-30 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 464313. benlangmuir added a comment. Canonicalize path separator(s) in between the (already canonicalized directory) and the filename. This could cause issues with redundant path separators (`foo//bar`) or non-native separators (`foo/bar` vs `foo\bar` on

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 464117. benlangmuir added a comment. Simplify modmap checks in test to use PREFIX CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/Lex/HeaderSear

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment. (You could also use the `PREFIX` variable you're passing to `FileCheck` to remove some `.*` regexes.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 ___ cfe-commits ma

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 464109. benlangmuir added a comment. Fix windows path separators using sed instead of regex "." CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land. Thanks, this LGTM with the test extension. Comment at: clang/test/ClangScanDeps/modules-symlink-dir.c:33 +// CHECK-NOT: symlink-to-module +// CHECK: "

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 464104. benlangmuir added a comment. Test -fmodule-map-file paths as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/Lex/HeaderSearch.cpp

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added a comment. In D134923#3825673 , @jansvoboda11 wrote: > IIUC this not only canonicalizes path to the main input file when compiling a > PCM, but also `-fmodule-map-file=` arguments for dependencies, correct? Since > that behavior is de

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:1307 + // Do not canonicalize within the framework; the module map parser expects + // Modules/ not Versions/A/Modules. + if (llvm::sys::path::filename(Dir) == "Modules") { Is that bec

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment. IIUC this not only canonicalizes path to the main input file when compiling a PCM, but also `-fmodule-map-file=` arguments for dependencies, correct? Since that behavior is desirable, I think it would make sense to check for that in the test, WDYT? Repository:

[PATCH] D134923: [clang][deps] Canonicalize module map path

2022-09-29 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When dep-scanning, canonicalize the module map path as much as