Re: r307175 - [Sema] Don't allow -Wunguarded-availability to be silenced with redecls

2017-07-06 Thread Nico Weber via cfe-commits
Thanks for the fast reply!

On Jul 6, 2017 11:42 PM, "Erik Pilkington" 
wrote:



On 7/6/17 2:12 PM, Nico Weber wrote:

We currently rely on this for chromium; that's how the warning used to work
when I added it. What's the transition plan? Can we have a flag to
incrementally transition to whatever the new way is? (Is it documented
anywhere?)

We currently have -Wunguarded-availability-new, which only warns for things
introduced as of macOS 10.13. If you switch to that flag then you won't get
warnings for code that used the redecl thing before 10.13,


Do we still get warnings for non-redecl'd older things then? We've been
using the redecl thing since 10.7 or so, and we rely on these warnings.

anything that uses APIs newer than that should either use @available or
annotate the context declaration with an availability attribute. This is
"documented" at the start of this wwdc talk: https://developer.apple.com/
videos/play/wwdc2017/411/

-Wunguarded-availability-new is somewhat unfortunate in that it won't catch
new code that you write today that uses older APIs, but maybe you can use
it and slowly change the redecl thing you have to use
@available/availability attributes then turn -Wunguarded-availability back
on when that is done.


Ah, that answers the above question.


Also, I think the replacement somehow needs the new runtime stuff from
libbuiltin -- does the driver know when to add that to the link line? If
so, where's the logic for that? If not, what library is supposed to provide
the runtime check function?

The runtime component (__isOSVersionAtLeast) is in compiler-rt/builtins.
Are you having problems linking with it?


I'm traveling and without a real computer, but iirc we tried using the new
thing and it got us a linker error. We don't currently bundle libBuiltin,
but -### didn't show clang trying to link it in and we failed to find
driver code that would add it. (We didn't look very hard though.) We were
also surprised that nothing else so far seemed to need libBuiltin.

It sounds like a transition path could be:

1. Bundle libBuiltin at older clang rev
2. Move stuff to @available
3. Move clang past this revision

Does that sound right?



Erik


On Jul 5, 2017 7:09 PM, "Erik Pilkington via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

Author: epilk
Date: Wed Jul  5 10:08:56 2017
New Revision: 307175

URL: http://llvm.org/viewvc/llvm-project?rev=307175=rev
Log:
[Sema] Don't allow -Wunguarded-availability to be silenced with redecls

Differential revision: https://reviews.llvm.org/D33816

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/attr-availability.c
cfe/trunk/test/Sema/attr-deprecated.c
cfe/trunk/test/Sema/attr-unavailable-message.c
cfe/trunk/test/SemaCXX/attr-deprecated.cpp
cfe/trunk/test/SemaObjC/attr-availability.m
cfe/trunk/test/SemaObjC/unguarded-availability-new.m
cfe/trunk/test/SemaObjC/unguarded-availability.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
Basic/DiagnosticSemaKinds.td?rev=307175=307174=307175=diff

==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul  5
10:08:56 2017
@@ -2880,7 +2880,7 @@ def warn_partial_availability : Warning<
 def warn_partial_availability_new : Warning,
   InGroup;
 def note_partial_availability_silence : Note<
-  "explicitly redeclare %0 to silence this warning">;
+  "annotate %select{%1|anonymous %1}0 with an availability attribute to
silence">;
 def note_unguarded_available_silence : Note<
   "enclose %0 in %select{an @available|a __builtin_available}1 check to
silence"
   " this warning">;

Modified: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
Sema/DelayedDiagnostic.h?rev=307175=307174=307175=diff

==
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h (original)
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h Wed Jul  5 10:08:56
2017
@@ -124,7 +124,8 @@ public:

   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
 SourceLocation Loc,
-const NamedDecl *D,
+const NamedDecl *ReferringDecl,
+const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl
*UnknownObjCClass,
 const 

Re: r307175 - [Sema] Don't allow -Wunguarded-availability to be silenced with redecls

2017-07-06 Thread Erik Pilkington via cfe-commits



On 7/6/17 2:12 PM, Nico Weber wrote:
We currently rely on this for chromium; that's how the warning used to 
work when I added it. What's the transition plan? Can we have a flag 
to incrementally transition to whatever the new way is? (Is it 
documented anywhere?)
We currently have -Wunguarded-availability-new, which only warns for 
things introduced as of macOS 10.13. If you switch to that flag then you 
won't get warnings for code that used the redecl thing before 10.13, 
anything that uses APIs newer than that should either use @available or 
annotate the context declaration with an availability attribute. This is 
"documented" at the start of this wwdc talk: 
https://developer.apple.com/videos/play/wwdc2017/411/


-Wunguarded-availability-new is somewhat unfortunate in that it won't 
catch new code that you write today that uses older APIs, but maybe you 
can use it and slowly change the redecl thing you have to use 
@available/availability attributes then turn -Wunguarded-availability 
back on when that is done.
Also, I think the replacement somehow needs the new runtime stuff from 
libbuiltin -- does the driver know when to add that to the link line? 
If so, where's the logic for that? If not, what library is supposed to 
provide the runtime check function?
The runtime component (__isOSVersionAtLeast) is in compiler-rt/builtins. 
Are you having problems linking with it?


Erik


On Jul 5, 2017 7:09 PM, "Erik Pilkington via cfe-commits" 
> wrote:


Author: epilk
Date: Wed Jul  5 10:08:56 2017
New Revision: 307175

URL: http://llvm.org/viewvc/llvm-project?rev=307175=rev

Log:
[Sema] Don't allow -Wunguarded-availability to be silenced with
redecls

Differential revision: https://reviews.llvm.org/D33816


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/attr-availability.c
cfe/trunk/test/Sema/attr-deprecated.c
cfe/trunk/test/Sema/attr-unavailable-message.c
cfe/trunk/test/SemaCXX/attr-deprecated.cpp
cfe/trunk/test/SemaObjC/attr-availability.m
cfe/trunk/test/SemaObjC/unguarded-availability-new.m
cfe/trunk/test/SemaObjC/unguarded-availability.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=307175=307174=307175=diff



==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul 
5 10:08:56 2017

@@ -2880,7 +2880,7 @@ def warn_partial_availability : Warning<
 def warn_partial_availability_new :
Warning,
   InGroup;
 def note_partial_availability_silence : Note<
-  "explicitly redeclare %0 to silence this warning">;
+  "annotate %select{%1|anonymous %1}0 with an availability
attribute to silence">;
 def note_unguarded_available_silence : Note<
   "enclose %0 in %select{an @available|a __builtin_available}1
check to silence"
   " this warning">;

Modified: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DelayedDiagnostic.h?rev=307175=307174=307175=diff



==
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h (original)
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h Wed Jul  5
10:08:56 2017
@@ -124,7 +124,8 @@ public:

   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
 SourceLocation Loc,
-const NamedDecl *D,
+const NamedDecl
*ReferringDecl,
+const NamedDecl
*OffendingDecl,
 const
ObjCInterfaceDecl *UnknownObjCClass,
 const
ObjCPropertyDecl  *ObjCProperty,
 StringRef Msg,
@@ -164,9 +165,13 @@ public:
 return *reinterpret_cast(AccessData);
   }

- 

Re: r307175 - [Sema] Don't allow -Wunguarded-availability to be silenced with redecls

2017-07-06 Thread Nico Weber via cfe-commits
We currently rely on this for chromium; that's how the warning used to work
when I added it. What's the transition plan? Can we have a flag to
incrementally transition to whatever the new way is? (Is it documented
anywhere?)

Also, I think the replacement somehow needs the new runtime stuff from
libbuiltin -- does the driver know when to add that to the link line? If
so, where's the logic for that? If not, what library is supposed to provide
the runtime check function?

On Jul 5, 2017 7:09 PM, "Erik Pilkington via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

Author: epilk
Date: Wed Jul  5 10:08:56 2017
New Revision: 307175

URL: http://llvm.org/viewvc/llvm-project?rev=307175=rev
Log:
[Sema] Don't allow -Wunguarded-availability to be silenced with redecls

Differential revision: https://reviews.llvm.org/D33816

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/attr-availability.c
cfe/trunk/test/Sema/attr-deprecated.c
cfe/trunk/test/Sema/attr-unavailable-message.c
cfe/trunk/test/SemaCXX/attr-deprecated.cpp
cfe/trunk/test/SemaObjC/attr-availability.m
cfe/trunk/test/SemaObjC/unguarded-availability-new.m
cfe/trunk/test/SemaObjC/unguarded-availability.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
DiagnosticSemaKinds.td?rev=307175=307174=307175=diff

==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul  5
10:08:56 2017
@@ -2880,7 +2880,7 @@ def warn_partial_availability : Warning<
 def warn_partial_availability_new : Warning,
   InGroup;
 def note_partial_availability_silence : Note<
-  "explicitly redeclare %0 to silence this warning">;
+  "annotate %select{%1|anonymous %1}0 with an availability attribute to
silence">;
 def note_unguarded_available_silence : Note<
   "enclose %0 in %select{an @available|a __builtin_available}1 check to
silence"
   " this warning">;

Modified: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
clang/Sema/DelayedDiagnostic.h?rev=307175=307174=307175=diff

==
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h (original)
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h Wed Jul  5 10:08:56
2017
@@ -124,7 +124,8 @@ public:

   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
 SourceLocation Loc,
-const NamedDecl *D,
+const NamedDecl *ReferringDecl,
+const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl
*UnknownObjCClass,
 const ObjCPropertyDecl
*ObjCProperty,
 StringRef Msg,
@@ -164,9 +165,13 @@ public:
 return *reinterpret_cast(AccessData);
   }

-  const NamedDecl *getAvailabilityDecl() const {
+  const NamedDecl *getAvailabilityReferringDecl() const {
 assert(Kind == Availability && "Not an availability diagnostic.");
-return AvailabilityData.Decl;
+return AvailabilityData.ReferringDecl;
+  }
+
+  const NamedDecl *getAvailabilityOffendingDecl() const {
+return AvailabilityData.OffendingDecl;
   }

   StringRef getAvailabilityMessage() const {
@@ -213,7 +218,8 @@ public:
 private:

   struct AD {
-const NamedDecl *Decl;
+const NamedDecl *ReferringDecl;
+const NamedDecl *OffendingDecl;
 const ObjCInterfaceDecl *UnknownObjCClass;
 const ObjCPropertyDecl  *ObjCProperty;
 const char *Message;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
clang/Sema/Sema.h?rev=307175=307174=307175=diff

==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Jul  5 10:08:56 2017
@@ -3881,7 +3881,9 @@ public:

   void redelayDiagnostics(sema::DelayedDiagnosticPool );

-  void EmitAvailabilityWarning(AvailabilityResult AR, NamedDecl *D,
+  void EmitAvailabilityWarning(AvailabilityResult AR,
+   const NamedDecl *ReferringDecl,
+   const NamedDecl *OffendingDecl,
StringRef Message, SourceLocation Loc,
const ObjCInterfaceDecl *UnknownObjCClass,
const