Related to now merged #4181 and some warnings it produces during compilation on macOS now.
In addition, there are some general "unused variable" warning eliminations. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/4228
-- Commit Summary --
* regex: Eliminate K&R function parameter syntax to avoid warnings * Define __USE_GNU for compiling all ctags sources * Define __USE_GNU inside tm_ctags.c * Remove unused variables * Avoid warning about unused variable when using quartz
-- File Changes --
M ctags/Makefile.am (3) M ctags/gnu_regex/regcomp.c (19) M ctags/gnu_regex/regexec.c (80) M ctags/parsers/geany_lcpp.c (2) M meson.build (9) M src/filetypes.c (2) M src/socket.c (2) M src/tagmanager/tm_ctags.c (2)
-- Patch Links --
https://github.com/geany/geany/pull/4228.patch https://github.com/geany/geany/pull/4228.diff
@b4n commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
I'm a bit worried by this, esp. when I see stuff like https://gcc.gnu.org/legacy-ml/fortran/2005-10/msg00365.html (yes it's old, but `__USE_GNU` doesn't seem trusty either).
What is the problem if only defining when building the `gnu_regex` library exactly? I admittedly didn't test, but I'm worried that in can have very unexpected impact on many system headers, potentially even breaking them (as IIUC this is something that should usually not appear if `_GNU_SOURCE` is not defined), so I'd be overly cautious with this. And I don't see what *should* be a problem: *regex.h* seem to check if it's defined or not properly, so not having it seems fine, as what it guards also seems to only be used internally.
Anyway, color me worried, so I'd really like to have all the details :-)
@techee commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
What is the problem if only defining when building the gnu_regex library exactly?
I was trying to eliminate the warnings below - these warnings come from the `regex.h` header which is included by other ctags sources (not the gnu_regex ones).
But we could also define `__USE_GNU` within `regex.h` itself if you are worried it could do something unexpected elsewhere.
``` CC dsl/optscript.lo CC dsl/es.lo CC main/args.lo CC libreadtags/readtags.lo CC main/colprint.lo CC main/CommonPrelude.lo CC main/debug.lo CC main/dependency.lo CC main/entry.lo In file included from dsl/es.c:35: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/dependency.c:18: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/debug.c:24: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/entry.c:56: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ CC main/entry_private.lo 1 warning generated. CC main/error.lo 1 warning generated. CC main/field.lo CC main/flags.lo CC main/fmt.lo In file included from main/entry_private.c:13: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ 1 warning generated. CC main/htable.lo In file included from main/field.c:27: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ CC main/keyword.lo CC main/kind.lo CC main/lregex.lo 1 warning generated. 1 warning generated. CC main/lregex-default.lo CC main/lxpath.lo 1 warning generated. CC main/main.lo CC main/mbcs.lo In file included from main/kind.c:24: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/lregex.c:38: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ CC main/mio.lo CC main/nestlevel.lo In file included from main/lregex-default.c:19: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ 1 warning generated. In file included from main/lxpath.c:16: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/main.c:61: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ 1 warning generated. 1 warning generated. CC main/numarray.lo CC main/objpool.lo CC main/options.lo 1 warning generated. CC main/param.lo CC main/parse.lo CC main/portable-scandir.lo CC main/promise.lo CC main/ptag.lo CC main/ptrarray.lo In file included from main/options.c:32: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/parse.c:32: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ main/options.c:2818:15: warning: cast to smaller integer type 'xtagType' (aka 'enum eXtagType') from 'bool *' [-Wpointer-to-enum-cast] 2818 | xtagType t = (xtagType)option->pValue; | ^~~~~~~~~~~~~~~~~~~~~~~~ CC main/rbtree.lo CC main/read.lo CC main/repoinfo.lo In file included from main/promise.c:14: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/ptag.c:21: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ 1 warning generated. 1 warning generated. CC main/routines.lo CC main/seccomp.lo CC main/script.lo 1 warning generated. In file included from main/read.c:28: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ CC main/selectors.lo CC main/sort.lo 2 warnings generated. CC main/stats.lo CC main/strlist.lo 1 warning generated. CC main/tokeninfo.lo CC main/trace.lo In file included from main/selectors.c:17: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ CC main/trashbox.lo CC main/unwindi.lo CC main/utf8_str.lo CC main/vstring.lo 1 warning generated. CC main/writer-ctags.lo CC main/writer-etags.lo CC main/writer-json.lo CC main/writer-xref.lo CC main/writer.lo CC main/xtag.lo CC optlib/forth.lo In file included from main/writer-ctags.c:17: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ In file included from main/writer-etags.c:19: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ CC optlib/meson.lo 1 warning generated. 1 warning generated. CC optlib/scss.lo CC parsers/cxx/cxx.lo CC parsers/cxx/cxx_debug.lo CC parsers/cxx/cxx_debug_type.lo In file included from main/xtag.c:18: In file included from main/parse_p.h:17: In file included from main/lregex_p.h:27: ./gnu_regex/regex.h:367:3: warning: declaration does not declare anything [-Wmissing-declarations] 367 | unsigned long int __REPB_PREFIX(used); | ^~~~~~~~~~~~~~~~~ ```
@b4n commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
Funny one :) It's a clash with the `__used` "attribute" I'd say.
Anyway, what about this instead, as we patch this old code anyway? ```diff diff --git a/ctags/gnu_regex/regex.h b/ctags/gnu_regex/regex.h index 213277215..12a5eca8a 100644 --- a/ctags/gnu_regex/regex.h +++ b/ctags/gnu_regex/regex.h @@ -350,7 +350,7 @@ typedef enum #ifdef __USE_GNU # define __REPB_PREFIX(name) name #else -# define __REPB_PREFIX(name) __##name +# define __REPB_PREFIX(name) __priv_##name #endif
struct re_pattern_buffer ```
@techee commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
Yes, this works.
Though maybe the "correct" way is to have `__USE_GNU` defined consistently - when it's defined for the `gnu_regex` library, it should be defined also for users of the library otherwise there might be incompatible declarations coming from the header (not sure if it's really the case, seems to work fine).
@b4n commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
I'm not sure `__USE_GNU` is really necessary in the interface, it seems to guard flags and private members, possibly only actually useful when actually building glibc/gnulib. It guards the `RE_*` macros, but we have no use of them outside gnu_regex itself, it hides some members with a prefix, and some clear GNU extensions we don't need. Nothing looks like it changes behavior depending on the flag, just whether some symbols are declared or not.
So I'd prefer applying this patch to avoid unexpected effects of setting it in other places.
@techee commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
OK, let's do it the way you suggest.
@techee pushed 3 commits.
eddaf38690b7e0119e46b5576f5ac0cd3e5ea221 Remove unused variables 265e32cc805d84db2f73b36325979c93ed791686 Avoid warning about unused variable when using quartz 1966593193dffdd3d4c908b6fea42bffd7318c66 Fix clang warning in regex.h
@techee commented on this pull request.
EXTRA_DIST = \
gnu_regex/README.txt
libctags_la_LIBADD += libgnu_regex.la -AM_CPPFLAGS += -I$(srcdir)/gnu_regex +AM_CPPFLAGS += -I$(srcdir)/gnu_regex -D__USE_GNU
OK, let's do it the way you suggest.
Done. And enough with coding for today :-).
Merged #4228 into master.
github-comments@lists.geany.org