[clang] [HLSL] Strict Availability Diagnostics (PR #93860)

Justin Bogner via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 11 09:15:38 PDT 2024


================
@@ -834,34 +841,41 @@ void DiagnoseUnguardedAvailability::DiagnoseDeclAvailability(
                                              OffendingDecl))
       return;
 
-    // We would like to emit the diagnostic even if -Wunguarded-availability is
-    // not specified for deployment targets >= to iOS 11 or equivalent or
-    // for declarations that were introduced in iOS 11 (macOS 10.13, ...) or
-    // later.
-    bool UseNewDiagKind = shouldDiagnoseAvailabilityByDefault(
-        SemaRef.Context,
-        SemaRef.Context.getTargetInfo().getPlatformMinVersion(), Introduced);
-
     const TargetInfo &TI = SemaRef.getASTContext().getTargetInfo();
     std::string PlatformName(
         AvailabilityAttr::getPrettyPlatformName(TI.getPlatformName()));
-    llvm::StringRef TargetEnvironment(AvailabilityAttr::getPrettyEnviromentName(
-        TI.getTriple().getEnvironment()));
+    llvm::StringRef TargetEnvironment(TI.getTriple().getEnvironmentName());
     llvm::StringRef AttrEnvironment =
-        AA->getEnvironment() ? AvailabilityAttr::getPrettyEnviromentName(
-                                   AvailabilityAttr::getEnvironmentType(
-                                       AA->getEnvironment()->getName()))
-                             : "";
+        AA->getEnvironment() ? AA->getEnvironment()->getName() : "";
     bool UseEnvironment =
         (!AttrEnvironment.empty() && !TargetEnvironment.empty());
 
-    unsigned DiagKind =
-        EnvironmentMatchesOrNone
-            ? (UseNewDiagKind ? diag::warn_unguarded_availability_new
-                              : diag::warn_unguarded_availability)
-            : (UseNewDiagKind
-                   ? diag::warn_unguarded_availability_unavailable_new
-                   : diag::warn_unguarded_availability_unavailable);
+    unsigned DiagKind;
+    if (SemaRef.getLangOpts().HLSL) {
+      // For HLSL, use diagnostic from HLSLAvailability group which
+      // are reported as errors in default and in strict diagnostic mode
+      // (-fhlsl-strict-availability) and as warnings in relaxed diagnostic
+      // mode (-Wno-error=hlsl-availability)
+      DiagKind = EnvironmentMatchesOrNone
+                     ? diag::warn_hlsl_availability
+                     : diag::warn_hlsl_availability_unavailable;
+
+    } else {
+      // For iOS, emit the diagnostic even if -Wunguarded-availability is
+      // not specified for deployment targets >= to iOS 11 or equivalent or
+      // for declarations that were introduced in iOS 11 (macOS 10.13, ...) or
+      // later.
+      bool UseNewDiagKind = shouldDiagnoseAvailabilityByDefault(
+          SemaRef.Context,
+          SemaRef.Context.getTargetInfo().getPlatformMinVersion(), Introduced);
+
+      DiagKind = EnvironmentMatchesOrNone
+                     ? (UseNewDiagKind ? diag::warn_unguarded_availability_new
+                                       : diag::warn_unguarded_availability)
+                     : (UseNewDiagKind
+                            ? diag::warn_unguarded_availability_unavailable_new
+                            : diag::warn_unguarded_availability_unavailable);
+    }
----------------
bogner wrote:

The name `shouldDiagnoseAvailabilityByDefault` is kind of confusing since it's only actually used to determine which availability diagnostic group to use. Should we rename it `getAvailabilityDiagnosticKind` and have it simply return which diagnostic is appropriate? It could do something like the following:
```c++
static unsigned getAvailabilityDiagnosticKind(
    const ASTContext &Context, const VersionTuple &DeploymentVersion,
    const VersionTuple &DeclVersion, bool HasMatchingEnv) {
  const auto &Triple = Context.getTargetInfo().getTriple();
  if (Triple.getOS() == llvm::Triple::ShaderModel)
    return HasMatchingEnv ? diag::warn_hlsl_availability
                          : diag::warn_hlsl_availability_unavailable;

  VersionTuple ForceAvailabilityFromVersion;
  switch (Triple.getOS()) {
  case llvm::Triple::IOS:
  case llvm::Triple::TvOS:
    ForceAvailabilityFromVersion = VersionTuple(/*Major=*/11);
    break;
  case llvm::Triple::WatchOS:
    ForceAvailabilityFromVersion = VersionTuple(/*Major=*/4);
    break;
  case llvm::Triple::Darwin:
  case llvm::Triple::MacOSX:
    ForceAvailabilityFromVersion = VersionTuple(/*Major=*/10, /*Minor=*/13);
    break;
  default:
    // New apple targets should always warn about availability.
    return Triple.getVendor() == llvm::Triple::Apple;
  }
  if (DeploymentVersion >= ForceAvailabilityFromVersion ||
      DeclVersion >= ForceAvailabilityFromVersion)
    return HasMatchingEnv ? diag::warn_unguarded_availability_new
                          : diag::warn_unguarded_availability_unavailable_new;
  return HasMatchingEnv ? diag::warn_unguarded_availability
                        : diag::warn_unguarded_availability_unavailable;
}
```

Related, I guess the only reason we have three separate diagnostics here is to handle different `-W` flags? I think this is fine for now but we should probably consider what this could look like in the future if more platforms start using availability and consider consolidating them somehow.

https://github.com/llvm/llvm-project/pull/93860


More information about the cfe-commits mailing list