Merge pull request #2358 from DataDog/ivoanjo/prof-6288-ci-warnings-i…

…nto-errors

[PROF-6288] Enable `-Werror` to turn compiler warnings into errors in CI

ext/ddtrace_profiling_loader/ddtrace_profiling_loader.c

@@ -29,10 +29,17 @@
   #define RTLD_DEEPBIND 0
 #endif
 
+// Used to mark function arguments that are deliberately left unused
+#ifdef __GNUC__
+  #define DDTRACE_UNUSED  __attribute__((unused))
+#else
+  #define DDTRACE_UNUSED
+#endif
+
 static VALUE ok_symbol = Qnil; // :ok in Ruby
 static VALUE error_symbol = Qnil; // :error in Ruby
 
-static VALUE _native_load(VALUE self, VALUE ruby_path, VALUE ruby_init_name);
+static VALUE _native_load(DDTRACE_UNUSED VALUE self, VALUE ruby_path, VALUE ruby_init_name);
 static bool failed_to_load(void *handle, VALUE *failure_details);
 static bool incompatible_library(void *handle, VALUE *failure_details);
 static bool failed_to_initialize(void *handle, char *init_name, VALUE *failure_details);
@@ -51,7 +58,7 @@ void DDTRACE_EXPORT Init_ddtrace_profiling_loader(void) {
   error_symbol = ID2SYM(rb_intern_const("error"));
 }
 
-static VALUE _native_load(VALUE self, VALUE ruby_path, VALUE ruby_init_name) {
+static VALUE _native_load(DDTRACE_UNUSED VALUE self, VALUE ruby_path, VALUE ruby_init_name) {
   Check_Type(ruby_path, T_STRING);
   Check_Type(ruby_init_name, T_STRING);
 

ext/ddtrace_profiling_loader/extconf.rb

@@ -24,6 +24,16 @@ def add_compiler_flag(flag)
   end
 end
 
+# Because we can't control what compiler versions our customers use, shipping with -Werror by default is a no-go.
+# But we can enable it in CI, so that we quickly spot any new warnings that just got introduced.
+add_compiler_flag '-Werror' if ENV['CI'] == 'true'
+
+# Older gcc releases may not default to C99 and we need to ask for this. This is also used:
+# * by upstream Ruby -- search for gnu99 in the codebase
+# * by msgpack, another ddtrace dependency
+#   (https://github.com/msgpack/msgpack-ruby/blob/18ce08f6d612fe973843c366ac9a0b74c4e50599/ext/msgpack/extconf.rb#L8)
+add_compiler_flag '-std=gnu99'
+
 # Gets really noisy when we include the MJIT header, let's omit it
 add_compiler_flag '-Wno-unused-function'
 
@@ -34,6 +44,9 @@ def add_compiler_flag(flag)
 # cause a segfault later. Let's ensure that never happens.
 add_compiler_flag '-Werror-implicit-function-declaration'
 
+# Warn on unused parameters to functions. Use `DDTRACE_UNUSED` to mark things as known-to-not-be-used.
+add_compiler_flag '-Wunused-parameter'
+
 # The native extension is not intended to expose any symbols/functions for other native libraries to use;
 # the sole exception being `Init_ddtrace_profiling_loader` which needs to be visible for Ruby to call it when
 # it `dlopen`s the library.
@@ -42,6 +55,10 @@ def add_compiler_flag(flag)
 # For more details see https://gcc.gnu.org/wiki/Visibility
 add_compiler_flag '-fvisibility=hidden'
 
+# Enable all other compiler warnings
+add_compiler_flag '-Wall'
+add_compiler_flag '-Wextra'
+
 # Tag the native extension library with the Ruby version and Ruby platform.
 # This makes it easier for development (avoids "oops I forgot to rebuild when I switched my Ruby") and ensures that
 # the wrong library is never loaded.

ext/ddtrace_profiling_native_extension/extconf.rb

@@ -77,6 +77,10 @@ def add_compiler_flag(flag)
   end
 end
 
+# Because we can't control what compiler versions our customers use, shipping with -Werror by default is a no-go.
+# But we can enable it in CI, so that we quickly spot any new warnings that just got introduced.
+add_compiler_flag '-Werror' if ENV['CI'] == 'true'
+
 # Older gcc releases may not default to C99 and we need to ask for this. This is also used:
 # * by upstream Ruby -- search for gnu99 in the codebase
 # * by msgpack, another ddtrace dependency