Page MenuHomePhabricator

[analyzer] Remove the default value arg from getChecker*Option
ClosedPublic

Authored by Szelethus on Mar 10 2019, 2:36 PM.

Details

Summary

Since D57922, the config table contains every checker option, and it's default value, so having it as an argument for getChecker*Option is redundant.

By the time any of the getChecker*Option function is called, we verified the value in CheckerRegistry (after D57860), so we can confidently assert here, as any irregularities detected at this point must be a programmer error. However, in compatibility mode, verification won't happen, so the default value must be restored.

This implies something else, other than adding removing one more potential point of failure -- debug.ConfigDumper will always contain valid values, and that's kinda nice :D

Diff Detail

Repository
rL LLVM

Event Timeline

Szelethus created this revision.Mar 10 2019, 2:36 PM

This is straightforward.

Szelethus updated this revision to Diff 190996.Mar 16 2019, 1:02 PM

Remove the default argument from the plugin, also add a test case for it.

NoQ accepted this revision.Mar 25 2019, 2:28 PM
NoQ added inline comments.
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
148–150 ↗(On Diff #190996)

Even though operator*() would have asserted that there's a value, i really appreciate the user-friendly assert message.

rnkovacs added inline comments.Mar 26 2019, 4:38 AM
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
148–150 ↗(On Diff #190996)

Sorry for the impertinence, but there's a typo: CheckerRegisrty.

171 ↗(On Diff #190996)

And the same typo here.

This revision was not accepted when it landed; it landed in state Needs Review.May 17 2019, 8:49 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2019, 8:49 AM