Add warning for c++ member variable shadowing

Saleem Abdulrasool via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 24 19:10:14 PST 2017


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 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 check, use isa.  Although, since you use the
value later, it is probably better to write this as:

    if (const auto *RD = cast<CXXRecordDecl>(CurContext))
      CheckShadowInheritedVariabless(Loc, Name.getAsString(), RD, RD);



On Tue, Jan 24, 2017 at 4:06 PM, James Sun via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Coding style change
>
>
>
> *From: *James Sun <jamessun at fb.com>
> *Date: *Tuesday, January 24, 2017 at 2:36 PM
> *To: *"cfe-commits at lists.llvm.org" <cfe-commits at 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 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) adds a member variable shadowing checking, and
>
> (2) incorporates it to the unit tests.
>
>
>
>
>
> Comments are welcome.
>
>
>
> Thanks
>
>
>
> James
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>


-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170124/cabfe81b/attachment.html>


More information about the cfe-commits mailing list