[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-24 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8fa5e98fd191: [clang-format] Remove duplciate code from Invalid BOM detection (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D689

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-23 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 ___ cfe-commits mailing list cfe-commits

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 225779. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. I think we agree it should be (StringRef ), no const,no reference CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D6891

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-17 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. Thanks for clearing it up! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-16 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D68914#1710407 , @owenpan wrote: > I did a quick search of the LLVM code base and found dozens of `const > StringRef &`, including the `Twine` API >

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. I did a quick search of the LLVM code base and found dozens of `const StringRef &`, including the `Twine` API . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https:

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-15 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment. In D68914#1709002 , @owenpan wrote: > Here are some `const StringRef &` examples > . StringRef already is a const pointe

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. Here are some `const StringRef &` examples . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 ___

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:232 +// nullptr +static const char *getInvalidBOM(StringRef BufStr); }; klimek wrote: > owenpan wrote: > > Can you make the parameter `const`? > StringRef is inherentl

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I tend to agree mainly given that StringRef is used without const elsewhere. I'll bow to better judgement. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 ___ cfe-com

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-14 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:232 +// nullptr +static const char *getInvalidBOM(StringRef BufStr); }; owenpan wrote: > Can you make the parameter `const`? StringRef is inherently const though, right

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 224776. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Updating with review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 Files: clang/include/clang/Basi

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:232 +// nullptr +static const char *getInvalidBOM(StringRef BufStr); }; Can you make the parameter `const`? Comment at: clang/lib/Basic/SourceManage

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. Thank you for taking care of this! When I fixed a bug in this part of the code last time, I had to do it in both files. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 _

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: bruno, arphaman, klimek, owenpan, mitchell-stellar, dexonsmith. MyDeveloperDay added projects: clang, clang-format, clang-tools-extra. Review comments on D68767: [clang-format] NFC - Move functionality into functions to help c