lib/gvpr: use GVDLL instead of _WIN32 to control gprDisc init in compile.c
Without this change, an upcoming commit that changes cgraph.h to use
GVDLL instead of _WIN32 to control storage-class attributes would
cause this error:
compile.c:85:29: error: initializer element is not constant
85 | static Agdisc_t gprDisc = { &AgMemDisc, &AgIdDisc, &gprIoDisc };
| ^
compile.c:85:29: note: (near initialization for ‘gprDisc.mem’)
Towards https://gitlab.com/graphviz/graphviz/-/issues/2173.
Similar to the preceding changes for `sources`, it is not clear to me why this
code was using the `targets` array when the output values are irrelevant.
It is not clear to me why this loop was using the `sources` array. AFAICT the
values written to this array are (1) not used by subsequent loop iterations (no
loop-carried dependency) and (2) not used after the loop before being
overwritten by other values.
This change cuts the dependency. A future commit will refactor the allocation in
this area to avoid allocating a possibly-unused array upfront.
lib/mingle: rephrase some open coded hypotenuse calculations
Surprisingly every `sqrt` call in this file except one is a hypotenuse
calculation that can be done more accurately. Maybe `hypot` was not widely
available when this code was written.
lib/mingle already has a C++ source, requiring it to link against the C++
standard library. There is no advantage to keeping some of its sources in C.
This does nothing fancy, just rename the existing file. All contained code looks
compatible across C and C++. Future commits will refactor this to more modern
C++ style.
When moving this code to C++ in an upcoming commit, this would otherwise
trigger:
lib/mingle/ink.cpp:46:22: error: uninitialized const 'Origin' [-fpermissive]
static const point_t Origin;
^
In file included from lib/mingle/ink.cpp:17:0:
lib/mingle/ink.h:19:16: note: 'const struct point_t' has no user-provided
default constructor
typedef struct {
^
lib/mingle/ink.h:20:10: note: and the implicitly-defined constructor does not
initialize 'double point_t::x'
double x, y;
^
This noisy initialization can probably be undone after mingle and libmingle are
moved to entirely C++.
Code in routespl.c used a long-lived buffer, `ps`, that it resized to deal with
various operations. This made the code thread-unsafe, and note that there are
also various work arounds in routespl.c to deal with re-entrancy which is
complicated by this design.
This commit rephrases `simpleSplineRoute` and `_routesplines` to dynamically
allocate a fresh buffer for each operation. The ergonomics cost of this is that
callers of `simpleSplineRoute`, `routesplines`, and `routepolylines` now need to
eventually free the returned pointer. This is unfortunately fairly noisy to deal
with in C and would have been cleaner in C++.
In future, this direction should probably be taken further to allow
`routesplinesinit` and `routesplinesterm` to be removed. This design that relies
on static globals is incompatible with multi-threading, so if Graphviz ever
wants to support multi-threading, this will need to be removed.
Note that this commit introduce two new warnings (for the `calloc` calls with
`int` values). The cost seems worth it and hopefully these warnings can be
removed in future.
This change has a performance impact. For smaller graphs, this likely slightly
degrades performance due to repeated small allocations that previously would not
have been required. For larger graphs, this perhaps improves performance as the
allocator no longer (spuriously) believes it needs to retain the contents of
`ps` across calls.
poly_gencode: remove claim that 'filled' is a boolean
This variable is not treated as a boolean within this function. Declaring it as
a boolean appears to have been a mistake and only worked out coincidentally as
all set values fit within the `boolean` type.
cdt: MSBuild: define GVDLL to ensure correct storage-class attributes
An upcoming commit will change the storage-class attributes to be
controlled by the GVDLL symbol in cdt.h.
Without this change, the following error would occur:
LINK : fatal error LNK1104: cannot open file 'C:\GitLab-Runner\builds\magjac\graphviz\Debug\Graphviz\bin\cdt.lib' [C:\GitLab-Runner\builds\magjac\graphviz\lib\pathplan\Pathplan.vcxproj]
Towards https://gitlab.com/graphviz/graphviz/-/issues/2173.
Magnus Jacobsson [Sun, 12 Dec 2021 11:27:44 +0000 (12:27 +0100)]
sparse: MSBuild: add windows include dir to find config.h
An upcoming commit which will include config.h in colorprocs.h would
otherwise cause this error:
C:\Users\magja\graphviz\lib\common\colorprocs.h(13,10): fatal error C1083: Cannot open include file: 'config.h': No such file or directory [C:\Users\magja\graphviz\lib\sparse\gvsparse.vcxproj]
cmd/mingle depends on lib/mingle, which in turn depends on libANN, a C++
library. Thus the `mingle` binary needs to link against the C++ standard
library. There is no advantage to Mingle being implemented in C, and some nice
advantages to it being implemented in C++ instead.
This commit does a basic rename, as AFAICT nothing in minglemain.c has varying
semantics between C and C++. Future commits will use C++ features to simplify
the code.
This has no effect at present. However, when moving this code to C++ in an
upcoming commit, it would trigger -fpermissive problems. This change
pre-emptively prevents this issue.
mingle: cast 'name' parameter values in 'agattr' calls
This has no effect at present. However, when moving this code to C++ in an
upcoming commit, it would trigger -Wwrite-strings warnings. This change
pre-emptively prevents these warnings.
This makes the intent of the code slightly clearer, but also pre-emptively
avoids an upcoming -Wwrite-strings warning when converting this code to C++.
The functions removed in this commit were intended to be aliases for the
same-named macros declared in cgraph.h. The claimed purpose is debugger
introspection.
It is unclear to me what value these have in a modern development environment.
Some modern debuggers can understand macro definitions. Those that cannot are
_impeded_ by these aliases. A developer debugging looks at the alias definition
and thinks they understand the code, but they do not. The code that is executing
on their system is _not_ the code these functions compile to. To state something
stronger: it is unclear to me how these definitions were ever helpful. They
appear to (accidentally) obscure what is actually occurring in the machine at
runtime.
Note that these functions also bloated the cgraph .so/.a size by including
functions that were never called.
These functions were introduced in c9b7b0719776e022d760195fae2727b84be82eda to
match `AGMKOUT` and `AGMKIN` (#1212). But there are no exposed prototypes for
these. So the only way to call them is to externally prototype them with a
best-guess type signature and hope Graphviz never changes. This commit removes
them to reduce portability problems.