Engineering Review and Interview Reasoning

Part IV · Module 12 · Chapter 12.4 · Engineering Review and Interview Reasoning

1. What a Review Is Actually For

A sed command in a pull request is a request for permission to change files that other people depend on. The reviewer's job is not to check that the regex is clever. It is to answer one question:

If this runs on the whole tree, what else does it touch?

Everything below is that question, split into seven parts you can ask in order — a fit question first, then six about the command itself.

The reason it needs a checklist at all is Chapter 12.3: sed does not fail loudly when it is wrong. A command that over-matches exits 0, produces a design that compiles, and passes the regression. There is no stage after the review that will catch it, so the review is the stage.

2. This Chapter Measures Nothing New

Every number quoted below was measured in an earlier chapter and is cited there. The value here is having them in one place, in the order you would actually ask them.

3. The Six Questions

Q0 — Fit: is this a sed job at all?

Asked first because a yes to the rest is worthless if the answer here is no.

Chapter 8.4's test: can the change be decided from the current line alone, with no arithmetic? A threshold, a total, a comparison between lines, a nested format, or anything needing the tool's own data is not a sed job — and Chapter 9.3 adds the HDL case, where scope, macros, strings and comments make a textual rename the wrong instrument regardless of how good the pattern is.

Ask: what is this command for? If the answer involves counting, comparing, or understanding a language's structure, the review ends here and the answer is awk, Python, Tcl inside the tool, or an HDL-aware refactor.

Q1 — Scope: which lines does this run on?

An unaddressed command runs on every line. That has to be a decision, not an omission.

Chapter 10.1 measured the difference on a run script: /^xrun /s|-l sim.log|…| changed 1 line; the same substitution with no address changed 3, including a comment and a different tool's option.

Ask: is there an address? If not, is "every line" really intended? If the file has several kinds of line — a run script, a Tcl setup, a filelist with comments — the answer is usually no.

Q2 — Width: what else matches this pattern?

The single most common cause of a sed incident, and the cheapest to check.

Chapter 12.3's incident renamed clk and also hit clk_div and gclk. The guard is one command:

   grep -oh '[a-z_0-9]*name[a-z_0-9]*' dir/*.sv | sort -u

One line of output means safe. More means the pattern needs word boundaries, more context, or a different approach. Note the containment form — Chapter 12.3 measured the prefix-only version returning 2 where the truth was 3, missing exactly the signal that mattered.

And the match count, which is the number Q5 will check against:

   sed -n 's/pat/&/gp' file | wc -l        lines the command would change
   grep -c 'pat' file                       lines containing a match

Ask: has the author shown this output? If not, ask for it before anything else.

Also ask what the input is assumed to look like. Every pattern encodes assumptions — that paths have no spaces, that the log uses this timestamp format, that comments start at column one, that the file is ASCII. Chapter 10.2 measured a time mask that was correct for one simulator and inert for another; Chapter 8.3 measured a filename with a space turning four targets into six. An assumption nobody states is one nobody checks.

Q3 — Delimiter: can it collide with the data?

Chapter 3.3: a / inside the pattern ends the command early. For paths, use |.

This one is usually self-correcting — a delimiter collision is a loud error, exit 1 — with one exception worth checking: when the value comes from a variable, the collision depends on runtime data. Chapter 8.2 measured OLD="/proj/old|x" breaking a |-delimited command.

Ask: is any part of this command interpolated? If so, what happens when that value contains the delimiter?

Q4 — Portability: will this work where it will run?

The table in section 5 is the full list. The short version for review: if the command contains \b, \|, \U, -i without an argument, -s, 0,/re/, first~step, a one-line i/a/c, a :label; on a shared line, or grep -Z, ask which sed and which grep it will run under.

Most of those fail silently on the other implementation.

Ask: does this run only on the build machines, or also on somebody's Mac?

Q5 — Evidence: what has been shown?

Chapter 7.2's procedure produces three artifacts, and a review should see at least two:

• a count predicted before the edit;
• the dry run — sed -n 's/…/…/p' or a diff -u;
• the after count, matching the prediction.

Chapter 12.1 is the model: 7 lines predicted from grep -c, and git diff --stat reporting 7 insertions afterwards. Two numbers from different tools agreeing is evidence; one number is a claim.

Ask: what did you expect to change, and what did change?

And then: what proves the result still works? Textual correctness is not functional correctness, and the two are routinely confused. Chapter 12.1's verification hierarchy puts the downstream tool last and separate for this reason — a filelist can be rewritten perfectly and name files that do not exist, and Chapter 12.3 measured a rename that compiled, simulated identically, and was still wrong.

Ask: was anything downstream run — a compile, a smoke test, a tool that consumes this file? If not, say so rather than implying it. "The diff is right" and "the flow still works" are different claims with different evidence.

Q6 — Rollback: what happens if this is wrong?

Chapter 12.3's recovery was git checkout -- rtl/, one command, and it worked only because the tree was committed first.

Ask: was git status --short clean before this ran? -i.bak is not an answer for anything that might be run twice — Chapter 7.2 measured the second run overwriting the backup with the first run's output.

4. Three Real Commands, Reviewed

Each of these appeared earlier in the track and was measured there.

Command A — from Chapter 12.3:

   sed -i 's/clk/clk_i/g' rtl/*.sv

Reject on Q0, and again on Q2. Q0 because a textual rename of an HDL identifier is the wrong instrument before the pattern is even read; Q2 because three identifiers contain clk. The measured outcome: clk_div and gclk renamed, both files compiling and simulating to identical output, exit 0 — and the only thing any tool caught was the interface break in a parent that still used the old port name.

Command B — from Chapter 10.1:

   sed -i 's|-l sim.log|-l logs/sim.log|' sim/run.sh

Reject on Q1. Measured: 3 lines changed where 1 was intended, including a comment. The fix is /^xrun / in front.

Command C — from Chapter 12.2:

   find sim -name 'run*.sh' -print0 | xargs -0 sed -i '' -f scripts/vcs-upgrade.sed

Approve, with one note on Q4. The difference between A and C is not sed skill — it is that C's author can answer all seven questions.

5. Every Way sed Exits 0 and Is Wrong

This is the artifact worth keeping. Every row was measured in the chapter named.

* the only row that is not exit 0, and even there the diagnostic goes to stderr.

Two observations a reviewer should carry:

Exit status is nearly useless as a correctness signal for sed. Fifteen of sixteen rows exit 0. A CI step that only checks $? will pass all of them.

The guard that catches almost every row is the same one. Compare the output with the input:

   sed … file | diff -q file - >/dev/null && echo "WARNING: sed changed nothing"

Chapter 7.4 measured that catching the forgotten script, the swallowed label, and every silent GNU-ism. It does not catch the rows where sed changed too much — for those the answer is Q2's census and Q5's diff.

6. Saying It Out Loud

A sed interview question is rarely about the flags. Given "rename axi_awvalid to m_axi_awvalid across the RTL", the weak answer is a command. The strong answer is the reasoning, in about four sentences:

"First I'd check what else contains that string — grep -oh '[a-z_0-9]*axi_awvalid[a-z_0-9]*' rtl/*.sv | sort -u. If _d and _q variants come back, a plain substitution would rename them too, so I'd need word boundaries — which are [[:<:]] on BSD and \b on GNU, and the GNU one silently matches nothing on a Mac, so I'd branch or assert the count. Then a dry run with sed -n 's/…/&/gp' | wc -l to get a number, apply it over a find -print0 | xargs -0 list on a committed tree, and check git diff --stat matches that number. Last, compile — and read the diff anyway, because a rename that compiles can still have hit a comment or a string."

That answer demonstrates the seven questions without naming them, and it is almost entirely drawn from Chapters 9.2, 9.3, 7.3 and 12.1.

Three things interviewers are listening for, in order:

1. Do you check before you write? The audit first, always.
2. Do you know what you cannot know? sed sees characters; scope, macros, strings and comments are invisible to it — Chapter 9.3.
3. Do you know when to stop? "That needs arithmetic, so it is awk" or "that needs the tool's API, so it is Tcl" is a stronger answer than a heroic one-liner — Chapter 8.4.

7. The End of the Track

Forty-five chapters, and the shape of it is worth stating once:

• Part I — what a stream editor is, and the cycle every later command obeys.
• Part II — the language: substitution, both regex dialects, addresses, the line commands, two buffers, control flow.
• Part III — doing it safely: what -i really does, proving an edit, diagnosing one that was too wide, reading the diagnostics, composing with the shell, and deciding whether it is a sed job at all.
• Part IV — the work: filelists, RTL, run scripts, logs, regressions, synthesis setup, constraints, timing reports, and these four case studies.

The claims in it were measured on the sed this machine ships — the inode that changes under -i, the hard link that breaks silently, the N that eats the last line, the label that swallows a whole script, the rename that compiles and is wrong. Where a tool was available it was used as an oracle: iverilog for the RTL renames, tclsh for the synthesis scripts, git for the rollbacks. Where one was not — no simulator, no EDA tool, no GNU sed — the chapters say so rather than inventing output.

There is no Module 13. The track is complete at twelve modules and forty-five chapters, and completion is not a reason to add more. If something here is wrong, it should be corrected; if something is missing, it should be a deliberate addition with its own measurements, not an extension for its own sake.

What to take away, if it compresses to one thing: sed is a small, sharp tool with no opinion about meaning. It will do exactly what you asked, to every line that matches, and tell you it succeeded. Everything in this track is about making sure that what you asked for and what you meant are the same thing — before the command runs, because afterwards nothing in the flow will tell you they were not.

8. Think It Through

1. You are asked to approve sed -i 's/clk/clk_i/g' rtl/*.sv. What is your first question?

Not about the command — ask for the output of grep -oh '[a-z_0-9]*clk[a-z_0-9]*' rtl/*.sv | sort -u. If it returns more than one identifier, the command is wrong regardless of anything else, and Chapter 12.3 measured that it returns three.

2. A colleague says their sed worked because it exited 0. What do you show them?

Section 5. Fifteen of sixteen measured failure modes exit 0, including one that empties a file and one that passes the input through completely unchanged. For sed, exit status answers "did it run", never "was it right".

3. Which review question would have caught Chapter 10.1's log-retarget bug?

Q1, scope. The pattern was specific and the file was not: -l sim.log appeared on two tool invocations and in a comment. An address naming the tool line reduces 3 changed lines to 1.

4. When is "no address" the right answer to Q1?

When the pattern is specific enough to be its own anchor and every occurrence should change — Chapter 10.1's +UVM_TESTNAME=old_test matched two lines and both were real invocations. The test is whether you can state why every match is intended, not whether the command is shorter.

5. What makes an upgrade script approvable where a chained one-liner is not?

Reviewability and repeatability. The rules carry comments explaining the release change, the file is committed and diffable, and Chapter 12.2 measured it idempotent — safe to re-run over a half-converted tree. The one-liner does the same substitutions and can answer none of Q1, Q5 or Q6.

6. Give the four-sentence answer to "rename a signal across the RTL".

Audit what else contains the string; if anything does, use word boundaries and remember the two spellings, one of which fails silently; dry-run to a count and apply over a NUL-safe list on a committed tree; then compile, and read the diff anyway for comments and strings. That is Chapters 9.2, 7.3, 8.3 and 9.3 in order, and it is the answer the question is looking for.

9. Summary

• A review answers one question: if this runs on the whole tree, what else does it touch?
• Seven questions, in order: fit (is this a sed job?), scope, width, delimiter, portability, evidence, rollback.
• Q2 is the cheapest and catches the most. grep -oh '[a-z_0-9]*name[a-z_0-9]*' … | sort -u — one line of output means safe, and use the containment form, not the prefix form.
• Q5 wants two numbers from different tools. Chapter 12.1 predicted 7 and git reported 7; one number alone is a claim.
• Q6 is settled before the command runs — a clean, committed tree, because -i.bak protects against exactly one mistake.
• Fifteen of sixteen measured silent failures exit 0. Exit status tells you sed ran, never that it was right.
• sed … f | diff -q f - catches the "changed nothing" half; the census and the diff catch the "changed too much" half.
• An interview answer is the seven questions said out loud, and what is being listened for is whether you check first, know what sed cannot see, and know when the job belongs to awk or Tcl.
• The track ends here, at 45 chapters. Measured where a tool existed to measure with; explicit about the limits where none did.