Skip to content

Commit 58ba266

Browse files
authored
Add clang-tidy and builds with assertions/memory sanitizers for libGD.js (#7051)
Only show in developer changelog
1 parent b34e802 commit 58ba266

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+507
-205
lines changed

.circleci/config.yml

+75-1
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ jobs:
176176

177177
# Build the WebAssembly library only (so that it's cached on a S3 and easy to re-use).
178178
build-gdevelop_js-wasm-only:
179+
resource_class: medium+ # Compilation time decrease linearly with the number of CPUs, but not linking (so "large" does not speedup total build time).
179180
docker:
180181
- image: cimg/node:16.13
181182

@@ -232,10 +233,83 @@ jobs:
232233
name: Deploy to S3 (latest)
233234
command: aws s3 sync Binaries/embuild/GDevelop.js s3://gdevelop-gdevelop.js/$(git rev-parse --abbrev-ref HEAD)/latest/
234235

236+
# Build the WebAssembly library with clang-tidy and memory sanitizers.
237+
build-gdevelop_js-debug-sanitizers-and-extra-checks:
238+
resource_class: xlarge # Total time decrease linearly with the number of CPUs.
239+
docker:
240+
- image: cimg/node:16.13
241+
242+
working_directory: ~/GDevelop
243+
244+
steps:
245+
- checkout
246+
- aws-cli/setup
247+
248+
# System dependencies (for Emscripten)
249+
- run:
250+
name: Install dependencies for Emscripten
251+
command: sudo apt-get update && sudo apt install cmake
252+
253+
- run:
254+
name: Install dependencies for clang-tidy v19
255+
command: wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && sudo ./llvm.sh 19 && sudo apt install clang-tidy-19
256+
257+
- run:
258+
name: Install Python3 dependencies for Emscripten
259+
command: sudo apt install python-is-python3 python3-distutils -y
260+
261+
- run:
262+
name: Install Emscripten (for GDevelop.js)
263+
command: git clone https://github.com/juj/emsdk.git && cd emsdk && ./emsdk install 3.1.21 && ./emsdk activate 3.1.21 && cd ..
264+
265+
# GDevelop.js dependencies
266+
- restore_cache:
267+
keys:
268+
- gdevelop.js-linux-nodejs-dependencies-{{ checksum "GDevelop.js/package-lock.json" }}
269+
# fallback to using the latest cache if no exact match is found
270+
- gdevelop.js-linux-nodejs-dependencies-
271+
272+
- run:
273+
name: Install GDevelop.js dependencies and build it
274+
command: cd GDevelop.js && npm install && cd ..
275+
276+
# Build GDevelop.js
277+
- run:
278+
name: Build GDevelop.js ('debug-sanitizers' variant)
279+
command: cd GDevelop.js && source ../emsdk/emsdk_env.sh && npm run build -- --variant=debug-sanitizers
280+
281+
- run:
282+
name: Run clang-tidy
283+
command: cd GDevelop.js && npm run lint
284+
285+
- run:
286+
name: Run tests
287+
command: cd GDevelop.js && npm run test -- --maxWorkers=4
288+
289+
# Upload artifacts (CircleCI)
290+
- store_artifacts:
291+
path: Binaries/embuild/GDevelop.js
292+
293+
# Upload artifacts (AWS)
294+
- run:
295+
name: Deploy to S3 (specific commit)
296+
command: aws s3 sync Binaries/embuild/GDevelop.js s3://gdevelop-gdevelop.js/$(git rev-parse --abbrev-ref HEAD)/variant/debug-sanitizers/commit/$(git rev-parse HEAD)/
297+
235298
workflows:
236-
builds:
299+
gdevelop_js-wasm:
237300
jobs:
238301
- build-gdevelop_js-wasm-only
302+
gdevelop_js-wasm-extra-checks:
303+
jobs:
304+
- build-gdevelop_js-debug-sanitizers-and-extra-checks:
305+
# Extra checks are resource intensive so don't all run them.
306+
filters:
307+
branches:
308+
only:
309+
- master
310+
- /experimental-build.*/
311+
builds:
312+
jobs:
239313
- build-macos:
240314
filters:
241315
branches:

.clang-tidy

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Checks: 'clang-diagnostic-*,clang-analyzer-*,cppcoreguidelines-*,-cppcoreguidelines-explicit-virtual-functions,-cppcoreguidelines-avoid-const-or-ref-data-members,-cppcoreguidelines-special-member-functions,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-non-private-member-variables-in-classes,-cppcoreguidelines-owning-memory,-cppcoreguidelines-virtual-class-destructor,-clang-analyzer-optin.performance.Padding,-cppcoreguidelines-narrowing-conversions'
2+
WarningsAsErrors: 'cppcoreguidelines-pro-type-member-init, clang-analyzer-optin.cplusplus.UninitializedObject'
3+
HeaderFilterRegex: '.*'
4+
FormatStyle: none

CMakeLists.txt

+7-1
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,18 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
6969
# uninitialized variables or other hard to debug bugs.
7070
add_compile_options(
7171
-Wall
72+
-Wextra
73+
-Wuninitialized
74+
-Wconditional-uninitialized
7275
-Wno-unknown-warning-option
7376
-Wno-reorder-ctor
7477
-Wno-reorder
78+
-Wno-unused-parameter
7579
-Wno-pessimizing-move
76-
-Wno-unused-variable
80+
-Wno-unused-variable # Not a good style, but not a risk
7781
-Wno-unused-private-field
82+
-Wno-ignored-qualifiers # Not a risk
83+
-Wno-sign-compare # Not a big risk
7884

7985
# Make as much warnings considered as errors as possible (only one for now).
8086
-Werror=return-stack-address

Core/GDCore/Events/Builtin/WhileEvent.h

-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ class GD_CORE_API WhileEvent : public gd::BaseEvent {
7272
///< de/activate infinite loop warning when the
7373
///< user create the event
7474

75-
mutable unsigned int whileConditionsHeight;
76-
7775
int GetConditionsHeight() const;
7876
int GetActionsHeight() const;
7977
int GetWhileConditionsHeight() const;

Core/GDCore/Events/Parsers/ExpressionParser2Node.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ struct GD_CORE_API ExpressionParserLocation {
3636

3737
private:
3838
bool isValid;
39-
size_t startPosition;
40-
size_t endPosition;
39+
size_t startPosition = 0;
40+
size_t endPosition = 0;
4141
};
4242

4343
/**

Core/GDCore/Extensions/Metadata/BehaviorMetadata.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ BehaviorMetadata::BehaviorMetadata(
3737
className(className_),
3838
iconFilename(icon24x24),
3939
instance(instance_),
40-
sharedDatasInstance(sharedDatasInstance_),
41-
quickCustomizationVisibility(QuickCustomization::Visibility::Default) {
40+
sharedDatasInstance(sharedDatasInstance_) {
4241
SetFullName(gd::String(fullname_));
4342
SetDescription(gd::String(description_));
4443
SetDefaultName(gd::String(defaultName_));

Core/GDCore/Extensions/Metadata/BehaviorMetadata.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ class GD_CORE_API BehaviorMetadata : public InstructionOrExpressionContainerMeta
394394
bool isPrivate = false;
395395
bool isHidden = false;
396396
gd::String openFullEditorLabel;
397-
QuickCustomization::Visibility quickCustomizationVisibility;
397+
QuickCustomization::Visibility quickCustomizationVisibility = QuickCustomization::Visibility::Default;
398398

399399
// TODO: Nitpicking: convert these to std::unique_ptr to clarify ownership.
400400
std::shared_ptr<gd::Behavior> instance;

Core/GDCore/Extensions/Metadata/EffectMetadata.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ class GD_CORE_API EffectMetadata {
185185
gd::String fullname;
186186
gd::String description;
187187
std::vector<gd::String> includeFiles;
188-
bool isMarkedAsNotWorkingForObjects;
189-
bool isMarkedAsOnlyWorkingFor2D;
190-
bool isMarkedAsOnlyWorkingFor3D;
191-
bool isMarkedAsUnique;
188+
bool isMarkedAsNotWorkingForObjects = false;
189+
bool isMarkedAsOnlyWorkingFor2D = false;
190+
bool isMarkedAsOnlyWorkingFor3D = false;
191+
bool isMarkedAsUnique = false;
192192
std::map<gd::String, gd::PropertyDescriptor> properties;
193193
};
194194

Core/GDCore/Extensions/Metadata/EventMetadata.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ EventMetadata::EventMetadata(const gd::String &name_,
1919
: fullname(fullname_),
2020
description(description_),
2121
group(group_),
22-
instance(instance_) {
22+
instance(instance_),
23+
hasCustomCodeGenerator(false) {
2324
ClearCodeGenerationAndPreprocessing();
2425
if (instance) instance->SetType(name_);
2526
}

Core/GDCore/Extensions/Metadata/EventMetadata.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class GD_CORE_API EventMetadata {
8383
gd::String group;
8484

8585
std::shared_ptr<gd::BaseEvent> instance;
86-
bool hasCustomCodeGenerator;
86+
bool hasCustomCodeGenerator = false;
8787
std::function<gd::String(gd::BaseEvent& event,
8888
gd::EventsCodeGenerator& codeGenerator,
8989
gd::EventsCodeGenerationContext& context)>

Core/GDCore/Extensions/Metadata/MetadataProvider.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ class GD_CORE_API MetadataProvider {
288288
static EffectMetadata badEffectMetadata;
289289
static gd::InstructionMetadata badInstructionMetadata;
290290
static gd::ExpressionMetadata badExpressionMetadata;
291-
int useless; // Useless member to avoid emscripten "must have a positive
292-
// integer typeid pointer" runtime error.
291+
int useless = 0; // Useless member to avoid emscripten "must have a positive
292+
// integer typeid pointer" runtime error.
293293
};
294294

295295
} // namespace gd

Core/GDCore/Extensions/PlatformExtension.h

+42-45
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ class Object;
4040
class ObjectConfiguration;
4141
} // namespace gd
4242

43-
typedef std::function<std::unique_ptr<gd::ObjectConfiguration>()>
44-
CreateFunPtr;
43+
typedef std::function<std::unique_ptr<gd::ObjectConfiguration>()> CreateFunPtr;
4544

4645
namespace gd {
4746

@@ -51,25 +50,25 @@ namespace gd {
5150
*/
5251
class GD_CORE_API CompilationInfo {
5352
public:
54-
CompilationInfo() : informationCompleted(false){};
55-
virtual ~CompilationInfo(){};
53+
CompilationInfo() {};
54+
virtual ~CompilationInfo() {};
5655

57-
bool informationCompleted;
56+
bool informationCompleted = false;
5857

59-
bool runtimeOnly; ///< True if the extension was compiled for a runtime use
60-
///< only
58+
bool runtimeOnly = false; ///< True if the extension was compiled for a
59+
///< runtime use only
6160

6261
#if defined(__GNUC__)
63-
int gccMajorVersion;
64-
int gccMinorVersion;
65-
int gccPatchLevel;
62+
int gccMajorVersion = 0;
63+
int gccMinorVersion = 0;
64+
int gccPatchLevel = 0;
6665
#endif
6766

68-
int sfmlMajorVersion;
69-
int sfmlMinorVersion;
67+
int sfmlMajorVersion = 0;
68+
int sfmlMinorVersion = 0;
7069

7170
gd::String gdCoreVersion;
72-
int sizeOfpInt;
71+
int sizeOfpInt = 0;
7372
};
7473

7574
struct GD_CORE_API DuplicatedInstructionOptions {
@@ -239,11 +238,12 @@ class GD_CORE_API PlatformExtension {
239238
* \param instance The "blueprint" object to be copied when a new object is
240239
asked for.
241240
*/
242-
gd::ObjectMetadata& AddObject(const gd::String& name_,
243-
const gd::String& fullname_,
244-
const gd::String& description_,
245-
const gd::String& icon_,
246-
std::shared_ptr<gd::ObjectConfiguration> instance);
241+
gd::ObjectMetadata& AddObject(
242+
const gd::String& name_,
243+
const gd::String& fullname_,
244+
const gd::String& description_,
245+
const gd::String& icon_,
246+
std::shared_ptr<gd::ObjectConfiguration> instance);
247247

248248
/**
249249
* \brief Declare a new events based object as being part of the extension.
@@ -253,11 +253,10 @@ class GD_CORE_API PlatformExtension {
253253
* \param description The user friendly description of the object
254254
* \param icon The icon of the object.
255255
*/
256-
gd::ObjectMetadata& AddEventsBasedObject(
257-
const gd::String& name_,
258-
const gd::String& fullname_,
259-
const gd::String& description_,
260-
const gd::String& icon_);
256+
gd::ObjectMetadata& AddEventsBasedObject(const gd::String& name_,
257+
const gd::String& fullname_,
258+
const gd::String& description_,
259+
const gd::String& icon_);
261260

262261
/**
263262
* \brief Declare a new behavior as being part of the extension.
@@ -420,8 +419,7 @@ class GD_CORE_API PlatformExtension {
420419
PlatformExtension& SetTags(const gd::String& csvTags) {
421420
tags.clear();
422421
tags = csvTags.Split(',');
423-
for (size_t i = 0; i < tags.size(); i++)
424-
{
422+
for (size_t i = 0; i < tags.size(); i++) {
425423
tags[i] = tags[i].Trim().LowerCase();
426424
}
427425
return *this;
@@ -634,31 +632,30 @@ class GD_CORE_API PlatformExtension {
634632
*/
635633
static gd::String GetNamespaceSeparator() { return "::"; }
636634

637-
static gd::String GetEventsFunctionFullType(const gd::String &extensionName,
638-
const gd::String &functionName);
639-
640-
static gd::String
641-
GetBehaviorEventsFunctionFullType(const gd::String &extensionName,
642-
const gd::String &behaviorName,
643-
const gd::String &functionName);
635+
static gd::String GetEventsFunctionFullType(const gd::String& extensionName,
636+
const gd::String& functionName);
644637

645-
static gd::String GetBehaviorFullType(const gd::String &extensionName,
646-
const gd::String &behaviorName);
638+
static gd::String GetBehaviorEventsFunctionFullType(
639+
const gd::String& extensionName,
640+
const gd::String& behaviorName,
641+
const gd::String& functionName);
647642

648-
static gd::String
649-
GetObjectEventsFunctionFullType(const gd::String &extensionName,
650-
const gd::String &objectName,
651-
const gd::String &functionName);
643+
static gd::String GetBehaviorFullType(const gd::String& extensionName,
644+
const gd::String& behaviorName);
652645

653-
static gd::String GetObjectFullType(const gd::String &extensionName,
654-
const gd::String &objectName);
646+
static gd::String GetObjectEventsFunctionFullType(
647+
const gd::String& extensionName,
648+
const gd::String& objectName,
649+
const gd::String& functionName);
655650

651+
static gd::String GetObjectFullType(const gd::String& extensionName,
652+
const gd::String& objectName);
656653

657654
static gd::String GetExtensionFromFullObjectType(const gd::String& type);
658655

659656
static gd::String GetObjectNameFromFullObjectType(const gd::String& type);
660657

661-
private:
658+
private:
662659
/**
663660
* Set the namespace (the string all actions/conditions/expressions start
664661
* with).
@@ -673,10 +670,10 @@ class GD_CORE_API PlatformExtension {
673670
gd::String fullname; ///< Name displayed to users in the editor.
674671
gd::String informations; ///< Description displayed to users in the editor.
675672
gd::String category;
676-
gd::String author; ///< Author displayed to users in the editor.
677-
gd::String license; ///< License name displayed to users in the editor.
678-
bool deprecated; ///< true if the extension is deprecated and shouldn't be
679-
///< shown in IDE.
673+
gd::String author; ///< Author displayed to users in the editor.
674+
gd::String license; ///< License name displayed to users in the editor.
675+
bool deprecated; ///< true if the extension is deprecated and shouldn't be
676+
///< shown in IDE.
680677
gd::String helpPath; ///< The relative path to the help for this extension in
681678
///< the documentation.
682679
gd::String iconUrl; ///< The URL to the icon to be shown for this extension.

Core/GDCore/Extensions/PlatformExtension.inl

+4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* project is released under the MIT License.
66
*/
77

8+
// NOLINTBEGIN
9+
810
#ifndef GDCORE_PLATFORMEXTENSION_INL
911
#define GDCORE_PLATFORMEXTENSION_INL
1012

@@ -36,3 +38,5 @@ gd::ObjectMetadata& PlatformExtension::AddObject(const gd::String& name,
3638
} // namespace gd
3739

3840
#endif
41+
42+
// NOLINTEND

Core/GDCore/IDE/Events/ExpressionCompletionFinder.h

+8-8
Original file line numberDiff line numberDiff line change
@@ -337,19 +337,19 @@ struct GD_CORE_API ExpressionCompletionDescription {
337337

338338
private:
339339
CompletionKind completionKind;
340-
gd::Variable::Type variableType;
341-
gd::VariablesContainer::SourceType variableScope;
340+
gd::Variable::Type variableType = gd::Variable::Unknown;
341+
gd::VariablesContainer::SourceType variableScope = gd::VariablesContainer::Unknown;
342342
gd::String type;
343343
gd::String prefix;
344344
gd::String completion;
345-
size_t replacementStartPosition;
346-
size_t replacementEndPosition;
345+
size_t replacementStartPosition = 0;
346+
size_t replacementEndPosition = 0;
347347
gd::String objectName;
348348
gd::String behaviorName;
349-
bool isExact;
350-
bool isLastParameter;
351-
const gd::ParameterMetadata* parameterMetadata;
352-
const gd::ObjectConfiguration* objectConfiguration;
349+
bool isExact = false;
350+
bool isLastParameter = false;
351+
const gd::ParameterMetadata* parameterMetadata = &badParameterMetadata;
352+
const gd::ObjectConfiguration* objectConfiguration = &badObjectConfiguration;
353353

354354
static const gd::ParameterMetadata badParameterMetadata;
355355
static const gd::ObjectConfiguration badObjectConfiguration;

0 commit comments

Comments
 (0)