Skip to content

Commit

Permalink
Merge branch 'clang-tidy+synsmell'
Browse files Browse the repository at this point in the history
* clang-tidy+synsmell:
  UI: Makefile.mk: ui/lint: add checks with misc/synsmell.py
  DEVICES: Makefile.mk: devices/lint: add checks with misc/synsmell.py
  ASE: Makefile.mk: ase/lint: add checks with misc/synsmell.py
  MISC: synsmell.py: check syntax for code smells, based on regular expressions
  ASE: Makefile.mk: allow double loop variable tests in mathutils.cc
  ASE: *.cc: fix clang-tidy warnings
  DEVICES: saturation/Makefile.mk: remove -Iexternal/pandaresampler/lib
  DEVICES: blepsynth/Makefile.mk: remove -Iexternal/pandaresampler/lib
  ASE: Makefile.mk: move -Iexternal/pandaresampler/lib here
  MISC: Makefile.mk: clang-tidy: strip $PWD
  MISC: colorize.sh: fix black-on-black coloring
  MISC: Makefile.mk: fix lint-unused rule
  MISC: Makefile.mk: fix lint-cppcheck rule
  MISC: Makefile.mk: clang-tidy: support target specific .CTIDY_FLAGS
  MISC: Makefile.mk: add clang-tidy-check: false exit status on errors/warnings
  MISC: Makefile.mk: clang-tidy: skip empty reports during printout
  MISC: Makefile.mk: clang-tidy: add -march=x86-64-v2 so atomics checks pass

Signed-off-by: Tim Janik <[email protected]>
  • Loading branch information
tim-janik committed Jan 25, 2024
2 parents 4dcc915 + f6c8135 commit ceaf526
Show file tree
Hide file tree
Showing 18 changed files with 240 additions and 46 deletions.
11 changes: 7 additions & 4 deletions ase/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ ASE_EXTERNAL_INCLUDES := $(strip \
-Iexternal/libsndfile/include \
-Iexternal/rapidjson/include \
-Iexternal/websocketpp \
)
-Iexternal/pandaresampler/lib \
) # also used by clang-tidy
$(ase/AnklangSynthEngine.objects): $(ase/include.deps) $(ase/libase.deps)
$(ase/AnklangSynthEngine.objects): EXTRA_INCLUDES ::= $(ASE_EXTERNAL_INCLUDES) -I$> -I$>/external/ $(ASEDEPS_CFLAGS)
$(lib/AnklangSynthEngine): $>/lib/libsndfile.so | $>/lib/
Expand All @@ -151,6 +152,8 @@ $(call BUILD_PROGRAM, \
../lib)
# Work around legacy code in external/websocketpp/*.hpp
ase/websocket.cc.FLAGS = -Wno-deprecated-dynamic-exception-spec -Wno-sign-promo
# Allow tests in mathutils.cc
ase/mathutils.cc.CTIDY_FLAGS = --checks=-clang-analyzer-security.FloatLoopCounter

# == jackdriver.so ==
lib/jackdriver.so ::= $>/lib/jackdriver.so
Expand Down Expand Up @@ -192,9 +195,9 @@ $(call INSTALL_BIN_RULE, $(basename $(lib/AnklangSynthEngine)), $(DESTDIR)$(pkgd
))

# == ase/lint ==
ase/lint: tscheck eslint stylelint
-$Q { TCOLOR=--color=always ; tty -s <&1 || TCOLOR=; } \
&& grep $$TCOLOR -nE '(/[*/]+[*/ ]*)?(FI[X]ME).*' -r ase/
ase/lint:
$(QGEN)
$Q misc/synsmell.py $(wildcard ase/*.[hc] ase/*.*[hc] ase/*/*.*[hc] jsonipc/*.hh)
.PHONY: ase/lint
lint: ase/lint

Expand Down
5 changes: 3 additions & 2 deletions ase/clapplugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ class ClapAudioProcessor : public AudioProcessor {
convert_clap_events (processinfo, input_preferred_dialect & CLAP_NOTE_DIALECT_CLAP);
processinfo.steady_time += processinfo.frames_count;
const clap_process_status status = clapplugin_->process (clapplugin_, &processinfo);
(void) status;
bool need_wakeup = dequeue_events (n_frames);
for (const auto &e : output_events_)
need_wakeup |= apply_param_value_event (e.value);
Expand Down Expand Up @@ -631,7 +632,7 @@ ClapAudioProcessor::convert_clap_events (const clap_process_t &process, const bo
}

// == ClapPluginHandleImpl ==
class ClapPluginHandleImpl : public ClapPluginHandle {
class ClapPluginHandleImpl final : public ClapPluginHandle {
public:
static String clapid (const clap_host *host) { return Ase::clapid (host); }
String clapid () const { return ClapPluginHandle::clapid(); }
Expand Down Expand Up @@ -1603,7 +1604,7 @@ ClapPluginHandleImpl::show_gui()
gui_windowid = cwindow.x11;
}
}
if (gui_windowid) {
if (gui_windowid && plugin_gui) {
gui_visible_ = plugin_gui->show (plugin_);
CDEBUG ("%s: gui_show: %d\n", clapid(), gui_visible_);
if (!gui_visible_)
Expand Down
5 changes: 3 additions & 2 deletions ase/compress.cc
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ is_compressed (const String &input)

}

class StreamReaderZStd : public StreamReader {
class StreamReaderZStd final : public StreamReader {
StreamReaderP istream_;
std::vector<uint8_t> ibuffer_;
ZSTD_inBuffer zinput_ = { nullptr, 0, 0 };
Expand Down Expand Up @@ -317,7 +317,7 @@ stream_reader_zstd (StreamReaderP &istream)

static constexpr bool PRINT_ADAPTIVE = false;

class StreamWriterZStd : public StreamWriter {
class StreamWriterZStd final : public StreamWriter {
StreamWriterP ostream_;
std::vector<uint8_t> obuffer_;
String name_;
Expand Down Expand Up @@ -378,6 +378,7 @@ class StreamWriterZStd : public StreamWriter {
{
zal_++;
const size_t pret = ZSTD_CCtx_setParameter (cctx_, ZSTD_c_compressionLevel, zstd_adaptive_level[zal_].level);
(void) pret;
if (PRINT_ADAPTIVE) printerr ("ZSTD_compressStream2: size=%u level=%u: %s\n", current_total, zstd_adaptive_level[zal_].level, ZSTD_getErrorName (pret));
zinput.size = zinput.pos;
ret = ZSTD_compressStream2 (cctx_, &zoutput, &zinput, ZSTD_e_end);
Expand Down
4 changes: 2 additions & 2 deletions ase/crawler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ FileCrawler::list_entries ()
String cwdfile = cwd_ + "/";
for (struct dirent *de = readdir (dir); de; de = readdir (dir))
{
bool is_dir = de->d_type == DT_DIR;
bool is_reg = de->d_type == DT_REG;
bool is_dir; // = de->d_type == DT_DIR;
bool is_reg; // = de->d_type == DT_REG;
ssize_t size = -1;
int64 mtime = 0;
if (true) // (de->d_type == DT_LNK || de->d_type == DT_UNKNOWN)
Expand Down
2 changes: 1 addition & 1 deletion ase/crawler.hh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Ase {

class FileCrawler : public ObjectImpl, public virtual ResourceCrawler {
class FileCrawler final : public ObjectImpl, public virtual ResourceCrawler {
String cwd_;
const uint constraindir_ : 1;
const uint constrainfile_ : 1;
Expand Down
2 changes: 1 addition & 1 deletion ase/processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ AudioParams::install (const AudioParams::Map &params)
i = 0;
for (auto [id,p] : params)
new_parameters[i++] = p;
assert_return (i == count);
parameters = new_parameters;
assert_return (i == count);
// values
values = new double[count] (); // value-initialized per ISO C++03 5.3.4[expr.new]/15
for (size_t i = 0; i < count; i++)
Expand Down
2 changes: 1 addition & 1 deletion ase/project.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public:
void operator+= (const VoidF &func);
};

class ProjectImpl : public DeviceImpl, public virtual Project {
class ProjectImpl final : public DeviceImpl, public virtual Project {
std::vector<TrackImplP> tracks_;
ASE_DEFINE_MAKE_SHARED (ProjectImpl);
TickSignature tick_sig_;
Expand Down
6 changes: 3 additions & 3 deletions ase/storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ StorageReader::search_dir (const String &dirname)
StreamReader::~StreamReader()
{}

class StreamReaderFile : public StreamReader {
class StreamReaderFile final : public StreamReader {
FILE *file_ = nullptr;
String name_;
public:
Expand Down Expand Up @@ -585,7 +585,7 @@ stream_reader_from_file (const String &file)
return nullptr;
}

class StreamReaderZipMember : public StreamReader {
class StreamReaderZipMember final : public StreamReader {
void *reader_ = nullptr;
bool entry_opened_ = false;
String name_, member_;
Expand Down Expand Up @@ -686,7 +686,7 @@ stream_reader_zip_member (const String &archive, const String &member, Storage::
StreamWriter::~StreamWriter ()
{}

class FileStreamWriter : public StreamWriter {
class FileStreamWriter final : public StreamWriter {
String name_;
int fd_ = -1;
public:
Expand Down
1 change: 1 addition & 0 deletions ase/strings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1532,6 +1532,7 @@ text_convert (const String &to_charset,
optr = obuffer;
olength = sizeof (obuffer);
n = iconv (alt_cd, const_cast<char**> (&iptr), &ilength, &optr, &olength);
(void) n;
/* transfer output */
output_string.append (obuffer, optr - obuffer);
}
Expand Down
4 changes: 2 additions & 2 deletions ase/transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ AudioTransport::update_current ()
current_minutes = time.minutes;
current_seconds = time.seconds;

if (false && old_next != next_bar_tick)
if (old_next != next_bar_tick && false)
printerr ("%3d.%2d.%5.2f %02d:%06.3f frame=%d tick=%d next=%d bpm=%d sig=%d/%d ppqn=%d pps=%f rate=%d\n",
current_bar, current_beat, current_semiquaver,
current_minutes, current_seconds,
Expand Down Expand Up @@ -337,7 +337,7 @@ transport_tests()
int32 hminutes, hours = divmod (tt.minutes, 60, &hminutes);
TickSignature::Beat tb = ts.beat_from_tick (testtick);
TCMP (ts.bar_from_tick (testtick), ==, tb.bar);
if (false)
if (hours && false)
printerr ("%03d.%02d.%06.3f %02d:%02d:%06.3f tick=%d\n",
tb.bar, tb.beat, tb.semiquaver,
hours, hminutes, tt.seconds, testtick);
Expand Down
8 changes: 4 additions & 4 deletions ase/wave.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ wav_header (const uint8_t n_bits, const uint32_t n_channels, const uint32_t samp
char*
puts (const char *str)
{
strcpy (c, str);
strncpy (c, str, 1024);
c += strlen (str);
return c;
}
Expand Down Expand Up @@ -148,7 +148,7 @@ wav_write (int fd, uint8_t n_bits, uint32_t n_channels, uint32_t sample_freq, co
return 0;
}

class WavWriterImpl : public WaveWriter {
class WavWriterImpl final : public WaveWriter {
String filename_;
uint32_t n_channels_ = 0;
uint32_t sample_freq_ = 0;
Expand Down Expand Up @@ -247,7 +247,7 @@ wave_writer_opus_version()
return opus_get_version_string();
}

class OpusWriter : public WaveWriter {
class OpusWriter final : public WaveWriter {
String name_;
OpusEncoder *enc_ = nullptr;
int fd_ = -1;
Expand Down Expand Up @@ -513,7 +513,7 @@ wave_writer_flac_version()
return FLAC__VERSION_STRING;
}

class FlacWriter : public WaveWriter {
class FlacWriter final : public WaveWriter {
String name_;
FLAC__StreamEncoder *enc_ = nullptr;
FLAC__StreamMetadata *metadata_[2] = { nullptr, nullptr };
Expand Down
6 changes: 3 additions & 3 deletions devices/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ include $(wildcard $(devices/4ase.objects:.o=.o.d))
CLEANDIRS += $>/devices/

# == devices/lint ==
devices/lint: tscheck eslint stylelint
-$Q { TCOLOR=--color=always ; tty -s <&1 || TCOLOR=; } \
&& grep $$TCOLOR -nE '(/[*/]+[*/ ]*)?(FI[X]ME).*' -r devices/
devices/lint:
$(QGEN)
$Q misc/synsmell.py $(wildcard devices/*.*[hc] devices/*/*.*[hc])
.PHONY: devices/lint
lint: devices/lint
1 change: 0 additions & 1 deletion devices/blepsynth/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@ devices/4ase.ccfiles += $(strip \
devices/blepsynth/bleposcdata.cc \
devices/blepsynth/blepsynth.cc \
)
$>/devices/blepsynth/blepsynth.o: EXTRA_FLAGS ::= -Iexternal/pandaresampler/lib
1 change: 0 additions & 1 deletion devices/saturation/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@
devices/4ase.ccfiles += $(strip \
devices/saturation/saturation.cc \
)
$>/devices/saturation/saturation.o: EXTRA_FLAGS ::= -Iexternal/pandaresampler/lib
32 changes: 18 additions & 14 deletions misc/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,41 @@ clean-misc:
CPPCHECK ?= cppcheck
CPPCHECK_CCENABLE := warning,style,performance,portability
lint-cppcheck: $>/ls-tree.lst misc/Makefile.mk | $>/misc/cppcheck/
$Q egrep $(CLANGTIDY_GLOB) < $< > $>/misc/cppcheck/sources.lst
$Q $(CPPCHECK) --enable=$(CPPCHECK_CCENABLE) $(CPPCHECK_DEFS) \
$$(cat $>/misc/cppcheck/sources.lst)
$Q egrep "^(ase|devices|jsonipc|ui)/.*\.(cc|hh)$$" < $< > $>/misc/$@.lst
$Q $(CPPCHECK) --enable=$(CPPCHECK_CCENABLE) $(CPPCHECK_DEFS) $$(cat $>/misc/$@.lst) $(wildcard $>/ase/*.cc)
CPPCHECK_DEFS := -D__SIZEOF_LONG__=8 -D__SIZEOF_WCHAR_T__=4 -D__linux__ -U_SC_NPROCESSORS_ONLN -U_WIN32 -U__clang__
.PHONY: lint-cppcheck

# == lint-unused ==
lint-unused: $>/ls-tree.lst misc/Makefile.mk | $>/misc/cppcheck/
$Q egrep $(CLANGTIDY_GLOB) < $< > $>/misc/cppcheck/sources.lst
$Q $(CPPCHECK) --enable=unusedFunction,$(CPPCHECK_CCENABLE) $(CPPCHECK_DEFS) \
$$(cat $>/misc/cppcheck/sources.lst) 2>&1 | \
grep -E '(\bunuse|reach)' | sort | tee $>/misc/cppcheck/lint-unused.log
$Q egrep "^(ase|devices|jsonipc|ui)/.*\.(cc|hh)$$" < $< > $>/misc/$@.lst
$Q $(CPPCHECK) --enable=unusedFunction,$(CPPCHECK_CCENABLE) $(CPPCHECK_DEFS) $$(cat $>/misc/$@.lst) $(wildcard $>/ase/*.cc) \
|& grep --color=auto -E '(\b(un)?use|\bnever\b|\b(un)?reach)\w*'
.PHONY: lint-unused

# == clang-tidy ==
CLANG_TIDY_FILES = $(filter %.c %.cc %.C %.cpp %.cxx, $(LS_TREE_LST))
CLANG_TIDY_LOGS = $(patsubst %, $>/clang-tidy/%.log, $(CLANG_TIDY_FILES))
clang-tidy: $(CLANG_TIDY_LOGS)
clang-tidy clang-tidy-check: $(CLANG_TIDY_LOGS)
$(QGEN)
$Q for log in $(CLANG_TIDY_LOGS) ; do misc/colorize.sh < $$log >&2 ; done
$>/clang-tidy/%.log: % $(GITCOMMITDEPS) misc/Makefile.mk | $>/clang-tidy/
$Q OK=true \
&& for log in $(CLANG_TIDY_LOGS) ; do \
grep -Eq ': (error|warning):' $$log && OK=false ; \
test 1 -ge `wc -l < $$log` || \
sed "s|^$$PWD/||" < $$log | misc/colorize.sh >&2 ; \
done \
&& test $@ != clang-tidy-check || $$OK
$>/clang-tidy/%.log: % $(GITCOMMITDEPS) | $>/clang-tidy/
$(QECHO) CLANG-TIDY $@
$Q mkdir -p $(dir $@) && rm -f $>/clang-tidy/$<.*
$Q set +o pipefail \
&& CTIDY_FLAGS=( $(ASE_EXTERNAL_INCLUDES) $(CLANG_TIDY_DEFS) $($<.LINT_CCFLAGS) ) \
&& [[ $< = @(*.[hc]) ]] || CTIDY_FLAGS+=( -std=gnu++20 ) \
&& (set -x ; $(CLANG_TIDY) --export-fixes=$>/clang-tidy/$<.yaml $< $($<.LINT_FLAGS) -- "$${CTIDY_FLAGS[@]}" ) >$@~ 2>&1 || :
&& CTIDY_DEFS=( $(ASE_EXTERNAL_INCLUDES) $(CLANG_TIDY_DEFS) $($<.CTIDY_DEFS) -march=x86-64-v2 ) \
&& [[ $< = @(*.[hc]) ]] || CTIDY_DEFS+=( -std=gnu++20 ) \
&& (set -x ; $(CLANG_TIDY) --export-fixes=$>/clang-tidy/$<.yaml $< $($<.CTIDY_FLAGS) -- "$${CTIDY_DEFS[@]}" ) >$@~ 2>&1 || :
$Q mv $@~ $@
CLANG_TIDY_DEFS := -I. -I$> -isystem external/ -isystem $>/external/ -DASE_COMPILATION $(ASEDEPS_CFLAGS) $(GTK2_CFLAGS)
# File specific LINT_FLAGS, example: ase/jsonapi.cc.LINT_FLAGS ::= --checks=-clang-analyzer-core.NullDereference
jsonipc/testjsonipc.cc.LINT_CCFLAGS ::= -D__JSONIPC_NULL_REFERENCE_THROWS__
jsonipc/testjsonipc.cc.CTIDY_DEFS ::= -D__JSONIPC_NULL_REFERENCE_THROWS__
clang-tidy-clean:
rm -f -r $>/clang-tidy/
.PHONY: clang-tid clang-tidy-clean
Expand Down
9 changes: 5 additions & 4 deletions misc/colorize.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,21 @@ K=$'\e[30m' # black
R=$'\e[31m' # red
T=$'\e[36m' # turquoise
Y=$'\e[33m' # yellow
M=$'\e[35m' # magenta
Z=$'\e[0m' # reset

# colorization patterns
PAT=(
"s/(\[Warning\/[^]]*])/$B$Y\1$Z/;" # eslint warning
"s/ (warning:) / $B$Y\1$Z /;" # cc warning
"s/(\[Warning\/[^]]*])/$B$M\1$Z/;" # eslint warning
"s/ (warning:) / $B$M\1$Z /;" # cc warning
"s/(\[Error\/[^]]*])/$B$R\1$Z/;" # eslint error
"s/ (error:) / $B$R\1$Z /;" # cc error
"s/ (note:[^$e]+)/ $B$K\1$Z/;" # lint note
"s/ (note:[^$e]+)/ $B$Y\1$Z/;" # lint note
)

# Emacs does its own line coloring
test "${INSIDE_EMACS:-}" == "" && PAT+=(
"s/^([^ :$e]+):([0-9:]+:)? /$B$C\1$F:\2$Z /;" # file name
"s/^([^ :$e]+):([0-9:]+:)? /$B\1:\2$Z /;" # file name
)

stdbuf -eL -oL \
Expand Down
Loading

0 comments on commit ceaf526

Please sign in to comment.