> 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]
> <mailto:[email protected]>> wrote:
> On Mon, Feb 2, 2015 at 10:26 PM, Douglas Gregor <[email protected]
> <mailto:[email protected]>> wrote:
> Hi Nico,
>
>> On Jan 8, 2015, at 6:14 PM, Nico Weber <[email protected]
>> <mailto:[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
> <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
