[PATCH] D108592: [clang][Fuchsia] Support __attribute__((availability)) on Fuchsia

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3557
+def warn_availability_fuchsia_unavailable_minor : Warning<
+  "Fuchsia API Level only support 'major', not '.minor[.subminor]'">,
+  InGroup<Availability>;
----------------



================
Comment at: clang/lib/Basic/Targets/OSTargets.h:893
       Builder.defineMacro("_GNU_SOURCE");
+    Builder.defineMacro("FUCHSIA_API_LEVEL", Twine(Opts.FuchsiaAPILevel));
+    this->PlatformName = "fuchsia";
----------------
I think this macro should be using a reserved name, as suggested by @phosek.


================
Comment at: clang/test/Frontend/attr-availability-fuchsia.c:2
+// Test that `-mfuchsia-version` is propagated to cc1.
+// RUN: %clang -target x86_64-unknown-fuchsia -ffuchsia-api-level=16 -c %s -### 2>&1| FileCheck %s
+//
----------------
Can you also add a test that shows how we handle `-ffuchsia-api-level=16.0.0`?


================
Comment at: clang/test/Frontend/attr-availability-fuchsia.c:2
+// Test that `-mfuchsia-version` is propagated to cc1.
+// RUN: %clang -target x86_64-unknown-fuchsia -mfuchsia-version=16 -c %s -### |& FileCheck %s
+//
----------------
aaron.ballman wrote:
> Is `|&` intentional? I think that's causing shell parse errors on Windows (same for the other test).
> 
> Also should this test be in `Driver` instead as it's testing the driver functionality?
This test should be in `Driver`, right?


================
Comment at: clang/test/Sema/attr-availability-fuchsia.c:14
+void f5(int) __attribute__((availability(ios, introduced = 3.2), availability(fuchsia, unavailable)));                                                       // expected-note{{'f5' has been explicitly marked unavailable here}}
+void f6(int) __attribute__((availability(fuchsia, introduced = 16.0)));                                                                                      // expected-warning {{Fuchsia API Level only support 'major', not '.minor[.subminor]}}
+void f7(int) __attribute__((availability(fuchsia, introduced = 16.1))); // expected-warning {{Fuchsia API Level only support 'major', not '.minor[.subminor]}}
----------------
FWIW, this surprises me. I would have expected that `16`, `16.0`, and `16.0.0` were all equivalent and the issue was with specifying a nonzero value for those. I've suggested new wording for the diagnostic that might make it more clear.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108592/new/

https://reviews.llvm.org/D108592