Enabled clang compiler support #8

VladimirTakeda · 2024-02-27T17:30:01Z

No description provided.

VladimirTakeda · 2024-02-27T17:43:58Z

.github/workflows/cmake-multi-platform.yml

-      # 3. <Linux, Release, latest Clang compiler toolchain on the default runner image, default generator>
+      # 3. <Linux, Debug, latest GCC compiler toolchain on the default runner image, default generator>
+      # 3. <Linux, Release, clang-15, default generator>
+      # 3. <Linux, Debug, clang-15, default generator>
      #


I have't noticed the numbers, so I have to fix here)

zproksi · 2024-02-27T21:22:31Z

srcbpatch/binarylexeme.cpp

@@ -20,7 +20,7 @@ const span<const char>& AbstractBinaryLexeme::Replace(size_t at, size_t alength,

 unique_ptr<AbstractBinaryLexeme> AbstractBinaryLexeme::LexemeFromVector(vector<char>&& asrcVec)
 {
-    return unique_ptr<AbstractBinaryLexeme>(new AbstractBinaryLexeme(move(asrcVec)));
+    return unique_ptr<AbstractBinaryLexeme>(new AbstractBinaryLexeme(std::move(asrcVec)));


check Line 7 in this file:
this change is unnecessary

zproksi · 2024-02-27T21:23:13Z

.gitignore

@@ -6,6 +6,8 @@ buildWinRelease/
 build/
 .vscode/
 x64/
+.idea/
+cmake-build-debug/


What is the reason of the "cmake-build-debug/" folder?

Take a look on the script rebuild.sh in the root of the project. run it on your mac - whether it will work?

(It is possible that this folder need to have another name.)

I use CLion ide and it contains this folders. So, it's just for simplify committing for me.

zproksi · 2024-02-27T21:24:06Z

srcbpatch/binarylexeme.cpp

@@ -51,7 +51,7 @@ unique_ptr<AbstractBinaryLexeme> AbstractBinaryLexeme::LexemeFromLexeme(unique_p


 AbstractBinaryLexeme::AbstractBinaryLexeme(vector<char>&& asrcVec)
-    : dataSequence_(move(asrcVec))
+    : dataSequence_(std::move(asrcVec))


check Line 7 in this file:
this change is unnecessary
NOTE: why vector at line 53 is not with namespace

zproksi · 2024-02-27T21:25:17Z

srcbpatch/consoleparametersreader.cpp

+#include <ranges>
+#include <algorithm>
+#include <cstring>
+


#include "stdafx.h" from line 1
already contains these includes

But it doesn't work for clang. I will send you proofs.

zproksi · 2024-02-27T21:25:46Z

srcbpatch/fileprocessing.cpp

@@ -1,6 +1,9 @@
 #include "stdafx.h"
 #include "fileprocessing.h"

+#include <ranges>


#include "stdafx.h" from line 1
already contains these includes

But it doesn't work for clang. I will send you proofs.

zproksi · 2024-02-27T21:27:32Z

srcbpatch/fileprocessing.cpp

@@ -24,11 +27,15 @@ using namespace std::filesystem;

 FileProcessing::FileProcessing(const char* fname, const char* mode)
 {
-#pragma warning(disable: 4996) // I would like to use fopen instead of fopen_s, because
+#if !defined(__linux__) && !(defined(__APPLE__) && defined(__MACH__))
+    #pragma warning(disable: 4996) // I would like to use fopen instead of fopen_s, because


any precompile directives must be written from 1-st position (no indentation)

zproksi · 2024-02-27T21:27:44Z

srcbpatch/fileprocessing.cpp

    if (stream_ = fopen(fname, mode); nullptr == stream_)
-#pragma warning(default: 4996)
+#if !defined(__linux__) && !(defined(__APPLE__) && defined(__MACH__))
+    #pragma warning(default: 4996)


any precompile directives must be written from 1-st position (no indentation)

zproksi · 2024-02-27T21:30:56Z

srcbpatch/processing.cpp

@@ -46,7 +46,7 @@ unique_ptr<ActionsCollection> CreateActionsFile(string_view actionsFileName)

    // Parsing of todo and lexemes
    // Dictionary will be inside
-    return unique_ptr<ActionsCollection>(new ActionsCollection(move(adata)));
+    return unique_ptr<ActionsCollection>(new ActionsCollection(std::move(adata)));


I hardly believe that we need to use std::move near the usage of unique_ptr without std::

Ok, I will double check this moment

zproksi · 2024-02-27T21:34:29Z

testbpatch/CMakeLists.txt

@@ -2,6 +2,17 @@ cmake_minimum_required(VERSION 3.19)
 set(pname bpatch)
 project(test${pname})

+include(FetchContent)


This code was in the version because it is tagged and must work without possibility of harmful code injections
note: GIT_TAG below

##get google test
include(FetchContent)
FetchContent_Declare(googletest
GIT_REPOSITORY https://github.com/google/googletest
GIT_TAG release-1.12.1)
FetchContent_GetProperties(googletest)
#googletest_POPULATED
#googletest_SOURCE_DIR
#googletest_BUILD_DIR
if(NOT googletest_POPULATED)
FetchContent_Populate(googletest)
add_subdirectory(${googletest_SOURCE_DIR} ${googletest_BUILD_DIR})
endif()

I use the code from official repo https://github.com/google/googletest/blob/main/googletest/README.md, so it's correct as well.

And I had some troubles with the existing code in clang. For some reason static libraries have been created in wrong directory so the cmake of testbpatch unable to find it.

zproksi · 2024-02-27T21:35:37Z

testbpatch/CMakeLists.txt

+# For Windows: Prevent overriding the parent project's compiler/linker settings
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+FetchContent_MakeAvailable(googletest)
+


one new line between sections in CMakeLists.txt.
PS. remove line 20 below to follow this rule, please

zproksi · 2024-02-27T21:38:09Z

wildcharacters/wildcharacters.cpp

@@ -1,4 +1,5 @@
 #include <filesystem>
+#include <sstream>


Reason of this include is unclear.
justify or remove

I got compilation error using clang. It doesn't see sstream functions

zproksi · 2024-02-27T21:42:50Z

.github/workflows/cmake-multi-platform.yml

          - os: ubuntu-latest
            c_compiler: cl

    steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4


Нет понимания почему тут поменялась 3 на 4.
расскажешь?

Azure/docker-login#63

Просто вышла новая версия, а на эту кидает ворнинг, что она устарела.

zproksi · 2024-02-27T21:43:35Z

.github/workflows/cmake-multi-platform.yml

+    - uses: actions/checkout@v4
+
+    - name: Install clang
+      if: matrix.os=='ubuntu-latest' && matrix.c_compiler=='clang-15'


Нет строк(и) комментария с описанием что происходит и почему это нужно

VladimirTakeda commented Feb 27, 2024

View reviewed changes

zproksi reviewed Feb 27, 2024

View reviewed changes

VladimirTakeda closed this Feb 29, 2024

VladimirTakeda force-pushed the master branch from 1e5beb6 to b71e8b5 Compare February 29, 2024 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabled clang compiler support #8

Enabled clang compiler support #8

VladimirTakeda commented Feb 27, 2024

VladimirTakeda Feb 27, 2024 •

edited

Loading

zproksi Feb 27, 2024

zproksi Feb 27, 2024 •

edited

Loading

VladimirTakeda Feb 27, 2024

zproksi Feb 27, 2024

zproksi Feb 27, 2024 •

edited

Loading

VladimirTakeda Feb 27, 2024

zproksi Feb 27, 2024

VladimirTakeda Feb 27, 2024

zproksi Feb 27, 2024

zproksi Feb 27, 2024

zproksi Feb 27, 2024

VladimirTakeda Feb 27, 2024

zproksi Feb 27, 2024

VladimirTakeda Feb 27, 2024

zproksi Feb 27, 2024

zproksi Feb 27, 2024

VladimirTakeda Feb 27, 2024

zproksi Feb 27, 2024

VladimirTakeda Feb 28, 2024

zproksi Feb 27, 2024 •

edited

Loading

Enabled clang compiler support #8

Enabled clang compiler support #8

Conversation

VladimirTakeda commented Feb 27, 2024

VladimirTakeda Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zproksi Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zproksi Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zproksi Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

VladimirTakeda Feb 27, 2024 •

edited

Loading

zproksi Feb 27, 2024 •

edited

Loading

zproksi Feb 27, 2024 •

edited

Loading

zproksi Feb 27, 2024 •

edited

Loading