r232750, thanks! On Tue, Mar 17, 2015 at 9:44 AM, Douglas Gregor <[email protected]> wrote:
> > On Mar 2, 2015, at 2:25 PM, Nico Weber <[email protected]> wrote: > > One more tweak: Apparently for `@interface A @end @class A;`, a lookup for > A finds the @interface decl, not the @class redecl. > > > Yeah, I think this is a longstanding hack from when we starting tracking > redeclaration chains for Objective-C class declarations (and @class started > being an ObjCInterfaceDecl). We (ahem, I) didn’t hunt down all of the > places where we were expecting name lookup to return the definition. > > So for ObjCInterfaceDecls, the explicit loop is necessary – this patch > adds it back for that case. With this change, I can build most of chrome > with this warning enabled (and some declaration tweaks in chrome). (Only > "most of" because the build is still running – no known bugs). > > > Okay. Everything else LGTM, I say “go for it!”. > > - Doug > > On Sun, Mar 1, 2015 at 7:52 PM, Nico Weber <[email protected]> wrote: > >> On Mon, Feb 2, 2015 at 10:26 PM, Douglas Gregor <[email protected]> >> wrote: >> >>> Hi Nico, >>> >>> On Jan 8, 2015, at 6:14 PM, Nico Weber <[email protected]> wrote: >>> >>> Hi, >>> >>> the Mac OS X and iOS SDKs add new functions in new releases. Apple >>> recommends using the newest SDK and setting the deployment target to >>> whatever old OS version one wants to support, and only calling new >>> functions after checking that they are available at runtime. >>> >>> In practice, we (Chromium) get this wrong. Others who support old OS X >>> versions get this wrong too. Hence, we (Chromium) use a very old SDK and >>> then manually declare new functions when we want to call them – this >>> reduces the chance of us forgetting if they are available at runtime >>> considerably, in practice. But using an old SDK has its problems – >>> sometimes the frameworks check which SDK an app was linked against and only >>> then activate bug fixes, and newer Xcodes don't ship include old SDKs. >>> >>> >>> That’s an interesting approach to handling the availability problem; I >>> hadn’t heard of it before, but I see the logic there. >>> >>> Ideally, we could use a new SDK but get a warning when we use a new API >>> without a manual redeclaration – this protects us against new APIs the same >>> way using an old SDK does without the drawbacks that this brings. >>> >>> The attached patch is a sketch how such a warning might work. How >>> repulsive is this idea? Are there other approaches to this problem? If the >>> basic idea is ok: >>> >>> >>> This is a drastically different approach than I’d imagined. Whenever >>> I’ve thought about this problem, I’ve always come back to some form of >>> dataflow analysis that checks whether uses of “not-yet-introduced” API is >>> used in a sane way: is it dominated by some check that implies the >>> availability, e.g., a -respondsToSelector: check on a method with at least >>> that availability, or checking whether “[NSFoo class]” is non-null when the >>> class has availability. I suspect that’s the idea behind Deploymate ( >>> http://www.deploymateapp.com), although I’ve never used it, and it has >>> the benefit that it should make idiomatic code (that does the right >>> checking) just work. >>> >>> It’s also a heck of a lot more work to implement than the approach >>> you’re using. >>> >> >> Right :-) >> >> >>> >>> Any comments on the implementation? >>> >>> >>> The implementation generally looks fine. One minor comment: >>> >>> + case AR_NotYetIntroduced: { >>> + // don't do this for enums, they can't be redeclared. >>> + if (isa<EnumConstantDecl>(D) || isa<EnumDecl>(D)) >>> + break; >>> + bool FoundRedecl = false; >>> + for (Decl *Redecl = D->getMostRecentDecl(); Redecl && >>> !FoundRedecl; >>> + Redecl = Redecl->getPreviousDecl()) { >>> + if (Redecl->getAttr<AvailabilityAttr>()->isInherited()) >>> + FoundRedecl = true; >>> + } >>> + if (!FoundRedecl) >>> + S.EmitAvailabilityWarning(Sema::AD_Partial, D, Message, Loc, >>> + UnknownObjCClass, ObjCPDecl, >>> + ObjCPropertyAccess); >>> + break; >>> + } >>> >>> Generally speaking, name lookup will always find the most recent >>> declaration, so you might be able to skip the D->getMostRecentDecl() bit >>> entirely and just check that the availability attribute was inherited. >>> >> >> That worked, done. >> >> I also had to add some explicit code for handling redeclarations in >> @interfaces, as these aren't modeled as redeclarations in the AST. I also >> added the property note used in the other availability warnings, and I >> added lots of tests. >> > > <clang-redecl.diff> > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
