Use -Werror in CI only (#201)

We keep getting compat reports for warnings in various compiler
versions. While we can keep merging PRs to resolve these warnings, it
would be nice if the users of other compilers or compiler versions weren't
blocked on us fixing this.

As such, this change disables Werror by default and only enables it when
requested, which happens in CI in test builds.

.github/workflows/build.yml

@@ -27,13 +27,13 @@ jobs:
     - uses: actions/checkout@v1
     - name: make test
       run: |
-        make -j2 config=sanitize test
+        make -j2 config=sanitize werror=1 test
     - name: make test w/flags
       run: |
-        make -j2 config=sanitize flags=true test
+        make -j2 config=sanitize werror=1 flags=true test
     - name: make cli
       run: |
-         make -j2 config=sanitize luau luau-analyze # match config with tests to improve build time
+         make -j2 config=sanitize werror=1 luau luau-analyze # match config with tests to improve build time
          ./luau tests/conformance/assert.lua
          ./luau-analyze tests/conformance/assert.lua
 
@@ -45,7 +45,7 @@ jobs:
     steps:
     - uses: actions/checkout@v1
     - name: cmake configure
-      run: cmake . -A ${{matrix.arch}}
+      run: cmake . -A ${{matrix.arch}} -DLUAU_WERROR=ON
     - name: cmake test
       shell: bash # necessary for fail-fast
       run: |

Analysis/src/TypeInfer.cpp

@@ -1509,7 +1509,7 @@ ExprResult<TypeId> TypeChecker::checkExpr(const ScopePtr& scope, const AstExprVa
         std::vector<TypeId> types = flatten(varargPack).first;
         return {!types.empty() ? types[0] : nilType};
     }
-    else if (auto ftp = get<FreeTypePack>(varargPack))
+    else if (get<FreeTypePack>(varargPack))
     {
         TypeId head = freshType(scope);
         TypePackId tail = freshTypePack(scope);
@@ -1539,7 +1539,7 @@ ExprResult<TypeId> TypeChecker::checkExpr(const ScopePtr& scope, const AstExprCa
     {
         return {pack->head.empty() ? nilType : pack->head[0], std::move(result.predicates)};
     }
-    else if (auto ftp = get<Unifiable::Free>(retPack))
+    else if (get<Unifiable::Free>(retPack))
     {
         TypeId head = freshType(scope);
         TypePackId pack = addTypePack(TypePackVar{TypePack{{head}, freshTypePack(scope)}});

Analysis/src/TypeVar.cpp

@@ -293,7 +293,7 @@ bool isGeneric(TypeId ty)
 bool maybeGeneric(TypeId ty)
 {
     ty = follow(ty);
-    if (auto ftv = get<FreeTypeVar>(ty))
+    if (get<FreeTypeVar>(ty))
         return true;
     else if (auto ttv = get<TableTypeVar>(ty))
     {

CMakeLists.txt

@@ -9,6 +9,7 @@ project(Luau LANGUAGES CXX)
 
 option(LUAU_BUILD_CLI "Build CLI" ON)
 option(LUAU_BUILD_TESTS "Build tests" ON)
+option(LUAU_WERROR "Warnings as errors" OFF)
 
 add_library(Luau.Ast STATIC)
 add_library(Luau.Compiler STATIC)
@@ -57,11 +58,18 @@ set(LUAU_OPTIONS)
 
 if(MSVC)
     list(APPEND LUAU_OPTIONS /D_CRT_SECURE_NO_WARNINGS) # We need to use the portable CRT functions.
-    list(APPEND LUAU_OPTIONS /WX) # Warnings are errors
     list(APPEND LUAU_OPTIONS /MP) # Distribute single project compilation across multiple cores
 else()
     list(APPEND LUAU_OPTIONS -Wall) # All warnings
-    list(APPEND LUAU_OPTIONS -Werror) # Warnings are errors
+endif()
+
+# Enabled in CI; we should be warning free on our main compiler versions but don't guarantee being warning free everywhere
+if(LUAU_WERROR)
+    if(MSVC)
+        list(APPEND LUAU_OPTIONS /WX) # Warnings are errors
+    else()
+        list(APPEND LUAU_OPTIONS -Werror) # Warnings are errors
+    endif()
 endif()
 
 target_compile_options(Luau.Ast PRIVATE ${LUAU_OPTIONS})

Makefile

@@ -46,14 +46,20 @@ endif
 OBJECTS=$(AST_OBJECTS) $(COMPILER_OBJECTS) $(ANALYSIS_OBJECTS) $(VM_OBJECTS) $(TESTS_OBJECTS) $(CLI_OBJECTS) $(FUZZ_OBJECTS)
 
 # common flags
-CXXFLAGS=-g -Wall -Werror
+CXXFLAGS=-g -Wall
 LDFLAGS=
 
-# temporary, for older gcc versions as they treat var in `if (type var = val)` as unused
+# some gcc versions treat var in `if (type var = val)` as unused
+# some gcc versions treat variables used in constexpr if blocks as unused
 ifeq ($(findstring g++,$(shell $(CXX) --version)),g++)
 	CXXFLAGS+=-Wno-unused
 endif
 
+# enabled in CI; we should be warning free on our main compiler versions but don't guarantee being warning free everywhere
+ifneq ($(werror),)
+	CXXFLAGS+=-Werror
+endif
+
 # configuration-specific flags
 ifeq ($(config),release)
 	CXXFLAGS+=-O2 -DNDEBUG