[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] refactor: processing of input makefile rules
|
From: |
Stefano Lattarini |
|
Subject: |
[PATCH] refactor: processing of input makefile rules |
|
Date: |
Fri, 20 Apr 2012 10:07:24 +0200 |
This is a pure refactoring, with no intended functional or semantic
changes. It breaks up an overly-long function in three smaller
sub-functions. This change will very especially useful for the work
on Automake-NG.
* lib/Automake/Rule.pm (define): Move quite a lot of code out, into ...
(_rule_defn_with_exeext_awareness, _maybe_warn_about_duplicated_target,
_conditionals_for_rule): ... these new subroutines.
Signed-off-by: Stefano Lattarini <address@hidden>
---
The testsuite still passes. I plan to push this change by tomorrow if
there are no objections.
Regards,
Stefano
lib/Automake/Rule.pm | 364 ++++++++++++++++++++++++++------------------------
1 file changed, 193 insertions(+), 171 deletions(-)
diff --git a/lib/Automake/Rule.pm b/lib/Automake/Rule.pm
index 3db8c44..3f17daa 100644
--- a/lib/Automake/Rule.pm
+++ b/lib/Automake/Rule.pm
@@ -575,6 +575,178 @@ sub _new ($$)
return $self;
}
+sub _rule_defn_with_exeext_awareness ($$$)
+{
+ my ($target, $cond, $where) = @_;
+
+ # For now 'foo:' will override 'foo$(EXEEXT):'. This is temporary,
+ # though, so we emit a warning.
+ (my $noexe = $target) =~ s/\$\(EXEEXT\)$//;
+ my $noexerule = rule $noexe;
+ my $tdef = $noexerule ? $noexerule->def ($cond) : undef;
+
+ if ($noexe ne $target
+ && $tdef
+ && $noexerule->name ne $target)
+ {
+ # The no-exeext option enables this feature.
+ if (! option 'no-exeext')
+ {
+ msg ('obsolete', $tdef->location,
+ "deprecated feature: target '$noexe' overrides "
+ . "'$noexe\$(EXEEXT)'\n"
+ . "change your target to read '$noexe\$(EXEEXT)'",
+ partial => 1);
+ msg ('obsolete', $where, "target '$target' was defined here");
+ }
+ }
+ return $tdef;
+}
+
+sub _maybe_warn_about_duplicated_target ($$$$$$)
+{
+ my ($target, $tdef, $source, $owner, $cond, $where) = @_;
+
+ my $oldowner = $tdef->owner;
+ # Ok, it's the name target, but the name maybe different because
+ # 'foo$(EXEEXT)' and 'foo' have the same key in our table.
+ my $oldname = $tdef->name;
+
+ # Don't mention true conditions in diagnostics.
+ my $condmsg =
+ $cond == TRUE ? '' : (" in condition '" . $cond->human . "'");
+
+ if ($owner == RULE_USER)
+ {
+ if ($oldowner == RULE_USER)
+ {
+ # Ignore '%'-style pattern rules. We'd need the
+ # dependencies to detect duplicates, and they are
+ # already diagnosed as unportable by -Wportability.
+ if ($target !~ /^[^%]*%[^%]*$/)
+ {
+ ## FIXME: Presently we can't diagnose duplicate user rules
+ ## because we don't distinguish rules with commands
+ ## from rules that only add dependencies. E.g.,
+ ## .PHONY: foo
+ ## .PHONY: bar
+ ## is legitimate. (This is phony.test.)
+
+ # msg ('syntax', $where,
+ # "redefinition of '$target'$condmsg ...", partial => 1);
+ # msg_cond_rule ('syntax', $cond, $target,
+ # "... '$target' previously defined here");
+ }
+ }
+ else
+ {
+ # Since we parse the user Makefile.am before reading
+ # the Automake fragments, this condition should never happen.
+ prog_error ("user target '$target'$condmsg seen after Automake's"
+ . " definition\nfrom " . $tdef->source);
+ }
+ }
+ else # $owner == RULE_AUTOMAKE
+ {
+ if ($oldowner == RULE_USER)
+ {
+ # -am targets listed in %dependencies support a -local
+ # variant. If the user tries to override TARGET or
+ # TARGET-am for which there exists a -local variant,
+ # just tell the user to use it.
+ my $hint = 0;
+ my $noam = $target;
+ $noam =~ s/-am$//;
+ if (exists $dependencies{"$noam-am"})
+ {
+ $hint = "consider using $noam-local instead of $target";
+ }
+
+ msg_cond_rule ('override', $cond, $target,
+ "user target '$target' defined here"
+ . "$condmsg ...", partial => 1);
+ msg ('override', $where,
+ "... overrides Automake target '$oldname' defined here",
+ partial => $hint);
+ msg_cond_rule ('override', $cond, $target, $hint)
+ if $hint;
+ }
+ else # $oldowner == RULE_AUTOMAKE
+ {
+ # Automake should ignore redefinitions of its own
+ # rules if they came from the same file. This makes
+ # it easier to process a Makefile fragment several times.
+ # However it's an error if the target is defined in many
+ # files. E.g., the user might be using bin_PROGRAMS = ctags
+ # which clashes with our 'ctags' rule.
+ # (It would be more accurate if we had a way to compare
+ # the *content* of both rules. Then $targets_source would
+ # be useless.)
+ my $oldsource = $tdef->source;
+ if (not ($source eq $oldsource && $target eq $oldname))
+ {
+ msg ('syntax',
+ $where, "redefinition of '$target'$condmsg ...",
+ partial => 1);
+ msg_cond_rule ('syntax', $cond, $target,
+ "... '$oldname' previously defined here");
+ }
+ }
+ }
+}
+
+# Return the list of conditionals in which the rule was defined. In case
+# an ambiguous conditional definition is detected, return the empty list.
+sub _conditionals_for_rule ($$$$)
+{
+ my ($rule, $owner, $cond, $where) = @_;
+ my $target = $rule->name;
+ my @conds;
+ my ($message, $ambig_cond) = $rule->conditions->ambiguous_p ($target, $cond);
+
+ return $cond if !$message; # No ambiguity.
+
+ if ($owner == RULE_USER)
+ {
+ # For user rules, just diagnose the ambiguity.
+ msg 'syntax', $where, "$message ...", partial => 1;
+ msg_cond_rule ('syntax', $ambig_cond, $target,
+ "... '$target' previously defined here");
+ return ();
+ }
+
+ # FIXME: for Automake rules, we can't diagnose ambiguities yet.
+ # The point is that Automake doesn't propagate conditions
+ # everywhere. For instance &handle_PROGRAMS doesn't care if
+ # bin_PROGRAMS was defined conditionally or not.
+ # On the following input
+ # if COND1
+ # foo:
+ # ...
+ # else
+ # bin_PROGRAMS = foo
+ # endif
+ # &handle_PROGRAMS will attempt to define a 'foo:' rule
+ # in condition TRUE (which conflicts with COND1). Fixing
+ # this in &handle_PROGRAMS and siblings seems hard: you'd
+ # have to explain &file_contents what to do with a
+ # condition. So for now we do our best *here*. If 'foo:'
+ # was already defined in condition COND1 and we want to define
+ # it in condition TRUE, then define it only in condition !COND1.
+ # (See cond14.test and cond15.test for some test cases.)
+ @conds = $rule->not_always_defined_in_cond ($cond)->conds;
+
+ # No conditions left to define the rule.
+ # Warn, because our workaround is meaningless in this case.
+ if (scalar @conds == 0)
+ {
+ msg 'syntax', $where, "$message ...", partial => 1;
+ msg_cond_rule ('syntax', $ambig_cond, $target,
+ "... '$target' previously defined here");
+ return ();
+ }
+ return @conds;
+}
=item C<@conds = define ($rulename, $source, $owner, $cond, $where)>
@@ -601,186 +773,38 @@ sub define ($$$$$)
# Don't even think about defining a rule in condition FALSE.
return () if $cond == FALSE;
- # For now 'foo:' will override 'foo$(EXEEXT):'. This is temporary,
- # though, so we emit a warning.
- (my $noexe = $target) =~ s,\$\(EXEEXT\)$,,;
- my $noexerule = rule $noexe;
- my $tdef = $noexerule ? $noexerule->def ($cond) : undef;
-
- if ($noexe ne $target
- && $tdef
- && $noexerule->name ne $target)
- {
- # The no-exeext option enables this feature.
- if (! option 'no-exeext')
- {
- msg ('obsolete', $tdef->location,
- "deprecated feature: target '$noexe' overrides "
- . "'$noexe\$(EXEEXT)'\n"
- . "change your target to read '$noexe\$(EXEEXT)'",
- partial => 1);
- msg ('obsolete', $where, "target '$target' was defined here");
- }
- # Don't 'return ()' now, as this might hide target clashes
- # detected below.
- }
-
+ my $tdef = _rule_defn_with_exeext_awareness ($target, $cond, $where);
# A GNU make-style pattern rule has a single "%" in the target name.
msg ('portability', $where,
"'%'-style pattern rules are a GNU make extension")
if $target =~ /^[^%]*%[^%]*$/;
- # Diagnose target redefinitions.
+ # See whether this is a duplicated target declaration.
if ($tdef)
{
- my $oldowner = $tdef->owner;
- # Ok, it's the name target, but the name maybe different because
- # 'foo$(EXEEXT)' and 'foo' have the same key in our table.
- my $oldname = $tdef->name;
-
- # Don't mention true conditions in diagnostics.
- my $condmsg =
- $cond == TRUE ? '' : " in condition '" . $cond->human . "'";
-
- if ($owner == RULE_USER)
- {
- if ($oldowner == RULE_USER)
- {
- # Ignore '%'-style pattern rules. We'd need the
- # dependencies to detect duplicates, and they are
- # already diagnosed as unportable by -Wportability.
- if ($target !~ /^[^%]*%[^%]*$/)
- {
- ## FIXME: Presently we can't diagnose duplicate user rules
- ## because we don't distinguish rules with commands
- ## from rules that only add dependencies. E.g.,
- ## .PHONY: foo
- ## .PHONY: bar
- ## is legitimate. (This is phony.test.)
-
- # msg ('syntax', $where,
- # "redefinition of '$target'$condmsg ...", partial => 1);
- # msg_cond_rule ('syntax', $cond, $target,
- # "... '$target' previously defined here");
- }
- # Return so we don't redefine the rule in our tables,
- # don't check for ambiguous condition, etc. The rule
- # will be output anyway because &read_am_file ignore the
- # return code.
- return ();
- }
- else
- {
- # Since we parse the user Makefile.am before reading
- # the Automake fragments, this condition should never happen.
- prog_error ("user target '$target'$condmsg seen after Automake's"
- . " definition\nfrom " . $tdef->source);
- }
- }
- else # $owner == RULE_AUTOMAKE
- {
- if ($oldowner == RULE_USER)
- {
- # -am targets listed in %dependencies support a -local
- # variant. If the user tries to override TARGET or
- # TARGET-am for which there exists a -local variant,
- # just tell the user to use it.
- my $hint = 0;
- my $noam = $target;
- $noam =~ s/-am$//;
- if (exists $dependencies{"$noam-am"})
- {
- $hint = "consider using $noam-local instead of $target";
- }
-
- msg_cond_rule ('override', $cond, $target,
- "user target '$target' defined here"
- . "$condmsg ...", partial => 1);
- msg ('override', $where,
- "... overrides Automake target '$oldname' defined here",
- partial => $hint);
- msg_cond_rule ('override', $cond, $target, $hint)
- if $hint;
-
- # Don't overwrite the user definition of TARGET.
- return ();
- }
- else # $oldowner == RULE_AUTOMAKE
- {
- # Automake should ignore redefinitions of its own
- # rules if they came from the same file. This makes
- # it easier to process a Makefile fragment several times.
- # However it's an error if the target is defined in many
- # files. E.g., the user might be using bin_PROGRAMS = ctags
- # which clashes with our 'ctags' rule.
- # (It would be more accurate if we had a way to compare
- # the *content* of both rules. Then $targets_source would
- # be useless.)
- my $oldsource = $tdef->source;
- return () if $source eq $oldsource && $target eq $oldname;
-
- msg ('syntax', $where, "redefinition of '$target'$condmsg ...",
- partial => 1);
- msg_cond_rule ('syntax', $cond, $target,
- "... '$oldname' previously defined here");
- return ();
- }
- }
- # Never reached.
- prog_error ("unreachable place reached");
+ # Diagnose invalid target redefinitions, if any. Note that some
+ # target redefinitions are valid (e.g., for multiple-targets
+ # pattern rules).
+ _maybe_warn_about_duplicated_target ($target, $tdef, $source,
+ $owner, $cond, $where);
+ # Return so we don't redefine the rule in our tables, don't check
+ # for ambiguous condition, etc. The rule will be output anyway
+ # because '&read_am_file' ignores the return code.
+ return ();
}
- # Conditions for which the rule should be defined.
- my @conds = $cond;
-
- # Check ambiguous conditional definitions.
my $rule = _crule $target;
- my ($message, $ambig_cond) = $rule->conditions->ambiguous_p ($target, $cond);
- if ($message) # We have an ambiguity.
- {
- if ($owner == RULE_USER)
- {
- # For user rules, just diagnose the ambiguity.
- msg 'syntax', $where, "$message ...", partial => 1;
- msg_cond_rule ('syntax', $ambig_cond, $target,
- "... '$target' previously defined here");
- return ();
- }
- else
- {
- # FIXME: for Automake rules, we can't diagnose ambiguities yet.
- # The point is that Automake doesn't propagate conditions
- # everywhere. For instance &handle_PROGRAMS doesn't care if
- # bin_PROGRAMS was defined conditionally or not.
- # On the following input
- # if COND1
- # foo:
- # ...
- # else
- # bin_PROGRAMS = foo
- # endif
- # &handle_PROGRAMS will attempt to define a 'foo:' rule
- # in condition TRUE (which conflicts with COND1). Fixing
- # this in &handle_PROGRAMS and siblings seems hard: you'd
- # have to explain &file_contents what to do with a
- # condition. So for now we do our best *here*. If 'foo:'
- # was already defined in condition COND1 and we want to define
- # it in condition TRUE, then define it only in condition !COND1.
- # (See cond14.test and cond15.test for some test cases.)
- @conds = $rule->not_always_defined_in_cond ($cond)->conds;
-
- # No conditions left to define the rule.
- # Warn, because our workaround is meaningless in this case.
- if (scalar @conds == 0)
- {
- msg 'syntax', $where, "$message ...", partial => 1;
- msg_cond_rule ('syntax', $ambig_cond, $target,
- "... '$target' previously defined here");
- return ();
- }
- }
- }
+
+ # Conditions for which the rule should be defined. Due to some
+ # complications in the automake internals, this aspect is not as
+ # obvious as it might be, and in come cases this list must contain
+ # other entries in addition to '$cond'. See the comments in
+ # '_conditionals_for_rule' for a rationale.
+ my @conds = _conditionals_for_rule ($rule, $owner, $cond, $where);
+
+ # Stop if we had ambiguous conditional definitions.
+ return unless @conds;
# Finally define this rule.
for my $c (@conds)
@@ -807,8 +831,6 @@ sub define ($$$$$)
# declared in SUFFIXES and are not known language
# extensions). Automake will complete SUFFIXES from
# @suffixes automatically (see handle_footer).
-
-
|| ($t =~ /$_SUFFIX_RULE_PATTERN/o && accept_extensions($1)))
{
++$inference_rule_count;
--
1.7.9.5
- [PATCH] refactor: processing of input makefile rules,
Stefano Lattarini <=