Skip to content
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

Update reformatting settings, trace wrapping, and printf-format strings #22

Merged
merged 15 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,27 @@ AlignConsecutiveMacros: true
AlignConsecutiveAssignments: true
AlignConsecutiveDeclarations: true
AlwaysBreakAfterReturnType: AllDefinitions
# Can enable the following section when llvm 12.x is out
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep track of the versions somehow (There were some version 11 changes that would be nice).
It will likely depend on the version of clang on the ubuntu runner provided github.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably update the format_source script to detect the version of clang-format that's installed and use the correct formatting file. And maybe set a minimum version to use, so that our formatting is stable.

#AttributeMacros:
# - H5_ATTR_FORMAT
# - H5_ATTR_UNUSED
# - H5_ATTR_DEPRECATED_USED
# - H5_ATTR_NDEBUG_UNUSED
# - H5_ATTR_DEBUG_API_USED
# - H5_ATTR_PARALLEL_UNUSED
# - H5_ATTR_PARALLEL_USED
# - H5_ATTR_NORETURN
# - H5_ATTR_CONST
# - H5_ATTR_PURE
# - H5_ATTR_FALLTHROUGH
BraceWrapping:
AfterFunction: true
BeforeCatch: true
BeforeElse: true
BreakBeforeBraces: Stroustrup
BreakAfterJavaFieldAnnotations: true
BreakStringLiterals: true
ColumnLimit: 110
ColumnLimit: 110 # Update $max_trace_macro_line_len in bin/trace also
IncludeCategories:
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
Priority: 3
Expand All @@ -33,6 +46,8 @@ IncludeIsMainRegex: '(public)?$'
IndentCaseLabels: true
IndentGotoLabels: false
IndentWidth: 4
MacroBlockBegin: "^BEGIN_FUNC"
MacroBlockEnd: "^END_FUNC"
ObjCBlockIndentWidth: 4
ReflowComments: true
SortIncludes: false
Expand All @@ -58,5 +73,6 @@ StatementMacros:
- HMPI_GOTO_ERROR
- H5_GCC_DIAG_OFF
- H5_GCC_DIAG_ON
- CATCH
...

14 changes: 12 additions & 2 deletions .github/workflows/clang-format-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,17 @@ jobs:
uses: DoozyX/[email protected]
with:
source: '.'
extensions: 'c,h,cpp'
extensions: 'c,h,cpp,hpp'
clangFormatVersion: 10
style: file
exclude: './config ./hl/src/H5LTanalyze.c'
exclude:
- ./config
- ./hl/src/H5LTanalyze.c
- ./hl/src/H5LTparse.c
- ./hl/src/H5LTparse.h
- ./src/H5Epubgen.h
- ./src/H5Einit.h
- ./src/H5Eterm.h
- ./src/H5Edefin.h
- ./src/H5version.h
- ./src/H5overflow.h
24 changes: 22 additions & 2 deletions bin/format_source
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
#!/bin/bash
#
# Recursively format all C & C++ sources and header files, except those in the
# 'config' directory and generated files, such as H5LTanalyze.c, etc.
#
# Note that any files or directories that are excluded here should also be
# added to the 'exclude' list in .github/workflows/clang-format-check.yml
#
# (Remember to update both bin/format_source and bin/format_source_patch)

find . -type d \( -path ./config \) -prune \
-o -iname *.h -o -iname *.c -o -iname *.cpp -o -iname *.hpp \
-or \( \( \! \( \
-name H5LTanalyze.c \
-or -name H5LTparse.c \
-or -name H5LTparse.h \
-or -name H5Epubgen.h \
-or -name H5Einit.h \
-or -name H5Eterm.h \
-or -name H5Edefin.h \
-or -name H5version.h \
-or -name H5overflow.h \
\) \) \
-and \( -iname *.h -or -iname *.c -or -iname *.cpp -or -iname *.hpp \) \) \
| xargs clang-format -style=file -i -fallback-style=none

exit 0
exit 0
24 changes: 22 additions & 2 deletions bin/format_source_patch
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
#!/bin/bash
#
# Recursively format all C & C++ sources and header files, except those in the
# 'config' directory and generated files, such as H5LTanalyze.c, etc.
#
# Note that any files or directories that are excluded here should also be
# added to the 'exclude' list in .github/workflows/clang-format-check.yml
#
# (Remember to update both bin/format_source and bin/format_source_patch)

find . -type d \( -path ./config \) -prune \
-o -iname *.h -o -iname *.c -o -iname *.cpp -o -iname *.hpp \
-or \( \( \! \( \
-name H5LTanalyze.c \
-or -name H5LTparse.c \
-or -name H5LTparse.h \
-or -name H5Epubgen.h \
-or -name H5Einit.h \
-or -name H5Eterm.h \
-or -name H5Edefin.h \
-or -name H5version.h \
-or -name H5overflow.h \
\) \) \
-and \( -iname *.h -or -iname *.c -or -iname *.cpp -or -iname *.hpp \) \) \
| xargs clang-format -style=file -i -fallback-style=none

git diff > clang_format.patch
Expand All @@ -11,4 +31,4 @@ then
rm clang_format.patch
fi

exit 0
exit 0
38 changes: 31 additions & 7 deletions bin/trace
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,14 @@ $Source = "";
"ssize_t" => "Zs",
);


##############################################################################
# Maximum length of H5TRACE macro line
# If the ColumnLimit in .clang-format is changed, this value will need to be updated
#
my $max_trace_macro_line_len = 110;


##############################################################################
# Print an error message.
#
Expand Down Expand Up @@ -287,7 +295,7 @@ sub rewrite_func ($$$$$) {

# Parse arguments
if ($args eq "void") {
$trace = "H5TRACE0(\"$rettype\",\"\");\n";
$trace = "H5TRACE0(\"$rettype\", \"\");\n";
} else {
# Split arguments. First convert `/*in,out*/' to get rid of the
# comma, then split the arguments on commas.
Expand Down Expand Up @@ -331,18 +339,34 @@ sub rewrite_func ($$$$$) {
}
}
}

# Compose the trace macro
$trace = "H5TRACE" . scalar(@arg_str) . "(\"$rettype\", \"";
$trace .= join("", @arg_str) . "\"";
my $len = 4 + length $trace;
my $len = 4 + length $trace; # Add 4, for indenting the line
for (@arg_name) {
if ($len + length >= 77) {
$trace .= ",\n $_";
$len = 13 + length;
# Wrap lines that will be longer than the limit, after ');' is added
if ($len + length >= ($max_trace_macro_line_len - 2)) {
# Wrap line, with indention
$trace .= ",\n ";
$len = 13; # Set to 13, for indention

# Indent an extra space to account for extra digit in 'H5TRACE' macro
if (scalar(@arg_str) >= 10) {
$trace .= " ";
$len++;
}
} else {
$trace .= ", $_";
$len += 1 + length;
$trace .= ", ";
$len += 2; # Add 2, for ', '
}

# Append argument
$trace .= "$_";
$len += length; # Add length of appended argument name
}

# Append final ');' for macro
$trace .= ");\n";
}
goto error if grep {/!/} @arg_str;
Expand Down
Loading