[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D120305#3347143 , @MaskRay wrote:

> In D120305#3347142 , @tstellar 
> wrote:
>
>> In D120305#3347139 , @MaskRay 
>> wrote:
>>
>>> In D120305#3347109 , @tstellar 
>>> wrote:
>>>
 [...]
 The issue here has nothing to do with the technical merits of the patch or 
 what the root cause of the problem is.  The policy for this project is 
 that if you commit a patch that breaks someone's configuration (especially 
 a buildbot), then it needs to be fixed quickly or reverted.  I get that 
 this policy can be frustrating as a committer when you feel your patch is 
 correct, and the real problem is elsewhere, but this is still the policy 
 and it should be followed.
>>>
>>> 7 hours ago my 
>>> https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad
>>>  was sufficient to fix the issue and was the suggested fix in my opinion.
>>> Unfortunately nobody on the PowerPC side made the change effective in the 
>>> build bot. Rather, I received such a heated message 
>>> (https://reviews.llvm.org/D120305#3347058).
>>> It was another way to fix the redness (revert) but IMO not justified.
>>
>> I feel like we are talking past each other at this point, but in general 
>> changing the buildbot configuration is not an acceptable solution to a 
>> broken bot.  Did the bot owner approve that change?
>
> Unsure why it isn't acceptable. There is fairly strong evidence that that 
> specific ppc64le buildbot has a stability issue and 
> `-DCLANG_DEFAULT_PIE_ON_LINUX=OFF` keeps it as if before this CMake patch.
> We can always discuss this with @nemanjai in another place, e.g. in IRC if we 
> want to stop bothering others subscribing to this thread.

It's not acceptable, because buildbots are meant to represent a specific 
configuration that the buildbot owner cares about.  Changing the buildbot 
configuration makes the buildbot no longer useful to them since it is now 
testing something different.  For example, if someone is running production 
builds that used the same configuration as the buildbot, those production 
builds are still broken even though the buildbot is green.  Configuration 
changes like this need to have approval of the bot owner.

I still don't understand why you can't revert the patch.  I've encountered this 
same situation numerous times while working on this project and no one has ever 
objected this much to doing a revert.  The fact that this is the second time 
this has happened is concerning to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347145 , @tstellar wrote:

> In D120305#3347143 , @MaskRay wrote:
>
>> In D120305#3347142 , @tstellar 
>> wrote:
>>
>>> In D120305#3347139 , @MaskRay 
>>> wrote:
>>>
 In D120305#3347109 , @tstellar 
 wrote:

> [...]
> The issue here has nothing to do with the technical merits of the patch 
> or what the root cause of the problem is.  The policy for this project is 
> that if you commit a patch that breaks someone's configuration 
> (especially a buildbot), then it needs to be fixed quickly or reverted.  
> I get that this policy can be frustrating as a committer when you feel 
> your patch is correct, and the real problem is elsewhere, but this is 
> still the policy and it should be followed.

 7 hours ago my 
 https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad
  was sufficient to fix the issue and was the suggested fix in my opinion.
 Unfortunately nobody on the PowerPC side made the change effective in the 
 build bot. Rather, I received such a heated message 
 (https://reviews.llvm.org/D120305#3347058).
 It was another way to fix the redness (revert) but IMO not justified.
>>>
>>> I feel like we are talking past each other at this point, but in general 
>>> changing the buildbot configuration is not an acceptable solution to a 
>>> broken bot.  Did the bot owner approve that change?
>>
>> Unsure why it isn't acceptable. There is fairly strong evidence that that 
>> specific ppc64le buildbot has a stability issue and 
>> `-DCLANG_DEFAULT_PIE_ON_LINUX=OFF` keeps it as if before this CMake patch.
>> We can always discuss this with @nemanjai in another place, e.g. in IRC if 
>> we want to stop bothering others subscribing to this thread.
>
> It's not acceptable, because buildbots are meant to represent a specific 
> configuration that the buildbot owner cares about.  Changing the buildbot 
> configuration makes the buildbot no longer useful to them since it is now 
> testing something different.  For example, if someone is running production 
> builds that used the same configuration as the buildbot, those production 
> builds are still broken even though the buildbot is green.  Configuration 
> changes like this need to have approval of the bot owner.

If clang-ppc64le-rhel works with `-DCLANG_DEFAULT_PIE_ON_LINUX=off` but not 
with  `-DCLANG_DEFAULT_PIE_ON_LINUX=on`, adding  
`-DCLANG_DEFAULT_PIE_ON_LINUX=off` makes the intention explicit. I am not sure 
why this isn't acceptable.
It's a tech debt, though, as we are making configurations fragmented.

> I still don't understand why you can't revert the patch.  I've encountered 
> this same situation numerous times while working on this project and no one 
> has ever objected this much to doing a revert.  The fact that this is the 
> second time this has happened is concerning to me.

I have stated my reasoning. Configuration churn can also be a problem to users.
Consider what if the DWARF v5 patch got reverted and relanded back and forth. 
Downstream users would keep observing changing behaviors.

In this case, really even normal ppc64le machines (including some bots) were 
happy and just that one was picky.
Given that the bot does not have a high success rate (track record) for at 
least the past month, I am unsure I am supposed to revert my change.

It is more concerning to me that a bot maintainer leaves an unstable bot for so 
long and is not even willing to spend very little time to make a llvm-zorg 
change live (perhaps just restart the bot software), but rather is more willing 
to **write such a long reply taking it very personally**.
(Sorry, I know when I write this, I was a bit in a mood. I felt quite 
frustrated at this point, so I could not keep using the tune when I made the 
reply https://reviews.llvm.org/D120305#3347094)

I can response to you that I have shared the thread to some folks and at least 
two agree with me that nemanjai's reply made the discussion less technical but 
more personal.
And one pointed out that your "nemanjai is correct" message was made a bit 
arbitrary, without evidence of considering the full context.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

https://lab.llvm.org/buildbot/#/builders/57 is green


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D120305#3347148 , @MaskRay wrote:

> In D120305#3347145 , @tstellar 
> wrote:
>
>> In D120305#3347143 , @MaskRay 
>> wrote:
>>
>>> In D120305#3347142 , @tstellar 
>>> wrote:
>>>
 In D120305#3347139 , @MaskRay 
 wrote:

> In D120305#3347109 , @tstellar 
> wrote:
>
>> [...]
>> The issue here has nothing to do with the technical merits of the patch 
>> or what the root cause of the problem is.  The policy for this project 
>> is that if you commit a patch that breaks someone's configuration 
>> (especially a buildbot), then it needs to be fixed quickly or reverted.  
>> I get that this policy can be frustrating as a committer when you feel 
>> your patch is correct, and the real problem is elsewhere, but this is 
>> still the policy and it should be followed.
>
> 7 hours ago my 
> https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad
>  was sufficient to fix the issue and was the suggested fix in my opinion.
> Unfortunately nobody on the PowerPC side made the change effective in the 
> build bot. Rather, I received such a heated message 
> (https://reviews.llvm.org/D120305#3347058).
> It was another way to fix the redness (revert) but IMO not justified.

 I feel like we are talking past each other at this point, but in general 
 changing the buildbot configuration is not an acceptable solution to a 
 broken bot.  Did the bot owner approve that change?
>>>
>>> Unsure why it isn't acceptable. There is fairly strong evidence that that 
>>> specific ppc64le buildbot has a stability issue and 
>>> `-DCLANG_DEFAULT_PIE_ON_LINUX=OFF` keeps it as if before this CMake patch.
>>> We can always discuss this with @nemanjai in another place, e.g. in IRC if 
>>> we want to stop bothering others subscribing to this thread.
>>
>> It's not acceptable, because buildbots are meant to represent a specific 
>> configuration that the buildbot owner cares about.  Changing the buildbot 
>> configuration makes the buildbot no longer useful to them since it is now 
>> testing something different.  For example, if someone is running production 
>> builds that used the same configuration as the buildbot, those production 
>> builds are still broken even though the buildbot is green.  Configuration 
>> changes like this need to have approval of the bot owner.
>
> If clang-ppc64le-rhel works with `-DCLANG_DEFAULT_PIE_ON_LINUX=off` but not 
> with  `-DCLANG_DEFAULT_PIE_ON_LINUX=on`, adding  
> `-DCLANG_DEFAULT_PIE_ON_LINUX=off` makes the intention explicit. I am not 
> sure why this isn't acceptable.
> It's a tech debt, though, as we are making configurations fragmented.
>
>> I still don't understand why you can't revert the patch.  I've encountered 
>> this same situation numerous times while working on this project and no one 
>> has ever objected this much to doing a revert.  The fact that this is the 
>> second time this has happened is concerning to me.
>
> I have stated my reasoning. Configuration churn can also be a problem to 
> users.
> Consider what if the DWARF v5 patch got reverted and relanded back and forth. 
> Downstream users would keep observing changing behaviors.
>
> In this case, really even normal ppc64le machines (including some bots) were 
> happy and just that one was picky.
> Given that the bot does not have a high success rate (track record) for at 
> least the past month, I am unsure I am supposed to revert my change.
>
> It is more concerning to me that a bot maintainer leaves an unstable bot for 
> so long and is not even willing to spend very little time to make a llvm-zorg 
> change live (perhaps just restart the bot software), but rather is more 
> willing to **write such a long reply taking it very personally**.
> (Sorry, I know when I write this, I was a bit in a mood. I felt quite 
> frustrated at this point, so I could not keep using the tune when I made the 
> reply https://reviews.llvm.org/D120305#3347094)
>
> I can response to you that I have shared the thread to some folks and at 
> least two agree with me that nemanjai's reply made the discussion less 
> technical but more personal.

Can these folks provide their feedback on the thread?

> And one pointed out that your "nemanjai is correct" message was made a bit 
> arbitrary, without evidence of considering the full context.

The Reversion Policy is documented here: 
https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

___
cfe-commits mailing list

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

>> Given that the bot does not have a high success rate (track record) for at 
>> least the past month, I am unsure I am supposed to revert my change.

What is interesting that always the patch author is the person to be blamed 
even when there are situations where it is so obvious that a bot has a problem. 
LLVM project/board should also care about buildbots and whether they work - 
also there should be strict policy about bot removal if maintainer does not 
want to spend any time to fix problems.

It is very annoying to have unstable buildbots. One one side LLVM tries to be 
very inclusive and open for new people, on the other hand LLVM scares them with 
broken bots - first experience could be very bad.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D120305#3347151 , @xbolva00 wrote:

>>> Given that the bot does not have a high success rate (track record) for at 
>>> least the past month, I am unsure I am supposed to revert my change.
>
> What is interesting that always the patch author is the person to be blamed 
> even when there are situations where it is so obvious that a bot has a 
> problem. LLVM project/board should also care about buildbots and whether they 
> work - also there should be strict policy about bot removal if maintainer 
> does not want to spend any time to fix problems.



> It is very annoying to have unstable buildbots. One one side LLVM tries to be 
> very inclusive and open for new people, on the other hand LLVM scares them 
> with broken bots - first experience could be very bad.



In D120305#3347151 , @xbolva00 wrote:

>>> Given that the bot does not have a high success rate (track record) for at 
>>> least the past month, I am unsure I am supposed to revert my change.
>
> What is interesting that always the patch author is the person to be blamed 
> even when there are situations where it is so obvious that a bot has a 
> problem. LLVM project/board should also care about buildbots and whether they 
> work - also there should be strict policy about bot removal if maintainer 
> does not want to spend any time to fix problems.
>
> It is very annoying to have unstable buildbots. One one side LLVM tries to be 
> very inclusive and open for new people, on the other hand LLVM scares them 
> with broken bots - first experience could be very bad.

The problem here is not an unstable bot.  If you look at the build log for the 
bot, it is very clear that this patch introduced the failures.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347152 , @tstellar wrote:

> In D120305#3347151 , @xbolva00 
> wrote:
>
 Given that the bot does not have a high success rate (track record) for at 
 least the past month, I am unsure I am supposed to revert my change.
>>
>> What is interesting that always the patch author is the person to be blamed 
>> even when there are situations where it is so obvious that a bot has a 
>> problem. LLVM project/board should also care about buildbots and whether 
>> they work - also there should be strict policy about bot removal if 
>> maintainer does not want to spend any time to fix problems.
>>
>> It is very annoying to have unstable buildbots. One one side LLVM tries to 
>> be very inclusive and open for new people, on the other hand LLVM scares 
>> them with broken bots - first experience could be very bad.
>
> The problem here is not an unstable bot.  If you look at the build log for 
> the bot, it is very clear that this patch introduced the failures.



- https://lab.llvm.org/buildbot/#/builders/57?numbuilds=1000 
(clang-ppc64le-rhel) the bot broken by this PIE change
- https://lab.llvm.org/buildbot/#/builders/19?numbuilds=1000 
(sanitizer-ppc64le-linux), happy with this change
- https://lab.llvm.org/buildbot/#/builders/18?numbuilds=1000 
(sanitizer-ppc64be-linux), happy with this change
- https://lab.llvm.org/buildbot/#/builders/121?numbuilds=1000 
(clang-ppc64le-linux-multistage), happy with this change

clang-ppc64le-rhel has the lowest success rate.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@MaskRay Please revert the change and all dependent changes you have made. A 
revert is not a personal affront to you. It's not a judgement that you or your 
change are bad. It's a simple matter of policy and standard procedure. There's 
a good chance that next week you'll get confirmation that it's indeed some 
outdated libraries on the buildbot, and the change can reland without any 
changes on your side. Or maybe it turns out that this default is not quite 
viable for powerpc targets yet. Who knows.

Don't worry about the churn. These short-term reverts happen all the time, 
we're used to it. Especially for tiny changes like this one, it's really no 
problem (reverts can be more annoying if the commit touches 1500 test files).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347160 , @nikic wrote:

> @MaskRay Please revert the change and all dependent changes you have made. A 
> revert is not a personal affront to you. It's not a judgement that you or 
> your change are bad. It's a simple matter of policy and standard procedure. 
> There's a good chance that next week you'll get confirmation that it's indeed 
> some outdated libraries on the buildbot, and the change can reland without 
> any changes on your side. Or maybe it turns out that this default is not 
> quite viable for powerpc targets yet. Who knows.
>
> Don't worry about the churn. These short-term reverts happen all the time, 
> we're used to it. Especially for tiny changes like this one, it's really no 
> problem (reverts can be more annoying if the commit touches 1500 test files).

I have mentioned that https://lab.llvm.org/buildbot/#/builders/57 has been 
green.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347150 , @tstellar wrote:

> [...]
>
>>> It's not acceptable, because buildbots are meant to represent a specific 
>>> configuration that the buildbot owner cares about.  Changing the buildbot 
>>> configuration makes the buildbot no longer useful to them since it is now 
>>> testing something different.  For example, if someone is running production 
>>> builds that used the same configuration as the buildbot, those production 
>>> builds are still broken even though the buildbot is green.  Configuration 
>>> changes like this need to have approval of the bot owner.
>>
>> If clang-ppc64le-rhel works with `-DCLANG_DEFAULT_PIE_ON_LINUX=off` but not 
>> with  `-DCLANG_DEFAULT_PIE_ON_LINUX=on`, adding  
>> `-DCLANG_DEFAULT_PIE_ON_LINUX=off` makes the intention explicit. I am not 
>> sure why this isn't acceptable.
>> It's a tech debt, though, as we are making configurations fragmented.
>>
>>> I still don't understand why you can't revert the patch.  I've encountered 
>>> this same situation numerous times while working on this project and no one 
>>> has ever objected this much to doing a revert.  The fact that this is the 
>>> second time this has happened is concerning to me.
>>
>> I have stated my reasoning. Configuration churn can also be a problem to 
>> users.
>> Consider what if the DWARF v5 patch got reverted and relanded back and 
>> forth. Downstream users would keep observing changing behaviors.
>>
>> In this case, really even normal ppc64le machines (including some bots) were 
>> happy and just that one was picky.
>> Given that the bot does not have a high success rate (track record) for at 
>> least the past month, I am unsure I am supposed to revert my change.
>>
>> It is more concerning to me that a bot maintainer leaves an unstable bot for 
>> so long and is not even willing to spend very little time to make a 
>> llvm-zorg change live (perhaps just restart the bot software), but rather is 
>> more willing to **write such a long reply taking it very personally**.
>> (Sorry, I know when I write this, I was a bit in a mood. I felt quite 
>> frustrated at this point, so I could not keep using the tune when I made the 
>> reply https://reviews.llvm.org/D120305#3347094)
>>
>> I can response to you that I have shared the thread to some folks and at 
>> least two agree with me that nemanjai's reply made the discussion less 
>> technical but more personal.
>
> Can these folks provide their feedback on the thread?

Folks willing to provide feedback usually want to be anonymous. You may 
understand that they don't want to be bothered by further messages and don't 
want to stand by one side.

I know my replies are wasting subscribers' time and I feel sorry about that.
But I can assure to you that if without 
https://reviews.llvm.org/D120305#3347058 and your reply to that comment, we 
were likely in a green state pretty early 🥳


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Yes, because you reverted the change for that one buildbot, of course it is 
green now. You could have also made the buildbot green by disabling tests on 
that bot. Or disabling sanitizers on it. Doesn't change the fact that the 
configuration it was originally testing is still broken, you just hid the 
failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[clang] f348039 - [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via cfe-commits]

Author: Itay Bookstein
Date: 2022-02-26T11:17:49+02:00
New Revision: f3480390be614604907be447afa3f5ab10b97d04

URL: 
https://github.com/llvm/llvm-project/commit/f3480390be614604907be447afa3f5ab10b97d04
DIFF: 
https://github.com/llvm/llvm-project/commit/f3480390be614604907be447afa3f5ab10b97d04.diff

LOG: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

The purpose of this change is to fix the following codegen bug:

```
// main.c
__attribute__((cpu_specific(generic)))
int *foo(void) { static int z; return &z;}
int main() { return *foo() = 5; }

// other.c
__attribute__((cpu_dispatch(generic))) int *foo(void);

// run:
clang main.c other.c -o main; ./main
```

This will segfault prior to the change, and return the correct
exit code 5 after the change.

The underlying cause is that when a translation unit contains
a cpu_specific function without the corresponding cpu_dispatch
the generated code binds the reference to foo() against a
GlobalIFunc whose resolver is undefined. This is invalid: the
resolver must be defined in the same translation unit as the
ifunc, but historically the LLVM bitcode verifier did not check
that. The generated code then binds against the resolver rather
than the ifunc, so it ends up calling the resolver rather than
the resolvee. In the example above it treats its return value as
an int *, therefore trying to write to program text.

The root issue at the representation level is that GlobalIFunc,
like GlobalAlias, does not support a "declaration" state. The
object which provides the correct semantics in these cases
is a Function declaration, but unlike Functions, changing a
declaration to a definition in the GlobalIFunc case constitutes
a change of the object type, as opposed to simply emitting code
into a Function.

I think this limitation is unlikely to change, so I implemented
the fix by returning a function declaration rather than an ifunc
when encountering cpu_specific, and upgrading it to an ifunc
when emitting cpu_dispatch.
This uses `takeName` + `replaceAllUsesWith` in similar vein to
other places where the correct IR object type cannot be known
locally/up-front, like in `CodeGenModule::EmitAliasDefinition`.

Previous discussion in: https://reviews.llvm.org/D112349

Signed-off-by: Itay Bookstein 

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D120266

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/attr-cpuspecific.c

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 37c0b793134f7..9c76648f5f193 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3593,16 +3593,28 @@ void 
CodeGenModule::emitCPUDispatchDefinition(GlobalDecl GD) {
   CGF.EmitMultiVersionResolver(ResolverFunc, Options);
 
   if (getTarget().supportsIFunc()) {
+llvm::GlobalValue::LinkageTypes Linkage = getMultiversionLinkage(*this, 
GD);
+auto *IFunc = cast(
+GetOrCreateMultiVersionResolver(GD, DeclTy, FD));
+
+// Fix up function declarations that were created for cpu_specific before
+// cpu_dispatch was known
+if (!dyn_cast(IFunc)) {
+  assert(cast(IFunc)->isDeclaration());
+  auto *GI = llvm::GlobalIFunc::create(DeclTy, 0, Linkage, "", 
ResolverFunc,
+   &getModule());
+  GI->takeName(IFunc);
+  IFunc->replaceAllUsesWith(GI);
+  IFunc->eraseFromParent();
+  IFunc = GI;
+}
+
 std::string AliasName = getMangledNameImpl(
 *this, GD, FD, /*OmitMultiVersionMangling=*/true);
 llvm::Constant *AliasFunc = GetGlobalValue(AliasName);
 if (!AliasFunc) {
-  auto *IFunc = cast(GetOrCreateLLVMFunction(
-  AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,
-  /*IsThunk=*/false, llvm::AttributeList(), NotForDefinition));
-  auto *GA = llvm::GlobalAlias::create(DeclTy, 0,
-   getMultiversionLinkage(*this, GD),
-   AliasName, IFunc, &getModule());
+  auto *GA = llvm::GlobalAlias::create(DeclTy, 0, Linkage, AliasName, 
IFunc,
+   &getModule());
   SetCommonAttributes(GD, GA);
 }
   }
@@ -3650,7 +3662,9 @@ llvm::Constant 
*CodeGenModule::GetOrCreateMultiVersionResolver(
 }
   }
 
-  if (getTarget().supportsIFunc()) {
+  // For cpu_specific, don't create an ifunc yet because we don't know if the
+  // cpu_dispatch will be emitted in this translation unit.
+  if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(
 DeclTy, getContext().getTargetAddressSpace(FD->getType())),

diff  --git a/clang/test/CodeGen/attr-cpuspecific.c 
b/clang/test/CodeGen/attr-cpuspecific.c
index

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3480390be61: [clang][CodeGen] Avoid emitting ifuncs with 
undefined resolvers (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,9 +8,12 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
+// LINUX: @OrderDispatchUsageSpecific = weak_odr alias void (), void ()* @OrderDispatchUsageSpecific.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
+// LINUX: @OrderSpecificUsageDispatch = weak_odr alias void (), void ()* @OrderSpecificUsageDispatch.ifunc
 // LINUX: @NoSpecifics = weak_odr alias void (), void ()* @NoSpecifics.ifunc
 // LINUX: @HasGeneric = weak_odr alias void (), void ()* @HasGeneric.ifunc
 // LINUX: @HasParams = weak_odr alias void (i32, double), void (i32, double)* @HasParams.ifunc
@@ -18,10 +21,12 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
+// LINUX: @OrderDispatchUsageSpecific.ifunc = weak_odr ifunc void (), void ()* ()* @OrderDispatchUsageSpecific.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
+// LINUX: @OrderSpecificUsageDispatch.ifunc = weak_odr ifunc void (), void ()* ()* @OrderSpecificUsageDispatch.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
 // LINUX: @HasGeneric.ifunc = weak_odr ifunc void (), void ()* ()* @HasGeneric.resolver
 // LINUX: @HasParams.ifunc = weak_odr ifunc void (i32, double), void (i32, double)* ()* @HasParams.resolver
@@ -34,6 +39,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +100,31 @@
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
 
+ATTR(cpu_specific(knl))
+void CpuSpecificNoDispatch(void) {}
+// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
+ATTR(cpu_dispatch(knl))
+void OrderDispatchUsageSpecific(void);
+// LINUX: define weak_odr void ()* @OrderDispatchUsageSpecific.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @OrderDispatchUsageSpecific.Z
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @OrderDispatchUsageSpecific() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @OrderDispatchUsageSpecific.Z()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
+// CHECK: define {{.*}}void @OrderDispatchUsageSpecific.Z()
+
+ATTR(cpu_specific(knl))
+void OrderSpecificUsageDispatch(void) {}
+// CHECK: define {{.*}}void @OrderSpecificUsageDispatch.Z() #[[K:[0-9]+]]
+
 void usages(void) {
   SingleVersion();
   // LINUX: @SingleVersion.ifunc()
@@ -93,8 +138,19 @@
   ThreeVersionsSameAttr();
   // LINUX: @ThreeVersionsSameAttr.ifunc()
   // WINDOWS: @ThreeVersionsSameAttr()
+  CpuSpecificNoDispatch();
+  // LINUX: @CpuSpecificNoDispatch.ifunc()
+  // WINDOWS: @CpuSpecificNoDispatch()
+  OrderDispatchUsageSpecific();
+  

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D120305#3347161 , @MaskRay wrote:

> In D120305#3347160 , @nikic wrote:
>
>> @MaskRay Please revert the change and all dependent changes you have made. A 
>> revert is not a personal affront to you. It's not a judgement that you or 
>> your change are bad. It's a simple matter of policy and standard procedure. 
>> There's a good chance that next week you'll get confirmation that it's 
>> indeed some outdated libraries on the buildbot, and the change can reland 
>> without any changes on your side. Or maybe it turns out that this default is 
>> not quite viable for powerpc targets yet. Who knows.
>>
>> Don't worry about the churn. These short-term reverts happen all the time, 
>> we're used to it. Especially for tiny changes like this one, it's really no 
>> problem (reverts can be more annoying if the commit touches 1500 test files).
>
> I have mentioned that https://lab.llvm.org/buildbot/#/builders/57 has been 
> green.

Disabling the failing tests with an unreviewed patch is not the right way to 
fix this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347177 , @nikic wrote:

> Yes, because you reverted the change for that one buildbot, of course it is 
> green now. You could have also made the buildbot green by disabling tests on 
> that bot. Or disabling sanitizers on it. Doesn't change the fact that the 
> configuration it was originally testing is still broken, you just hid the 
> failure.



In D120305#3347184 , @tstellar wrote:

> In D120305#3347161 , @MaskRay wrote:
>
>> In D120305#3347160 , @nikic wrote:
>>
>>> @MaskRay Please revert the change and all dependent changes you have made. 
>>> A revert is not a personal affront to you. It's not a judgement that you or 
>>> your change are bad. It's a simple matter of policy and standard procedure. 
>>> There's a good chance that next week you'll get confirmation that it's 
>>> indeed some outdated libraries on the buildbot, and the change can reland 
>>> without any changes on your side. Or maybe it turns out that this default 
>>> is not quite viable for powerpc targets yet. Who knows.
>>>
>>> Don't worry about the churn. These short-term reverts happen all the time, 
>>> we're used to it. Especially for tiny changes like this one, it's really no 
>>> problem (reverts can be more annoying if the commit touches 1500 test 
>>> files).
>>
>> I have mentioned that https://lab.llvm.org/buildbot/#/builders/57 has been 
>> green.
>
> Disabling the failing tests with an unreviewed patch is not the right way to 
> fix this.

clang-ppc64le-rhel got `-DCLANG_DEFAULT_PIE_ON_LINUX=OFF` ~9 hours ago. If any 
of you can make the configuration live on the bot, it will work and we can 
re-enable the tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D120305#3347177 , @nikic wrote:

> Yes, because you reverted the change for that one buildbot, of course it is 
> green now. You could have also made the buildbot green by disabling tests on 
> that bot. Or disabling sanitizers on it. Doesn't change the fact that the 
> configuration it was originally testing is still broken, you just hid the 
> failure.

The build is green because of this commit: 
https://github.com/llvm/llvm-project/commit/274ec425dcc3e3f637dd006c5e9ae33bd0e2e917
  The buildbot change you are referring to, which is 
https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad,
 has not taken effect yet, because the buildbot server has not been restarted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

In D120305#3347192 , @tstellar wrote:

> In D120305#3347177 , @nikic wrote:
>
>> Yes, because you reverted the change for that one buildbot, of course it is 
>> green now. You could have also made the buildbot green by disabling tests on 
>> that bot. Or disabling sanitizers on it. Doesn't change the fact that the 
>> configuration it was originally testing is still broken, you just hid the 
>> failure.
>
> The build is green because of this commit: 
> https://github.com/llvm/llvm-project/commit/274ec425dcc3e3f637dd006c5e9ae33bd0e2e917
>   The buildbot change you are referring to, which is 
> https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad,
>  has not taken effect yet, because the buildbot server has not been restarted.

Wow, that's even worse. So now it's not just a change to that one buildbot, but 
sanitizer tests were disabled for powerpc entirely?!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347193 , @nikic wrote:

> In D120305#3347192 , @tstellar 
> wrote:
>
>> In D120305#3347177 , @nikic wrote:
>>
>>> Yes, because you reverted the change for that one buildbot, of course it is 
>>> green now. You could have also made the buildbot green by disabling tests 
>>> on that bot. Or disabling sanitizers on it. Doesn't change the fact that 
>>> the configuration it was originally testing is still broken, you just hid 
>>> the failure.
>>
>> The build is green because of this commit: 
>> https://github.com/llvm/llvm-project/commit/274ec425dcc3e3f637dd006c5e9ae33bd0e2e917
>>   The buildbot change you are referring to, which is 
>> https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad,
>>  has not taken effect yet, because the buildbot server has not been 
>> restarted.
>
> Wow, that's even worse. So now it's not just a change to that one buildbot, 
> but sanitizer tests were disabled for powerpc entirely?!

Only few sanitizer_common and lsan tests, not entirely. It should be re-enabled 
pretty soon once the llvm-zorg change is made live.
That was I mentioned that "we can enable the tests".

I think at this point, if you prefer, I am happy to revert this change the 
disabling (it needs quite a bit of tests), so that we can know whether -fno-pic 
and -fpie have a large difference on sqlite3 performance.

I lost my control when I saw https://reviews.llvm.org/D120305#3347058 and Tom's 
first reply to it.
For quite few hours yesterday, I did not know my fixed did not fix the problem 
or that nobody tried making llvm-zorg change work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 411589.
xbolva00 added a comment.

Fixed failing Parser test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119061/new/

https://reviews.llvm.org/D119061

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-noinline.cpp
  clang/test/Parser/stmt-attributes.c
  clang/test/Sema/attr-noinline.c
  clang/test/Sema/attr-noinline.cpp

Index: clang/test/Sema/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-noinline.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int bar();
+
+[[gnu::always_inline]] void always_inline_fn(void) { }
+[[gnu::flatten]] void flatten_fn(void) { }
+
+[[gnu::noinline]] void noinline_fn(void) { }
+
+void foo() {
+  [[clang::noinline]] bar();
+  [[clang::noinline(0)]] bar(); // expected-error {{'noinline' attribute takes no arguments}}
+  int x;
+  [[clang::noinline]] x = 0; // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] { asm("nop"); } // expected-warning {{'noinline' attribute is ignored because there exists no call expression inside the statement}}
+  [[clang::noinline]] label: x = 1; // expected-error {{'noinline' attribute only applies to functions and statements}}
+
+
+  [[clang::noinline]] always_inline_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
+  [[clang::noinline]] flatten_fn(); // expected-warning {{statement attribute 'noinline' has higher precedence than function attribute 'flatten'}}
+  [[clang::noinline]] noinline_fn();
+
+  [[gnu::noinline]] bar(); // expected-warning {{attribute is ignored on this statement as it only applies to functions; use '[[clang::noinline]]' on statements}}
+  __attribute__((noinline)) bar(); // expected-warning {{attribute is ignored on this statement as it only applies to functions; use '[[clang::noinline]]' on statements}}
+}
+
+[[clang::noinline]] static int i = bar(); // expected-error {{'noinline' attribute only applies to functions and statements}}
Index: clang/test/Sema/attr-noinline.c
===
--- clang/test/Sema/attr-noinline.c
+++ clang/test/Sema/attr-noinline.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only
 
-int a __attribute__((noinline)); // expected-warning {{'noinline' attribute only applies to functions}}
+int a __attribute__((noinline)); // expected-error {{'noinline' attribute only applies to functions and statements}}
 
 void t1(void) __attribute__((noinline));
 
Index: clang/test/Parser/stmt-attributes.c
===
--- clang/test/Parser/stmt-attributes.c
+++ clang/test/Parser/stmt-attributes.c
@@ -45,7 +45,7 @@
   }
 
   __attribute__((fastcall)) goto there; // expected-error {{'fastcall' attribute cannot be applied to a statement}}
-  __attribute__((noinline)) there : // expected-warning {{'noinline' attribute only applies to functions}}
+  __attribute__((noinline)) there : // expected-error {{'noinline' attribute only applies to functions and statements}}
 
 __attribute__((weakref)) return; // expected-error {{'weakref' attribute only applies to variables and functions}}
 
Index: clang/test/CodeGen/attr-noinline.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noinline.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+void g(bool);
+
+static int baz(int x) {
+return x * 10;
+}
+
+[[clang::noinline]] bool noi() { }
+
+void foo(int i) {
+  [[clang::noinline]] bar();
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR:[0-9]+]]
+  [[clang::noinline]] i = baz(i);
+// CHECK: call noundef i32 @_ZL3bazi({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] (i = 4, bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] (void)(bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+  [[clang::noinline]] f(bar(), bar());
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call noundef zeroext i1 @_Z3barv() #[[NOINLINEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOINLINEATTR]]
+  [[clang::noinline]] [] { bar(); bar(); }(); // noinline only applies to the anonymous function call
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* {{[^,]*}} %ref.tmp) #[[NOINLINEATTR]]
+  [[clang::noinline]] for (bar(); bar(); bar()) {}
+// CHECK: call noundef zeroext i1 @_Z3barv() #[

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3347194 , @MaskRay wrote:

> In D120305#3347193 , @nikic wrote:
>
>> In D120305#3347192 , @tstellar 
>> wrote:
>>
>>> In D120305#3347177 , @nikic wrote:
>>>
 Yes, because you reverted the change for that one buildbot, of course it 
 is green now. You could have also made the buildbot green by disabling 
 tests on that bot. Or disabling sanitizers on it. Doesn't change the fact 
 that the configuration it was originally testing is still broken, you just 
 hid the failure.
>>>
>>> The build is green because of this commit: 
>>> https://github.com/llvm/llvm-project/commit/274ec425dcc3e3f637dd006c5e9ae33bd0e2e917
>>>   The buildbot change you are referring to, which is 
>>> https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad,
>>>  has not taken effect yet, because the buildbot server has not been 
>>> restarted.
>>
>> Wow, that's even worse. So now it's not just a change to that one buildbot, 
>> but sanitizer tests were disabled for powerpc entirely?!
>
> Only few sanitizer_common and lsan tests, not entirely. It should be 
> re-enabled pretty soon once the llvm-zorg change is made live.
> That was I mentioned that "we can enable the tests".
>
> I think at this point, if you prefer, I am happy to revert this change the 
> disabling (it needs quite a bit of tests), so that we can know whether 
> -fno-pic and -fpie have a large difference on sqlite3 performance.
>
> I lost my control when I saw https://reviews.llvm.org/D120305#3347058 and 
> Tom's first reply to it.
> For quite few hours yesterday, I did not know my fixed did not fix the 
> problem or that nobody tried making llvm-zorg change work.

@nikic If you still want to revert, you can use https://reviews.llvm.org/P8281
I have done some testing that it is fine.

It is too late here so I'll not be responsive for many hours.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[clang] 1a76d25 - [C++20][Modules][5/8] Diagnose wrong import/export for partition CMIs.

[Iain Sandoe via cfe-commits]

Author: Iain Sandoe
Date: 2022-02-26T11:27:08Z
New Revision: 1a76d2563940e6b4bfcb9e77b8a4d1d3f0cc7d30

URL: 
https://github.com/llvm/llvm-project/commit/1a76d2563940e6b4bfcb9e77b8a4d1d3f0cc7d30
DIFF: 
https://github.com/llvm/llvm-project/commit/1a76d2563940e6b4bfcb9e77b8a4d1d3f0cc7d30.diff

LOG: [C++20][Modules][5/8] Diagnose wrong import/export for partition CMIs.

We cannot export partition implementation CMIs, but we can export the content
of partition interface CMIs.

Differential Revision: https://reviews.llvm.org/D118588

Added: 
clang/test/Modules/cxx20-10-3-ex1.cpp
clang/test/Modules/cxx20-10-3-ex2.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/cxx20-import-diagnostics-a.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 15487260eb732..2528777678571 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10991,6 +10991,8 @@ def ext_module_import_not_at_top_level_noop : ExtWarn<
 def note_module_import_not_at_top_level : Note<"%0 begins here">;
 def err_module_self_import : Error<
   "import of module '%0' appears within same top-level module '%1'">;
+def err_module_self_import_cxx20 : Error<
+  "import of module '%0' appears within its own 
%select{interface|implementation}1">;
 def err_module_import_in_implementation : Error<
   "@import of module '%0' in implementation of '%1'; use #import">;
 
@@ -11024,6 +11026,8 @@ def err_export_using_internal : Error<
 def err_export_not_in_module_interface : Error<
   "export declaration can only be used within a module interface unit"
   "%select{ after the module declaration|}0">;
+def err_export_partition_impl : Error<
+  "module partition implementations cannot be exported">;
 def err_export_in_private_module_fragment : Error<
   "export declaration cannot be used in a private module fragment">;
 def note_private_module_fragment : Note<

diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index b2515075a7f23..0606b3a4edae8 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -403,10 +403,16 @@ DeclResult Sema::ActOnModuleImport(SourceLocation 
StartLoc,
   }
 
   // Diagnose self-import before attempting a load.
+  // [module.import]/9
+  // A module implementation unit of a module M that is not a module partition
+  // shall not contain a module-import-declaration nominating M.
+  // (for an implementation, the module interface is imported implicitly,
+  //  but that's handled in the module decl code).
+
   if (getLangOpts().CPlusPlusModules && isCurrentModulePurview() &&
   getCurrentModule()->Name == ModuleName) {
-Diag(ImportLoc, diag::err_module_self_import)
-<< ModuleName << getLangOpts().CurrentModule;
+Diag(ImportLoc, diag::err_module_self_import_cxx20)
+<< ModuleName << !ModuleScopes.back().ModuleInterface;
 return true;
   }
 
@@ -440,8 +446,7 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
   // of the same top-level module. Until we do, make it an error rather than
   // silently ignoring the import.
   // FIXME: Should we warn on a redundant import of the current module?
-  if (!getLangOpts().CPlusPlusModules &&
-  Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
+  if (Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
   (getLangOpts().isCompilingModule() || !getLangOpts().ModulesTS)) {
 Diag(ImportLoc, getLangOpts().isCompilingModule()
 ? diag::err_module_self_import
@@ -482,7 +487,12 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
   if (!ModuleScopes.empty())
 Context.addModuleInitializer(ModuleScopes.back().Module, Import);
 
-  if (!ModuleScopes.empty() && ModuleScopes.back().ModuleInterface) {
+  // A module (partition) implementation unit shall not be exported.
+  if (getLangOpts().CPlusPlusModules && Mod && ExportLoc.isValid() &&
+  Mod->Kind == Module::ModuleKind::ModulePartitionImplementation) {
+Diag(ExportLoc, diag::err_export_partition_impl)
+<< SourceRange(ExportLoc, Path.back().second);
+  } else if (!ModuleScopes.empty() && ModuleScopes.back().ModuleInterface) {
 // Re-export the module if the imported module is exported.
 // Note that we don't need to add re-exported module to Imports field
 // since `Exports` implies the module is imported already.
@@ -494,7 +504,9 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
 // [module.interface]p1:
 // An export-declaration shall inhabit a namespace scope and appear in the
 // purview of a module interface unit.
-Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
+Diag(ExportLoc, diag::err_export_not_in_module

[PATCH] D118588: [C++20][Modules]5/8] Diagnose wrong import/export for partition CMIs.

[Iain Sandoe via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a76d2563940: [C++20][Modules][5/8] Diagnose wrong 
import/export for partition CMIs. (authored by iains).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118588/new/

https://reviews.llvm.org/D118588

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-10-3-ex1.cpp
  clang/test/Modules/cxx20-10-3-ex2.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -118,13 +118,13 @@
 
 module B;
 
-import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+import B; // expected-error {{import of module 'B' appears within its own implementation}}
 
 //--- import-diags-tu10.cpp
 
 export module B;
 
-import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+import B; // expected-error {{import of module 'B' appears within its own interface}}
 
 //--- import-diags-tu11.cpp
 
Index: clang/test/Modules/cxx20-10-3-ex2.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-3-ex2.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-3-ex2-tu1.cpp \
+// RUN:  -o %t/M.pcm
+
+// RUN: %clang_cc1 -std=c++20 -S %t/std10-3-ex2-tu2.cpp \
+// RUN:  -fmodule-file=%t/M.pcm -o %t/tu_8.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-3-ex2-tu3.cpp \
+// RUN:  -o %t/M.pcm -verify
+
+//--- std10-3-ex2-tu1.cpp
+export module M;
+
+//--- std10-3-ex2-tu2.cpp
+module M;
+  // error: cannot import M in its own unit
+import M; // expected-error {{import of module 'M' appears within its own implementation}}
+
+//--- std10-3-ex2-tu3.cpp
+export module M;
+  // error: cannot import M in its own unit
+import M; // expected-error {{import of module 'M' appears within its own interface}}
Index: clang/test/Modules/cxx20-10-3-ex1.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-10-3-ex1.cpp
@@ -0,0 +1,36 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-3-ex1-tu1.cpp \
+// RUN:  -o %t/M_PartImpl.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-3-ex1-tu2.cpp \
+// RUN:  -fmodule-file=%t/M_PartImpl.pcm -o %t/M.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-3-ex1-tu3.cpp \
+// RUN:  -o %t/M_Part.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-3-ex1-tu4.cpp \
+// RUN:  -fmodule-file=%t/M_Part.pcm -o %t/M.pcm
+
+//--- std10-3-ex1-tu1.cpp
+module M:PartImpl;
+
+// expected-no-diagnostics
+
+//--- std10-3-ex1-tu2.cpp
+export module M;
+ // error: exported partition :Part is an implementation unit
+export import :PartImpl; // expected-error {{module partition implementations cannot be exported}}
+
+//--- std10-3-ex1-tu3.cpp
+export module M:Part;
+
+// expected-no-diagnostics
+
+//--- std10-3-ex1-tu4.cpp
+export module M;
+export import :Part;
+
+// expected-no-diagnostics
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -403,10 +403,16 @@
   }
 
   // Diagnose self-import before attempting a load.
+  // [module.import]/9
+  // A module implementation unit of a module M that is not a module partition
+  // shall not contain a module-import-declaration nominating M.
+  // (for an implementation, the module interface is imported implicitly,
+  //  but that's handled in the module decl code).
+
   if (getLangOpts().CPlusPlusModules && isCurrentModulePurview() &&
   getCurrentModule()->Name == ModuleName) {
-Diag(ImportLoc, diag::err_module_self_import)
-<< ModuleName << getLangOpts().CurrentModule;
+Diag(ImportLoc, diag::err_module_self_import_cxx20)
+<< ModuleName << !ModuleScopes.back().ModuleInterface;
 return true;
   }
 
@@ -440,8 +446,7 @@
   // of the same top-level module. Until we do, make it an error rather than
   // silently ignoring the import.
   // FIXME: Should we warn on a redundant import of the current module?
-  if (!getLangOpts().CPlusPlusModules &&
-  Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
+  if (Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
   (getLangOpts().isCompilingModule() || !getLangOpts().ModulesTS)) {
 Diag(ImportLoc, getLangOpt

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 planned changes to this revision.
denzor200 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp:22
+
+#define BOOST_FOREACH(VAR, COL) for(VAR;(COL, false);)
+#define IDENTITY_TYPE(...) __VA_ARGS__

Unfortunately, example does not correspond to the real `BOOST_FOREACH`, the 
tests do not reveal real problems. Will have to redo. I will come back to this 
patch in the future


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

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

[clang-tools-extra] dfed8f5 - [clangd] Add a missing include. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-02-26T13:23:06+01:00
New Revision: dfed8f556d270cec0ac849ce1208671f33454534

URL: 
https://github.com/llvm/llvm-project/commit/dfed8f556d270cec0ac849ce1208671f33454534
DIFF: 
https://github.com/llvm/llvm-project/commit/dfed8f556d270cec0ac849ce1208671f33454534.diff

LOG: [clangd] Add a missing include. NFC.

Added: 


Modified: 
clang-tools-extra/clangd/index/dex/Trigram.h

Removed: 




diff  --git a/clang-tools-extra/clangd/index/dex/Trigram.h 
b/clang-tools-extra/clangd/index/dex/Trigram.h
index f931004ebc223..3fe975e76fe88 100644
--- a/clang-tools-extra/clangd/index/dex/Trigram.h
+++ b/clang-tools-extra/clangd/index/dex/Trigram.h
@@ -26,6 +26,7 @@
 #include "index/dex/Token.h"
 #include "llvm/ADT/bit.h"
 
+#include 
 #include 
 
 namespace clang {



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

[clang-tools-extra] 6c72a97 - [clangd] Qualify calls to std::move to silence -Wunqualified-std-cast-call. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-02-26T13:36:14+01:00
New Revision: 6c72a97c0e2d9dd884d67315629be1c219da3b34

URL: 
https://github.com/llvm/llvm-project/commit/6c72a97c0e2d9dd884d67315629be1c219da3b34
DIFF: 
https://github.com/llvm/llvm-project/commit/6c72a97c0e2d9dd884d67315629be1c219da3b34.diff

LOG: [clangd] Qualify calls to std::move to silence 
-Wunqualified-std-cast-call. NFC.

Added: 


Modified: 
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/Iterator.h
clang-tools-extra/clangd/unittests/DexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 37baf9d3d862a..6aec87d197ed2 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -1504,7 +1504,7 @@ TUScheduler::TUScheduler(const GlobalCompilationDatabase 
&CDB,
  const Options &Opts,
  std::unique_ptr Callbacks)
 : CDB(CDB), Opts(Opts),
-  Callbacks(Callbacks ? move(Callbacks)
+  Callbacks(Callbacks ? std::move(Callbacks)
   : std::make_unique()),
   Barrier(Opts.AsyncThreadsCount), QuickRunBarrier(Opts.AsyncThreadsCount),
   IdleASTs(

diff  --git a/clang-tools-extra/clangd/index/dex/Dex.cpp 
b/clang-tools-extra/clangd/index/dex/Dex.cpp
index 6975533c01f08..5829d780c6072 100644
--- a/clang-tools-extra/clangd/index/dex/Dex.cpp
+++ b/clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -206,7 +206,7 @@ bool Dex::fuzzyFind(const FuzzyFindRequest &Req,
   std::vector> TrigramIterators;
   for (const auto &Trigram : TrigramTokens)
 TrigramIterators.push_back(iterator(Trigram));
-  Criteria.push_back(Corpus.intersect(move(TrigramIterators)));
+  Criteria.push_back(Corpus.intersect(std::move(TrigramIterators)));
 
   // Generate scope tokens for search query.
   std::vector> ScopeIterators;
@@ -215,7 +215,7 @@ bool Dex::fuzzyFind(const FuzzyFindRequest &Req,
   if (Req.AnyScope)
 ScopeIterators.push_back(
 Corpus.boost(Corpus.all(), ScopeIterators.empty() ? 1.0 : 0.2));
-  Criteria.push_back(Corpus.unionOf(move(ScopeIterators)));
+  Criteria.push_back(Corpus.unionOf(std::move(ScopeIterators)));
 
   // Add proximity paths boosting (all symbols, some boosted).
   Criteria.push_back(createFileProximityIterator(Req.ProximityPaths));
@@ -227,12 +227,12 @@ bool Dex::fuzzyFind(const FuzzyFindRequest &Req,
 
   // Use TRUE iterator if both trigrams and scopes from the query are not
   // present in the symbol index.
-  auto Root = Corpus.intersect(move(Criteria));
+  auto Root = Corpus.intersect(std::move(Criteria));
   // Retrieve more items than it was requested: some of  the items with high
   // final score might not be retrieved otherwise.
   // FIXME(kbobyrev): Tune this ratio.
   if (Req.Limit)
-Root = Corpus.limit(move(Root), *Req.Limit * 100);
+Root = Corpus.limit(std::move(Root), *Req.Limit * 100);
   SPAN_ATTACH(Tracer, "query", llvm::to_string(*Root));
   vlog("Dex query tree: {0}", *Root);
 

diff  --git a/clang-tools-extra/clangd/index/dex/Iterator.h 
b/clang-tools-extra/clangd/index/dex/Iterator.h
index 85661eed9fdfd..c7a5bae4adbba 100644
--- a/clang-tools-extra/clangd/index/dex/Iterator.h
+++ b/clang-tools-extra/clangd/index/dex/Iterator.h
@@ -124,8 +124,8 @@ inline void 
populateChildren(std::vector> &) {}
 template 
 void populateChildren(std::vector> &Children,
   std::unique_ptr Head, TailT... Tail) {
-  Children.push_back(move(Head));
-  populateChildren(Children, move(Tail)...);
+  Children.push_back(std::move(Head));
+  populateChildren(Children, std::move(Tail)...);
 }
 } // namespace detail
 
@@ -178,7 +178,7 @@ class Corpus {
   std::unique_ptr intersect(Args... args) const {
 std::vector> Children;
 detail::populateChildren(Children, std::forward(args)...);
-return intersect(move(Children));
+return intersect(std::move(Children));
   }
 
   /// This allows unionOf(create(...), create(...)) syntax.
@@ -186,7 +186,7 @@ class Corpus {
   std::unique_ptr unionOf(Args... args) const {
 std::vector> Children;
 detail::populateChildren(Children, std::forward(args)...);
-return unionOf(move(Children));
+return unionOf(std::move(Children));
   }
 };
 

diff  --git a/clang-tools-extra/clangd/unittests/DexTests.cpp 
b/clang-tools-extra/clangd/unittests/DexTests.cpp
index 17cf8de921d08..cafbfd324840c 100644
--- a/clang-tools-extra/clangd/unittests/DexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -252,7 +252,7 @@ TEST(DexIterators, StringRepresentation) {
   auto I2 = L1.iterator(&Tok);
   EXPECT_EQ(llvm::to_string(*I2), "T=L2");
 
-  auto Tree = C.limit(C.intersect(move(I1), move(I2)), 10);
+  auto Tree = C.limit(C.intersect(std::move(I1), std::move(I2)), 10);
   // AND reo

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 411600.
philnik marked an inline comment as done.
philnik added a comment.

- Fix rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114425/new/

https://reviews.llvm.org/D114425

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtin-bswap128.c
  clang/test/CodeGen/builtins.cpp
  clang/test/Sema/builtin-bswap128.c
  clang/test/Sema/constant-builtins-2.c

Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/Sema/builtin-bswap128.c
===
--- /dev/null
+++ clang/test/Sema/builtin-bswap128.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple i386-mingw32 -fsyntax-only -verify %s
+
+void test() {
+  __builtin_bswap128(1); // expected-error {{use of unknown builtin '__builtin_bswap128'}}
+}
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/test/CodeGen/builtin-bswap128.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-bswap128.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[R1:%.*]] = alloca i128, align 16
+// CHECK-NEXT:[[R2:%.*]] = alloca i128, align 16
+// CHECK-NEXT:store i128 1329227995784915872903807060280344576, i128* [[R1:%.*]], align 16
+// CHECK-NEXT:store i128 2658455991569831745807614120560689152, i128* [[R2:%.*]], align 16
+void test() {
+  __int128 r1 = __builtin_bswap128(1);
+  __int128 r2 = __builtin_bswap128((__int128)2);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2929,7 +2929,8 @@
   }
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::bswap));
   }
   case Builtin::BI__builtin_bitreverse8:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11784,7 +11784,8 @@
 
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10839,8 +10839,8 @@
 /// to be an Integer Constant Expression.
 static QualType DecodeTypeFromStr(const char *&Str, const ASTContext &Context,
   ASTContext::GetBuiltinTypeError &Error,
-  bool &RequiresICE,
-  bool AllowTypeModifiers) {
+  bool &RequiresICE, bool AllowTypeModifiers,
+  bool AllowInt128 = false) {
   // Modifiers.
   int HowLong = 0;
   bool Signed = false, Unsigned = false;
@@ -10983,9 +10983,13 @@
   Type = Context.ShortTy;
 break;
   case 'i':
-if (HowLong == 3)
+if (HowLong == 3) {
+  if (!AllowInt128 && !Context.getTargetInfo().hasInt128Type()) {
+Error = ASTContext::GE_Missing_type;
+return {};
+  }
   Type = Unsigned ? Context.UnsignedInt128Ty : Context.Int128Ty;
-else if (HowLong == 2)
+} else if (HowLong == 2)
   Type = Unsigned ? Context.UnsignedLongLongTy : Context.LongLongTy;
 else if (HowLong == 1)
   Type = Unsigned ? Context.Unsigne

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 411601.
phyBrackets added a comment.

aligned the indentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120489/new/

https://reviews.llvm.org/D120489

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/bstring.c

Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -70,6 +70,11 @@
 
 #endif /* VARIANT */
 
+void top(char *dst) {
+  char buf[10];
+  memcpy(dst, buf, 10); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  (void)buf;
+}
 
 void memcpy0 () {
   char src[] = {1, 2, 3, 4};
@@ -297,9 +302,12 @@
   int dst[5] = {0};
   int *p;
 
-  p = mempcpy(dst, src, 4 * sizeof(int));
+  p = mempcpy(dst, src, 4 * sizeof(int)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  // FIXME: This behaviour is actually Unexpected and needs to be fix, 
+  // mempcpy seems to consider the src buffered byte as uninitialized
+  // and returning undef which is actually not the case It should return something like Unknown .
 
-  clang_analyzer_eval(p == &dst[4]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(p == &dst[4]); // no-warning (above is fatal)
 }
 
 struct st {
@@ -314,9 +322,10 @@
   struct st *p2;
 
   p1 = (&s2) + 1;
-  p2 = mempcpy(&s2, &s1, sizeof(struct st));
-
-  clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}}
+  p2 = mempcpy(&s2, &s1, sizeof(struct st)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  // FIXME: It seems same as mempcpy14() case.
+  
+  clang_analyzer_eval(p1 == p2); // no-warning (above is fatal)
 }
 
 void mempcpy16() {
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -80,7 +80,7 @@
  check::RegionChanges
  > {
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
-  BT_NotCString, BT_AdditionOverflow;
+  BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
   mutable const char *CurrentFunctionDescription;
 
@@ -92,11 +92,13 @@
 DefaultBool CheckCStringOutOfBounds;
 DefaultBool CheckCStringBufferOverlap;
 DefaultBool CheckCStringNotNullTerm;
+DefaultBool CheckCStringUninitializedRead;
 
 CheckerNameRef CheckNameCStringNullArg;
 CheckerNameRef CheckNameCStringOutOfBounds;
 CheckerNameRef CheckNameCStringBufferOverlap;
 CheckerNameRef CheckNameCStringNotNullTerm;
+CheckerNameRef CheckNameCStringUninitializedRead;
   };
 
   CStringChecksFilter Filter;
@@ -257,6 +259,8 @@
   void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
  const Stmt *S, StringRef WarningMsg) const;
   void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
+  void emitUninitializedReadBug(CheckerContext &C, ProgramStateRef State,
+const Expr *E) const;
 
   ProgramStateRef checkAdditionOverflow(CheckerContext &C,
 ProgramStateRef state,
@@ -368,6 +372,14 @@
 return nullptr;
   }
 
+  // Ensure that we wouldn't read uninitialized value.
+  if (Access == AccessKind::read) {
+if (StInBound->getSVal(ER).isUndef()) {
+  emitUninitializedReadBug(C, StInBound, Buffer.Expression);
+  return nullptr;
+}
+  }
+
   // Array bound check succeeded.  From this point forward the array bound
   // should always succeed.
   return StInBound;
@@ -421,6 +433,7 @@
 SVal BufEnd =
 svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
 
+State = CheckLocation(C, State, Buffer, BufStart, Access);
 State = CheckLocation(C, State, Buffer, BufEnd, Access);
 
 // If the buffer isn't large enough, abort.
@@ -580,6 +593,26 @@
   }
 }
 
+void CStringChecker::emitUninitializedReadBug(CheckerContext &C,
+  ProgramStateRef State,
+  const Expr *E) const {
+  if (ExplodedNode *N = C.generateErrorNode(State)) {
+const char *Msg =
+"Bytes string function accesses uninitialized/garbage values";
+if (!BT_UninitRead)
+  BT_UninitRead.reset(
+  new BuiltinBug(Filter.CheckNameCStringUninitializedRead,
+ "Accessing unitialized/garbage values", Msg));
+
+BuiltinBug *BT = static_cast(BT_UninitRead.get());
+
+auto Report = std::make_unique(*BT, Msg, N);
+Report->addRange(E->getSourceRange());
+bugreporter::trackExpressionValue(N, E, *Report)

[clang-tools-extra] e63d7bd - [clangd] Fix include-cleaner false-positive bug

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-02-26T14:11:48+01:00
New Revision: e63d7bdc28e42b537ba29708e7db9f5d1dedefc0

URL: 
https://github.com/llvm/llvm-project/commit/e63d7bdc28e42b537ba29708e7db9f5d1dedefc0
DIFF: 
https://github.com/llvm/llvm-project/commit/e63d7bdc28e42b537ba29708e7db9f5d1dedefc0.diff

LOG: [clangd] Fix include-cleaner false-positive bug

For TemplateSpecializationType, we were checking the node's newness
twice, so it always failed the second test.

Fixes https://github.com/clangd/clangd/issues/1036

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index cb50804585082..90dfb2b4a3501 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -73,10 +73,8 @@ class ReferencedLocationCrawler
   }
 
   bool VisitTemplateSpecializationType(TemplateSpecializationType *TST) {
-if (isNew(TST)) {
-  add(TST->getTemplateName().getAsTemplateDecl()); // Primary template.
-  add(TST->getAsCXXRecordDecl());  // Specialization
-}
+add(TST->getTemplateName().getAsTemplateDecl()); // Primary template.
+add(TST->getAsCXXRecordDecl());  // Specialization
 return true;
   }
 

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp 
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 65942fe29c72c..b6bf7695f8873 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -85,6 +85,19 @@ TEST(IncludeCleaner, ReferencedLocations) {
   "typedef bool ^Y; template  struct ^X {};",
   "X x;",
   },
+  {
+  // https://github.com/clangd/clangd/issues/1036
+  R"cpp(
+struct ^Base { void ^base(); };
+template  struct ^Derived : Base {};
+  )cpp",
+  R"cpp(
+class Holder {
+  void foo() { Member.base(); }
+  Derived<0> Member;
+};
+  )cpp",
+  },
   {
   "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
   "Bar b;",
@@ -206,7 +219,8 @@ TEST(IncludeCleaner, ReferencedLocations) {
   {
   "enum class ^Color : char {};",
   "Color *c;",
-  }};
+  },
+  };
   for (const TestCase &T : Cases) {
 TestTU TU;
 TU.Code = T.MainCode;



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

[clang-tools-extra] b09c12c - [clangd] Fix wrong included header. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-02-26T14:21:52+01:00
New Revision: b09c12c4b9767e97ce8b02c707eb0dd3272e6de3

URL: 
https://github.com/llvm/llvm-project/commit/b09c12c4b9767e97ce8b02c707eb0dd3272e6de3
DIFF: 
https://github.com/llvm/llvm-project/commit/b09c12c4b9767e97ce8b02c707eb0dd3272e6de3.diff

LOG: [clangd] Fix wrong included header. NFC

Added: 


Modified: 
clang-tools-extra/clangd/index/IndexAction.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/IndexAction.cpp 
b/clang-tools-extra/clangd/index/IndexAction.cpp
index 708df9ad9f3a..3899902839a7 100644
--- a/clang-tools-extra/clangd/index/IndexAction.cpp
+++ b/clang-tools-extra/clangd/index/IndexAction.cpp
@@ -16,7 +16,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexingOptions.h"
 #include 



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

[PATCH] D119481: run-clang-tidy: Fix infinite loop on windows

[Fabian Keßler via Phabricator via cfe-commits]

Febbe updated this revision to Diff 411606.
Febbe added a comment.

Applied the feedback


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119481/new/

https://reviews.llvm.org/D119481

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -64,13 +64,14 @@
 
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
-  result = './'
+  result = os.path.realpath('./')
   while not os.path.isfile(os.path.join(result, path)):
-if os.path.realpath(result) == '/':
+parent = os.path.dirname(result)
+if result == parent:
   print('Error: could not find compilation database.')
   sys.exit(1)
-result += '../'
-  return os.path.realpath(result)
+result = parent
+  return result
 
 
 def make_absolute(f, directory):


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -64,13 +64,14 @@
 
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
-  result = './'
+  result = os.path.realpath('./')
   while not os.path.isfile(os.path.join(result, path)):
-if os.path.realpath(result) == '/':
+parent = os.path.dirname(result)
+if result == parent:
   print('Error: could not find compilation database.')
   sys.exit(1)
-result += '../'
-  return os.path.realpath(result)
+result = parent
+  return result
 
 
 def make_absolute(f, directory):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119481: run-clang-tidy: Fix infinite loop on windows

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added a comment.

@JonasToth yes, it would be nice, to test this and then push it for me. Also a 
backport to 14.0 would be good :).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119481/new/

https://reviews.llvm.org/D119481

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

[PATCH] D119481: run-clang-tidy: Fix infinite loop on windows

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added a comment.

@salman-javed-nz you're welcome, I only fixed it because I saw the bug in the 
trace directly. Normally, I would only fix C/C++ stuff :D.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119481/new/

https://reviews.llvm.org/D119481

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

[clang-tools-extra] 671eab2 - [clangd] Support IncludeFixer or base specifiers

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-02-26T15:45:59+01:00
New Revision: 671eab254a7fc912857b697d5b6e414b71b75a60

URL: 
https://github.com/llvm/llvm-project/commit/671eab254a7fc912857b697d5b6e414b71b75a60
DIFF: 
https://github.com/llvm/llvm-project/commit/671eab254a7fc912857b697d5b6e414b71b75a60.diff

LOG: [clangd] Support IncludeFixer or base specifiers

Added: 


Modified: 
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeFixer.cpp 
b/clang-tools-extra/clangd/IncludeFixer.cpp
index 7994e5f49920..66e955270914 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
@@ -182,6 +183,8 @@ std::vector IncludeFixer::fix(DiagnosticsEngine::Level 
DiagLevel,
 
   case diag::err_unknown_typename:
   case diag::err_unknown_typename_suggest:
+  case diag::err_unknown_type_or_class_name_suggest:
+  case diag::err_expected_class_name:
   case diag::err_typename_nested_not_found:
   case diag::err_no_template:
   case diag::err_no_template_suggest:

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 7de3746d2251..7fd7babe68a7 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1044,6 +1044,7 @@ void foo() {
   // considered the unresolved type.
   $unqualified2[[X]]::Nested n;
 }
+struct S : $base[[X]] {};
 }
 void bar() {
   ns::$qualified1[[X]] x; // ns:: is valid.
@@ -1087,7 +1088,11 @@ using Type = ns::$template[[Foo]];
  "no template named 'Foo' in namespace 'ns'"),
 diagName("no_member_template"),
 withFix(Fix(Test.range("insert"), "#include \"foo.h\"\n",
-"Include \"foo.h\" for symbol ns::Foo");
+"Include \"foo.h\" for symbol ns::Foo"))),
+  AllOf(Diag(Test.range("base"), "expected class name"),
+diagName("expected_class_name"),
+withFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Include \"x.h\" for symbol ns::X");
 }
 
 TEST(IncludeFixerTest, MultipleMatchedSymbols) {



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

[PATCH] D120616: [clangd] IncludeFixer: resolve Class in ns::Class::method();

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: usaxena95.
Herald added subscribers: kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

We distinguish by case, assuming a::b:: is a namespace and a::B:: is a class.
Crude, but be a significant improvement over assuming it's always a namespace.

(Motivating case from clangd: clangd::CommandMangler::detect()).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120616

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1178,25 +1178,34 @@
 
 TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) {
   Annotations Test(R"cpp(// error-ok
-$insert[[]]namespace ns {
+$insert[[]]namespace ns {}
+void g() {
+  ns::$lower[[scope]]::X_Y();
+  ns::$upper[[Scope]]::X_Y();
 }
-void g() {  ns::$[[scope]]::X_Y();  }
   )cpp");
   TestTU TU;
   TU.Code = std::string(Test.code());
   // FIXME: Figure out why this is needed and remove it, PR43662.
   TU.ExtraArgs.push_back("-fno-ms-compatibility");
   auto Index = buildIndexWithSymbol(
-  SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+  {SymbolWithHeader{"ns::scope::X_Y", "unittest:///ns.h", "\"ns.h\""},
+   SymbolWithHeader{"ns::Scope", "unittest:///class.h", "\"class.h\""}});
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
   UnorderedElementsAre(
-  AllOf(Diag(Test.range(), "no member named 'scope' in namespace 
'ns'"),
+  AllOf(Diag(Test.range("lower"),
+ "no member named 'scope' in namespace 'ns'"),
 diagName("no_member"),
-withFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
-"Include \"x.h\" for symbol ns::scope::X_Y");
+withFix(Fix(Test.range("insert"), "#include \"ns.h\"\n",
+"Include \"ns.h\" for symbol ns::scope::X_Y"))),
+  AllOf(Diag(Test.range("upper"),
+ "no member named 'Scope' in namespace 'ns'"),
+diagName("no_member"),
+withFix(Fix(Test.range("insert"), "#include \"class.h\"\n",
+"Include \"class.h\" for symbol ns::Scope");
 }
 
 TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) {
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -417,15 +417,20 @@
 }
   }
 
-  if (UnresolvedIsSpecifier) {
-// If the unresolved name is a specifier e.g.
-//  clang::clangd::X
-// ~~
-// We try to resolve clang::clangd::X instead of clang::clangd.
-// FIXME: We won't be able to fix include if the specifier is what we
-// should resolve (e.g. it's a class scope specifier). Collecting include
-// headers for nested types could make this work.
-
+  // If the unresolved name is a namespace qualifier e.g.
+  //  clang::clangd::X
+  // ~~
+  // we want to resolve clang::clangd::X instead of clang::clangd.
+  //
+  // On the other hand if it's a class name qualifier e.g.
+  //  clang::PrecompiledPreamble::Build()
+  // ~~~
+  // then we should resolve the class clang::PrecompiledPreamble.
+  // 
+  // We simply guess based on the case of the identifier. Namespaces are almost
+  // universally lowercase, while class names are often uppercase.
+  assert(!Result.Name.empty());
+  if (UnresolvedIsSpecifier && !isUppercase(Result.Name.front())) {
 // Not using the end location as it doesn't always point to the end of
 // identifier.
 if (auto QualifiedByUnresolved =


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1178,25 +1178,34 @@
 
 TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) {
   Annotations Test(R"cpp(// error-ok
-$insert[[]]namespace ns {
+$insert[[]]namespace ns {}
+void g() {
+  ns::$lower[[scope]]::X_Y();
+  ns::$upper[[Scope]]::X_Y();
 }
-void g() {  ns::$[[scope]]::X_Y();  }
   )cpp");
   TestTU TU;
   TU.Code = std::string(Test.code());
   // FIXME: Figure out why this is needed and remove it, PR43662.
   TU.ExtraArgs.push_back("-fno-ms-compatibility");
   auto Index = buildIndexWithSymbol(
-  SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+  

[PATCH] D120618: [clang-format][docs] Fix a bad comment

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry created this revision.
kuzkry added a reviewer: HazardyKnusperkeks.
kuzkry added a project: clang-format.
kuzkry requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Follow up to 8f310d1967c20d348c617af3a30999031c71fee0 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120618

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2524,7 +2524,7 @@
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
   ///
-  /// In clang-format 13 and 14 it was named ``IndentRequires``.
+  /// In clang-format 12, 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2710,7 +2710,7 @@
   Indent the requires clause in a template. This only applies when
   ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
 
-  In clang-format 13 and 14 it was named ``IndentRequires``.
+  In clang-format 12, 13 and 14 it was named ``IndentRequires``.
 
   .. code-block:: c++
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2524,7 +2524,7 @@
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
   ///
-  /// In clang-format 13 and 14 it was named ``IndentRequires``.
+  /// In clang-format 12, 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2710,7 +2710,7 @@
   Indent the requires clause in a template. This only applies when
   ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
 
-  In clang-format 13 and 14 it was named ``IndentRequires``.
+  In clang-format 12, 13 and 14 it was named ``IndentRequires``.
 
   .. code-block:: c++
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120618: [clang-format][docs] Fix a bad comment

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.

Hey @HazardyKnusperkeks,

I've just noticed that the suggestion from your inline comment in D119682 
 is slightly incorrect. The first commit that 
introduced `IndentRequires` was 840e651dc6d7fe652667eb8b4d04ef4daf4769df 
 and it 
was released already in clang 12.

Btw. could you please delived this one for me? My name and email in git format 
is "Krystian Kuzniarek "


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120618/new/

https://reviews.llvm.org/D120618

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

[PATCH] D120619: [clangd] Support include-fixer inside macro arguments.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: adamcz.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Motivating case: EXPECT_EQ(42, missingFunction(bar));


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120619

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -36,6 +36,7 @@
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Contains;
+using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
@@ -1090,6 +1091,24 @@
 "Include \"foo.h\" for symbol ns::Foo");
 }
 
+TEST(IncludeFixerTest, TypoInMacro) {
+  auto TU = TestTU::withCode(R"cpp(// error-ok
+#define ID(T) T
+X a1;
+ID(X a2);
+ns::X a3;
+ID(ns::X a4);
+namespace ns{};
+ns::X a5;
+ID(ns::X a6);
+)cpp");
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"ns::X", "unittest:///ns.h", "\"x.h\""}});
+  TU.ExternalIndex = Index.get();
+  EXPECT_THAT(*TU.build().getDiagnostics(), Each(withFix(_)));
+}
+
 TEST(IncludeFixerTest, MultipleMatchedSymbols) {
   Annotations Test(R"cpp(// error-ok
 $insert[[]]namespace na {
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -340,8 +340,8 @@
   SourceLocation Loc,
   const LangOptions &LangOpts) {
   std::string Result;
-
-  SourceLocation NextLoc = Loc;
+  // Accept qualifier written within macro arguments, but not macro bodies.
+  SourceLocation NextLoc = SM.getTopMacroCallerLoc(Loc);
   while (auto CCTok = Lexer::findNextToken(NextLoc, SM, LangOpts)) {
 if (!CCTok->is(tok::coloncolon))
   break;
@@ -370,40 +370,46 @@
   llvm::Optional UnresolvedScope;
 };
 
+llvm::Optional getSpelledSpecifier(const CXXScopeSpec &SS,
+const SourceManager &SM) {
+  // Support specifiers written within a single macro argument.
+  if (!SM.isWrittenInSameFile(SS.getBeginLoc(), SS.getEndLoc()))
+return llvm::None;
+  SourceRange Range(SM.getTopMacroCallerLoc(SS.getBeginLoc()), SM.getTopMacroCallerLoc(SS.getEndLoc()));
+  if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID())
+return llvm::None;
+
+  return (toSourceCode(SM, Range) + "::").str();
+}
+
 // Extracts unresolved name and scope information around \p Unresolved.
 // FIXME: try to merge this with the scope-wrangling code in CodeComplete.
 llvm::Optional extractUnresolvedNameCheaply(
 const SourceManager &SM, const DeclarationNameInfo &Unresolved,
 CXXScopeSpec *SS, const LangOptions &LangOpts, bool UnresolvedIsSpecifier) {
-  bool Invalid = false;
-  llvm::StringRef Code = SM.getBufferData(
-  SM.getDecomposedLoc(Unresolved.getBeginLoc()).first, &Invalid);
-  if (Invalid)
-return llvm::None;
   CheapUnresolvedName Result;
   Result.Name = Unresolved.getAsString();
   if (SS && SS->isNotEmpty()) { // "::" or "ns::"
 if (auto *Nested = SS->getScopeRep()) {
-  if (Nested->getKind() == NestedNameSpecifier::Global)
+  if (Nested->getKind() == NestedNameSpecifier::Global) {
 Result.ResolvedScope = "";
-  else if (const auto *NS = Nested->getAsNamespace()) {
-auto SpecifiedNS = printNamespaceScope(*NS);
+  } else if (const auto *NS = Nested->getAsNamespace()) {
+std::string SpecifiedNS = printNamespaceScope(*NS);
+llvm::Optional Spelling = getSpelledSpecifier(*SS, SM);
 
 // Check the specifier spelled in the source.
-// If the resolved scope doesn't end with the spelled scope. The
-// resolved scope can come from a sema typo correction. For example,
+// If the resolved scope doesn't end with the spelled scope, the
+// resolved scope may come from a sema typo correction. For example,
 // sema assumes that "clangd::" is a typo of "clang::" and uses
 // "clang::" as the specified scope in:
 // namespace clang { clangd::X; }
 // In this case, we use the "typo" specifier as extra scope instead
 // of using the scope assumed by sema.
-auto B = SM.getFileOffset(SS->getBeginLoc());
-auto E = SM.getFileOffset(SS->getEndLoc());
-std::string Spelling = (Code.substr(B, E - B) + "::").str();
-if (llvm::StringRef(SpecifiedNS).endswith(Spelling))
-  Result.ResolvedSc

[PATCH] D120621: [clang-format] Fix BreakBeforeBinaryOperators with TemplateCloser on the lhs

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, curdeius, MyDeveloperDay.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The check for TT_templateCloser was done first and prevented any line breaks. 
Rearrange the statements a bit.

Fixes https://github.com/llvm/llvm-project/issues/54057


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120621

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6440,6 +6440,46 @@
Style);
 }
 
+TEST_F(FormatTest, BreakBinaryOperatorsInPresenceOfTemplates) {
+  auto Style = getLLVMStyleWithColumns(45);
+  EXPECT_EQ(Style.BreakBeforeBinaryOperators, FormatStyle::BOS_None);
+  verifyFormat(
+  "bool b =\n"
+  "is_default_constructible_v> and\n"
+  "is_copy_constructible_v> and\n"
+  "is_move_constructible_v> and\n"
+  "is_copy_assignable_v> and\n"
+  "is_move_assignable_v> and\n"
+  "is_destructible_v> and\n"
+  "is_swappable_v> and\n"
+  "is_callable_v(T)>;",
+  Style);
+
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  verifyFormat(
+  "bool b = is_default_constructible_v>\n"
+  " and is_copy_constructible_v>\n"
+  " and is_move_constructible_v>\n"
+  " and is_copy_assignable_v>\n"
+  " and is_move_assignable_v>\n"
+  " and is_destructible_v>\n"
+  " and is_swappable_v>\n"
+  " and is_callable_v(T)>;",
+  Style);
+
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  verifyFormat(
+  "bool b = is_default_constructible_v>\n"
+  " and is_copy_constructible_v>\n"
+  " and is_move_constructible_v>\n"
+  " and is_copy_assignable_v>\n"
+  " and is_move_assignable_v>\n"
+  " and is_destructible_v>\n"
+  " and is_swappable_v>\n"
+  " and is_callable_v(T)>;",
+  Style);
+}
+
 TEST_F(FormatTest, ConstructorInitializers) {
   verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
   verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
@@ -24060,6 +24100,40 @@
"return number_zero_v;\n"
"  }\n"
"};");
+
+  auto Style = getLLVMStyle();
+
+  verifyFormat(
+  "template \n"
+  "  requires is_default_constructible_v> and\n"
+  "   is_copy_constructible_v> and\n"
+  "   is_move_constructible_v> and\n"
+  "   is_copy_assignable_v> and "
+  "is_move_assignable_v> and\n"
+  "   is_destructible_v> and is_swappable_v> and\n"
+  "   is_callable_v(T)> and\n"
+  "   is_same_v(declval()))> and\n"
+  "   is_same_v(declval()))> and\n"
+  "   is_same_v(declval()))>\n"
+  "struct S {};",
+  Style);
+
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  verifyFormat(
+  "template \n"
+  "  requires is_default_constructible_v>\n"
+  "   and is_copy_constructible_v>\n"
+  "   and is_move_constructible_v>\n"
+  "   and is_copy_assignable_v> and "
+  "is_move_assignable_v>\n"
+  "   and is_destructible_v> and is_swappable_v>\n"
+  "   and is_callable_v(T)>\n"
+  "   and is_same_v(declval()))>\n"
+  "   and is_same_v(declval()))>\n"
+  "   and is_same_v(declval()))>\n"
+  "struct S {};",
+  Style);
 }
 
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4410,6 +4410,14 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(TT_TemplateOpener))
 return true;
+  if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
+  (Left.is(tok::less) && Right.is(tok::less)))
+return false;
+  if (Right.is(TT_BinaryOperator) &&
+  Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None &&
+  (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_All ||
+   Right.getPrecedence() != prec::Assignment))
+return true;
   if (Left.isOneOf(TT_TemplateCloser, TT_UnaryOperator) ||
   Left.is(tok::kw_operator))
 return false;
@@ -4483,14 +4491,6 @@
   if (Right.is(TT_InheritanceComma) &&
   Style.BreakInheritanceList == FormatStyle::BILS_BeforeComma)
 return true;
-  if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
-  (Left.is(tok::less) && Right.is(tok::le

[clang] 1d03548 - [clang-format][NFC] Remove redundant semi

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-02-26T21:22:36+01:00
New Revision: 1d03548f63add672b4adf03360e396eba852bce5

URL: 
https://github.com/llvm/llvm-project/commit/1d03548f63add672b4adf03360e396eba852bce5
DIFF: 
https://github.com/llvm/llvm-project/commit/1d03548f63add672b4adf03360e396eba852bce5.diff

LOG: [clang-format][NFC] Remove redundant semi

All "calls" have a semi, as they should, remove the one from the macro.

Differential Revision: https://reviews.llvm.org/D120359

Added: 


Modified: 
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index e7bc26b5a9b54..60137b8a23c68 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -42,7 +42,7 @@ class TokenAnnotatorTest : public ::testing::Test {
   do { 
\
 EXPECT_TOKEN_KIND(FormatTok, Kind);
\
 EXPECT_TOKEN_TYPE(FormatTok, Type);
\
-  } while (false);
+  } while (false)
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
   // This is a regression test for mis-parsing the & after decltype as a binary



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

[PATCH] D120359: [clang-format][NFC] Remove redundant semi

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d03548f63ad: [clang-format][NFC] Remove redundant semi 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120359/new/

https://reviews.llvm.org/D120359

Files:
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -42,7 +42,7 @@
   do { 
\
 EXPECT_TOKEN_KIND(FormatTok, Kind);
\
 EXPECT_TOKEN_TYPE(FormatTok, Type);
\
-  } while (false);
+  } while (false)
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
   // This is a regression test for mis-parsing the & after decltype as a binary


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -42,7 +42,7 @@
   do { \
 EXPECT_TOKEN_KIND(FormatTok, Kind);\
 EXPECT_TOKEN_TYPE(FormatTok, Type);\
-  } while (false);
+  } while (false)
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
   // This is a regression test for mis-parsing the & after decltype as a binary
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a74ff3a - [clang-format][NFC] Rename test and remove comments

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-02-26T21:22:37+01:00
New Revision: a74ff3ac2edcb631ef78d41c12da7c2641ff2e15

URL: 
https://github.com/llvm/llvm-project/commit/a74ff3ac2edcb631ef78d41c12da7c2641ff2e15
DIFF: 
https://github.com/llvm/llvm-project/commit/a74ff3ac2edcb631ef78d41c12da7c2641ff2e15.diff

LOG: [clang-format][NFC] Rename test and remove comments

Why put "InMacros" in the name? We test other things to, and I will add
more, withut macros.

Also all our tests are regression tests.

Differential Revision: https://reviews.llvm.org/D120401

Added: 


Modified: 
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 60137b8a23c6..4c59bef4004d 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -44,9 +44,7 @@ class TokenAnnotatorTest : public ::testing::Test {
 EXPECT_TOKEN_TYPE(FormatTok, Type);
\
   } while (false)
 
-TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
-  // This is a regression test for mis-parsing the & after decltype as a binary
-  // operator instead of a reference (when inside a macro definition).
+TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   auto Tokens = annotate("auto x = [](const decltype(x) &ptr) {};");
   EXPECT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_decltype, TT_Unknown);
@@ -54,13 +52,12 @@ TEST_F(TokenAnnotatorTest, 
UnderstandsUsesOfStarAndAmpInMacroDefinition) {
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
-  // Same again with * instead of &:
+
   Tokens = annotate("auto x = [](const decltype(x) *ptr) {};");
   EXPECT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
 
-  // Also check that we parse correctly within a macro definition:
   Tokens = annotate("#define lambda [](const decltype(x) &ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_decltype, TT_Unknown);
@@ -68,7 +65,7 @@ TEST_F(TokenAnnotatorTest, 
UnderstandsUsesOfStarAndAmpInMacroDefinition) {
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
-  // Same again with * instead of &:
+
   Tokens = annotate("#define lambda [](const decltype(x) *ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);



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

[PATCH] D120401: [clang-format][NFC] Rename test and remove comments

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa74ff3ac2edc: [clang-format][NFC] Rename test and remove 
comments (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120401/new/

https://reviews.llvm.org/D120401

Files:
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -44,9 +44,7 @@
 EXPECT_TOKEN_TYPE(FormatTok, Type);
\
   } while (false)
 
-TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
-  // This is a regression test for mis-parsing the & after decltype as a binary
-  // operator instead of a reference (when inside a macro definition).
+TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   auto Tokens = annotate("auto x = [](const decltype(x) &ptr) {};");
   EXPECT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_decltype, TT_Unknown);
@@ -54,13 +52,12 @@
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
-  // Same again with * instead of &:
+
   Tokens = annotate("auto x = [](const decltype(x) *ptr) {};");
   EXPECT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
 
-  // Also check that we parse correctly within a macro definition:
   Tokens = annotate("#define lambda [](const decltype(x) &ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_decltype, TT_Unknown);
@@ -68,7 +65,7 @@
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
-  // Same again with * instead of &:
+
   Tokens = annotate("#define lambda [](const decltype(x) *ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -44,9 +44,7 @@
 EXPECT_TOKEN_TYPE(FormatTok, Type);\
   } while (false)
 
-TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
-  // This is a regression test for mis-parsing the & after decltype as a binary
-  // operator instead of a reference (when inside a macro definition).
+TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   auto Tokens = annotate("auto x = [](const decltype(x) &ptr) {};");
   EXPECT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_decltype, TT_Unknown);
@@ -54,13 +52,12 @@
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
-  // Same again with * instead of &:
+
   Tokens = annotate("auto x = [](const decltype(x) *ptr) {};");
   EXPECT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
 
-  // Also check that we parse correctly within a macro definition:
   Tokens = annotate("#define lambda [](const decltype(x) &ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_decltype, TT_Unknown);
@@ -68,7 +65,7 @@
   EXPECT_TOKEN(Tokens[9], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
-  // Same again with * instead of &:
+
   Tokens = annotate("#define lambda [](const decltype(x) *ptr) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120416: [clangd] Function return type hints: support lambdas, don't duplicate "->"

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:626
+auto f4() -> auto* { return "foo"; }
+
 // `auto` conversion operator

Trass3r wrote:
> Should there be a test for `void` too or does it just work?
Added a test verifying that an auto-typed function/lambda with no return 
statements gets a `-> void` hint.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120416/new/

https://reviews.llvm.org/D120416

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

[PATCH] D120416: [clangd] Function return type hints: support lambdas, don't duplicate "->"

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG257559ed9ab7: [clangd] Function return type hints: support 
lambdas, don't duplicate "->" (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120416/new/

https://reviews.llvm.org/D120416

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -527,13 +527,18 @@
   assertTypeHints(R"cpp(
 void f() {
   int cap = 42;
-  auto $L[[L]] = [cap, $init[[init]] = 1 + 1](int a) { 
+  auto $L[[L]] = [cap, $init[[init]] = 1 + 1](int a$ret[[)]] { 
 return a + cap + init; 
   };
 }
   )cpp",
   ExpectedHint{": (lambda)", "L"},
-  ExpectedHint{": int", "init"});
+  ExpectedHint{": int", "init"}, ExpectedHint{"-> int", 
"ret"});
+
+  // Lambda return hint shown even if no param list.
+  assertTypeHints("auto $L[[x]] = <:$ret[[:>]]{return 42;};",
+  ExpectedHint{": (lambda)", "L"},
+  ExpectedHint{"-> int", "ret"});
 }
 
 // Structured bindings tests.
@@ -616,6 +621,9 @@
 // Do not hint `auto` for trailing return type.
 auto f3() -> int;
 
+// Do not hint when a trailing return type is specified.
+auto f4() -> auto* { return "foo"; }
+
 // `auto` conversion operator
 struct A {
   operator auto($retConv[[)]] { return 42; }
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -254,17 +254,30 @@
   }
 
   bool VisitFunctionDecl(FunctionDecl *D) {
-if (auto *AT = D->getReturnType()->getContainedAutoType()) {
-  QualType Deduced = AT->getDeducedType();
-  if (!Deduced.isNull()) {
-addTypeHint(D->getFunctionTypeLoc().getRParenLoc(), D->getReturnType(),
-/*Prefix=*/"-> ");
-  }
+if (auto *FPT =
+llvm::dyn_cast(D->getType().getTypePtr())) {
+  if (!FPT->hasTrailingReturn())
+addReturnTypeHint(D, D->getFunctionTypeLoc().getRParenLoc());
 }
+return true;
+  }
 
+  bool VisitLambdaExpr(LambdaExpr *E) {
+FunctionDecl *D = E->getCallOperator();
+if (!E->hasExplicitResultType())
+  addReturnTypeHint(D, E->hasExplicitParameters()
+   ? D->getFunctionTypeLoc().getRParenLoc()
+   : E->getIntroducerRange().getEnd());
 return true;
   }
 
+  void addReturnTypeHint(FunctionDecl *D, SourceLocation Loc) {
+auto *AT = D->getReturnType()->getContainedAutoType();
+if (!AT || AT->getDeducedType().isNull())
+  return;
+addTypeHint(Loc, D->getReturnType(), /*Prefix=*/"-> ");
+  }
+
   bool VisitVarDecl(VarDecl *D) {
 // Do not show hints for the aggregate in a structured binding,
 // but show hints for the individual bindings.


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -527,13 +527,18 @@
   assertTypeHints(R"cpp(
 void f() {
   int cap = 42;
-  auto $L[[L]] = [cap, $init[[init]] = 1 + 1](int a) { 
+  auto $L[[L]] = [cap, $init[[init]] = 1 + 1](int a$ret[[)]] { 
 return a + cap + init; 
   };
 }
   )cpp",
   ExpectedHint{": (lambda)", "L"},
-  ExpectedHint{": int", "init"});
+  ExpectedHint{": int", "init"}, ExpectedHint{"-> int", "ret"});
+
+  // Lambda return hint shown even if no param list.
+  assertTypeHints("auto $L[[x]] = <:$ret[[:>]]{return 42;};",
+  ExpectedHint{": (lambda)", "L"},
+  ExpectedHint{"-> int", "ret"});
 }
 
 // Structured bindings tests.
@@ -616,6 +621,9 @@
 // Do not hint `auto` for trailing return type.
 auto f3() -> int;
 
+// Do not hint when a trailing return type is specified.
+auto f4() -> auto* { return "foo"; }
+
 // `auto` conversion operator
 struct A {
   operator auto($retConv[[)]] { return 42; }
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -254,17 +254,30 @@
   }
 
   bool VisitFunctionDecl(FunctionDecl *D) {
-if (auto *AT = D->getReturnType()->getContainedAutoType()) {
-  QualType Deduced = AT->getDeducedType();
-  if (!Deduced.isNull()) {
-  

[clang-tools-extra] 257559e - [clangd] Function return type hints: support lambdas, don't duplicate "->"

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-02-26T21:28:09+01:00
New Revision: 257559ed9ab74c2dd3882075c45b4ae002256425

URL: 
https://github.com/llvm/llvm-project/commit/257559ed9ab74c2dd3882075c45b4ae002256425
DIFF: 
https://github.com/llvm/llvm-project/commit/257559ed9ab74c2dd3882075c45b4ae002256425.diff

LOG: [clangd] Function return type hints: support lambdas, don't duplicate "->"

While here, fix an ugliness:
  auto foo()->auto { return 42; }
This (silly) code gains a "-> int" hint. While correct and useful, it renders as
  auto foo()->int->auto { return 42; }
which is confusing enough to do more harm than good I think.

Differential Revision: https://reviews.llvm.org/D120416

Added: 


Modified: 
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 671f9a151d40f..2f9624f68af3e 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -254,17 +254,30 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
   }
 
   bool VisitFunctionDecl(FunctionDecl *D) {
-if (auto *AT = D->getReturnType()->getContainedAutoType()) {
-  QualType Deduced = AT->getDeducedType();
-  if (!Deduced.isNull()) {
-addTypeHint(D->getFunctionTypeLoc().getRParenLoc(), D->getReturnType(),
-/*Prefix=*/"-> ");
-  }
+if (auto *FPT =
+llvm::dyn_cast(D->getType().getTypePtr())) {
+  if (!FPT->hasTrailingReturn())
+addReturnTypeHint(D, D->getFunctionTypeLoc().getRParenLoc());
 }
+return true;
+  }
 
+  bool VisitLambdaExpr(LambdaExpr *E) {
+FunctionDecl *D = E->getCallOperator();
+if (!E->hasExplicitResultType())
+  addReturnTypeHint(D, E->hasExplicitParameters()
+   ? D->getFunctionTypeLoc().getRParenLoc()
+   : E->getIntroducerRange().getEnd());
 return true;
   }
 
+  void addReturnTypeHint(FunctionDecl *D, SourceLocation Loc) {
+auto *AT = D->getReturnType()->getContainedAutoType();
+if (!AT || AT->getDeducedType().isNull())
+  return;
+addTypeHint(Loc, D->getReturnType(), /*Prefix=*/"-> ");
+  }
+
   bool VisitVarDecl(VarDecl *D) {
 // Do not show hints for the aggregate in a structured binding,
 // but show hints for the individual bindings.

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 6c3ac0d62e0e5..d93a6e68a6ee8 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -527,13 +527,18 @@ TEST(TypeHints, Lambda) {
   assertTypeHints(R"cpp(
 void f() {
   int cap = 42;
-  auto $L[[L]] = [cap, $init[[init]] = 1 + 1](int a) { 
+  auto $L[[L]] = [cap, $init[[init]] = 1 + 1](int a$ret[[)]] { 
 return a + cap + init; 
   };
 }
   )cpp",
   ExpectedHint{": (lambda)", "L"},
-  ExpectedHint{": int", "init"});
+  ExpectedHint{": int", "init"}, ExpectedHint{"-> int", 
"ret"});
+
+  // Lambda return hint shown even if no param list.
+  assertTypeHints("auto $L[[x]] = <:$ret[[:>]]{return 42;};",
+  ExpectedHint{": (lambda)", "L"},
+  ExpectedHint{"-> int", "ret"});
 }
 
 // Structured bindings tests.
@@ -616,6 +621,9 @@ TEST(TypeHints, ReturnTypeDeduction) {
 // Do not hint `auto` for trailing return type.
 auto f3() -> int;
 
+// Do not hint when a trailing return type is specified.
+auto f4() -> auto* { return "foo"; }
+
 // `auto` conversion operator
 struct A {
   operator auto($retConv[[)]] { return 42; }



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

[clang-tools-extra] 42cb812 - [clangd] Test fixes missing from 257559ed9

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-02-26T21:38:25+01:00
New Revision: 42cb812da707e7092aa3c030a44b739bbaf36b98

URL: 
https://github.com/llvm/llvm-project/commit/42cb812da707e7092aa3c030a44b739bbaf36b98
DIFF: 
https://github.com/llvm/llvm-project/commit/42cb812da707e7092aa3c030a44b739bbaf36b98.diff

LOG: [clangd] Test fixes missing from 257559ed9

Added: 


Modified: 
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index d93a6e68a6ee8..1054090ae716b 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -536,6 +536,7 @@ TEST(TypeHints, Lambda) {
   ExpectedHint{": int", "init"}, ExpectedHint{"-> int", 
"ret"});
 
   // Lambda return hint shown even if no param list.
+  // (The digraph :> is just a ] that doesn't conflict with the annotations).
   assertTypeHints("auto $L[[x]] = <:$ret[[:>]]{return 42;};",
   ExpectedHint{": (lambda)", "L"},
   ExpectedHint{"-> int", "ret"});
@@ -624,6 +625,8 @@ TEST(TypeHints, ReturnTypeDeduction) {
 // Do not hint when a trailing return type is specified.
 auto f4() -> auto* { return "foo"; }
 
+auto f5($noreturn[[)]] {}
+
 // `auto` conversion operator
 struct A {
   operator auto($retConv[[)]] { return 42; }
@@ -636,7 +639,8 @@ TEST(TypeHints, ReturnTypeDeduction) {
 };
   )cpp",
   ExpectedHint{"-> int", "ret1a"}, ExpectedHint{"-> int", "ret1b"},
-  ExpectedHint{"-> int &", "ret2"}, ExpectedHint{"-> int", "retConv"});
+  ExpectedHint{"-> int &", "ret2"}, ExpectedHint{"-> void", "noreturn"},
+  ExpectedHint{"-> int", "retConv"});
 }
 
 TEST(TypeHints, DependentType) {



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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D120305#3344938 , @nikic wrote:

> Hm, it looks like enabling PIE has a pretty big negative compile-time impact, 
> with a 20% regression on sqlite3: 
> http://llvm-compile-time-tracker.com/compare.php?from=611122892e6d5813444bdd0e1fbe0a96f6e09779&to=3c4ed02698afec021c6bca80740d1e58e3ee019e&stat=instructions
>  Code-size on kimwitu++ regresses by 13%.
>
> Is that kind of impact expected?

Many groups build sqlite3 with -fPIC. -fPIC and -fPIE have similar compile time.
The metric on llvm-compile-time-tracker.com is related to 
`llvm-test-suite/CTMark/sqlite3`, which just focuses on (without the CMake 
patch) `-fno-pic` performance.

(
I checked run time, no big difference.

  % hyperfine --warmup 2 --min-runs 16 "/tmp/c/sqlite3.nopie -init sqlite3rc 
:memory: < commands" 
  Benchmark 1: /tmp/c/sqlite3.nopie -init sqlite3rc :memory: < commands
Time (mean ± σ):  2.068 s ±  0.011 s[User: 2.044 s, System: 0.024 s]
Range (min … max):2.044 s …  2.085 s16 runs
  
  % hyperfine --warmup 2 --min-runs 16 "/tmp/c/sqlite3.pie -init sqlite3rc 
:memory: < commands"  
  Benchmark 1: /tmp/c/sqlite3.pie -init sqlite3rc :memory: < commands
Time (mean ± σ):  2.053 s ±  0.015 s[User: 2.034 s, System: 0.018 s]
Range (min … max):2.027 s …  2.080 s16 runs

)

About compile time,

I run `=time -f "CPU: %Us\tReal: %es\tRAM: %MKB" /tmp/out/custom1/bin/clang 
-DNDEBUG -O3 -DNDEBUG -w -Werror=date-time -DSTDC_HEADERS=1 
-DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 
-DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 
-DHAVE_UNISTD_H=1 -DSQLITE_OMIT_LOAD_EXTENSION=1 -DSQLITE_THREADSAFE=0 -I. -o 
CTMark/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o -c 
~/Dev/llvm-test-suite/CTMark/sqlite3/sqlite3.c -ftime-report` with 
-fno-pie/-fpie/-fpic to get some insight.

The IR has minor difference but pie's is slightly larger:

  % wc -l nopie.ll pie.ll
157975 nopie.ll
159675 pie.ll
  ...

If someone wants to investigate, `sqlite3Parser` changes a lot from -fno-pic to 
-fPIE, but that looks organic changes to me.
-ftime-report comparison suggests that every pass is slightly slower with 
-fPIE. ModuleInlinerWrapperPass and DevirtSCCRepeatedPass contribute most of 
the slowness.

  -fno-pic -ftime-report
 ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- 
Name ---
 5.1753 ( 28.1%)   0.2359 ( 27.5%)   5.4112 ( 28.0%)   5.4123 ( 28.1%)  
ModuleInlinerWrapperPass
 5.1447 ( 27.9%)   0.2309 ( 26.9%)   5.3755 ( 27.8%)   5.3767 ( 27.9%)  
DevirtSCCRepeatedPass
 1.8716 ( 10.1%)   0.0936 ( 10.9%)   1.9652 ( 10.2%)   1.9613 ( 10.2%)  
InstCombinePass
 0.7674 (  4.2%)   0.0254 (  3.0%)   0.7928 (  4.1%)   0.7921 (  4.1%)  
GVNPass
 0.4563 (  2.5%)   0.0288 (  3.4%)   0.4851 (  2.5%)   0.4844 (  2.5%)  
InlinerPass
 0.4194 (  2.3%)   0.0174 (  2.0%)   0.4368 (  2.3%)   0.4359 (  2.3%)  
MemorySSAAnalysis
 0.3399 (  1.8%)   0.0191 (  2.2%)   0.3589 (  1.9%)   0.3578 (  1.9%)  
SimplifyCFGPass
 0.3206 (  1.7%)   0.0143 (  1.7%)   0.3349 (  1.7%)   0.3354 (  1.7%)  
BlockFrequencyAnalysis
 0.2985 (  1.6%)   0.0081 (  0.9%)   0.3065 (  1.6%)   0.3061 (  1.6%)  
CorrelatedValuePropagationPass
 0.2608 (  1.4%)   0.0158 (  1.8%)   0.2767 (  1.4%)   0.2763 (  1.4%)  
EarlyCSEPass



  -fpie -ftime-report
 ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- 
Name ---
 7.8109 ( 29.2%)   0.2239 ( 26.8%)   8.0347 ( 29.1%)   8.0363 ( 29.2%)  
ModuleInlinerWrapperPass
 7.7752 ( 29.1%)   0.2230 ( 26.6%)   7.9981 ( 29.0%)   7.9997 ( 29.0%)  
DevirtSCCRepeatedPass
 2.5352 (  9.5%)   0.0807 (  9.6%)   2.6158 (  9.5%)   2.6114 (  9.5%)  
InstCombinePass
 1.2792 (  4.8%)   0.0162 (  1.9%)   1.2954 (  4.7%)   1.2948 (  4.7%)  
GVNPass
 0.6529 (  2.4%)   0.0157 (  1.9%)   0.6686 (  2.4%)   0.6674 (  2.4%)  
MemorySSAAnalysis
 0.6048 (  2.3%)   0.0261 (  3.1%)   0.6309 (  2.3%)   0.6306 (  2.3%)  
InlinerPass
 0.4625 (  1.7%)   0.0126 (  1.5%)   0.4751 (  1.7%)   0.4743 (  1.7%)  
CorrelatedValuePropagationPass
 0.4541 (  1.7%)   0.0160 (  1.9%)   0.4701 (  1.7%)   0.4690 (  1.7%)  
SimplifyCFGPass
 0.3981 (  1.5%)   0.0124 (  1.5%)   0.4105 (  1.5%)   0.4101 (  1.5%)  
EarlyCSEPass



- -fno-pie: Time (mean ± σ): 11.999 s ±  0.043 s
- -fpie: Time (mean ± σ): 14.643 s ±  0.073 s
- -fno-pie -flegacy-pass-manager: Time (mean ± σ): 16.831 s ±  0.027 s
- -fpie -flegacy-pass-manager: Time (mean ± σ): 16.887 s ±  0.099 s


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

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

[PATCH] D120498: [AST] Test RecursiveASTVisitor's current traversal of templates.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 411640.
sammccall added a comment.

Move the description of shouldVisitTemplateInstantiations() to RAV.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120498/new/

https://reviews.llvm.org/D120498

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
  clang/unittests/Tooling/TestVisitor.h

Index: clang/unittests/Tooling/TestVisitor.h
===
--- clang/unittests/Tooling/TestVisitor.h
+++ clang/unittests/Tooling/TestVisitor.h
@@ -95,7 +95,7 @@
 
 void HandleTranslationUnit(clang::ASTContext &Context) override {
   Visitor->Context = &Context;
-  Visitor->TraverseDecl(Context.getTranslationUnitDecl());
+  Visitor->getDerived().TraverseAST(Context);
 }
 
   private:
Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
@@ -0,0 +1,409 @@
+//===- Templates.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This tests exactly which template-related declarations are traversed by
+// RecursiveASTVisitor in the default mode (written code only) and
+// shouldVisitTemplateInstantiations() mode.
+//
+// Because RecursiveASTVisitor should also be able to traverse arbitrary
+// subtrees, the nodes representing explicit instantiations should be partially
+// traversed when passed to TraverseDecl(). This is also tested.
+// (The historical behavior for functions with shouldVisitTI() off was:
+// explicit function instantiations were not visited at all, but if explicitly
+// passed to TraverseDecl() then the whole body was traversed).
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Specifiers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+
+namespace {
+
+// This RecursiveASTVisitor logs the traversal of template-related declarations,
+// as well as a few leaf nodes (so we can tell whether bodies are traversed).
+class TemplateVisitor : public TestVisitor {
+  using Base = TestVisitor;
+
+public:
+  bool Instantiations = false;
+  std::function ChooseTraversalRoot =
+  [](ASTContext &Ctx) { return Ctx.getTranslationUnitDecl(); };
+
+  bool shouldVisitTemplateInstantiations() { return Instantiations; }
+  bool shouldVisitImplicitCode() { return false; }
+
+  bool TraverseAST(ASTContext &Ctx) {
+return TraverseDecl(ChooseTraversalRoot(Ctx));
+  }
+
+#define LOG_VISIT(NodeTy)  \
+  bool Visit##NodeTy(NodeTy *N) {  \
+addLog("Visit" #NodeTy);   \
+addNodeDetail(N);  \
+return Base::Visit##NodeTy(N); \
+  }
+#define LOG_TRAVERSE(NodeTy)   \
+  LOG_VISIT(NodeTy)\
+  bool Traverse##NodeTy(NodeTy *N) {   \
+addLog("Traverse" #NodeTy);\
+++Indent;  \
+bool Ret = Base::Traverse##NodeTy(N);  \
+--Indent;  \
+return Ret;\
+  }
+  LOG_TRAVERSE(FunctionTemplateDecl)
+  LOG_TRAVERSE(ClassTemplateDecl)
+  LOG_TRAVERSE(VarTemplateDecl)
+  LOG_TRAVERSE(FunctionDecl)
+  LOG_TRAVERSE(CXXRecordDecl)
+  LOG_TRAVERSE(VarDecl)
+  LOG_TRAVERSE(ClassTemplateSpecializationDecl)
+  LOG_TRAVERSE(ClassTemplatePartialSpecializationDecl)
+  LOG_TRAVERSE(VarTemplateSpecializationDecl)
+  LOG_TRAVERSE(VarTemplatePartialSpecializationDecl)
+  LOG_VISIT(CompoundStmt)
+  LOG_VISIT(FieldDecl)
+  LOG_VISIT(BinaryOperator)
+#undef LOG_TRAVERSE
+#undef LOG_VISIT
+
+  std::string takeLog() {
+std::string Result = llvm::join(Log, "\n");
+Log.clear();
+return Result;
+  }
+
+private:
+  template  void addNodeDetail(T) {}
+  void addNodeDetail(FunctionDecl *D) {
+addDetail(D->getNameAsString());
+if (D->getDescribedFunctionTemplate())
+  addDetail("Generic");
+addDetail(D->getTempla

[PATCH] D120618: [clang-format][docs] Fix a bad comment

[Björn Schäpers via Phabricator via cfe-commits]

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

Will do with my next push batch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120618/new/

https://reviews.llvm.org/D120618

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

[clang] cbe9911 - [clang] MarkVarDeclODRUsed - remove redundant nullptr check. NFCI.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-26T21:24:26Z
New Revision: cbe9911845eed262b265d2c6b5f600825145330b

URL: 
https://github.com/llvm/llvm-project/commit/cbe9911845eed262b265d2c6b5f600825145330b
DIFF: 
https://github.com/llvm/llvm-project/commit/cbe9911845eed262b265d2c6b5f600825145330b.diff

LOG: [clang] MarkVarDeclODRUsed - remove redundant nullptr check. NFCI.

The function has already been dereferenced the Var pointer

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b24caa56a38ad..bd7110d0315d9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17500,7 +17500,7 @@ MarkVarDeclODRUsed(VarDecl *Var, SourceLocation Loc, 
Sema &SemaRef,
 CaptureType, DeclRefType,
 FunctionScopeIndexToStopAt);
 
-  if (SemaRef.LangOpts.CUDA && Var && Var->hasGlobalStorage()) {
+  if (SemaRef.LangOpts.CUDA && Var->hasGlobalStorage()) {
 auto *FD = dyn_cast_or_null(SemaRef.CurContext);
 auto VarTarget = SemaRef.IdentifyCUDATarget(Var);
 auto UserTarget = SemaRef.IdentifyCUDATarget(FD);



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

[PATCH] D120621: [clang-format] Fix BreakBeforeBinaryOperators with TemplateCloser on the lhs

[Marek Kurdej via Phabricator via cfe-commits]

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

Great! Please fix formatting before landing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120621/new/

https://reviews.llvm.org/D120621

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

[PATCH] D120511: [clang-format] Allow to set token types final

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks updated this revision to Diff 411643.
HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.

- Format
- Add Assert
- And some stuff to deal with the assert - Should be removed in the long run
- Add some documentation


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120511/new/

https://reviews.llvm.org/D120511

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -70,6 +70,14 @@
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_TypeDeclarationParen);
   EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void f() {\n"
+"  while (p < a && *p == 'a')\n"
+"p++;\n"
+"}");
+  EXPECT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -500,7 +500,7 @@
   break;
 case tok::l_brace:
   if (NextLBracesType != TT_Unknown)
-FormatTok->setType(NextLBracesType);
+FormatTok->setFinalizedType(NextLBracesType);
   else if (FormatTok->Previous &&
FormatTok->Previous->ClosesRequiresClause) {
 // We need the 'default' case here to correctly parse a function
@@ -1240,7 +1240,7 @@
   nextToken();
   while (!eof()) {
 if (FormatTok->is(tok::colon)) {
-  FormatTok->setType(TT_ModulePartitionColon);
+  FormatTok->setFinalizedType(TT_ModulePartitionColon);
 }
 // Handle import  as we would an include statement.
 else if (FormatTok->is(tok::less)) {
@@ -1250,7 +1250,7 @@
 // literals.
 if (FormatTok->isNot(tok::comment) &&
 !FormatTok->TokenText.startswith("//"))
-  FormatTok->setType(TT_ImplicitStringLiteral);
+  FormatTok->setFinalizedType(TT_ImplicitStringLiteral);
 nextToken();
   }
 }
@@ -1325,11 +1325,11 @@
   case tok::kw_asm:
 nextToken();
 if (FormatTok->is(tok::l_brace)) {
-  FormatTok->setType(TT_InlineASMBrace);
+  FormatTok->setFinalizedType(TT_InlineASMBrace);
   nextToken();
   while (FormatTok && FormatTok->isNot(tok::eof)) {
 if (FormatTok->is(tok::r_brace)) {
-  FormatTok->setType(TT_InlineASMBrace);
+  FormatTok->setFinalizedType(TT_InlineASMBrace);
   nextToken();
   addUnwrappedLine();
   break;
@@ -1651,7 +1651,7 @@
   break;
 case tok::l_brace:
   if (NextLBracesType != TT_Unknown)
-FormatTok->setType(NextLBracesType);
+FormatTok->setFinalizedType(NextLBracesType);
   if (!tryToParsePropertyAccessor() && !tryToParseBracedList()) {
 // A block outside of parentheses must be the last part of a
 // structural element.
@@ -1668,7 +1668,7 @@
   addUnwrappedLine();
 }
 if (!Line->InPPDirective)
-  FormatTok->setType(TT_FunctionLBrace);
+  FormatTok->setFinalizedType(TT_FunctionLBrace);
 parseBlock();
 addUnwrappedLine();
 return;
@@ -1773,7 +1773,7 @@
 
 if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
 tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
-  PreviousToken->setType(TT_FunctionLikeOrFreestandingMacro);
+  PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
   addUnwrappedLine();
   return;
 }
@@ -1997,7 +1997,7 @@
   // This might or might not actually be a lambda arrow (this could be an
   // ObjC method invocation followed by a dereferencing arrow). We might
   // reset this back to TT_Unknown in TokenAnnotator.
-  FormatTok->setType(TT_LambdaArrow);
+  FormatTok->setFinalizedType(TT_LambdaArrow);
   SeenArrow = true;
   nextToken();
   break;
@@ -2005,8 +2005,8 @@
   return true;
 }
   }
-  FormatTok->setType(TT_LambdaLBrace);
-  LSquare.setType(TT_LambdaLSquare);
+  FormatTok->setFinalizedType(TT_LambdaLBrace);
+  LSquare.setFinalizedType(TT_LambdaLSquare);
   parseChildBlock();
   return true;
 }
@@ -2038,7 +2038,7 @@
 
   // Consume * (generator function). Treat it like C++'s overloaded operators.
   if (FormatTok->is(tok::star)) {
-FormatTok->setType(TT_OverloadedOperator);
+FormatTok->setFinalizedType(TT_OverloadedOperator);
 nextToken();
   }

[PATCH] D113694: [Clang] Conform to C++17 definition of literal types

[Léo Lam via Phabricator via cfe-commits]

leoetlino added a comment.

ping (as per the contributing guidelines)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113694/new/

https://reviews.llvm.org/D113694

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

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 411652.
phyBrackets added a comment.

fix some alignment issue


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120489/new/

https://reviews.llvm.org/D120489

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/bstring.c

Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -70,6 +70,11 @@
 
 #endif /* VARIANT */
 
+void top(char *dst) {
+  char buf[10];
+  memcpy(dst, buf, 10); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  (void)buf;
+}
 
 void memcpy0 () {
   char src[] = {1, 2, 3, 4};
@@ -297,9 +302,12 @@
   int dst[5] = {0};
   int *p;
 
-  p = mempcpy(dst, src, 4 * sizeof(int));
+  p = mempcpy(dst, src, 4 * sizeof(int)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  // FIXME: This behaviour is actually Unexpected and needs to be fix, 
+  // mempcpy seems to consider the src buffered byte as uninitialized
+  // and returning undef which is actually not the case It should return something like Unknown .
 
-  clang_analyzer_eval(p == &dst[4]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(p == &dst[4]); // no-warning (above is fatal)
 }
 
 struct st {
@@ -314,9 +322,10 @@
   struct st *p2;
 
   p1 = (&s2) + 1;
-  p2 = mempcpy(&s2, &s1, sizeof(struct st));
-
-  clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}}
+  p2 = mempcpy(&s2, &s1, sizeof(struct st)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  // FIXME: It seems same as mempcpy14() case.
+  
+  clang_analyzer_eval(p1 == p2); // no-warning (above is fatal)
 }
 
 void mempcpy16() {
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -80,7 +80,7 @@
  check::RegionChanges
  > {
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
-  BT_NotCString, BT_AdditionOverflow;
+  BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
   mutable const char *CurrentFunctionDescription;
 
@@ -92,11 +92,13 @@
 DefaultBool CheckCStringOutOfBounds;
 DefaultBool CheckCStringBufferOverlap;
 DefaultBool CheckCStringNotNullTerm;
+DefaultBool CheckCStringUninitializedRead;
 
 CheckerNameRef CheckNameCStringNullArg;
 CheckerNameRef CheckNameCStringOutOfBounds;
 CheckerNameRef CheckNameCStringBufferOverlap;
 CheckerNameRef CheckNameCStringNotNullTerm;
+CheckerNameRef CheckNameCStringUninitializedRead;
   };
 
   CStringChecksFilter Filter;
@@ -257,6 +259,8 @@
   void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
  const Stmt *S, StringRef WarningMsg) const;
   void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
+  void emitUninitializedReadBug(CheckerContext &C, ProgramStateRef State,
+const Expr *E) const;
 
   ProgramStateRef checkAdditionOverflow(CheckerContext &C,
 ProgramStateRef state,
@@ -368,6 +372,14 @@
 return nullptr;
   }
 
+  // Ensure that we wouldn't read uninitialized value.
+  if (Access == AccessKind::read) {
+if (StInBound->getSVal(ER).isUndef()) {
+  emitUninitializedReadBug(C, StInBound, Buffer.Expression);
+  return nullptr;
+}
+  }
+
   // Array bound check succeeded.  From this point forward the array bound
   // should always succeed.
   return StInBound;
@@ -421,6 +433,7 @@
 SVal BufEnd =
 svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
 
+State = CheckLocation(C, State, Buffer, BufStart, Access);
 State = CheckLocation(C, State, Buffer, BufEnd, Access);
 
 // If the buffer isn't large enough, abort.
@@ -580,6 +593,26 @@
   }
 }
 
+void CStringChecker::emitUninitializedReadBug(CheckerContext &C,
+  ProgramStateRef State,
+  const Expr *E) const {
+  if (ExplodedNode *N = C.generateErrorNode(State)) {
+const char *Msg =
+"Bytes string function accesses uninitialized/garbage values";
+if (!BT_UninitRead)
+  BT_UninitRead.reset(
+  new BuiltinBug(Filter.CheckNameCStringUninitializedRead,
+ "Accessing unitialized/garbage values", Msg));
+
+BuiltinBug *BT = static_cast(BT_UninitRead.get());
+
+auto Report = std::make_unique(*BT, Msg, N);
+Report->addRange(E->getSourceRange());
+bugreporter::trackExpressionValue(N, E, *Report

[PATCH] D120512: [clang-format] Fix requires related crash

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks updated this revision to Diff 411658.
HazardyKnusperkeks added a comment.

Changed test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120512/new/

https://reviews.llvm.org/D120512

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24051,6 +24051,13 @@
"return number_zero_v;\n"
"  }\n"
"};");
+
+  // Not a clause, but we once hit an assert.
+  verifyFormat("#if 0\n"
+   "#else\n"
+   "foo();\n"
+   "#endif\n"
+   "bar(requires);");
 }
 
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2994,7 +2994,6 @@
 void UnwrappedLineParser::parseRequiresClause(FormatToken *RequiresToken) {
   assert(FormatTok->getPreviousNonComment() == RequiresToken);
   assert(RequiresToken->is(tok::kw_requires) && "'requires' expected");
-  assert(RequiresToken->getType() == TT_Unknown);
 
   // If there is no previous token, we are within a requires expression,
   // otherwise we will always have the template or function declaration in 
front
@@ -3023,7 +3022,6 @@
 void UnwrappedLineParser::parseRequiresExpression(FormatToken *RequiresToken) {
   assert(FormatTok->getPreviousNonComment() == RequiresToken);
   assert(RequiresToken->is(tok::kw_requires) && "'requires' expected");
-  assert(RequiresToken->getType() == TT_Unknown);
 
   RequiresToken->setFinalizedType(TT_RequiresExpression);
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24051,6 +24051,13 @@
"return number_zero_v;\n"
"  }\n"
"};");
+
+  // Not a clause, but we once hit an assert.
+  verifyFormat("#if 0\n"
+   "#else\n"
+   "foo();\n"
+   "#endif\n"
+   "bar(requires);");
 }
 
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2994,7 +2994,6 @@
 void UnwrappedLineParser::parseRequiresClause(FormatToken *RequiresToken) {
   assert(FormatTok->getPreviousNonComment() == RequiresToken);
   assert(RequiresToken->is(tok::kw_requires) && "'requires' expected");
-  assert(RequiresToken->getType() == TT_Unknown);
 
   // If there is no previous token, we are within a requires expression,
   // otherwise we will always have the template or function declaration in front
@@ -3023,7 +3022,6 @@
 void UnwrappedLineParser::parseRequiresExpression(FormatToken *RequiresToken) {
   assert(FormatTok->getPreviousNonComment() == RequiresToken);
   assert(RequiresToken->is(tok::kw_requires) && "'requires' expected");
-  assert(RequiresToken->getType() == TT_Unknown);
 
   RequiresToken->setFinalizedType(TT_RequiresExpression);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120511: [clang-format] Allow to set token types final

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/FormatToken.h:394
+  }
+  bool typeIsFinalized() const { return TypeIsFinalized; }
 

Nit, to make everyone happy, the function could be called `isTypeFinalized`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120511/new/

https://reviews.llvm.org/D120511

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

[PATCH] D120511: [clang-format] Allow to set token types final

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/FormatToken.h:394
+  }
+  bool typeIsFinalized() const { return TypeIsFinalized; }
 

curdeius wrote:
> Nit, to make everyone happy, the function could be called `isTypeFinalized`.
+1.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120511/new/

https://reviews.llvm.org/D120511

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