Clang tidy pr #5
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| #ifndef _MESH_PRIMITIVE_H | ||
| #define _MESH_PRIMITIVE_H | ||
|
|
||
| #include "Primitives/Primitive.h" |
There was a problem hiding this comment.
warning: 'Primitives/Primitive.h' file not found [clang-diagnostic-error]
^| #ifndef _MESH_PRIMITIVE_H | ||
| #define _MESH_PRIMITIVE_H | ||
|
|
||
| #include "Primitives/Primitive.h" |
There was a problem hiding this comment.
warning: declaration uses identifier '_MESH_PRIMITIVE_H', which is a reserved identifier [bugprone-reserved-identifier]
Suggested change
| #include "Primitives/Primitive.h" | |
| H | |
| HMESH_PRIMITIVE_H |
| #include <system_error> | ||
| #include <vector> | ||
|
|
||
| namespace Enigma::Renderables |
There was a problem hiding this comment.
warning: variable 'Enigma' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
^| @@ -0,0 +1,345 @@ | |||
| #include "MeshPrimitive.h" | |||
There was a problem hiding this comment.
warning: 'MeshPrimitive.h' file not found [clang-diagnostic-error]
#include "MeshPrimitive.h"
^| @@ -41,10 +44,10 @@ class bank | |||
| { | |||
| std::cout << "bank destruct. \n"; | |||
| } | |||
| void set(const std::any& a) | |||
| void set(const std::any& aB) | |||
| { | |||
| std::cout << "set bank data. \n"; | |||
There was a problem hiding this comment.
warning: parameter name 'aB' is too short, expected at least 3 characters [readability-identifier-length]
\n";
^
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
Cpp-linter Review
Used clang-tidy v12.0.1
Only 35 out of 76 clang-tidy concerns fit within this pull request's diff.
Click here for the full clang-tidy patch
diff --git a/MeshPrimitive.h b/MeshPrimitive.h
index 60d7425..205f36f 100644
--- a/MeshPrimitive.h
+++ b/MeshPrimitive.h
@@ -67 +67 @@ namespace Enigma::Renderables
- static error updateRenderBuffer();
+ error updateRenderBuffer();
@@ -77 +77 @@ namespace Enigma::Renderables
- const MathLib::Matrix4& mx_world, const Engine::RenderLightingState& lighting_state) override;
+ const MathLib::Matrix4& mxWorld, const Engine::RenderLightingState& lightingState) override;
@@ -102 +102 @@ namespace Enigma::Renderables
- static void createRenderElements();
+ void createRenderElements();
@@ -108,4 +108,4 @@ namespace Enigma::Renderables
- static void bindPrimitiveMaterials();
- static void bindSegmentMaterial(unsigned index);
- static void loosePrimitiveMaterials();
- static void looseSegmentMaterial(unsigned index);
+ void bindPrimitiveMaterials();
+ void bindSegmentMaterial(unsigned index);
+ void loosePrimitiveMaterials();
+ void looseSegmentMaterial(unsigned index);
diff --git a/StdAny/StdAny/MeshPrimitive.cpp b/StdAny/StdAny/MeshPrimitive.cpp
index cae0254..5c87f82 100644
--- a/StdAny/StdAny/MeshPrimitive.cpp
+++ b/StdAny/StdAny/MeshPrimitive.cpp
@@ -22 +22 @@ using namespace Enigma::Graphics;
-DEFINE_RTTI(Renderables, MeshPrimitive, Primitive);
+defineRtti(Renderables, MeshPrimitive, Primitive);
@@ -53 +53 @@ std::shared_ptr<Enigma::Primitives::PrimitiveAssembler> MeshPrimitive::assembler
-void MeshPrimitive::assemble(const std::shared_ptr<Primitives::PrimitiveAssembler>& assembler) const
+static void MeshPrimitive::assemble(const std::shared_ptr<Primitives::PrimitiveAssembler>& /*assembler*/)
@@ -81 +81 @@ std::shared_ptr<Enigma::Primitives::PrimitiveDisassembler> MeshPrimitive::disass
-void MeshPrimitive::disassemble(const std::shared_ptr<Primitives::PrimitiveDisassembler>& disassembler)
+void MeshPrimitive::disassemble(const std::shared_ptr<Primitives::PrimitiveDisassembler>& /*disassembler*/)
@@ -124 +124 @@ unsigned MeshPrimitive::getMaterialCount() const
-void MeshPrimitive::changeMaterials(const std::vector<std::shared_ptr<PrimitiveMaterial>>& materials)
+void MeshPrimitive::changeMaterials(const std::vector<std::shared_ptr<PrimitiveMaterial>>& /*materials*/)
@@ -137 +137 @@ void MeshPrimitive::rebindMaterials()
-void MeshPrimitive::changeSemanticTexture(const Engine::EffectSemanticTexture& semantic_texture)
+void MeshPrimitive::changeSemanticTexture(const Engine::EffectSemanticTexture& /*semantic_texture*/)
@@ -149 +149 @@ void MeshPrimitive::changeSemanticTexture(const Engine::EffectSemanticTexture& s
-void MeshPrimitive::bindSemanticTexture(const Engine::EffectSemanticTexture& semantic_texture)
+void MeshPrimitive::bindSemanticTexture(const Engine::EffectSemanticTexture& /*semantic_texture*/)
@@ -161 +161 @@ void MeshPrimitive::bindSemanticTexture(const Engine::EffectSemanticTexture& sem
-void MeshPrimitive::bindSemanticTextures(const std::vector<EffectSemanticTexture>& textures)
+void MeshPrimitive::bindSemanticTextures(const std::vector<EffectSemanticTexture>& /*textures*/)
@@ -181,2 +181,2 @@ error MeshPrimitive::updateRenderBuffer()
-error MeshPrimitive::rangedUpdateRenderBuffer(unsigned vtx_offset, unsigned vtx_count,
- std::optional<unsigned> idx_offset, std::optional<unsigned> idx_count)
+static error MeshPrimitive::rangedUpdateRenderBuffer(unsigned vtx_offset, unsigned vtx_count,
+ std::optional<unsigned> /*idx_offset*/, std::optional<unsigned> /*idx_count*/)
@@ -192 +192 @@ error MeshPrimitive::rangedUpdateRenderBuffer(unsigned vtx_offset, unsigned vtx_
-error MeshPrimitive::insertToRendererWithTransformUpdating(const std::shared_ptr<Engine::IRenderer>& renderer,
+static error MeshPrimitive::insertToRendererWithTransformUpdating(const std::shared_ptr<Engine::IRenderer>& /*renderer*/,
@@ -212 +212 @@ error MeshPrimitive::insertToRendererWithTransformUpdating(const std::shared_ptr
-error MeshPrimitive::removeFromRenderer(const std::shared_ptr<Engine::IRenderer>& renderer)
+static error MeshPrimitive::removeFromRenderer(const std::shared_ptr<Engine::IRenderer>& /*renderer*/)
@@ -233 +233 @@ void MeshPrimitive::calculateBoundingVolume(bool axis_align)
-void MeshPrimitive::updateWorldTransform(const MathLib::Matrix4& mxWorld)
+void MeshPrimitive::updateWorldTransform(const MathLib::Matrix4& mx_world)
@@ -238 +238 @@ void MeshPrimitive::updateWorldTransform(const MathLib::Matrix4& mxWorld)
-void MeshPrimitive::selectVisualTechnique(const std::string& techniqueName)
+void MeshPrimitive::selectVisualTechnique(const std::string& technique_name)
@@ -240 +240 @@ void MeshPrimitive::selectVisualTechnique(const std::string& techniqueName)
- Primitive::selectVisualTechnique(techniqueName);
+ Primitive::selectVisualTechnique(technique_name);
@@ -247 +247 @@ void MeshPrimitive::selectVisualTechnique(const std::string& techniqueName)
-void MeshPrimitive::linkGeometryData(const Geometries::GeometryDataPtr& geo, const Engine::RenderBufferPtr& render_buffer)
+void MeshPrimitive::linkGeometryData(const Geometries::GeometryDataPtr& /*geo*/, const Engine::RenderBufferPtr& /*render_buffer*/)
@@ -255 +255 @@ void MeshPrimitive::linkGeometryData(const Geometries::GeometryDataPtr& geo, con
-void MeshPrimitive::changeEffectInSegment(unsigned index, const std::shared_ptr<EffectMaterial>& effect)
+void MeshPrimitive::changeEffectInSegment(unsigned index, const std::shared_ptr<EffectMaterial>& /*effect*/)
@@ -263 +263 @@ void MeshPrimitive::changeEffectInSegment(unsigned index, const std::shared_ptr<
-void MeshPrimitive::changeEffects(const EffectMaterialList& effects)
+void MeshPrimitive::changeEffects(const EffectMaterialList& /*effects*/)
@@ -273 +273 @@ void MeshPrimitive::changeEffects(const EffectMaterialList& effects)
-void MeshPrimitive::changeTextureMapInSegment(unsigned index, const Engine::EffectTextureMap& tex_map)
+void MeshPrimitive::changeTextureMapInSegment(unsigned index, const Engine::EffectTextureMap& /*tex_map*/)
@@ -281 +281 @@ void MeshPrimitive::changeTextureMapInSegment(unsigned index, const Engine::Effe
-void MeshPrimitive::changeTextureMaps(const TextureMapList& tex_maps)
+void MeshPrimitive::changeTextureMaps(const TextureMapList& /*tex_maps*/)
diff --git a/StdAny/StdAny/StdAny.cpp b/StdAny/StdAny/StdAny.cpp
index 76ae59b..75853b0 100644
--- a/StdAny/StdAny/StdAny.cpp
+++ b/StdAny/StdAny/StdAny.cpp
@@ -11 +11 @@ static int s_counter = 0;
-class obj
+class Obj
@@ -14 +14 @@ public:
- obj() : m_a{ s_counter++ }
+ Obj() : m_a{ s_counter++ }
@@ -18 +18 @@ public:
- obj(const obj& o)
+ Obj(const Obj& o)
@@ -24 +24 @@ public:
- ~obj()
+ ~Obj()
@@ -32 +32 @@ private:
- static int m_b;
+ static int s_mB;
@@ -36 +36 @@ private:
-class bank
+class Bank
@@ -39 +39 @@ public:
- bank()
+ Bank()
@@ -43 +43 @@ public:
- ~bank()
+ ~Bank()
@@ -47 +47 @@ public:
- void set(const std::any& aB)
+ void set(const std::any& a_b)
@@ -50 +50 @@ public:
- m_data = aB;
+ m_data = a_b;
@@ -54 +54 @@ public:
- template <class T> void set_org(const T& a) /// 這樣是物件參考，參數不會複製
+ template <class T> void setOrg(const T& a) /// 這樣是物件參考，參數不會複製
@@ -60 +60 @@ public:
- const std::any& get()
+ const std::any& get() const
@@ -65 +65 @@ public:
- template <class T> T get_org()
+ template <class T> T getOrg()
@@ -70 +70 @@ public:
- template <class T> std::optional<T> get_opt()
+ template <class T> std::optional<T> getOpt()
@@ -75 +75 @@ public:
- template <class T> std::optional<std::reference_wrapper<T>> get_opt_ref()
+ template <class T> std::optional<std::reference_wrapper<T>> getOptRef()
@@ -83 +83 @@ public:
- std::vector<obj> m_org_data;
+ std::vector<Obj> m_mOrgData;
@@ -87 +87 @@ using func_any = std::function<std::any(const std::string&)>;
-class func_store
+class FuncStore
@@ -93 +93,2 @@ public:
- if (found != m_funcs.end()) return found->second(param);
+ if (found != m_funcs.end()) { return found->second(param);
+}
@@ -108 +109 @@ void func(const std::any& a)
- auto ob = std::any_cast<std::reference_wrapper<std::vector<obj>>>(a1); /// by reference 要這樣轉
+ auto ob = std::any_cast<std::reference_wrapper<std::vector<Obj>>>(a1); /// by reference 要這樣轉
@@ -114 +115 @@ void func(const std::any& a)
-std::any any_invoker(const std::string& param)
+std::any anyInvoker(const std::string& param)
@@ -124 +125 @@ int main()
- std::vector<obj> obj_vec{ 10 };
+ std::vector<Obj> obj_vec{ 10 };
@@ -138 +139 @@ int main()
- auto b = new bank();
+ auto *b = new Bank();
@@ -141,2 +142,2 @@ int main()
- std::vector<obj> obj_vec{ 10 };
- b->set_org(obj_vec);
+ std::vector<Obj> obj_vec{ 10 };
+ b->setOrg(obj_vec);
@@ -159 +160 @@ int main()
- auto& ov2 = b->get_org<std::vector<obj>&>(); /// 沒有複製, 可以取出來改
+ auto& ov2 = b->getOrg<std::vector<Obj>&>(); /// 沒有複製, 可以取出來改
@@ -167 +168 @@ int main()
- auto b1 = new bank();
+ auto *b1 = new Bank();
@@ -170,2 +171,2 @@ int main()
- std::vector<obj> obj_vec{ 10 };
- b1->set_org(obj_vec);
+ std::vector<Obj> obj_vec{ 10 };
+ b1->setOrg(obj_vec);
@@ -174 +175 @@ int main()
- auto ov1 = b1->get_opt<std::vector<obj>>(); /// optional 給 value, 所以會複製
+ auto ov1 = b1->getOpt<std::vector<Obj>>(); /// optional 給 value, 所以會複製
@@ -176 +177 @@ int main()
- auto ov2 = b1->get_opt_ref<std::any>(); /// 只能取 std::any
+ auto ov2 = b1->getOptRef<std::any>(); /// 只能取 std::any
@@ -184,2 +185,2 @@ int main()
- auto fs = new func_store();
- fs->m_funcs.emplace("any_invoker", any_invoker);
+ auto *fs = new FuncStore();
+ fs->m_funcs.emplace("any_invoker", anyInvoker);
Have any feedback or feature suggestions? Share it here.
| void bindSemanticTextures(const std::vector<Engine::EffectSemanticTexture>& textures); | ||
|
|
||
| /** update render buffer */ | ||
| error updateRenderBuffer(); |
There was a problem hiding this comment.
clang-tidy suggestion
Suggested change
| error updateRenderBuffer(); | |
| error updateRenderBuffer(); |
clang-tidy diagnostic
MeshPrimitive.h:9:9: warning: [bugprone-reserved-identifier]
declaration uses identifier '_MESH_PRIMITIVE_H', which is a reserved identifier
#define _MESH_PRIMITIVE_H
^
note: this fix will not be applied because it overlaps with another fixclang-tidy diagnostic
MeshPrimitive.h:9:9: warning: [readability-identifier-naming]
invalid case style for macro definition '_MESH_PRIMITIVE_H'
note: this fix will not be applied because it overlaps with another fixclang-tidy diagnostic
MeshPrimitive.h:11:10: error: [clang-diagnostic-error]
'Primitives/Primitive.h' file not found
#include "Primitives/Primitive.h"
^clang-tidy diagnostic
MeshPrimitive.h:59:38: warning: [readability-avoid-const-params-in-decls]
parameter 1 is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions
virtual void changeMaterials(const std::vector<std::shared_ptr<PrimitiveMaterial>>& materials);
^|
|
||
| /** insert to renderer */ | ||
| virtual error insertToRendererWithTransformUpdating(const std::shared_ptr<Engine::IRenderer>& renderer, | ||
| const MathLib::Matrix4& mxWorld, const Engine::RenderLightingState& lightingState) override; |
There was a problem hiding this comment.
clang-tidy suggestion
Suggested change
| const MathLib::Matrix4& mxWorld, const Engine::RenderLightingState& lightingState) override; | |
| const MathLib::Matrix4& mxWorld, const Engine::RenderLightingState& lightingState) override; |
| /** change primitive's texture map */ | ||
| void changeTextureMaps(const TextureMapList& tex_maps); | ||
| /** create render elements */ | ||
| void createRenderElements(); |
There was a problem hiding this comment.
clang-tidy suggestion
Suggested change
| void createRenderElements(); | |
| void createRenderElements(); |
Comment on lines
+108
to
+111
| void bindPrimitiveMaterials(); | ||
| void bindSegmentMaterial(unsigned index); | ||
| void loosePrimitiveMaterials(); | ||
| void looseSegmentMaterial(unsigned index); |
There was a problem hiding this comment.
clang-tidy suggestion
Suggested change
| void bindPrimitiveMaterials(); | |
| void bindSegmentMaterial(unsigned index); | |
| void loosePrimitiveMaterials(); | |
| void looseSegmentMaterial(unsigned index); | |
| void bindPrimitiveMaterials(); | |
| void bindSegmentMaterial(unsigned index); | |
| void loosePrimitiveMaterials(); | |
| void looseSegmentMaterial(unsigned index); |
|
|
||
| protected: | ||
| using RenderElementList = std::vector<std::shared_ptr<Renderer::RenderElement>>; | ||
| Geometries::GeometryDataPtr m_geometry; |
There was a problem hiding this comment.
clang-tidy diagnostic
MeshPrimitive.h:115:37: warning: [misc-non-private-member-variables-in-classes]
member variable 'm_geometry' has protected visibility
Geometries::GeometryDataPtr m_geometry;
^| @@ -54,7 +57,7 @@ class bank | |||
| m_data = a; /// 這裡會複製 | |||
| } | |||
|
|
|||
| const std::any& get() | |||
| const std::any& get() | |||
There was a problem hiding this comment.
clang-tidy diagnostics
- method 'get' can be made const [readability-make-member-function-const]
Suggested change
| const std::any& get() | |
| const std::any& get() const |
| @@ -77,6 +80,7 @@ class bank | |||
| return std::ref(m_data); | |||
| } | |||
| std::any m_data; | |||
| std::vector<obj> m_org_data; | |||
There was a problem hiding this comment.
clang-tidy diagnostics
- invalid case style for class 'obj' [readability-identifier-naming]
- invalid case style for member 'm_org_data' [readability-identifier-naming]
Suggested change
| std::vector<obj> m_org_data; | |
| std::vector<Obj> m_mOrgData; |
clang-tidy diagnostic
StdAny/StdAny/StdAny.cpp:82:14: warning: [misc-non-private-member-variables-in-classes]
member variable 'm_data' has public visibility
std::any m_data;
^clang-tidy diagnostic
StdAny/StdAny/StdAny.cpp:83:22: warning: [misc-non-private-member-variables-in-classes]
member variable 'm_org_data' has public visibility
std::vector<obj> m_org_data;
^| @@ -103,7 +107,7 @@ void func(const std::any& a) | |||
| //auto ob = std::any_cast<std::vector<obj>>(std::cref(a)); /// 這樣不能轉 | |||
| auto ob = std::any_cast<std::reference_wrapper<std::vector<obj>>>(a1); /// by reference 要這樣轉 | |||
There was a problem hiding this comment.
clang-tidy diagnostics
- invalid case style for class 'obj' [readability-identifier-naming]
Suggested change
| auto ob = std::any_cast<std::reference_wrapper<std::vector<obj>>>(a1); /// by reference 要這樣轉 | |
| auto ob = std::any_cast<std::reference_wrapper<std::vector<Obj>>>(a1); /// by reference 要這樣轉 |
| @@ -153,7 +157,7 @@ int main() | |||
| //auto ov2 = b->get_org<const std::vector<obj>&>(); /// 有複製, auto 不是引用 | |||
| //auto& ov2 = b->get_org<const std::vector<obj>&>(); /// 沒有複製 | |||
| auto& ov2 = b->get_org<std::vector<obj>&>(); /// 沒有複製, 可以取出來改 | |||
There was a problem hiding this comment.
clang-tidy diagnostics
- invalid case style for class 'obj' [readability-identifier-naming]
- invalid case style for method 'get_org' [readability-identifier-naming]
Suggested change
| auto& ov2 = b->get_org<std::vector<obj>&>(); /// 沒有複製, 可以取出來改 | |
| auto& ov2 = b->getOrg<std::vector<Obj>&>(); /// 沒有複製, 可以取出來改 |
| @@ -153,7 +157,7 @@ int main() | |||
| //auto ov2 = b->get_org<const std::vector<obj>&>(); /// 有複製, auto 不是引用 | |||
| //auto& ov2 = b->get_org<const std::vector<obj>&>(); /// 沒有複製 | |||
| auto& ov2 = b->get_org<std::vector<obj>&>(); /// 沒有複製, 可以取出來改 | |||
| ov2[8].m_a = -23; | |||
| ov2[8].a() = -23; | |||
There was a problem hiding this comment.
clang-tidy diagnostic
StdAny/StdAny/StdAny.cpp:160:13: warning: [readability-magic-numbers]
8 is a magic number; consider replacing it with a named constant
ov2[8].a() = -23;
^clang-tidy diagnostic
StdAny/StdAny/StdAny.cpp:160:23: warning: [readability-magic-numbers]
23 is a magic number; consider replacing it with a named constant
ov2[8].a() = -23;
^
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.