-
Notifications
You must be signed in to change notification settings - Fork 684
Replace vera with clang-format #4518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note: depends on #4515. Moreover, if the style change is accepted by the maintainers, I'll update the coding standards documentation. |
360b693
to
3f05c58
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some, feedback. I surely haven't exhaustively reviewed all changed files.
.clang-format
Outdated
BreakBeforeBraces: Custom | ||
BreakBeforeTernaryOperators: true | ||
BreakStringLiterals: true | ||
ColumnLimit: 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is indeed current style. Driving me crazy sometimes. Is this the right time to discuss it? (Or is this about mimicking existing formatting without any changes?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the auto generated gnu-stlye descriptor with jerry-style modifications. I didn't take effort on unify the style in the descriptor but once the approach is accepted, I'll definitely update it as well.
I like this idea, maybe we need python script to format files automatically |
a79307e
to
f0ba049
Compare
10717ab
to
515b387
Compare
snapshot_buffer_size, | ||
globals_p); | ||
uint32_t offset = | ||
static_snapshot_add_compiled_code (bytecode_p, snapshot_buffer_p, snapshot_buffer_size, globals_p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks incorrect syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer.
@@ -535,8 +527,7 @@ jerry_snapshot_set_offsets (uint32_t *buffer_p, /**< buffer */ | |||
JERRY_ASSERT ((code_size % sizeof (uint32_t)) == 0); | |||
buffer_p += code_size / sizeof (uint32_t); | |||
size -= code_size; | |||
} | |||
while (size > 0); | |||
} while (size > 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks incorrect syntax.
@@ -439,8 +432,7 @@ static_snapshot_add_compiled_code (const ecma_compiled_code_t *compiled_code_p, | |||
|
|||
while (literal_start_p < (ecma_value_t *) buffer_p) | |||
{ | |||
if (!ecma_is_value_direct_string (*literal_start_p) | |||
&& !ecma_is_value_empty (*literal_start_p)) | |||
if (!ecma_is_value_direct_string (*literal_start_p) && !ecma_is_value_empty (*literal_start_p)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is moved to one line? These are often harder to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it fits into one line.
@@ -928,8 +905,7 @@ jerry_exec_snapshot (const uint32_t *snapshot_p, /**< snapshot */ | |||
|
|||
const jerry_snapshot_header_t *header_p = (const jerry_snapshot_header_t *) snapshot_data_p; | |||
|
|||
if (header_p->magic != JERRY_SNAPSHOT_MAGIC | |||
|| header_p->version != JERRY_SNAPSHOT_VERSION | |||
if (header_p->magic != JERRY_SNAPSHOT_MAGIC || header_p->version != JERRY_SNAPSHOT_VERSION | |||
|| !snapshot_check_global_flags (header_p->global_flags)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect syntax.
@@ -99,7 +101,7 @@ ecma_dealloc_object (ecma_object_t *object_p) /**< object to be freed */ | |||
* | |||
* @return pointer to allocated memory | |||
*/ | |||
extern inline ecma_extended_object_t * JERRY_ATTR_ALWAYS_INLINE | |||
extern inline ecma_extended_object_t *JERRY_ATTR_ALWAYS_INLINE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since it's a marco, not a real __attribute
it won't be aligned as before.
@@ -20,114 +20,114 @@ | |||
/** | |||
* Error message, if an argument is has an error flag | |||
*/ | |||
const char * const ecma_error_value_msg_p = "Argument cannot be marked as error"; | |||
const char* const ecma_error_value_msg_p = "Argument cannot be marked as error"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect syntax.
#include "ecma-extended-info.h" | ||
|
||
#include "ecma-helpers.h" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These newlines are unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's valid.
Include order separated to incluce blocks:
- own header
- system header
- jerry* header
- ecma* header
- everything else.
|
||
#if defined (__GNUC__) || defined (__clang__) | ||
#define ECMA_UINT128_DIV10(name) \ | ||
{ \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect syntax, the slash should be kept in its place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slash is aligned no the longest line of the macro, so still correct.
dadafd9
to
6db5106
Compare
JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]
6db5106
to
25b5003
Compare
The patch is ready, the 08.CODING-STARDARDS.MD needs to be updated in a follow up patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.