[cfe-commits] [PATCH] Add a warning for a function declaration that shadows a tag type

[Kaelyn Uhrain]

Thanks for the review Manuel!

On Fri, Apr 13, 2012 at 9:13 AM, Manuel Klimek <klimek at google.com> 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 <rikka at google.com> 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 <rikka at google.com> 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
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120413/3a262bd1/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: warn-func-shadows-tag.diff
Type: application/octet-stream
Size: 3639 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120413/3a262bd1/attachment.obj>