[llvm] [cmake] Fix the dead VSINSTALLDIR check in FindDIASDK (PR #217071)

Larry Meadows via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 19 03:01:51 PDT 2026


================
@@ -18,15 +20,20 @@ if(NOT WIN32)
   return()
 endif()
 
+set(DIASDK_LOCATION_INFERRED FALSE)
+
 if(LLVM_WINSYSROOT)
   set(MSVC_DIA_SDK_DIR "${LLVM_WINSYSROOT}/DIA SDK" CACHE PATH
       "Path to the DIA SDK")
-elseif($ENV{VSINSTALLDIR})
-  set(MSVC_DIA_SDK_DIR "$ENV{VSINSTALLDIR}DIA SDK" CACHE PATH
-      "Path to the DIA SDK")
 elseif(NOT DEFINED MSVC_DIA_SDK_DIR)
-  message(STATUS "MSVC_DIA_SDK_DIR not set, and could not be inferred. DIA SDK "
-                 "may not be found.")
+  if(DEFINED ENV{VSINSTALLDIR})
+    set(MSVC_DIA_SDK_DIR "$ENV{VSINSTALLDIR}DIA SDK" CACHE PATH
+        "Path to the DIA SDK")
+    set(DIASDK_LOCATION_INFERRED TRUE)
----------------
lfmeadow wrote:

The staleness is real, thanks — the consequence is a step off, though.

`option()` leaves an existing cache entry alone, so on a plain reconfigure `LLVM_ENABLE_DIA_SDK` keeps the `OFF` it got on the first configure even after the computed default flips. Running the module against a stubbed SDK tree with only `VSINSTALLDIR` set:

```
configure 1   found=TRUE  inferred=TRUE   computed-default=OFF  option=OFF
configure 2   found=TRUE  inferred=FALSE  computed-default=ON   option=OFF
```

So DIA does not come back on for the bots. But `inferred=FALSE` on the second line is exactly the defect you describe, and it is reachable: in a build directory where `LLVM_ENABLE_DIA_SDK` is absent from the cache while `MSVC_DIA_SDK_DIR` is present — a configure that failed after `find_package(DIASDK)`, or a hand-edited cache — the option comes out `ON` for a location nobody chose.

Fixed by caching the flag as `INTERNAL` next to the path it describes, so it survives the reconfigures where the branches that set it no longer run:

| configuration | inferred | option |
| --- | --- | --- |
| explicit `MSVC_DIA_SDK_DIR` | FALSE | ON |
| `LLVM_WINSYSROOT` | FALSE | ON |
| only `VSINSTALLDIR` | TRUE | OFF |
| reconfigure of the above | TRUE | OFF |
| same, with the option dropped from the cache | TRUE | OFF |
| nothing set | FALSE | OFF |


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


More information about the llvm-commits mailing list