Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

This revision was automatically updated to reflect the committed changes. Closed by commit rL260873: [clang-tidy] ClangTidy check to flag uninitialized builtin and pointer fields. (authored by flx). Changed prior to commit: http://reviews.llvm.org/D16517?vs=47757&id=47942#toc Repository: rL

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thank you for the new awesome check and to Jonathan for the original patch! http://reviews.llvm.org/D16517 ___ cfe-commits mailing l

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:206 @@ +205,3 @@ + // Do not propose fixes in macros since we cannot place them correctly. + if (Ctor->getLocStart().isMacroID()) +return; alexfh wrote: > IIUC wha

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx updated this revision to Diff 47757. flx marked 2 inline comments as done. http://reviews.llvm.org/D16517 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp clang-tidy/

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:206 @@ +205,3 @@ + // Do not propose fixes in macros since we cannot place them correctly. + if (Ctor->getLocStart().isMacroID()) +return; IIUC what this is doi

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx updated this revision to Diff 47594. flx added a comment. Added comment that we're returning early if this constructor simply delegates to another constructor. http://reviews.llvm.org/D16517 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelin

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:205 @@ +204,3 @@ + Diag << FixItHint::CreateInsertion( + Field->getSourceRange().getEnd().getLocWithOffset( + Field->getName().size()), alexfh

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx updated this revision to Diff 47590. flx marked 3 inline comments as done. http://reviews.llvm.org/D16517 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp clang-tidy/

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:88 @@ +87,3 @@ +if (InitializerBefore != nullptr) + return InitializerBefore->getRParenLoc().getLocWithOffset(1); +auto StartLocation = InitializerAfter != nullptr --

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx updated this revision to Diff 47280. flx added a comment. Renamed the check to: cppcoreguidelines-pro-type-member-init, note the extra dash between member and init to be consistent with the camelcase notation of the class name ProTypeMemberInitCheck. http://reviews.llvm.org/D16517 Files:

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx added inline comments. Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:178 @@ +177,3 @@ + + SmallPtrSet FieldsToInit; + fieldsRequiringInit(MemberFields, FieldsToInit); alexfh wrote: > What about this? (relates to line 184 now) My comment on this was

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added a comment. Looks good apart from renaming and one comment that's still relevant. Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:178 @@ +177,3 @@ + + SmallPtrSet FieldsToInit; + fieldsRequiringInit(MemberFields, FieldsToInit); What about t

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx updated this revision to Diff 47159. flx marked 12 inline comments as done. flx added a comment. Thanks for the review, Alex! This is an intermediate updated patch that addresses your detailed comments. I'll rename the whole check in the next couple of days and will re-upload again. http:/

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added a comment. In http://reviews.llvm.org/D16517#336157, @flx wrote: > In http://reviews.llvm.org/D16517#336148, @alexfh wrote: > > > A high-level comment: I think, this comment > > still applies. I'm also slightly > > concerned about having this

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx added a comment. Hi Alex, could you take a look at the questions I posted in my last comment. Maybe there is some renaming work I can tackle while you review the change in more detail. http://reviews.llvm.org/D16517 ___ cfe-commits mailing lis

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx added a comment. In http://reviews.llvm.org/D16517#336148, @alexfh wrote: > A high-level comment: I think, this comment > still applies. I'm also slightly > concerned about having this check in misc-, since the check isn't universally > applicable (e

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added a comment. A high-level comment: I think, this comment still applies. I'm also slightly concerned about having this check in misc-, since the check isn't universally applicable (e.g. based on a couple of discussions, I guess LLVM community wo

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx updated this revision to Diff 45838. http://reviews.llvm.org/D16517 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/UninitializedFieldCheck.cpp clang-tidy/misc/UninitializedFieldCheck.h docs/clang-tidy/checks/misc-uninitialized-field.rst te

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

flx removed rL LLVM as the repository for this revision. flx updated this revision to Diff 45837. flx added a comment. Removed unused include and changed casing on Builder variable. http://reviews.llvm.org/D16517 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp cla