klimek added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:362
+bool Parser::parseBindID(std::string , TokenInfo ) {
+ // Parse .bind("foo")
CloseToken seems to not be used afterwards either here or in the follow-up
patch?
Repository:
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/D51259
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
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/D51261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1307
+ (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
State.Stack.back().IsInsideObjCArrayLiteral =
klimek
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/D50697
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added a comment.
Sorry that I lost track of that, but feel free to ping once / week -
unfortunately the patch doesn't apply cleanly, can you rebase?
Repository:
rC Clang
https://reviews.llvm.org/D45719
___
cfe-commits mailing list
klimek added a comment.
The problem here is that we have different opinions on how the formatting on
namespace macros should behave in the first place- I think they should behave
like namespaces, you want them to be formatted differently.
Given that you want full control over the formatting of
klimek added a comment.
Yea, if we go down this route I'd go with this by default:
some ? thing :
else ? otherthing :
unrelated ? but :
finally;
Theoretically we could even use:
some ? thing :
else;
by default ;)
Repository:
rC Clang
https://reviews.llvm.org/D50078
klimek added a comment.
Usually we use match(anyOf(node), hasDescendant(node)). Or did I misunderstand
what you want?
Repository:
rC Clang
https://reviews.llvm.org/D49840
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
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/D49724
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1307
+ (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
State.Stack.back().IsInsideObjCArrayLiteral =
I think
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
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
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
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
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:
> > >
klimek added a reviewer: rsmith.
klimek added inline comments.
Comment at: clangd/XRefs.cpp:559
+ //- auto& i = 1;
+ bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+if (!D->getTypeSourceInfo() ||
malaperle wrote:
> klimek wrote:
> > sammccall wrote:
> > >
klimek added a comment.
In https://reviews.llvm.org/D28462#1147019, @enyquist wrote:
> @klimek fair point. To be honest, I've pretty much lost interest / momentum
> on this feature, I very much doubt I will ever go back and re-implement from
> scratch as you suggest.
> Not meaning to sound
klimek added inline comments.
Comment at: clangd/XRefs.cpp:559
+ //- auto& i = 1;
+ bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+if (!D->getTypeSourceInfo() ||
sammccall wrote:
> malaperle wrote:
> > sammccall wrote:
> > > out of curiosity, why not
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?
>
>
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
klimek added inline comments.
Comment at: unittests/Format/FormatTest.cpp:4449-4450
+ " })\n"
+ "
.foo(\"aaa\"\n"
+ " \"bb\");\n"
+
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
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
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;
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
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
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;
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?
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
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
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
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
klimek added a comment.
Is this written down somewhere? https://webkit.org/code-style-guidelines/
doesn't seem to mention it.
Repository:
rC Clang
https://reviews.llvm.org/D46024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added inline comments.
Comment at: include/clang/Format/Format.h:891
+ /// \endcode
+ bool BreakBeforeLambdaBody;
+
I'd just make that default for Allman style.
Comment at: lib/Format/TokenAnnotator.cpp:2844
if (isAllmanBrace(Left)
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/D46062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added inline comments.
Comment at: unittests/Format/FormatTest.cpp:4359
+ "return 3;\n"
+ " }).as("");\n"
+ "}");
What would be interesting is tests that:
a) have another value after the closing }; doesn't
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330573: Format closing braces when reformatting the line
containing the opening brace. (authored by klimek, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
klimek updated this revision to Diff 143274.
klimek marked 2 inline comments as done.
klimek added a comment.
Address comments.
Repository:
rC Clang
https://reviews.llvm.org/D45726
Files:
lib/Format/AffectedRangeManager.cpp
lib/Format/AffectedRangeManager.h
lib/Format/Format.cpp
klimek added a comment.
In https://reviews.llvm.org/D45726#1071925, @krasimir wrote:
> Another point: for the example in the summary about bailing-out early, is
> there a test for this already? If not, we should add one.
Yep, there already is one - I regressed that with my change at first ;)
klimek added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:438
+
+ AlignTokens(Style,
+ [&](const Change ) {
I'm not sure whether we should use AlignTokens here, given that we pass in a
parameter to basically skip all its
klimek created this revision.
klimek added a reviewer: krasimir.
...doesn't
work correctly for "} else {".
2. We needed to change the API of AffectedRangeManager to not use iterators; we
always passed in begin / end for the whole container before, so there was no
mismatch in generality.
3.
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D38615
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
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
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
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
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 ||
+
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
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
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
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
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
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
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
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
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
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
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
> }
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
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
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:
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
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
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
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
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
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
klimek added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1707
+ RemainingTokenColumns = Token->getRemainingLength(
+ NextLineIndex, TailOffset, ContentStartColumn);
+ Reflow = true;
krasimir wrote:
> When we're
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
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
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
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
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
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
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
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
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
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?
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
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
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
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
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
> > >
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
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,
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
> >
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
+
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
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
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
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
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"
> "*
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___");
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
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___");
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
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
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
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
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
201 - 300 of 525 matches
Mail list logo