Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Gábor Horváth via cfe-commits]

2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):

I replied to a commit in the wrong thread (https://reviews.llvm.org/rL326323),
sorry.
Here are the important bits:

This change introduced the following cyclic dependency in the build system:
StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.


I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CMakeLists.txt

Do I miss something here?

Thanks in advance,
Gábor


I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
broke our internal integrate (we use a different buildsystem, which breaks
on cyclic deps) and it's really messy to workaround it since CrossTU
depends on both Index and Frontend.
Moving the code that uses CrossTU from StaticAnalyzerCore to
StaticAnalyzerFrontend should probably fix it, but I don't have enough
context to come up with a fix.



On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
cfe-commits  wrote:

> a.sidorin reopened this revision.
> a.sidorin added a comment.
>
> The changes were reverted: http://llvm.org/viewvc/llvm-
> project?rev=326432&view=rev
> Gabor, could you take a look?
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D30691
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


-- 
Regards,
Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Gábor Horváth via cfe-commits]

I am away from my workstation so I would really appreciate if you could
recommit.

Thanks in advance,
Gábor

2018. márc. 1. 15:28 ezt írta ("Ilya Biryukov" ):

> You're right. We have this extra dependency in our internal build files
> for some reason and I missed that.
> It's totally my fault.
>
> Should I resubmit the patch that I reverted or you would rather do it
> yourself?
>
>
>
> On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth  wrote:
>
>>
>>
>> 2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):
>>
>> I replied to a commit in the wrong thread (https://reviews.llvm.org/
>> rL326323), sorry.
>> Here are the important bits:
>>
>> This change introduced the following cyclic dependency in the build
>> system: StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
>>
>>
>> I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
>> https://github.com/llvm-mirror/clang/blob/master/lib/
>> Frontend/CMakeLists.txt
>>
>> Do I miss something here?
>>
>> Thanks in advance,
>> Gábor
>>
>>
>> I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
>> broke our internal integrate (we use a different buildsystem, which breaks
>> on cyclic deps) and it's really messy to workaround it since CrossTU
>> depends on both Index and Frontend.
>> Moving the code that uses CrossTU from StaticAnalyzerCore to
>> StaticAnalyzerFrontend should probably fix it, but I don't have enough
>> context to come up with a fix.
>>
>>
>>
>> On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
>> cfe-commits  wrote:
>>
>>> a.sidorin reopened this revision.
>>> a.sidorin added a comment.
>>>
>>> The changes were reverted: http://llvm.org/viewvc/llvm-
>>> project?rev=326432&view=rev
>>> Gabor, could you take a look?
>>>
>>>
>>> Repository:
>>>   rC Clang
>>>
>>> https://reviews.llvm.org/D30691
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>>
>>
>
> --
> Regards,
> Ilya Biryukov
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368459 - Fix a build bot failure and multiple warnings instances for range base for loops

[Gábor Horváth via cfe-commits]

Hmm, strange. Looking into it! If I do not manage to find the root cause in
a few minutes I will revert!

On Fri, 9 Aug 2019 at 11:32, Jonas Devlieghere 
wrote:

> I think this is causing a stage2 failure:
>
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/124/consoleFull#-95886206949ba4694-19c4-4d7e-bec5-911270d8a58c
>
> On Fri, Aug 9, 2019 at 10:41 AM Gabor Horvath via cfe-commits
>  wrote:
> >
> > Author: xazax
> > Date: Fri Aug  9 10:42:41 2019
> > New Revision: 368459
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=368459&view=rev
> > Log:
> > Fix a build bot failure and multiple warnings instances for range base
> for loops
> >
> > Modified:
> > cfe/trunk/lib/Sema/SemaInit.cpp
> > cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
> >
> > Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368459&r1=368458&r2=368459&view=diff
> >
> ==
> > --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 10:42:41 2019
> > @@ -6616,7 +6616,7 @@ static void handleGslAnnotatedTypes(Indi
> >  return;
> >} else if (auto *OCE = dyn_cast(Call)) {
> >  FunctionDecl *Callee = OCE->getDirectCallee();
> > -if (Callee->isCXXInstanceMember() &&
> > +if (Callee && Callee->isCXXInstanceMember() &&
> >  shouldTrackImplicitObjectArg(cast(Callee)))
> >VisitPointerArg(Callee, OCE->getArg(0));
> >  return;
> > @@ -7070,8 +7070,11 @@ static SourceRange nextPathEntryRange(co
> >// supporting lifetime extension.
> >break;
> >
> > -case IndirectLocalPathEntry::DefaultInit:
> >  case IndirectLocalPathEntry::VarInit:
> > +  if (cast(Path[I].D)->isImplicit())
> > +return SourceRange();
> > +  LLVM_FALLTHROUGH;
> > +case IndirectLocalPathEntry::DefaultInit:
> >return Path[I].E->getSourceRange();
> >  }
> >}
> > @@ -7133,7 +7136,7 @@ void Sema::checkInitializerLifetime(cons
> >  return false;
> >}
> >
> > -  if (IsGslPtrInitWithGslTempOwner) {
> > +  if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
> >  Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) <<
> DiagRange;
> >  return false;
> >}
> >
> > Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368459&r1=368458&r2=368459&view=diff
> >
> ==
> > --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
> > +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug  9
> 10:42:41 2019
> > @@ -201,6 +201,13 @@ void danglingReferenceFromTempOwner() {
> >  std::vector getTempVec();
> >  std::optional> getTempOptVec();
> >
> > +void testLoops() {
> > +  for (auto i : getTempVec()) // ok
> > +;
> > +  for (auto i : *getTempOptVec()) // expected-warning {{object backing
> the pointer will be destroyed at the end of the full-expression}}
> > +;
> > +}
> > +
> >  int &usedToBeFalsePositive(std::vector &v) {
> >std::vector::iterator it = v.begin();
> >int& value = *it;
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368459 - Fix a build bot failure and multiple warnings instances for range base for loops

[Gábor Horváth via cfe-commits]

I reverted but I cannot reproduce this locally on a linux box. Is there any
way to get more information from the build bot (like preprocessed files?)?

On Fri, 9 Aug 2019 at 11:38, Gábor Horváth  wrote:

> Hmm, strange. Looking into it! If I do not manage to find the root cause
> in a few minutes I will revert!
>
> On Fri, 9 Aug 2019 at 11:32, Jonas Devlieghere 
> wrote:
>
>> I think this is causing a stage2 failure:
>>
>> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/124/consoleFull#-95886206949ba4694-19c4-4d7e-bec5-911270d8a58c
>>
>> On Fri, Aug 9, 2019 at 10:41 AM Gabor Horvath via cfe-commits
>>  wrote:
>> >
>> > Author: xazax
>> > Date: Fri Aug  9 10:42:41 2019
>> > New Revision: 368459
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=368459&view=rev
>> > Log:
>> > Fix a build bot failure and multiple warnings instances for range base
>> for loops
>> >
>> > Modified:
>> > cfe/trunk/lib/Sema/SemaInit.cpp
>> > cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> >
>> > Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368459&r1=368458&r2=368459&view=diff
>> >
>> ==
>> > --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> > +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 10:42:41 2019
>> > @@ -6616,7 +6616,7 @@ static void handleGslAnnotatedTypes(Indi
>> >  return;
>> >} else if (auto *OCE = dyn_cast(Call)) {
>> >  FunctionDecl *Callee = OCE->getDirectCallee();
>> > -if (Callee->isCXXInstanceMember() &&
>> > +if (Callee && Callee->isCXXInstanceMember() &&
>> >  shouldTrackImplicitObjectArg(cast(Callee)))
>> >VisitPointerArg(Callee, OCE->getArg(0));
>> >  return;
>> > @@ -7070,8 +7070,11 @@ static SourceRange nextPathEntryRange(co
>> >// supporting lifetime extension.
>> >break;
>> >
>> > -case IndirectLocalPathEntry::DefaultInit:
>> >  case IndirectLocalPathEntry::VarInit:
>> > +  if (cast(Path[I].D)->isImplicit())
>> > +return SourceRange();
>> > +  LLVM_FALLTHROUGH;
>> > +case IndirectLocalPathEntry::DefaultInit:
>> >return Path[I].E->getSourceRange();
>> >  }
>> >}
>> > @@ -7133,7 +7136,7 @@ void Sema::checkInitializerLifetime(cons
>> >  return false;
>> >}
>> >
>> > -  if (IsGslPtrInitWithGslTempOwner) {
>> > +  if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
>> >  Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) <<
>> DiagRange;
>> >  return false;
>> >}
>> >
>> > Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368459&r1=368458&r2=368459&view=diff
>> >
>> ==
>> > --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
>> > +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug  9
>> 10:42:41 2019
>> > @@ -201,6 +201,13 @@ void danglingReferenceFromTempOwner() {
>> >  std::vector getTempVec();
>> >  std::optional> getTempOptVec();
>> >
>> > +void testLoops() {
>> > +  for (auto i : getTempVec()) // ok
>> > +;
>> > +  for (auto i : *getTempOptVec()) // expected-warning {{object backing
>> the pointer will be destroyed at the end of the full-expression}}
>> > +;
>> > +}
>> > +
>> >  int &usedToBeFalsePositive(std::vector &v) {
>> >std::vector::iterator it = v.begin();
>> >int& value = *it;
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368459 - Fix a build bot failure and multiple warnings instances for range base for loops

[Gábor Horváth via cfe-commits]

I tried both with libc++ instead of libstdc++ and modules but none of them
reproduced in my local machine I would be glad if you could try it locally
as well!

Thanks in advance,
Gabor

On Fri, 9 Aug 2019 at 13:10, Jonas Devlieghere 
wrote:

> The bot is a little special in that it has modules enabled. Maybe that
> explains it? Let me know if that doesn't work and I can try
> reproducing locally.
>
> On Fri, Aug 9, 2019 at 12:02 PM Gábor Horváth  wrote:
> >
> > I reverted but I cannot reproduce this locally on a linux box. Is there
> any way to get more information from the build bot (like preprocessed
> files?)?
> >
> > On Fri, 9 Aug 2019 at 11:38, Gábor Horváth  wrote:
> >>
> >> Hmm, strange. Looking into it! If I do not manage to find the root
> cause in a few minutes I will revert!
> >>
> >> On Fri, 9 Aug 2019 at 11:32, Jonas Devlieghere 
> wrote:
> >>>
> >>> I think this is causing a stage2 failure:
> >>>
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/124/consoleFull#-95886206949ba4694-19c4-4d7e-bec5-911270d8a58c
> >>>
> >>> On Fri, Aug 9, 2019 at 10:41 AM Gabor Horvath via cfe-commits
> >>>  wrote:
> >>> >
> >>> > Author: xazax
> >>> > Date: Fri Aug  9 10:42:41 2019
> >>> > New Revision: 368459
> >>> >
> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=368459&view=rev
> >>> > Log:
> >>> > Fix a build bot failure and multiple warnings instances for range
> base for loops
> >>> >
> >>> > Modified:
> >>> > cfe/trunk/lib/Sema/SemaInit.cpp
> >>> > cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
> >>> >
> >>> > Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> >>> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368459&r1=368458&r2=368459&view=diff
> >>> >
> ==
> >>> > --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> >>> > +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 10:42:41 2019
> >>> > @@ -6616,7 +6616,7 @@ static void handleGslAnnotatedTypes(Indi
> >>> >  return;
> >>> >} else if (auto *OCE = dyn_cast(Call)) {
> >>> >  FunctionDecl *Callee = OCE->getDirectCallee();
> >>> > -if (Callee->isCXXInstanceMember() &&
> >>> > +if (Callee && Callee->isCXXInstanceMember() &&
> >>> >  shouldTrackImplicitObjectArg(cast(Callee)))
> >>> >VisitPointerArg(Callee, OCE->getArg(0));
> >>> >  return;
> >>> > @@ -7070,8 +7070,11 @@ static SourceRange nextPathEntryRange(co
> >>> >// supporting lifetime extension.
> >>> >break;
> >>> >
> >>> > -case IndirectLocalPathEntry::DefaultInit:
> >>> >  case IndirectLocalPathEntry::VarInit:
> >>> > +  if (cast(Path[I].D)->isImplicit())
> >>> > +return SourceRange();
> >>> > +  LLVM_FALLTHROUGH;
> >>> > +case IndirectLocalPathEntry::DefaultInit:
> >>> >return Path[I].E->getSourceRange();
> >>> >  }
> >>> >}
> >>> > @@ -7133,7 +7136,7 @@ void Sema::checkInitializerLifetime(cons
> >>> >  return false;
> >>> >}
> >>> >
> >>> > -  if (IsGslPtrInitWithGslTempOwner) {
> >>> > +  if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
> >>> >  Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) <<
> DiagRange;
> >>> >  return false;
> >>> >}
> >>> >
> >>> > Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
> >>> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368459&r1=368458&r2=368459&view=diff
> >>> >
> ==
> >>> > --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
> >>> > +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug  9
> 10:42:41 2019
> >>> > @@ -201,6 +201,13 @@ void danglingReferenceFromTempOwner() {
> >>> >  std::vector getTempVec();
> >>> >  std::optional> getTempOptVec();
> >>> >
> >>> > +void testLoops() {
> >>> > +  for (auto i : getTempVec()) // ok
> >>> > +;
> >>> > +  for (auto i : *getTempOptVec()) // expected-warning {{object
> backing the pointer will be destroyed at the end of the full-expression}}
> >>> > +;
> >>> > +}
> >>> > +
> >>> >  int &usedToBeFalsePositive(std::vector &v) {
> >>> >std::vector::iterator it = v.begin();
> >>> >int& value = *it;
> >>> >
> >>> >
> >>> > ___
> >>> > cfe-commits mailing list
> >>> > cfe-commits@lists.llvm.org
> >>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368459 - Fix a build bot failure and multiple warnings instances for range base for loops

[Gábor Horváth via cfe-commits]

Also, how does this stage2 bot work?
I clearly see which revision is being compiled from the logs but it is not
apparent to me what is the version of the compiler that is used to compiler
it. Is it possible that due to some race condition a compiler before my fix
is picked up and that is causing the failures?

Thanks,
Gabor

On Fri, 9 Aug 2019 at 15:05, Gábor Horváth  wrote:

> I tried both with libc++ instead of libstdc++ and modules but none of them
> reproduced in my local machine I would be glad if you could try it locally
> as well!
>
> Thanks in advance,
> Gabor
>
> On Fri, 9 Aug 2019 at 13:10, Jonas Devlieghere 
> wrote:
>
>> The bot is a little special in that it has modules enabled. Maybe that
>> explains it? Let me know if that doesn't work and I can try
>> reproducing locally.
>>
>> On Fri, Aug 9, 2019 at 12:02 PM Gábor Horváth 
>> wrote:
>> >
>> > I reverted but I cannot reproduce this locally on a linux box. Is there
>> any way to get more information from the build bot (like preprocessed
>> files?)?
>> >
>> > On Fri, 9 Aug 2019 at 11:38, Gábor Horváth  wrote:
>> >>
>> >> Hmm, strange. Looking into it! If I do not manage to find the root
>> cause in a few minutes I will revert!
>> >>
>> >> On Fri, 9 Aug 2019 at 11:32, Jonas Devlieghere 
>> wrote:
>> >>>
>> >>> I think this is causing a stage2 failure:
>> >>>
>> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/124/consoleFull#-95886206949ba4694-19c4-4d7e-bec5-911270d8a58c
>> >>>
>> >>> On Fri, Aug 9, 2019 at 10:41 AM Gabor Horvath via cfe-commits
>> >>>  wrote:
>> >>> >
>> >>> > Author: xazax
>> >>> > Date: Fri Aug  9 10:42:41 2019
>> >>> > New Revision: 368459
>> >>> >
>> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=368459&view=rev
>> >>> > Log:
>> >>> > Fix a build bot failure and multiple warnings instances for range
>> base for loops
>> >>> >
>> >>> > Modified:
>> >>> > cfe/trunk/lib/Sema/SemaInit.cpp
>> >>> > cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> >>> >
>> >>> > Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> >>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368459&r1=368458&r2=368459&view=diff
>> >>> >
>> ==
>> >>> > --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> >>> > +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 10:42:41 2019
>> >>> > @@ -6616,7 +6616,7 @@ static void handleGslAnnotatedTypes(Indi
>> >>> >  return;
>> >>> >} else if (auto *OCE = dyn_cast(Call)) {
>> >>> >  FunctionDecl *Callee = OCE->getDirectCallee();
>> >>> > -if (Callee->isCXXInstanceMember() &&
>> >>> > +if (Callee && Callee->isCXXInstanceMember() &&
>> >>> >  shouldTrackImplicitObjectArg(cast(Callee)))
>> >>> >VisitPointerArg(Callee, OCE->getArg(0));
>> >>> >  return;
>> >>> > @@ -7070,8 +7070,11 @@ static SourceRange nextPathEntryRange(co
>> >>> >// supporting lifetime extension.
>> >>> >break;
>> >>> >
>> >>> > -case IndirectLocalPathEntry::DefaultInit:
>> >>> >  case IndirectLocalPathEntry::VarInit:
>> >>> > +  if (cast(Path[I].D)->isImplicit())
>> >>> > +return SourceRange();
>> >>> > +  LLVM_FALLTHROUGH;
>> >>> > +case IndirectLocalPathEntry::DefaultInit:
>> >>> >return Path[I].E->getSourceRange();
>> >>> >  }
>> >>> >}
>> >>> > @@ -7133,7 +7136,7 @@ void Sema::checkInitializerLifetime(cons
>> >>> >  return false;
>> >>> >}
>> >>> >
>> >>> > -  if (IsGslPtrInitWithGslTempOwner) {
>> >>> > +  if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
>> >>> >  Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) <<
>> DiagRange;
>> >>> >  return false;
>> >>> >}
>> >>> >
>> >>> > Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> >>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368459&r1=368458&r2=368459&view=diff
>> >>> >
>> ==
>> >>> > --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
>> >>> > +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug  9
>> 10:42:41 2019
>> >>> > @@ -201,6 +201,13 @@ void danglingReferenceFromTempOwner() {
>> >>> >  std::vector getTempVec();
>> >>> >  std::optional> getTempOptVec();
>> >>> >
>> >>> > +void testLoops() {
>> >>> > +  for (auto i : getTempVec()) // ok
>> >>> > +;
>> >>> > +  for (auto i : *getTempOptVec()) // expected-warning {{object
>> backing the pointer will be destroyed at the end of the full-expression}}
>> >>> > +;
>> >>> > +}
>> >>> > +
>> >>> >  int &usedToBeFalsePositive(std::vector &v) {
>> >>> >std::vector::iterator it = v.begin();
>> >>> >int& value = *it;
>> >>> >
>> >>> >
>> >>> > ___
>> >>> > cfe-commits mailing list
>> >>> > cfe-commits@lists.llvm.org
>> >>> > https://lists.ll

Re: r368499 - Attempt to reapply "Even more warnings utilizing gsl::Owner/gsl::Pointer annotations"

[Gábor Horváth via cfe-commits]

Indeed!

There pointer is moved later on! Interestingly, I run these warnings on
300+ projects and none of them had this pattern. Will revert or fix the
patch soon.

On Fri, 9 Aug 2019 at 17:13, Nico Weber  wrote:

> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/14045
>
> FAILED:
> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o
>
> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/install/stage1/bin/clang++
>   -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/Frontend
> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend
> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/include
> -Itools/clang/include -Iinclude
> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/include
> -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time
> -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> -Wmissing-field-initializers -pedantic -Wno-long-long
> -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type
> -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion
> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common
> -Woverloaded-virtual -Wno-nested-anon-types -O3-UNDEBUG
>  -fno-exceptions -fno-rtti -MD -MT
> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o
> -MF
> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o.d
> -o
> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o
> -c
> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend/ASTConsumers.cpp
> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend/ASTConsumers.cpp:40:22:
> error: binding reference member 'Out' to stack allocated parameter 'Out'
> [-Werror,-Wdangling-field]
> : Out(Out ? *Out : llvm::outs()), OwnedOut(std::move(Out)),
>  ^~~
> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend/ASTConsumers.cpp:98:18:
> note: reference member declared here
> raw_ostream &Out;
>  ^
> 1 error generated.
>
> http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/ASTConsumers.cpp#40
>
> That looks like a false positive.
>
> On Fri, Aug 9, 2019 at 7:02 PM Gabor Horvath via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Fri Aug  9 16:03:50 2019
>> New Revision: 368499
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=368499&view=rev
>> Log:
>> Attempt to reapply "Even more warnings utilizing gsl::Owner/gsl::Pointer
>> annotations"
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368499&r1=368498&r2=368499&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 16:03:50 2019
>> @@ -6568,19 +6568,33 @@ static bool shouldTrackImplicitObjectArg
>>if (auto *Conv = dyn_cast_or_null(Callee))
>>  if (isRecordWithAttr(Conv->getConversionType()))
>>return true;
>> -  if (!Callee->getParent()->isInStdNamespace() ||
>> !Callee->getIdentifier())
>> +  if (!Callee->getParent()->isInStdNamespace())
>>  return false;
>>if (!isRecordWithAttr(Callee->getThisObjectType()) &&
>>!isRecordWithAttr(Callee->getThisObjectType()))
>>  return false;
>> -  if (!isRecordWithAttr(Callee->getReturnType()) &&
>> -  !Callee->getReturnType()->isPointerType())
>> -return false;
>> -  return llvm::StringSwitch(Callee->getName())
>> -  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
>> -  .Cases("end", "rend", "cend", "crend", true)
>> -  .Cases("c_str", "data", "get", true)
>> -  .Default(false);
>> +  if (Callee->getReturnType()->isPointerType() ||
>> +  isRecordWithAttr(Callee->getReturnType())) {
>> +if (!Callee->getIdentifier())
>> +  return false;
>> +return llvm::StringSwitch(Callee->getName())
>> +.Cases("begin", "rbegin", "cbegin", "crbegin", true)
>> +.Cases("end", "rend", "cend", "crend", true)
>> +.Cases("c_str", "data", "get", true)
>> +// Map and set types.
>> +.Cases("find", "equal_range", "lower_bound", "upper_bound", true)
>> +.Default(false);
>> +  } else if (Callee->getReturnType()->isReferenceType()) {
>> +if (!Callee->getIdentifier()) {
>> +  auto OO = Callee->getOverloadedOperator();
>> +  return OO == OverloadedOperatorKind::OO_Subscript ||
>> + OO == OverloadedOperatorKind::OO_Star;
>> +}
>> +return llvm::StringSwitch(Callee->getName())
>

Re: r368499 - Attempt to reapply "Even more warnings utilizing gsl::Owner/gsl::Pointer annotations"

[Gábor Horváth via cfe-commits]

It was an easy fix! In case any problem persists I will revert the commit
with the fix itself.

On Fri, 9 Aug 2019 at 17:16, Gábor Horváth  wrote:

> Indeed!
>
> There pointer is moved later on! Interestingly, I run these warnings on
> 300+ projects and none of them had this pattern. Will revert or fix the
> patch soon.
>
> On Fri, 9 Aug 2019 at 17:13, Nico Weber  wrote:
>
>> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/14045
>>
>> FAILED:
>> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o
>>
>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/install/stage1/bin/clang++
>>   -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
>> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/Frontend
>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend
>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/include
>> -Itools/clang/include -Iinclude
>> -I/home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/include
>> -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time
>> -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra
>> -Wno-unused-parameter -Wwrite-strings -Wcast-qual
>> -Wmissing-field-initializers -pedantic -Wno-long-long
>> -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type
>> -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion
>> -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common
>> -Woverloaded-virtual -Wno-nested-anon-types -O3-UNDEBUG
>>  -fno-exceptions -fno-rtti -MD -MT
>> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o
>> -MF
>> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o.d
>> -o
>> tools/clang/lib/Frontend/CMakeFiles/obj.clangFrontend.dir/ASTConsumers.cpp.o
>> -c
>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend/ASTConsumers.cpp
>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend/ASTConsumers.cpp:40:22:
>> error: binding reference member 'Out' to stack allocated parameter 'Out'
>> [-Werror,-Wdangling-field]
>> : Out(Out ? *Out : llvm::outs()), OwnedOut(std::move(Out)),
>>  ^~~
>> /home/buildslave/buildslave1a/clang-with-lto-ubuntu/llvm.src/tools/clang/lib/Frontend/ASTConsumers.cpp:98:18:
>> note: reference member declared here
>> raw_ostream &Out;
>>  ^
>> 1 error generated.
>>
>> http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/ASTConsumers.cpp#40
>>
>> That looks like a false positive.
>>
>> On Fri, Aug 9, 2019 at 7:02 PM Gabor Horvath via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: xazax
>>> Date: Fri Aug  9 16:03:50 2019
>>> New Revision: 368499
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=368499&view=rev
>>> Log:
>>> Attempt to reapply "Even more warnings utilizing gsl::Owner/gsl::Pointer
>>> annotations"
>>>
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaInit.cpp
>>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368499&r1=368498&r2=368499&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 16:03:50 2019
>>> @@ -6568,19 +6568,33 @@ static bool shouldTrackImplicitObjectArg
>>>if (auto *Conv = dyn_cast_or_null(Callee))
>>>  if (isRecordWithAttr(Conv->getConversionType()))
>>>return true;
>>> -  if (!Callee->getParent()->isInStdNamespace() ||
>>> !Callee->getIdentifier())
>>> +  if (!Callee->getParent()->isInStdNamespace())
>>>  return false;
>>>if (!isRecordWithAttr(Callee->getThisObjectType()) &&
>>>!isRecordWithAttr(Callee->getThisObjectType()))
>>>  return false;
>>> -  if (!isRecordWithAttr(Callee->getReturnType()) &&
>>> -  !Callee->getReturnType()->isPointerType())
>>> -return false;
>>> -  return llvm::StringSwitch(Callee->getName())
>>> -  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
>>> -  .Cases("end", "rend", "cend", "crend", true)
>>> -  .Cases("c_str", "data", "get", true)
>>> -  .Default(false);
>>> +  if (Callee->getReturnType()->isPointerType() ||
>>> +  isRecordWithAttr(Callee->getReturnType())) {
>>> +if (!Callee->getIdentifier())
>>> +  return false;
>>> +return llvm::StringSwitch(Callee->getName())
>>> +.Cases("begin", "rbegin", "cbegin", "crbegin", true)
>>> +.Cases("end", "rend", "cend", "crend", true)
>>> +.Cases("c_str", "data", "get", true)
>>> +// Map and set types.
>>> +.Cases("find", "equal_range", "lower_bound", "upper_bound",
>>> true)
>>> +.Default(false);
>>> +  } else if (Callee->getReturnType()->isReferenceType()) {
>>> +if (!

Re: r368501 - Fix a false positive warning when initializing members with gsl::Owners.

[Gábor Horváth via cfe-commits]

It does! Do you want me to commit that test as well?

On Fri, 9 Aug 2019 at 17:50, Nico Weber  wrote:

> This fixes `+  X(std::unique_ptr up) : pointee(up.get()),
> pointer(std::move(up)) {}` as well, right?
>
> On Fri, Aug 9, 2019 at 8:31 PM Gabor Horvath via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Fri Aug  9 17:32:29 2019
>> New Revision: 368501
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=368501&view=rev
>> Log:
>> Fix a false positive warning when initializing members with gsl::Owners.
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368501&r1=368500&r2=368501&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 17:32:29 2019
>> @@ -7217,6 +7217,11 @@ void Sema::checkInitializerLifetime(cons
>>  if (pathContainsInit(Path))
>>return false;
>>
>> +// Suppress false positives for code like the below:
>> +//   Ctor(unique_ptr up) : member(*up), member2(move(up)) {}
>> +if (IsLocalGslOwner && pathOnlyInitializesGslPointer(Path))
>> +  return false;
>> +
>>  auto *DRE = dyn_cast(L);
>>  auto *VD = DRE ? dyn_cast(DRE->getDecl()) : nullptr;
>>  if (!VD) {
>>
>> Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368501&r1=368500&r2=368501&view=diff
>>
>> ==
>> --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
>> +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug  9
>> 17:32:29 2019
>> @@ -120,6 +120,13 @@ void initLocalGslPtrWithTempOwner() {
>>  }
>>
>>  namespace std {
>> +template struct remove_reference   { typedef T type; };
>> +template struct remove_reference  { typedef T type; };
>> +template struct remove_reference { typedef T type; };
>> +
>> +template
>> +typename remove_reference::type &&move(T &&t) noexcept;
>> +
>>  template 
>>  struct basic_iterator {
>>basic_iterator operator++();
>> @@ -153,6 +160,7 @@ struct basic_string {
>>
>>  template
>>  struct unique_ptr {
>> +  T &operator*();
>>T *get() const;
>>  };
>>
>> @@ -217,3 +225,10 @@ int &doNotFollowReferencesForLocalOwner(
>>  const char *trackThroughMultiplePointer() {
>>return
>> std::basic_string_view(std::basic_string()).begin(); //
>> expected-warning {{returning address of local temporary object}}
>>  }
>> +
>> +struct X {
>> +  X(std::unique_ptr up) : pointee(*up), pointer(std::move(up)) {}
>> +
>> +  int &pointee;
>> +  std::unique_ptr pointer;
>> +};
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

[Gábor Horváth via cfe-commits]

You are right!

I will look into this tomorrow. If you think this is urgent feel free to
revert the commits.

Cheers,
Gabor

On Sat, Aug 10, 2019, 6:41 PM Nico Weber  wrote:

> Hi Gabor,
>
> this makes clang warn on this program:
>
> $ cat test.cc
> #include 
> #include 
>
> struct S {
>   bool operator==(const S &rhs) const;
> };
>
> const S &f(const std::vector &v) {
>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>   return *it;
> }
>
>
> $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path) -isystem
> libcxx/include/ -Wall
> test.cc:10:11: warning: returning reference to local temporary object
> [-Wreturn-stack-address]
>   return *it;
>   ^~
> test.cc:9:15: note: binding reference variable 'it' here
>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>   ^
> 1 warning generated.
>
>
> Is the warning correct here? The `const auto &it` is lifetime-extended to
> the end of the block, and *it should return a reference to an element in
> the vector. Since the vector's passed in, this should be fine. Or am I
> missing something?
>
> Thanks,
> Nico
>
> On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Fri Aug  9 08:16:35 2019
>> New Revision: 368446
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
>> Log:
>> More warnings regarding gsl::Pointer and gsl::Owner attributes
>>
>> Differential Revision: https://reviews.llvm.org/D65120
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/Analysis/inner-pointer.cpp
>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 08:16:35 2019
>> @@ -6564,6 +6564,25 @@ template  static bool isReco
>>return false;
>>  }
>>
>> +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
>> +  if (auto *Conv = dyn_cast_or_null(Callee))
>> +if (isRecordWithAttr(Conv->getConversionType()))
>> +  return true;
>> +  if (!Callee->getParent()->isInStdNamespace() ||
>> !Callee->getIdentifier())
>> +return false;
>> +  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
>> +  !isRecordWithAttr(Callee->getThisObjectType()))
>> +return false;
>> +  if (!isRecordWithAttr(Callee->getReturnType()) &&
>> +  !Callee->getReturnType()->isPointerType())
>> +return false;
>> +  return llvm::StringSwitch(Callee->getName())
>> +  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
>> +  .Cases("end", "rend", "cend", "crend", true)
>> +  .Cases("c_str", "data", "get", true)
>> +  .Default(false);
>> +}
>> +
>>  static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
>>  LocalVisitor Visit) {
>>auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
>> @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
>>};
>>
>>if (auto *MCE = dyn_cast(Call)) {
>> -const FunctionDecl *Callee = MCE->getDirectCallee();
>> -if (auto *Conv = dyn_cast_or_null(Callee))
>> -  if (isRecordWithAttr(Conv->getConversionType()))
>> -VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
>> +const auto *MD = cast_or_null(MCE->getDirectCallee());
>> +if (MD && shouldTrackImplicitObjectArg(MD))
>> +  VisitPointerArg(MD, MCE->getImplicitObjectArgument());
>>  return;
>>}
>>
>>
>> Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/inner-pointer.cpp (original)
>> +++ cfe/trunk/test/Analysis/inner-pointer.cpp Fri Aug  9 08:16:35 2019
>> @@ -1,4 +1,5 @@
>> -// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
>> +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer   \
>> +// RUN:   -Wno-dangling -Wno-dangling-field -Wno-return-stack-address \
>>  // RUN:   %s -analyzer-output=text -verify
>>
>>  #include "Inputs/system-header-simulator-cxx.h"
>>
>> Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>
>> ==
>> --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
>> +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug  9
>> 08:16:35 2019
>> @@ -2,7 +2,6 @@
>>  struct [[gsl::O

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

[Gábor Horváth via cfe-commits]

This should be fixed now.

On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth  wrote:

> You are right!
>
> I will look into this tomorrow. If you think this is urgent feel free to
> revert the commits.
>
> Cheers,
> Gabor
>
> On Sat, Aug 10, 2019, 6:41 PM Nico Weber  wrote:
>
>> Hi Gabor,
>>
>> this makes clang warn on this program:
>>
>> $ cat test.cc
>> #include 
>> #include 
>>
>> struct S {
>>   bool operator==(const S &rhs) const;
>> };
>>
>> const S &f(const std::vector &v) {
>>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>>   return *it;
>> }
>>
>>
>> $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path) -isystem
>> libcxx/include/ -Wall
>> test.cc:10:11: warning: returning reference to local temporary object
>> [-Wreturn-stack-address]
>>   return *it;
>>   ^~
>> test.cc:9:15: note: binding reference variable 'it' here
>>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>>   ^
>> 1 warning generated.
>>
>>
>> Is the warning correct here? The `const auto &it` is lifetime-extended to
>> the end of the block, and *it should return a reference to an element in
>> the vector. Since the vector's passed in, this should be fine. Or am I
>> missing something?
>>
>> Thanks,
>> Nico
>>
>> On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: xazax
>>> Date: Fri Aug  9 08:16:35 2019
>>> New Revision: 368446
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
>>> Log:
>>> More warnings regarding gsl::Pointer and gsl::Owner attributes
>>>
>>> Differential Revision: https://reviews.llvm.org/D65120
>>>
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaInit.cpp
>>> cfe/trunk/test/Analysis/inner-pointer.cpp
>>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 08:16:35 2019
>>> @@ -6564,6 +6564,25 @@ template  static bool isReco
>>>return false;
>>>  }
>>>
>>> +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
>>> +  if (auto *Conv = dyn_cast_or_null(Callee))
>>> +if (isRecordWithAttr(Conv->getConversionType()))
>>> +  return true;
>>> +  if (!Callee->getParent()->isInStdNamespace() ||
>>> !Callee->getIdentifier())
>>> +return false;
>>> +  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
>>> +  !isRecordWithAttr(Callee->getThisObjectType()))
>>> +return false;
>>> +  if (!isRecordWithAttr(Callee->getReturnType()) &&
>>> +  !Callee->getReturnType()->isPointerType())
>>> +return false;
>>> +  return llvm::StringSwitch(Callee->getName())
>>> +  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
>>> +  .Cases("end", "rend", "cend", "crend", true)
>>> +  .Cases("c_str", "data", "get", true)
>>> +  .Default(false);
>>> +}
>>> +
>>>  static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
>>>  LocalVisitor Visit) {
>>>auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
>>> @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
>>>};
>>>
>>>if (auto *MCE = dyn_cast(Call)) {
>>> -const FunctionDecl *Callee = MCE->getDirectCallee();
>>> -if (auto *Conv = dyn_cast_or_null(Callee))
>>> -  if (isRecordWithAttr(Conv->getConversionType()))
>>> -VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
>>> +const auto *MD =
>>> cast_or_null(MCE->getDirectCallee());
>>> +if (MD && shouldTrackImplicitObjectArg(MD))
>>> +  VisitPointerArg(MD, MCE->getImplicitObjectArgument());
>>>  return;
>>>}
>>>
>>>
>>> Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>>
>>> ==
>>> --- cfe/trunk/test/Analysis/inner-pointer.cpp (original)
>>> +++ cfe/trunk/test/Analysis/inner-pointer.cpp Fri Aug  9 08:16:35 2019
>>> @@ -1,4 +1,5 @@
>>> -// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
>>> +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer   \
>>> +// RUN:   -Wno-dangling -Wno-dangling-field -Wno-return-stack-address \
>>>  // RUN:   %s -analyzer-output=text -verify
>>>
>>>  #include "Inputs/system-header-simulator-cxx.h"
>>>
>>> Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>>
>>> ===

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

[Gábor Horváth via cfe-commits]

Great news!

Thanks for all the feedback and patience! :)

On Sun, Aug 11, 2019, 4:35 PM Nico Weber  wrote:

> Confirmed, after r368534 the Chromium builds looks good again. (r368528
> without r368534 regressed something else, but r368534 fixed things. I'm
> assuming you don't need a repro for the breakage in r368528
> without r368534.)
>
> Your warnings even found a bug:
> https://bugs.chromium.org/p/openscreen/issues/detail?id=63 Thanks! :)
>
> On Sun, Aug 11, 2019 at 3:18 PM Gábor Horváth  wrote:
>
>> This should be fixed now.
>>
>> On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth  wrote:
>>
>>> You are right!
>>>
>>> I will look into this tomorrow. If you think this is urgent feel free to
>>> revert the commits.
>>>
>>> Cheers,
>>> Gabor
>>>
>>> On Sat, Aug 10, 2019, 6:41 PM Nico Weber  wrote:
>>>
 Hi Gabor,

 this makes clang warn on this program:

 $ cat test.cc
 #include 
 #include 

 struct S {
   bool operator==(const S &rhs) const;
 };

 const S &f(const std::vector &v) {
   const auto &it = std::find(v.rbegin(), v.rend(), S());
   return *it;
 }


 $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path)
 -isystem libcxx/include/ -Wall
 test.cc:10:11: warning: returning reference to local temporary object
 [-Wreturn-stack-address]
   return *it;
   ^~
 test.cc:9:15: note: binding reference variable 'it' here
   const auto &it = std::find(v.rbegin(), v.rend(), S());
   ^
 1 warning generated.


 Is the warning correct here? The `const auto &it` is lifetime-extended
 to the end of the block, and *it should return a reference to an element in
 the vector. Since the vector's passed in, this should be fine. Or am I
 missing something?

 Thanks,
 Nico

 On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: xazax
> Date: Fri Aug  9 08:16:35 2019
> New Revision: 368446
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
> Log:
> More warnings regarding gsl::Pointer and gsl::Owner attributes
>
> Differential Revision: https://reviews.llvm.org/D65120
>
> Modified:
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/Analysis/inner-pointer.cpp
> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 08:16:35 2019
> @@ -6564,6 +6564,25 @@ template  static bool isReco
>return false;
>  }
>
> +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee)
> {
> +  if (auto *Conv = dyn_cast_or_null(Callee))
> +if (isRecordWithAttr(Conv->getConversionType()))
> +  return true;
> +  if (!Callee->getParent()->isInStdNamespace() ||
> !Callee->getIdentifier())
> +return false;
> +  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
> +  !isRecordWithAttr(Callee->getThisObjectType()))
> +return false;
> +  if (!isRecordWithAttr(Callee->getReturnType()) &&
> +  !Callee->getReturnType()->isPointerType())
> +return false;
> +  return llvm::StringSwitch(Callee->getName())
> +  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
> +  .Cases("end", "rend", "cend", "crend", true)
> +  .Cases("c_str", "data", "get", true)
> +  .Default(false);
> +}
> +
>  static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr
> *Call,
>  LocalVisitor Visit) {
>auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
> @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
>};
>
>if (auto *MCE = dyn_cast(Call)) {
> -const FunctionDecl *Callee = MCE->getDirectCallee();
> -if (auto *Conv = dyn_cast_or_null(Callee))
> -  if (isRecordWithAttr(Conv->getConversionType()))
> -VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
> +const auto *MD =
> cast_or_null(MCE->getDirectCallee());
> +if (MD && shouldTrackImplicitObjectArg(MD))
> +  VisitPointerArg(MD, MCE->getImplicitObjectArgument());
>  return;
>}
>
>
> Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff
>
> ===

[PATCH] D24881: [clang-tidy] Cleaning up language options.

[Gábor Horváth via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: hokein, alexfh.
xazax.hun added a subscriber: cfe-commits.
xazax.hun set the repository for this revision to rL LLVM.
xazax.hun added a project: clang-tools-extra.
Herald added a subscriber: nemanjai.

In some cases do not register the matcher when the check method of a checker 
should not run anyways.

Refactor some checks to use a shorter way to get language options.

Repository:
  rL LLVM

https://reviews.llvm.org/D24881

Files:
  clang-tidy/cert/StrToNumCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp
  clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tidy/google/ExplicitConstructorCheck.cpp
  clang-tidy/llvm/TwineLocalCheck.cpp
  clang-tidy/misc/AssertSideEffectCheck.cpp
  clang-tidy/misc/InaccurateEraseCheck.cpp
  clang-tidy/misc/InefficientAlgorithmCheck.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.cpp
  clang-tidy/misc/SuspiciousStringCompareCheck.cpp
  clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
  clang-tidy/misc/UnusedAliasDeclsCheck.cpp
  clang-tidy/misc/UnusedRAIICheck.cpp
  clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tidy/modernize/PassByValueCheck.cpp
  clang-tidy/modernize/RawStringLiteralCheck.cpp
  clang-tidy/modernize/RedundantVoidArgCheck.cpp
  clang-tidy/modernize/ShrinkToFitCheck.cpp
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/mpi/TypeMismatchCheck.cpp
  clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tidy/readability/NamespaceCommentCheck.cpp
  clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tidy/readability/RedundantSmartptrGetCheck.cpp
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
  clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp

Index: clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
===
--- clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
+++ clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
@@ -53,7 +53,7 @@
 
   SourceLocation AfterPtr =
   Lexer::getLocForEndOfToken(PtrExpr->getLocEnd(), 0, *Result.SourceManager,
- Result.Context->getLangOpts());
+ getLangOpts());
 
   diag(DeleteExpr->getLocStart(),
"prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> "
Index: clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
===
--- clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
+++ clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
@@ -54,11 +54,11 @@
   SourceLocation Loc = Def->getSourceRange().getBegin();
   while (Loc < Def->getSourceRange().getEnd() &&
  !Lexer::getRawToken(Loc, Tok, *Result.SourceManager,
- Result.Context->getLangOpts(), true)) {
+ getLangOpts(), true)) {
 SourceRange TokenRange(Tok.getLocation(), Tok.getEndLoc());
 StringRef SourceText = Lexer::getSourceText(
 CharSourceRange::getTokenRange(TokenRange),
-*Result.SourceManager, Result.Context->getLangOpts());
+*Result.SourceManager, getLangOpts());
 if (SourceText == "static") {
   Diag << FixItHint::CreateRemoval(TokenRange);
   break;
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -561,7 +561,7 @@
 StringRef Replacement) {
   CharSourceRange CharRange = Lexer::makeFileCharRange(
   CharSourceRange::getTokenRange(ReplacementRange), *Result.SourceManager,
-  Result.Context->getLangOpts());
+  getLangOpts());
 
   DiagnosticBuilder Diag = diag(Loc, Description);
   if (!containsDiscardedTokens(Result, CharRange))
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -119,7 +119,7 @@
 
   StringRef SmartptrText = Lexer::getSourceText(
   CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
-  *Result.SourceManager, Result.Context->getLangOpts());
+  *Result.SourceManager, getLangOpts());
   // Replace foo->get() with *foo, and foo.get() with foo.
   std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str();
   diag(GetCall->getLocStart(), "redundant get() call on smart pointer")
Index: clang-tidy/readability/RedundantControlFlowCheck.cpp
===
--- clang-tidy/readability/RedundantControlFlowCheck.cpp
+++ clang-tidy/readability/RedundantControlFlowCheck.cpp
@@ -81,14 +81

Re: [PATCH] D24881: [clang-tidy] Cleaning up language options.

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D24881#551487, @Eugene.Zelenko wrote:

> I think will be good idea to enable modernize-loop-convert, 
> modernize-use-auto, modernize-use-default, modernize-use-nullptr only in 
> C++11 mode.
>
> Probably modernize-avoid-bind code code be simplified too.


As far as I can see there is a discrepancy between these modernization checks. 
Loop convert, use auto, use default, use nullptr is registered for older 
standards deliberately.

See the following comment:

  // Only register the matchers for C++. Because this checker is used for
  // modernization, it is reasonable to run it on any C++ standard with the
  // assumption the user is trying to modernize their codebase.

So basically the question is: should we require the users to run the clang tidy 
in C++11 mode (hence change their build settings) to get these modernization 
checks?

I am okay with either of the answers but I wonder what do you think.


Repository:
  rL LLVM

https://reviews.llvm.org/D24881



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24894: [clang-tidy] Prefer transparent functors to non-transparent one.

[Gábor Horváth via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: alexfh, hokein.
xazax.hun added a subscriber: cfe-commits.
xazax.hun set the repository for this revision to rL LLVM.
xazax.hun added a project: clang-tools-extra.
Herald added subscribers: mgorny, beanz.

This check finds the usages of non-transparent functors and suggest to use the 
transparent ones.

This check seems to be surprisingly slow though. Maybe because the amount of 
typelocs in the source code? I did not have a chance to do a profiling yet, but 
plan to do it later. Feel free to postpone the review until the profiling is 
done.

Repository:
  rL LLVM

https://reviews.llvm.org/D24894

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
  clang-tidy/modernize/UseTransparentFunctorsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-transparent-functors.rst
  test/clang-tidy/modernize-use-transparent-functors.cpp

Index: test/clang-tidy/modernize-use-transparent-functors.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-transparent-functors.cpp
@@ -0,0 +1,100 @@
+// RUN: %check_clang_tidy %s modernize-use-transparent-functors %t -- -- -std=c++14
+
+namespace std {
+template
+struct remove_reference;
+
+template 
+constexpr T &&forward(typename std::remove_reference::type &t);
+
+template 
+constexpr T &&forward(typename std::remove_reference::type &&t);
+
+template 
+struct plus {
+  constexpr T operator()(const T &Lhs, const T &Rhs) const;
+};
+
+template <>
+struct plus {
+  template 
+  constexpr auto operator()(T &&Lhs, U &&Rhs) const -> 
+decltype(forward(Lhs) + forward(Rhs));
+};
+
+template 
+struct less {
+  constexpr bool operator()(const T &Lhs, const T &Rhs) const;
+};
+
+template <>
+struct less {
+  template 
+  constexpr bool operator()(T &&Lhs, U &&Rhs) const;
+};
+
+template 
+struct logical_not {
+  constexpr bool operator()(const T &Arg) const;
+};
+
+template <>
+struct logical_not {
+  template 
+  constexpr bool operator()(T &&Arg) const;
+};
+
+template 
+class allocator;
+
+template <
+class Key,
+class Compare = std::less<>,
+class Allocator = std::allocator>
+class set {};
+
+template 
+InputIt find_if(InputIt first, InputIt last,
+UnaryPredicate p);
+
+template 
+void sort(RandomIt first, RandomIt last, Compare comp);
+
+class iterator {};
+class string {};
+}
+
+int main() {
+  using std::set;
+  using std::less;
+  std::set> s;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer transparent functors [modernize-use-transparent-functors]
+  // CHECK-FIXES: {{^}}  std::set> s;{{$}}
+  set> s2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  set> s2;{{$}}
+  set> s3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  set> s3;{{$}}
+  std::set> s4;
+  std::set> s5;
+  std::set>, std::less<>> s6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  std::set>, std::less<>> s6;{{$}}
+  std::iterator begin, end;
+  sort(begin, end, std::less());
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: prefer transparent functors
+  std::sort(begin, end, std::less<>());
+  find_if(begin, end, std::logical_not());
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer transparent functors
+  std::find_if(begin, end, std::logical_not<>());
+  using my_set = std::set>;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  using my_set = std::set>;{{$}}
+  using my_set2 = std::set>;
+  using my_less = std::less;
+  find_if(begin, end, my_less());
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer transparent functors
+}
+
+
Index: docs/clang-tidy/checks/modernize-use-transparent-functors.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-transparent-functors.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - modernize-use-transparent-functors
+
+modernize-use-transparent-functors
+==
+
+Prefer transparent functors to non-transparent ones. Using transparent functors
+the type does not need to be repeated. The code is easier to read, maintain and
+less prone to errors. It not possible to introduce unwanted conversions.
+
+  .. code-block:: c++
+
+// Non-transparent functor  
+std::map> s;
+
+// Transparent functor.
+std::map> s;
+
+It is not always a safe transformation though. The following case will be 
+untouched to preserve the semantics.
+
+  .. code-block:: c++
+
+// Non-transparent functor  
+std::map> s;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ do

Re: [PATCH] D19586: Misleading Indentation check

[Gábor Horváth via cfe-commits]

xazax.hun added inline comments.


Comment at: test/clang-tidy/readability-misleading-indentation.cpp:16
@@ +15,3 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: wrong indentation, 
'else' belongs to 'if(cond2)' statement
+  // CHECK-FIXES: {{^}}  // comment1
+

Pajesz wrote:
> alexfh wrote:
> > Did you run the tests after changes? I don't think the `// comment1` line 
> > can actually appear in the fixed code, when it's not there before the fix.
> Could you please explain what check-fixes does and how? Im not sure about it.
You can read the documentation here: 
http://clang.llvm.org/extra/clang-tidy/#testing-checks

Didn't you execute the tests? (Using make check-all or some similar command)

The CHECK-FIXES line will check the content of the file after the fixits are 
applied. The check will fail when the specified pattern is not found in the 
result.


https://reviews.llvm.org/D19586



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24905: Fix unreachable code false positive, vardecl in switch

[Gábor Horváth via cfe-commits]

xazax.hun added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp:195
@@ +194,3 @@
+if (Optional S = I->getAs()) {
+  if (isa(S->getStmt()))
+return S->getStmt();

Maybe I would prefer something like !isa which is a less intrusive 
change.


Repository:
  rL LLVM

https://reviews.llvm.org/D24905



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24905: Fix unreachable code false positive, vardecl in switch

[Gábor Horváth via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D24905



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24878: ASTImporter: expressions, pt.2

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

The referenced patch was commited. Could you close/abandon this one?


https://reviews.llvm.org/D24878



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25429: [analyzer] Link libStaticAnalyzerCheckers to libASTMatchers.

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

I might be confused, but as far as I remember, when you do static linking, 
symbols tend to be linked in lazily. So as long as you do not use the matchers, 
those numbers might be misleading.


https://reviews.llvm.org/D25429



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25429: [analyzer] Link libStaticAnalyzerCheckers to libASTMatchers.

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D25429#565940, @NoQ wrote:

> In https://reviews.llvm.org/D25429#565939, @xazax.hun wrote:
>
> > I might be confused, but as far as I remember, when you do static linking, 
> > symbols tend to be linked in lazily. So as long as you do not use the 
> > matchers, those numbers might be misleading.
>
>
> Yep, i did actually obtain these numbers by using matchers in one of the 
> checkers.


Great! I am in favour of this change. I wonder, however, if it is worth to ask 
this on the mailing list as well.


https://reviews.llvm.org/D25429



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][solver] On SymSym RelOps, check EQClass members for contradictions (PR #71284)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

> So, if I understand you correctly, at the 3rd if statement, we should 
> canonicalize the symbol we are constraining by walking every sub-symbol and 
> substituting it with its equivalent counterpart (if any), by basically with 
> its eqclass' representative symbol.

I am not 100% sure if I'd go that far (doing canonicalization). Symbols carry a 
lot of information, like provenance. Bug reporters, or maybe even some custom 
checker state might rely on this. I am afraid that replacing/rewriting like 
that might have unexpected consequences, nothing we cannot solve, but I am not 
sure whether we want to solve them. What I'd suggest is more like always adding 
all the constraints to the representative, and lazily propagating those 
constraints to the other members of the equivalence classes, only when we 
mention them in a constraint.

This might also be challenging for bug reporters to explain in some scenarios, 
but at least we preserve the provenance information. 

@steakhal

https://github.com/llvm/llvm-project/pull/71284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][solver] On SymSym RelOps, check EQClass members for contradictions (PR #71284)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

> So to be explicit, no other component would know what canonicalization 
> happens within the range-based solver.

Oh, I see! Thanks for the clarification, this sounds good to me.

https://github.com/llvm/llvm-project/pull/71284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Fix assert-fail when calling assignment operator with by-value parameter. (PR #71384)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/71384
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Replace one remaining call to deprecated `addToFlowCondition()`. (PR #71547)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/71547
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow][NFC] Fix stale comments. (PR #71654)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/71654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Switch to PostStmt callbacks in ArrayBoundV2 (PR #72107)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.

I wish we could clean up `checkLocation` to work as expected for all cases. It 
is more future proof in the sense that if C++ introduces new kinds of 
statements or expressions that could index into something (like 
multi-dimensional `operator[]`), checks continue to work without change after 
the analysis engine is updated. 

It is a higher-level abstraction that lets checker authors focus on the 
semantics without having to enumerate all the patterns the check might be 
interested in which is often impossible in C++, let alone considering all the 
extensions implemented by Clang. So, these high-level abstractions supposed to 
deduplicate a lot of work by doing all the pattern matching once and for all in 
the engine, so checkers could benefit from this work without duplicating some 
of that pattern matching in every checker.

All that being said, I understand that it is more practical to just work these 
engine problems around in this check for now as we do not have a volunteer to 
revamp `checkLocation` at the moment. I know @steakhal looked into it, but it 
turned out to be a bigger fish than we expected.  So, I'd say let's land this 
as a practical middle-term solution on the hope that we will be able to come 
back and clean this up at some point using something that is more future proof. 

https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Switch to PostStmt callbacks in ArrayBoundV2 (PR #72107)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] EnumCastOutOfRangeChecker: report the value (PR #74503)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.

LG, thanks!

https://github.com/llvm/llvm-project/pull/74503
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Simplify BugType handling in core.BitwiseShift (PR #74609)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.

Thanks!

https://github.com/llvm/llvm-project/pull/74609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Convert `SpecialBoolAnalysis` to synthetic fields. (PR #74706)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/74706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][CLANG] Fix static analyzer bugs about large copy by values (PR #75060)

[Gábor Horváth via cfe-commits]

@@ -110,7 +110,7 @@ void 
ModuleDepCollector::addOutputPaths(CowCompilerInvocation &CI,
 }
 
 static CowCompilerInvocation
-makeCommonInvocationForModuleBuild(CompilerInvocation CI) {
+makeCommonInvocationForModuleBuild(const CompilerInvocation &CI) {

Xazax-hun wrote:

This does not compile, `CI` is mutated in this function.

https://github.com/llvm/llvm-project/pull/75060
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][CLANG] Fix static analyzer bugs about large copy by values (PR #75060)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/75060
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][CLANG] Fix static analyzer bugs about large copy by values (PR #75060)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

Could you also update the description of the patch?

https://github.com/llvm/llvm-project/pull/75060
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Move alpha checker EnumCastOutOfRange to optin (PR #67157)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

I am a bit concerned about the bad user experience with bitwise enums, but I 
think that is probably OK since this is an optin check. That being said, I 
think:
* It would be easy to address this, there is already a clang tidy check for 
bitwise enums, the heuristics to recognize them could be reused from there and 
used to suppress bogus warnings.
* As long as this limitation is documented, I am happy to get this merged as is.

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Discard unneeded `ExprToLoc` and `ExprToVal` entries. (PR #72850)

[Gábor Horváth via cfe-commits]

@@ -311,7 +318,10 @@ computeBlockInputState(const CFGBlock &Block, 
AnalysisContext &AC) {
 }
   }
 
-  JoinedStateBuilder Builder(AC);
+  // When performing the join, only retain state for those expressions that are
+  // consumed by this block. This avoids performing joins and potentially
+  // extending the flow condition for expressions that we won't need anyway.
+  JoinedStateBuilder Builder(AC, AC.CFCtx.getExprConsumedByBlock(&Block));

Xazax-hun wrote:

What about expressions that are consumed by a successor of this block?  Is it 
OK to drop those?
Also, depending on the answer to these questions, this almost sounds like "live 
expressions analysis".
In case this is related, we might want to use terminology from that as opposed 
to "consumed". 

https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Discard unneeded `ExprToLoc` and `ExprToVal` entries. (PR #72850)

[Gábor Horváth via cfe-commits]

@@ -52,23 +52,39 @@ class ControlFlowContext {
 return StmtToBlock;
   }
 
+  /// Returns the expressions consumed by `Block`. These are all children of
+  /// statements in `Block` as well as children of a possible terminator.
+  const llvm::DenseSet *
+  getExprConsumedByBlock(const CFGBlock *Block) const {
+auto It = ExprConsumedByBlock.find(Block);
+if (It == ExprConsumedByBlock.end())
+  return nullptr;
+return &It->second;
+  }
+
   /// Returns whether `B` is reachable from the entry block.
   bool isBlockReachable(const CFGBlock &B) const {
 return BlockReachable[B.getBlockID()];
   }
 
 private:
-  ControlFlowContext(const Decl &D, std::unique_ptr Cfg,
- llvm::DenseMap 
StmtToBlock,
- llvm::BitVector BlockReachable)
+  ControlFlowContext(
+  const Decl &D, std::unique_ptr Cfg,
+  llvm::DenseMap StmtToBlock,
+  llvm::DenseMap>
+  ExprConsumedByBlock,
+  llvm::BitVector BlockReachable)
   : ContainingDecl(D), Cfg(std::move(Cfg)),
 StmtToBlock(std::move(StmtToBlock)),
+ExprConsumedByBlock(std::move(ExprConsumedByBlock)),
 BlockReachable(std::move(BlockReachable)) {}
 
   /// The `Decl` containing the statement used to construct the CFG.
   const Decl &ContainingDecl;
   std::unique_ptr Cfg;
   llvm::DenseMap StmtToBlock;
+  const llvm::DenseMap>

Xazax-hun wrote:

I do not have strong feelings, but some people dislike const data members due 
to potential interactions with move ctors. 

https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Discard unneeded `ExprToLoc` and `ExprToVal` entries. (PR #72850)

[Gábor Horváth via cfe-commits]

@@ -311,7 +318,10 @@ computeBlockInputState(const CFGBlock &Block, 
AnalysisContext &AC) {
 }
   }
 
-  JoinedStateBuilder Builder(AC);
+  // When performing the join, only retain state for those expressions that are
+  // consumed by this block. This avoids performing joins and potentially
+  // extending the flow condition for expressions that we won't need anyway.
+  JoinedStateBuilder Builder(AC, AC.CFCtx.getExprConsumedByBlock(&Block));

Xazax-hun wrote:

Thanks! I think my main concerns are around convergence and `ExprToLoc`. 
Consider a simple loop like:
```
B1 ---> B2
^   |
 \__/
```

We have getStableStorageLocation that will first look up an expression in 
`ExprToLoc` and it will try to reuse the some location. In case we clear 
`ExprToLoc` along the back edge, in the second iteration through processing 
`B2` we would create new locations for the expressions.

Is this the intended effect here? 


https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Discard unneeded `ExprToLoc` and `ExprToVal` entries. (PR #72850)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Discard unneeded `ExprToLoc` and `ExprToVal` entries. (PR #72850)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

+1 for the simpler approach. I think I have a high-level question about the 
stability of the locations, its effect on convergence. But I am happy with the 
`ExprToVal` part of the simple patch.

https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Clear `ExprToLoc` and `ExprToVal` at the start of a block. (PR #72985)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/72985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Fix assertion failure in CXXInstanceCall::getCXXThisVal (PR #70837)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/70837
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Fix assertion failure in CXXInstanceCall::getCXXThisVal (PR #70837)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

Hmm, I wonder if we should leave a FIXME comment, but it looks good to me.

https://github.com/llvm/llvm-project/pull/70837
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Fix assertion failure in `CXXInstanceCall::getCXXThisVal` (PR #70837)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

> WDYT?

I like this! I hope we do not add too much redundant work, but at least we make 
it clear what is the plan to fix this in the future.

https://github.com/llvm/llvm-project/pull/70837
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Fix assertion failure in `CXXInstanceCall::getCXXThisVal` (PR #70837)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/70837
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Rework SVal kind representation (PR #71039)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/71039
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Do not analyze opaque types in CXXDeleteChecker (PR #70638)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

In case it is not too hard to synthesize a test case, could you add a 
regression test as well in a follow-up commit?

https://github.com/llvm/llvm-project/pull/70638
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][solver] On SymSym RelOps, check EQClass members for contradictions (PR #71284)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

I think every time we need to iterate over all member of an equivalence class, 
we might do something wrong. The point of the equivalence class would be to 
make sure those elements are equivalent. One way to avoid iteration would be to 
always use the representative of the equivalence class. E.g., each time we 
record a new constraint to a member of the class, we add this information to 
the representative element. Every time we do a query, we first look up the 
representative element which already should have all the info from the class 
and use it instead of the original symbol.

Would something like this work or am I missing something?

https://github.com/llvm/llvm-project/pull/71284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Move alpha checker EnumCastOutOfRange to optin (PR #67157)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

Sounds good!

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Move alpha checker EnumCastOutOfRange to optin (PR #67157)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen pointer comparison. (PR #75170)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/75170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen pointer comparison. (PR #75170)

[Gábor Horváth via cfe-commits]

@@ -152,6 +153,47 @@ TEST_F(EnvironmentTest, JoinRecords) {
   }
 }
 
+TEST_F(EnvironmentTest, DifferentReferenceLocInJoin) {

Xazax-hun wrote:

While running analysis on C++ code is indirect, it has the advantage of 
documenting what code can trigger such behavior. I think having direct tests is 
fine, I just wanted to make this decision explicit. 

https://github.com/llvm/llvm-project/pull/75170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen pointer comparison. (PR #75170)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.

This is a massive improvement! Thanks! Looks great to me, I have some not too 
important comments inline. 

https://github.com/llvm/llvm-project/pull/75170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen pointer comparison. (PR #75170)

[Gábor Horváth via cfe-commits]

@@ -3893,6 +3893,66 @@ TEST(TransferTest, BooleanInequality) {
   });
 }
 
+TEST(TransferTest, PointerEquality) {

Xazax-hun wrote:

I think it would be great to add a test case with union members to document the 
limitation. 

https://github.com/llvm/llvm-project/pull/75170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen pointer comparison. (PR #75170)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

An alternative approach would be to do [hash 
consing](https://en.wikipedia.org/wiki/Hash_consing).  Since pointer values are 
completely determined by the pointee (for some definition of the pointee, e.g., 
we might have `void*` and `int*` pointers pointing to the same storage 
location), we could have a single representation for all of the equal pointer 
values and continue to rely on identity. This can also be beneficial for memory 
consumption, but we have to pay for that by making value creation a bit more 
expensive (and the pointer values needs to be immutable). 

That being said, this would be a bit of a departure from the current 
architecture, so I think even if we want to do this, it should be a separate 
PR.  

https://github.com/llvm/llvm-project/pull/75170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen pointer comparison. (PR #75170)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

> It's probably a good thing for storage locations to have types (because we 
> want to be able to model the dynamic type of an object), but this probably 
> means that we want to model addresses separately from storage locations.

Our knowledge of the dynamic type might change during the analysis. Consider 
the following snippet:

```
void f(Base* b)
{
  if (dynamic_cast(b)) {
// Here, we know that the dynamic type is (a subclass of) Derived
  }
}
``` 

So, we either need to mutate storage locations, or we need to store the dynamic 
types on the side. The Clang Static Analyzer (CSA) is doing the latter. While 
CSA currently has types associated with the storage locations, we are 
considering undoing this decision and make storage locations only represented 
by sizes and offsets. We plan to always rely on the AST to determine the static 
types, and use a data structure on the side to track the dynamic type 
information. 

That being said, for some frameworks having typed storage locations might be a 
good idea, it is just not obvious to me where is the sweet spot and it will 
need careful consideration. (And as far as I understand, we are not even sure 
whether we want to switch to a different representation like access paths in 
the future.) 

> But all of this would be a major undertaking, I think.

Absolutely agree. The Clang Static Analyzer still has problems with type casts 
in some cases, it is a really challenging problem. That being said, the CSA is 
already immensely useful even with the partial modeling it has, so this is 
probably one of the less pressing problems to solve in the near/medium term. 

https://github.com/llvm/llvm-project/pull/75170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Fix an issue with `Environment::getResultObjectLocation()`. (PR #75483)

[Gábor Horváth via cfe-commits]

@@ -726,27 +726,69 @@ void Environment::setStorageLocation(const Expr &E, 
StorageLocation &Loc) {
   // so allow these as an exception.
   assert(E.isGLValue() ||
  E.getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn));
-  setStorageLocationInternal(E, Loc);
+  const Expr &CanonE = ignoreCFGOmittedNodes(E);
+  assert(!ExprToLoc.contains(&CanonE));
+  ExprToLoc[&CanonE] = &Loc;
 }
 
 StorageLocation *Environment::getStorageLocation(const Expr &E) const {
   // See comment in `setStorageLocation()`.
   assert(E.isGLValue() ||
  E.getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn));
-  return getStorageLocationInternal(E);
+  auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
+  return It == ExprToLoc.end() ? nullptr : &*It->second;
+}
+
+// Returns whether a prvalue of record type is the one that originally
+// constructs the object (i.e. it doesn't propagate it from one of its
+// children).
+static bool isOriginalRecordConstructor(const Expr &RecordPRValue) {

Xazax-hun wrote:

I am wondering whether it is better to enumerate the "transparent" AST nodes 
where we need to do the propagation.
My questions are:
* Roughly how many such transparent nodes do we have?
* In case we miss something, is it less desirable to erroneously propagate 
something or to erroneously create a new PRValue?


https://github.com/llvm/llvm-project/pull/75483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Fix an issue with `Environment::getResultObjectLocation()`. (PR #75483)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/75483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add `Environment::get<>()`. (PR #76027)

[Gábor Horváth via cfe-commits]

@@ -489,8 +489,7 @@ Environment Environment::pushCall(const CallExpr *Call) 
const {
   if (const auto *MethodCall = dyn_cast(Call)) {
 if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
   if (!isa(Arg))
-  Env.ThisPointeeLoc =
-  cast(getStorageLocation(*Arg));
+Env.ThisPointeeLoc = get(*Arg);

Xazax-hun wrote:

+1, alternatively we could introduce a bool template argument that would 
determine whether `null` is allowed with a default value. 

https://github.com/llvm/llvm-project/pull/76027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add `Environment::get<>()`. (PR #76027)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/76027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/76042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/76042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Gábor Horváth via cfe-commits]

@@ -636,40 +636,37 @@ class OptionalIntAnalysis final
 if (!CS)
   return;
 const Stmt *S = CS->getStmt();
-auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
-auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
-
-SmallVector Matches = match(
-stmt(anyOf(cxxConstructExpr(HasOptionalIntType).bind("construct"),
-   cxxOperatorCallExpr(
-   callee(cxxMethodDecl(ofClass(OptionalIntRecordDecl
-   .bind("operator"))),
-*S, getASTContext());
-if (const auto *E = selectFirst(
-"construct", Matches)) {
-  cast(Env.getValue(*E))
-  ->setProperty("has_value", Env.getBoolLiteralValue(false));
-} else if (const auto *E =
-   selectFirst("operator", Matches)) {
-  assert(E->getNumArgs() > 0);
-  auto *Object = E->getArg(0);
-  assert(Object != nullptr);
-
-  refreshRecordValue(*Object, Env)
-  .setProperty("has_value", Env.getBoolLiteralValue(true));
+const Expr *E = dyn_cast(S);
+if (!E)
+  return;
+
+if (!E->getType()->isPointerType())
+  return;
+
+// Make sure we have a `PointerValue` for `E`.
+auto *PtrVal = cast_or_null(Env.getValue(*E));

Xazax-hun wrote:

This is only a test, but this code snippet made me think. I wonder if it is a 
good idea to let checks create arbitrary `Value`s. Specifically, I am concerned 
about a poorly written check triggering divergence. 

https://github.com/llvm/llvm-project/pull/76042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Gábor Horváth via cfe-commits]

@@ -184,33 +188,23 @@ class PointerValue final : public Value {
 /// In C++, prvalues of class type serve only a limited purpose: They can only
 /// be used to initialize a result object. It is not possible to access member
 /// variables or call member functions on a prvalue of class type.
-/// Correspondingly, `RecordValue` also serves only two limited purposes:
-/// - It conveys a prvalue of class type from the place where the object is
-///   constructed to the result object that it initializes.
+/// Correspondingly, `RecordValue` also serves only a limited purpose: It
+/// conveys a prvalue of class type from the place where the object isx

Xazax-hun wrote:

typo?

https://github.com/llvm/llvm-project/pull/76042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Switch to PostStmt callbacks in ArrayBoundV2 (PR #72107)

[Gábor Horváth via cfe-commits]

@@ -34,20 +34,37 @@ using llvm::formatv;
 namespace {
 enum OOB_Kind { OOB_Precedes, OOB_Exceeds, OOB_Taint };
 
-class ArrayBoundCheckerV2 :
-public Checker {
+struct Messages {
+  std::string Short, Full;
+};
+
+class ArrayBoundCheckerV2 : public Checker,
+   check::PostStmt,
+   check::PostStmt> {
   BugType BT{this, "Out-of-bound access"};
   BugType TaintBT{this, "Out-of-bound access", categories::TaintedData};
 
+  void performCheck(const Expr *E, CheckerContext &C) const;
+
   void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, OOB_Kind Kind,
- NonLoc Offset, std::string RegName, std::string Msg) const;
+ NonLoc Offset, Messages Msgs) const;
 
   static bool isFromCtypeMacro(const Stmt *S, ASTContext &AC);
 
 public:
-  void checkLocation(SVal l, bool isLoad, const Stmt *S,
- CheckerContext &C) const;
+  void checkPostStmt(const ArraySubscriptExpr *E, CheckerContext &C) const {

Xazax-hun wrote:

Any particular reason for PostStmt vs PreStmt? The index being in bound is a 
precondition, and I think we generally prefer checking preconditions in 
PreStmt. 

https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Switch to PostStmt callbacks in ArrayBoundV2 (PR #72107)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

Note that &array[idx] is perfectly valid code when `idx == number of elements`. 
And it is relatively common to do that when one is using STL algorithms on 
arrays:
```
auto it = std::find(&array[0], &array[size], foo);
```

Of course, one could use the `begin/end` free functions, but those are only 
available since C++11. 

Could you elaborate on alternative approaches you considered fixing the problem 
and why you chose this one? E.g., would trying to look at the parent regions 
for expressions like `foo[idx].bar` work? Or is the source of the problem that 
you'd also need the exact expression for the subscript instead of the 
`MemberExpr`?

Alternatively, would it be possible to suppress warnings on the common pattern 
`&array[idx]` by checking the parent of the subscript expression in the AST 
(but still emitting a warning when the pointer is dereferenced)?


https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Move `security.cert.env.InvalidPtr` out of `alpha` (PR #71912)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/71912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fprintf` in the SecuritySyntaxChecker (PR #73247)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/73247
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Support `fprintf` in the SecuritySyntaxChecker (PR #73247)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

This is unrelated to this particular patch, but it would be great if this 
checker would also give suggestions what other functions should be used 
instead. 

https://github.com/llvm/llvm-project/pull/73247
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use AllocaRegion in MallocChecker (PR #72402)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

I think the original `alloca(0)` warning message might be clearer/easier to 
understand. While it might have platform or compiler dependent meaning, those 
behaviors are non-portable, so I think it is undesirable in most cases and 
people probably want to be notified about it. Regarding binding the return 
value outside of `evalCall`, I think that could be addressed in a separate PR. 
This one does not make the current situation any worse. But the very least we 
should add a FIXME now (and potentially open a ticket), to reduce the chances 
of this getting forgotten.

https://github.com/llvm/llvm-project/pull/72402
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[compiler-rt] [flang] [llvm] [libc] [libcxx] [clang] [clang-tools-extra] [clang][cleanup] simplify ReachableCode scanFromBlock (PR #72257)

[Gábor Horváth via cfe-commits]

@@ -341,30 +341,27 @@ static unsigned scanFromBlock(const CFGBlock *Start,
 // This allows us to potentially uncover some "always unreachable" code
 // within the "sometimes unreachable" code.
 // Look at the successors and mark then reachable.
-std::optional TreatAllSuccessorsAsReachable;
-if (!IncludeSometimesUnreachableEdges)
+bool TreatAllSuccessorsAsReachable;
+if (IncludeSometimesUnreachableEdges) {
+  assert(PP);
+  TreatAllSuccessorsAsReachable =
+  shouldTreatSuccessorsAsReachable(item, *PP);
+} else {
   TreatAllSuccessorsAsReachable = false;
+}
 
 for (CFGBlock::const_succ_iterator I = item->succ_begin(),
  E = item->succ_end(); I != E; ++I) {
   const CFGBlock *B = *I;
-  if (!B) do {
+  if (!B) {
 const CFGBlock *UB = I->getPossiblyUnreachableBlock();
 if (!UB)
   break;

Xazax-hun wrote:

This looks like a functional change. Previously this break was breaking out of 
the `do-while` loop, now it is breaking out of the `for` loop.

https://github.com/llvm/llvm-project/pull/72257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Strengthen widening of boolean values. (PR #73484)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/73484
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Switch to PostStmt callbacks in ArrayBoundV2 (PR #72107)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

I think another question is whether warning on code like `int& val = arr[i]` 
where `val` is not actually used on the path where `i == size` is OK. I would 
not block this PR on a case like this, but it might be nice to add a test and 
potentially a comment about the desired behavior.

https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun commented:

I generally love this direction, this was just a first pass, not a full review. 
I plan to take a second look but wanted to familiarize myself with the PR and 
start some conversations.

https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

@@ -88,12 +94,12 @@ class ScalarStorageLocation final : public StorageLocation {
 class RecordStorageLocation final : public StorageLocation {
 public:
   using FieldToLoc = llvm::DenseMap;
+  using SyntheticFieldMap = llvm::StringMap;

Xazax-hun wrote:

I was wondering, if we want this map to **not** be indexed by strings. An 
alternative would be to somehow access the `IdentifierTable` and create an 
identifier from the names and use the pointer to the identifier as the key. 
This has the potential to be a bit lighter on memory (identifiers only stored 
once as opposed to storing the keys in each RecordStorageLocation instance), 
and potentially a bit quicker lookup. 

That being said, I am not insisting on the change, and even if we want to do 
it, it is probably OK in a follow-up PR. 

https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

@@ -92,11 +96,39 @@ class DataflowAnalysisContext {
   /*Logger=*/nullptr});
   ~DataflowAnalysisContext();
 
+  /// Sets a callback that returns the names and types of the synthetic fields
+  /// to add to a `RecordStorageLocation` of a given type.
+  /// Typically, this is called from the constructor of a `DataflowAnalysis`
+  ///
+  /// To maintain the invariant that all `RecordStorageLocation`s of a given
+  /// type have the same fields:
+  /// *  The callback must always return the same result for a given type
+  /// *  `setSyntheticFieldCallback()` must be called before any
+  // `RecordStorageLocation`s are created.

Xazax-hun wrote:

I was wondering if having a single `SyntheticFieldCallback` will be too 
restrictive in the future. What if an ananlysis wants to use modeling both for 
optionals and some other data structure, both having its own callbacks?

Although this is mostly food for thought for the future, no need to address in 
this PR.  

https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

@@ -92,11 +96,39 @@ class DataflowAnalysisContext {
   /*Logger=*/nullptr});
   ~DataflowAnalysisContext();
 
+  /// Sets a callback that returns the names and types of the synthetic fields
+  /// to add to a `RecordStorageLocation` of a given type.
+  /// Typically, this is called from the constructor of a `DataflowAnalysis`
+  ///
+  /// To maintain the invariant that all `RecordStorageLocation`s of a given
+  /// type have the same fields:
+  /// *  The callback must always return the same result for a given type
+  /// *  `setSyntheticFieldCallback()` must be called before any
+  // `RecordStorageLocation`s are created.
+  void setSyntheticFieldCallback(
+  std::function(QualType)> CB) {
+assert(!RecordStorageLocationCreated);
+SyntheticFieldCallback = CB;
+  }
+
   /// Returns a new storage location appropriate for `Type`.
   ///
   /// A null `Type` is interpreted as the pointee type of `std::nullptr_t`.
   StorageLocation &createStorageLocation(QualType Type);
 
+  /// Creates a `RecordStorageLocation` for the given type and with the given
+  /// fields.
+  ///
+  /// Requirements:
+  ///
+  ///  `FieldLocs` must contain exactly the fields returned by

Xazax-hun wrote:

With requirements like this, I was wondering if this should be a private API.

https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

@@ -367,3 +394,14 @@ clang::dataflow::FieldSet 
clang::dataflow::getObjectFields(QualType Type) {
   getFieldsFromClassHierarchy(Type, Fields);
   return Fields;
 }
+
+bool clang::dataflow::containsSameFields(
+const clang::dataflow::FieldSet &Fields,
+const clang::dataflow::RecordStorageLocation::FieldToLoc &FieldLocs) {
+  if (Fields.size() != FieldLocs.size())
+return false;
+  for ([[maybe_unused]] auto [Field, Loc] : FieldLocs)
+if (!Fields.contains(cast_or_null(Field)))

Xazax-hun wrote:

What are the cases when we expect to see a null pointer here?

https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.

Overall, I love it! I have some comments on some features in the future, but 
those are probably not going to be addressed any time soon. First of all, I 
think in the future when we reason about the values of the pointers, synthetic 
fields might need to have offsets to be able to know things like 
`&modeled.getField1() != &modeled.getField2()`. Offsets might also be a way to 
support unions, multiple synthetic fields starting at the same offset might 
express just that.

Another big item is properly supporting inheritance. Consider:
```
struct B { int a; };
struct D : /*virtual*/ B {};
struct E : /*virtual*/  B {};
struct F: D, E {
void m() {
int* l = &(D::a);
int* m = &(E::a);
}
};

``` 

Here, whether we have the same or different memory locations for `D::a` and 
`E::a` depends on whether we are doing virtual inheritance. This is something 
that should work both for regular fields and synthetic fields.

https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (PR #73860)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/73860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Let the checkers query upper and lower bounds on symbols (PR #74141)

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

Does this work with Z3 as the solver? Since Z3 is not officially supported, I 
think it is not a blocker, but I'd love to see some FIXMEs/tickets opened in 
that case. 

https://github.com/llvm/llvm-project/pull/74141
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Gábor Horváth via cfe-commits]

@@ -636,40 +636,37 @@ class OptionalIntAnalysis final
 if (!CS)
   return;
 const Stmt *S = CS->getStmt();
-auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
-auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
-
-SmallVector Matches = match(
-stmt(anyOf(cxxConstructExpr(HasOptionalIntType).bind("construct"),
-   cxxOperatorCallExpr(
-   callee(cxxMethodDecl(ofClass(OptionalIntRecordDecl
-   .bind("operator"))),
-*S, getASTContext());
-if (const auto *E = selectFirst(
-"construct", Matches)) {
-  cast(Env.getValue(*E))
-  ->setProperty("has_value", Env.getBoolLiteralValue(false));
-} else if (const auto *E =
-   selectFirst("operator", Matches)) {
-  assert(E->getNumArgs() > 0);
-  auto *Object = E->getArg(0);
-  assert(Object != nullptr);
-
-  refreshRecordValue(*Object, Env)
-  .setProperty("has_value", Env.getBoolLiteralValue(true));
+const Expr *E = dyn_cast(S);
+if (!E)
+  return;
+
+if (!E->getType()->isPointerType())
+  return;
+
+// Make sure we have a `PointerValue` for `E`.
+auto *PtrVal = cast_or_null(Env.getValue(*E));

Xazax-hun wrote:

I think the reason why it might be worth spending some time thinking about 
convergence because this is probably one of the bigger challenges for people 
writing checks. The other reason why this might be an interesting question, 
because in case we want to run multiple checks in the same fixed-point 
iteration, we might not want arbitrary interactions between them. 

That being said, there are many moving pieces that we have not figured yet out, 
so I 100% agree that it might be too early to try to solve this problem now. 

https://github.com/llvm/llvm-project/pull/76042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/76042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][docs] Update the release notes for llvm-18 (PR #76446)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/76446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][docs] Update the release notes for llvm-18 (PR #76446)

[Gábor Horváth via cfe-commits]

@@ -1072,16 +1146,36 @@ Static Analyzer
   Read the PR for the details.
   (`#66086 `_)
 
+- Other taint-related improvements.
+  (`#66358 `_,
+  `#66074 `_,
+  `#66358 `_)
+
 - A few crashes have been found and fixed using randomized testing related
-  to the use of ``_BitInt()`` in tidy checks and in clang analysis. See
-  `#67212 `_,
+  to the use of ``_BitInt()`` in tidy checks and in clang analysis.
+  (`#67212 `_,
   `#66782 `_,
   `#65889 `_,
-  `#65888 `_, and
-  `#65887 `_
+  `#65888 `_,
+  `#65887 `_)
 
-- Move checker ``alpha.cplusplus.EnumCastOutOfRange`` out of the ``alpha``
-  package to ``optin.core.EnumCastOutOfRange``.
+- Fixed note links of the HTML output.
+  (`#64054 `_)
+
+- Allow widening rage-based for loops.
+  (`#70190 `_)
+
+- Fixed uninitialized base class with initializer list when ctor is not
+  declared in the base class.
+  (`#70464 `_,
+  `#59493 `_,
+  `#54533 `_)
+
+- Added support for the ``cleanup`` attribute.
+  `Documentation 
`__.

Xazax-hun wrote:

In some cases, we link to the PR/commit, in other cases only to the 
documentation. I am OK with this, I was only wondering whether we want to add 
links to the commits everywhere. Feel free to ignore.

https://github.com/llvm/llvm-project/pull/76446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][docs] Update the release notes for llvm-18 (PR #76446)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun edited 
https://github.com/llvm/llvm-project/pull/76446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][docs] Update the release notes for llvm-18 (PR #76446)

[Gábor Horváth via cfe-commits]

@@ -1052,18 +1052,92 @@ libclang
 Static Analyzer
 ---
 
+- Implemented the ``[[clang::suppress]]`` attribute for suppressing diagnostics
+  of static analysis tools, such as the Clang Static Analyzer.
+  `Documentation 
`__.
+
+- Added a new experimental checker ``alpha.core.StdVariant`` to detect variant
+  accesses via wrong alternatives.
+  (`#66481 `_)
+
+- Added a new experimental checker ``alpha.cplusplus.ArrayDelete`` to detect
+  destructions of arrays of polymorphic objects that are destructed as their
+  base class (`CERT EXP51-CPP 
`_).
+  `Documentation 
`__.
+  (`0e246bb67573 
`_)
+
 - Added a new checker ``core.BitwiseShift`` which reports situations where
   bitwise shift operators produce undefined behavior (because some operand is
   negative or too large).
+  `Documentation 
`__.
+
+- Support "Deducing this" (P0847R7). (Worked out of the box)
+  (`af4751738db8 
`__)
 
 - Move checker ``alpha.unix.Errno`` out of the ``alpha`` package
   to ``unix.Errno``.
+  `Documentation 
`__.
 
 - Move checker ``alpha.unix.StdCLibraryFunctions`` out of the ``alpha`` package
   to ``unix.StdCLibraryFunctions``.
 
+- Added a new checker configuration option to

Xazax-hun wrote:

Maybe we want to have a short sentence about what the configuration option is 
doing?

https://github.com/llvm/llvm-project/pull/76446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Cleanup BugType lazy-init patterns (PR #76655)

[Gábor Horváth via cfe-commits]

@@ -31,14 +31,14 @@ class InvalidatedIteratorChecker
check::PreStmt,
check::PreStmt> {
 
-  std::unique_ptr InvalidatedBugType;
+  const BugType InvalidatedBugType{this, "Iterator invalidated",
+   "Misuse of STL APIs"};
 
   void verifyAccess(CheckerContext &C, const SVal &Val) const;
-  void reportBug(const StringRef &Message, const SVal &Val,
- CheckerContext &C, ExplodedNode *ErrNode) const;
-public:
-  InvalidatedIteratorChecker();
+  void reportBug(const StringRef &Message, const SVal &Val, CheckerContext &C,

Xazax-hun wrote:

Since we are changing this code, I guess we do not want to take `StringRef`s as 
const references.

https://github.com/llvm/llvm-project/pull/76655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Cleanup BugType lazy-init patterns (PR #76655)

[Gábor Horváth via cfe-commits]

@@ -17,19 +17,18 @@
 
 #include "MPITypes.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace ento {
 namespace mpi {
 
 class MPIBugReporter {
 public:
-  MPIBugReporter(const CheckerBase &CB) {
-UnmatchedWaitBugType.reset(new BugType(&CB, "Unmatched wait", MPIError));
-DoubleNonblockingBugType.reset(
-new BugType(&CB, "Double nonblocking", MPIError));
-MissingWaitBugType.reset(new BugType(&CB, "Missing wait", MPIError));
-  }
+  MPIBugReporter(const CheckerBase &CB)
+  : UnmatchedWaitBugType(&CB, "Unmatched wait", MPIError),

Xazax-hun wrote:

Move to field initializers?

https://github.com/llvm/llvm-project/pull/76655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Cleanup BugType lazy-init patterns (PR #76655)

[Gábor Horváth via cfe-commits]

@@ -52,10 +52,13 @@ class SimpleStreamChecker : public Checker {
-  CallDescription OpenFn, CloseFn;
+  const CallDescription OpenFn{{"fopen"}, 2};
+  const CallDescription CloseFn{{"fclose"}, 1};

Xazax-hun wrote:

Since we mostly use `CallDescription`s with compile time known inputs, it would 
be nice to make their ctors constexpr. This is not for this PR, just a note for 
the future.

https://github.com/llvm/llvm-project/pull/76655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Cleanup BugType lazy-init patterns (PR #76655)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/76655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Take small objects by value (PR #76688)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.

LGTM! Thanks!

https://github.com/llvm/llvm-project/pull/76688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Fix bug in `Value` comparison. (PR #76746)

[Gábor Horváth via cfe-commits]

@@ -27,9 +27,13 @@ static bool areEquivalentIndirectionValues(const Value &Val1,
 }
 
 bool areEquivalentValues(const Value &Val1, const Value &Val2) {
-  return &Val1 == &Val2 || (Val1.getKind() == Val2.getKind() &&
-(isa(&Val1) ||
- areEquivalentIndirectionValues(Val1, Val2)));
+  // If values are distinct and have properties, we don't consider them equal,
+  // leaving equality up to the user model.
+  return &Val1 == &Val2 ||
+ (Val1.getKind() == Val2.getKind() &&
+  (Val1.properties().empty() && Val2.properties().empty()) &&

Xazax-hun wrote:

Do we want to consider them different when only one of them has properties?

https://github.com/llvm/llvm-project/pull/76746
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Fix bug in `Value` comparison. (PR #76746)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/76746
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Add an early-out to `flowConditionImplies()` / `flowConditionAllows()`. (PR #77453)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/77453
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Make cap on block visits configurable by caller. (PR #77481)

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/77481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)

[Gábor Horváth via cfe-commits]

@@ -212,8 +212,23 @@ TEST(TransferTest, CopyRecordFromDerivedToBase) {
   // [[p]]
 }
   )";
+  auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap {
+CXXRecordDecl *ADecl = nullptr;
+if (Ty.getAsString() == "A")
+  ADecl = Ty->getAsCXXRecordDecl();
+else if (Ty.getAsString() == "B")
+  ADecl = Ty->getAsCXXRecordDecl()
+  ->bases_begin()
+  ->getType()
+  ->getAsCXXRecordDecl();
+else
+  return {};
+QualType IntTy = getFieldNamed(ADecl, "i")->getType();
+return {{"synth_int", IntTy}};
+  };
+  // Copy derived to base class.

Xazax-hun wrote:

What does this comment refer to?

https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Model assignment to derived class from base. (PR #85064)

[Gábor Horváth via cfe-commits]

@@ -14,18 +14,52 @@
 
 #define DEBUG_TYPE "dataflow"
 
-void clang::dataflow::copyRecord(RecordStorageLocation &Src,
- RecordStorageLocation &Dst, Environment &Env) 
{
+namespace clang::dataflow {
+
+static void copyField(const ValueDecl *Field, StorageLocation *SrcFieldLoc,

Xazax-hun wrote:

Could `SrcFieldLoc` be a ptr to const? (Same below).

https://github.com/llvm/llvm-project/pull/85064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits