[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line updated this revision to Diff 482424. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139919/new/ https://reviews.llvm.org/D139919 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.h

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line updated this revision to Diff 482418. addr2line added a comment. finally CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139919/new/ https://reviews.llvm.org/D139919 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.h

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line added a comment. In D139919#3991407 , @carlosgalvezp wrote: > Sounds like the change to the YAML parser can potentially affect many other > components other than clang-tidy - should that be done in a separate patch? Only these left.

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line updated this revision to Diff 482412. addr2line added a comment. Also in YAMLTraits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139919/new/ https://reviews.llvm.org/D139919 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Sounds like the change to the YAML parser can potentially affect many other components other than clang-tidy - should that be done in a separate patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139919/new/ https://reviews.llvm.org/D139919

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line updated this revision to Diff 482404. addr2line marked an inline comment as done. addr2line added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. It also need to change the return type in YAMLParser. CHANGES SINCE LAST ACTION

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line marked an inline comment as done. addr2line added a comment. Oops, the check on Debian failed. I'm looking into it... Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:457 - } // namespace tidy } // namespace clang MaskRay wrote: > drop

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line updated this revision to Diff 482395. addr2line added a comment. remove the change to the unrelated lines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139919/new/ https://reviews.llvm.org/D139919 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D139919#3991250 , @MaskRay wrote: > In D139919#3991242 , @carlosgalvezp > wrote: > >> AFAIK it's preferred to use the LLVM types instead of the Standard types: >> >>> When both

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D139919#3991247 , @addr2line wrote: > Sorry, I just saw a lot of changes in the llvm repo that change > llvm::Optional to std::optional, like this one >

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D139919#3991242 , @carlosgalvezp wrote: > AFAIK it's preferred to use the LLVM types instead of the Standard types: > >> When both C++ and the LLVM support libraries provide similar functionality, >> and there isn’t a

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line added a comment. Sorry, I just saw a lot of changes in the llvm repo that change llvm::Optional to std::optional, like this one . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. AFAIK it's preferred to use the LLVM types instead of the Standard types: > When both C++ and the LLVM support libraries provide similar functionality, > and there

[PATCH] D139919: use std::optional in ClangTidyCheck

2022-12-13 Thread Yuao Ma via Phabricator via cfe-commits
addr2line created this revision. addr2line added a reviewer: MaskRay. addr2line added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, StephenFan. Herald added a reviewer: njames93. Herald added a project: All. addr2line requested review of this revision. Herald added a