jtbandes updated this revision to Diff 133440.
jtbandes added a comment.
Using a slightly more invasive fix. I haven't come up with any other test cases
that exhibit the problem, which makes me unsure this fix is needed in all these
locations. Maybe someone with more knowledge of this function c
jtbandes added a comment.
Please take another look when you get a chance :)
Repository:
rC Clang
https://reviews.llvm.org/D40284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes created this revision.
Herald added a subscriber: klimek.
The original changes for ref qualifiers in https://reviews.llvm.org/rL272537
and https://reviews.llvm.org/rL272548 allowed function const+ref qualifier
spacing to diverge from the spacing used for variables. It seems more
consis
jtbandes created this revision.
Herald added a subscriber: klimek.
Changes to handle `if constexpr` the same way as `if`.
https://reviews.llvm.org/D34330
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTe
jtbandes added a comment.
Hm, I probably should've searched first — but I just re-implemented this in
https://reviews.llvm.org/D34330. Actually, I think my implementation solves
the `AllowShortIfStatementsOnASingleLine` issue you were mentioning here 🎉
https://reviews.llvm.org/D26953
_
jtbandes added a comment.
Thanks for the review. Please note that there was a prior effort to implement
this in https://reviews.llvm.org/D26953. However if you are happy with this
version, feel free to commit (as I don’t have commit access).
https://reviews.llvm.org/D34330
_
This revision was automatically updated to reflect the committed changes.
Closed by commit rC321592: [Sema] Improve diagnostics for const- and
ref-qualified member functions (authored by jtbandes, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D39937?vs=123481&id=128366#toc
jtbandes created this revision.
jtbandes added a reviewer: aaron.ballman.
Re-submission of https://reviews.llvm.org/D39937 with additional fixed tests.
Repository:
rC Clang
https://reviews.llvm.org/D41646
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaOverload.cpp
test/
jtbandes added a comment.
After merging the buildbots informed me some other tests were broken that I
failed to notice. I reverted this commit and submitted
https://reviews.llvm.org/D41646 which reintroduces the changes and fixes the
other broken tests.
Repository:
rC Clang
https://reviews
This revision was automatically updated to reflect the committed changes.
Closed by commit rL321609: [Sema] Improve diagnostics for const- and
ref-qualified member functions (authored by jtbandes, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D41646
Files:
cfe/trunk/include/c
jtbandes created this revision.
Add a flag `-fno-digraphs` to disable digraphs in the lexer, similar to
`-fno-operator-names` which disables alternative names for C++ operators.
Repository:
rC Clang
https://reviews.llvm.org/D48266
Files:
include/clang/Driver/Options.td
lib/Driver/ToolCh
jtbandes marked an inline comment as done.
jtbandes added inline comments.
Comment at: include/clang/Driver/Options.td:1337-1338
Flags<[CC1Option]>, Group;
+def fno_digraphs : Flag<["-"], "fno-digraphs">, Group,
Flags<[CC1Option]>,
+ HelpText<"Disallow alternative token re
jtbandes updated this revision to Diff 151771.
jtbandes added a comment.
Added `-fdigraphs`. I kept it as a cc1 option because it seemed awkward to
"check whether the last arg was -fno-digraphs and pass only that arg to cc1"
(if just directly forwarding all args, there would be an unrecognized a
jtbandes updated this revision to Diff 151858.
jtbandes marked an inline comment as done.
jtbandes added a comment.
Added an error when language standard doesn't support digraphs.
Still keeping `-fdigraphs` as a cc1 option because then I can distinguish
explicitly-enabled/disabled from the absen
jtbandes added a comment.
Friendly ping!
Repository:
rC Clang
https://reviews.llvm.org/D48266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes updated this revision to Diff 155115.
jtbandes added a comment.
- remove diagnostic; fix formatting
Repository:
rC Clang
https://reviews.llvm.org/D48266
Files:
include/clang/Driver/Options.td
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/Lexer/digr
jtbandes marked 2 inline comments as done.
jtbandes added a comment.
If I understood your comment correctly, you meant to remove the diagnostic
completely (regardless of whether `-fdigraphs` or `-fno-digraphs` is given, and
regardless of whether digraphs were enabled for the selected language)?
jtbandes updated this revision to Diff 155116.
jtbandes marked an inline comment as done.
jtbandes added a comment.
- include %:%: in help text
Repository:
rC Clang
https://reviews.llvm.org/D48266
Files:
include/clang/Driver/Options.td
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/Compi
jtbandes added a comment.
Ping again 😇
Repository:
rC Clang
https://reviews.llvm.org/D48266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337232: [Driver] Add -fno-digraphs (authored by jtbandes,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48266?vs=155116&id=155809#toc
Repos
jtbandes marked an inline comment as done.
jtbandes added a comment.
Ping again 😇
Repository:
rL LLVM
https://reviews.llvm.org/D48266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
jtbandes added a reviewer: rsmith.
jtbandes added a subscriber: rsmith.
jtbandes added a comment.
Adding @rsmith for review based on the commit history of
SemaTemplateDeduction.cpp :)
https://reviews.llvm.org/D40284
___
cfe-commits mailing list
cfe
jtbandes updated this revision to Diff 124033.
jtbandes added a comment.
@erik.pilkington Updated to use a wrapper function. This is definitely less
invasive, but it could defeat some optimizations (any approach that checks the
return value will defeat tail recursion, I suppose...)
https://rev
jtbandes added a comment.
Bump :)
https://reviews.llvm.org/D39937
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes added a comment.
Bump :)
https://reviews.llvm.org/D40284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes created this revision.
Herald added a subscriber: klimek.
This fixes a bug in `ENAS_DontAlign` (introduced in
https://reviews.llvm.org/D32733) where blank lines had an EscapedNewlineColumn
of 0, causing a subtraction to overflow when converted back to `unsigned` and
leading to runaway
jtbandes added a comment.
I can add some clarity but I can't claim to fully understand the whole program
flow here yet, so my explanation is probably insufficient.
The overflow (underflow? but I think that means something specific to FP) is on
line formerly-650: `EscapedNewlineColumn - Offset -
jtbandes updated this revision to Diff 108860.
jtbandes added a comment.
Okay, I think this approach is better:
- Rename the version of `appendNewlineText` used for escaped newlines to
`appendEscapedNewlineText` to reduce confusability.
- If `ENAS_DontAlign`, skip all of the offset calculation l
jtbandes updated this revision to Diff 108863.
jtbandes added a comment.
- Undo change in argument list
https://reviews.llvm.org/D36019
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
=
jtbandes added a comment.
@djasper Bump :)
https://reviews.llvm.org/D36019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes updated this revision to Diff 109898.
jtbandes added a comment.
@djasper ok, done
https://reviews.llvm.org/D36019
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
==
jtbandes added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:650
+for (unsigned i = 0; i < Newlines; ++i)
+ Text.append(UseCRLF ? " \\\r\n" : " \\\n");
+return;
djasper wrote:
> Note that when you have an empty line, this would turn i
jtbandes added a comment.
Thanks. Can you commit this when you get a chance? I don't have permissions.
https://reviews.llvm.org/D36019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes added a comment.
@djasper bump, any thoughts on this?
https://reviews.llvm.org/D34324
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310539: clang-format: Fix bug with ENAS_DontAlign and empty
lines (authored by jtbandes).
Repository:
rL LLVM
https://reviews.llvm.org/D36019
Files:
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/
jtbandes added a comment.
FWIW, I'm able to reproduce the failure using Docker:
Dockerfile:
FROM ubuntu:xenial
RUN apt-get update
RUN apt-get install -y build-essential ca-certificates subversion python
cmake --no-install-recommends
WORKDIR /
RUN svn co -q -r 310537 http://llvm.org
jtbandes added a comment.
I'm still seeing a failure after r301549:
https://gist.github.com/jtbandes/de6118abaadc6c5a5c9b4223a62f596c
Repository:
rL LLVM
https://reviews.llvm.org/D29660
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
jtbandes added a comment.
@gtbercea Hi, I just saw your comment on my gist. (Unfortunately github does
not send email notifications about gist comments; commenting here is probably
better.) If you have Docker installed, it should be easy to get whatever output
you like — just change the Dockerf
jtbandes added a comment.
Herald added subscribers: arphaman, ldionne.
Herald added a project: LLVM.
The lack of `_LIBCPP_CONSTEXPR_AFTER_CXX14` on the `array`
specialization's `begin()`, `end()`, and other methods seems to be a bug:
https://stackoverflow.com/questions/60462569/empty-stdarrayt-0
jtbandes created this revision.
Adjust wording for const-qualification mismatch to be a little more clear.
Also add another diagnostic for a ref qualifier mismatch, which previously
produced a useless error (this error path is simply very old; see
https://reviews.llvm.org/rL119336):
Before:
jtbandes added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1595-1597
+def err_member_function_call_other : Error<
+ "cannot initialize object parameter of type %0 with an expression "
+ "of type %1">;
aaron.ballman wrote:
> I don't t
jtbandes added inline comments.
Comment at: test/CXX/over/over.match/over.match.funcs/p4-0x.cpp:22-24
+ void lvalue() &; // expected-note 2 {{'lvalue' declared here}}
+ void const_lvalue() const&;
+ void rvalue() &&; // expected-note {{'rvalue' declared here}}
jtbandes updated this revision to Diff 123476.
jtbandes added a comment.
- feedback from review & more tests
https://reviews.llvm.org/D39937
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaOverload.cpp
test/CXX/over/over.match/over.match.funcs/p4-0x.cpp
test/SemaCXX/copy-
jtbandes updated this revision to Diff 123481.
jtbandes added a comment.
- spell out full diagnostic the first time
https://reviews.llvm.org/D39937
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaOverload.cpp
test/CXX/over/over.match/over.match.funcs/p4-0x.cpp
test/SemaCX
jtbandes added a comment.
Thanks, will do. Is there an automated system that can run all the tests
//before// I merge rather than waiting for a potential build failure after the
fact?
https://reviews.llvm.org/D39937
___
cfe-commits mailing list
cf
jtbandes created this revision.
The goal of this change is to fix the following suboptimal replacements
currently suggested by clang-tidy:
// with MemberPrefix == "_"
int __foo; // accepted without complaint
// with MemberPrefix == "m_"
int _foo;
^~
m__foo
I fixed this
jtbandes updated this revision to Diff 96180.
jtbandes edited the summary of this revision.
jtbandes added a comment.
Remove unnecessary checks for empty prefix/suffix
https://reviews.llvm.org/D32333
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identif
jtbandes updated this revision to Diff 96181.
jtbandes added a comment.
Cleanup
https://reviews.llvm.org/D32333
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
==
jtbandes created this revision.
Herald added a subscriber: klimek.
This is an attempt to fix the issue described in my recent email:
http://lists.llvm.org/pipermail/cfe-dev/2017-April/053632.html
After digging in for a while, I learned that:
- the spurious line breaks were occurring inside the
jtbandes updated this revision to Diff 96762.
jtbandes added a comment.
Fixed nit
https://reviews.llvm.org/D32333
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
jtbandes marked an inline comment as done.
jtbandes added a comment.
Done, thanks for the review!
What is the procedure for merging patches in? I'm sure I don't have permissions
to do it myself.
https://reviews.llvm.org/D32333
___
cfe-commits mail
jtbandes added a reviewer: bkramer.
jtbandes added a comment.
Not exactly sure who is the right person for this.
https://reviews.llvm.org/D32475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
jtbandes added a comment.
Thanks for the feedback, I'll work on making those changes.
In https://reviews.llvm.org/D32475#738425, @djasper wrote:
> What happens if the function call where this happens actually does not have
> multiple parameters but one parameter with many operands, e.g. changin
jtbandes added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:923
+// Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
+if (Current.FakeLParens.size() > 0 &&
+Current.FakeLParens.back() > prec::Comma) {
jtb
jtbandes added inline comments.
Comment at: unittests/Format/FormatTest.cpp:2597
+ Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+ Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+ Style.BinPackArguments = false;
djasper wrote:
jtbandes updated this revision to Diff 96815.
jtbandes marked 6 inline comments as done.
jtbandes added a comment.
Updates from review
https://reviews.llvm.org/D32475
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
jtbandes added a comment.
@djasper how does this look?
I could try to simplify the examples further, but I feel it's important to have
calls with many subexpressions to exercise this behavior properly. FWIW, my
real-world use case is with `<<` appearing as an output stream operator inside
a ma
jtbandes added a comment.
Friendly ping :) happy to make more changes if needed. Thanks again for your
time.
https://reviews.llvm.org/D32475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
jtbandes created this revision.
Herald added a subscriber: klimek.
This converts the clang-format option `AlignEscapedNewlinesLeft` from a boolean
to an enum, named `AlignEscapedNewlines`, with options `Left` (prev. `true`),
`Right` (prev. `false`), and a new option `DontAlign`.
When set to `Do
jtbandes added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:523
+ if (C.NewlinesBefore > 0 && C.ContinuesPPDirective)
+C.EscapedNewlineColumn = C.PreviousEndOfTokenColumn + 2;
+return;
djasper wrote:
> I think we should not dupli
jtbandes updated this revision to Diff 97404.
jtbandes added a comment.
Modified `appendNewlineText` so we can simply `return` in the DontAlign case.
https://reviews.llvm.org/D32733
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
unittests/For
jtbandes marked an inline comment as done.
jtbandes added a comment.
This seems to work fine.
Separately I noticed a strange edge case, which I think is an existing bug:
#define One\
two \
three;
jtbandes added a comment.
In https://reviews.llvm.org/D32733#743116, @djasper wrote:
> This is an edge case in actual C++. An escaped newline literally gets
> expanded to nothing. So what this reads is
>
> #define Onetwo \
> ...
Yeah, I noticed that, but nonetheless it shouldn't break the ali
jtbandes created this revision.
Herald added a subscriber: klimek.
Fixes an issue where `struct A { int X; };` would be broken onto multiple
lines, but `typedef struct A { int X; } A2;` was collapsed onto a single line.
https://reviews.llvm.org/D32825
Files:
lib/Format/UnwrappedLineFormatter
jtbandes added a comment.
It strikes me that this doesn't handle `using`-style type aliases, but it seems
hard to do this correctly in general, so still valuable to catch this simple,
common case. Let me know if you have any better suggestions!
https://reviews.llvm.org/D32825
__
jtbandes added a comment.
Ping :)
https://reviews.llvm.org/D32825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jtbandes marked an inline comment as done.
jtbandes added inline comments.
Comment at: lib/Format/UnwrappedLineFormatter.cpp:368
// We don't merge short records.
- if (Line.First->isOneOf(tok::kw_class, tok::kw_union, tok::kw_struct,
- Key
jtbandes updated this revision to Diff 98593.
jtbandes added a comment.
Update from review
https://reviews.llvm.org/D32825
Files:
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
==
jtbandes marked an inline comment as done.
jtbandes added a comment.
Another ping.
https://reviews.llvm.org/D32825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
69 matches
Mail list logo