Add warning for c++ member variable shadowing

2017-01-24 Thread James Sun via cfe-commits
Dear members Here is a patch (attached) to create warnings where a member variable shadows the one in one of its inheriting classes. For cases where we really don't want to shadow member variables, e.g. class a { int foo; } class b : a { int foo; // Generate a warning } This patch (1) add

Re: Add warning for c++ member variable shadowing

2017-01-24 Thread James Sun via cfe-commits
Coding style change From: James Sun Date: Tuesday, January 24, 2017 at 2:36 PM To: "cfe-commits@lists.llvm.org" Subject: Add warning for c++ member variable shadowing Dear members Here is a patch (attached) to create warnings where a member variable shadows the one in one of its inheriting cl

Re: Add warning for c++ member variable shadowing

2017-01-24 Thread James Sun via cfe-commits
onst auto *RD = cast(CurContext)) CheckShadowInheritedVariabless(Loc, Name.getAsString(), RD, RD); On Tue, Jan 24, 2017 at 4:06 PM, James Sun via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: Coding style change From: James Sun mailto:james...@fb.com>> Date: Tuesday, J

Re: Add warning for c++ member variable shadowing

2017-01-30 Thread James Sun via cfe-commits
the name? Why are you checking that the DeclContext has a definition rather than the record itself? Space after the <<. Don't use the cast for the check, use isa. Although, since you use the value later, it is probably better to write this as: if (const auto *RD = cast(CurContex

Re: Add warning for c++ member variable shadowing

2017-01-31 Thread James Sun via cfe-commits
as a definition rather than the record itself? Space after the <<. Don't use the cast for the check, use isa. Although, since you use the value later, it is probably better to write this as: if (const auto *RD = cast(CurContext)) CheckShadowInheritedVariabless(Loc, Name.get

Re: Add warning for c++ member variable shadowing

2017-01-31 Thread James Sun via cfe-commits
h the name? Why are you checking that the DeclContext has a definition rather than the record itself? Space after the <<. Don't use the cast for the check, use isa. Although, since you use the value later, it is probably better to write this as: if (const auto *RD = cast(Cur

Re: Add warning for c++ member variable shadowing

2017-02-02 Thread James Sun via cfe-commits
ore warning comments are restating what is in the code, please remove them. Could you make the header and the source file match the name? Why are you checking that the DeclContext has a definition rather than the record itself? Space after the <<. Don't use the cast for the c

Re: Add warning for c++ member variable shadowing

2017-02-04 Thread James Sun via cfe-commits
n form. Im not sure if its in line with the rest of the code. The ignore warning comments are restating what is in the code, please remove them. Could you make the header and the source file match the name? Why are you checking that the DeclContext has a definition rather than the reco

Re: Add warning for c++ member variable shadowing

2017-02-04 Thread James Sun via cfe-commits
Ballman mailto:aa...@aaronballman.com>>, Richard Smith mailto:rich...@metafoo.co.uk>> Subject: Re: Add warning for c++ member variable shadowing Some more stylistic comments: The description that you have on CheckShadowInheritedVariables isn't really the type of comments that w

Re: Add warning for c++ member variable shadowing

2017-02-04 Thread James Sun via cfe-commits
bject: Re: Add warning for c++ member variable shadowing Some more stylistic comments: The description that you have on CheckShadowInheritedVariables isn't really the type of comments that we have in doxygen form. Im not sure if its in line with the rest of the code. The ignore wa

Re: Add warning for c++ member variable shadowing

2017-02-04 Thread James Sun via cfe-commits
e stylistic comments: The description that you have on CheckShadowInheritedVariables isn't really the type of comments that we have in doxygen form. Im not sure if its in line with the rest of the code. The ignore warning comments are restating what is in the code, please remove them. Cou

Re: Add warning for c++ member variable shadowing

2017-02-04 Thread James Sun via cfe-commits
riables isn't really the type of comments that we have in doxygen form. Im not sure if its in line with the rest of the code. The ignore warning comments are restating what is in the code, please remove them. Could you make the header and the source file match the name? Why are you chec

Re: Add warning for c++ member variable shadowing

2017-02-04 Thread James Sun via cfe-commits
lto:cfe-commits@lists.llvm.org>>, Aaron Ballman mailto:aa...@aaronballman.com>>, Richard Smith mailto:rich...@metafoo.co.uk>> Subject: Re: Add warning for c++ member variable shadowing Some more stylistic comments: The description that you have on CheckShadowInheritedVariables

Re: Add warning for c++ member variable shadowing

2017-02-07 Thread James Sun via cfe-commits
ists.llvm.org<mailto:cfe-commits@lists.llvm.org>" mailto:cfe-commits@lists.llvm.org>>, Aaron Ballman mailto:aa...@aaronballman.com>>, Richard Smith mailto:rich...@metafoo.co.uk>> Subject: Re: Add warning for c++ member variable shadowing Some more stylistic comments: