[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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=155809#toc

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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)?

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

[PATCH] D40284: [Sema] Improve diagnostics for template arg deduction

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

[PATCH] D40284: [Sema] Improve diagnostics for template arg deduction

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

[PATCH] D41646: [Sema] Improve diagnostics for const- and ref-qualified member functions

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:

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D41646: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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=128366#toc

[PATCH] D40284: [Sema] Improve diagnostics for template arg deduction

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D40284: [Sema] Improve diagnostics for template arg deduction

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

[PATCH] D40284: [Sema] Improve diagnostics for template arg deduction

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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}}

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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

[PATCH] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

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:

[PATCH] D29660: [OpenMP] Add flag for overwriting default PTX version for OpenMP targets

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

[PATCH] D29660: [OpenMP] Add flag for overwriting default PTX version for OpenMP targets

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

[PATCH] D29660: [OpenMP] Add flag for overwriting default PTX version for OpenMP targets

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D34324: [clang-format] let PointerAlignment dictate spacing of function ref qualifiers

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

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

[PATCH] D34330: [clang-format] handle `if constexpr`

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

[PATCH] D26953: clang-format: handle formatting on constexpr if

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

[PATCH] D34330: [clang-format] handle `if constexpr`

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

[PATCH] D34324: [clang-format] let PointerAlignment dictate spacing of function ref qualifiers

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

[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

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

[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

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

[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

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, -

[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

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

[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

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

[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

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:

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

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

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

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;

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

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

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

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

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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) {

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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.

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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

[PATCH] D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores

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

[PATCH] D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

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

[PATCH] D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores

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

[PATCH] D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores

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

[PATCH] D32333: [clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores

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