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

Add 'warning density' computation to the warnhist script #3910

Merged
merged 2 commits into from
Dec 29, 2023

Conversation

qkoziol
Copy link
Contributor

@qkoziol qkoziol commented Dec 24, 2023

Add 'warning density' computation to the warnhist script, along with several cleanups to it. Add "--enable-show-all-warnings" configure (and CMake) option to disable compiler diagnostic suppression (and therefore show all the
otherwise suppressed compiler diagnostics), disabled by default. Clean up a buncn of misc. warnings.

…several

cleanups to it.   Add "--enable-show-all-warnings" configure (and CMake)
option to disable compiler diagnostic suppression (and therefore show all the
otherwise suppressed compiler diagnostics), disabled by default.  Clean up
a buncn of misc. warnings.

Signed-off-by: Quincey Koziol <qkoziol@amazon.com>
print "\t\t<file string list> is a comma separated list, with no spaces\n";
print "\t\tFor example: 'H5Fint' or 'H5Fint,H5Gnode'\n";
print "\t-l\tDisplay line numbers for file/warning\n";
print "\t-u\tLeave 'unique' types in warnings, instead of genericizing them\n";
print "\t-i <name list>\tIgnore named files, <name list>\n";
print "\t\t<name list> is a comma separated list, with no spaces\n";
print "\t\tFor example: 'H5LTparse' or 'H5LTparse,H5LTanalyze'\n";
print "\t-d\tCompute warning density for compiled source files. Paths to the\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Warning density computation requires knowing where the source code files are located (so that their lines of code can be computed, for the density calculation). So, I added the '-p ' option as well.

print "\t\tnecessary for just analyzing warnings in build output.\n";
print "\t\t<path list> is a comma separated list, with no spaces\n";
print "\t\tFor example: '/home/koziol/hdf5' or '.,~/dev/hdf5,~/dev/build'\n";
print "\t-q\tSuppress warning output\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also added a '-q' ("quiet") option to suppress warnings from the script itself.

@@ -67,29 +86,196 @@ sub do_help {
print "\t\t<n> can be a single value, a range, or a comma separated list\n";
print "\t\tFor example: '0' or '0,4' or '8-10' or '0,2-4,8-10,13'\n";
print "\t-F\tDisplay warnings for all files\n";
print "\t-s <warning string list>\tDisplay files for warnings which contain a string, <warning string list>\n";
print "\t-s <warning string list> Display files for warnings which contain a\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reflowed lines to display better, no text change.

print "\t\t<warning string list> is a comma separated list, with no spaces\n";
print "\t\tFor example: 'Wunused-dummy-argument' or 'Wunused-dummy-argument,Wunused-variable'\n";
print "\t-S <file string list>\tDisplay warnings for files which contain a string, <file string list>\n";
print "\t\tFor example: 'Wunused-dummy-argument' or\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reflowed lines to display better, no text change.

print "Usage: 'warnhist [-h, --help] [-t <prefix>] [-w <n>] [-W] [-f <n>] [-F] [-s <warning string list>] [-S <file string list] [-l] [-u] [-i <name list>] [file]'\n";
print "\t-h, --help\tDisplay this usage\n";
print "\t-t <prefix>\tTrim pathname prefix from filenames, <prefix>\n";
print "Usage: 'warnhist [-h, --help] [-t <prefix>] [-w <n>] [-W] [-f <n>] [-F]\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added new '-d', '-p', and '-q' options to command-line summary.

@@ -54,11 +59,25 @@ my $last_fort_name;
my $last_fort_line;
my $last_fort_offset;

# Info about source files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used for warning density calculation

@@ -22,8 +22,13 @@ use warnings;
# Perl modules/settings
use strict;
use Getopt::Std;
use File::Find;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used for warning density calculation

print "\tfile\tFilename containing build output\n";
print "\t\tIf no file is given, standard input is used.\n";
exit;
}

# Count # of lines in a file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Count lines in a [source] file.


# Recursively search a directory hierarchy for source files
# Adds results to the global %c_files, %cpp_files, and %fort_files hashes
sub parse_tree {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Store the paths to source file names in a directory hierarchy, for later use.

#print Dumper \%fort_files;
}

sub count_file_loc {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Locate source files, counting their lines of code. Handles multiple source files correctly, as long as the number of compiles equals the number of times a file of that name is found in the directory hierachy(s).

}
}

# Compute LOC for compiled source file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Determine the type of source file being compiled, then locate it in the source directory hierarchy(s), and count its lines of code.

}
}

sub sanity_check_loc {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make certain that the number of compiles of a source file name equals the number of files with that name.

sub main::HELP_MESSAGE {
do_help();
}

# declare the Perl command line flags/options we want to allow
my %options=();
getopts("FWhut:w:f:s:S:i:l", \%options);
getopts("FWhut:w:f:s:S:i:ldp:q", \%options);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add new command-line options.

@@ -185,6 +371,20 @@ if($options{u}) {
$genericize = 0;
}

# Scan source files, if warning density requested
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scan the source file paths if warning density is requested. (Note that '.' is used if no source file paths are given)

@@ -197,7 +397,7 @@ while (<>) {
my $extra2;

# Retain last FORTRAN compile line, which comes a few lines before warning
if($_ =~ /.*\.[fF]90:.*/) {
if($_ =~ /.*((\.inc)|(\.f90)|(\.f)|(\.f03)|(\.f08)|(\.f77)|(\.f95)|(\.for)|(\.fpp))\:.*/i) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Detect FORTRAN source & include files better.

@@ -212,17 +412,30 @@ while (<>) {
$last_c_name = $_;
}

# Compute LOC for compiled source files, if warning density requested
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Count lines-of-code for compiled files (lines that start with CC ..., FC ..., etc)

# Skip lines that don't have the word "warning"
next if $_ !~ /[Ww]arning/;

# Skip warnings from linker
next if $_ =~ /ld: warning:/;

# Skip warnings from make
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't count Makefile warnings as compile warnings

@@ -397,6 +605,11 @@ while (<>) {
$warning =~ s/=[A-Za-z_0-9]*\]/=-\]/g;
}

# Genericize C/C++ "No such file or directory" warnings into "-"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New warning to genericize.

@@ -449,7 +662,31 @@ while (<>) {
# print STDERR "warning = \"$warning\"\n";
}

print "Total unique [non-ignored] warnings: $totalcount\n";
# Sanity check compiled source files with multiple paths when computing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try to make certain that the # of compiles of source files with same name matches the number of times files with that name are found in the source directory hierarchy(s).

# Display results
#

print "\nTotal unique [non-ignored] warnings: $totalcount\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add warning density output.

@@ -484,7 +721,7 @@ for my $x (sort {$warn_count{$b} <=> $warn_count{$a}} keys(%warn_count)) {
$match = 1;
}

if($match) {
if($match || exists $options{W}) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix: All warnings should be expanded with '-W'.

@@ -522,7 +759,7 @@ for my $x (sort {$file_count{$b} <=> $file_count{$a}} keys(%file_count)) {
$match = 1;
}

if($match) {
if($match || exists $options{F}) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix: All files should be expanded with '-F'.

@@ -594,6 +594,17 @@ if (HDF5_ENABLE_CODESTACK)
endif ()
MARK_AS_ADVANCED (HDF5_ENABLE_CODESTACK)

# ----------------------------------------------------------------------
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add new CMake option to disable diagnostic suppression.

@@ -2311,6 +2311,45 @@ case "X-$DEV_WARNINGS" in
;;
esac

## ----------------------------------------------------------------------
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add new autotools option to disable diagnostic suppression.

@@ -28,8 +28,8 @@
#endif

! Define if on APPLE
#cmakedefine01 H5_HAVE_DARWIN
#if H5_HAVE_DARWIN == 0
#cmakedefine01 CMAKE_H5_HAVE_DARWIN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix typo that was generating warning.

@@ -727,7 +727,7 @@ H5_DLL herr_t H5D__chunk_allocated(const H5D_t *dset, hsize_t *nbytes);
H5_DLL herr_t H5D__chunk_allocate(const H5D_t *dset, bool full_overwrite, const hsize_t old_dim[]);
H5_DLL herr_t H5D__chunk_file_alloc(const H5D_chk_idx_info_t *idx_info, const H5F_block_t *old_chunk,
H5F_block_t *new_chunk, bool *need_insert, const hsize_t *scaled);
H5_DLL void *H5D__chunk_mem_alloc(size_t size, void *pline);
H5_DLL void *H5D__chunk_mem_alloc(size_t size, void *pline) H5_ATTR_MALLOC;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -389,7 +389,7 @@ H5_DLL herr_t H5EA__destroy_flush_depend(H5AC_info_t *parent_entry, H5AC_info_t
H5_DLL H5EA_hdr_t *H5EA__hdr_alloc(H5F_t *f);
H5_DLL herr_t H5EA__hdr_init(H5EA_hdr_t *hdr, void *ctx_udata);
H5_DLL haddr_t H5EA__hdr_create(H5F_t *f, const H5EA_create_t *cparam, void *ctx_udata);
H5_DLL void *H5EA__hdr_alloc_elmts(H5EA_hdr_t *hdr, size_t nelmts);
H5_DLL void *H5EA__hdr_alloc_elmts(H5EA_hdr_t *hdr, size_t nelmts) H5_ATTR_MALLOC;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -75,7 +75,7 @@ typedef int (*H5ES_list_iter_func_t)(H5ES_event_t *ev, void *ctx);
/******************************/
/* Package Private Prototypes */
/******************************/
H5_DLL H5ES_t *H5ES__create(void);
H5_DLL H5ES_t *H5ES__create(void) H5_ATTR_MALLOC;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -59,7 +59,10 @@ typedef struct H5I_id_info_t {
hid_t id; /* ID for this info */
unsigned count; /* Ref. count for this ID */
unsigned app_count; /* Ref. count of application visible IDs */
const void *object; /* Pointer associated with the ID */
union {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allow an ID's object to be treated as 'const' or not 'const' as needed, avoiding warnings / diagnostic suppression noise in H5I code.

@@ -175,7 +175,7 @@ H5_DLL herr_t H5P_insert(H5P_genplist_t *plist, const char *name, size_t size, v
H5_DLL herr_t H5P_remove(H5P_genplist_t *plist, const char *name);
H5_DLL htri_t H5P_exist_plist(const H5P_genplist_t *plist, const char *name);
H5_DLL htri_t H5P_class_isa(const H5P_genclass_t *pclass1, const H5P_genclass_t *pclass2);
H5_DLL char *H5P_get_class_name(H5P_genclass_t *pclass);
H5_DLL char *H5P_get_class_name(H5P_genclass_t *pclass) H5_ATTR_MALLOC;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -870,7 +870,7 @@ H5_DLL herr_t H5T__enum_insert(const H5T_t *dt, const char *name, const void *va
H5_DLL herr_t H5T__get_member_value(const H5T_t *dt, unsigned membno, void *value);

/* Field functions (for both compound & enumerated types) */
H5_DLL char *H5T__get_member_name(H5T_t const *dt, unsigned membno);
H5_DLL char *H5T__get_member_name(H5T_t const *dt, unsigned membno) H5_ATTR_MALLOC;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -23,7 +23,7 @@ static const char *FILENAME[] = {"event_set_1", NULL};
static hid_t connector_ids_g[EVENT_SET_NUM_CONNECTOR_IDS];

herr_t fake_wait_request_wait(void *req, uint64_t timeout, H5VL_request_status_t *status);
herr_t fake_wait_request_free(void *req);
H5_ATTR_CONST herr_t fake_wait_request_free(void *req);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -497,8 +497,16 @@
#define H5_DIAG_DO_PRAGMA(x) _Pragma(#x)
#define H5_DIAG_PRAGMA(x) H5_DIAG_DO_PRAGMA(GCC diagnostic x)

/* Allow suppression of compiler diagnostics unless H5_SHOW_ALL_WARNINGS is
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the 'show all warnings' build option is used, don't suppress compiler diagnostics.

int32_t swap_int32(int32_t val);
int64_t swap_int64(int64_t val);
uint64_t swap_uint64(uint64_t val);
H5_ATTR_CONST uint16_t swap_uint16(uint16_t val);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add attribute to address warning.

@@ -29,7 +29,6 @@
/* in detailed logging */
#define HEXDUMP_WRITEDATA 0 /* Toggle whether to print bytes to write */
/* in detailed logging */
#define LISTENQ 80 /* max pending Driver requests */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused.

@bmribler bmribler added Type - New Feature Add a new API call, functionality, or tool Type - Improvement Improvements that don't add a new feature or functionality and removed Type - New Feature Add a new API call, functionality, or tool labels Dec 27, 2023
@lrknox
Copy link
Collaborator

lrknox commented Dec 28, 2023

@derobins, @brtnfld, @epourmal, this PR needs a review from one of you as Fortran code owner.
Never mind. The only Fortran change was to correct a CMake variable name. I'm going to merge it.

@lrknox lrknox merged commit 3a21ee0 into HDFGroup:develop Dec 29, 2023
@qkoziol qkoziol deleted the warnhist_loc branch December 29, 2023 14:49
brtnfld pushed a commit to brtnfld/hdf5 that referenced this pull request Dec 31, 2023
* Add 'warning density' computation to the warnhist script, along with several
cleanups to it.   Add "--enable-show-all-warnings" configure (and CMake)
option to disable compiler diagnostic suppression (and therefore show all the
otherwise suppressed compiler diagnostics), disabled by default.  Clean up
a buncn of misc. warnings.

Signed-off-by: Quincey Koziol <qkoziol@amazon.com>
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Jan 4, 2024
* Add 'warning density' computation to the warnhist script, along with several
cleanups to it.   Add "--enable-show-all-warnings" configure (and CMake)
option to disable compiler diagnostic suppression (and therefore show all the
otherwise suppressed compiler diagnostics), disabled by default.  Clean up
a buncn of misc. warnings.

Signed-off-by: Quincey Koziol <qkoziol@amazon.com>
lrknox added a commit that referenced this pull request Jan 8, 2024
* Fix build error on freebsd (#3883)

Fixes:

checking for config freebsd12.1... no
checking for config freebsd... found
compiler '/home/svcpetsc/petsc-hash-pkgs/39f577/bin/mpicc' is GNU gcc-9.2.0
compiler '/home/svcpetsc/petsc-hash-pkgs/39f577/bin/mpif90' is GNU gfortran-9.2.0
stdout: .: cannot open ./config/classic-fflags: No such file or directory

* Correct CMake command and example packaging (#3888)

* Feat: Hashpin sensitive dependencies on GitHub Actions and enable Dependabot to update them monthly (#3892)

* feat: hashpin sensitive dependencies on GHAs

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* feat: enable dependabot for monthly updates on GHA

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

---------

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* Some changes to portal links when they could be found on docs.hdfgroup.org, and changed the helpdesk link to help.hdfgroup.org (#3893)

* Updated some portal links to go directly to docs.hdfgroup. 

* Fixed some portal and help desk links

* Add variable option syncing for examples (#3885)

* Add period(.) at the end of the sentence for consistency. (#3897)

* Remove redundant backslash character from comment. (#3899)

* Disable doxygen as errors for netcdf (#3900)

* disable building doxygen for netcdf test

* Doc versions (#3903)

* Added missing \since tags to H5D.

* Committing clang-format changes

* Fixed H5T version info.

* Committing clang-format changes

* Added missing version info to H5E.

* Committing clang-format changes

* Added version info to H5F public APIs.

* Committing clang-format changes

* Added missing H5Z public API version info.

* Added missing version info to H5G public APIs

* Added missing version info to H5I public API.

* Added missing version info to H5 public APIs

* Committing clang-format changes

* Added missing version info to H5P public APIs

* Added missing version info to H5R public APIs

* Fix comment error.

* Committing clang-format changes

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Change Trouble Shooting to Troubleshooting (#3905)

* Implement optimized support for vector I/O in Subfiling VFD (#3896)

Vector I/O requests are now processed within a single
set of I/O call batches, rather than each I/O vector
entry (tuple constructed from the types, addrs, sizes
and bufs arrays) being processed individually. This allows I/O to be
more efficiently parallelized among the I/O concentrator processes
during large I/O requests.

* Fixed some calculations and add test cases for issues spotted from review

* Removed a variable that was compensating for previous miscalculations

* Add 'warning density' computation to the warnhist script (#3910)

* Add 'warning density' computation to the warnhist script, along with several
cleanups to it.   Add "--enable-show-all-warnings" configure (and CMake)
option to disable compiler diagnostic suppression (and therefore show all the
otherwise suppressed compiler diagnostics), disabled by default.  Clean up
a buncn of misc. warnings.

Signed-off-by: Quincey Koziol <qkoziol@amazon.com>

* Added H5Fdelete_f with test (#3912)

* New Fortran Examples added (#3916)

* added subfiling example

* Added filtered writes with no selection example

* Version and space corrections.

* Restore H5_VERSION definition in configure.ac.

* renamed defined H5_VERS* to avoid conflicts (#3926)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants