[PATCH] D110489: Make windows resource generation more robust to misconfiguration.

Stella Stamenova via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 27 13:16:54 PDT 2021


stella.stamenova added a comment.

LGTM. I find it easier to read if it's all done together (see comment), but it's fine either way.



================
Comment at: llvm/cmake/modules/AddLLVM.cmake:386
+  # blank causes the resource compiler to fail with obscure errors.
+  if (NOT ARG_VERSION_MAJOR)
+    set(ARG_VERSION_MAJOR 0)
----------------
Since the issue you saw was when LLVM_VERSION* was not defined, I think it would be a bit cleaner if you made all the assignments together e.g.:

```
if (NOT DEFINED ARG_VERSION_MAJOR)
  if (${LLVM_VERSION_MAJOR})
    set(ARG_VERSION_MAJOR ${LLVM_VERSION_MAJOR})
  else
    set(ARG_VERSION_MAJOR 0)
  endif
endif
```

It's not any less verbose, but it's a bit more explicit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110489



More information about the llvm-commits mailing list