I just realized that this check probably only applies to C++ code since (unless things changed in a recent standard) C doesn't allow struct/enum names to be used without the corresponding keyword, the way C++ does. I've updated the patch to only apply the check for C++, which reduced the number of unit tests that fail with the flag default-enabled to 13.
Cheers, Kaelyn On Fri, Apr 13, 2012 at 11:29 AM, Kaelyn Uhrain <[email protected]> wrote: > Thanks for the review Manuel! > > On Fri, Apr 13, 2012 at 9:13 AM, Manuel Klimek <[email protected]> wrote: > >> + "function %0 shadows %1 %0; uses of %1 %0 will require an '%1' tag">, >> >> I'd use 'a' or a selector for enum. >> > > Ah, thanks for catching that! I was too focused on the "enum" case when > figuring out the wording. ;) > > >> >> +void Sema::ActOnStartFunctionDeclarator(Scope* S, Declarator& D) { >> >> * and & to the variable. >> > > Fixed. Though I just noticed the same issue is present for > ActOnFunctionDeclarator (where I'd copied the two parameters from) and > three other ActOn*Decl* functions a little > above ActOnStartFunctionDeclarator in Sema.h. > > >> + if (TagDecl *TD = dyn_cast_or_null<TagDecl>(*I)) { >> >> I've searched, but couldn't find why *I would ever be NULL here... >> > > Good point; changed to dyn_cast. > > I've attached a new version that also limits which shadowings it warns on > to tag types found in the current lexical context, reducing the number of > other unit test failures from 27 to 20. I also switched the warning to > DefaultIgnore to take care of the remaining 20 failures (since the tests > otherwise work as expected). > > >> Cheers, >> /Manuel >> >> On Thu, Apr 12, 2012 at 10:05 PM, Kaelyn Uhrain <[email protected]> wrote: >> > Ping. >> > >> > Unless I hear something, I'll go ahead and commit the patch tomorrow >> > morning. >> > >> > >> > On Wed, Apr 11, 2012 at 1:44 PM, Kaelyn Uhrain <[email protected]> >> wrote: >> >> >> >> Hi, >> >> >> >> This warning is to help make code like: >> >> >> >> class Foo { >> >> public: >> >> enum Bar { X, Y }; >> >> void SetBar(Bar bar); // Setter >> >> Bar Bar() // Getter >> >> private: >> >> Bar bar_; >> >> }; >> >> void Foo::SetBar(Bar bar) { bar_ = bar; } >> >> Foo::Bar Foo::Bar() { return bar_; } >> >> >> >> >> >> be a bit easier to diagnose. Currently clang spits out a bunch of >> errors >> >> without any real indication of what went wrong (the first error is the >> only >> >> remotely helpful one, but it doesn't give any reason why an 'enum' tag >> has >> >> to be added): >> >> >> >> tmp.cc:7:3: error: must use 'enum' tag to refer to type 'Bar' in this >> >> scope >> >> Bar bar_; >> >> ^ >> >> enum >> >> tmp.cc:9:11: error: variable has incomplete type 'void' >> >> void Foo::SetBar(Bar bar) { bar_ = bar; } >> >> ^ >> >> tmp.cc:9:18: error: use of undeclared identifier 'Bar' >> >> void Foo::SetBar(Bar bar) { bar_ = bar; } >> >> ^ >> >> tmp.cc:9:26: error: expected ';' after top level declarator >> >> void Foo::SetBar(Bar bar) { bar_ = bar; } >> >> ^ >> >> ; >> >> 4 errors generated. >> >> >> >> >> >> With the patch, clang emits a warning before that cascade of errors: >> >> >> >> tmp.cc:5:7: warning: function 'Bar' shadows enum 'Bar'; uses of enum >> 'Bar' >> >> will require an 'enum' >> >> tag [-Wshadow] >> >> Bar Bar(); // Getter >> >> ^ >> >> tmp.cc:3:3: note: 'Bar' declared here >> >> enum Bar { X, Y }; >> >> ^ >> >> tmp.cc:7:3: error: must use 'enum' tag to refer to type 'Bar' in this >> >> scope >> >> <and the rest of the messages from the first output example> >> >> >> >> >> >> I'm sending the patch for pre-commit review both because I'm uncertain >> >> about the wording of the wording, and primarily because I'm not sure >> that >> >> modifying Sema::ActOnStartFunctionDeclarator is the right place for the >> >> check (though it was the best candidate I could find). >> >> >> >> I'd also like to clean up the errors (in a separate patch) but the >> errors >> >> seem to involve a lot of interactions between the parser and sema and >> may >> >> take a while for me to figure out how to get things to work right. ;) >> >> >> >> Cheers, >> >> Kaelyn >> > >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >> > >
warn-func-shadows-tag.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
