From b7d26b371a31795d6c4e2e164fce7375c764bcc3 Mon Sep 17 00:00:00 2001
From: Arseny Kapoulkine <arseny.kapoulkine@gmail.com>
Date: Fri, 12 Nov 2021 06:56:25 -0800
Subject: [PATCH] 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 |  8 ++++----
 Analysis/src/TypeInfer.cpp  |  4 ++--
 Analysis/src/TypeVar.cpp    |  2 +-
 CMakeLists.txt              | 12 ++++++++++--
 Makefile                    | 10 ++++++++--
 5 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index cdee2f6..506a4c8 100644
--- a/.github/workflows/build.yml
+++ b/.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: |
diff --git a/Analysis/src/TypeInfer.cpp b/Analysis/src/TypeInfer.cpp
index a6696ef..96b7992 100644
--- a/Analysis/src/TypeInfer.cpp
+++ b/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)}});
diff --git a/Analysis/src/TypeVar.cpp b/Analysis/src/TypeVar.cpp
index cd447ca..4ee5cb8 100644
--- a/Analysis/src/TypeVar.cpp
+++ b/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))
     {
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 36014a9..d9ef9c0 100644
--- a/CMakeLists.txt
+++ b/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})
diff --git a/Makefile b/Makefile
index ffe4309..ce461af 100644
--- a/Makefile
+++ b/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