Make windows resource generation more robust to misconfiguration.

[stellaraccident]

Summary

I was experiencing this in an out-of-tree project when building a DLL in a context that didn't have LLVM_VERSION_* and friends set. It causes obscure errors in the resource compiler if not integers.

[stella.stamenova]

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

llvm/cmake/modules/AddLLVM.cmake

386

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.

[stellaraccident]

Another contributor cloned and submitted this with suggested fixes as a new patch: https://reviews.llvm.org/D135650

Closing this one.

llvm/cmake/modules/AddLLVM.cmake

 #   )
 function(set_windows_version_resource_properties name resource_file)
   cmake_parse_arguments(ARG
     ""
     "VERSION_MAJOR;VERSION_MINOR;VERSION_PATCHLEVEL;VERSION_STRING;PRODUCT_NAME"
     ""
     ${ARGN})
 
+  # Set from arguments.
   if (NOT DEFINED ARG_VERSION_MAJOR)
     set(ARG_VERSION_MAJOR ${LLVM_VERSION_MAJOR})
   endif()
-
   if (NOT DEFINED ARG_VERSION_MINOR)
     set(ARG_VERSION_MINOR ${LLVM_VERSION_MINOR})
   endif()
-
   if (NOT DEFINED ARG_VERSION_PATCHLEVEL)
     set(ARG_VERSION_PATCHLEVEL ${LLVM_VERSION_PATCH})
   endif()
-
   if (NOT DEFINED ARG_VERSION_STRING)
     set(ARG_VERSION_STRING ${PACKAGE_VERSION})
   endif()
-
   if (NOT DEFINED ARG_PRODUCT_NAME)
     set(ARG_PRODUCT_NAME "LLVM")
   endif()
 
+  # Make sure to default integer fields in the case that version information
+  # is not defaulted above (can happen in out of tree projects). Leaving these
+  # blank causes the resource compiler to fail with obscure errors.
+  if (NOT ARG_VERSION_MAJOR)
+    set(ARG_VERSION_MAJOR 0)
+  endif()
+  if (NOT ARG_VERSION_MINOR)
+    set(ARG_VERSION_MINOR 0)
+  endif()
+  if (NOT ARG_VERSION_PATCHLEVEL)
+    set(ARG_VERSION_PATCHLEVEL 0)
+  endif()
+  if (NOT ARG_VERSION_STRING)
+    set(ARG_VERSION_STRING 0)
+  endif()
+
   set_property(SOURCE ${resource_file}
                PROPERTY COMPILE_FLAGS /nologo)
   set_property(SOURCE ${resource_file}
                PROPERTY COMPILE_DEFINITIONS
                "RC_VERSION_FIELD_1=${ARG_VERSION_MAJOR}"
                "RC_VERSION_FIELD_2=${ARG_VERSION_MINOR}"
                "RC_VERSION_FIELD_3=${ARG_VERSION_PATCHLEVEL}"
                "RC_VERSION_FIELD_4=0"