[PATCH] D135192: Fix incorrect check for running out of source locations.

ppluzhnikov added a comment. In D135192#3866294 , @dblaikie wrote: > Sounds good (guess this is pretty impractical to have a regression test/would > require very large files to test?) Yea, overflowing this requires a very convoluted setup; I don't

[PATCH] D135192: Fix incorrect check for running out of source locations.

ppluzhnikov added a comment. Richard, ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135192/new/ https://reviews.llvm.org/D135192 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D135192: Fix incorrect check for running out of source locations.

ppluzhnikov added a reviewer: HAPPY. ppluzhnikov added a comment. Richard, could you take a look? You've added the check in commit 78d81ecfc38c2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135192/new/ https://reviews.llvm.org/D135192

[PATCH] D135192: Fix incorrect check for running out of source locations.

ppluzhnikov created this revision. Herald added a project: All. ppluzhnikov published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. When CurrentLoadedOffset is less than TotalSize, current code will trigger unsigned overflow and will not return

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

ppluzhnikov added a comment. Thanks for the review. Could you commit this? I don't have commit rights. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125742/new/ https://reviews.llvm.org/D125742 ___

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

ppluzhnikov added a comment. Thanks for the review. Comments addressed in new revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125742/new/ https://reviews.llvm.org/D125742 ___ cfe-commits

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

ppluzhnikov updated this revision to Diff 434828. ppluzhnikov marked 4 inline comments as done. ppluzhnikov added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125742/new/ https://reviews.llvm.org/D125742

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov abandoned this revision. ppluzhnikov added a comment. This proved to be too hard :-( A smaller change: https://reviews.llvm.org/D126396 fixed one aspect of this problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

ppluzhnikov added a comment. Ping x2? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125742/new/ https://reviews.llvm.org/D125742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127009: [test] Modify test case so it verifies the fix from D126396

ppluzhnikov added a comment. I think I've done the cleanups requested. If this looks good, please push it (I don't have commit rights). Thanks, Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127009/new/ https://reviews.llvm.org/D127009

[PATCH] D127009: [test] Modify test case so it verifies the fix from D126396

ppluzhnikov updated this revision to Diff 434202. ppluzhnikov added a comment. Update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127009/new/ https://reviews.llvm.org/D127009 Files: clang/test/Preprocessor/file_test.c Index:

[PATCH] D127009: Modify test case so it verifies the fix from D126396.

ppluzhnikov updated this revision to Diff 434199. ppluzhnikov added a comment. Make comment better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127009/new/ https://reviews.llvm.org/D127009 Files: clang/test/Preprocessor/file_test.c Index:

[PATCH] D127009: Modify test case so it verifies the fix from D126396.

ppluzhnikov updated this revision to Diff 434198. ppluzhnikov marked an inline comment as done. ppluzhnikov added a comment. Add comment and suppress clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127009/new/

[PATCH] D126396: Clean "./" from __FILE__ expansion.

ppluzhnikov added a comment. In D126396#3556321 , @MaskRay wrote: > Is it feasible to add a test? Sure. I extended the existing tests added in https://reviews.llvm.org/D122766 to cover this. https://reviews.llvm.org/D127009 Repository: rG LLVM

[PATCH] D127009: Modify test case so it verifies the fix from D126396.

ppluzhnikov created this revision. ppluzhnikov added a reviewer: MaskRay. Herald added a subscriber: StephenFan. Herald added a project: All. ppluzhnikov requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D126396: Clean "./" from __FILE__ expansion.

ppluzhnikov added a comment. Alan, Thanks for the approval. Could you push this patch -- I don't have commit rights. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126396/new/ https://reviews.llvm.org/D126396

[PATCH] D126396: Clean "./" from __FILE__ expansion.

ppluzhnikov created this revision. Herald added a project: All. ppluzhnikov updated this revision to Diff 432064. ppluzhnikov added a comment. Herald added a subscriber: hiraditya. ppluzhnikov updated this revision to Diff 433837. ppluzhnikov published this revision for review. Herald added a

[PATCH] D126412: Fix a buglet in remove_dots().

ppluzhnikov added a comment. In D126412#3553796 , @rnk wrote: > do you need someone to push this? Yes, please. I do not have commit rights. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126412/new/

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

ppluzhnikov added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125742/new/ https://reviews.llvm.org/D125742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D126412: Fix a buglet in remove_dots().

ppluzhnikov created this revision. Herald added a subscriber: hiraditya. Herald added a project: All. ppluzhnikov updated this revision to Diff 432324. ppluzhnikov added a comment. ppluzhnikov updated this revision to Diff 432461. ppluzhnikov updated this revision to Diff 432551. ppluzhnikov

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:546 + {"include/wordexp.h", ""}, + {"include/x86intrin.h", ""}, + {"include/xlocale.h", ""}, ilya-biryukov wrote: > ppluzhnikov wrote:

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

ppluzhnikov created this revision. ppluzhnikov added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. ppluzhnikov requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:218 + llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false); + Filename = CleanFilename; + kadircet wrote: > this is actually breaking the [contract of >

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov planned changes to this revision. ppluzhnikov added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:24 + static auto *Mappings = + new std::array, 645>{{ + {"algorithm", ""}, ilya-biryukov wrote: >

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 428693. ppluzhnikov added a comment. Fix clangd test failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 Files:

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 428293. ppluzhnikov added a comment. Herald added subscribers: usaxena95, kadircet. Fix FIXME in CanonicalIncludesTests.cpp (yet another Winx64 failure). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 428275. ppluzhnikov added a comment. Fix one more Winx64 failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 Files:

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 427526. ppluzhnikov added a comment. Fix more UNIX and Winx64 failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 Files:

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 427460. ppluzhnikov added a comment. Herald added a subscriber: carlosgalvezp. Fix Winx64 `clang-tidy/checkers/google-upgrade-googletest-case.cpp` failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov added a comment. > Ignoring the ".." path components for the moment, is this patch good to go as > is? It doesn't affect that behavior. This patch still breaks Winx64 `clang-tidy/checkers/google-upgrade-googletest-case.cpp`. The failure log here

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 416633. ppluzhnikov added a comment. More Debian and Win x64 fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 Files:

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov planned changes to this revision. ppluzhnikov added a comment. Pending further Win x64 failure cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov added a comment. > There's also some others where I wouldn't expect them to be failing in this > patch, eg. the ones from `/` -> `{{[/\\]}}`. These are failing because `remove_dots` (un-intuitively) also changes "foo/bar\\baz" to "foo\\bar\\baz" > Are there tests that we can't

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov added a comment. > @ppluzhnikov, can you give more context on how this interacts with > https://reviews.llvm.org/D121658? I had a quick look but it wasn't > immediately obvious. D121658 broke Winx64 test due to "./" vs. ".\\" mismatch. This

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 416330. ppluzhnikov added a comment. Use single quotes in sed -- don't want shell expansion there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 Files:

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov updated this revision to Diff 416255. ppluzhnikov added a comment. Herald added a project: clang-tools-extra. Fix Win x64 failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 Files:

[PATCH] D121733: Clean pathnames in FileManager.

ppluzhnikov created this revision. Herald added subscribers: dexonsmith, arphaman. Herald added a project: All. ppluzhnikov requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This reduces visual noise ("./foo.h" -> "foo.h") in error messages.

[PATCH] D121658: Use llvm::sys::path::append to fix FIXME

ppluzhnikov abandoned this revision. ppluzhnikov added a comment. Breaks Windows tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121658/new/ https://reviews.llvm.org/D121658 ___ cfe-commits mailing

[PATCH] D121658: Use llvm::sys::path::append to fix FIXME

ppluzhnikov created this revision. Herald added a project: All. ppluzhnikov requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121658 Files: clang/lib/Lex/HeaderSearch.cpp