[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

2018-02-01 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added inline comments. Comment at: lib/Tooling/Tooling.cpp:404 if (CompileCommandsForFile.empty()) { // FIXME: There are two use cases here: doing a fuzzy // "find . -name '*.cc' |xargs tool" match, where as a user I don't care ioeric w

[PATCH] D42810: [Sema] Add implicit members even for invalid CXXRecordDecls

2018-02-01 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D42810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

2018-02-02 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. Removing FIXME seems right to me. Repository: rC Clang https://reviews.llvm.org/D42361 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

2017-02-13 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer created this revision. Herald added a subscriber: mgorny. This requires an accessible compilation database. The parsing is done asynchronously on a separate thread. https://reviews.llvm.org/D29886 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/CMakeLists.txt clangd/Clan

[PATCH] D29899: [clang-tidy] Add support for NOLINTNEXTLINE.

2017-02-13 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer created this revision. Herald added a subscriber: JDevlieghere. https://reviews.llvm.org/D29899 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp test/clang-tidy/nolintnextline.cpp Index: test/clang-tidy/nolintnextline.cpp

[PATCH] D29899: [clang-tidy] Add support for NOLINTNEXTLINE.

2017-02-14 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer updated this revision to Diff 88352. bkramer added a comment. - Simplify code by not worrying about \r - Don't allow blank lines between NOLINTNEXTLINE and the warning https://reviews.llvm.org/D29899 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp test/clang-tidy/nolintnextline.cp

[PATCH] D29755: Cache FileID when translating diagnostics in PCH files

2017-02-14 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D29755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

2017-02-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer updated this revision to Diff 88515. bkramer marked 16 inline comments as done. bkramer added a comment. - Address review comments. https://reviews.llvm.org/D29886 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/CMakeLists.txt clangd/ClangDMain.cpp clangd/DocumentStore

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

2017-02-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added inline comments. Comment at: clangd/ASTManager.cpp:21 +using namespace clang; +using namespace clangd; + ioeric wrote: > Any reason not to wrap code in namespaces instead? I don't really have an opinion one way or the other, but this seems to be the

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

2017-02-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer updated this revision to Diff 88522. bkramer added a comment. - Do not lock while running DocumentStore callbacks - Replace fake queue with a real queue (but it still has at most one element. https://reviews.llvm.org/D29886 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/C

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

2017-02-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer updated this revision to Diff 88523. bkramer added a comment. - Inline the request struct again. https://reviews.llvm.org/D29886 Files: clangd/ASTManager.cpp clangd/ASTManager.h clangd/CMakeLists.txt clangd/ClangDMain.cpp clangd/DocumentStore.h test/clangd/diagnostics.test

[PATCH] D29990: [clangd] Implement format on type

2017-02-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added inline comments. Comment at: clangd/ProtocolHandlers.cpp:117 + // starting from there. + StringRef Code = Store.getDocument(DOTFP->textDocument.uri); + size_t CursorPos = positionToOffset(Code, DOTFP->position); This should be a std::string in tr

[PATCH] D29990: [clangd] Implement format on type

2017-02-16 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D29990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D30191: [clang-tidy] Reword the "code outside header guard" warning.

2017-02-21 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer created this revision. Herald added a subscriber: JDevlieghere. The check doesn't really know if the code it is warning about came before or after the header guard, so phrase it more neutral instead of complaining about code before the header guard. The location for the warning is still no

[PATCH] D30399: clang-format: [JS] whitespace after async in arrow functions.

2017-02-27 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg, but I don't really know JS ;) https://reviews.llvm.org/D30399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-27 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. lg Comment at: include-fixer/SymbolIndexManager.cpp:156 + for (const auto &SymAndSig : MatchedSymbols) +Res.push_back(SymAndSig.Symbol); + return Res; std::move Comment at: includ

[PATCH] D30498: [clangd] Add support for FixIts.

2017-03-01 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer created this revision. This uses CodeActions to show 'apply fix' actions when code actions are requested for a location. The actions themselves make use of a clangd.applyFix command which has to be implemented on the editor side. I included an implementation for vscode. This also adds a -

[PATCH] D30498: [clangd] Add support for FixIts.

2017-03-01 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer updated this revision to Diff 90181. bkramer added a comment. - Use typedef instead of decltype() - Rename local variable not to shadow member. - Give FixIts their own mutex - Use const instead of let in typescript https://reviews.llvm.org/D30498 Files: clangd/ASTManager.cpp clangd/

[PATCH] D27810: FileManager: mark virtual file entries as valid entries

2017-03-08 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. I assume this is fine but I don't really understand what's going on. A test case would be great. https://reviews.llvm.org/D27810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D30675: [clangd] Fix not being able to attach a debugger on macOS

2017-03-08 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. Generally makes sense. Is there any reason for the #ifdef? Windows has errno and EINTR too. Repository: rL LLVM https://reviews.llvm.org/D30675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D30685: [include-fixer] Remove line number from Symbol identity

2017-03-08 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. lg https://reviews.llvm.org/D30685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. lg as a prototype. https://reviews.llvm.org/D30720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27132: Do not do raw name replacement when FromDecl is a class forward-declaration.

2016-11-25 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added inline comments. This revision is now accepted and ready to land. Comment at: lib/Tooling/Core/Lookup.cpp:131 + // still considered as referring to the original definition given the nature + // of forward-declarations, so we can't d

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

2016-12-01 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. This looks good to me. We can add finer-grained desugaring later if it's really needed. https://reviews.llvm.org/D27207 ___ cfe-commits mailin

[PATCH] D27504: Compilation database test: don't try to output to CWD

2016-12-07 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. Annoying. This test really shouldn't generate binary object files. Can we check in this patch now to unblock stuff and then look into how to do a dry run with -MJ? Usually driver tests use `

[PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

2016-12-07 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer updated this revision to Diff 80574. bkramer marked 9 inline comments as done. bkramer added a comment. Herald added a subscriber: JDevlieghere. - Moved external linkage check to matcher - added msvcrt entry point check - fixed comment typos. https://reviews.llvm.org/D23130 Files: cla

[PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

2016-12-07 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added inline comments. Comment at: clang-tidy/google/GlobalNamesCheck.cpp:90 +// extern "C" globals need to be in the global namespace. +if (VDecl->isExternC()) + return; alexfh wrote: > Is this already filtered-out by the matcher? Nope. ==

[PATCH] D27523: [change-namespace] don't fix using shadow decls in classes.

2016-12-07 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added inline comments. This revision is now accepted and ready to land. Comment at: change-namespace/ChangeNamespace.cpp:331 + // Using shadow declarations in classes always refers to base class, which + // does not need to be qualified s

[PATCH] D26887: [Driver] Fix finding multilib gcc install on Gentoo (with gcc-config)

2016-12-12 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. I believe this is fine. https://reviews.llvm.org/D26887 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D32238: [Clangd] Failed to decode params using 1.x-compatible request message

2017-04-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. A test case would be nice. Repository: rL LLVM https://reviews.llvm.org/D32238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32234: [Clangd] Support Authority-less URIs

2017-04-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. This is fine. Test case would be nice though. Repository: rL LLVM https://reviews.llvm.org/D32234 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D32238: [Clangd] Failed to decode params using 1.x-compatible request message

2017-04-21 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D32238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D32351: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources

2017-04-21 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. lg from my side. It would be good to wait until Manuel is back though, I think he had plans for extending this interface at some point. https://reviews.llvm.org/D32351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32234: [Clangd] Support Authority-less URIs

2017-04-21 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. Still looking good. Will commit this soon. https://reviews.llvm.org/D32234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.

2017-04-28 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. Can you please run clang-format on this change? There are pieces that don't follow the style. Also the mutable state in PrintingPolicy is really really ugly, is there no better way for this? :( https://reviews.llvm.org/D30946 ___

[PATCH] D33103: [clang-tidy] TwineLocalCheck: add param # checking

2017-05-12 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. looks good, thanks! https://reviews.llvm.org/D33103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D33047: [ClangD] Refactor clangd into separate components

2017-05-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. I believe this is good enough now. https://reviews.llvm.org/D33047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D33273: Recommit "[include-fixer] Don't throw exception when parsing unknown ar… …guments in vim script."

2017-05-17 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. looks good, thanks! https://reviews.llvm.org/D33273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D33270: [Frontend] Remove unused TemporaryFiles

2017-05-17 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. Kill it :) https://reviews.llvm.org/D33270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33350: [clangd] Switch to incomplete translation units

2017-05-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D33350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D33042: [libclang] Allow to suspend a translation unit.

2017-05-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. This makes sense to me. Comment at: lib/Frontend/ASTUnit.cpp:2089 +void ASTUnit::ResetForParse() +{ + SavedMainFileBuffer.reset(); Put the `{` on the same

[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).

2017-05-22 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. looks good to me https://reviews.llvm.org/D33395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D33397: Allow to use vfs::FileSystem for file accesses inside ASTUnit.

2017-05-23 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D33397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.

2017-05-23 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added inline comments. Comment at: unittests/clangd/ClangdTests.cpp:118 + llvm::SmallString<128> TmpDir2; + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2); + We should delete those when we're done, otherwise the unit test will

[PATCH] D116037: [clang-include-fixer] Fix incorrect ranking because of dangling references

2021-12-20 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116037/new/ https://reviews.llvm.org/D116037 ___

[PATCH] D116037: [clang-include-fixer] Fix incorrect ranking because of dangling references

2021-12-20 Thread Benjamin Kramer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGcff192739bb6: [clang-include-fixer] Fix incorrect ranking because of dangling references (authored by danlark, committed by bkramer). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

2021-03-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer created this revision. bkramer added reviewers: rupprecht, rjmccall, rsmith. bkramer requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This looks like an oversight from a875721d8a2d

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

2021-03-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. Landing this as it's a pretty bad crasher. Feel free to point out all my mistakes in post-commit review :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98980/new/ https://reviews.llvm.org/D98980 _

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

2021-03-19 Thread Benjamin Kramer via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG19d2c65ddd75: [CodeGen] Don't crash on for loops with cond variables and no increment (authored by bkramer). Repository: rG LLVM Github Monorepo

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

2021-03-19 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment. Invalid IR generation should be addressed by 19d2c65ddd757997785163709800f837857f686d Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98816/new/ https://

<    7   8   9   10   11   12