This is an archive of the discontinued LLVM Phabricator instance.

[ASan] rename internal references to left of/right of.
Needs ReviewPublic

Authored by fmayer on Sep 9 2022, 8:47 AM.

Details

Reviewers
eugenis

Diff Detail

Event Timeline

fmayer created this revision.Sep 9 2022, 8:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2022, 8:47 AM
fmayer requested review of this revision.Sep 9 2022, 8:47 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 9 2022, 8:47 AM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
fmayer added a comment.Sep 9 2022, 9:07 AM

Another option (probably better, WDYT?): replace preceding/succeeding with front/back.

fmayer updated this revision to Diff 459106.Sep 9 2022, 10:09 AM

preceding -> front, succeeding -> back

fmayer added a subscriber: enh.Sep 9 2022, 10:12 AM
enh added a comment.Sep 9 2022, 10:16 AM

yeah, that makes sense to me.

(looking at the code also explains why "left" and "right" didn't seem as bad to the authors of this code as it does to mere users... your view of memory is quite different to ours, which only contains the actual objects we've allocated, and has no "chunks"...)

I think I'd rather keep left/right as internal concepts and only translate them to before/after in the output.

compiler-rt/lib/asan/asan_allocator.cpp
75

I don't get this. Why P and S vs F and B?

443

Not sure about this either. Before can be understood as in time or in space, while left and right are unambiguous, as soon as we agree on conceptual memory layout.

compiler-rt/lib/asan/asan_thread.cpp
393

Every time I read front/back I have to stop and think which is which. The terms make sense for a container, but IMHO not for a chunk of memory.

fmayer marked an inline comment as done.Sep 12 2022, 2:14 PM

PTAL. I'd like to merge this earlier rather than later if possible, to avoid merge conflicts.

compiler-rt/lib/asan/asan_allocator.cpp
75

if you were referring to the below, that was a mistake (it used to be preceding / succeeding but i think front / back is better. happy to revert back to that).

fmayer marked an inline comment as done.Sep 12 2022, 2:14 PM

PTAL. I'd like to merge this earlier rather than later if possible, to avoid merge conflicts.

Sorry, that was left over. Didn't mean to send this.