[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG66945b62f42f: Add Optional overload to DiagnosticBuilder operator (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249361. njames93 marked 2 inline comments as done. njames93 added a comment. - remove unnecessary semi colons Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/ https://reviews.llvm.org/D75714 Files:

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/Diagnostic.h:1298 + return DB; +}; + s/;// (and in functions below as well) Repository: rG LLVM

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249307. njames93 added a comment. Herald added subscribers: arphaman, kbarton, nemanjai. - Add usages, removed optional on arrayrefs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75714#1913230 , @aaron.ballman wrote: > Is there any code we can cleanup as a result of adding these overloads? I > would have expected to see some code changes justifying each additional > overload, which would also give

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Is there any code we can cleanup as a result of adding these overloads? I would have expected to see some code changes justifying each additional overload, which would also give us test coverage for the changes. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249121. njames93 added a comment. - Removed template in favour of specific overloads Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/ https://reviews.llvm.org/D75714 Files:

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. I can see it being useful for fix-its and source ranges, however I have a concern about it accidentally eating arguments that were supposed to go into a format string. For that, I second Aaron's suggestion to implement an extension for format strings. If you don't

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D75714#1912400 , @njames93 wrote: > In D75714#1912326 , @lebedev.ri > wrote: > > > What's the use case here? > > > Mainly syntactic sugar, If you have a function that maybe

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75714#1912326 , @lebedev.ri wrote: > What's the use case here? Mainly syntactic sugar, If you have a function that maybe returns a `FixItHint` or `SourceRange` you can pass it directly to the `DiagnosticBuilder`.

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. What's the use case here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/ https://reviews.llvm.org/D75714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang/include/clang/Basic/Diagnostic.h:1318 + const llvm::Optional ) { + if (Opt) { +DB << *Opt; Should this be disabled on

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-05 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75714 Files: clang/include/clang/Basic/Diagnostic.h Index: clang/include/clang/Basic/Diagnostic.h