arphaman added inline comments.
================ Comment at: lib/Sema/SemaDecl.cpp:5547 + if (ShadowedDecl && !Redeclaration) { + CheckShadow(NewTD, ShadowedDecl, Previous); ---------------- You don't need to use `{}` braces here. ================ Comment at: lib/Sema/SemaDecl.cpp:6753 // the constructor initializes the field with the parameter. - if (isa<CXXConstructorDecl>(NewDC) && isa<ParmVarDecl>(D)) { - // Remember that this was shadowed so we can either warn about its - // modification or its existence depending on warning settings. - D = D->getCanonicalDecl(); - ShadowingDecls.insert({D, FD}); - return; - } + if (isa<CXXConstructorDecl>(NewDC)) + if (ParmVarDecl* PVD = dyn_cast<ParmVarDecl>(D)) { ---------------- ahmedasadi wrote: > arphaman wrote: > > Why is the change to this `if` necessary? It doesn't seem that related to > > the main change. > VarDecl overrides getCanonicalDecl() to return a VarDecl*. As the type of D > was changed from VarDecl* to NamedDecl*, getCanonicalDecl() now returns a > NamedDecl*. > > I created a new ParmVarDecl variable so getCanonicalDecl() returns a VarDecl* > which can be inserted into ShadowingDecls. > > Perhaps it might be better to just cast D->getCanonicalDecl() to a VarDecl > when inserting it into ShadowingDecls? I see, thanks for the explanation. The change is fine then. ================ Comment at: lib/Sema/SemaDecl.cpp:6754 + if (isa<CXXConstructorDecl>(NewDC)) + if (ParmVarDecl* PVD = dyn_cast<ParmVarDecl>(D)) { + // Remember that this was shadowed so we can either warn about its ---------------- You can use `const auto` here. ================ Comment at: test/SemaCXX/warn-shadow.cpp:65 char *data; // expected-warning {{declaration shadows a static data member of 'A'}} + char *a1; // expected-warning {{declaration shadows a typedef in 'A'}} + char *a2; // expected-warning {{declaration shadows a type alias in 'A'}} ---------------- It looks like previously this wouldn't have been a warning. Should we really warn about local variables that shadow typedef names? https://reviews.llvm.org/D31235 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits