[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] [analyzer] Refactoring AnalyzerOptions
From: Kristóf_Umann via cfe-dev <cfe-dev () lists ! llvm ! org>
Date: 2018-10-30 14:43:07
Message-ID: CAGcXOD7nbysjrhTqo+tBM+npCr16nNpWK4oXGFMiAVcOhTSZfQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
The biggest complication is the existence of external plugins. What are
your feelings on the second solution I proposed? Whether I use a .def file
or a .td file makes little difference to me at the moment, that's something
I can always change, if I have a lot of free time and more then 3-4
handfuls of hair.
Kristóf Umann <dkszelethus@gmail.com> ezt írta (időpont: 2018. okt. 29., H,
19:57):
> Actually, I already have a fork on which I managed to get this info into
> Checkers.td (and have come to the same conclusion about tblgen being
> difficult to manage).
>
> Hashtags sound awesome, but I'll probably be burnt out with options by the
> time I finish the already planned changes.
>
> On 29 Oct 2018 19:39, "Artem Dergachev" <noqnoqneo@gmail.com> wrote:
>
> What do you think about refactoring Checkers.td into a .def-file and
> listing checker options there? Eg.,
>
> CHECKER(Malloc, core,
> "Check for memory leaks, double free, and use-after-free
> problems.")
> OPTION(Malloc, Optimistic, bool, false,
> "A useless option that needs to be removed.")
>
> CHECKER(PthreadLock, alpha.unix,
> "Simple lock -> unlock checker"
>
> We could also de-duplicate packages (though i don't think that's
> necessary, as it's a matter of simple string prefix comparison):
>
> BEGIN_PACKAGE(unix, alpha)
> CHECKER(PthreadLock, "Simple lock -> unlock checker")
> CHECKER(...)
> OPTION(...)
> END_PACKAGE
>
> =====
>
> As an unrelated note, i've been dreaming for a while now about replacing
> packages with hashtags for more flexibility. Eg.,
>
> CHECKER(PthreadLock, "Simple lock -> unlock checker", "#alpha #posix
> #pathsensitive #threading")
>
> Of course, we can always keep packages around for backwards compatibility.
>
>
>
> On 10/26/18 3:17 PM, Kristóf Umann wrote:
>
>> Too bad I cant edit mails.
>>
>> Where I talked about extracting all isn't easily accessable fields and
>> related methods to CheckerManagerData, I actually meant *easily* accessible
>> (since some checkers actually need to access LangOptions, as well as
>> AnalyerOptions, both avaible when other similar -help options are handled).
>>
>> On 26 Oct 2018 20:58, "Kristóf Umann" <dkszelethus@gmail.com <mailto:
>> dkszelethus@gmail.com>> wrote:
>>
>> Hi!
>>
>> We did have a thread about this but with a very misleading title,
>> so here's a link to that, and I'll get into this mail:
>> http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html
>> <http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html>
>>
>> AnalyzerOptions shouldn't be mutable once fully initialized (which
>> should be achieveble by the time the actual analysis begins), and
>> the greatest enemies of this idea are checker options, because
>> * we can either forget about collecting all checker options and
>> possibly diagnose them, and let checkers use AnalyzerOptions as a
>> sort of set of user supplied options. This is the current state of
>> things, and should my non-checker option refactoring effort go
>> through, AnalyzerOptions can be made const straight away.
>> * we could supply a mutable AnalyzerOptions object to the checkers
>> when registering, let them register and evaluate their options,
>> and make it immutable for the rest of the analysis.
>>
>> I'm highly in favour of the second option, but it's a non-trivial
>> issue, mostly because of external plugins, which is why I'm
>> looking for some feedback on my ideas.
>>
>> In order to register (and, more importantly, initialize) checkers,
>> one needs to have access to a CheckerManager object, which isn't
>> trivial to create, which makes it impossible to implement a help
>> flag (like -analyzer-checker-help or the proposed
>> -analyzer-config-help). I'm proposing two possible solutions.
>>
>> 1. Extract everything that isn't easily accessible to a new
>> CheckerManagerData class, make CheckerManager only responsible for
>> interacting (but not registering) checkers. I've got a fork on
>> which I managed to get this working, but I disliked this approach,
>> and went on to find a better solution.
>>
>> 2. Force checkers to properly register their options in a new,
>> registerOptionsFor##CHECKERNAME function, which would take
>> AnalyzerOptions as a parameter, alongside register##CHECKERNAME.
>> This would add one more complication to the already
>> very-not-trivial registering process, but could also be
>> autogenerated using tblgen.
>>
>> It's clear to me that the second option is superior to the second,
>> but going forward with either is a lot of work, so I'm looking for
>> feedback.
>>
>> Thanks to everyone who already took the time to help me with this
>> effort!
>>
>> Cheers,
>> Kristóf
>>
>>
>
>
[Attachment #5 (text/html)]
<div dir="ltr">The biggest complication is the existence of external plugins. What \
are your feelings on the second solution I proposed? Whether I use a .def file or a \
.td file makes little difference to me at the moment, that's something I can \
always change, if I have a lot of free time and more then 3-4 handfuls of \
hair.<br></div><br><div class="gmail_quote"><div dir="ltr">Kristóf Umann <<a \
href="mailto:dkszelethus@gmail.com">dkszelethus@gmail.com</a>> ezt írta \
(időpont: 2018. okt. 29., H, 19:57):<br></div><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="auto">Actually, I already have a fork on which I managed to get this info into \
Checkers.td (and have come to the same conclusion about tblgen being difficult to \
manage).<br><div class="gmail_extra" dir="auto"><br></div><div class="gmail_extra" \
dir="auto">Hashtags sound awesome, but I'll probably be burnt out with options by \
the time I finish the already planned changes.</div><div class="gmail_extra" \
dir="auto"><br><div class="gmail_quote">On 29 Oct 2018 19:39, "Artem \
Dergachev" <<a href="mailto:noqnoqneo@gmail.com" \
target="_blank">noqnoqneo@gmail.com</a>> wrote:<br type="attribution"><blockquote \
class="m_-5451842506843134508quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">What do you think about refactoring Checkers.td into a \
.def-file and listing checker options there? Eg.,<br> <br>
CHECKER(Malloc, core,<br>
"Check for memory leaks, double free, and use-after-free \
problems.")<br> OPTION(Malloc, Optimistic, bool, false,<br>
"A useless option that needs to be removed.")<br>
<br>
CHECKER(PthreadLock, alpha.unix,<br>
"Simple lock -> unlock checker"<br>
<br>
We could also de-duplicate packages (though i don't think that's necessary, \
as it's a matter of simple string prefix comparison):<br> <br>
BEGIN_PACKAGE(unix, alpha)<br>
CHECKER(PthreadLock, "Simple lock -> unlock checker")<br>
CHECKER(...)<br>
OPTION(...)<br>
END_PACKAGE<br>
<br>
=====<br>
<br>
As an unrelated note, i've been dreaming for a while now about replacing packages \
with hashtags for more flexibility. Eg.,<br> <br>
CHECKER(PthreadLock, "Simple lock -> unlock checker", \
"#alpha #posix #pathsensitive #threading")<br> <br>
Of course, we can always keep packages around for backwards compatibility.<div \
class="m_-5451842506843134508quoted-text"><br> <br>
<br>
On 10/26/18 3:17 PM, Kristóf Umann wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="m_-5451842506843134508quoted-text"> Too bad I \
cant edit mails.<br> <br>
Where I talked about extracting all isn't easily accessable fields and related \
methods to CheckerManagerData, I actually meant *easily* accessible (since some \
checkers actually need to access LangOptions, as well as AnalyerOptions, both avaible \
when other similar -help options are handled).<br> <br></div><div \
class="m_-5451842506843134508elided-text"> On 26 Oct 2018 20:58, "Kristóf \
Umann" <<a href="mailto:dkszelethus@gmail.com" \
target="_blank">dkszelethus@gmail.com</a> <mailto:<a \
href="mailto:dkszelethus@gmail.com" target="_blank">dkszelethus@gmail.com</a>>> \
wrote:<br> <br>
Hi!<br>
<br>
We did have a thread about this but with a very misleading title,<br>
so here's a link to that, and I'll get into this mail:<br>
<a href="http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html" \
rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html</a><br>
<<a href="http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html" \
rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html</a>><br>
<br>
AnalyzerOptions shouldn't be mutable once fully initialized (which<br>
should be achieveble by the time the actual analysis begins), and<br>
the greatest enemies of this idea are checker options, because<br>
* we can either forget about collecting all checker options and<br>
possibly diagnose them, and let checkers use AnalyzerOptions as a<br>
sort of set of user supplied options. This is the current state of<br>
things, and should my non-checker option refactoring effort go<br>
through, AnalyzerOptions can be made const straight away.<br>
* we could supply a mutable AnalyzerOptions object to the checkers<br>
when registering, let them register and evaluate their options,<br>
and make it immutable for the rest of the analysis.<br>
<br>
I'm highly in favour of the second option, but it's a non-trivial<br>
issue, mostly because of external plugins, which is why I'm<br>
looking for some feedback on my ideas.<br>
<br>
In order to register (and, more importantly, initialize) checkers,<br>
one needs to have access to a CheckerManager object, which isn't<br>
trivial to create, which makes it impossible to implement a help<br>
flag (like -analyzer-checker-help or the proposed<br>
-analyzer-config-help). I'm proposing two possible solutions.<br>
<br>
1. Extract everything that isn't easily accessible to a new<br>
CheckerManagerData class, make CheckerManager only responsible for<br>
interacting (but not registering) checkers. I've got a fork on<br>
which I managed to get this working, but I disliked this approach,<br>
and went on to find a better solution.<br>
<br>
2. Force checkers to properly register their options in a new,<br>
registerOptionsFor##CHECKERNAME function, which would take<br>
AnalyzerOptions as a parameter, alongside register##CHECKERNAME.<br>
This would add one more complication to the already<br>
very-not-trivial registering process, but could also be<br>
autogenerated using tblgen.<br>
<br>
It's clear to me that the second option is superior to the second,<br>
but going forward with either is a lot of work, so I'm looking for<br>
feedback.<br>
<br>
Thanks to everyone who already took the time to help me with this<br>
effort!<br>
<br>
Cheers,<br>
Kristóf<br>
<br>
</div></blockquote>
<br>
</blockquote></div><br></div></div>
</blockquote></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-dev mailing list
cfe-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic