This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Encode options using llvm::Optional
ClosedPublic

Authored by samestep on Aug 12 2022, 7:52 AM.

Details

Summary

This patch restructures DataflowAnalysisOptions and TransferOptions to use llvm::Optional, in preparation for adding more sub-options to the ContextSensitiveOptions struct introduced here.

Diff Detail

Event Timeline

samestep created this revision.Aug 12 2022, 7:52 AM
Herald added a project: Restricted Project. · View Herald Transcript
samestep requested review of this revision.Aug 12 2022, 7:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 7:52 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
samestep edited the summary of this revision. (Show Details)Aug 12 2022, 7:54 AM

LLVM switched to C++17 recently. I am not sure whether this means std::optional is preferred over llvm::Optional.

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
3902

I think llvm::None is a more concise way to create an empty optional (akin to std::nullopt).

sgatev accepted this revision.Aug 12 2022, 8:41 AM
sgatev added inline comments.
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
70
clang/include/clang/Analysis/FlowSensitive/Transfer.h
31

Perhaps keep the name ContextSensitive? In the context of TransferOptions it seems clear what this is.

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
322

For consistency.

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
69
3902
This revision is now accepted and ready to land.Aug 12 2022, 8:41 AM
samestep updated this revision to Diff 452193.Aug 12 2022, 8:43 AM

Use llvm::None per Gábor's suggestion

samestep added inline comments.Aug 12 2022, 8:45 AM
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
70

I tried that, but I get the following error message:

Incompatible operand types ('TransferOptions' and 'const NoneType')

clang/include/clang/Analysis/FlowSensitive/Transfer.h
31

I don't have a strong preference either way; does anyone else have an opinion on this?

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
322

Ah thanks! Will do.

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
69

Same error mentioned above.

samestep updated this revision to Diff 452196.Aug 12 2022, 8:46 AM

Remove unnecessary explicit call to hasValue

xazax.hun accepted this revision.Aug 12 2022, 8:57 AM
This revision was landed with ongoing or failed builds.Aug 12 2022, 9:29 AM
This revision was automatically updated to reflect the committed changes.