[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

tetsuo-cpp added a comment. Thanks for the review @nridge! I'd really appreciate if you could help me with the commit (I don't have commit access). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157990/new/ https://reviews.llvm.org/D157990 ___

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

tetsuo-cpp created this revision. tetsuo-cpp added reviewers: sammccall, nridge. tetsuo-cpp added projects: clang-tools-extra, All. Herald added subscribers: kadircet, arphaman. tetsuo-cpp requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. When using `clangd` for

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp added a comment. Friendly ping @njames93. Does this look ok to you? If so, could you please help me with the commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 ___ cfe-commits mailing l

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp marked an inline comment as done. tetsuo-cpp added a comment. @aaron.ballman Thanks! I took your suggestion and I think this looks cleaner. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 ___

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp updated this revision to Diff 280082. tetsuo-cpp added a comment. Cleanup check conditional. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 Files: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp clan

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp added a comment. Friendly ping. Could I please get this looked at? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp added a comment. Friendly ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp updated this revision to Diff 276022. tetsuo-cpp added a comment. Address comments: Rename variables, remove unnecessary tests and avoid unnecessary check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 Files: clang-tools-extra/cl

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp updated this revision to Diff 275592. tetsuo-cpp added a comment. Remove the unnecessary template. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 Files: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

tetsuo-cpp created this revision. tetsuo-cpp added reviewers: njames93, MaskRay. tetsuo-cpp added projects: clang, clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=45189 This patch is adding support for members to the `bugprone-b

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

tetsuo-cpp added a comment. In D75901#1920397 , @njames93 wrote: > In D75901#1916119 , @tetsuo-cpp > wrote: > > > @njames93 @MaskRay > > Thanks for helping me with testing. I'll remember this for next time. > > >

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

tetsuo-cpp marked an inline comment as done. tetsuo-cpp added a comment. @njames93 @MaskRay Thanks for helping me with testing. I'll remember this for next time. I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in some other checks (`RedundantExpressionCheck` and `Static

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

tetsuo-cpp updated this revision to Diff 249538. tetsuo-cpp added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75901/new/ https://reviews.llvm.org/D75901 Files: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp clang-tools

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

tetsuo-cpp created this revision. tetsuo-cpp added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. tetsuo-cpp added reviewers: njames93, MaskRay. Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=27702 I wasn't sure how this type of thin

[PATCH] D19031: [clang-format] Flexible line endings

cameron314 added a comment. I've got a rebased diff but Phabricator won't let me attach it to this differential review (too old?). What do I do? EDIT: Ah, it must be because I'm not the owner, despite it being editable by all. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19031/new/

[PATCH] D19031: [clang-format] Flexible line endings

cameron314 updated this revision to Diff 229533. cameron314 added a comment. Rebased diff on latest upstream. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19031/new/ https://reviews.llvm.org/D19031 Files: clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/unit

[PATCH] D19031: [clang-format] Flexible line endings

cameron314 added a comment. @MyDeveloperDay: @mxbOctasic has moved on but I'm still maintaining this patch upstream, so I'm still interested in having it merged if possible. The advantage of having two options is that a default line ending can still be specified when none can be derived (this i

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

tetsuo-cpp added a comment. @MyDeveloperDay Thanks for your suggestions! I think it's better now. This is my first LLVM patch (hopefully first of many) so I don't have commit permissions yet. If you're still happy with this change, could you please commit on my behalf? Repository: rC Clang

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

tetsuo-cpp updated this revision to Diff 219582. tetsuo-cpp added a comment. Addressed review comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67037/new/ https://reviews.llvm.org/D67037 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

tetsuo-cpp added a comment. Friendly ping. Could I please get this looked at? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67037/new/ https://reviews.llvm.org/D67037 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

tetsuo-cpp created this revision. tetsuo-cpp added a reviewer: klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. This option determines whether goto labels are indented according to scope. Setting this option to false causes goto labels to be flushed to the left. This

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

cameron314 added a comment. @yvvan: The clang frontend tests (`PCHPreambleTest` and friends) are disabled on Windows in the makefile (I think because the VFS tests depend on linux-like paths). So running the tests on Windows without failures is encouraging but not the whole story. Repository:

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

This revision was automatically updated to reflect the committed changes. Closed by commit rC322503: [PCH] Serialize skipped preprocessor ranges (authored by cameron314, committed by ). Changed prior to commit: https://reviews.llvm.org/D20124?vs=125333&id=129892#toc Repository: rC Clang htt

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. Excellent, I'll rebase and commit. Thanks everyone for your patience! https://reviews.llvm.org/D20124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. Ping? The patch is ready to go, just needs a final approval... https://reviews.llvm.org/D20124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

cameron314 added a comment. It's been a while since I was in this code, but I seem to recall the file needed to exist on disk in order to uniquely identify it (via inode). Does this break the up-to-date check? Repository: rC Clang https://reviews.llvm.org/D41005 _

[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

cameron314 added inline comments. Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365 cast(FD)->getParent(), ObjectType, - ObjectClassification, Args.slice(1), CandidateSet, + ObjectClassification, F

[PATCH] D39903: [libclang] Allow pretty printing declarations

cameron314 added a comment. Locally we've done something similar (adding a `clang_getCursorPrettyPrintedDeclaration` function, though without exposing the `PrintingPolicy`) and overhauled `DeclPrinter` to produce proper pretty names. This is a hack that works well for us, but can never be upstr

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 updated this revision to Diff 125333. cameron314 added a comment. Here's the final patch that fixes `clang_getSkippedRegions` with regions in the preamble (as well as serializing the skipped regions in the PCH file). https://reviews.llvm.org/D20124 Files: include/clang/Lex/Preproc

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. Brilliant, didn't know `isInPreambleFileID` existed. All my tests pass for me now with that change, thanks :-) I'll update the patch on Monday. https://reviews.llvm.org/D20124 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. In https://reviews.llvm.org/D20124#928552, @ilya-biryukov wrote: > Why do we store raw source locations in `PPSkippedRange`? Would storing > `SourceLocation` and using `ASTWriter::AddSourceLocation` and `ASTReader:: > ReadSourceLocation` do the trick? I followed t

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. Well, it seems like preamble PCH source location translation is fundamentally broken. The entry file has a single, positive file ID. The preamble PCH is treated as an imported module, so it has a negative file ID for the part that overlaps the preamble of the entry f

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. Alright, with my patch the `c-index-test` *does* correctly serialize and restore the skipped ranges; the problem is that it searches for only ranges matching the target file. When there's a preamble, it's seen as a different file than the main file, even though they'

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 marked an inline comment as done. cameron314 added a comment. - Well that's odd, because the test definitely fails for me without the patch. I'm only a few days behind the trunk. - I'm looking at your test case now. I can reproduce it even with the patch; I'm investigating what's happ

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 updated this revision to Diff 123044. cameron314 added a comment. Fully rebased, with a test. https://reviews.llvm.org/D20124 Files: include/clang/Lex/PreprocessingRecord.h include/clang/Serialization/ASTBitCodes.h include/clang/Serialization/ASTReader.h include/clang/Seriali

[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

cameron314 added inline comments. Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6396 } else { AddTemplateOverloadCandidate(FunTmpl, F.getPair(), ExplicitTemplateArgs, Args, The same slice that was added abov

[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

cameron314 added inline comments. Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365 cast(FD)->getParent(), ObjectType, - ObjectClassification, Args.slice(1), CandidateSet, + ObjectClassification, F

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

cameron314 added a comment. I'll rebase the patch and add a test. Thanks for looking at this! Repository: rL LLVM https://reviews.llvm.org/D20124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

cameron314 added a comment. @nik, pretty sure this fix I submitted ages ago fixes that. Been using it in production a couple years. Would be nice if someone reviewed it so we could finally upstream it. https://reviews.llvm.org/D38578 ___

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

This revision was automatically updated to reflect the committed changes. Closed by commit rL313796: [PCH] Fixed preamble breaking with BOM presence (and particularly, fluctuating… (authored by cameron314). Changed prior to commit: https://reviews.llvm.org/D37491?vs=116043&id=116049#toc Reposi

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 updated this revision to Diff 116043. cameron314 added a comment. Final diff. Test passes! https://reviews.llvm.org/D37491 Files: include/clang/Frontend/PrecompiledPreamble.h include/clang/Lex/Lexer.h include/clang/Lex/PreprocessorOptions.h lib/Frontend/FrontendActions.cpp

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 added inline comments. Comment at: include/clang/Lex/Lexer.h:52 + /// a BOM is present at the start of the file. + unsigned StartOffset; + /// \brief Size of the preamble in bytes. ilya-biryukov wrote: > cameron314 wrote: > > ilya-biryukov wrote: >

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 added inline comments. Comment at: include/clang/Lex/Lexer.h:52 + /// a BOM is present at the start of the file. + unsigned StartOffset; + /// \brief Size of the preamble in bytes. ilya-biryukov wrote: > We could simplify it further by removing `Sta

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 updated this revision to Diff 115104. cameron314 edited the summary of this revision. cameron314 added a comment. Alright, I've changed the patch so that the preamble takes into account the BOM presence and is invalidated when it changes. This automatically fixes all clients of `Preco

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

This revision was automatically updated to reflect the committed changes. Closed by commit rL312917: [PCH] Allow VFS to be used for tests that generate PCH files (authored by cameron314). Changed prior to commit: https://reviews.llvm.org/D37474?vs=114604&id=114612#toc Repository: rL LLVM ht

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 added inline comments. Comment at: include/clang/Frontend/PrecompiledPreamble.h:100 /// PreambleBounds used to build the preamble PreambleBounds getBounds() const; ilya-biryukov wrote: > Not introduced by this change, but could you also add a f

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 updated this revision to Diff 114604. cameron314 added a comment. Final diff, will commit soon. Thanks! https://reviews.llvm.org/D37474 Files: include/clang/Frontend/PrecompiledPreamble.h lib/Frontend/ASTUnit.cpp unittests/Frontend/CMakeLists.txt unittests/Frontend/PCHPreambl

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 added a comment. It seems there's other users of `PrecompiledPreamble` that would have to be fixed, yes. If we go with my original fix of taking into account the BOM in the preamble bounds, there's no way of reusing the PCH when the BOM appears/disappears. I still maintain this is a

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 updated this revision to Diff 114414. cameron314 added a comment. The latest patch. I think this one should do the trick :-) https://reviews.llvm.org/D37474 Files: include/clang/Frontend/PrecompiledPreamble.h lib/Frontend/ASTUnit.cpp unittests/Frontend/CMakeLists.txt unittest

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 added inline comments. Comment at: lib/Frontend/ASTUnit.cpp:1014 +/// with another virtual file system. +class PCHOverlayFileSystem : public vfs::FileSystem +{ ilya-biryukov wrote: > Maybe create a combination of `InMemoryFileSystem` and `OverlayFileSy

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 updated this revision to Diff 114228. cameron314 added a comment. Here's an updated patch. The code required to make it work is much simpler when the BOM is simply ignored :-) https://reviews.llvm.org/D37491 Files: include/clang/Frontend/PrecompiledPreamble.h include/clang/Lex/L

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 added a comment. > Maybe there's a third option option to remove the BOM from the buffer before > passing it to clang? > Could you elaborate on your use-case a little more? Is there no way to > consistently always pass buffers either with or without BOM? > Out of two options you men

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 added a comment. Looking at the way remapped buffers are handled, I just remembered that they must exist on the file system (at the very least, in a directory that exists) or the remapping is not taken into account. So that pretty much rules out the other approach, I think. https:/

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 updated this revision to Diff 114016. cameron314 added a comment. Here's an updated patch that allows only the PCH to be accessed from the real FS when a VSF is present. Tests still pass. https://reviews.llvm.org/D37474 Files: lib/Frontend/ASTUnit.cpp unittests/Frontend/CMakeLis

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 added a comment. I'll change the overlay to only allow access to the PCH. I agree that it would be nice to only read the PCH using real filesystem APIs in order to be symmetrical, but this seems non-trivial. It also feels like a step in the wrong direction -- ideally the VFS would ho

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 added a comment. Thanks for the response! > How are various preprocessor offests (and SourceLocation offsets) are > calculated? Do they account for BOM presence and ignore it? Everything is in byte offsets; the `SourceLocation` after the BOM is not the same as before the BOM. The le

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

cameron314 created this revision. This patch fixes preamble skipping when the preamble region includes a byte order mark (BOM). Previously, parsing would fail if preamble PCH generation was enabled and a BOM was present. This also fixes the preamble incorrectly being invalidated when a BOM appe

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 added a comment. I suppose we could do the overlay manually in all the tests that need it; I guess it depends on what the goal of the VFS support is. To my mind, it doesn't make sense that a file is placed in the RealFS in the first place if a VFS is used, but this is quite entrenche

[PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged

cameron314 abandoned this revision. cameron314 added a comment. This patch is obsolete. While waiting over a year for a review, somebody else came across the same fix (for a different manifestation of the same bug) in https://reviews.llvm.org/D27810 and managed to get it through. I think my test

[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

cameron314 created this revision. Herald added a subscriber: mgorny. When using a virtual file-system (VFS) and a preamble file (PCH) is generated, it is generated on-disk in the real file-system instead of in the VFS (which I guess makes sense, since the VFS is read-only). However, when subsequ

[PATCH] D27810: FileManager: mark virtual file entries as valid entries

cameron314 added a comment. The `IsValid = true` line coincidentally fixes a tangentially related bug -- see https://reviews.llvm.org/D20338 (in which I tried to introduce the same fix almost a year earlier, but nobody accepted the review). I guess I have to maintain the test for that case out

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

cameron314 added a comment. >> Thanks, I'll check these out! Btw, I noticed that the clang-format tests are >> non-Windows due to path assumptions. Is this a lost cause, or just something >> no one's bothered to look into yet? > > No one's bothered looking into it yet. Which tests? We run the