Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Thanks for the comments. The new version is attached.
Wrt two of your questions:

(1)  “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.”
I’ve read through the doxygen wiki; hopefully it’s fixed; let me know if it’s still wrong

(2) “Why are you checking that the DeclContext has a definition rather than the record itself?”
There are cases like “struct A; struct B : A {};”, where A does not have a definition. The compiler will hit an assertion failure if we call A.bases() directly.

Thanks

James


From: Saleem Abdulrasool <compnerd at compnerd.org>
Date: Tuesday, January 24, 2017 at 7:10 PM
To: James Sun <jamessun at fb.com>
Cc: "cfe-commits at lists.llvm.org" <cfe-commits at lists.llvm.org>, Aaron Ballman <aaron at aaronballman.com>, Richard Smith <richard at 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 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<mailto:cfe-commits at lists.llvm.org>> wrote:
Coding style change

From: James Sun <jamessun at fb.com<mailto:jamessun at fb.com>>
Date: Tuesday, January 24, 2017 at 2:36 PM
To: "cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>" <cfe-commits at lists.llvm.org<mailto: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<mailto:cfe-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=ikRH8URaurZA7JMys57d3w&m=lheFEjRie_ahss0mWHaJIa1eNMlFv2DMH5ZWHGQvo8U&s=750RLygVMQIDJB7IKBhOef4zIDHerGwb7aJZAY2aP9U&e=>



--
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170125/7242a3ab/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: inheritance-shadow-warning-v0.3.patch
Type: application/octet-stream
Size: 9822 bytes
Desc: inheritance-shadow-warning-v0.3.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170125/7242a3ab/attachment-0001.obj>