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

[Haowei Wu via Phabricator via cfe-commits]

haowei marked an inline comment as done.
haowei added inline comments.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:893
       Builder.defineMacro("_GNU_SOURCE");
+    Builder.defineMacro("FUCHSIA_API_LEVEL", Twine(Opts.FuchsiaAPILevel));
+    this->PlatformName = "fuchsia";
----------------
phosek wrote:
> aaron.ballman wrote:
> > I think this macro should be using a reserved name, as suggested by @phosek.
> @aaron.ballman Do you have any thoughts on the letter case? I think that `__Fuchsia_API_level__` might be a better match for `__Fuchsia__`. Upper case is perhaps more usual, but other platforms like *BSD also use mixed case.
I changed it to "__Fuchsia_API_level__" for now.


================
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
+//
----------------
aaron.ballman wrote:
> Can you also add a test that shows how we handle `-ffuchsia-api-level=16.0.0`?
I did. But it does not fail. I will look into it to see if I can add the same constraint like the attributes tomorrow.


================
Comment at: clang/test/Frontend/attr-availability-fuchsia.c:4
+//
+// It should also be exposed to non-fuchsia platforms. This is desireable when
+// using common Fuchsia headers for building host libraries that also depend on
----------------
phosek wrote:
> Would this also set the availability attribute? Can we test it? If not then this isn't particularly useful.
Do you mean the macros? I can add one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108592