Skip to content

makedependfile(.SH)?] clean up#24565

Open
khwilliamson wants to merge 7 commits into
Perl:bleadfrom
khwilliamson:makedepend
Open

makedependfile(.SH)?] clean up#24565
khwilliamson wants to merge 7 commits into
Perl:bleadfrom
khwilliamson:makedepend

Conversation

@khwilliamson

Copy link
Copy Markdown
Contributor

makedependfile had a stray apostrophe on z/OS only code which prevented dependencies from being generated on that platform

makedependfile.SH had nearly identical branches for z/OS versus everything else. The change to use clang allows those two be collapsed, so that conditionals that apply just one of them will simply not be found when executed on the other.

It fixes too many backslashes in outputting a newline, removes obsolete code, now recognizes an additional syntax for the preprocessor output of line number marker lines, and normalizes white space for clarity

  • This set of changes does not require a perldelta entry.

Comment thread makedependfile.SH
-e 's|[ ]*/\*[^*].*$||' \
-e 's|[ ]*/\*\*[^/].*$||' \
-e 's/.\{255\}/&\\\n/g' \
-e 's/.\{255\}/&\n/g' \

@tonycoz tonycoz Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change is correct. The original turns:

very very(248 chars here) long line

into

very very(248 chars here)\
long line

splitting very long lines into at most 257 characters (255 original text, followed by \, followed by a newline.

I expect the replacement to be
& - the original text
\\ - becomes a single \
\n - becomes a newline

From the comment:

At the end, we unjoin very long lines to avoid preprocessor limitations

Not that I ever did much with sed.

Edit: make sure \ is treated literally in the replacement paragraph.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above this you'll see:

      -e '/\\$/{'                                            \
            -e 'N'                                             \
            -e 'b tstcont'                                     \
        -e '}'                                                 \

which turns physical lines with continuations:

abc\
def

into logical lines:

abcdef

Reproducing what the compiler does in step 2 of the translation phases (C99 or C11 5.1.1.2):

  1. Each instance of a backslash character () immediately followed by a new-line
    character is deleted, splicing physical source lines to form logical source lines.
    Only the last backslash on any physical source line shall be eligible for being part
    of such a splice. A source file that is not empty shall end in a new-line character,
    which shall not be immediately preceded by a backslash character before any such
    splicing takes place

So that /^[space]#/ only matches at the beginning of a logical line.

The code you've changed here, trying to turn those back into physical lines short enough that an old cpp won't choke.

The \ (escaped to \\) you've removed is needed so the preprocessor sees the separate physical lines here as one logical line.

Comment thread makedependfile.SH Outdated
-e '}' \
-e 's/\\\n//g' \
-e '/^#line/d' \
-e '/^# [0-9][0-9]* /d' \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the space after the hash be more generic, ie [ ]+ (raw tab and a space in the [])?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but it doesn't do any harm to add that. Now done.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you changed:

-e '/^# *[0-9][0-9]* *[".\/]/!d' \

to:

     -e '/^#[	 ]+[0-9][0-9]* *[".\/]/!d'                      \

but not the

-e '/^# [0-9][0-9]* /d'                                \

where my comment should have appeared.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. Now fixed

This workaround code was added in 1999, removed in 2002, added back in
2003, and commented out in 2013 (via
6edd329), but left in in case it was
needed.  13 years later, it hasn't been needed, so finally remove it..
Before, this largish sed script was almost entirely duplicated, using a
pipe on os390, vs saving to files on every other system.  I don't know
the reason, but now that we are using clang, we can use files with it,
so can combine the two versions.

I'm unsure of the motivation for many of the sed commands, but this
commit preserves all of them, applying the ones that affected just one
type to now the other type as well.  I'm pretty sure these won't harm
anything.  For reference these are the commands that newly apply to
z/OS:
    -e '1d'
    -e '/^#.*"\/.*\/"/d'
    -e '/: file path prefix .* never used$/d'

And this is the one that newly applies to other platforms:

    -e '/".*In file included from.*:/d'
We want the script to actually create a newline, not a symbol for the
newline.
The 'line' is omitted, as not all preprocessors understand it
There is nothing in this header that interests us in finding
dependencies.  So, instead of removing lines that got expanded later,
remove its include before expansion
This adds/subtracts white space to make it easier to see where
statements begin and end, and vertical aligns continuation marks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants