[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

klimek added a comment. In https://reviews.llvm.org/D36574#860197, @arphaman wrote: > In https://reviews.llvm.org/D36574#858763, @klimek wrote: > > > One of my main concerns is still that I don't see the need for all the > > template magic yet :) Why doesn't everybody use the RefactoringResult

[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

klimek added inline comments. Comment at: test/Refactor/LocalRename/Field.cpp:4 +class Baz { + int /*range=*/Foo; // CHECK: symbol [[@LINE]]:17 -> [[@LINE]]:20 +public: arphaman wrote: > klimek wrote: > > arphaman wrote: > > > klimek wrote: > > > > Does this

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

klimek added inline comments. Comment at: include/clang/Tooling/CommonOptionsParser.h:90 /// - /// This constructor exits program in case of error. + /// If \p ExitOnError is set (default), This constructor exits program in case + /// of error; otherwise, this sets the

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

klimek added a comment. Whee, thanks for driving this, much appreciated :D Comment at: include/clang/FrontendTool/Utils.h:25 + +std::unique_ptr CreateFrontendAction(CompilerInstance ); Please add a comment now that this is exported.

[PATCH] D40072: [libclang] Support querying whether a declaration is invalid

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: include/clang-c/Index.h:2622 + * + * A declaration is invalid if it could not be parsed successfully. + */ Perhaps add that we need to have

[PATCH] D40310: Restructure how we break tokens.

klimek added a comment. Restructured to make the invariants clearer based on a chat with Krasimir. https://reviews.llvm.org/D40310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124381. klimek added a comment. Restructure based on code review feedback. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1525 + if (!DryRun) +Token->adaptStartOfLine(0, Whitespaces); + krasimir wrote: > If we indent here, shouldn't that also change ContentStartColumn? Nope, this will exactly adapt

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124581. klimek marked 3 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D40060: [clangd] Fuzzy match scorer

klimek added inline comments. Comment at: clangd/FuzzyMatch.cpp:69 +: NPat(std::min(MaxPat, Pattern.size())), NWord(0), + ScoreScale(0.5f / NPat) { + memcpy(Pat, Pattern.data(), NPat); Why .5? Comment at: clangd/FuzzyMatch.cpp:88 +

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

klimek added a comment. In https://reviews.llvm.org/D40068#931679, @Typz wrote: > Generally, this indeed improves the situation (though I cannot say much about > the code itself, it is still too subtle for my shallow knowledge of > clang-format). > > But it seems to give some strange looking

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

klimek added a comment. In https://reviews.llvm.org/D33589#933568, @klimek wrote: > In https://reviews.llvm.org/D33589#931723, @Typz wrote: > > > In https://reviews.llvm.org/D33589#925903, @klimek wrote: > > > > > I think this patch doesn't handle a couple of cases that I'd like to see > > >

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

klimek added a comment. In https://reviews.llvm.org/D33589#931802, @Typz wrote: > Btw, another issue I am having is that reflowing does not respect the > alignment. For exemple: > > enum { > Foo,///< This is a very long comment > Bar,///< This is shorter > BarBar,

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

klimek added a comment. In https://reviews.llvm.org/D33589#931723, @Typz wrote: > In https://reviews.llvm.org/D33589#925903, @klimek wrote: > > > 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 > >

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#930065, @Typz wrote: > ping? Argh, very sorry for the delay in response. In https://reviews.llvm.org/D37813#905257, @Typz wrote: > In https://reviews.llvm.org/D37813#876227, @klimek wrote: > > > I think instead of introducing more

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

klimek added a comment. In https://reviews.llvm.org/D33589#933746, @Typz wrote: > with this setting, a "non wrapped" comment will actually be reflown to > ColumnLimit+10... Isn't the same true for code then, though? Generally, code will protrude by 10 columns before being broken?

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

klimek added a comment. In https://reviews.llvm.org/D33589#933746, @Typz wrote: > > @klimek wrote: > > In the above example, we add 3 line breaks, and we'd add 1 (or more) > > additional line breaks when reflowing below the column limit. > > I agree that that can lead to different overall

[PATCH] D40310: Restructure how we break tokens.

klimek marked an inline comment as done. klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1504 : Style.PenaltyBreakComment; - unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength; + // Stores

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124095. klimek added a comment. Pull out getRemainingLength. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:198 + "Getting the length of a part of the string literal indicates that " + "the code tries to reflow it."); + return UnbreakableTailLength + Postfix.size() + krasimir

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124075. klimek marked 10 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1707 + RemainingTokenColumns = Token->getRemainingLength( + NextLineIndex, TailOffset, ContentStartColumn); + Reflow = true; krasimir wrote: > When we're

[PATCH] D40310: Restructure how we break tokens.

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

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1749 + } + if (!Reflow) { +// If we didn't reflow into the next line, the only space to consider is krasimir wrote: > nit: Maybe change this to `if (Reflow)` and

[PATCH] D40310: Restructure how we break tokens.

This revision was automatically updated to reflect the committed changes. Closed by commit rL319314: Restructure how we break tokens. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D40310 Files: cfe/trunk/lib/Format/BreakableToken.cpp

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek created this revision. When we break a long line like: Column limit: 21 | // foo foo foo foo foo foo foo foo foo foo foo foo The local decision when to allow protruding vs. breaking can lead to this outcome (2 excess characters, 2 breaks): // foo foo foo foo

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek updated this revision to Diff 124940. klimek added a comment. Fix incorrect return value leading to unnecessary computation. Repository: rC Clang https://reviews.llvm.org/D40605 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1518 + unsigned RemainingTokenColumns = 0; + // The column number we're currently at. + unsigned ContentStartColumn = 0; krasimir wrote: > Could you please spell out the invariants

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1422 + Strict); +} } krasimir wrote: > The problem here is that we're calling breakProtrudingToken 3 times more than > we used to. Seems like such a

[PATCH] D41130: git-clang-format: cleanup: Use run() when possible.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D41130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41147: git-clang-format: Add new --staged option.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D41147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format:252 + assert len(commits) != 0 + if len(commits) >= 2: +return

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. I've talked with Daniel and we both believe this patch is not the right way to go forward for clang-format. It is adding configuration mechanisms that we need to maintain going forward, without addressing a general problem; specifically for how to handle macros, I

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#958116, @Typz wrote: > OK. > > So you mean a solution like the one discussed earlier would be the way to go? > > > I mean that we can configure macros in the format style, like "define A(X) > > class X {". I'm not 100% sure whether we

[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

[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" > "*

[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___");

[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___");

[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

[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

[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

[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] 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 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

[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

[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. > >

[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

[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

[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

[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] 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

[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

[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

[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

[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

[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

[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] 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] 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

[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

[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.clear(); + llvm::ErrorOr File = sammccall wrote:

[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

[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] 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 ); + Perhaps loadFromTextFile? Comment at: lib/Tooling/CompilationDatabase.cpp:312

[PATCH] D40909: [clang-format] Reorganize raw string delimiters

klimek added inline comments. Comment at: include/clang/Format/Format.h:1375 +std::vector EnclosingFunctionNames; +/// \brief The canonical delimiter for this language. +std::string CanonicalDelimiter; djasper wrote: > krasimir wrote: > > djasper

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek updated this revision to Diff 125094. klimek marked an inline comment as done. klimek added a comment. Add test. Repository: rC Clang https://reviews.llvm.org/D40605 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h unittests/Format/FormatTest.cpp

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

This revision was automatically updated to reflect the committed changes. Closed by commit rL319541: Better trade-off for excess characters vs. staying within the column limits. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D40605 Files:

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

klimek added a comment. In https://reviews.llvm.org/D33589#941979, @Typz wrote: > I think the difference between code and comments is that code "words" are > easily 10 characters or more, whereas actual words (in comments) are very > often less than 10 characters: so code overflowing by 10

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#941987, @Typz wrote: > Definitely that would be much more expressive. But this approach is also > incomplete: in case of namespace (and possibly others?), we also need to > perform the reverse operation, e.g. to "generate" a macro call

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

klimek added a comment. In https://reviews.llvm.org/D33589#942128, @Typz wrote: > Indeed, looks good, thanks! > > Though that exacerbates the alignment issue, I now get things like this: > > enum { > SomeLongerEnum, // comment > SomeThing, // comment > Foo, // something > }

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#942137, @Typz wrote: > As far as "parsing" and formatting inside the block is concerned, this is > indeed unrelated (and would totally work if all macros where specified with > some preprocessor definitions). > > But identifying the

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

klimek added inline comments. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > ilya-biryukov wrote: > > > > sammccall wrote: > > > > > We add complexity here

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#945125, @Typz wrote: > I don't think this is really relevant for this tool: if someone changes the > implementation of the macro, then *they* must indeed if it should not be > formatted like a namespace (and keep the clang-format

[PATCH] D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions

klimek added inline comments. Comment at: docs/RefactoringActionTutorial.rst:7 + + This tutorial talks about a work-in-progress library in Clang. + Some of the described features might not be available yet in trunk, but should hokein wrote: > I'm a bit

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

klimek added inline comments. Comment at: include/clang/Tooling/Execution.h:76 + + void appendArgumentsAdjuster(ArgumentsAdjuster Adjuster); + I think the argument adjuster adjustment shouldn't be part of this interface, as the argument adjusters cannot be

[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

[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

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

klimek added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:2904 +internal::Matcher, InnerMatcher, 1) { + QualType QT = internal::getUnderlyingType(Node); + if (!QT.isNull()) In which cases can getUnderlyingType return a null type?

[PATCH] D48269: [ASTMatchers] Don't assert-fail in specifiesTypeLoc().

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg, thanks! Repository: rC Clang https://reviews.llvm.org/D48269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

klimek accepted this revision. klimek added a comment. lg, thx Repository: rC Clang https://reviews.llvm.org/D48242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:678 // Indent with tabs only when there's at least one full tab. -if (FirstTabWidth + Style.TabWidth <= Spaces) { +if (Style.TabWidth <= Spaces) { Spaces -= FirstTabWidth;

[PATCH] D47759: [Format] Do not use a global static value for EOF within ScopedMacroState.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. Repository: rC Clang https://reviews.llvm.org/D47759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

klimek accepted this revision. klimek added a comment. In https://reviews.llvm.org/D46024#1129350, @hans wrote: > In https://reviews.llvm.org/D46024#1121242, @rkirsling wrote: > > > FWIW, please note that this space-before-brace style is not specific to > > WebKit; CppCoreGuidelines exhibits it

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

klimek accepted this revision. klimek added a comment. LG Repository: rC Clang https://reviews.llvm.org/D47095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

klimek accepted this revision. klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || klimek wrote: > malaperle wrote: > > klimek wrote: > > >

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

klimek added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:328 void OverlayFileSystem::pushOverlay(IntrusiveRefCntPtr FS) { + // FIXME: OverlayFS containing another one in its stack could be flattened. FSList.push_back(FS); ilya-biryukov

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added a comment. In https://reviews.llvm.org/D28462#1148732, @enyquist wrote: > @klimek having gotten that out of the way, I do occasionally drink too much > and have sudden urges to re-implement things from scratch. Close it if you > need to, since I can't commit to anything, but

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); ank wrote: > klimek wrote: > > What would be interesting is tests that: > > a) have another

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); ank wrote: > klimek wrote: > > ank wrote: > > > klimek wrote: > > > > What would be

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4449-4450 + " })\n" + " .foo(\"aaa\"\n" + " \"bb\");\n" +

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added a comment. In https://reviews.llvm.org/D44609#1143895, @Wawha wrote: > Hello, > > after my last modification (require by previous comment), I do not see any > feedback or validation for this patch. > Is their something special to do in order to go forward on this patch? > >

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added inline comments. Herald added a subscriber: acoomans. Comment at: lib/Format/TokenAnnotator.cpp:627 } +if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) { +++Left->BlockParameterCount; Why do we want to

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:627 } +if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) { +++Left->BlockParameterCount; Wawha wrote: > klimek wrote: > > Why do we want to increase

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

klimek added a comment. I believe this was accepted by rnk - are you waiting for specific further feedback? https://reviews.llvm.org/D36610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41487: [clang-format] Adds a FormatStyleSet

klimek added inline comments. Comment at: lib/Format/Format.cpp:906-907 + } + if (!LanguageFound) +return make_error_code(ParseError::Unsuitable); + *Style = *StyleSet.Get(Language); Optional: I'd probably slightly re-structure the above to: if

[PATCH] D42036: [clang-format] Keep comments aligned to macros

klimek added a comment. Just from a formatting point of view, why not: //. Comment #.define X Repository: rC Clang https://reviews.llvm.org/D42036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41487: [clang-format] Adds a FormatStyleSet

klimek added inline comments. Comment at: lib/Format/Format.cpp:893 for (int i = Styles.size() - 1; i >= 0; --i) { -if (Styles[i].Language == Language || -Styles[i].Language == FormatStyle::LK_None) { +if (!LanguageFound && (Styles[i].Language == Language || +

[PATCH] D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:678 // Indent with tabs only when there's at least one full tab. -if (FirstTabWidth + Style.TabWidth <= Spaces) { +if (Style.TabWidth <= Spaces) { Spaces -= FirstTabWidth;

<    1   2   3   4   5   6   >