Initial work to add gcc support (#176)

* Remove clang-tidy target

It doesn't work as is since it attempts to redefine the target upon every sub-project include.
At the moment we use uncrustify anyways.

* Initial work to enable gcc support

Started as a hack to workaround cmake's FetchContent not appearing to
pass the CMAKE_COMPILER_ID through when building with CXX=clang++-10

This also fixes a few minor issues that gcc complained about.

TODO:
- [ ] Enable CI pipeline tests

* allow overriding the compiler choice on the cli

* remove cdecl

* add gcc testing to ci pipeline

* tweaks

* include the cxx compiler in the name

* exclude gcc from Ubuntu 16.04

* remove some old debugging code

* remove need for -fdeclspec from clang

also shorten the name of the attr macro

* porting some changes from grlap over

* change the style of initializer we use for std::array for reflection ...

... to something that gcc in Ubuntu 16.04 can use

* enable gcc on 16.04 again

* uncrustify fixups

* tweak the job names

* default to clang, but allow it to be overridden

Co-authored-by: Greg Lapinski <grlap@microsoft.com>

.github/workflows/main.yml

@@ -213,14 +213,20 @@ jobs:
  #load: true
 
  linux-build-test:
- name: Ubuntu ${{ matrix.UbuntuVersion }} ${{ matrix.configuration }} build/test C++/C#
+ name: Ubuntu ${{ matrix.UbuntuVersion }} ${{ matrix.configuration }} ${{ matrix.cxx }} build/test C++/C#
  runs-on: ubuntu-latest
  needs: [prep-vars, docker-image-cached-build]
  timeout-minutes: 30
  strategy:
  matrix:
  UbuntuVersion: ${{ fromJson(needs.prep-vars.outputs.UbuntuVersionMatrix) }}
  configuration: [Debug, Release]
+ cxx: [clang++-10, g++]
+ include:
+ - cxx: clang++-10
+ cc: clang-10
+ - cxx: g++
+ cc: gcc
  steps:
  - name: Checkout
  uses: actions/checkout@v2
@@ -258,8 +264,8 @@ jobs:
  - name: Start ${{ matrix.configuration }} docker instance for Ubuntu ${{ matrix.UbuntuVersion }}
  shell: bash
  run: |
- echo $UID
  docker run -it -d -v $PWD:/src/MLOS -u $UID --env CONFIGURATION=${{ matrix.configuration }} \
+ --env CC=${{ matrix.cc }} --env CXX=${{ matrix.cxx }} \
  --name mlos-${{ matrix.Configuration }}-build-ubuntu-${{ matrix.UbuntuVersion }} \
  mlos-build-ubuntu-${{ matrix.UbuntuVersion }}:${{ github.sha }}
  - name: Setup local user in docker Container
@@ -279,7 +285,7 @@ jobs:
  run: |
  docker exec mlos-${{ matrix.configuration }}-build-ubuntu-${{ matrix.UbuntuVersion }} \
  make dotnet-test
- - name: Run ${{ matrix.configuration }} cmake build
+ - name: Run ${{ matrix.configuration }} cmake build (CXX=${{ matrix.cxx }})
  timeout-minutes: 5
  shell: bash
  run: |

build.linux.sh

@@ -55,13 +55,20 @@ if [ "$CAKE_VERSION" != "$CAKE_INSTALLED_VERSION" ]; then
  dotnet tool install Cake.Tool --tool-path "$TOOLS_DIR" --version $CAKE_VERSION
 
  if [ $? -ne 0 ]; then
- echo "An error occured while installing Cake."
+ echo "An error occurred while installing Cake."
  exit 1
  fi
 fi
 
-export CC=/usr/bin/clang-$CLANG_VERSION
-export CXX=/usr/bin/clang++-$CLANG_VERSION
+if [ "$CC" == '' ] || [ "$CXX" == '' ]; then
+ echo "Defaulting to CXX=clang++-$CLANG_VERSION"
+ export CC=/usr/bin/clang-$CLANG_VERSION
+ export CXX=/usr/bin/clang++-$CLANG_VERSION
+fi
+
+if [ -z "$(which "$CXX" || true)" ]; then
+ echo "WARNING: CXX=$CXX not found." >&2
+fi
 
 $CXX --version
 

build/Common.mk

@@ -19,8 +19,13 @@ MKDIR := mkdir -p
 
 # We currently depend on clang due to use of __declspec(selectany) and other
 # attributes in the codegen output.
-CC := clang-10
-CXX := clang++-10
+# gcc is now in part supported, but we still prefer clang if available
+ifeq ($(origin CC),default)
+ CC := clang-10
+endif
+ifeq ($(origin CXX),default)
+ CXX := clang++-10
+endif
 export CC
 export CXX
 

build/Mlos.Cpp.cmake

@@ -18,12 +18,6 @@ set(CMAKE_CXX_STANDARD_REQUIRED True)
 add_compile_options(-Wall -Wextra -Wpedantic -Werror)
 add_link_options(-Wall -Wextra -Wpedantic -Werror)
 
-# The codegen output currently relies on __declspec(selectany) attributes to
-# instruct the linker to ignore extra definitions resulting from including the
-# same header in multiple places.
-# NOTE: This option is only available with clang, not gcc.
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fdeclspec")
-
 # When compiling for Debug build, make sure that DEBUG is defined for the compiler.
 # This is to mimic MSVC behavior so that our #ifdefs can remain the same rather
 # than having to switch to using NDEBUG.
@@ -34,40 +28,6 @@ set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DDEBUG")
 add_compile_options(-g)
 add_link_options(-g)
 
-# TODO: Search for clang compiler and set the appropriate C/CXX compiler variables.
-#if(NOT (CMAKE_CXX_COMPILER_ID MATCHES "Clang"))
-# # TODO: Add local version of clang to use?
-# find_program(CLANGCPP
-# NAMES clang++-10
-# #PATHS ENV PATH
-# )
-# if(CLANGCPP)
-# message(WARNING "Forcing CXX compiler to ${CLANGCPP}")
-# set(CMAKE_CXX_COMPILER ${CLANGPP})
-# set(CMAKE_CXX_COMPILER_ID "Clang")
-# endif()
-#endif()
-
-# For now we just abort if Clang is not the compiler selected.
-if(NOT (CMAKE_CXX_COMPILER_ID MATCHES "Clang"))
- message(SEND_ERROR
- "MLOS currently only supports clang (not '${CMAKE_CXX_COMPILER_ID}') for compilation.\n"
- "Please re-run (c)make with 'CC=clang-10 CXX=clang++-10' set.\n")
-endif()
-
-# TODO: This just finds the program, but doesn't actually enable it as a
-# target. To use clang-tidy, we will need to provide a .clang-format config
-# and probably reformat some code again.
-find_program(CLANG_TIDY NAMES clang-tidy clang-tidy-6.0)
-if(CLANG_TIDY)
- add_custom_target(
- clang-tidy
- COMMAND ${CLANG_TIDY}
- ${SOURCE_FILES}
- --
- -I ${CMAKE_SOURCE_DIR}/include)
-endif()
-
 # https://github.com/google/sanitizers/wiki/AddressSanitizer
 #
 option(ADDRESS_SANITIZER "Enable Clang AddressSanitizer" OFF)

source/Examples/SmartCache/Main.cpp

@@ -52,7 +52,6 @@ void ThrowIfFail(HRESULT hr)
 }
 
 int
-__cdecl
 main(
  _In_ int argc,
  _In_ char* argv[])

source/Examples/SmartSharedChannel/Main.cpp

@@ -72,7 +72,6 @@ void AssertFailed(
 // NOTES:
 //
 int
-__cdecl
 main(
  _In_ int argc,
  _In_ char* argv[])

source/Mlos.Core/Mlos.Core.h

@@ -74,6 +74,16 @@ constexpr int32_t INVALID_FD_VALUE = -1;
 #define MLOS_RETAIL_ASSERT(result) { if (!result) Mlos::Core::MlosPlatform::TerminateProcess(); }
 #define MLOS_UNUSED_ARG(x) (void)x
 
+#ifdef _MSC_VER
+#define MLOS_SELECTANY_ATTR __declspec(selectany)
+#elif __clang__
+#define MLOS_SELECTANY_ATTR __attribute__((weak))
+#elif __GNUC__
+#define MLOS_SELECTANY_ATTR __attribute__((weak))
+#else
+#warning Unhandled compiler.
+#endif
+
 #include "MlosPlatform.h"
 
 #include "BytePtr.h"

source/Mlos.Core/ObjectDeserializationCallback.h

@@ -53,12 +53,12 @@ class DispatchTable : public std::array<::Mlos::Core::DispatchEntry, N>
  {
  DispatchTable<N + N1> result = {};
 
- for (size_t i = 0; i < N; ++i)
+ for (size_t i = 0; i != N; ++i)
  {
  result[i] = (*this)[i];
  }
 
- for (size_t i = 0; i < N1; ++i)
+ for (size_t i = 0; i != N1; ++i)
  {
  result[i + N] = arr[i];
  }

source/Mlos.SettingsSystem.CodeGen/CodeWriters/CppObjectExchangeCodeWriters/CppObjectDeserializeHandlerCodeWriter.cs

@@ -49,7 +49,7 @@ public override void WriteBeginFile()
 
  // Define a global dispatch table.
  //
- WriteLine("__declspec(selectany) ::Mlos::Core::DispatchEntry DispatchTable[] = ");
+ WriteLine("MLOS_SELECTANY_ATTR ::Mlos::Core::DispatchEntry DispatchTable[] = ");
  WriteLine("{");
 
  IndentationLevel++;

source/Mlos.SettingsSystem.CodeGen/CodeWriters/CppObjectExchangeCodeWriters/CppObjectDeserializeRuntimeCallbackCodeWriter.cs

@@ -82,7 +82,7 @@ public override void BeginVisitType(Type sourceType)
  //
  string cppTypeNameAsField = $"{sourceType.Name}_Callback";
 
- WriteLine($"__declspec(selectany) std::function<void ({cppProxyTypeFullName}&&)> {cppTypeNameAsField} = nullptr;");
+ WriteLine($"MLOS_SELECTANY_ATTR std::function<void ({cppProxyTypeFullName}&&)> {cppTypeNameAsField} = nullptr;");
  WriteLine();
  }
 

source/Mlos.SettingsSystem.CodeGen/CodeWriters/CppObjectExchangeCodeWriters/CppObjectGetVariableDataSizeCodeWriter.cs

@@ -43,7 +43,7 @@ public override void WriteBeginFile()
  public override void WriteEndFile()
  {
  IndentationLevel--;
- WriteLine("};");
+ WriteLine("}");
  WriteLine();
  }
 

source/Mlos.SettingsSystem.CodeGen/CodeWriters/CppObjectExchangeCodeWriters/CppObjectSerializationVariableDataCodeWriter.cs

@@ -50,7 +50,7 @@ public override void WriteBeginFile()
  public override void WriteEndFile()
  {
  IndentationLevel--;
- WriteLine("};");
+ WriteLine("}");
  WriteLine();
  }
 
@@ -99,7 +99,7 @@ public override void EndVisitType(Type sourceType)
  WriteLine("return totalDataSize;");
  IndentationLevel--;
 
- WriteLine("};");
+ WriteLine("}");
  WriteLine();
  }
 

source/Mlos.SettingsSystem.CodeGen/CodeWriters/CppObjectExchangeCodeWriters/CppProxyVerifyVariableDataCodeWriter.cs

@@ -40,7 +40,7 @@ public override void WriteBeginFile()
  public override void WriteEndFile()
  {
  IndentationLevel--;
- WriteLine("};");
+ WriteLine("}");
  WriteLine();
  }
 

source/Mlos.SettingsSystem.CodeGen/CodeWriters/CppObjectExchangeCodeWriters/CppTypeReflectionTableCodeWriter.cs

@@ -84,7 +84,7 @@ public override void BeginVisitType(Type sourceType)
  // Write a class name using std::array.
  // std::array<5, char> = { 'C', 'l', 'a', 's', 's', '\0' };
  //
- WriteLine($"std::array<char, {nameLength}> TypeName_{cppClassName} = {{ {string.Join(",", cppClassName.Select(r => $"'{r}'"))}, '\\0' }};");
+ WriteLine($"std::array<char, {nameLength}> TypeName_{cppClassName} {{{{ {string.Join(",", cppClassName.Select(r => $"'{r}'"))}, '\\0' }}}};");
 
  metadataOffset += nameLength + sizeof(uint);
 

source/Mlos.UnitTest/Main.cpp

@@ -29,7 +29,6 @@ using ::testing::TestPartResult;
 using ::testing::UnitTest;
 
 int
-__cdecl
 main(
  _In_ int argc,
  _In_ char* argv[])

source/Mlos.UnitTest/SharedChannelTests.cpp

@@ -26,10 +26,10 @@ class TestFlatBuffer : public BytePtr
  }
 
 private:
- std::array<byte, T> array = { 0 };
+ std::array<byte, T> array { { 0 } };
 };
 
-namespace
+namespace SharedChannelTests
 {
 #ifndef DEBUG
 // Verify buffer size.
@@ -212,7 +212,7 @@ TEST(SharedChannel, VerifySendingReceivingArrayStruct)
  ChannelSynchronization sync = {};
  TestSharedChannel sharedChannel(sync, buffer, 128);
 
- Mlos::UnitTest::Line line;
+ Mlos::UnitTest::Line line = {};
  line.Points[0] = { 3, 5 };
  line.Points[1] = { 7, 9 };
  line.Height = { 1.3f, 3.9f };
@@ -222,8 +222,8 @@ TEST(SharedChannel, VerifySendingReceivingArrayStruct)
  //
  ObjectDeserializationCallback::Mlos::UnitTest::Line_Callback = [line](Proxy::Mlos::UnitTest::Line&& recvLine)
  {
- EXPECT_EQ(recvLine.Points()[0].X(), 3.0);
- EXPECT_EQ(recvLine.Points()[0].Y(), 5);
+ EXPECT_EQ(recvLine.Points()[0].X(), line.Points[0].X);
+ EXPECT_EQ(recvLine.Points()[0].Y(), line.Points[0].Y);
  EXPECT_EQ(recvLine.Points()[1].X(), 7);
  EXPECT_EQ(recvLine.Points()[1].Y(), 9);
  EXPECT_EQ(recvLine.Height()[0], 1.3f);
@@ -368,6 +368,7 @@ TEST(SharedChannel, StressSendReceive)
  bool result =
  resultFromWriter1.get() &&
  resultFromWriter2.get();
+ MLOS_UNUSED_ARG(result);
 
  sharedChannel.SendMessage(Mlos::Core::TerminateReaderThreadRequestMessage());
 
@@ -376,5 +377,7 @@ TEST(SharedChannel, StressSendReceive)
  result =
  resultFromReader1.get() &&
  resultFromReader2.get();
+
+ EXPECT_EQ(result, true);
 }
 }