Merge "Fixes for versioner guard generation"

tools/versioner/src/Preprocessor.cpp

@@ -98,7 +98,8 @@ static DeclarationAvailability calculateRequiredGuard(const Declaration& declara
   }
 
   for (Arch arch : supported_archs) {
-    if (result.arch_availability[arch].introduced <= arch_visibility[arch]) {
+    if (result.arch_availability[arch].introduced <= arch_visibility[arch] ||
+        result.arch_availability[arch].introduced <= arch_min_api[arch]) {
       result.arch_availability[arch].introduced = 0;
     }
   }
@@ -139,7 +140,6 @@ static std::string generateGuardCondition(const DeclarationAvailability& avail)
   // Logically orred expressions that constitute the macro guard.
   std::vector<std::string> expressions;
   static const std::vector<std::pair<std::string, std::set<Arch>>> arch_sets = {
-    { "", supported_archs },
       {"!defined(__LP64__)", {Arch::arm, Arch::x86}},
       {"defined(__LP64__)", {Arch::arm64, Arch::riscv64, Arch::x86_64}},
   };
@@ -168,6 +168,9 @@ static std::string generateGuardCondition(const DeclarationAvailability& avail)
     }
 
     if (avail.global_availability.introduced == 0) {
+      // We currently get here for the "__sF" symbol because it's marked __REMOVED_IN(23). This
+      // symbol is the only use of __REMOVED_IN, and it's already guarded manually, so there's no
+      // need to do anything.
       fprintf(stderr, "warning: attempted to generate guard with empty availability: %s\n",
               to_string(avail).c_str());
       return "";
@@ -186,25 +189,35 @@ static std::string generateGuardCondition(const DeclarationAvailability& avail)
 
     D("  Checking arch set '%s'\n", arch_expr.c_str());
 
-    int version = avail.arch_availability[*it.second.begin()].introduced;
+    int version = 0;
 
-    // The maximum min_version of the set.
+    // Find the architectures that need to check __ANDROID_API__ and verify that they check against
-    int max_min_version = 0;
+    // the same API level.
     for (Arch arch : archs) {
-      if (arch_min_api[arch] > max_min_version) {
+      const int arch_version = avail.arch_availability[arch].introduced;
-        max_min_version = arch_min_api[arch];
+      if (arch_version == 0) {
-      }
+        continue;
-
+      } else if (version == 0) {
-      if (avail.arch_availability[arch].introduced != version) {
+        version = arch_version;
+      } else if (version != arch_version) {
         D("    Skipping arch set, availability for %s doesn't match %s\n",
           to_string(*it.second.begin()).c_str(), to_string(arch).c_str());
         goto skip;
       }
     }
 
-    // If all of the archs in the set have a min_api that satifies version, elide the check.
+    // Verify that a non-zero version is acceptable to reuse for other archs with a higher minimum
-    if (max_min_version >= version) {
+    // API, like riscv64. (e.g. It's OK to reuse an (__ANDROID_API__ >= 24) check if the arch's
-      version = 0;
+    // minimum API is 35.)
+    if (version != 0) {
+      for (Arch arch : archs) {
+        const int arch_version = avail.arch_availability[arch].introduced;
+        if (arch_version == 0 && version > arch_min_api[arch]) {
+          D("    Skipping arch set, availability for %s doesn't match %s\n",
+            to_string(*it.second.begin()).c_str(), to_string(arch).c_str());
+          goto skip;
+        }
+      }
     }
 
     expressions.emplace_back(generate_guard(arch_expr, version));
@@ -222,12 +235,8 @@ static std::string generateGuardCondition(const DeclarationAvailability& avail)
   for (const auto& it : individual_archs) {
     const std::string& arch_expr = it.second;
     int introduced = avail.arch_availability[it.first].introduced;
-    if (introduced == 0) {
-      expressions.emplace_back(arch_expr);
-    } else {
     expressions.emplace_back(generate_guard(arch_expr, introduced));
   }
-  }
 
   if (expressions.size() == 0) {
     errx(1, "generated empty guard for availability %s", to_string(avail).c_str());

tools/versioner/tests/preprocessor/expected/foo.h

@@ -65,6 +65,14 @@ int multiple_introduced_2() __INTRODUCED_IN_ARM(13) __INTRODUCED_IN_X86(13) __IN
 #endif /* (!defined(__LP64__) && __ANDROID_API__ >= 13) || (defined(__LP64__) && __ANDROID_API__ >= 22) */
 
 
+// This produces both an LP64 and a not-LP64 check, but it doesn't need to check for all 64-bit
+// targets separately.
+
+#if (!defined(__LP64__) && __ANDROID_API__ >= 23) || (defined(__LP64__) && __ANDROID_API__ >= 23)
+int multiple_introduced_3() __INTRODUCED_IN_32(23) __INTRODUCED_IN_64(23);
+#endif /* (!defined(__LP64__) && __ANDROID_API__ >= 23) || (defined(__LP64__) && __ANDROID_API__ >= 23) */
+
+
 
 #if (!defined(__LP64__) && __ANDROID_API__ >= 12) || (defined(__LP64__))
 int group_lp32() __INTRODUCED_IN_ARM(12) __INTRODUCED_IN_X86(12);

tools/versioner/tests/preprocessor/headers/foo.h

@@ -41,6 +41,10 @@ int multiple_introduced_1() __INTRODUCED_IN_ARM(13) __INTRODUCED_IN_X86(13) __IN
 
 int multiple_introduced_2() __INTRODUCED_IN_ARM(13) __INTRODUCED_IN_X86(13) __INTRODUCED_IN_64(22);
 
+// This produces both an LP64 and a not-LP64 check, but it doesn't need to check for all 64-bit
+// targets separately.
+int multiple_introduced_3() __INTRODUCED_IN_32(23) __INTRODUCED_IN_64(23);
+
 int group_lp32() __INTRODUCED_IN_ARM(12) __INTRODUCED_IN_X86(12);
 
 #if defined(__cplusplus)

tools/versioner/tests/preprocessor/run.sh

@@ -4,7 +4,7 @@ function run_test {
   SRC=$1
   DST=$2
   rm -rf $2
-  versioner -a 9 -a 12 -a 13 -a 14 -a 15 $1 -i -o $2
+  versioner -a 9 -a 12 -a 13 -a 14 -a 15 -a 21 -a 23 $1 -i -o $2
   diff -q -w -B $2 expected
 }