-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use logs in Adt_rel
#1207
Use logs in Adt_rel
#1207
Conversation
eea1760
to
539dfa4
Compare
src/lib/reasoners/adt_rel.ml
Outdated
"%a" X.print r | ||
|
||
let assume l = | ||
Logs.debug ~src:Sources.adt_rel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use Logs.debug ~src:...
. Instead, define module Log = (val Logs.src_log Sources.adt_rel)
at the top of the file, and use Log.debug
directly. See the example from Logs
's documentation.
let pp_domains loc domains = | ||
if Options.get_debug_adt () then begin | ||
print_dbg ~flushed:false ~module_name:"Adt_rel" | ||
"@[<v 2>--ADT env %s ---------------------------------@ " loc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this PR removes the the ---
separators that can make the logs more readable; is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is intentional because Logs
uses correctly indentations. So we will get something like that:
[DEBUG][Adt_rel] environment before assume:
the environment here
....
[DEBUG][Adt_rel] ...
The indentation makes clear in which messages we are.
2799f7f
to
17847a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please merge #1206 first and rebase before merging.
17847a7
to
2828de1
Compare
Actually I already rebased it and I expected that github will skip empty commit. I rebased it again to be sure but now the CI is dead :D (it is not our fault). |
This PR is rebased on #1204 and #1206