This is an archive of the discontinued LLVM Phabricator instance.

[dexter] Change line label reference syntax to enable label-relative offsets (1/2)
ClosedPublic

Authored by Orlando on Apr 23 2021, 1:40 AM.

Details

Summary

This patch changes how line labels are resolved in order to enable
label-relative offsets to be used in commands. This is a breaking change in
dexter. Instead of using label references directly as argument values, labels
will instead be referenced through a function ref(str).

// No way to use offsets currently.
Currently: DexExpectWatchValue('x', '1', on_line='labled_line')
Patched:   DexExpectWatchValue('x', '1', on_line=ref('labled_line'))
Patched:   DexExpectWatchValue('x', '1', on_line=ref('labled_line') + 3)

A dexter command is "parsed" by finding the whole command and sending it off to
eval. This change adds a function called ref to the eval globals map that
simply looks up the name and returns an int. If the line name hasn't been
defined, or a name is defined more than once, an error is reported (see
err_bad_label_ref.cpp and err_duplicate_label.cpp). Label offsets can be
achieved by simply writing the desired expression.

The rationale behind removing the existing label referencing mechanic is for
consistency and to simplify the code required to make labels work.

I've separated the update to llvm's dexter tests into another patch for ease of
review here (D101148). Here is a small python script which can be used to
update tests to use the new syntax:
https://gist.github.com/OCHyams/8255efe7757cac266440ed2ba55f1442

If it helps anyone using dexter on downstream tests we can come up with a
deprecation plan instead out outright removing the existing syntax.

Diff Detail

Event Timeline

Orlando requested review of this revision.Apr 23 2021, 1:40 AM
Orlando created this revision.

LGTM, but keen to have additional reviews.

jmorse accepted this revision.May 20 2021, 5:29 AM

LGTM. While this is a breaking change, AFAIUI the body of Dexter tests "out there" is small. If this is a serious issue for someone, we can revert and come up with a better plan.

debuginfo-tests/dexter/Commands.md
217–219

Best to give an example of the line number having a literal added to it, or casual readers won't be aware of the main use case.

This revision is now accepted and ready to land.May 20 2021, 5:29 AM
This revision was landed with ongoing or failed builds.May 21 2021, 12:59 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2021, 12:59 AM
Orlando marked an inline comment as done.May 21 2021, 1:01 AM

Thanks both