[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