Page MenuHomePhabricator

Make DebugVariable class available in DebugInfoMetadata
ClosedPublic

Authored by StephenTozer on Nov 20 2019, 2:59 AM.

Details

Summary

This review is a prerequisite to: https://reviews.llvm.org/D70318

This patch moves the DebugVariable class from LiveDebugValues.cpp to DebugInfoMetadata.h, making it available in a much broader scope. In the patch linked above, this class is used in SimplifyCFG as an identifier for variables which require setting as undefined.

Diff Detail

Event Timeline

StephenTozer created this revision.Nov 20 2019, 2:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2019, 2:59 AM
Orlando added inline comments.
llvm/include/llvm/IR/DebugInfoMetadata.h
3284

Could this be const OptFragmentInfo &?

How about adding a quick unit test in the metadata unit test file?

llvm/include/llvm/IR/DebugInfoMetadata.h
3266

thanks!

3284

FragmentInfo small and meant to be used as a value type. I would prefer

Optional<FragmentInfo> (literally, without the using to hide the Optional). It's easier to read.

3293

Again, let's just return this by value.

3296

This warrants a comment explaining what this function does.

Is this actually more readable than Fragment.getValueOr(DefaultFragment)?

3300

isDefaultFragment?

3320

///

3322

return DV(nullptr, {}, nullptr)

3325

/// ... .

3327

DV(nullptr, {0, 0}, nullptr);

StephenTozer marked an inline comment as done.Nov 21 2019, 4:43 AM
StephenTozer added inline comments.
llvm/include/llvm/IR/DebugInfoMetadata.h
3296

Does getFragmentOrDefault() sound more descriptive? It seems like a useful helper to have, in no small part because just having a static const member isn't as clear to a user as there being an explicit "get-value-or-default" function.

Clean up code, fix comments.

Use NoneType() to call correct DebugVariable constructor

Add unit test, allow nullptr for DIExpression in DebugVariable ctor

Remove redundant assignment on test variable construction

aprantl accepted this revision.Dec 2 2019, 1:34 PM
aprantl added inline comments.
llvm/include/llvm/IR/DebugInfoMetadata.h
3289

getVariable?

This revision is now accepted and ready to land.Dec 2 2019, 1:34 PM

Rebased on master, rename getVar -> getVariable

StephenTozer closed this revision.Dec 3 2019, 7:48 AM

Merged in commit: llvmorg-10-init-11043-g269a9afe25cb