klimek updated this revision to Diff 499771.
klimek marked 9 inline comments as done.
klimek added a comment.
Address reviewer comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144170/new/
https://reviews.llvm.org/D144170
Files:
clang/incl
klimek added inline comments.
Comment at: clang/lib/Format/ContinuationIndenter.cpp:744
+ // Align following lines within parenthesis / brackets if configured.
+ // For a line of macro parents, the commas that follow the opening
parenthesis
+ // in the line come after the ope
klimek updated this revision to Diff 499777.
klimek added a comment.
Add comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144170/new/
https://reviews.llvm.org/D144170
Files:
clang/include/clang/Format/Format.h
clang/lib/Format/Continuati
klimek updated this revision to Diff 500121.
klimek marked 6 inline comments as done.
klimek added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144170/new/
https://reviews.llvm.org/D144170
Files:
clang/includ
klimek added inline comments.
Comment at: clang/unittests/Format/FormatTest.cpp:22584
+ Style.Macros.push_back("CALL(x)=f([] { x })");
+ Style.Macros.push_back("ASSIGN_OR_RETURN(a, b, c)=a = (b) || (c)");
+
sammccall wrote:
> klimek wrote:
> > sammccall wrote:
klimek updated this revision to Diff 500181.
klimek marked 5 inline comments as done.
klimek added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144170/new/
https://reviews.llvm.org/D144170
Files:
clang/includ
klimek added inline comments.
Comment at: clang/lib/Format/MacroExpander.cpp:172
assert(defined(ID->TokenText));
+ assert(
+ (!OptionalArgs && ObjectLike.find(ID->TokenText) != ObjectLike.end()) ||
sammccall wrote:
> this assertion failure isn't going to
klimek updated this revision to Diff 500191.
klimek added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144170/new/
https://reviews.llvm.org/D144170
Files:
clang/include/clang/Format/Format.h
clang/lib/Forma
klimek added inline comments.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:4592
+ !Macros.hasArity(ID->TokenText, Args->size())) {
+// The macro is overloaded to be both object-like and function-like,
+// but none of the function-like arities matc
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG01402831aaae: [clang-format] Add simple macro replacements
in formatting. (authored by klimek).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e9e096ae95b: [clang-format] Fix dropped 'else'.
(authored by klimek).
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
H
klimek added a comment.
Submitted.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146310/new/
https://reviews.llvm.org/D146310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.or
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
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
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
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;
---
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
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
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
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
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
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
+
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
klimek added inline comments.
Comment at: lib/Format/BreakableToken.h:40
+/// of the content after a split has been used for breaking, and
+/// - insertBreak, for executing the split using a whitespace manager.
+///
Do we want to describe how replaceWhitespace
klimek added inline comments.
Comment at: lib/Format/BreakableToken.h:55-56
+/// been reformatted, and
+/// - replaceWhitespaceBefore, for executing the reflow using a whitespace
+/// manager.
+///
Shouldn't that be called insertBreakBefore for consistency th
klimek added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1158-1159
+CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
+Current.TokenText.substr(2).ltrim().startswith("clang-format on") ||
+Current.TokenText.substr(2).ltrim().st
klimek added a comment.
This is starting to be pretty awesome. The one thing that would help me review
the gist of the implementation a bit more is if that had a bit more comments.
Perhaps go over the math in the code and put some comments in why we're doing
what at various steps.
==
klimek added inline comments.
Comment at: lib/Format/BreakableToken.h:42-48
+/// There is a pair of operations that are used to compress a long whitespace
+/// range with a single space if that will bring the line lenght under the
+/// column limit:
+/// - getLineLengthAfterCompr
klimek added inline comments.
Comment at: lib/Format/BreakableToken.cpp:279-280
+ return Content.size() >= 2 &&
+ Content != "clang-format on" &&
+ Content != "clang-format off" &&
+ !Content.endswith("\\") &&
Can we now use delimitsRegion here?
klimek added inline comments.
Comment at: lib/Format/BreakableToken.cpp:747
+Split SplitBefore, WhitespaceManager &Whitespaces) {
+ // If this is the first line of a token, we need to inform Whitespace Manager
+ // about it: either adapt the whitespace range preceding it, o
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
https://reviews.llvm.org/D28764
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D29271
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added a comment.
Generally looks like the right direction, minus that I'm not sure yet what the
plan for things broken in BreakableToken are.
Comment at: lib/Format/TokenAnnotator.cpp:1843
+Current->MatchingParen->opensBlockOrBlockTypeList(Style))
+ --Inden
klimek added inline comments.
Comment at: lib/Format/BreakableToken.cpp:685
LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+ // A line comment section needs to broken by a line comment that is
Could we p
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D29300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg minus adding the FIXME to the place where we need the change.
Comment at: lib/Format/UnwrappedLineParser.cpp:2127-2129
+// Additional fine-grained breaking of line com
klimek accepted this revision.
klimek added a comment.
LG. Couple of questions.
Comment at: clangd/ClangDMain.cpp:65
+// Now read the JSON.
+std::vector JSON;
+JSON.resize(Len);
Adi wrote:
> Avoid unnecessary JSON.resize(Len) & potential reallocatio
klimek added inline comments.
Comment at: clangd/JSONRPCDispatcher.h:25-32
+ virtual ~Handler() = default;
+
+ /// Called when the server receives a method call. This is supposed to return
+ /// a result on Outs.
+ virtual void handleMethod(llvm::yaml::MappingNode *Params, St
klimek added inline comments.
Comment at: clang-query/QueryReplace.cpp:50
+
+void QueryReplaceTool::addOperation(clang::query::QueryReplaceSpec &Spec) {
+ ast_matchers::dynamic::Diagnostics Diag;
Shouldn't that also just return the error?
Comm
klimek added a reviewer: hans.
klimek added a comment.
+hans
+1 to "format only current document but save all" not making much sense :)
Comment at: tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs:7
+{
+public sealed class TypeConverterUtils
+{
klimek added a comment.
I think this looks pretty good. More comments would help :) Also, organize is
spelled with a 'z' in American.
https://reviews.llvm.org/D29626
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
klimek added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:2207-2208
+const FormatToken *NextTok) {
+ // Decides which comment tokens should be added to the current line and which
+ // should be added as comments before the next token.
+ //
--
klimek added inline comments.
Comment at: lib/Format/UnwrappedLineParser.h:121-123
+ // Comments specifies the sequence of comment tokens to analyze. They get
+ // either pushed to the current line or added to the comments before the next
+ // token.
Given thi
klimek added inline comments.
Comment at: lib/Format/UnwrappedLineParser.h:121-123
+ // Comments specifies the sequence of comment tokens to analyze. They get
+ // either pushed to the current line or added to the comments before the next
+ // token.
krasimir
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D29699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added inline comments.
Comment at: lib/Format/UnwrappedLineParser.h:121-123
+ // Comments specifies the sequence of comment tokens to analyze. They get
+ // either pushed to the current line or added to the comments before the next
+ // token.
krasimir
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg.
I'd probably still call it consumeComments or something, as we require a flush
afterwards for the unconsumed comments, but I don't feel too strongly about
that.
https://reviews.llvm.org
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Repository:
rL LLVM
https://reviews.llvm.org/D34687
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
klimek added a comment.
In https://reviews.llvm.org/D33644#783903, @yvvan wrote:
> Do not evaluate numbers.
> Check for != "=" is needed not to mess with invalid default arguments or
> their types (without it I get "const Bar& bar = =" when Bar is not defined)
Shouldn't we than instead check
klimek added a comment.
In https://reviews.llvm.org/D33644#793577, @yvvan wrote:
> In https://reviews.llvm.org/D33644#793573, @klimek wrote:
>
> > In https://reviews.llvm.org/D33644#783903, @yvvan wrote:
> >
> > > Do not evaluate numbers.
> > > Check for != "=" is needed not to mess with invalid
klimek added a comment.
In https://reviews.llvm.org/D33644#793601, @yvvan wrote:
> In https://reviews.llvm.org/D33644#793594, @klimek wrote:
>
> > In https://reviews.llvm.org/D33644#793577, @yvvan wrote:
> >
> > > In https://reviews.llvm.org/D33644#793573, @klimek wrote:
> > >
> > > > In https://
klimek added a comment.
The main thing I'm concerned about is having the main code in core, but having
all tests in tools-extra. I think if we go that route we should also move
clang-rename and its tests to core. Thoughts?
Repository:
rL LLVM
https://reviews.llvm.org/D34696
_
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D34755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added a comment.
In https://reviews.llvm.org/D34696#795020, @arphaman wrote:
> In https://reviews.llvm.org/D34696#793613, @klimek wrote:
>
> > The main thing I'm concerned about is having the main code in core, but
> > having all tests in tools-extra. I think if we go that route we should
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG, thx for bearing with me, this looks great.
(and sorry if I didn't send this earlier, just noticed I forgot to hit submit
:( )
Comment at: lib/Tooling/ArgumentsAdjusters.
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Repository:
rL LLVM
https://reviews.llvm.org/D34696
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
klimek added a comment.
In https://reviews.llvm.org/D34512#800490, @xazax.hun wrote:
> It looks like Richard approved libTooling as a dependency for clang on the
> mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html).
> If it is ok to have this code in libTooling (for no
klimek added a reviewer: rsmith.
klimek added a comment.
+Richard as top-level code owner for new libs.
For bikeshedding the name: I'd have liked libIndex, but that's already taken.
CrossTU doesn't seem too bad to me, too, though.
https://reviews.llvm.org/D34512
klimek added a comment.
In https://reviews.llvm.org/D34512#800626, @whisperity wrote:
> In https://reviews.llvm.org/D34512#800502, @xazax.hun wrote:
>
> > In https://reviews.llvm.org/D34512#800499, @klimek wrote:
> >
> > > In https://reviews.llvm.org/D34512#800490, @xazax.hun wrote:
> > >
> > > >
klimek added a comment.
Specifically, ping Richard for new top-level lib in clang.
https://reviews.llvm.org/D34512
___
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/Sema/SemaCodeComplete.cpp:2411-2412
+ if (DefValue.at(0) != '=') {
+// If we don't have = in front of value
+return " = " + DefValue;
+ }
Can you expand in the comment on when each of these happens? Intuitiv
klimek added a comment.
Yep, I want Richard's approval for the name. I think he already expressed a
pro-pulling-out stance.
https://reviews.llvm.org/D34512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
klimek added a reviewer: rsmith.
klimek added a comment.
+Richard, as apparently we get the source ranges for default values of built-in
types without the preceding "=", but for user-defined types including the
preceding "=" - is that intentional?
Comment at: lib/Sema/SemaCod
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Repository:
rL LLVM
https://reviews.llvm.org/D34949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D34267
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added a comment.
In https://reviews.llvm.org/D34440#808156, @vladimir.plyashkun wrote:
> **To discuss:**
> This patch is required for correct integration //Clang-Tidy// with //CLion
> IDE//.
> By this moment, we unable to use //CompilationDatabase.json// from //CLion
> //side which is
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
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
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.
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
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
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
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
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:
> > arphaman wro
klimek added a comment.
In https://reviews.llvm.org/D34440#812938, @vladimir.plyashkun wrote:
> > If you want one unified format, the compilation database is it.
>
> Is the `clang-tidy ... -- ` meant to be more or less a drop-in
> replacement for `clang ` (arguments-wise)?
> If yes, this expan
klimek added a comment.
Your patch description sounds like you're only switching the warning off when
the scope ends in a macro, but the patch looks different. For example:
#define M int x
int x;
void f() {
M;
x = 42; // the user probably meant ::x here
}
would also be suppre
klimek added a comment.
Could we perhaps only suppress the warning when we shadow a variable within the
same declcontext? Generally, with macros, shadowing local variables is more
surprising.
https://reviews.llvm.org/D35783
___
cfe-commits mailing
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: lib/Format/UnwrappedLineParser.h:126
bool eof() const;
- void nextToken();
+ // LevelDifference is the difference of levels after and before this token.
+
klimek created this revision.
CurrentDir was set as the path of the current module, but that can change as
part of a chain of loaded modules.
When we try to locate a file mentioned in a module that does not exist, we use
a heuristic to look at the relative path between the original location of th
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308962: Fix incorrect use of current directory to find moved
paths in ASTReader. (authored by klimek).
Repository:
rL LLVM
https://reviews.llvm.org/D35828
Files:
cfe/trunk/include/clang/Serializatio
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
http://lists.llvm.org/cgi-bin/ma
klimek added inline comments.
Comment at: lib/Basic/FileManager.cpp:167-168
DirNameStr = DirName.str() + '.';
+llvm::sys::path::native(DirNameStr);
+DirName = DirNameStr;
+ } else {
I'd add a canonicalizePath function that:
-> on unix just returns t
klimek added inline comments.
Comment at: clangd/ASTManager.cpp:69
+ // our one-element queue is empty.
+ if (!RequestIsPending && !Done)
+ClangRequestCV.wait(Lock);
arphaman wrote:
> This is just a suggestion based on personal opinion: `Request
klimek added inline comments.
Comment at: clangd/ASTManager.h:67
+ /// Setting Done to true will make the worker thread terminate.
+ std::atomic Done;
+};
bkramer wrote:
> klimek wrote:
> > arphaman wrote:
> > > It looks like `Done` is always accessed in a scop
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: clangd/ASTManager.cpp:138-139
+ // Currently we discard all pending requests and just enqueue the latest one.
+ while (!RequestQueue.empty())
+RequestQue
klimek added inline comments.
Comment at: lib/Format/BreakableToken.cpp:397
+//
+// Don't try aligning if there are less lines, since some patterns like
+//
fewer lines
Comment at: lib/Format/BreakableToken.cpp:402
+
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D29943
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added inline comments.
Comment at: test/Format/check-coding-style-mozilla.cpp:7-9
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:251
+ } else {
+assert(false && "Must match a NamedDecl!");
+ }
You can use llvm_unreachable instead. Or do you actually want to fall through
in release mode?
==
klimek added inline comments.
Comment at: include/clang/Rewrite/Core/Rewriter.h:188-197
+ /// prewrite(FID) is called after all changes to FID have been written to a
+ /// temporary file, but before that temporary file is moved into FID's
+ /// location. Windows's ReplaceFile
klimek added a comment.
Can we open the files with FILE_SHARE_DELETE instead?
https://reviews.llvm.org/D30385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek added a comment.
Also, do we want to not call ReplaceFile in Path.inc on Win if it potentially
leaves temp files lying around?
Reading the code, it looks like we currently actually believe it may fail, and
retry with MoveFileEx afterwards...
https://reviews.llvm.org/D30385
__
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D27054
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added inline comments.
Comment at: docs/ClangFormatStyleOptions.rst:218
+
+#define A \
+int ; \
Do we really align these 3 spaces out?
Comment at: docs/ClangFormatStyleOptions.rst:447
+ SomeClass::Constructor()
+
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D30636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added a comment.
Is the diff view on phab broken, or am I missing something? I only see a single
line of diff now, and don't see a way to change the diff.
https://reviews.llvm.org/D27810
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
klimek added a comment.
In https://reviews.llvm.org/D30650#693165, @Eugene.Zelenko wrote:
> I think we should refactor this check as part of Static Analyzer, since it's
> path-sensitive.
Are you saying it should be path sensitive? Because currently it's not, right?
https://reviews.llvm.org/D
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D30735
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added inline comments.
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:129
+struct ApplyChangesSpec {
+ // If true, cleans up redundant/erroneous around changed code with
+ // clang-format's cleanup functionality, e.g. redundant commas around deleted
---
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
401 - 500 of 532 matches
Mail list logo