[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] D31992: [clangd] Escape only necessary characters in JSON output

klimek added inline comments. Comment at: clangd/Protocol.cpp:26-50 + for (llvm::StringRef::iterator i = Input.begin(), e = Input.end(); i != e; ++i) { +if (*i == '\\') + EscapedInput += ""; +else if (*i == '"') + EscapedInput += "\\\""; +// bell +

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

klimek added a comment. In https://reviews.llvm.org/D31992#725912, @malaperle-ericsson wrote: > In https://reviews.llvm.org/D31992#725866, @krasimir wrote: > > > Seems that we're starting to hit some YAML/JSON mismatches, or is it that > > your YAML string support is lacking? > > > I don't think

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

klimek added a reviewer: ioeric. klimek added a comment. +eric, who has some experience llvm::Error'ing our interfaces :) llvm::ErrorOr seems like the right approach here? https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@list

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

klimek added a comment. In https://reviews.llvm.org/D27440#626477, @amaiorano wrote: > In https://reviews.llvm.org/D27440#626415, @ioeric wrote: > > > `llvm::ErrorOr` carries `std::error_code`. If you want richer information > > (e.g. error_code + error message), `llvm::Expcted` and `llvm::Error

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

klimek added a comment. Why isn't the right solution to make getStyle() use vfs::FileSystem? Generally, everything in clang-format (well, in clang) should use vfs::FileSystem for file access. https://reviews.llvm.org/D27971 ___ cfe-commits mailing

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Apart from the error handling LG. Thanks! Comment at: lib/Format/Format.cpp:1920-1922 + std::error_code EC = FS->makeAbsolute(Path); + assert(!EC); + (void)EC; ---

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

klimek added inline comments. Comment at: lib/Format/Format.cpp:1920-1922 + std::error_code EC = FS->makeAbsolute(Path); + assert(!EC); + (void)EC; amaiorano wrote: > klimek wrote: > > I think if makeAbsolute doesn't work, we will probably want to err out here

[PATCH] D28465: clang-format: [JS] ASI after imports

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

[PATCH] D28260: Add an argumentsAre matcher

klimek added a reviewer: bkramer. klimek added a comment. +benjamin https://reviews.llvm.org/D28260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

klimek created this revision. klimek added a reviewer: bkramer. klimek added a subscriber: cfe-commits. Instead of just using popularity, we also take into account how similar the path of the current file is to the path of the header. Our first approach is to get popularity into a reasonably small

[PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

This revision was automatically updated to reflect the committed changes. Closed by commit rL291664: Improve include fixer's ranking by taking the paths into account. (authored by klimek). Changed prior to commit: https://reviews.llvm.org/D28548?vs=83930&id=83937#toc Repository: rL LLVM htt

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } Optional: I'd probably let the nodeToCommandLine h

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > Optional: I'd pro

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > joerg wrote: > >

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

klimek updated this revision to Diff 79530. klimek added a comment. Add tests, update hasDeclaration to work for ElaboratedType. https://reviews.llvm.org/D27104 Files: include/clang/ASTMatchers/ASTMatchersInternal.h unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Index: unittests/ASTMat

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

klimek added a comment. In https://reviews.llvm.org/D27104#607161, @lukasza wrote: > Forcing shallow matching means that unit test below will stop passing after > this CL. > > TEST(HasDeclaration, DeepTagType) { > std::string input = > "class Foo {};\n" > "using Bar = Foo;\

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek created this revision. klimek added reviewers: bkramer, lukasza. klimek added a subscriber: cfe-commits. https://reviews.llvm.org/D27207 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/ASTMa

[PATCH] D27140: Allow clang to write compilation database records

klimek added reviewers: bkramer, akyrtzi. klimek added a comment. Adding some folks. One question is whether we can use the additional output stuff doug added at some point for this. https://reviews.llvm.org/D27140 ___ cfe-commits mailing list cfe-

[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl

klimek added a comment. Can you add a test to ASTMatchersNodeTests.cpp? Otherwise LG for the matcher parts. https://reviews.llvm.org/D27181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > joerg wrote: > >

[PATCH] D27211: [clang-format] Implement comment reflowing

klimek added a comment. > for these cases the > original Breakable{Line,Block}Comment still breaks the long lines. Do you intend for this to continue to be the case? https://reviews.llvm.org/D27211 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added a comment. In https://reviews.llvm.org/D27138#607786, @joerg wrote: > It's not the directory, but the output file. That's optional since it is a > new addition and I don't want to invalidate all existing JSON databases. It seems like we're talking past each other. I'm not suggesti

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek added a comment. hasUnqualifiedDesugaredType(hasDeclaration( How about using hasUnqualifiedDesugaredType(recordType(hasDeclaration instead? https://reviews.llvm.org/D27207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D27054: Introducing clang::tooling::EditList for refactoring tools.

klimek added inline comments. Comment at: include/clang/Tooling/Refactoring/EditList.h:41 + /// \brief Creates an edit list for a key position. + EditList(const SourceManager &SM, SourceLocation KeyPosition); + ioeric wrote: > djasper wrote: > > I wonder whethe

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek updated this revision to Diff 79891. klimek added a comment. Add multi-level using test. https://reviews.llvm.org/D27207 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/ASTMatchersTraversa

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D27138#607859, @joerg wrote: > Which struct are we talking about, `CompileCommandRef` or `CompileCommand`? > It is a pointer in the former and a plain StringRef in

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek added a comment. I added a test for multi-stage desugaring. Generally, we want to model Clang's AST where it makes sense. The AST has a getUnqualifiedDesugaredType, thus the matcher as it is is expected. We can add single step desugaring or hasAnyDeclaration or similar things when they a

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

This revision was automatically updated to reflect the committed changes. Closed by commit rL288366: Adds hasUnqualifiedDesugaredType to allow matching through type sugar. (authored by klimek). Changed prior to commit: https://reviews.llvm.org/D27207?vs=79891&id=79912#toc Repository: rL LLVM

[PATCH] D27350: CFGBuilder: Fix crash when visiting delete expression on dependent type

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

[PATCH] D27447: [ASTMatcher] Add hasReplacementType matcher for SubstTemplateTypeParmType

klimek added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5034 +/// \c substTemplateTypeParmType(hasReplacementType(type())) matches int +AST_MATCHER_P(SubstTemplateTypeParmType, hasReplacementType, + ast_matchers::internal::Matcher, InnerMatc

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

klimek added a comment. Pondering this a bit - one question is whether we should make clang-format not return 0 if we pass -fallback-style=none (it already has this option) and we can't figure out a style. What do you think? https://reviews.llvm.org/D27440 __

[PATCH] D27140: Allow clang to write compilation database records

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. We can always add more intricate ways later on. Repository: rL LLVM https://reviews.llvm.org/D27140 ___ cfe-commits mailing list cfe-commi

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

klimek added a reviewer: djasper. klimek added a comment. Adding djasper, who had brought forward the idea. https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D27501: clang-format-vsix: add command to format document

klimek added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + + Clang Format Document + hans wrote: > amaiorano wrote: > > hans wrote: > > > I think File would be better than Document when referring to

[PATCH] D27447: [ASTMatcher] Add hasReplacementType matcher for SubstTemplateTypeParmType

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Looks good. Thanks! https://reviews.llvm.org/D27447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D27557: Update the default of the Mozilla coding style

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

[PATCH] D27501: clang-format-vsix: add command to format document

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. I also think it makes the code nicer by breaking out the right functions. Thanks! Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + +

[PATCH] D27754: [clang-format] Implement comment reflowing (again).

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:471 + WhitespaceManager &Whitespaces) { + if (Tok.is(TT_LineComment)) { +// If this is the first line of a token, inform Whitespace Manager about it. -

[PATCH] D27754: [clang-format] Implement comment reflowing (again).

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1174-1175 LineIndex != EndIndex; ++LineIndex) { -if (!DryRun) - Token->replaceWhitespaceBefore(LineIndex, Whitespaces); +Token->replaceWhitespaceBefore(LineIndex, RemainingTokenColu

[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] 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 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 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] 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] 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] 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] D88299: [clang-format] Add MacroUnexpander.

klimek marked 7 inline comments as done. klimek added inline comments. Comment at: clang/lib/Format/Macros.h:201 + /// Generally, this line tries to have the same structure as the expanded, + /// formatted unwrapped lines handed in via \c addLine(), with the exception + /// th

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

klimek updated this revision to Diff 443729. klimek added a comment. Address 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/Format/CMakeLists.txt clang/lib/Format/Form

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd6d0dc1f4537: [clang-format] Add MacroUnexpander. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

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

klimek added a comment. In D88299#3660772 , @nridge wrote: > Does this patch change the formatting behaviour of clang-format? > > If so, are there any test cases that show before/after formatting? The > MacroCallReconstructorTest unit test seems like it's

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

klimek added a comment. In D88299#3660779 , @nridge wrote: > Thanks! (I was intrigued by Sam's "solves a whole class of clang-format's > biggest problems" comment :-)) The end-result hopefully will :) Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D56841: [clangd] Filter out plugin related flags.

klimek added a comment. In D56841#1361395 , @kadircet wrote: > It seems like the most relevant place in tooling is ArgumentsAdjuster as > @ioeric pointed out. Happy to move it to there if @sammccall and @klimek > agrees as well. > > As a side note `Argum

[PATCH] D34440: [Clang] Expand response files before loading compilation database

klimek added a comment. In https://reviews.llvm.org/D34440#809325, @vladimir.plyashkun wrote: > Even if i'll change content of //arguments.rsp// to > `-std=c++11 -Ipath/to/include -Ipath/to/include2 -DMACRO ` > and will try to call clang-tidy process in this way: > `clang-tidy -checks=* ma

[PATCH] D34440: [Clang] Expand response files before loading compilation database

klimek added a comment. In https://reviews.llvm.org/D34440#809581, @vladimir.plyashkun wrote: > > Are there any concerns using the alternative? > > I can't say that it's a big problems, but i think that: > //CompilationDatabase.json// is more //CMake //specific format. > It can be generated au

[PATCH] D35012: [refactor] Add the AST source selection component

klimek added inline comments. Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:30 + /// inside of its source range. + ContainsSelectionPoint, + It's unclear what a selection point is. I'd have expected this to mean "overlaps" when first reading it.

[PATCH] D35012: [refactor] Add the AST source selection component

klimek added inline comments. Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:100 + SelectionStack.back().Children.push_back(std::move(Node)); +return true; + } arphaman wrote: > klimek wrote: > > Why do we always stop traversal? > False stops trav

[PATCH] D33644: Add default values for function parameter chunks

klimek added a comment. In https://reviews.llvm.org/D33644#812693, @yvvan wrote: > So do we wait until the '=' case is more clear? This change should not break > anything if it's fixed in either direction (if '=' will be provided always or > never) Ok, if you add a TODO I think this is fine f

[PATCH] D33644: Add default values for function parameter chunks

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Sema/SemaCodeComplete.cpp:2422 + std::string DefValue{srcText}; + // TODO: remove this check if the Lexer::getSourceText value is fixed and + // this va

[PATCH] D35012: [refactor] Add the AST source selection component

klimek added inline comments. Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164 + unsigned NumMatches = 0; + for (Decl *D : Context.getTranslationUnitDecl()->decls()) { +if (ObjCImplEndLoc.isValid() && arphaman wrote: > klimek wrote: > > Why don't we

<    1   2   3   4   5   6   >