There was no need to export Targ_PrintNode at all. All the callers need
is a high-level API for printing a single node or a list of nodes. The
implementation detail that Targ_PrintNode was used as a callback to
Lst_ForEach should have never leaked into the API.
The migration from null-passing Lst functions to argument-checking Lst
functions is completed.
There were 2 surprises: The targets list may be NULL, and in Dir_AddDir,
the path may be NULL. The latter case is especially surprising since
that function turns into an almost-nop in that case. This is another
case where probably 2 independent functions have been squeezed into a
single function. This may be improved in a follow-up commit.
All other lists were fine. They were always defined and thus didn't
need much work.
In most cases the Lst functions are only called when the arguments are
indeed valid. It's not guaranteed though, therefore each function call
needs to be analyzed and converted individually.
While here, remove a few statements that were only useful when the Lst
functions handled circular lists.
When printing an enum value in debugging mode, distinguish between
bitsets containing flags and ordinary enums that just contain different
values.
Make the macros in enum.h more uniform. Provide a simple scheme for
defining the run-time type information of enums whose number of values
is a number with more than 2 bits set in the binary representation.
This case was not obvious before, and it was pure luck that the current
interesting enum types only had 3, 10 or 32 different values.
The type with the 32 different values actually only has 31 significant
bits since the enum constant OP_OPMASK is only used when querying the
enum, not for defining or describing the possible values. For this
reason, it was unavoidable to refactor the rtti macros, to support even
this case.
A string like OP_DEPENDS|OP_OPTIONAL|OP_PRECIOUS is much easier to read
and understand than the bit pattern 00000089.
The implementation in enum.h looks really bloated and ugly, but using
this API is as simple and natural as possible. That's the trade-off.
In enum.h, I thought about choosing the numbers in the macros such that
it is always possible to combine two of them in order to reach an
arbitrary number, because of the "part1, part2" in the ENUM__SPEC macro.
The powers of 2 are not these numbers, as 7 cannot be expressed as the
sum of two of them. Neither are the fibonacci numbers since 12 cannot
be expressed as the sum of 2 fibonacci numbers. I tried to find a
general pattern to generate these minimal 2-sum numbers, but failed.
Lst_Duplicate would have passed through any null pointer, which was not
needed for make. It was the last function that used Lst_AtEnd, which in
turn was the last function that used LstInsertAfter. As a result, these
two functions have been removed.
Instead of the two-in-one Lst_Concat, having two separate functions is
easier to understand. There is no need for a long API comment anymore
since the new functions have a single purpose that is accurately
described by their name.
The long comment inside Lst_Concat has been removed since it only
repeated the exact code, only in more words.
The comments in make.c about appending the cohorts had been wrong. They
were not appended but prepended. Once more, the function name expresses
everything that the comment said, making the comment redundant. There
is no need to test whether the cohorts list is empty, doing nothing is
implied by the word All in Lst_PrependAllS.
Since the lists don't contain null pointers, it doesn't make sense to
search for a null pointer. All calls but one already had obviously
non-null arguments. The one remaining call using targ->suff has been
guarded for now.
The code for Lst_Member became much simpler than before. Partly because
the old code had an extra condition for circular lists, which are not
used by make.
Except for once instance in parse.c, the usage pattern for Lst_Dequeue
was to first test whether the list is empty. This pattern allowed the
implementation of Lst_Dequeue to become simpler since the null check is
not needed anymore.
The calls to Lst_Enqueue never pass an invalid list or a null pointer,
therefore making them strict was trivial.
In these cases, the list is guaranteed to be valid, therefore no
assertion is expected.
For comparison, the Lst_AtFront in dir.c needs to be kept since the path
may be null there.
The general-purpose list library that is included in make allows to call
Lst_AtEnd for invalid lists, silently ignoring this programming error.
This is a flexibility that make doesn't need.
Another unneeded "feature" is that list items can theoretically be null
pointers. This doesn't make sense as well and is therefore not needed
by make.
These programming errors are now caught early by assertions.
There is no need to duplicate the list of .USEBEFORE commands, just to
destroy that list immediately. Instead, it's simpler to just prepend
the commands from the .USEBEFORE node to the parent.
This change ensures that there is actually something added to the list.
Lst_AtEnd had silently skipped the addition if the list was invalid
(null pointer), which was not intended in these cases. The "(void)" is
assumed to mean "I know that this cannot fail", while it could also mean
"I don't care whether something actually happened".
Running "./build.sh -j6 tools" still succeeds after this change,
therefore chances are very low that this change breaks anything. If
there is any change, it's an obvious assertion failure. There is no
silent change in behavior though.
In several places, it just doesn't make sense to have a null pointer
when a list is expected.
In the existing unit tests, the list passed to Lst_Open is always valid,
but that's not a guarantee for real-world usage. Therefore, Lst_Open
has been left for now, and Lst_OpenS is only the preferred alternative
to it.
Up to now, the list library didn't distinguish between programming
mistakes (violations of invariants, illegal parameter values) and
actually interesting situations like "element not found in list".
The current code contains many branches for conditions that are neither
exercised by the unit tests nor by real-world usage. There is no point
in keeping this unnecessary code.
The list functions will be migrated from their lenient variants to the
stricter variants in small parts, each function getting the S suffix
when it is made strict, to avoid any confusion about how strict a
particular function is. When all functions have been migrated, they
will be renamed back to their original names.
While here, the comments of the functions are cleaned up since they
mention irrelevant implementation details in the API comments, as well
as "side effects" that are really main effects.
The list library had probably been imported from a general-purpose
library that also supported circular lists. These are not used by make
though.
After replacing Lst_Init(FALSE) with Lst_Init(), only a single call to
Lst_Init remained with a non-constant argument, and that was in
Lst_Concat, which was to be expected.
The first parameter from Var_Subst had been a literal NULL in all cases.
These have been fixed using this command:
sed -i 's|Var_Subst(NULL, |Var_Subst(|' *.c
The one remaining case was not found because the "NULL," was followed by
a line break instead of a space.
The removed code probably wouldn't have worked as expected anyway.
Expanding a single variable to a literal string would have led to
unexpected behavior for cases like ${VAR:M${pattern}}, in case pattern
would contain an unescaped ':' itself.
In var.c there are lots of different flag types. To make any accidental
mixture obvious, each flag group gets its own prefix.
The only flag group that is visible outside of var.c is concerned with
evaluating variables, therefore the "e", which replaces the former "f"
that probably just meant "flag".
The enum corresponding to this int parameter is only defined in var.c,
which makes it impractical for the outside to set this parameter to
anything but 0.
On x86_64, this reduces the size of the resulting executable by 5 kB.
Quite extensive rewrite of the Suff module. Some ripple effects into
Parse and Targ modules too.
Dependency searches in general were made to honor explicit rules so
implicit and explicit sources are no longer applied on targets that
do not invoke a transformation rule.
Archive member dependency search was rewritten. Explicit rules now
work properly and $(.TARGET) is set correctly. POSIX semantics for
lib(member.o) and .s1.a rules are supported.
.SUFFIXES list maintenance was rewritten so that scanning of existing
rules works when suffixes are added and that clearing the suffix list
removes single suffix rules too. Transformation rule nodes are now
mixed with regular nodes so they are available as regular targets too
if needed (especially after the known suffixes are cleared).
The .NULL target was documented in the manual page, especially to
warn against using it when a single suffix rule would work.
A deprecation warning was also added to the manual and make also
warns the user if it encounters .NULL.
Search for suffix rules no longer allows the explicit dependencies
to override the selected transformation rule. A check is made in
the search that the transformation that would be tried does not
already exist in the chain. This prevents getting stuck in an infinite
loop under specific circumstances. Local variables are now set
before node's children are expanded so dynamic sources work in
multi-stage transformations. Make_HandleUse() no longer expands
the added children for transformation nodes, preventing triple
expansion and allowing the Suff module to properly postpone their
expansion until proper values are set for the local variables.
Directory prefix is no longer removed from $(.PREFIX) if the target
is found via directory search.
The last rule defined is now used instead of the first one (POSIX
requirement) in case a rule is defined multiple times. Everything
defined in the first instance is undone, but things added "globally"
are honored. To implement this, each node tracks attribute bits
which have been set by special targets (global) instead of special
sources (local). They also track dependencies that were added by
a rule with commands (local) instead of rule with no commands (global).
New attribute, OP_FROM_SYS_MK is introduced. It is set on all targets
found in system makefiles so that they are not eligible to become
the main target. We cannot just set OP_NOTMAIN because it is one of
the attributes inherited from transformation and .USE rules and would
make any eligible target that uses a built-in inference rule ineligible.
The $(.IMPSRC) local variable now works like in gmake: it is set to
the first prerequisite for explicit rules. For implicit rules it
is still the implied source.
The manual page is improved regarding the fixed features. Test cases
for the fixed problems are added.
Other improvements in the Suff module include:
- better debug messages for transformation rule search (length of
the chain is now visualized by indentation)
- Suff structures are created, destroyed and moved around by a set
of maintenance functions so their reference counts are easier
to track (this also gets rid of a lot of code duplication)
- some unreasonably long functions were split into smaller ones
- many local variables had their names changed to describe their
purpose instead of their type
with versions prefixed by MAKE_ATTR_* to avoid modifying the
implementation namespace. Make sure they are available in all places
using nonints.h to fix bootstrap on Linux.
