[PATCH] D118754: [DebugInfo] Always emit `.debug_names` with dwarf 5 for Apple platforms

This revision was automatically updated to reflect the committed changes. Closed by commit rGe0d57295bf6a: [DebugInfo] Always emit `.debug_names` with DWARF 5 for Apple platforms (authored by JDevlieghere). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LL

[PATCH] D31887: [clangd] Add documentation page

JDevlieghere added a comment. Great to see this coming along! Comment at: docs/clangd.rst:10 + +:program:`clangd` is an implementation of the `Language Server Protocol `_ leveraging Clang. +Clangd's goal is to provide lang

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere created this revision. JDevlieghere added reviewers: hokein, Prazek, aaron.ballman, alexfh. JDevlieghere added a subscriber: cfe-commits. JDevlieghere set the repository for this revision to rL LLVM. JDevlieghere added a project: clang-tools-extra. Herald added a subscriber: mgorny. R

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 84543. JDevlieghere added a comment. - Fixed comments from @malcolm.parsons In https://reviews.llvm.org/D28768#647177, @malcolm.parsons wrote: > What happens if the function has `auto` as the return type? Nothing, the constructor is left untouched.

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. In https://reviews.llvm.org/D28768#647203, @malcolm.parsons wrote: > In https://reviews.llvm.org/D28768#647198, @JDevlieghere wrote: > > > In https://reviews.llvm.org/D28768#647177, @malcolm.parsons wrote: > > > > > What

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere added a comment. In https://reviews.llvm.org/D28768#647206, @malcolm.parsons wrote: > In https://reviews.llvm.org/D28768#647204, @JDevlieghere wrote: > > > I wanted to do that but it seems that the test script hard codes it to > > C++11, and the `auto` return type is a C++14 feature

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 84549. JDevlieghere added a comment. Add test for auto return type. Repository: rL LLVM https://reviews.llvm.org/D28768 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/ReturnBracedIn

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 84581. JDevlieghere marked 8 inline comments as done. JDevlieghere added a comment. - Added more tests - Improved matchers - Addressed code review comments from @malcolm.parsons, @Prazek and @aaron.ballman Repository: rL LLVM https://reviews.llvm.or

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere added a comment. In https://reviews.llvm.org/D28768#647333, @Prazek wrote: > Thanks for the check. Have you run it on llvm? Not yet, there was an issue with templated types and I wanted to fix that first. It's running now. Repository: rL LLVM https://reviews.llvm.org/D28768

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 84589. JDevlieghere added a comment. - Don't replace explicit constructors - Add @Prazek's suggested tests Repository: rL LLVM https://reviews.llvm.org/D28768 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cp

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere added a comment. In https://reviews.llvm.org/D28768#647949, @Prazek wrote: > Do you have some results from running it on LLVM? If nothing crashes and all > fixit are correct then LGTM. There's still an issue with type narrowing, which is allowed in regular constructor but not in

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 84700. JDevlieghere added a comment. - Added test cases suggested by @Prazek - Added test cases suggested by @alexfh I don't match on `CXXUnresolvedConstructExpr` so the template dependent cases are not impacted by this check. This is what I intended, b

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 85937. JDevlieghere added a comment. - Added missing instantiations in test Repository: rL LLVM https://reviews.llvm.org/D28768 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/Return

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 87228. JDevlieghere added a comment. - Add explicit check to matcher as suggested by @aaron.ballman Repository: rL LLVM https://reviews.llvm.org/D28768 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp cla

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 87823. JDevlieghere marked 11 inline comments as done. JDevlieghere added a comment. - Fixed issues raised by @alexfh Repository: rL LLVM https://reviews.llvm.org/D28768 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTi

[PATCH] D34932: [clang-tidy] Resolve cppcoreguidelines-pro-type-member-init false positive

This revision was automatically updated to reflect the committed changes. Closed by commit rL307040: [clang-tidy] Resolve cppcoreguidelines-pro-type-member-init false positive (authored by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D34932?vs=105023&id=105102#toc Reposito

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 88226. JDevlieghere marked 5 inline comments as done. JDevlieghere added a comment. Thanks for reviewing @aaron.ballman and @alexfh. I have updated the diff to address your issues. While looking at the logic that checked for matching argument types I di

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 88409. JDevlieghere marked 4 inline comments as done. JDevlieghere added a comment. - Updated the diff with comments from @alexfh Thanks for mentioning the check fixes pattern, it's quite crucial but I never really thought about that! Repository: rL

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

JDevlieghere updated this revision to Diff 88550. JDevlieghere added a comment. Fixed latest comment from @aaron.ballman before landing. Repository: rL LLVM https://reviews.llvm.org/D28768 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tid

[PATCH] D30792: Use callback for internalizing linked symbols.

JDevlieghere updated this revision to Diff 91282. JDevlieghere marked 3 inline comments as done. JDevlieghere added a comment. - Reformat - Call helper rather than initializing the pass Repository: rL LLVM https://reviews.llvm.org/D30792 Files: include/clang/CodeGen/CodeGenAction.h inclu

[PATCH] D30792: Use callback for internalizing linked symbols.

JDevlieghere updated this revision to Diff 91292. JDevlieghere added a comment. - Pass StringSet by const ref. Repository: rL LLVM https://reviews.llvm.org/D30792 Files: include/clang/CodeGen/CodeGenAction.h include/clang/Frontend/CodeGenOptions.h lib/CodeGen/CodeGenAction.cpp lib/Fr

[PATCH] D30792: Use callback for internalizing linked symbols.

JDevlieghere abandoned this revision. JDevlieghere added a comment. In https://reviews.llvm.org/D30792#697802, @mehdi_amini wrote: > Off topic, but since this is a rev lock change with LLVM, you can to all of > in a single revision with: > http://llvm.org/docs/GettingStarted.html#for-developers

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

JDevlieghere added a comment. Some small stuff I noticed while reading through the code, I didn't check it in much detail though. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:43 +void OneNamePerDeclarationCheck::check(const MatchFinder::MatchResult &Result

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

JDevlieghere added a comment. In https://reviews.llvm.org/D26167#621928, @malcolm.parsons wrote: > I tried this check on my company's codebase. > It misses cases where malloc is used through a function pointer. > Should clang-tidy warn about making a function pointer to these functions? Sound

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

JDevlieghere added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 + const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair"); + assert(InnerCtorCall || MakePairCall); It's highly recommended to put some kind of error message i

[PATCH] D31887: [clangd] Add documentation page

JDevlieghere added a comment. Looks good, nothing to add from my side. https://reviews.llvm.org/D31887 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

JDevlieghere added a comment. No comments from me either, looks good! https://reviews.llvm.org/D32690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2706605 , @ldionne wrote: > In D45639#2706589 , @dexonsmith > wrote: > >> I'm not sure I'm totally following, but just want to double check that the >> tests won't somehow u

[PATCH] D100901: [CMake][llvm] avoid conflict w/ (and use when available) new builtin check_linker_flag

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100901/new/ https://reviews.llvm.org/D100901 __

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

JDevlieghere added a comment. Hey Kristina, this broke TestSetData.py on GreenDragon: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/39089/ Since the bot has been red for several hours I went ahead and reverted your change in 4cb79294e8df8c91ae15264d1014361815d34a53

[PATCH] D115438: [lldb] Remove unused lldb.cpp

JDevlieghere added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115438/new/ https://reviews.llvm.org/D115438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D100086: Include `llvm-config` and `not` in AppleClang toolchains.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100086/new/ https://reviews.llvm.org/D100086 __

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. This breaks `TestAppleSimulatorOSType.py ` on GreenDragon. First failed build: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31346/ /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang main.o -g -O0 -fno-builtin -isysroot "/Application

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. @phosek I've reverted this in 05eeed9691aeb3e0316712195b998e9078cdceb0 to turn the bot green again. Happy to help you look into this tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2705919 , @phosek wrote: > In D45639#2705702 , @ldionne wrote: > >> In D45639#2703913 , @JDevlieghere >> wrote: >> >>> This breaks `T

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. Given that these tests are macOS specific and already require a specific SDK, I'll just update them to use the compiler from the SDK instead of the just-built one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45639/n

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2706364 , @JDevlieghere wrote: > Given that these tests are macOS specific and already require a specific SDK, > I'll just update them to use the compiler from the SDK instead of the > just-built one. Done in 5d1

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

JDevlieghere added a comment. LGTM for LLDB Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94374/new/ https://reviews.llvm.org/D94374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D111269: [clang][Driver] Make multiarch output file basenames reproducible

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111269/new/ https://reviews.llvm.org/D111269 ___

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1179-1180 + if (ExternalFS) +ExternalFS->setCurrentWorkingDirectory(Path); + I'm pretty sure there was a reason we stopped doing this. There should be some discussion a

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added a comment. I'm sure I'm missing something, but after rereading the patch several times I still don't see the functional change. It just looks like it's renaming every instance of `Path` to `CanonicalPath` and `Path_` to `Path`? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added a comment. Keith and I discussed this offline. My suggestion was to do the following: 1. Check the overlay for the canonicalized path 2. Check the fall-through for the canonicalized path 3. Check the fall-through for the original path If I understand correctly, this patch does

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added a comment. Yes, Keith and I came to the same conclusion yesterday. I was worried about tracking both paths at all times, but I like your suggestion of only changing the path when requested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

<    1   2   3