[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rL309810: Adapt clang-tidy checks to changing semantics of hasDeclaration. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D36154 Files: clang-tools-extra/trunk/clang-tidy/google/St

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

klimek added inline comments. Comment at: clangd/ClangdServer.h:105 +/// A helper class to pass concurrency parameters to ClangdScheduler. +class SchedulingParams { +public: I think calling it "Options" is more idiomatic. Comment at: clangd/Cla

[PATCH] D36308: Add handling for DeducedType to HasDeclarationMatcher

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Thanks! LG after comment change. Also, should we add some tests to clang-tidy? :) Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:745 + +// For deduced types

[PATCH] D35355: Fix templated type alias completion when using global completion cache

klimek added a comment. Sorry for missing this - can you add a test? https://reviews.llvm.org/D35355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

klimek added inline comments. Comment at: clangd/ClangdUnitStore.cpp:45 + .first; +Result.RemovedFile = nullptr; + } else if (!compileCommandsAreEqual(It->second->getCompileCommand(), Just say RemovedFile = nullptr in the struct? ==

[PATCH] D36397: [clangd] Fixed a data race.

klimek added a comment. Tests? https://reviews.llvm.org/D36397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36397: [clangd] Fixed a data race.

klimek added a comment. In https://reviews.llvm.org/D36397#833890, @ilya-biryukov wrote: > In https://reviews.llvm.org/D36397#833883, @klimek wrote: > > > Tests? > > > TSAN does not catch this (as it's a logical error) and it requires a rather > bizarre timing of file reparses to trigger. > I c

[PATCH] D36159: clang-format: [JS] handle single lines comments ending in `\\`.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Format/FormatTokenLexer.cpp:544-545 +while (BackslashPos != StringRef::npos) { + if (BackslashPos + 1 < FormatTok->TokenText.size() && +

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

klimek added a comment. High-level question: Why can't we use llvm::ThreadPool? https://reviews.llvm.org/D36261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

klimek added a comment. Also missing tests :) Comment at: clangd/ClangdUnitStore.cpp:45 + .first; +Result.RemovedFile = nullptr; + } else if (!compileCommandsAreEqual(It->second->getCompileCommand(), ilya-biryukov wrote: > klimek wrote: > > Jus

[PATCH] D35955: clang-format: Add preprocessor directive indentation

klimek added a comment. In https://reviews.llvm.org/D35955#834914, @djasper wrote: > Manuel: Can you take a look at the last comment here? Why does PPBranchLevel > start at -1? It's a perhaps too-clever implementation to make sure that we can have a per-branch data structure (indexed from 0)

[PATCH] D35955: clang-format: Add preprocessor directive indentation

klimek added a comment. In https://reviews.llvm.org/D35955#835994, @euhlmann wrote: > In https://reviews.llvm.org/D35955#835439, @klimek wrote: > > > I think if we need this info, we can just make it count down to -1 again > > (or, but that's isomorphic, let it run from 0 and make sure we never

[PATCH] D35355: Fix templated type alias completion when using global completion cache

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D35355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:470-471 + int FD; + auto EC = llvm::sys::fs::createTemporaryFile(Prefix, Suffix, /*ref*/ FD, + /*ref*/ File); if (EC) I don't underst

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG, as discussed in person, it's probably a good idea to try to get rid of the non-file-creating version, if possible, or at least fix the comments on the functions to make this behavior clear

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D36261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: unittests/clangd/ClangdTests.cpp:509-510 + /*RunSynchronously=*/true); + // No need to sync reparses, because RunSynchronously is set +

[PATCH] D35955: clang-format: Add preprocessor directive indentation

klimek added a comment. LG from my side. https://reviews.llvm.org/D35955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36397: [clangd] Fixed a data race.

klimek added inline comments. Comment at: clangd/ClangdUnit.cpp:714 -void CppFile::cancelRebuilds() { +void CppFile::cancelRebuild() { deferCancelRebuild().get(); } + Somewhat orthogonal question - why CppFile? We do support other languages in Clangd, right? (

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek updated this revision to Diff 292970. klimek marked 7 inline comments as done. klimek added a comment. Worked in review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296 Files: clang/lib/Fo

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/FormatToken.h:177 +/// EndOfExpansion: 0 0 0 21 0 1 +struct MacroContext { + MacroContext(MacroRole Role) : Role(Role) {} sammccall wrote: > "context" is often pretty vague - "MacroSource" i

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

klimek added a comment. FWIW, finding this due to seeing the added complexity. Do you have data on what the difference is on clang-format for either overall memory use or performance? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84306/new

[PATCH] D83296: [clang-format] Add a MacroExpander.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. klimek marked 5 inline comments as done. Closed by commit rGe336b74c995d: [clang-format] Add a MacroExpander. (authored by klimek). Changed prior to commit: https://

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek created this revision. klimek added a reviewer: sammccall. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. klimek requested review of this revision. MacroUnexpander applies the structural formatting of expanded lines into UnwrappedLines to the corresponding une

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

klimek added a comment. In D84306#2294952 , @MyDeveloperDay wrote: > Which bit do you find more complex? adding something to the FormatToken or > the use of the `is()` calls? I think mainly that a) the language doesn't support bitfields well yet, as ev

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} sammccall wrote: > you're pushing here without copying. This means the original tokens from the

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} sammccall wrote: > klimek wrote: > > sammccall wrote: > > > you're pushing here without copying.

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. In D83296#2299062 , @nridge wrote: > What does this change mean for users of clang-format -- better formatting of > complicated (e.g. multi-line) macro invocations? In addition to what Sam said, this also attempts to be an improve

[PATCH] D88956: [clang-format] Fix misformatted macro definitions after D86959

klimek added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8333 +TEST_F(FormatTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) { + // This is a regression test for mis-parsing the & after decltype as a binary I'd put this into a new tes

[PATCH] D88956: [clang-format] Fix misformatted macro definitions after D86959

klimek added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8333 +TEST_F(FormatTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) { + // This is a regression test for mis-parsing the & after decltype as a binary arichardson wrote: > aricha

[PATCH] D88952: [clang-format][tests] Fix MacroExpander lexer not parsing C++ keywords

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Nice catch, thanks! LG! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88952/new/ https://reviews.llvm.org/D88952 __

[PATCH] D91996: [clang-format] Remove double trim

klimek added a comment. Isn't the comment incorrect after this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91996/new/ https://reviews.llvm.org/D91996 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89920/new/ https://reviews.llvm.org/D89920 ___ cfe

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 301253. klimek marked 3 inline comments as done. klimek added a comment. Adapted based on review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files: clang

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added inline comments. Comment at: clang/lib/Format/FormatToken.h:449 + /// When macro expansion introduces parents, those are marked as + /// \c MacroParent, so formatting knows their children need to be formatted. sammccall wrote: > I can't really un

[PATCH] D96114: [ASTMatchers] Fix parent-child traversal between functions and parms

klimek added a comment. Can you explain the change and the before/after a bit more? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96114/new/ https://reviews.llvm.org/D96114 ___ cfe-commits mailin

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

klimek added a comment. In D69764#2934032 , @MyDeveloperDay wrote: > In D69764#2932929 , @steveire wrote: > >> > > @steveire, thanks for your comments, I also agree that a second tool > shouldn't be needed, espec

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

klimek added a comment. In D69764#2934686 , @aaron.ballman wrote: > In D69764#2934612 , @MyDeveloperDay > wrote: > >>> I was referring to @rsmith and @aaron.ballman (to clarify), both are >>> maintainers in 'clan

[PATCH] D114430: [clang-format] NFC - recent changes caused clang-format to no longer be clang-formatted.

klimek added a comment. Thanks for cleaning up after me, and sorry for the mess - do y'all have clang-format set up as a presubmit or do you just remember to format manually? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114430/new/ https://review

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 390057. klimek marked 49 inline comments as done. klimek added a comment. Work in review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files: clang/lib/For

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. Noticed I should have waiting with the renaming of the file until the review is done :( Sorry for the extra confusion. Comment at: clang/lib/Format/MacroUnexpander.cpp:77 + // stream, we need to continue the unexpansion until we find the right token +

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3804910 , @owenpan wrote: > @sstwcw thanks for pointing it out. See D134329 > . Thanks Owen, really appreciate this! And sorry for not getting to it yet myself :( Repository: rG LLV

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: All. klimek requested review of this revision. Herald added a project: clang. In preparation for configured macro replacements in formatting, add the ability to insert tokens to FormatTokenSource, and impleme

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. Herald added a project: All. Hey ho, sorry for the late comment here, but adding peekNextToken(n) is problematic, as this gets in the way of future changes we want to do to handle macros better. Usually we want to use X = Tokens->getPosition() and FormatTok = Tokens->set

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. Generally, why do we need to have that much information? I.e. why do we need to know the exact type of the "requires" keyword? I do understand we need to know the brace type, but that seems like it would be easier to figure out in the TokenAnnotator (where we already pars

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. I changed it in 49aca00d63e14df8bc68fc4329e6cbc9c9805eb8 . "We" is the people working on clang-format :) I hope that we have a common goal of making clang-format as easy to maintain as we can. FWIW, I

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. For non-functional clean-ups generally llvm doesn't require pre-commit review - I did communicate here so people involved in the original change wouldn't miss the clean-up. I do agree that what commits to pre-review is a fine line, and usually try to err on the side of p

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

klimek added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403 +} else if (Current.Previous && Current.Previous->is(tok::r_paren) && + Current.startsSequence(tok::arrow, tok::identifier, tok::less)) { + // Deduction guides trailing

[PATCH] D70633: clang-format-vs : Fix Unicode formatting

klimek added a comment. generally makes sense Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70633/new/ https://reviews.llvm.org/D70633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D70664: update string comparison in clang-format.py

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70664/new/ https://reviews.llvm.org/D70664 ___ c

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

klimek added a comment. I'm not generally opposed to this, given that a) clang-format already changes code; I think by now we're not fixing double semicolon mainly for workflow reasons, would be fine to add b) the implementation is very self contained Comment at: clang/docs/Cl

[PATCH] D73537: [clangd] add CODE_OWNERS

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73537/new/ https://reviews.llvm.org/D73537 ___ c

[PATCH] D43969: Improve completion experience for headers

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

klimek added a comment. Just FYI, I talked with Richard and he'll not get to it before next week. Hope that's fine. https://reviews.llvm.org/D5767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D34272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

klimek added a comment. Weekly ping. This week is C++ committee, so it's again going to be hard to get Richards attention :( It's an unfortunate time of year to land major API-touching changes. https://reviews.llvm.org/D5767 ___ cfe-commits mailin

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

klimek added a comment. Sorry for jumping in late, but I'm somewhat questioning whether an extra function in the API is worth the cost here: If you already have a printing policy, getting the type name will be 1 line instead of 2; having an extra function in a large API seems not worth it at a

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

klimek added inline comments. Comment at: include/clang/Tooling/CompilationDatabase.h:188 + static std::unique_ptr + loadFromFile(StringRef Path, std::string &ErrorMsg); + Perhaps loadFromTextFile? Comment at: lib/Tooling/CompilationDatabase.

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

klimek added a comment. In https://reviews.llvm.org/D32478#806757, @Typz wrote: > I renamed the option to `AlignAfterOperator`, is it acceptable now? > (I also thought of `UnindentOperator`, but I think it is better to keep the > notion of alignment) > > Note that this style is used in multiple

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

klimek accepted this revision. klimek added a comment. lg Comment at: lib/Tooling/CompilationDatabase.cpp:312 +FixedCompilationDatabase::loadFromFile(StringRef Path, std::string &ErrorMsg) { + ErrorMsg.clear(); + llvm::ErrorOr> File = sammccall wrote: > klime

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

klimek added a comment. In https://reviews.llvm.org/D32478#920275, @Typz wrote: > > Sorry for the long response time. I don't see this style rule expressed > > explicitly in the Skia or QtCreator style guides - is this something that > > just happens to be done sometimes in the code bases? > >

[PATCH] D39836: [clangd] Drop impossible completions (unavailable or inaccessible)

klimek added a comment. In https://reviews.llvm.org/D39836#920313, @sammccall wrote: > In https://reviews.llvm.org/D39836#920306, @ilya-biryukov wrote: > > > I personally think they're useful to have anyway, and they don't add much > > noise when they're at the end of completions list. > > I so

[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.

klimek created this revision. This patch allowed me to experiment with various alternatives to https://reviews.llvm.org/D33589. https://reviews.llvm.org/D39900 Files: lib/Format/BreakableToken.cpp lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h unittests/Format/Fo

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: include/clang/Frontend/PrecompiledPreamble.h:99 /// Changes options inside \p CI to use PCH from this preamble. Also remaps - /// main file to \p MainFileBuffer. + /// main file to \p MainFileBuffer and updates \p VFS to ensure pream

[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.

klimek updated this revision to Diff 122433. klimek marked an inline comment as done. klimek added a comment. - Add test. Updating D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly more ===

[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1028 + unsigned Penalty = + handleEndOfLine(Current, State, DryRun, AllowBreak); krasimir wrote: > Why `handleEndOfLine`? Is it guaranteed that here we've reached the end of >

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

klimek added a comment. In https://reviews.llvm.org/D32478#920345, @Typz wrote: > > Unless I'm missing something, I'd agree with Daniel; this is not a rule > > that's widely used, and I'd say reformatting a code base to the > > clang-formatted variant will not regress readability. > > Unfortuna

[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.

klimek added a comment. In https://reviews.llvm.org/D39900#921885, @krasimir wrote: > Maybe we should further refactor `getRawStringStyle` into > `llvm::Optional> getRawStringStyleAndDelimiter` > and that would nicely take care of the duplicated effort? I thought about that, too, but I'm stil

[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.

This revision was automatically updated to reflect the committed changes. Closed by commit rL318141: Refactor ContinuationIndenter's breakProtrudingToken logic. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D39900 Files: cfe/trunk/lib/Format/BreakableToken.cpp cfe/tru

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#920160, @Typz wrote: > ping ? I'm working on understanding this better :) I've refactored the code a bit so I could fully understand the problem, which I now do (sorry for this taking a while, but it took me multiple hours to work thr

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. One interesting trade-off I'm running into: My gut feeling is that we really want to make local decisions about whether we want to break/reflow - this makes the code significantly simpler (IMO), and handles all tests in this patch correctly, but is fundamentally limiting

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek created this revision. This is a counter-proposal to https://reviews.llvm.org/D33589. https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTest

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. I think this patch doesn't handle a couple of cases that I'd like to see handled. A counter-proposal with different trade-offs is in https://reviews.llvm.org/D40068. https://reviews.llvm.org/D33589 ___ cfe-commits mailing l

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:206 + std::unique_ptr Storage; + if (InMemStorage) { +OS = llvm::make_unique(*InMemStorage); ilya-biryukov wrote: > klimek wrote: > > It looks like we should pass in the output s

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: lib/Frontend/PrecompiledPreamble.cpp:490 PreprocessorOpts.DisablePCHValidation = true; + if (Storage.getKind() == PCHStorage::Kind::TempFile) { +const

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:8007 +"\"aaabbbcc ddde \"\n" +"\"efff\");", format("someFunction(\"aaabbbcc ddde efff\");", krasimir wrote: > Why did the string got on a newlin

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek updated this revision to Diff 123139. klimek marked 4 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/Fo

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:44 +SmallString<64> Path; +llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, Path); +llvm::sys::path::append(Path, "___clang_inmemory_preamble___"); ilya-biry

[PATCH] D40120: [clang-format] Add text proto filename detection

klimek added a comment. Should that be configured? METADATA seems pretty domain specific ;) https://reviews.llvm.org/D40120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:44 +SmallString<64> Path; +llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, Path); +llvm::sys::path::append(Path, "___clang_inmemory_preamble___"); ilya-biry

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:9951 + "\n*/", + Style)); + krasimir wrote: > Why didn't this get reformatted as: > ``` > EXPECT_EQ("int a; /* first line\n" > "* s

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek updated this revision to Diff 123309. klimek marked an inline comment as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/Fo

[PATCH] D40120: [clang-format] Add text proto filename detection

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D40120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek updated this revision to Diff 123359. klimek added a comment. Just exporting https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestComments.

[PATCH] D40310: Restructure how we break tokens.

klimek created this revision. This fixes some bugs in the reflowing logic and splits out the concerns of reflowing from BreakableToken. Things to do after this patch: - Refactor the breakProtrudingToken function possibly into a class, so we can split it up into methods that operate on the commo

[PATCH] D146704: [clang-format] NFC Format.h and ClangFormatStyleOptions.rst are out of date

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Thank you!! Sorry for forgetting that I needed to do this, CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146704/new/ https://reviews.llvm.org/D146704 ___

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

klimek added a comment. Thanks, yes, I did not intend to delete the else. This only triggers with fuzzing with rather involved inputs, thus I wasn't able to create a nice enough unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146310/new/

[PATCH] D145244: [clang-format] Add ability to trace tokens.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: All. klimek requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145244 Files: clang/lib/Format/CMakeLists.txt clang/lib/Form

[PATCH] D145244: [clang-format] Add ability to trace tokens.

klimek updated this revision to Diff 502123. klimek added a comment. Remove superfluous include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145244/new/ https://reviews.llvm.org/D145244 Files: clang/lib/Format/CMakeLists.txt clang/lib/Format

[PATCH] D145244: [clang-format] Add ability to trace tokens.

klimek added a comment. Answering the fundamental design question first. Comment at: clang/lib/Format/ContinuationIndenter.cpp:289 Current.closesBlockOrBlockTypeList(Style))) { +DEBUG_FORMAT_TRACE_TOKEN(Current, "!canBreak"); return

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

klimek added a comment. I apologize for not tagging appropriately - is that process documented somewhere? Herald added a comment. NOTE: Clang-Format Team Automated Review Comment It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes hav

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

klimek added a comment. And thanks a lot for cleaning up, really appreciate it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147176/new/ https://reviews.llvm.org/D147176 ___ cfe-commits mailing list cfe

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek updated this revision to Diff 497415. klimek marked 3 inline comments as done. klimek added a comment. Address reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143070/new/ https://reviews.llvm.org/D143070 Files: clang/lib/

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek added inline comments. Comment at: clang/lib/Format/FormatTokenSource.h:74 public: - IndexedTokenSource(ArrayRef Tokens) + IndexedTokenSource(SmallVectorImpl &Tokens) : Tokens(Tokens), Position(-1) {} sammccall wrote: > As I understand it, this p

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek updated this revision to Diff 497416. klimek added a comment. Undo changes to ownership of initial set of FormatTokens. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143070/new/ https://reviews.llvm.org/D143070 Files: clang/lib/Format/For

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek updated this revision to Diff 497637. klimek marked an inline comment as done. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143070/new/ https://reviews.llvm.org/D143070 Files: clang/lib/Fo

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.h:287 // owned outside of and handed into the UnwrappedLineParser. + // FIXME: The above fixme doesn't work if we need to create tokens while + // parsing. sammccall wrote: > I'm

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1995d4424505: [clang-format] Enable FormatTokenSource to insert tokens. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: All. klimek requested review of this revision. Herald added a project: clang. Add configuration to specify macros. Macros will be expanded, and the code will be parsed and annotated in the expanded state. In

<    1   2   3   4   5   6   >