[PATCH] D138403: [LoongArch] Fix issue on CMake Xcode build configuration

[Gong LingQin via Phabricator via cfe-commits]

gonglingqin created this revision.
gonglingqin added reviewers: xen0n, xry111, SixWeining, wangleiat, MaskRay, 
XiaodongLoong.
Herald added a subscriber: StephenFan.
Herald added a project: All.
gonglingqin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add missing dependency for loongarch-resource-headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138403

Files:
  clang/lib/Headers/CMakeLists.txt


Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -394,6 +394,7 @@
  "hexagon-resource-headers"
  "hip-resource-headers"
  "hlsl-resource-headers"
+ "loongarch-resource-headers"
  "mips-resource-headers"
  "ppc-resource-headers"
  "ppc-htm-resource-headers"


Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -394,6 +394,7 @@
  "hexagon-resource-headers"
  "hip-resource-headers"
  "hlsl-resource-headers"
+ "loongarch-resource-headers"
  "mips-resource-headers"
  "ppc-resource-headers"
  "ppc-htm-resource-headers"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136906: [Clang][LoongArch] Implement __builtin_loongarch_dbar builtin

[Gong LingQin via Phabricator via cfe-commits]

gonglingqin added a comment.

In D136906#3940289 , 
@WowbaggersLiquidLunch wrote:

> Hi, this seems to be causing some problems when generating Xcode project.
>
> When I try to generate the Xcode project with the following command:
>
>   cmake -G "Xcode" -DCMAKE_BUILD_TYPE=Debug -DLLVM_TARGETS_TO_BUILD="X86" 
> -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_OPTIMIZED_TABLEGEN=ON 
> -DLLVM_ENABLE_IDE=ON -DLLVM_CCACHE_BUILD=ON ../../llvm
>
> I get the following error:
>
>> CMake Error in /path/to/llvm-project/clang/lib/Headers/CMakeLists.txt:
>>
>>   The custom command generating
>>   
>> 
>> /path/to/llvm-project/build/Xcode-Debug/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)/lib/clang/16/include/larchintrin.h
>>   
>>   is attached to multiple targets:
>>   
>> clang-resource-headers
>> loongarch-resource-headers
>>   
>>   but none of these is a common dependency of the other(s).  This is not
>>   allowed by the Xcode "new build system".
>>
>> CMake Error in /path/to/llvm-project/third-party/benchmark/CMakeLists.txt:
>>
>>   The custom command generating
>>   
>> 
>> /path/to/llvm-project/build/Xcode-Debug/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)/lib/clang/16/include/larchintrin.h
>>   
>>   is attached to multiple targets:
>>   
>> clang-resource-headers
>> loongarch-resource-headers
>>   
>>   but none of these is a common dependency of the other(s).  This is not
>>   allowed by the Xcode "new build system".

I realized that it might be my environment that prevented me from repeating 
this problem. Can you use https://reviews.llvm.org/D138403 to solve this 
problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136906

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

[PATCH] D138403: [LoongArch] Fix issue on CMake Xcode build configuration

[Lu Weining via Phabricator via cfe-commits]

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

LGTM but wait for @WowbaggersLiquidLunch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138403

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

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

In D136594#3940143 , @nridge wrote:

> A couple of high-level thoughts on this:
>
> 1. Based on the discussion in https://github.com/clangd/clangd/issues/1115, I 
> believe highlighting of **built-in** operators should be out of scope for 
> semantic highlighting, at least in the default mode; client-side highlighting 
> should be sufficient for these, similar to strings and literals.
> 2. An alternative to assigning (user-provided) operators a new token kind 
> would be to assign them the same token kind as the entity they invoke (i.e. 
> function or method). Both approaches have their advantages:
>   - If we use the function/method kinds, then uses of user-provided operators 
> will be highlighted differently from built-in operators even when using a 
> default / standard theme that doesn't know about clangd-specific token types.
>   - If we use a dedicated operator kind, users can configure different styles 
> for operators vs. function/methods (and they may want different styles given 
> that syntactically the two look quite different).
>
> One way to get the best of both worlds could be to use the 
> function/method kinds in combination with an `operator` **modifier**. That 
> would color overloaded operators out of the box while also allowing users to 
> customize the style based on the presence of the modifier. What do you think 
> about this approach?

The problem with "operator" as a modifier is that if and when the 
augmentsSyntaxTokens feature is implemented, we'll need an operator token kind 
anyway (and having both seems weird).
The modifier semantics could also be switched around, btw, so that instead of 
"userProvided" we could have "builtIn" (which might also apply to other token 
kinds). I don't have a strong opinion on this.
Should I perhaps add the augmentsSyntaxTokens option and rebase this patch to 
make it its first user?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136594

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

[PATCH] D136906: [Clang][LoongArch] Implement __builtin_loongarch_dbar builtin

[冀卓疌 via Phabricator via cfe-commits]

WowbaggersLiquidLunch added a comment.

> I get the following error:
>
>> CMake Error: Could not create named generator Xcode

Sorry, I forgot to mention that Xcode is only available on macOS.

> But when I change Xcode to Ninja, no error output is generated.

Indeed. Ninja works fine for me as well. The problem is I kind of need both 
Xcode and Ninja in my workflow (Xcode for editing, Ninja for building).

> Do you have any suggestions for reproducing the bugs you mentioned?

I think the following steps should be able to reproduce the error messages I'm 
seeing (It's a bit long):

1. Install macOS 12.6 or newer. (I'm currently on 12.6, but newer versions such 
as 12.6.1 and 13 (the latest) should produce the same error messages.)

2. Install Xcode 14.1 and/or the Xcode command line tools
  - Xcode can be installed via this link 

 (generally recommended), or from the Mac App Store via this link 
.
- The first link downloads a `.xip` file which can take a long time to 
decompress. This step can be sped up significantly by decompressing the file 
using this tool .
  - The command line tools can be installed via this link 
,
 via the following commands, or by launching Xcode after it's installed. (I 
don't remember it clearly, but I think you probably can skip this step and have 
Homebrew install it for you in step 3)

`sudo rm -rf /Library/Developer/CommandLineTools`

`sudo xcode-select --install`

3. Install LLVM's dependencies 
 via Homebrew
  - First we need to install brew  using the following command

`/bin/bash -c "$(curl -fsSL 
https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`
  - Then install Cmake and Python with `brew`. (I'm not sure if Python is 
really needed)

`brew install cmake python`

4. Clone LLVM and create the build directory.
5. Generate Xcode project using CMake.

  `cmake -G "Xcode" -DCMAKE_BUILD_TYPE=Debug -DLLVM_TARGETS_TO_BUILD="X86" 
-DLLVM_ENABLE_PROJECTS="clang" -DLLVM_OPTIMIZED_TABLEGEN=ON 
-DLLVM_ENABLE_IDE=ON -DLLVM_CCACHE_BUILD=ON path/to/llvm`

  Flags other than `-G "Xcode"` probably are optional for the purpose of 
reproducing the error messages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136906

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

[PATCH] D136906: [Clang][LoongArch] Implement __builtin_loongarch_dbar builtin

[冀卓疌 via Phabricator via cfe-commits]

WowbaggersLiquidLunch added a comment.

In D136906#3940377 , @gonglingqin 
wrote:

> Can you use https://reviews.llvm.org/D138403 to solve this problem?

Thanks for letting me know. I'll give it a try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136906

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

[PATCH] D138403: [LoongArch] Fix issue on CMake Xcode build configuration

[冀卓疌 via Phabricator via cfe-commits]

WowbaggersLiquidLunch accepted this revision.
WowbaggersLiquidLunch added a comment.

Yes this works! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138403

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

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D136594#3940143 , @nridge wrote:

> I believe highlighting of **built-in** operators should be out of scope for 
> semantic highlighting, at least in the default mode; client-side highlighting 
> should be sufficient for these, similar to strings and literals.

I'm not sure if "built-in" (as opposed to overloaded) is the right distinction 
here: whether `a == b` can be highlighted client-side seems unrelated to 
whether the type is `int` or `string`.
But I agree we only need to provide this (by default) if it's something that 
can't be determined by lexing.

An (IMO) useful distinction that can't be found by the lexer is the use of `*` 
as a declarator (`int *x`) vs an operator (`return *x`). These are both 
potentially "built-in". https://github.com/helix-editor/helix/pull/4278 has a 
fuller example. (In practice, clients are going to highlight declarator-stars 
as operators on the client-side, so to make this work we have to highlight them 
as something else rather than just mark operators. But this patch looks like 
the right direction).

> 2. An alternative to assigning (user-provided) operators a new token kind 
> would be to assign them the same token kind as the entity they invoke (i.e. 
> function or method). Both approaches have their advantages:
>   - If we use the function/method kinds, then uses of user-provided operators 
> will be highlighted differently from built-in operators even when using a 
> default / standard theme that doesn't know about clangd-specific token types.
>   - If we use a dedicated operator kind, users can configure different styles 
> for operators vs. function/methods (and they may want different styles given 
> that syntactically the two look quite different).
>
> One way to get the best of both worlds could be to use the 
> function/method kinds in combination with an `operator` **modifier**. That 
> would color overloaded operators out of the box while also allowing users to 
> customize the style based on the presence of the modifier. What do you think 
> about this approach?

I think since LSP specifies an `operator` SemanticTokenType, we should use it 
unless there's a *really* strong reason not to.
A well-behaved editor will provide a themeable color for (client-side) 
highlighting of operators, and also map LSP's `operator` SemanticTokenType onto 
that color by default. If we decide to do our own different thing, we'll break 
that happy path.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76
   ConstructorOrDestructor,
+  UserProvided,
 

Can you give some background here or on the bug tracker about what kind of 
distinction you're trying to draw here and why it's important?
(Most clients are never going to benefit from nonstandard modifiers so they 
should be pretty compelling)



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76
   ConstructorOrDestructor,
+  UserProvided,
 

sammccall wrote:
> Can you give some background here or on the bug tracker about what kind of 
> distinction you're trying to draw here and why it's important?
> (Most clients are never going to benefit from nonstandard modifiers so they 
> should be pretty compelling)
as well as being jargony, "user-provided" has a specific technical meaning that 
I don't think you intend here. For example, `auto operator<=>(const S&) const = 
default` is not user-provided (defaulted on first declaration). 
https://eel.is/c++draft/dcl.fct.def.default#5

If we need this and can't get away with reusing `defaultLibrary` (which would 
include `std::`) then maybe we should add something like `builtin` which seems 
quite reusable.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76
   ConstructorOrDestructor,
+  UserProvided,
 

sammccall wrote:
> sammccall wrote:
> > Can you give some background here or on the bug tracker about what kind of 
> > distinction you're trying to draw here and why it's important?
> > (Most clients are never going to benefit from nonstandard modifiers so they 
> > should be pretty compelling)
> as well as being jargony, "user-provided" has a specific technical meaning 
> that I don't think you intend here. For example, `auto operator<=>(const S&) 
> const = default` is not user-provided (defaulted on first declaration). 
> https://eel.is/c++draft/dcl.fct.def.default#5
> 
> If we need this and can't get away with reusing `defaultLibrary` (which would 
> include `std::`) then maybe we should add something like `builtin` which 
> seems quite reusable.
Since we often can't say whether an operator is user-provided or not (in 
templates), we should consider what we want the highlighting to be in these 
cases.
(If templates should be highlighted as built-in, we should prefer a modifier 
like `UserProvided`, if they should be highlighted as user-provided, we should 
prefer a mo

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Not sure this is ready for review again, ignore me if not...

I'm still not sure why this is correct in principle. Without that, if someone 
finds a misparse 6 months from now I don't know how we determine where to fix 
it.

For example, this path is called from 
`Parser::isKnownToBeDeclarationSpecifier()` whose contract is `Return true if 
we know that we are definitely looking at a decl-specifier... Return false if 
it's no a decl-specifier, or we're not sure.` There doesn't seem to be any room 
for heuristics, unless we're going to change that contract and audit all the 
callers. If this *isn't* a heuristic (it sure looks like one) it needs some 
comments on why it's correct.




Comment at: clang/test/Parser/recovery.c:105
+  unknown_t a; // expected-error {{unknown type name 'unknown_t'}}
+  unknown_t *b; // expected-error {{unknown type name 'unknown_t'}}
+  unknown_t const *c; // expected-error {{unknown type name 'unknown_t'}}

this diagnostic is worse than the old one (less accurate).

(I think it's OK to trade off diagnostics quality if it's better on balance, 
maybe leave a comment?)


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

https://reviews.llvm.org/D137020

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

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D137205#3939720 , @njames93 wrote:

> @sammccall I have a feeling you're gonna want to examine this checks 
> feasibility in clangd.

Thanks! As it uses the CFG, by default we're going to have to turn it off 
(AFAIK building the CFG with broken code can still crash).
If you don't mind, please add it to the exclude list in 
`clang-tools-extra/clangd/TidyProvider.cpp` next to `-bugprone-use-after-move` 
(or I can do this after it lands).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D136906: [Clang][LoongArch] Implement __builtin_loongarch_dbar builtin

[Gong LingQin via Phabricator via cfe-commits]

gonglingqin added a comment.

In D136906#3940427 , 
@WowbaggersLiquidLunch wrote:

>> I get the following error:
>>
>>> CMake Error: Could not create named generator Xcode
>
> Sorry, I forgot to mention that Xcode is only available on macOS.
>
>> But when I change Xcode to Ninja, no error output is generated.
>
> Indeed. Ninja works fine for me as well. The problem is I kind of need both 
> Xcode and Ninja in my workflow (Xcode for editing, Ninja for building).
>
>> Do you have any suggestions for reproducing the bugs you mentioned?
>
> I think the following steps should be able to reproduce the error messages 
> I'm seeing (It's a bit long):
>
> 1. Install macOS 12.6 or newer. (I'm currently on 12.6, but newer versions 
> such as 12.6.1 and 13 (the latest) should produce the same error messages.)
>
> 2. Install Xcode 14.1 and/or the Xcode command line tools
>   - Xcode can be installed via this link 
> 
>  (generally recommended), or from the Mac App Store via this link 
> .
> - The first link downloads a `.xip` file which can take a long time to 
> decompress. This step can be sped up significantly by decompressing the file 
> using this tool .
>   - The command line tools can be installed via this link 
> ,
>  via the following commands, or by launching Xcode after it's installed. (I 
> don't remember it clearly, but I think you probably can skip this step and 
> have Homebrew install it for you in step 3)
>
> `sudo rm -rf /Library/Developer/CommandLineTools`
>
> `sudo xcode-select --install`
>
> 3. Install LLVM's dependencies 
>  via Homebrew
>   - First we need to install brew  using the following 
> command
>
> `/bin/bash -c "$(curl -fsSL 
> https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"`
>   - Then install Cmake and Python with `brew`. (I'm not sure if Python is 
> really needed)
>
> `brew install cmake python`
>
> 4. Clone LLVM and create the build directory.
> 5. Generate Xcode project using CMake.
>
>   `cmake -G "Xcode" -DCMAKE_BUILD_TYPE=Debug -DLLVM_TARGETS_TO_BUILD="X86" 
> -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_OPTIMIZED_TABLEGEN=ON 
> -DLLVM_ENABLE_IDE=ON -DLLVM_CCACHE_BUILD=ON path/to/llvm`
>
>   Flags other than `-G "Xcode"` probably are optional for the purpose of 
> reproducing the error messages.

Thanks! Very nice to know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136906

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

[PATCH] D136906: [Clang][LoongArch] Implement __builtin_loongarch_dbar builtin

[Gong LingQin via Phabricator via cfe-commits]

gonglingqin added a comment.

In D136906#3940432 , 
@WowbaggersLiquidLunch wrote:

> In D136906#3940377 , @gonglingqin 
> wrote:
>
>> Can you use https://reviews.llvm.org/D138403 to solve this problem?
>
> Thanks for letting me know. I'll give it a try.

Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136906

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

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76
   ConstructorOrDestructor,
+  UserProvided,
 

sammccall wrote:
> sammccall wrote:
> > sammccall wrote:
> > > Can you give some background here or on the bug tracker about what kind 
> > > of distinction you're trying to draw here and why it's important?
> > > (Most clients are never going to benefit from nonstandard modifiers so 
> > > they should be pretty compelling)
> > as well as being jargony, "user-provided" has a specific technical meaning 
> > that I don't think you intend here. For example, `auto operator<=>(const 
> > S&) const = default` is not user-provided (defaulted on first declaration). 
> > https://eel.is/c++draft/dcl.fct.def.default#5
> > 
> > If we need this and can't get away with reusing `defaultLibrary` (which 
> > would include `std::`) then maybe we should add something like `builtin` 
> > which seems quite reusable.
> Since we often can't say whether an operator is user-provided or not (in 
> templates), we should consider what we want the highlighting to be in these 
> cases.
> (If templates should be highlighted as built-in, we should prefer a modifier 
> like `UserProvided`, if they should be highlighted as user-provided, we 
> should prefer a modifier like `Builtin`)
> as well as being jargony, "user-provided" has a specific technical meaning 
> that I don't think you intend here. For example, `auto operator<=>(const S&) 
> const = default` is not user-provided (defaulted on first declaration). 
> https://eel.is/c++draft/dcl.fct.def.default#5
> 
> If we need this and can't get away with reusing `defaultLibrary` (which would 
> include `std::`) then maybe we should add something like `builtin` which 
> seems quite reusable.

I use "userProvided" here in the sense of "not built-in" or "overloaded". I do 
not cling to the term, and have also suggested the opposite default of using 
"builtin" in another comment.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76
   ConstructorOrDestructor,
+  UserProvided,
 

ckandeler wrote:
> sammccall wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > Can you give some background here or on the bug tracker about what kind 
> > > > of distinction you're trying to draw here and why it's important?
> > > > (Most clients are never going to benefit from nonstandard modifiers so 
> > > > they should be pretty compelling)
> > > as well as being jargony, "user-provided" has a specific technical 
> > > meaning that I don't think you intend here. For example, `auto 
> > > operator<=>(const S&) const = default` is not user-provided (defaulted on 
> > > first declaration). https://eel.is/c++draft/dcl.fct.def.default#5
> > > 
> > > If we need this and can't get away with reusing `defaultLibrary` (which 
> > > would include `std::`) then maybe we should add something like `builtin` 
> > > which seems quite reusable.
> > Since we often can't say whether an operator is user-provided or not (in 
> > templates), we should consider what we want the highlighting to be in these 
> > cases.
> > (If templates should be highlighted as built-in, we should prefer a 
> > modifier like `UserProvided`, if they should be highlighted as 
> > user-provided, we should prefer a modifier like `Builtin`)
> > as well as being jargony, "user-provided" has a specific technical meaning 
> > that I don't think you intend here. For example, `auto operator<=>(const 
> > S&) const = default` is not user-provided (defaulted on first declaration). 
> > https://eel.is/c++draft/dcl.fct.def.default#5
> > 
> > If we need this and can't get away with reusing `defaultLibrary` (which 
> > would include `std::`) then maybe we should add something like `builtin` 
> > which seems quite reusable.
> 
> I use "userProvided" here in the sense of "not built-in" or "overloaded". I 
> do not cling to the term, and have also suggested the opposite default of 
> using "builtin" in another comment.
> Since we often can't say whether an operator is user-provided or not (in 
> templates), we should consider what we want the highlighting to be in these 
> cases.

True, I have not considered this.

> (If templates should be highlighted as built-in, we should prefer a modifier 
> like `UserProvided`, if they should be highlighted as user-provided, we 
> should prefer a modifier like `Builtin`)

Intuitively, it seems we should be conservative and not claim the operator is 
overloaded unless we know it is. So "built-in" might then mean "we can't prove 
it's not a built-in". It's probably not worth to introduce a modifier 
CouldBeEitherWay just to explicitly express ambiguity ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136594

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llv

[PATCH] D138143: [FPEnv] Enable strict fp for AArch64 in clang

[Oliver Stannard via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138143

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

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Protocol.h:231
 
+/// Special Location-like type to be used for textDocument/references
+/// Contains an additional field for the context in which the reference occurs

This doesn't seem as clear as it could be: special how, additional to what? 
Maybe:

```
/// Extends Locations returned by textDocument/references with extra info.
/// This is a clangd extenison: LSP uses `Location`.
```



Comment at: clang-tools-extra/clangd/Protocol.h:236
+  /// reference occurs
+  llvm::Optional context;
+

"context" is vague, and is generally used in LSP to mean "details about how a 
request was triggered in the client".

Unless we have a reason to keep this abstract, I'd use `containerName` as it's 
used elsewhere in LSP for this purpose.
(I kind of hate the name, but at least people will be familiar with it)



Comment at: clang-tools-extra/clangd/Protocol.h:244
+
+  friend bool operator!=(const ReferenceLocation &LHS,
+ const ReferenceLocation &RHS) {

are these operators actually needed? especially `operator<` we usually get away 
without



Comment at: clang-tools-extra/clangd/XRefs.cpp:1307
+llvm::Optional
+getContextStringForMainFileRef(const DeclContext *DeclCtx) {
+  for (auto *Ctx = DeclCtx; Ctx; Ctx = Ctx->getParent()) {

a few things to consider here:
 - for e.g. lambdas, do you want `foo::bar::(anonymous class)::operator()`, or 
`foo::bar`, or something else?
 - there are a few other function-like decls, see Decl::isFunctionOrMethod().
 - only functions? suppose you have `struct S { T member; };` and you're 
looking up xrefs for T. Don't we want to return `S` as the context/container 
name
 - in general, I think want to get the same container for the index vs AST path 
here. This suggests we should be sharing `getRefContainer` from 
`SymbolCollector.cpp` rather than implementing something ifferent



Comment at: clang-tools-extra/clangd/XRefs.cpp:1310
+if (const auto *FD = llvm::dyn_cast(Ctx))
+  return FD->getQualifiedNameAsString();
+if (const auto *RD = llvm::dyn_cast(Ctx))

again for stringifying, I think we want the index/non-index cases to be the 
same, which suggests the logic to generate Scope+Name (i.e. 
clangd::printQualifiedName)+Signature and then concatenating them.

Generating the signature is a bit complicated, so maybe leave it out (from both 
AST + index codepath) from initial patch?



Comment at: clang-tools-extra/clangd/XRefs.cpp:1458
+  assert(Ref != RefIndexForContainer.end());
+  // Name is not useful here, because it's just gonna be the name of 
the
+  // function the cursor is on. Scope is interesting though, since this

To me, this doesn't seem like a sufficient reason to have irregular behavior 
for different refs.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1462
+  Results.References[Ref->getSecond()].Loc.context =
+  Container.Scope.drop_back(2).str(); // Drop trailing "::"
+});

nit: unconditional drop_back(2) seems brittle, consume_back instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137894

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

[PATCH] D124730: [RISCV][NFC] Refactor RISC-V vector intrinsic utils.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng marked 2 inline comments as done.
kito-cheng added a comment.

@kadircet ooops, sorry for missing your comment, let me figure out how to fix 
that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124730

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

[PATCH] D138287: [clang][RISCV] Drop caching from RVVType as it introduces data races

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: kito.cheng.
sammccall added a comment.

I think there almost certainly *is* a better way here, but it may be a 
significant amount of work and we need to get back to a correct state (revert 
doesn't seem feasible).

Not approving yet in the hope that @kito-cheng (@kito.cheng?) or @khchen can 
find something better, if we don't hear back I'll stamp in a week or so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138287

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

Re: [PATCH] D124730: [RISCV][NFC] Refactor RISC-V vector intrinsic utils.

[Kadir Çetinkaya via cfe-commits]

thanks Kito! not sure if you've noticed D138287, but if landing a solution
is going to take a long while here, i'd like to move forward with that
approach to make sure this doesn't stay in a broken state. so it'd be great
if you can give some updates/estimates about fixing this soon.

On Mon, Nov 21, 2022 at 11:14 AM Kito Cheng via Phabricator <
revi...@reviews.llvm.org> wrote:

> kito-cheng marked 2 inline comments as done.
> kito-cheng added a comment.
>
> @kadircet ooops, sorry for missing your comment, let me figure out how to
> fix that.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D124730/new/
>
> https://reviews.llvm.org/D124730
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137943: [clangd] Mark "override" and "final" as keywords

[Sam McCall via Phabricator via cfe-commits]

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

LGTM with optional mechanical replacement of `Keyword` with `Modifier`.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:52
   Macro,
+  Keyword,
 

LSP provides both `keyword` and `modifier`, which obviously overlap.
I think `modifier` is more specific and applies here, so would lean towards 
that. WDYT?



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:868
+  // override and final
+  R"cpp(
+class $Class_def_abstract[[Base]] { virtual void 
$Method_decl_abstract_virtual[[m]]() = 0; };

can you add something like `int override, final;` to check that these are *not* 
highlighted?



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:872
+class $Class_def[[MoreDerived]] : public $Class[[Derived]] { void 
$Method_decl_virtual[[m]]() $Keyword[[final]]; };
   // Issue 1222: readonly modifier for generic parameter
+  )cpp",

this comment should not be inside the string literal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137943

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

[PATCH] D119541: [RISCV] Fix RISCVTargetInfo::initFeatureMap, add non-ISA features back after implication

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.
Herald added subscribers: sunshaoce, StephenFan, shiva0217.
Herald added a project: All.

I just noticed that target features (e.g. -mrelax) are broken in all LLVM 14 
releases due to D113336  . This should have 
been cherry-picked back tot the release branch, but it's too late now. In the 
future please ensure that important fixes such as this one end up on the 
release branch as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119541

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

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Since this is a protocol extension, you might want to add an end-to-end test: 
variant of `clang-tools-extra/clangd/test/xrefs.test` (`xrefs-container.test`? 
to avoid complicating all the existing tests).

And one this lands, one of us should update 
https://github.com/llvm/clangd-www/blob/main/extensions.md


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137894

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

[PATCH] D138287: [clang][RISCV] Drop caching from RVVType as it introduces data races

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Apparently I missed those comments during llvm dev meeting, I'll figure out a 
fix soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138287

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

[PATCH] D138287: [clang][RISCV] Drop caching from RVVType as it introduces data races

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D138287#3940602 , @kito-cheng 
wrote:

> Apparently I missed those comments during llvm dev meeting, I'll figure out a 
> fix soon.

Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138287

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

[PATCH] D137943: [clangd] Mark "override" and "final" as modifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 476843.
ckandeler marked 3 inline comments as done.
ckandeler retitled this revision from "[clangd] Mark "override" and "final" as 
keywords" to "[clangd] Mark "override" and "final" as modifiers".
ckandeler edited the summary of this revision.
ckandeler added a comment.

Incorporated review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137943

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -864,6 +864,12 @@
 const char *$LocalVariable_def_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // override and final
+  R"cpp(
+class $Class_def_abstract[[Base]] { virtual void 
$Method_decl_abstract_virtual[[m]]() = 0; };
+class $Class_def[[override]] : public $Class_abstract[[Base]] { void 
$Method_decl_virtual[[m]]() $Modifier[[override]]; };
+class $Class_def[[final]] : public $Class[[override]] { void 
$Method_decl_virtual[[m]]() $Modifier[[override]] $Modifier[[final]]; };
+  )cpp",
   // Issue 1222: readonly modifier for generic parameter
   R"cpp(
 template 
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -49,6 +49,7 @@
   Concept,
   Primitive,
   Macro,
+  Modifier,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -809,6 +809,18 @@
 return true;
   }
 
+  bool VisitAttr(Attr *A) {
+switch (A->getKind()) {
+case attr::Override:
+case attr::Final:
+  H.addToken(A->getLocation(), HighlightingKind::Modifier);
+  break;
+default:
+  break;
+}
+return true;
+  }
+
   bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
 H.addToken(L.getNameLoc(), HighlightingKind::Type)
 .addModifier(HighlightingModifier::DependentName)
@@ -985,6 +997,8 @@
 return OS << "Primitive";
   case HighlightingKind::Macro:
 return OS << "Macro";
+  case HighlightingKind::Modifier:
+return OS << "Modifier";
   case HighlightingKind::InactiveCode:
 return OS << "InactiveCode";
   }
@@ -1119,6 +1133,8 @@
 return "type";
   case HighlightingKind::Macro:
 return "macro";
+  case HighlightingKind::Modifier:
+return "modifier";
   case HighlightingKind::InactiveCode:
 return "comment";
   }


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -864,6 +864,12 @@
 const char *$LocalVariable_def_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // override and final
+  R"cpp(
+class $Class_def_abstract[[Base]] { virtual void $Method_decl_abstract_virtual[[m]]() = 0; };
+class $Class_def[[override]] : public $Class_abstract[[Base]] { void $Method_decl_virtual[[m]]() $Modifier[[override]]; };
+class $Class_def[[final]] : public $Class[[override]] { void $Method_decl_virtual[[m]]() $Modifier[[override]] $Modifier[[final]]; };
+  )cpp",
   // Issue 1222: readonly modifier for generic parameter
   R"cpp(
 template 
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -49,6 +49,7 @@
   Concept,
   Primitive,
   Macro,
+  Modifier,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -809,6 +809,18 @@
 return true;
   }
 
+  bool VisitAttr(Attr *A) {
+switch (A->getKind()) {
+case attr::Override:
+case attr::Final:
+  H.addToken(A->getLocation(), HighlightingKi

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Now that Kona is over, I'm hoping to get back to that in the coming weeks. 
Rebasing will be... fun.
I want to make sure we are all okay making that a DR following WG21 guidance, 
given that not making it a DR would have potentially large impact on the PR.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D138378: [clang-format][NFC] Skip unneeded calculations

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1434-1436
+  if (Style.UseTab == FormatStyle::UT_Never) {
+ReplacementText.append(Spaces, ' ');
+  } else {

owenpan wrote:
> Doing this would lose some of the abstraction we have now without any gain in 
> performance? Then we should leave it (and `appendIndentText` below) as is.
The `UT_Never` case would be faster, if that would be measurable I don't know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138378

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

[clang] aff838f - [clang-format][NFC] Remove unneeded braces

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

Author: Björn Schäpers
Date: 2022-11-21T13:24:17+01:00
New Revision: aff838fb8f1d398e8d9fc6ec15b5a10db3e1c057

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

LOG: [clang-format][NFC] Remove unneeded braces

Done by clang-format itself.

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index e7280df4c78e4..87e70c367fde0 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1352,9 +1352,8 @@ class AnnotatingParser {
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator))
   CurrentToken->setType(TT_ImplicitStringLiteral);
-}
 next();
   }
 }



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

[clang] c89d2fd - [clang-format][NFC] Format lambda in conditional

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

Author: Björn Schäpers
Date: 2022-11-21T13:24:18+01:00
New Revision: c89d2fd596cec8cf3a8100c504f045b9727fa98a

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

LOG: [clang-format][NFC] Format lambda in conditional

This sort of amends cdbe296853b1b3fc6415236f05770360e23f0d39

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 87e70c367fde..21152b8c20e1 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2583,7 +2583,7 @@ class ExpressionParser {
   (Start->Previous &&
Start->Previous->isOneOf(TT_RequiresClause,
 TT_RequiresClauseInARequiresExpression))
-  ? [this](){
+  ? [this]() {
   auto Ret = Current ? Current : Line.Last;
   while (!Ret->ClosesRequiresClause && Ret->Previous)
 Ret = Ret->Previous;



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

[clang] 9e00909 - [clang-format][NFC] Don't add a load of 0es

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

Author: Björn Schäpers
Date: 2022-11-21T13:24:18+01:00
New Revision: 9e00909b00f9e75b4f710bc60f13ceea1215746c

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

LOG: [clang-format][NFC] Don't add a load of 0es

Shift is 0 for all non comments, maybe even for comments. Don't add the
0 up to three times.

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

Added: 


Modified: 
clang/lib/Format/WhitespaceManager.cpp

Removed: 




diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index cd9773921dae..ff7ef7edc984 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -1049,7 +1049,7 @@ void WhitespaceManager::alignTrailingComments(unsigned 
Start, unsigned End,
   Changes[i].StartOfBlockComment->StartOfTokenColumn -
   Changes[i].StartOfTokenColumn;
 }
-if (Shift < 0)
+if (Shift <= 0)
   continue;
 Changes[i].Spaces += Shift;
 if (i + 1 != Changes.size())



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

[clang] 5ba5f7c - [clang-format][NFC] Removed unused include

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

Author: Björn Schäpers
Date: 2022-11-21T13:24:19+01:00
New Revision: 5ba5f7c46ccfe98fd68d6e31d25f386d67d484b4

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

LOG: [clang-format][NFC] Removed unused include

Added: 


Modified: 
clang/unittests/Format/FormatTestComments.cpp

Removed: 




diff  --git a/clang/unittests/Format/FormatTestComments.cpp 
b/clang/unittests/Format/FormatTestComments.cpp
index e95e89ae98a54..524bf87f37ba5 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -12,7 +12,6 @@
 #include "FormatTestUtils.h"
 
 #include "llvm/Support/Debug.h"
-#include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
 
 #define DEBUG_TYPE "format-test"



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

[PATCH] D138354: [clang-format][NFC] Remove unneeded braces

[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 rGaff838fb8f1d: [clang-format][NFC] Remove unneeded braces 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138354

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1352,9 +1352,8 @@
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator))
   CurrentToken->setType(TT_ImplicitStringLiteral);
-}
 next();
   }
 }


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1352,9 +1352,8 @@
   next();
   next(); // Consume first token (so we fix leading whitespace).
   while (CurrentToken) {
-if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) {
+if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator))
   CurrentToken->setType(TT_ImplicitStringLiteral);
-}
 next();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138357: [clang-format][NFC] Don't add a load of 0es

[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 rG9e00909b00f9: [clang-format][NFC] Don't add a load of 
0es (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138357

Files:
  clang/lib/Format/WhitespaceManager.cpp


Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1049,7 +1049,7 @@
   Changes[i].StartOfBlockComment->StartOfTokenColumn -
   Changes[i].StartOfTokenColumn;
 }
-if (Shift < 0)
+if (Shift <= 0)
   continue;
 Changes[i].Spaces += Shift;
 if (i + 1 != Changes.size())


Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1049,7 +1049,7 @@
   Changes[i].StartOfBlockComment->StartOfTokenColumn -
   Changes[i].StartOfTokenColumn;
 }
-if (Shift < 0)
+if (Shift <= 0)
   continue;
 Changes[i].Spaces += Shift;
 if (i + 1 != Changes.size())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138356: [clang-format][NFC] Format lambda in conditional

[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 rGc89d2fd596ce: [clang-format][NFC] Format lambda in 
conditional (authored by HazardyKnusperkeks).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138356

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2583,7 +2583,7 @@
   (Start->Previous &&
Start->Previous->isOneOf(TT_RequiresClause,
 TT_RequiresClauseInARequiresExpression))
-  ? [this](){
+  ? [this]() {
   auto Ret = Current ? Current : Line.Last;
   while (!Ret->ClosesRequiresClause && Ret->Previous)
 Ret = Ret->Previous;


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2583,7 +2583,7 @@
   (Start->Previous &&
Start->Previous->isOneOf(TT_RequiresClause,
 TT_RequiresClauseInARequiresExpression))
-  ? [this](){
+  ? [this]() {
   auto Ret = Current ? Current : Line.Last;
   while (!Ret->ClosesRequiresClause && Ret->Previous)
 Ret = Ret->Previous;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

[Tobias Grosser via Phabricator via cfe-commits]

grosser added a comment.

This patch overcomes the issue of cl options being defined multiple times. That 
is great!

I just tried it on Windows on top of e7fb6c394f519d6e6812f1fbbff1571d5e51f5c4 
 with 
(msys2), as well as LLVM_LINK_LLVM_DYLIB, LLVM_BUILD_LLVM_DYLIB, and MLIR 
enabled. I get the following error:

  cmd.exe /C "cd . && C:\msys64\clang64\bin\clang++.exe 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections 
-fdata-sections -Werror=mismatched-tags -O3 -DNDEBUG -Wl,--stack,16777216
-Wl,--gc-sections 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj
 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeFormatGen.cpp.obj
 tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/DialectGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/DirectiveCommonGen.cpp.obj
 tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/EnumsGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/FormatGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/LLVMIRConversionGen.cpp.obj
 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/LLVMIRIntrinsicGen.cpp.obj
 tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/mlir-tblgen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpClass.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpDefinitionsGen.cpp.obj
 tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpDocGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpFormatGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpGenHelpers.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpInterfacesGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpPythonBindingGen.cpp.obj
 tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/PassCAPIGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/PassDocGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/PassGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/RewriterGen.cpp.obj 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/SPIRVUtilsGen.cpp.obj 
-o bin\mlir-tblgen.exe -Wl,--out-implib,lib\libmlir-tblgen.dll.a 
-Wl,--major-image-version,0,--minor-image-version,0  
lib/libMLIRSupportIndentedOstream.a  lib/libMLIRTblgenLib.a  
lib/libMLIRTableGen.a  lib/libLLVM-16git.dll.a  -lkernel32 -luser32 -lgdi32 
-lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
  ld.lld: error: undefined symbol: 
llvm::RecordKeeper::getAllDerivedDefinitionsIfDefined(llvm::StringRef) const
  >>> referenced by 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj:(std::__1::__function::__func<$_2,
 std::__1::allocator<$_2>, bool (llvm::RecordKeeper const&, 
llvm::raw_ostream&)>::operator()(llvm::RecordKeeper const&, llvm::raw_ostream&))
  >>> referenced by 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj:(std::__1::__function::__func<$_3,
 std::__1::allocator<$_3>, bool (llvm::RecordKeeper const&, 
llvm::raw_ostream&)>::operator()(llvm::RecordKeeper const&, llvm::raw_ostream&))
  >>> referenced by 
tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj:(std::__1::__function::__func<$_4,
 std::__1::allocator<$_4>, bool (llvm::RecordKeeper const&, 
llvm::raw_ostream&)>::operator()(llvm::RecordKeeper const&, llvm::raw_ostream&))
  >>> referenced 8 more times


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138278

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

[PATCH] D138418: [LoongArch] Add remaining intrinsics for CRC check instructions

[Gong LingQin via Phabricator via cfe-commits]

gonglingqin created this revision.
gonglingqin added reviewers: xen0n, xry111, SixWeining, wangleiat, MaskRay, 
XiaodongLoong.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
gonglingqin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

After D137316  implements the intrinsics of 
the first crc check instruction
and related diagnosis, this patch implements the intrinsics of all remaining
crc check instructions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138418

Files:
  clang/include/clang/Basic/BuiltinsLoongArch.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/larchintrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/LoongArch/intrinsic-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la64.c
  llvm/include/llvm/IR/IntrinsicsLoongArch.td
  llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
  llvm/lib/Target/LoongArch/LoongArchISelLowering.h
  llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
  llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
  llvm/test/CodeGen/LoongArch/intrinsic-la64.ll

Index: llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
@@ -1,7 +1,41 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc --mtriple=loongarch64 < %s | FileCheck %s
 
+declare i32 @llvm.loongarch.crc.w.b.w(i32, i32)
+declare i32 @llvm.loongarch.crc.w.h.w(i32, i32)
+declare i32 @llvm.loongarch.crc.w.w.w(i32, i32)
 declare i32 @llvm.loongarch.crc.w.d.w(i64, i32)
+declare i32 @llvm.loongarch.crcc.w.b.w(i32, i32)
+declare i32 @llvm.loongarch.crcc.w.h.w(i32, i32)
+declare i32 @llvm.loongarch.crcc.w.w.w(i32, i32)
+declare i32 @llvm.loongarch.crcc.w.d.w(i64, i32)
+
+define i32 @crc_w_b_w(i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: crc_w_b_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crc.w.b.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crc.w.b.w(i32 %a, i32 %b)
+  ret i32 %res
+}
+
+define i32 @crc_w_h_w(i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: crc_w_h_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crc.w.h.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crc.w.h.w(i32 %a, i32 %b)
+  ret i32 %res
+}
+
+define i32 @crc_w_w_w(i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: crc_w_w_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crc.w.w.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crc.w.w.w(i32 %a, i32 %b)
+  ret i32 %res
+}
 
 define i32 @crc_w_d_w(i64 %a, i32 %b) nounwind {
 ; CHECK-LABEL: crc_w_d_w:
@@ -11,3 +45,39 @@
   %res = call i32 @llvm.loongarch.crc.w.d.w(i64 %a, i32 %b)
   ret i32 %res
 }
+
+define i32 @crcc_w_b_w(i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: crcc_w_b_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crcc.w.b.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crcc.w.b.w(i32 %a, i32 %b)
+  ret i32 %res
+}
+
+define i32 @crcc_w_h_w(i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: crcc_w_h_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crcc.w.h.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crcc.w.h.w(i32 %a, i32 %b)
+  ret i32 %res
+}
+
+define i32 @crcc_w_w_w(i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: crcc_w_w_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crcc.w.w.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crcc.w.w.w(i32 %a, i32 %b)
+  ret i32 %res
+}
+
+define i32 @crcc_w_d_w(i64 %a, i32 %b) nounwind {
+; CHECK-LABEL: crcc_w_d_w:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:crcc.w.d.w $a0, $a0, $a1
+; CHECK-NEXT:ret
+  %res = call i32 @llvm.loongarch.crcc.w.d.w(i64 %a, i32 %b)
+  ret i32 %res
+}
Index: llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
@@ -1,6 +1,34 @@
 ; RUN: not llc --mtriple=loongarch32 --disable-verify < %s 2>&1 | FileCheck %s
 
+declare i32 @llvm.loongarch.crc.w.b.w(i32, i32)
+declare i32 @llvm.loongarch.crc.w.h.w(i32, i32)
+declare i32 @llvm.loongarch.crc.w.w.w(i32, i32)
 declare i32 @llvm.loongarch.crc.w.d.w(i64, i32)
+declare i32 @llvm.loongarch.crcc.w.b.w(i32, i32)
+declare i32 @llvm.loongarch.crcc.w.h.w(i32, i32)
+declare i32 @llvm.loongarch.crcc.w.w.w(i32, i32)
+declare i32 @llvm.loongarch.crcc.w.d.w(i64, i32)
+
+define i32 @crc_w_b_w(i32 %a, i32 %b) nounwind {
+; CHECK: llvm.loongarch.crc.w.b.w requires target: loongarch64
+entry:
+  %res = call i32 @llvm.loongarch.crc.w.b.w(i32 %a, i32 %b)
+  ret i32 %res
+}
+
+define i32 @crc_w_h_w(i32 %a, i32 %b) nounwind {
+; CHECK: llvm.loongarch.crc.w.h.w requires target: loongarch64
+entry:
+  %res = call i32 @llvm.loongarch.crc.w.h.w(i32

[PATCH] D137794: [clangd] Enable configuring include insertions

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

The replacement rules design is just too complicated I'm afraid.
It's unclear that it solves a widespread problem, and even when there is one 
and the solution applies, the result is still difficult to use.

Angles vs quotes is something we've had multiple reports on, though, and may be 
narrow enough to find a simple solution.

- maybe something config-based like `Style: { IncludeAngled: 
'match/a/suffix/.*' }`
- or something based on seeing how the header is included and storing this in 
the index. This is nice as it avoids config, but seems complicated in other 
ways (unpredictable, relies on complete index, can go wrong, may regress 
existing behavior)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137794

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

[PATCH] D138088: [clang][docs] Use `option` directive in User's Manual

[Aaron Ballman via Phabricator via cfe-commits]

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

In D138088#3938687 , @kawashima-fj 
wrote:

> In D138088#3937680 , @aaron.ballman 
> wrote:
>
>> Thank you for this cleanup! In general, I thin this looks correct. However, 
>> I know we've had to fix a bunch of options that cause the sphinx build to 
>> fail (IIRC, oftentimes due to duplicate options) and our precommit CI 
>> doesn't test the documentation build. Did you try building the docs locally 
>> to ensure there are no new warnings/errors from Sphinx?
>
> Yes. I confirmed no new warnings/errors with the following commands. I used 
> Sphinx packaged by distributions (Ubuntu 22.04 and Debian GNU/Linux 11). Is 
> it sufficient? If no, let me know.
>
>   cmake -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang 
> -DLLVM_ENABLE_SPHINX=ON -DLLVM_INCLUDE_DOCS=ON [other unrelated options ...]
>   ninja docs-clang-html

Excellent, thank you! This LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138088

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

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: clang-vendors.
aaron.ballman added a comment.

In D124351#3940786 , @cor3ntin wrote:

> Now that Kona is over, I'm hoping to get back to that in the coming weeks. 
> Rebasing will be... fun.
> I want to make sure we are all okay making that a DR following WG21 guidance, 
> given that not making it a DR would have potentially large impact on the PR.
> Thanks!

To me, it depends on how much existing code the DR breaks in practice. If we 
don't expect it to break a significant body of code (or if the code that breaks 
will be materially improved as a result of the required changes), then we 
should implement it as a DR as far back as we can go. If it breaks too much 
code in practice, then we might not want to implement it as a DR in older 
language modes. If that situation comes up and it's very hard for us to carry 
both implementations, then we should go back to WG21 with the further 
information before the DIS ballot goes out (if possible).

I'm adding `clang-vendors` to the review group because of the potential for 
disruption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D124351#3940888 , @aaron.ballman 
wrote:

> In D124351#3940786 , @cor3ntin 
> wrote:
>
>> Now that Kona is over, I'm hoping to get back to that in the coming weeks. 
>> Rebasing will be... fun.
>> I want to make sure we are all okay making that a DR following WG21 
>> guidance, given that not making it a DR would have potentially large impact 
>> on the PR.
>> Thanks!
>
> To me, it depends on how much existing code the DR breaks in practice. If we 
> don't expect it to break a significant body of code (or if the code that 
> breaks will be materially improved as a result of the required changes), then 
> we should implement it as a DR as far back as we can go.

We expect very little, if any, breakage

> If it breaks too much code in practice, then we might not want to implement 
> it as a DR in older language modes. If that situation comes up and it's very 
> hard for us to carry both implementations, then we should go back to WG21 
> with the further information before the DIS ballot goes out (if possible).

Agreed, but time is short

> I'm adding `clang-vendors` to the review group because of the potential for 
> disruption.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[David Friberg via Phabricator via cfe-commits]

dfrib added a comment.

In D126818#3935740 , @rjmccall wrote:

> I'm too often slow to actually apply edits to the ABI document.  There's been 
> plenty of time for feedback on this one; go ahead and act like it's accepted.

CWG 2596 was discussed at Kona and, afaict, CWG is opting for a path of least 
effort, with a different result  than 
what is implemented this patch and previously discussed in the ABI issue 
:

> **CWG 2022-11-10**
>
> The friend definitions should conflict with friend definitions from other 
> instantiations of the same class template, consistent with how 
> non-constrained friends would work. Note that the enclosing dependent class 
> type does not appear in the friend function's signature, which is unusual.




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

https://reviews.llvm.org/D126818

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

[PATCH] D127462: [Clang] Begin implementing Plan 9 C extensions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D127462#3939243 , @ksaunders wrote:

> Hi Aaron. Unfortunately, I don't feel I can make a great case for why these 
> extensions should be in Clang. Although there are users of Plan 9 C 
> extensions, I don't see these features being adopted more generally enough to 
> warrant its inclusion in Clang which violates the inclusion policy.

Just to check -- do you think (some of) these features are something you wish 
to propose to WG14 for adoption into C? e.g., are you aiming to get multiple 
compilers to implement Plan 9 extensions to demonstrate to WG14 that this is 
existing practice in C compilers?

> To this effect, I tried using libTooling to rewrite Plan 9 C to standard C 
> that can be correctly compiled with Clang, but because the AST creation 
> requires semantic analysis to run it leaves the AST in a state of disrepair 
> (it can parse Plan 9 C, but the analyzer gets confused with duplicate fields 
> and so on).
>
> I'll have to decide if I am going to keep these changes in a Clang fork or 
> modify another C compiler for LLVM. Regardless, I believe my diffs for adding 
> the Plan 9 calling convention to LLVM still apply (they are simple), so I 
> will send them upstream when I feel they are ready.

SGTM

> ---
>
> I think it also makes sense to address your questions here for the sake of 
> completeness.

Thank you, I appreciate the education. :-)

>> I'm wondering if you could go into a bit more detail about what Automatic 
>> embedded structure type decay and Embedded structure type name accesses mean 
>> in practice (some code examples would be helpful).
>
> Absolutely. "Automatic embedded structure type decay" and "Embedded structure 
> type name accesses" are features best described by example:
>
>   typedef struct Lock Lock;
>   typedef struct Rc Rc;
>   typedef struct Resource Resource;
>   
>   struct Lock
>   {
> int hold;
>   };
>   
>   struct Rc
>   {
> int references;
>   }:
>   
>   struct Resource
>   {
> Rc;
> Lock;
> void *buffer;
> size_t size;
>   };
>
> Now with "Embedded structure type name accesses" enabled, if we have a value 
> like `Resource *r`, we can do `r->Lock`. This simply returns the field as if 
> `Lock;` was declared as `Lock Lock;`, but this special declaration also 
> brings all names into scope (like an anonymous struct) so we can do 
> `r->hold`. This also does NOT work if you declare the field as `struct 
> Lock;`, it must be a typedef name.

What an interesting extension! What happens with something like this?

  typedef struct Lock FirstLock;
  typedef struct Lock SecondLock;
  typedef struct Rc Rc;
  typedef struct Resource Resource;
  
  struct Lock
  {
int hold;
  };
   
  struct Rc
  {
int references;
  };
  
  struct Resource
  {
Rc;
FirstLock;
SecondLock;
void *buffer;
size_t size;
  };

Does this work for accessing `r->FirstLock` but give an ambiguous lookup for 
`r->hold`? Or do you only allow one member of the underlying canonical type?

Also, why does it require a typedef name?

> Further, with "Automatic embedded structure type decay" structure pointers 
> are automatically converted into an access of an embedded compatible 
> structure. So we have a function like: `void lock(Lock *);` we can call it 
> with `lock(r);` and the compiler will automatically search all unnamed 
> structures in `Resource` recursively until it finds a matching type. Note 
> that `Lock;` is declared after `Rc;`, this is intentional. In standard C it 
> is possible to have a pointer to a struct declay to a pointer to the first 
> field in the struct. That is completely separate from this extension.

Ah, interesting. So this is another case where multiple members of the same 
type would be a problem.  Does this only find structure/union members, or does 
this also work for other members? e.g. `void size(size_t *)` being called with 
`lock(r)`? And if it works for other members... what does it do for bit-field 
members which share an allocation unit?

> If that was unclear, GCC also supports this functionality and it is 
> documented here for a different explanation: 
> https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
>
>> Are you planning to add a new driver for Clang to emulate the Plan 9 
>> compiler driver (similar to how we have a driver for MSVC compatibility)?
>
> For now, no.
>
> Adding the Plan 9 object format to LLD is out-of-scope for this project (this 
> was discussed previously 
> ) so I 
> don't think it's necessary to add a new driver, we can just use the generic 
> ELF driver.
>
> Similarly, adding the Plan 9 assembler syntax is not necessary either as most 
> programs are C so the assembler can be trivially converted as the idea is 
> that programs will be compiled with the Plan 9 calling convention and C ABI.
>
>> Are there other extensions planned, or

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D124351#3940890 , @cor3ntin wrote:

> In D124351#3940888 , @aaron.ballman 
> wrote:
>
>> In D124351#3940786 , @cor3ntin 
>> wrote:
>>
>>> Now that Kona is over, I'm hoping to get back to that in the coming weeks. 
>>> Rebasing will be... fun.
>>> I want to make sure we are all okay making that a DR following WG21 
>>> guidance, given that not making it a DR would have potentially large impact 
>>> on the PR.
>>> Thanks!
>>
>> To me, it depends on how much existing code the DR breaks in practice. If we 
>> don't expect it to break a significant body of code (or if the code that 
>> breaks will be materially improved as a result of the required changes), 
>> then we should implement it as a DR as far back as we can go.
>
> We expect very little, if any, breakage

Then I think we should approach it as a DR and we can react if we get further 
feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D137818: Add support for querying SubstTemplateTypeParm types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D137818#3939213 , @anderslanglands 
wrote:

> Arg just noticed an issue: clang_Type_getReplacementType should be in the 
> LLVM_16 block in libclang.map not in the LLVM_13 block. I've blown away my 
> local branch so don't know how to fix the patch. Shall I just make a new 
> patch once this is merged?

In Phabricator, you can click the "Download Raw Diff" link (right-hand side of 
the page, near-ish the top) to get the patch back; you should be able to use 
that to get your local branch back to the same state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137818

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

[clang] 281a5c7 - [llvm,polly,clang] Stop setting config.enable_shared in most places

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2022-11-21T08:54:14-05:00
New Revision: 281a5c7ef112d5d09dd947ecd3f85484047c5502

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

LOG: [llvm,polly,clang] Stop setting config.enable_shared in most places

Clang's lit.cfg.py reads this to add an "enable-shared" feature that
three of clang's lit tests use. Nothing else reads enable_shared, so
remove it from most lit.site.cfg.py.in files.

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

Added: 


Modified: 
clang/test/Unit/lit.site.cfg.py.in
llvm/test/Unit/lit.site.cfg.py.in
llvm/test/lit.site.cfg.py.in
llvm/utils/gn/secondary/clang/test/BUILD.gn
llvm/utils/gn/secondary/llvm/test/BUILD.gn
polly/test/Unit/lit.site.cfg.in

Removed: 




diff  --git a/clang/test/Unit/lit.site.cfg.py.in 
b/clang/test/Unit/lit.site.cfg.py.in
index 778ab2ef05156..842560dd99ea5 100644
--- a/clang/test/Unit/lit.site.cfg.py.in
+++ b/clang/test/Unit/lit.site.cfg.py.in
@@ -8,7 +8,6 @@ config.llvm_tools_dir = 
lit_config.substitute(path(r"@LLVM_TOOLS_DIR@"))
 config.llvm_libs_dir = lit_config.substitute(path(r"@LLVM_LIBS_DIR@"))
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.clang_obj_root = path(r"@CLANG_BINARY_DIR@")
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = lit_config.substitute(path(r"@SHLIBDIR@"))
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 

diff  --git a/llvm/test/Unit/lit.site.cfg.py.in 
b/llvm/test/Unit/lit.site.cfg.py.in
index 9535dde62aee9..1d7d765801494 100644
--- a/llvm/test/Unit/lit.site.cfg.py.in
+++ b/llvm/test/Unit/lit.site.cfg.py.in
@@ -6,7 +6,6 @@ config.llvm_src_root = path(r"@LLVM_SOURCE_DIR@")
 config.llvm_obj_root = path(r"@LLVM_BINARY_DIR@")
 config.llvm_tools_dir = lit_config.substitute(path(r"@LLVM_TOOLS_DIR@"))
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = lit_config.substitute(path(r"@SHLIBDIR@"))
 
 # Let the main config do the real work.

diff  --git a/llvm/test/lit.site.cfg.py.in b/llvm/test/lit.site.cfg.py.in
index 5531732e16bc3..a23355fbf24a1 100644
--- a/llvm/test/lit.site.cfg.py.in
+++ b/llvm/test/lit.site.cfg.py.in
@@ -22,7 +22,6 @@ config.ocamlfind_executable = "@OCAMLFIND@"
 config.have_ocamlopt = @HAVE_OCAMLOPT@
 config.ocaml_flags = "@OCAMLFLAGS@"
 config.ptxas_executable = "@PTXAS_EXECUTABLE@"
-config.enable_shared = @ENABLE_SHARED@
 config.enable_assertions = @ENABLE_ASSERTIONS@
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.native_target = "@LLVM_NATIVE_ARCH@"

diff  --git a/llvm/utils/gn/secondary/clang/test/BUILD.gn 
b/llvm/utils/gn/secondary/clang/test/BUILD.gn
index 63eb4f296b6c8..d7d1be5f5bd9b 100644
--- a/llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -19,7 +19,6 @@ template("write_lit_config") {
   "CLANG_BINARY_DIR=" +
   rebase_path(get_label_info("//clang", "target_out_dir"), dir),
   "CLANG_SOURCE_DIR=" + rebase_path("//clang", dir),
-  "ENABLE_SHARED=0",
   "LLVM_BINARY_DIR=" +
   rebase_path(get_label_info("//llvm", "target_out_dir"), dir),
   "LLVM_LIBS_DIR=",  # needed only for shared builds
@@ -61,6 +60,7 @@ write_lit_config("lit_site_cfg") {
 "CMAKE_CXX_COMPILER=c++",
 "CMAKE_C_COMPILER=cc",
 "ENABLE_BACKTRACES=1",
+"ENABLE_SHARED=0",
 "LLVM_ENABLE_ZSTD=0",
 "LLVM_EXTERNAL_LIT=",
 "LLVM_HOST_TRIPLE=$llvm_current_triple",

diff  --git a/llvm/utils/gn/secondary/llvm/test/BUILD.gn 
b/llvm/utils/gn/secondary/llvm/test/BUILD.gn
index 38729e0f7e19c..03ffe11607040 100644
--- a/llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -21,7 +21,6 @@ template("write_lit_config") {
 values = [
   "LIT_SITE_CFG_IN_HEADER=" +
   "## Autogenerated from $input, do not edit\n\n" + lit_path_function,
-  "ENABLE_SHARED=0",
   "LLVM_BINARY_DIR=" +
   rebase_path(get_label_info("//llvm", "target_out_dir"), dir),
   "LLVM_SOURCE_DIR=" + rebase_path("//llvm", dir),

diff  --git a/polly/test/Unit/lit.site.cfg.in b/polly/test/Unit/lit.site.cfg.in
index 75420db8d30c7..2aeaf197f06cf 100644
--- a/polly/test/Unit/lit.site.cfg.in
+++ b/polly/test/Unit/lit.site.cfg.in
@@ -9,7 +9,6 @@ config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"



___
cfe-commits mailing list
cfe-com

[PATCH] D138301: [llvm, polly, clang] Stop setting config.enable_shared in most places

[Nico Weber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG281a5c7ef112: [llvm,polly,clang] Stop setting 
config.enable_shared in most places (authored by thakis).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138301

Files:
  clang/test/Unit/lit.site.cfg.py.in
  llvm/test/Unit/lit.site.cfg.py.in
  llvm/test/lit.site.cfg.py.in
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn
  polly/test/Unit/lit.site.cfg.in


Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -9,7 +9,6 @@
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -21,7 +21,6 @@
 values = [
   "LIT_SITE_CFG_IN_HEADER=" +
   "## Autogenerated from $input, do not edit\n\n" + lit_path_function,
-  "ENABLE_SHARED=0",
   "LLVM_BINARY_DIR=" +
   rebase_path(get_label_info("//llvm", "target_out_dir"), dir),
   "LLVM_SOURCE_DIR=" + rebase_path("//llvm", dir),
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -19,7 +19,6 @@
   "CLANG_BINARY_DIR=" +
   rebase_path(get_label_info("//clang", "target_out_dir"), dir),
   "CLANG_SOURCE_DIR=" + rebase_path("//clang", dir),
-  "ENABLE_SHARED=0",
   "LLVM_BINARY_DIR=" +
   rebase_path(get_label_info("//llvm", "target_out_dir"), dir),
   "LLVM_LIBS_DIR=",  # needed only for shared builds
@@ -61,6 +60,7 @@
 "CMAKE_CXX_COMPILER=c++",
 "CMAKE_C_COMPILER=cc",
 "ENABLE_BACKTRACES=1",
+"ENABLE_SHARED=0",
 "LLVM_ENABLE_ZSTD=0",
 "LLVM_EXTERNAL_LIT=",
 "LLVM_HOST_TRIPLE=$llvm_current_triple",
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -22,7 +22,6 @@
 config.have_ocamlopt = @HAVE_OCAMLOPT@
 config.ocaml_flags = "@OCAMLFLAGS@"
 config.ptxas_executable = "@PTXAS_EXECUTABLE@"
-config.enable_shared = @ENABLE_SHARED@
 config.enable_assertions = @ENABLE_ASSERTIONS@
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.native_target = "@LLVM_NATIVE_ARCH@"
Index: llvm/test/Unit/lit.site.cfg.py.in
===
--- llvm/test/Unit/lit.site.cfg.py.in
+++ llvm/test/Unit/lit.site.cfg.py.in
@@ -6,7 +6,6 @@
 config.llvm_obj_root = path(r"@LLVM_BINARY_DIR@")
 config.llvm_tools_dir = lit_config.substitute(path(r"@LLVM_TOOLS_DIR@"))
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = lit_config.substitute(path(r"@SHLIBDIR@"))
 
 # Let the main config do the real work.
Index: clang/test/Unit/lit.site.cfg.py.in
===
--- clang/test/Unit/lit.site.cfg.py.in
+++ clang/test/Unit/lit.site.cfg.py.in
@@ -8,7 +8,6 @@
 config.llvm_libs_dir = lit_config.substitute(path(r"@LLVM_LIBS_DIR@"))
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.clang_obj_root = path(r"@CLANG_BINARY_DIR@")
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = lit_config.substitute(path(r"@SHLIBDIR@"))
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 


Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -9,7 +9,6 @@
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
-config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -21,7 +21,6 @@
 values = [
   "LIT_SITE_CFG_IN_HEADER=" +
   "## Autogenerated from $input, do not edit\n\n" + lit_path_function,
-  "ENABLE_SH

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

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

Relevant issue: https://github.com/clangd/clangd/issues/1387


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138425

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
@@ -459,12 +459,13 @@
   foo(args...);
 }
 template <>
-void bar(int b);
+void bar<$par0[[int]]>(int b);
 void baz() {
-  bar($param[[42]]);
+  bar($par1[[42]]);
 }
   )cpp",
-   ExpectedHint{"b: ", "param"});
+   ExpectedHint{"Args[0]: ", "par0"},
+   ExpectedHint{"b: ", "par1"});
 }
 
 TEST(ParameterHints, VariadicNameFromSpecializationRecursive) {
@@ -481,12 +482,13 @@
   foo(args...);
 }
 template <>
-void foo(int b);
+void foo<$par0[[int]]>(int b);
 void baz() {
-  bar($param[[42]]);
+  bar($par1[[42]]);
 }
   )cpp",
-   ExpectedHint{"b: ", "param"});
+   ExpectedHint{"Args[0]: ", "par0"},
+   ExpectedHint{"b: ", "par1"});
 }
 
 TEST(ParameterHints, VariadicOverloaded) {
@@ -671,13 +673,13 @@
   }
 };
 void foo() {
-  container c;
+  container<$param0[[S]]> c;
   c.emplace($param1[[1]]);
   c.emplace($param2[[2]], $param3[[3]]);
 }
   )cpp",
-  ExpectedHint{"A: ", "param1"}, ExpectedHint{"B: ", "param2"},
-  ExpectedHint{"C: ", "param3"});
+  ExpectedHint{"T: ", "param0"}, ExpectedHint{"A: ", "param1"},
+  ExpectedHint{"B: ", "param2"}, ExpectedHint{"C: ", "param3"});
 }
 
 TEST(ParameterHints, VariadicReferenceHint) {
@@ -1113,6 +1115,70 @@
   EXPECT_EQ(hintsOfKind(*AST, InlayHintKind::Parameter).size(), 0u);
 }
 
+TEST(ParameterHints, TemplateSpecialization) {
+  assertParameterHints(
+  R"cpp(
+template  struct A {};
+template  struct B {};
+template  struct C {};
+template  struct D {};
+
+template 
+using E = A;
+template 
+using F = A<$par0[[U]]>;
+template 
+using G = C<$par1[[int]], $par2[[float]], Ts...>;
+
+B b;
+D<$par3[[int]], $par4[[float]]> d;
+  )cpp",
+  ExpectedHint{"T: ", "par0"}, ExpectedHint{"Ts[0]: ", "par1"},
+  ExpectedHint{"Ts[1]: ", "par2"}, ExpectedHint{"[0]: ", "par3"},
+  ExpectedHint{"[1]: ", "par4"});
+}
+
+TEST(ParameterHints, ConceptSpecialization) {
+  assertParameterHints(R"cpp(
+template  concept A = true;
+bool a = A<$par0[[int]]>;
+  )cpp",
+   ExpectedHint{"T: ", "par0"});
+}
+
+TEST(ParameterHints, ClassSpecialization) {
+  assertParameterHints(R"cpp(
+template  struct A {};
+template  struct A<$par0[[int]], $par1[[T]]> {};
+template <> struct A<$par2[[int]], $par3[[float]]> {};
+  )cpp",
+   ExpectedHint{"T: ", "par0"}, ExpectedHint{"U: ", "par1"},
+   ExpectedHint{"T: ", "par2"},
+   ExpectedHint{"U: ", "par3"});
+}
+
+TEST(ParameterHints, VarSpecialization) {
+  assertParameterHints(R"cpp(
+template 
+constexpr int value = 0;
+template 
+constexpr int value<$par0[[int]], $par1[[T]]> = 0;
+template <>
+constexpr int value<$par2[[int]], $par3[[float]]> = 0;
+   )cpp",
+   ExpectedHint{"T: ", "par0"}, ExpectedHint{"U: ", "par1"},
+   ExpectedHint{"T: ", "par2"},
+   ExpectedHint{"U: ", "par3"});
+}
+
+TEST(ParameterHints, FunctionSpecialization) {
+  assertParameterHints(R"cpp(
+template  T add(T lhs, T rhs);
+template <> int add<$par0[[int]]>(int lhs, int rhs);
+  )cpp",
+   ExpectedHint{"T: ", "par0"});
+}
+
 TEST(TypeHints, Smoke) {
   assertTypeHints(R"cpp(
 auto $waldo[[waldo]] = 42;
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -266,6 +266,11 @@
   addReturnTypeHint(D, FTL.getRParenLoc());
   }
 }
+if (D->isFunctionTemplateSpecialization()) {
+  auto TP = D->getPrimaryTemplate()->getTemplateParameters()->asArray();
+  if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
+addTemplateArgHint(TP, Args->arguments());
+}
 return true;
   }
 
@@ -370,11 +375,80 @@
 return true;
   }
 
+  bool VisitTemplateSpecializationTypeLoc(Templat

[PATCH] D138402: [clang-format] Correctly count a tab's width in a comment

[Emilia Dreamer via Phabricator via cfe-commits]

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

The behaviour of tabs still doesn't match that of spaces, but at least this 
makes it idempotent.

This also fixes https://github.com/llvm/llvm-project/issues/51808, I'm closing 
it as duplicate in favor of the issue fixed by this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138402

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

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added a comment.

Thanks for your feedback @rnk! I hope some of the ObjCARCOpts authors will 
share their opinions as well.




Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:767
+const ColorVector &CV = BlockColors.find(BB)->second;
+assert(CV.size() == 1 && "non-unique color for block!");
+BasicBlock *EHPadBB = CV.front();

rnk wrote:
> I don't think the verifier changes you made guarantee this. I would consider 
> strengthening this to `report_fatal_error`, since valid IR transforms can 
> probably reach this code.
Right, I guess the Verifier change is correct and this should change to work 
for multi-color BBs?
```
  assert(CV.size() > 0 && "Uncolored block");
  for (BasicBlock *EHPadBB : CV)
if (auto *EHPad = 
dyn_cast(ColorFirstBB->getFirstNonPHI())) {
  OpBundles.emplace_back("funclet", EHPad);
  break;
}
```

There is no test that covers this case, but it appears that the unique color 
invariant only holds **after** WinEHPrepare. [The assertion 
here](https://github.com/llvm/llvm-project/blob/release/15.x/llvm/lib/CodeGen/WinEHPrepare.cpp#L185)
 seems to imply it:
```
assert(BBColors.size() == 1 && "multi-color BB not removed by preparation");
```

Since it would be equivalent to the Verifier check, I'd stick with the 
assertion and not report a fatal error.



Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:2040-2041
+  DenseMap BlockColors;
+  if (F.hasPersonalityFn() &&
+  isScopedEHPersonality(classifyEHPersonality(F.getPersonalityFn(
+BlockColors = colorEHFunclets(F);

rnk wrote:
> I think this code snippet probably deserves a Function helper, 
> `F->hasScopedEHPersonality()`. Another part of me thinks we should cache the 
> personality enum similar to the way we cache intrinsic ids, but I wouldn't 
> want to increase Function memory usage, so that seems premature. "No action 
> required", I guess.
It doesn't really fit the the scope of this patch but I am happy to add the 
helper function in a follow-up (for now without personality enum caching).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137944

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

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

This patch implements:

- Template specialization type

- Class template specialization declaration

- Variable template specialization declaration

- Function specialization declaration

It would be sweet to also implement:

  template  constexpr int value = 0;
  int I = value;
  
  template  T add(T lhs, T rhs);
  add(1, 2);

But I got stuck on implementing these. I tried `VisitDeclRefExpr` but failed 
and have no idea. I prefer to implement these in future patches. But yes, if 
anyone could give me some hints I am happy to update the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[PATCH] D138426: Fix #58958 on github

[Alexey Kreshchuk via Phabricator via cfe-commits]

krsch created this revision.
krsch added reviewers: NoQ, xazax.hun.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
krsch requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Do not suggest to take the address of a const pointer to get void*


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138426

Files:
  clang/lib/Sema/SemaFixItUtils.cpp
  clang/test/FixIt/fixit-function-call.cpp


Index: clang/test/FixIt/fixit-function-call.cpp
===
--- clang/test/FixIt/fixit-function-call.cpp
+++ clang/test/FixIt/fixit-function-call.cpp
@@ -115,4 +115,16 @@
   u(c);
 }
 
+void accept_void(void*);
+
+void issue58958(const char* a) {
+// CHECK: no matching function for call to 'accept_void'
+// CHECK-NOT: take the address of the argument with &
+accept_void(a);
+char b;
+// CHECK: no matching function for call to 'accept_void'
+// CHECK: take the address of the argument with &
+accept_void(b);
+}
+
 // CHECK: errors generated
Index: clang/lib/Sema/SemaFixItUtils.cpp
===
--- clang/lib/Sema/SemaFixItUtils.cpp
+++ clang/lib/Sema/SemaFixItUtils.cpp
@@ -132,6 +132,13 @@
 if (!Expr->isLValue() || Expr->getObjectKind() != OK_Ordinary)
   return false;
 
+// Do no take address of const pointer to get void*
+const PointerType *FromPtrTy = dyn_cast(FromQTy);
+const PointerType *ToPtrTy = dyn_cast(ToQTy);
+if (FromPtrTy && FromPtrTy->getPointeeType().isConstQualified() &&
+ToPtrTy->isVoidPointerType())
+  return false;
+
 CanConvert = CompareTypes(S.Context.getPointerType(FromQTy), ToQTy, S,
   Begin, VK_PRValue);
 if (CanConvert) {


Index: clang/test/FixIt/fixit-function-call.cpp
===
--- clang/test/FixIt/fixit-function-call.cpp
+++ clang/test/FixIt/fixit-function-call.cpp
@@ -115,4 +115,16 @@
   u(c);
 }
 
+void accept_void(void*);
+
+void issue58958(const char* a) {
+// CHECK: no matching function for call to 'accept_void'
+// CHECK-NOT: take the address of the argument with &
+accept_void(a);
+char b;
+// CHECK: no matching function for call to 'accept_void'
+// CHECK: take the address of the argument with &
+accept_void(b);
+}
+
 // CHECK: errors generated
Index: clang/lib/Sema/SemaFixItUtils.cpp
===
--- clang/lib/Sema/SemaFixItUtils.cpp
+++ clang/lib/Sema/SemaFixItUtils.cpp
@@ -132,6 +132,13 @@
 if (!Expr->isLValue() || Expr->getObjectKind() != OK_Ordinary)
   return false;
 
+// Do no take address of const pointer to get void*
+const PointerType *FromPtrTy = dyn_cast(FromQTy);
+const PointerType *ToPtrTy = dyn_cast(ToQTy);
+if (FromPtrTy && FromPtrTy->getPointeeType().isConstQualified() &&
+ToPtrTy->isVoidPointerType())
+  return false;
+
 CanConvert = CompareTypes(S.Context.getPointerType(FromQTy), ToQTy, S,
   Begin, VK_PRValue);
 if (CanConvert) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D126818#3940898 , @dfrib wrote:

> In D126818#3935740 , @rjmccall 
> wrote:
>
>> I'm too often slow to actually apply edits to the ABI document.  There's 
>> been plenty of time for feedback on this one; go ahead and act like it's 
>> accepted.
>
> CWG 2596 was discussed at Kona and, afaict, CWG is opting for a path of least 
> effort, with a different result  than 
> what is implemented this patch and previously discussed in the ABI issue 
> :
>
>> **CWG 2022-11-10**
>>
>> The friend definitions should conflict with friend definitions from other 
>> instantiations of the same class template, consistent with how 
>> non-constrained friends would work. Note that the enclosing dependent class 
>> type does not appear in the friend function's signature, which is unusual.

Can you clarify the difference here?  What did we choose different?  The 
example from that CWG issue is exactly in the test for this (see the top of 
`useS`) so I'm not sure what difference we're missing? Can you clarify what I'm 
not matching here?


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

https://reviews.llvm.org/D126818

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

[clang] eb4373a - Remove -cc1 -fconcepts-ts flag

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2022-11-21T06:31:29-08:00
New Revision: eb4373abe829f8731b3e6d4da97ad88a9a93aa28

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

LOG: Remove -cc1 -fconcepts-ts flag

The -fconcepts-ts flag has been deprecated for 5 releases now, so just
remove it, concepts is supported in C++20 mode.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dcb5f132fce1b..b2b085409116a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -485,6 +485,8 @@ Modified Compiler Flags
 
 Removed Compiler Flags
 -
+- Clang now no longer supports ``-cc1 -fconcepts-ts``.  This flag has been 
deprecated
+  and encouraged use of ``-std=c++20`` since Clang 10, so we're now removing 
it.
 
 New Pragmas in Clang
 

diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 26083e3fc8d88..d0f672ae5a1bd 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -132,9 +132,6 @@ def err_fe_invalid_alignment : Error<
 "invalid value '%1' in '%0'; alignment must be a power of 2">;
 def err_fe_invalid_exception_model
: Error<"invalid exception model '%select{none|sjlj|seh|dwarf|wasm}0' for 
target '%1'">;
-def warn_fe_concepts_ts_flag : Warning<
-  "-fconcepts-ts is deprecated - use '-std=c++20' for Concepts support">,
-  InGroup;
 def err_fe_invalid_source_date_epoch : Error<
 "environment variable 'SOURCE_DATE_EPOCH' ('%0') must be a non-negative 
decimal integer <= %1">;
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d67dd6f99a9c4..8da5e25bd38d0 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5881,8 +5881,6 @@ def ftest_module_file_extension_EQ :
   Joined<["-"], "ftest-module-file-extension=">,
   HelpText<"introduce a module file extension for testing purposes. "
"The argument is parsed as blockname:major:minor:hashed:user info">;
-def fconcepts_ts : Flag<["-"], "fconcepts-ts">,
-  HelpText<"Enable C++ Extensions for Concepts. (deprecated - use 
-std=c++2a)">;
 
 defm recovery_ast : BoolOption<"f", "recovery-ast",
   LangOpts<"RecoveryAST">, DefaultTrue,

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f09d5e18c4c4d..a13da5a4e078c 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -491,9 +491,6 @@ static bool FixupInvocation(CompilerInvocation &Invocation,
   if (LangOpts.AppleKext && !LangOpts.CPlusPlus)
 Diags.Report(diag::warn_c_kext);
 
-  if (Args.hasArg(OPT_fconcepts_ts))
-Diags.Report(diag::warn_fe_concepts_ts_flag);
-
   if (LangOpts.NewAlignOverride &&
   !llvm::isPowerOf2_32(LangOpts.NewAlignOverride)) {
 Arg *A = Args.getLastArg(OPT_fnew_alignment_EQ);



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

[PATCH] D138425: [clangd] Parameter hints for template specialization

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

The code looks pretty good and this makes a lot of logical sense.

However, there's an ugly practical issue: it's overwhelmingly common for 
template parameters to have cryptic and useless names.
It seems like turning `vector` into `vector` actually makes 
things worse.

And this gets yet worse: because this is type `parameter` and controlled by 
that config option, so people can't disable this without disabling the (very 
practically useful) function param hints.

Have you been using this for a while? Do you find it to be a practical help? 
Spammy?




Comment at: clang-tools-extra/clangd/InlayHints.cpp:378
 
+  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
+auto TP = L.getTypePtr()

we need a bailout `if (!Cfg.InlayHints.Parameters)`



Comment at: clang-tools-extra/clangd/InlayHints.cpp:434
+break;
+  if (auto Name = TP[I]->getName(); shouldHintName(TA[I], Name))
+addInlayHint(TA[I].getSourceRange(), HintSide::Left,

we're missing some mangling of the name to remove the leading `_`
(stripLeadingUnderscore?)



Comment at: clang-tools-extra/clangd/InlayHints.cpp:553
+llvm::raw_string_ostream Out(ArgContent);
+TA.getArgument().print(AST.getPrintingPolicy(), Out, false);
+if (ArgContent == ParamName)

I think we should avoid running the printer on every arg. Instead probably 
switch over the types of templatearguments, and delegate to 
shouldHintName(Expr, handle TagType/Typedef/other common nodes specifically, as 
done in shouldHintName).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[PATCH] D138429: [clang][RISCV][NFC] Prevent data race in RVVType::computeType

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng created this revision.
kito-cheng added reviewers: ilya-biryukov, kadircet, sammccall, khchen.
Herald added subscribers: sunshaoce, VincentWu, vkmr, frasercrmck, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, arichardson.
Herald added a project: All.
kito-cheng requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

Introduce a RVVTypeCache to hold the cache instead of using a local
static variable to maintain a cache.

Also made construct of RVVType to private, make sure that could be only
created by a cache manager.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138429

Files:
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -95,6 +95,7 @@
 class RVVEmitter {
 private:
   RecordKeeper &Records;
+  RVVTypeCache TypeCache;
 
 public:
   RVVEmitter(RecordKeeper &R) : Records(R) {}
@@ -349,8 +350,8 @@
   constexpr int Log2LMULs[] = {-3, -2, -1, 0, 1, 2, 3};
   // Print RVV boolean types.
   for (int Log2LMUL : Log2LMULs) {
-auto T = RVVType::computeType(BasicType::Int8, Log2LMUL,
-  PrototypeDescriptor::Mask);
+auto T = TypeCache.computeType(BasicType::Int8, Log2LMUL,
+   PrototypeDescriptor::Mask);
 if (T)
   printType(T.value());
   }
@@ -358,10 +359,10 @@
   for (char I : StringRef("csil")) {
 BasicType BT = ParseBasicType(I);
 for (int Log2LMUL : Log2LMULs) {
-  auto T = RVVType::computeType(BT, Log2LMUL, PrototypeDescriptor::Vector);
+  auto T = TypeCache.computeType(BT, Log2LMUL, PrototypeDescriptor::Vector);
   if (T) {
 printType(T.value());
-auto UT = RVVType::computeType(
+auto UT = TypeCache.computeType(
 BT, Log2LMUL,
 PrototypeDescriptor(BaseTypeModifier::Vector,
 VectorTypeModifier::NoModifier,
@@ -372,8 +373,8 @@
   }
   OS << "#if defined(__riscv_zvfh)\n";
   for (int Log2LMUL : Log2LMULs) {
-auto T = RVVType::computeType(BasicType::Float16, Log2LMUL,
-  PrototypeDescriptor::Vector);
+auto T = TypeCache.computeType(BasicType::Float16, Log2LMUL,
+   PrototypeDescriptor::Vector);
 if (T)
   printType(T.value());
   }
@@ -381,8 +382,8 @@
 
   OS << "#if (__riscv_v_elen_fp >= 32)\n";
   for (int Log2LMUL : Log2LMULs) {
-auto T = RVVType::computeType(BasicType::Float32, Log2LMUL,
-  PrototypeDescriptor::Vector);
+auto T = TypeCache.computeType(BasicType::Float32, Log2LMUL,
+   PrototypeDescriptor::Vector);
 if (T)
   printType(T.value());
   }
@@ -390,8 +391,8 @@
 
   OS << "#if (__riscv_v_elen_fp >= 64)\n";
   for (int Log2LMUL : Log2LMULs) {
-auto T = RVVType::computeType(BasicType::Float64, Log2LMUL,
-  PrototypeDescriptor::Vector);
+auto T = TypeCache.computeType(BasicType::Float64, Log2LMUL,
+   PrototypeDescriptor::Vector);
 if (T)
   printType(T.value());
   }
@@ -553,14 +554,15 @@
   for (int Log2LMUL : Log2LMULList) {
 BasicType BT = ParseBasicType(I);
 Optional Types =
-RVVType::computeTypes(BT, Log2LMUL, NF, Prototype);
+TypeCache.computeTypes(BT, Log2LMUL, NF, Prototype);
 // Ignored to create new intrinsic if there are any illegal types.
 if (!Types)
   continue;
 
-auto SuffixStr = RVVIntrinsic::getSuffixStr(BT, Log2LMUL, SuffixDesc);
-auto OverloadedSuffixStr =
-RVVIntrinsic::getSuffixStr(BT, Log2LMUL, OverloadedSuffixDesc);
+auto SuffixStr =
+RVVIntrinsic::getSuffixStr(TypeCache, BT, Log2LMUL, SuffixDesc);
+auto OverloadedSuffixStr = RVVIntrinsic::getSuffixStr(
+TypeCache, BT, Log2LMUL, OverloadedSuffixDesc);
 // Create a unmasked intrinsic
 Out.push_back(std::make_unique(
 Name, SuffixStr, OverloadedName, OverloadedSuffixStr, IRName,
@@ -576,7 +578,7 @@
 /*HasMaskedOffOperand=*/false, HasVL, NF,
 IsPrototypeDefaultTU, UnMaskedPolicyScheme, P);
 Optional PolicyTypes =
-RVVType::computeTypes(BT, Log2LMUL, NF, PolicyPrototype);
+TypeCache.computeTypes(BT, Log2LMUL, NF, PolicyPrototype);
 Out.push_

[PATCH] D138287: [clang][RISCV] Drop caching from RVVType as it introduces data races

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Proposed fix: D138429 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138287

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

[PATCH] D124730: [RISCV][NFC] Refactor RISC-V vector intrinsic utils.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

@kadircet Proposed fix: D138429 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124730

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

[PATCH] D138434: [Clang][Sema] Added space after ',' in a warning

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar created this revision.
Herald added a project: All.
fahadnayyar requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change fixes a typo in a warning message.

rdar://79707705


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138434

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaObjC/atomic-property-synthesis-rules.m


Index: clang/test/SemaObjC/atomic-property-synthesis-rules.m
===
--- clang/test/SemaObjC/atomic-property-synthesis-rules.m
+++ clang/test/SemaObjC/atomic-property-synthesis-rules.m
@@ -108,7 +108,7 @@
 // read-write - might warn
 @property int GetSet;
 @property int Get; // expected-note {{property declared here}} \
-// expected-note {{setter and getter must both be 
synthesized}}
+// expected-note {{setter and getter must both be 
synthesized, or both be user defined, or the property must be nonatomic}}
 @property int Set; // expected-note {{property declared here}} \
 // expected-note {{setter and getter must both be 
synthesized}}
 @property int None;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1326,7 +1326,7 @@
   "with a user defined %select{getter|setter}2">,
   InGroup>;
 def note_atomic_property_fixup_suggest : Note<"setter and getter must both be "
-  "synthesized, or both be user defined,or the property must be nonatomic">;
+  "synthesized, or both be user defined, or the property must be nonatomic">;
 def err_atomic_property_nontrivial_assign_op : Error<
   "atomic property of reference type %0 cannot have non-trivial assignment"
   " operator">;


Index: clang/test/SemaObjC/atomic-property-synthesis-rules.m
===
--- clang/test/SemaObjC/atomic-property-synthesis-rules.m
+++ clang/test/SemaObjC/atomic-property-synthesis-rules.m
@@ -108,7 +108,7 @@
 // read-write - might warn
 @property int GetSet;
 @property int Get;	// expected-note {{property declared here}} \
-// expected-note {{setter and getter must both be synthesized}}
+// expected-note {{setter and getter must both be synthesized, or both be user defined, or the property must be nonatomic}}
 @property int Set;	// expected-note {{property declared here}} \
 // expected-note {{setter and getter must both be synthesized}}
 @property int None;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1326,7 +1326,7 @@
   "with a user defined %select{getter|setter}2">,
   InGroup>;
 def note_atomic_property_fixup_suggest : Note<"setter and getter must both be "
-  "synthesized, or both be user defined,or the property must be nonatomic">;
+  "synthesized, or both be user defined, or the property must be nonatomic">;
 def err_atomic_property_nontrivial_assign_op : Error<
   "atomic property of reference type %0 cannot have non-trivial assignment"
   " operator">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138429: [clang][RISCV][NFC] Prevent data race in RVVType::computeType

[Sam McCall via Phabricator via cfe-commits]

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

LG from a thread-safety perspective, thanks!

Approving because it looks mechanically "obviously correct", but you may want 
to wait for another reviewer to confirm the API changes are sane (I don't 
really know what this code does or how it's used).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138429

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

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 476892.
yaxunl added a comment.
Herald added subscribers: kerbowa, jvesely.

add test emulation-amdgpu.s


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

https://reviews.llvm.org/D138221

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-device-only.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  lld/ELF/Driver.cpp
  lld/test/ELF/emulation-amdgpu.s

Index: lld/test/ELF/emulation-amdgpu.s
===
--- /dev/null
+++ lld/test/ELF/emulation-amdgpu.s
@@ -0,0 +1,36 @@
+# REQUIRES: amdgpu
+
+# RUN: llvm-mc -filetype=obj -triple=amdgcn-amd-amdhsa %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: llvm-readobj --file-headers %t | FileCheck %s
+# RUN: ld.lld -m elf64_amdgpu %t.o -o %t
+# RUN: llvm-readobj --file-headers %t | FileCheck %s
+
+# CHECK:  ElfHeader {
+# CHECK-NEXT:   Ident {
+# CHECK-NEXT: Magic: (7F 45 4C 46)
+# CHECK-NEXT: Class: 64-bit (0x2)
+# CHECK-NEXT: DataEncoding: LittleEndian (0x1)
+# CHECK-NEXT: FileVersion: 1
+# CHECK-NEXT: OS/ABI: AMDGPU_HSA (0x40)
+# CHECK-NEXT: ABIVersion: 2
+# CHECK-NEXT: Unused: (00 00 00 00 00 00 00)
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Type: Executable (0x2)
+# CHECK-NEXT:   Machine: EM_AMDGPU (0xE0)
+# CHECK-NEXT:   Version: 1
+# CHECK-NEXT:   Entry:
+# CHECK-NEXT:   ProgramHeaderOffset: 0x40
+# CHECK-NEXT:   SectionHeaderOffset:
+# CHECK-NEXT:   Flags [ (0x0)
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   HeaderSize: 64
+# CHECK-NEXT:   ProgramHeaderEntrySize: 56
+# CHECK-NEXT:   ProgramHeaderCount:
+# CHECK-NEXT:   SectionHeaderEntrySize: 64
+# CHECK-NEXT:   SectionHeaderCount:
+# CHECK-NEXT:   StringTableSectionIndex:
+# CHECK-NEXT: }
+
+.globl _start
+_start:
Index: lld/ELF/Driver.cpp
===
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -176,12 +176,15 @@
   .Case("elf_iamcu", {ELF32LEKind, EM_IAMCU})
   .Case("elf64_sparc", {ELF64BEKind, EM_SPARCV9})
   .Case("msp430elf", {ELF32LEKind, EM_MSP430})
+  .Case("elf64_amdgpu", {ELF64LEKind, EM_AMDGPU})
   .Default({ELFNoneKind, EM_NONE});
 
   if (ret.first == ELFNoneKind)
 error("unknown emulation: " + emul);
   if (ret.second == EM_MSP430)
 osabi = ELFOSABI_STANDALONE;
+  else if (ret.second == EM_AMDGPU)
+osabi = ELFOSABI_AMDGPU_HSA;
   return std::make_tuple(ret.first, ret.second, osabi);
 }
 
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -59,7 +59,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 //
@@ -82,7 +82,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 //
@@ -122,7 +122,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_803:.*out]]" [[OBJ_DEV_B_803]]
 
 //
@@ -145,7 +145,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_900:.*out]]" [[OBJ_DEV_B_900]]
 
 //
Index: clang/test/Driver/hip-toolchain-device-only.hip
===
--- clang/test/Driver/hip-toolchain-device-only.hip
+++ clang/test/Driver/hip-toolchain-device-only.hip
@@ -12,7 +12,7 @@
 // CHECK-SAME: "-target-cpu" "gfx803"
 // CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_803:".*o"]] "-x" "hip"
 
-// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa"
@@ -21,7 +21,7 @@
 // CHECK-SAME: "-target-cpu" "gfx900"
 // CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_900:".*o"]] "-x" "hip"
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[David Friberg via Phabricator via cfe-commits]

dfrib added a comment.

In D126818#3941036 , @erichkeane 
wrote:

> In D126818#3940898 , @dfrib wrote:
>
>> In D126818#3935740 , @rjmccall 
>> wrote:
>>
>>> I'm too often slow to actually apply edits to the ABI document.  There's 
>>> been plenty of time for feedback on this one; go ahead and act like it's 
>>> accepted.
>>
>> CWG 2596 was discussed at Kona and, afaict, CWG is opting for a path of 
>> least effort, with a different result 
>>  than what is implemented this patch 
>> and previously discussed in the ABI issue 
>> :
>>
>>> **CWG 2022-11-10**
>>>
>>> The friend definitions **should conflict with friend definitions from other 
>>> instantiations of the same class template**, consistent with how 
>>> non-constrained friends would work. Note that the enclosing dependent class 
>>> type does not appear in the friend function's signature, which is unusual.
>
> Can you clarify the difference here?  What did we choose different?  The 
> example from that CWG issue is exactly in the test for this (see the top of 
> `useS`) so I'm not sure what difference we're missing? Can you clarify what 
> I'm not matching here?

I wrote the CWG issue and the example therein indeed flags exactly what is 
being addressed by this patch, but I'm referring particularly to the comment 
added from CWGs Kona meeting 2022-11-10 (see now **emphasized** quote above). I 
may be mistaken, but I read it as proposing to make `useS` ill-formed, 
emphasizing that you need to include the class somewhere in the hidden friend's 
signature, whereas overloading as shown in the `useS` example would not be 
supported.

For more details, see the Kona minutes 

 at EDG-wiki.


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

https://reviews.llvm.org/D126818

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

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

[Tobias Grosser via Phabricator via cfe-commits]

grosser added a comment.

I just checked and I get the same error on a linux system. @nhaehnle, did you 
try an mlir build with the previously mentioned cmake options?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138278

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

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

ping




Comment at: clang/lib/CodeGen/CGCall.cpp:5402-5411
+if (SanOpts.has(SanitizerKind::ExceptionEscape) &&
+ExceptionEscapeUBLastInvokeSrcLoc) {
+  llvm::Constant *CheckSourceLocation = EmitCheckSourceLocation(Loc);
+  Builder.CreateStore(
+  CheckSourceLocation,
+  Address(ExceptionEscapeUBLastInvokeSrcLoc,
+  CheckSourceLocation->getType(),

lebedev.ri wrote:
> @rjmccall
> So there are two issues:
> 1. `getInvokeDest()` isn't necessarily called just before creating an 
> `invoke`, see e.g. `-EHa` handling in `CodeGenFunction::PopCleanupBlock()`
> 2. Even if we ignore that, we need to do this for *every* `invoke`, not just 
> those going to the our UB landing pad, consider: 
> https://godbolt.org/z/qTeKor41a <- the invoke leads to a normal landing pad, 
> yet we immediately rethrow the just-caught exception, and now end up in the 
> UB landing pad.
> 
> So i'm not really seeing an alternative path here?
@rjmccall do you agree with my reasoning here?
Perhaps there is some other solution that i'm not seeing,
other than not having the source location?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381

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

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added a comment.

In D138221#3939384 , @MaskRay wrote:

>> Some host relocatable objects may not contain device relocatable objects, 
>> where an empty file is passed to lld, which causes lld to fail.
>
> How is an empty file (size=0) passed to lld? If a dummy relocatable object 
> file is parsed to lld, lld can infer the machine type from `e_machine` in the 
> ELF header.
> How does it work in other cases? Because a non-empty relocatable object file 
> is used?
>
> I do not object to a new emulation which does not exist in GNU ld, but the 
> description needs to be clarified.

Updated description. The empty file is generated by clang-offload-bundler, 
which does not know how to create a dummy relocatable object file for a device 
target.




Comment at: lld/ELF/Driver.cpp:179
   .Case("msp430elf", {ELF32LEKind, EM_MSP430})
+  .Case("elf64_amdgpu", {ELF64LEKind, EM_AMDGPU})
   .Default({ELFNoneKind, EM_NONE});

MaskRay wrote:
> This needs an `emulation-amdgpu.s` test.
added


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

https://reviews.llvm.org/D138221

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

[PATCH] D138425: [clangd] Parameter hints for template specialization

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@sammccall Thank you for reviewing and giving suggestions!

I must admit I didn't use it for very long. But I do think this is helpful, at 
least for templates I'm unfamiliar with.

Yes, there is a common situation where people use a meaningless template 
parameter name, but I think the same for functions. I have seen many 
meaningless parameter names like `D`, `E` even in the LLVM codebase. Since we 
can tolerate these, why can't we bear the template parameter?

And yes, it is a serious problem this is unconfigurable.




Comment at: clang-tools-extra/clangd/InlayHints.cpp:434
+break;
+  if (auto Name = TP[I]->getName(); shouldHintName(TA[I], Name))
+addInlayHint(TA[I].getSourceRange(), HintSide::Left,

sammccall wrote:
> we're missing some mangling of the name to remove the leading `_`
> (stripLeadingUnderscore?)
I don't think we should, since we don't do this for function parameters, are 
there any special reasons for us to do this for template parameters?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[PATCH] D138429: [clang][RISCV][NFC] Prevent data race in RVVType::computeType

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks! +1 from my side as well for thread-safety purposes (and I hope having 2 
separate caches for tablegen and sema doesn't have unexpected effects).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138429

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

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D126818#3941165 , @dfrib wrote:

> In D126818#3941036 , @erichkeane 
> wrote:
>
>> In D126818#3940898 , @dfrib wrote:
>>
>>> In D126818#3935740 , @rjmccall 
>>> wrote:
>>>
 I'm too often slow to actually apply edits to the ABI document.  There's 
 been plenty of time for feedback on this one; go ahead and act like it's 
 accepted.
>>>
>>> CWG 2596 was discussed at Kona and, afaict, CWG is opting for a path of 
>>> least effort, with a different result 
>>>  than what is implemented this patch 
>>> and previously discussed in the ABI issue 
>>> :
>>>
 **CWG 2022-11-10**

 The friend definitions **should conflict with friend definitions from 
 other instantiations of the same class template**, consistent with how 
 non-constrained friends would work. Note that the enclosing dependent 
 class type does not appear in the friend function's signature, which is 
 unusual.
>>
>> Can you clarify the difference here?  What did we choose different?  The 
>> example from that CWG issue is exactly in the test for this (see the top of 
>> `useS`) so I'm not sure what difference we're missing? Can you clarify what 
>> I'm not matching here?
>
> I wrote the CWG issue and the example therein indeed flags exactly what is 
> being addressed by this patch, but I'm referring particularly to the comment 
> added from CWGs Kona meeting 2022-11-10 (see now **emphasized** quote above). 
> I may be mistaken, but I read it as proposing to make `useS` ill-formed, 
> emphasizing that you need to include the class somewhere in the hidden 
> friend's signature, whereas overloading as shown in the `useS` example would 
> not be supported.
>
> For more details, see the Kona minutes 
> 
>  at EDG-wiki.

Hmm... I'm still a bit confused and hoping someone else can show up and let me 
know how to fix this.  There is quite a bit of work to do temp.friend p9 that I 
implemented to mean a constrained-non-template friend or a 
constrained-template-friend-that-refers-to-enclosing-scope don't conflict, 
based on the wording.

The minutes aren't particularly clear to me unfortunately as to what they 
really want, particularly since the suggested wording says the opposite of what 
I THOUGHT the discussion was doing at the end?  The wording/example are still 
consistent with my interpretation/implementation, so I have no idea what is 
going on.  We might find ourselves wanting to hold off until CWG comes up with 
actual wording?

The "Refers to an enclosing template" part seems sensible enough (and I have 
work in place to detect and store that), so the question is whether we want 
non-template friends to conflict ever.  IMO, the current rule (implemented in 
clang and here, where constrained non-template friends are somehow different) 
is a little odd-ball, but at least easy enough to reason about.

I think perhaps we need to wait on CWG to clarify what they mean, at least by 
including a wording consistent with that top thing.  The unfortunate part here 
is that Clang implements 1/2 of this at the moment: we implement the SEMA 
changes, but not the mangling changes for the current wording.


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

https://reviews.llvm.org/D126818

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

[PATCH] D138434: [Clang][Sema] Added space after ',' in a warning

[Egor Zhdan via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138434

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

[PATCH] D138425: [clangd] Parameter hints for template specialization

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D138425#3941183 , @v1nh1shungry 
wrote:

> @sammccall Thank you for reviewing and giving suggestions!
>
> I must admit I didn't use it for very long. But I do think this is helpful, 
> at least for templates I'm unfamiliar with.

That sounds good, maybe you can give some examples?
I'm thinking about a heuristic like "show hints where the param name is >2 
chars long" or something, at least as a starting point - would be good to see 
whether this would still provide the value you're seeing.

> Yes, there is a common situation where people use a meaningless template 
> parameter name, but I think the same for functions.

I don't think it's the same: my wild guess would be 30% of function params have 
useless names, and 90% of template params do.
If this were accurate, it seems function param hints are usually (potentially) 
useful, while template params are almost always useless.

> I have seen many meaningless parameter names like `D`, `E` even in the LLVM 
> codebase.

Agree. LLVM does this (much) more than the other codebases I'm familiar with.
I'd personally be very happy if we could somehow detect and suppress hints for 
such params.
But it's not clear-cut: `template class map` can be 
meaningful, and with push-to-show-hints interaction the lack of a hint can be 
confusing.

> Since we can tolerate these, why can't we bear the template parameter?

If template params are useless a larger fraction of the time, then enabling 
them without doing some filtering might be a net negative in practice.

> And yes, it is a serious problem this is unconfigurable.

We can add a new config category for these (use 
Config::InlayHints::TemplateParameters), it's just a bit ugly to do so.




Comment at: clang-tools-extra/clangd/InlayHints.cpp:434
+break;
+  if (auto Name = TP[I]->getName(); shouldHintName(TA[I], Name))
+addInlayHint(TA[I].getSourceRange(), HintSide::Left,

v1nh1shungry wrote:
> sammccall wrote:
> > we're missing some mangling of the name to remove the leading `_`
> > (stripLeadingUnderscore?)
> I don't think we should, since we don't do this for function parameters, are 
> there any special reasons for us to do this for template parameters?
We do this for function params, see line 574 of the LHS


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138425

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

[clang-tools-extra] fd733a6 - [clangd] Extend --check to time clang-tidy checks, so we can block slow ones

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-11-21T16:24:47+01:00
New Revision: fd733a65de5c37c8d5ec4fbe45c8f1a347816858

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

LOG: [clangd] Extend --check to time clang-tidy checks, so we can block slow 
ones

misc-const-correctness is so catastrophically slow that we need to block it
from running. But we need a way to detect this without breaking users first.

This is part of a plan to run only fast checks by default.
More details in  https://github.com/clangd/clangd/issues/1337

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

Added: 


Modified: 
clang-tools-extra/clangd/tool/Check.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index d216c9d08e89a..40a545e240d43 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -24,10 +24,13 @@
 //
 
//===--===//
 
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "CompileCommands.h"
 #include "Config.h"
+#include "Feature.h"
 #include "GlobalCompilationDatabase.h"
 #include "Hover.h"
 #include "InlayHints.h"
@@ -43,17 +46,25 @@
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
+// These will never be shown in --help, ClangdMain doesn't list the category.
+llvm::cl::opt CheckTidyTime(
+"check-tidy-time",
+llvm::cl::desc("Print the overhead of checks matching this glob"),
+llvm::cl::init(""));
+
 // Print (and count) the error-level diagnostics (warnings are ignored).
 unsigned showErrors(llvm::ArrayRef Diags) {
   unsigned ErrCount = 0;
@@ -66,6 +77,19 @@ unsigned showErrors(llvm::ArrayRef Diags) {
   return ErrCount;
 }
 
+std::vector listTidyChecks(llvm::StringRef Glob) {
+  tidy::GlobList G(Glob);
+  tidy::ClangTidyCheckFactories CTFactories;
+  for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+E.instantiate()->addCheckFactories(CTFactories);
+  std::vector Result;
+  for (const auto &E : CTFactories)
+if (G.contains(E.getKey()))
+  Result.push_back(E.getKey().str());
+  llvm::sort(Result);
+  return Result;
+}
+
 // This class is just a linear pipeline whose functions get called in sequence.
 // Each exercises part of clangd's logic on our test file and logs results.
 // Later steps depend on state built in earlier ones (such as the AST).
@@ -193,9 +217,100 @@ class Checker {
   log("Indexing AST...");
   Index.updateMain(File, *AST);
 }
+
+if (!CheckTidyTime.empty()) {
+  if (!CLANGD_TIDY_CHECKS) {
+elog("-{0} requires -DCLANGD_TIDY_CHECKS!", CheckTidyTime.ArgStr);
+return false;
+  }
+  checkTidyTimes();
+}
+
 return true;
   }
 
+  // For each check foo, we want to build with checks=-* and checks=-*,foo.
+  // (We do a full build rather than just AST matchers to meausre PPCallbacks).
+  //
+  // However, performance has both random noise and systematic changes, such as
+  // step-function slowdowns due to CPU scaling.
+  // We take the median of 5 measurements, and after every check discard the
+  // measurement if the baseline changed by >3%.
+  void checkTidyTimes() {
+double Stability = 0.03;
+log("Timing AST build with individual clang-tidy checks (target accuracy "
+"{0:P0})",
+Stability);
+
+using Duration = std::chrono::nanoseconds;
+// Measure time elapsed by a block of code. Currently: user CPU time.
+auto Time = [&](auto &&Run) -> Duration {
+  llvm::sys::TimePoint<> Elapsed;
+  std::chrono::nanoseconds UserBegin, UserEnd, System;
+  llvm::sys::Process::GetTimeUsage(Elapsed, UserBegin, System);
+  Run();
+  llvm::sys::Process::GetTimeUsage(Elapsed, UserEnd, System);
+  return UserEnd - UserBegin;
+};
+auto Change = [&](Duration Exp, Duration Base) -> double {
+  return (double)(Exp.count() - Base.count()) / Base.count();
+};
+// Build ParsedAST with a fixed check glob, and return the time taken.
+auto Build = [&](llvm::StringRef Checks) -> Duration {
+  TidyProvider CTProvider = [&](tidy::ClangTidyOptions &Opts,
+llvm::StringRef) {
+Opts.Checks = Checks.str();
+  };
+  Inputs.Cl

[PATCH] D136082: [clangd] Extend --check to time clang-tidy checks, so we can block slow ones

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/tool/Check.cpp:66
+llvm::cl::desc("Print the overhead of checks matching this glob"),
+llvm::cl::init(""));
+

kadircet wrote:
> i guess we also need a `llvm::cl::cat(Check)` here?
Removed the category instead, it's useless afaict


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136082

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

[PATCH] D136082: [clangd] Extend --check to time clang-tidy checks, so we can block slow ones

[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 rGfd733a65de5c: [clangd] Extend --check to time clang-tidy 
checks, so we can block slow ones (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D136082?vs=468198&id=476896#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136082

Files:
  clang-tools-extra/clangd/tool/Check.cpp

Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -24,10 +24,13 @@
 //
 //===--===//
 
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "CompileCommands.h"
 #include "Config.h"
+#include "Feature.h"
 #include "GlobalCompilationDatabase.h"
 #include "Hover.h"
 #include "InlayHints.h"
@@ -43,17 +46,25 @@
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
+// These will never be shown in --help, ClangdMain doesn't list the category.
+llvm::cl::opt CheckTidyTime(
+"check-tidy-time",
+llvm::cl::desc("Print the overhead of checks matching this glob"),
+llvm::cl::init(""));
+
 // Print (and count) the error-level diagnostics (warnings are ignored).
 unsigned showErrors(llvm::ArrayRef Diags) {
   unsigned ErrCount = 0;
@@ -66,6 +77,19 @@
   return ErrCount;
 }
 
+std::vector listTidyChecks(llvm::StringRef Glob) {
+  tidy::GlobList G(Glob);
+  tidy::ClangTidyCheckFactories CTFactories;
+  for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+E.instantiate()->addCheckFactories(CTFactories);
+  std::vector Result;
+  for (const auto &E : CTFactories)
+if (G.contains(E.getKey()))
+  Result.push_back(E.getKey().str());
+  llvm::sort(Result);
+  return Result;
+}
+
 // This class is just a linear pipeline whose functions get called in sequence.
 // Each exercises part of clangd's logic on our test file and logs results.
 // Later steps depend on state built in earlier ones (such as the AST).
@@ -193,9 +217,100 @@
   log("Indexing AST...");
   Index.updateMain(File, *AST);
 }
+
+if (!CheckTidyTime.empty()) {
+  if (!CLANGD_TIDY_CHECKS) {
+elog("-{0} requires -DCLANGD_TIDY_CHECKS!", CheckTidyTime.ArgStr);
+return false;
+  }
+  checkTidyTimes();
+}
+
 return true;
   }
 
+  // For each check foo, we want to build with checks=-* and checks=-*,foo.
+  // (We do a full build rather than just AST matchers to meausre PPCallbacks).
+  //
+  // However, performance has both random noise and systematic changes, such as
+  // step-function slowdowns due to CPU scaling.
+  // We take the median of 5 measurements, and after every check discard the
+  // measurement if the baseline changed by >3%.
+  void checkTidyTimes() {
+double Stability = 0.03;
+log("Timing AST build with individual clang-tidy checks (target accuracy "
+"{0:P0})",
+Stability);
+
+using Duration = std::chrono::nanoseconds;
+// Measure time elapsed by a block of code. Currently: user CPU time.
+auto Time = [&](auto &&Run) -> Duration {
+  llvm::sys::TimePoint<> Elapsed;
+  std::chrono::nanoseconds UserBegin, UserEnd, System;
+  llvm::sys::Process::GetTimeUsage(Elapsed, UserBegin, System);
+  Run();
+  llvm::sys::Process::GetTimeUsage(Elapsed, UserEnd, System);
+  return UserEnd - UserBegin;
+};
+auto Change = [&](Duration Exp, Duration Base) -> double {
+  return (double)(Exp.count() - Base.count()) / Base.count();
+};
+// Build ParsedAST with a fixed check glob, and return the time taken.
+auto Build = [&](llvm::StringRef Checks) -> Duration {
+  TidyProvider CTProvider = [&](tidy::ClangTidyOptions &Opts,
+llvm::StringRef) {
+Opts.Checks = Checks.str();
+  };
+  Inputs.ClangTidyProvider = CTProvider;
+  // Sigh, can't reuse the CompilerInvocation.
+  IgnoringDiagConsumer IgnoreDiags;
+  auto Invocation = buildCompilerInvocation(Inputs, IgnoreDiags);
+  Duration Val = Time([&] {
+ParsedAST::build(File, Inputs, std::move(Invocation), {}, Preamble);
+  });
+  vlog("Measured {0} ==> {1}", Checks, Val);
+  return Val;
+};
+// Measure several times, return the me

[clang-tools-extra] 5696f2d - [clangd] Move --check options into Check.cpp. Add --check-completion.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-11-21T16:39:27+01:00
New Revision: 5696f2dfce5f23967a047f68897a7ff2e3d8e7d1

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

LOG: [clangd] Move --check options into Check.cpp. Add --check-completion.

This is less plumbing and clutter in ClangdMain.cpp.
Having --check-lines imply completion was just about minimizing plumbing
I think, so make that explicit.

Added: 


Modified: 
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index 40a545e240d4..9517d74123c6 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -60,10 +60,22 @@ namespace clangd {
 namespace {
 
 // These will never be shown in --help, ClangdMain doesn't list the category.
-llvm::cl::opt CheckTidyTime(
+llvm::cl::opt CheckTidyTime{
 "check-tidy-time",
 llvm::cl::desc("Print the overhead of checks matching this glob"),
-llvm::cl::init(""));
+llvm::cl::init("")};
+llvm::cl::opt CheckFileLines{
+"check-lines",
+llvm::cl::desc(
+"Limits the range of tokens in -check file on which "
+"various features are tested. Example --check-lines=3-7 restricts "
+"testing to lines 3 to 7 (inclusive) or --check-lines=5 to restrict "
+"to one line. Default is testing entire file."),
+llvm::cl::init("")};
+llvm::cl::opt CheckCompletion{
+"check-completion",
+llvm::cl::desc("Run code-completion at each point (slow)"),
+llvm::cl::init(false)};
 
 // Print (and count) the error-level diagnostics (warnings are ignored).
 unsigned showErrors(llvm::ArrayRef Diags) {
@@ -330,8 +342,7 @@ class Checker {
   }
 
   // Run AST-based features at each token in the file.
-  void testLocationFeatures(llvm::Optional LineRange,
-const bool EnableCodeCompletion) {
+  void testLocationFeatures(llvm::Optional LineRange) {
 trace::Span Trace("testLocationFeatures");
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
@@ -383,7 +394,7 @@ class Checker {
   unsigned DocHighlights = findDocumentHighlights(*AST, Pos).size();
   vlog("documentHighlight: {0}", DocHighlights);
 
-  if (EnableCodeCompletion) {
+  if (CheckCompletion) {
 Position EndPos = offsetToPosition(Inputs.Contents, End);
 auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
 vlog("code completion: {0}",
@@ -395,9 +406,27 @@ class Checker {
 
 } // namespace
 
-bool check(llvm::StringRef File, llvm::Optional LineRange,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
-   bool EnableCodeCompletion) {
+bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
+   const ClangdLSPServer::Options &Opts) {
+  llvm::Optional LineRange;
+  if (!CheckFileLines.empty()) {
+uint32_t Begin = 0, End = std::numeric_limits::max();
+StringRef RangeStr(CheckFileLines);
+bool ParseError = RangeStr.consumeInteger(0, Begin);
+if (RangeStr.empty()) {
+  End = Begin;
+} else {
+  ParseError |= !RangeStr.consume_front("-");
+  ParseError |= RangeStr.consumeInteger(0, End);
+}
+if (ParseError || !RangeStr.empty() || Begin <= 0 || End < Begin) {
+  elog("Invalid --check-lines specified. Use Begin-End format, e.g. 3-17");
+  return false;
+}
+LineRange = Range{Position{static_cast(Begin - 1), 0},
+  Position{static_cast(End), 0}};
+  }
+
   llvm::SmallString<0> FakeFile;
   llvm::Optional Contents;
   if (File.empty()) {
@@ -426,7 +455,7 @@ bool check(llvm::StringRef File, llvm::Optional 
LineRange,
 return false;
   C.buildInlayHints(LineRange);
   C.buildSemanticHighlighting(LineRange);
-  C.testLocationFeatures(LineRange, EnableCodeCompletion);
+  C.testLocationFeatures(LineRange);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 53b6653c51f2..22d407be5033 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -61,9 +61,8 @@ namespace clang {
 namespace clangd {
 
 // Implemented in Check.cpp.
-bool check(const llvm::StringRef File, llvm::Optional LineRange,
-   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
-   bool EnableCodeCompletion);
+bool check(const llvm::StringRef File, const ThreadsafeFS &TFS,
+   const ClangdLSPServer::Options &Opts);
 
 namespace {
 
@@ -392,17 +391,6 @@ opt CheckFile

[clang-tools-extra] c67b710 - [clangd] Add option to skip per-location checks.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-11-21T16:41:53+01:00
New Revision: c67b710379402a48b0a158fa6247ae421ff9aa77

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

LOG: [clangd] Add option to skip per-location checks.

This avoids a slow step after timing tidy checks for 
https://github.com/clangd/clangd/issues/1337

Added: 


Modified: 
clang-tools-extra/clangd/tool/Check.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index 9517d74123c6..49dfad90593d 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -72,6 +72,12 @@ llvm::cl::opt CheckFileLines{
 "testing to lines 3 to 7 (inclusive) or --check-lines=5 to restrict "
 "to one line. Default is testing entire file."),
 llvm::cl::init("")};
+llvm::cl::opt CheckLocations{
+"check-locations",
+llvm::cl::desc(
+"Runs certain features (e.g. hover) at each point in the file. "
+"Somewhat slow."),
+llvm::cl::init(true)};
 llvm::cl::opt CheckCompletion{
 "check-completion",
 llvm::cl::desc("Run code-completion at each point (slow)"),
@@ -455,7 +461,8 @@ bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
 return false;
   C.buildInlayHints(LineRange);
   C.buildSemanticHighlighting(LineRange);
-  C.testLocationFeatures(LineRange);
+  if (CheckLocations)
+C.testLocationFeatures(LineRange);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;



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

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D137020#3940523 , @sammccall wrote:

> Not sure this is ready for review again, ignore me if not...
>
> I'm still not sure why this is correct in principle. Without that, if someone 
> finds a misparse 6 months from now I don't know how we determine where to fix 
> it.
>
> For example, this path is called from 
> `Parser::isKnownToBeDeclarationSpecifier()` whose contract is `Return true if 
> we know that we are definitely looking at a decl-specifier... Return false if 
> it's no a decl-specifier, or we're not sure.` There doesn't seem to be any 
> room for heuristics, unless we're going to change that contract and audit all 
> the callers. If this *isn't* a heuristic (it sure looks like one) it needs 
> some comments on why it's correct.

FWIW, I agree with the concerns here; I touched on the same thing with my 
earlier comments.


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

https://reviews.llvm.org/D137020

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

[PATCH] D138234: [clang-format] Support new file formatting with vim

[Jinsong Ji via Phabricator via cfe-commits]

jsji updated this revision to Diff 476905.
jsji added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138234

Files:
  clang/tools/clang-format/clang-format.py


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -41,11 +41,11 @@
 
 import difflib
 import json
+import os.path
 import platform
 import subprocess
 import sys
 import vim
-import os.path
 
 # set g:clang_format_path to the path to clang-format if it is not on the path
 # Change this to the full path if clang-format is not on the path.
@@ -77,7 +77,8 @@
   # Determine range to format.
   if vim.eval('exists("l:lines")') == '1':
 lines = ['-lines', vim.eval('l:lines')]
-  elif vim.eval('exists("l:formatdiff")') == '1' and 
os.path.exists(vim.current.buffer.name):
+  elif vim.eval('exists("l:formatdiff")') == '1' and \
+   os.path.exists(vim.current.buffer.name):
 with open(vim.current.buffer.name, 'r') as f:
   ondisk = f.read().splitlines();
 sequence = difflib.SequenceMatcher(None, ondisk, vim.current.buffer)


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -41,11 +41,11 @@
 
 import difflib
 import json
+import os.path
 import platform
 import subprocess
 import sys
 import vim
-import os.path
 
 # set g:clang_format_path to the path to clang-format if it is not on the path
 # Change this to the full path if clang-format is not on the path.
@@ -77,7 +77,8 @@
   # Determine range to format.
   if vim.eval('exists("l:lines")') == '1':
 lines = ['-lines', vim.eval('l:lines')]
-  elif vim.eval('exists("l:formatdiff")') == '1' and os.path.exists(vim.current.buffer.name):
+  elif vim.eval('exists("l:formatdiff")') == '1' and \
+   os.path.exists(vim.current.buffer.name):
 with open(vim.current.buffer.name, 'r') as f:
   ondisk = f.read().splitlines();
 sequence = difflib.SequenceMatcher(None, ondisk, vim.current.buffer)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9e3264a - [FPEnv] Enable strict fp for AArch64 in clang

[John Brawn via cfe-commits]

Author: John Brawn
Date: 2022-11-21T16:02:54Z
New Revision: 9e3264ab2021e07246e2cb491497d9cd514c213c

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

LOG: [FPEnv] Enable strict fp for AArch64 in clang

The AArch64 target now has the necessary support for strict fp, so
enable it in clang.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Basic/Targets/AArch64.cpp
clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
clang/test/CodeGen/aarch64-neon-misc-constrained.c
clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem-constrained.c
clang/test/CodeGen/aarch64-strictfp-builtins.c
clang/test/CodeGen/aarch64-v8.2a-fp16-intrinsics-constrained.c
clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c
clang/test/CodeGen/arm-neon-directed-rounding-constrained.c
clang/test/CodeGen/arm64-vrnd-constrained.c
clang/test/CodeGen/fp16-ops-strictfp.c
clang/test/Parser/pragma-fp-warn.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b2b085409116a..6a98dd3b7da8d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -765,6 +765,9 @@ Arm and AArch64 Support in Clang
   * Arm Cortex-A715 (cortex-a715).
   * Arm Cortex-X3 (cortex-x3).
   * Arm Neoverse V2 (neoverse-v2)
+- Strict floating point has been enabled for AArch64, which means that
+  ``-ftrapping-math``, ``-frounding-math``, ``-ffp-model=strict``, and
+  ``-ffp-exception-behaviour=`` are now accepted.
 
 Floating Point Support in Clang
 ---

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index c283aca85f4e2..d85a21397abc1 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -84,6 +84,7 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple 
&Triple,
   HasLegalHalfType = true;
   HalfArgsAndReturns = true;
   HasFloat16 = true;
+  HasStrictFP = true;
 
   if (Triple.isArch64Bit())
 LongWidth = LongAlign = PointerWidth = PointerAlign = 64;

diff  --git a/clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c 
b/clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
index 58af968725be9..33700f0d0d34c 100644
--- a/clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
+++ b/clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
@@ -4,7 +4,7 @@
 // RUN: | FileCheck --check-prefixes=COMMON,COMMONIR,UNCONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN: -S -disable-O0-optnone \
-// RUN:  -ffp-exception-behavior=strict -fexperimental-strict-floating-point \
+// RUN:  -ffp-exception-behavior=strict \
 // RUN:  -flax-vector-conversions=none -emit-llvm -o - %s | opt -S 
-passes=mem2reg \
 // RUN: | FileCheck --check-prefixes=COMMON,COMMONIR,CONSTRAINED %s
 

diff  --git a/clang/test/CodeGen/aarch64-neon-misc-constrained.c 
b/clang/test/CodeGen/aarch64-neon-misc-constrained.c
index dce36c392d63f..e24e129d2bc7d 100644
--- a/clang/test/CodeGen/aarch64-neon-misc-constrained.c
+++ b/clang/test/CodeGen/aarch64-neon-misc-constrained.c
@@ -3,7 +3,6 @@
 // RUN: | opt -S -passes=mem2reg | FileCheck --check-prefix=COMMON 
--check-prefix=COMMONIR --check-prefix=UNCONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN:  -ffp-exception-behavior=strict \
-// RUN:  -fexperimental-strict-floating-point \
 // RUN:  -disable-O0-optnone -emit-llvm -o - %s \
 // RUN: | opt -S -passes=mem2reg | FileCheck --check-prefix=COMMON 
--check-prefix=COMMONIR --check-prefix=CONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
@@ -11,7 +10,6 @@
 // RUN: | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN:  -ffp-exception-behavior=strict \
-// RUN:  -fexperimental-strict-floating-point \
 // RUN:  -disable-O0-optnone -S -o - %s \
 // RUN: | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM %s
 

diff  --git 
a/clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem-constrained.c 
b/clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem-constrained.c
index bd0501b42dd6e..6371339f0a40d 100644
--- a/clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem-constrained.c
+++ b/clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem-constrained.c
@@ -3,7 +3,6 @@
 // RUN: | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR 
--check-prefix=UNCONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon 
-target-cpu cyclone \
 // RUN: -ffp-exception-behavior=strict \
-// RUN: -fexperimental-strict-floating-point \
 // RUN: -disable-O0-optno

[PATCH] D138143: [FPEnv] Enable strict fp for AArch64 in clang

[John Brawn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e3264ab2021: [FPEnv] Enable strict fp for AArch64 in clang 
(authored by john.brawn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138143

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
  clang/test/CodeGen/aarch64-neon-misc-constrained.c
  clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem-constrained.c
  clang/test/CodeGen/aarch64-strictfp-builtins.c
  clang/test/CodeGen/aarch64-v8.2a-fp16-intrinsics-constrained.c
  clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c
  clang/test/CodeGen/arm-neon-directed-rounding-constrained.c
  clang/test/CodeGen/arm64-vrnd-constrained.c
  clang/test/CodeGen/fp16-ops-strictfp.c
  clang/test/Parser/pragma-fp-warn.c

Index: clang/test/Parser/pragma-fp-warn.c
===
--- clang/test/Parser/pragma-fp-warn.c
+++ clang/test/Parser/pragma-fp-warn.c
@@ -1,7 +1,7 @@
 
 // RUN: %clang_cc1 -triple wasm32 -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
 // RUN: %clang_cc1 -triple thumbv7 -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
-// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
+// RUN: %clang_cc1 -DEXPOK -triple aarch64 -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
 // RUN: %clang_cc1 -DEXPOK -triple x86_64 -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
 // RUN: %clang_cc1 -DEXPOK -triple systemz -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
 // RUN: %clang_cc1 -DEXPOK -triple powerpc -fsyntax-only -Wno-unknown-pragmas -Wignored-pragmas -verify %s
Index: clang/test/CodeGen/fp16-ops-strictfp.c
===
--- clang/test/CodeGen/fp16-ops-strictfp.c
+++ clang/test/CodeGen/fp16-ops-strictfp.c
@@ -1,10 +1,10 @@
 // REQUIRES: arm-registered-target
 // RUN: %clang_cc1 -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -emit-llvm -o - -triple arm-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK -vv -dump-input=fail
-// RUN: %clang_cc1 -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -emit-llvm -o - -triple aarch64-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
+// RUN: %clang_cc1 -ffp-exception-behavior=maytrap -emit-llvm -o - -triple aarch64-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
 // RUN: %clang_cc1 -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -emit-llvm -o - -triple x86_64-linux-gnu %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
 // RUN: %clang_cc1 -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -emit-llvm -o - -triple arm-none-linux-gnueabi -fnative-half-type %s \
 // RUN:   | FileCheck %s --check-prefix=NATIVE-HALF --check-prefix=CHECK
-// RUN: %clang_cc1 -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -emit-llvm -o - -triple aarch64-none-linux-gnueabi -fnative-half-type %s \
+// RUN: %clang_cc1 -ffp-exception-behavior=maytrap -emit-llvm -o - -triple aarch64-none-linux-gnueabi -fnative-half-type %s \
 // RUN:   | FileCheck %s --check-prefix=NATIVE-HALF --check-prefix=CHECK
 //
 // Test that the constrained intrinsics are picking up the exception
Index: clang/test/CodeGen/arm64-vrnd-constrained.c
===
--- clang/test/CodeGen/arm64-vrnd-constrained.c
+++ clang/test/CodeGen/arm64-vrnd-constrained.c
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -triple arm64-apple-ios7 -target-feature +neon -ffreestanding -flax-vector-conversions=none -emit-llvm -o - %s \
 // RUN: | FileCheck --check-prefix=COMMON --check-prefix=UNCONSTRAINED %s
-// RUN: %clang_cc1 -triple arm64-apple-ios7 -target-feature +neon -ffreestanding -flax-vector-conversions=none -fexperimental-strict-floating-point -ffp-exception-behavior=strict -emit-llvm -o - %s \
+// RUN: %clang_cc1 -triple arm64-apple-ios7 -target-feature +neon -ffreestanding -flax-vector-conversions=none -ffp-exception-behavior=strict -emit-llvm -o - %s \
 // RUN: | FileCheck --check-prefix=COMMON --check-prefix=CONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-apple-ios7 -target-feature +neon -ffreestanding -flax-vector-conversions=none -emit-llvm -o - %s | llc -o=- - \
 // RUN: | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM %s
-// RUN: %clang_cc1 -triple arm64-apple-ios7 -target-feature +neon -ffreestanding -flax-vector-conversions=none -fexperimental-strict-floating-point -ffp-exception-behavior=strict -emit-llvm -o - %s | llc -o=- - \
+// RUN: %clang_cc1 -triple arm64-apple-ios7 -target-feature +neon -ffreestanding -flax-vector-c

[PATCH] D138439: clang: Fix cast failure when using -fsanitize=undefined for HIP

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, samsonov, tra, bkramer.
Herald added a subscriber: arichardson.
Herald added a project: All.
arsenm requested review of this revision.
Herald added a subscriber: wdng.

This was assuming a direct reference to the global variable. The
constant string is placed in addrspace 4, and has a constexpr
addrspacecast to the generic address space.


https://reviews.llvm.org/D138439

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenHIP/sanitize-undefined-null.hip


Index: clang/test/CodeGenHIP/sanitize-undefined-null.hip
===
--- /dev/null
+++ clang/test/CodeGenHIP/sanitize-undefined-null.hip
@@ -0,0 +1,36 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -emit-llvm 
-disable-llvm-passes -fcuda-is-device -fsanitize=null \
+// RUN:   -o - %s | FileCheck --enable-var-scope %s
+
+// Check there are no assertions when trying to sanitize when globals have 
non-0
+// address spaces.
+
+#define __device__ __attribute__((device))
+
+//.
+// CHECK: @.src = private unnamed_addr addrspace(4) constant [{{[0-9]+}} x i8] 
c
+// CHECK: @0 = private unnamed_addr addrspace(1) constant { i16, i16, [7 x i8] 
} { i16 0, i16 7, [7 x i8] c"'char'\00" }
+// CHECK: @1 = private unnamed_addr addrspace(1) global { { ptr, i32, i32 }, 
ptr addrspace(1), i8, i8 } { { ptr, i32, i32 } { ptr addrspacecast (ptr 
addrspace(4) @.src to ptr), i32 {{[0-9]+}}, i32 3 }, ptr addrspace(1) @0, i8 1, 
i8 1 }
+//.
+// CHECK-LABEL: @_Z3fooPc(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i32, align 4, addrspace(5)
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca ptr, align 8, addrspace(5)
+// CHECK-NEXT:[[RETVAL_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[RETVAL]] to ptr
+// CHECK-NEXT:[[P_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[P_ADDR]] to ptr
+// CHECK-NEXT:store ptr [[P:%.*]], ptr [[P_ADDR_ASCAST]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load ptr, ptr [[P_ADDR_ASCAST]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = icmp ne ptr [[TMP0]], null, !nosanitize !3
+// CHECK-NEXT:br i1 [[TMP1]], label [[CONT:%.*]], label 
[[HANDLER_TYPE_MISMATCH:%.*]], !prof [[PROF4:![0-9]+]], !nosanitize !3
+// CHECK:   handler.type_mismatch:
+// CHECK-NEXT:[[TMP2:%.*]] = ptrtoint ptr [[TMP0]] to i64, !nosanitize !3
+// CHECK-NEXT:call void @__ubsan_handle_type_mismatch_v1_abort(ptr 
addrspace(1) @[[GLOB1:[0-9]+]], i64 [[TMP2]]) #[[ATTR2:[0-9]+]], !nosanitize !3
+// CHECK-NEXT:unreachable, !nosanitize !3
+// CHECK:   cont:
+// CHECK-NEXT:store i8 0, ptr [[TMP0]], align 1
+// CHECK-NEXT:ret i32 3
+//
+__device__ int foo(char *p) {
+  *p = 0;
+  return 3;
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3157,7 +3157,8 @@
 auto FilenameGV =
 CGM.GetAddrOfConstantCString(std::string(FilenameString), ".src");
 CGM.getSanitizerMetadata()->disableSanitizerForGlobal(
-  cast(FilenameGV.getPointer()));
+cast(
+FilenameGV.getPointer()->stripPointerCasts()));
 Filename = FilenameGV.getPointer();
 Line = PLoc.getLine();
 Column = PLoc.getColumn();
@@ -3325,13 +3326,15 @@
 // Emit handler arguments and create handler function type.
 if (!StaticArgs.empty()) {
   llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
-  auto *InfoPtr =
-  new llvm::GlobalVariable(CGM.getModule(), Info->getType(), false,
-   llvm::GlobalVariable::PrivateLinkage, Info);
+  auto *InfoPtr = new llvm::GlobalVariable(
+  CGM.getModule(), Info->getType(), false,
+  llvm::GlobalVariable::PrivateLinkage, Info, "", nullptr,
+  llvm::GlobalVariable::NotThreadLocal,
+  CGM.getDataLayout().getDefaultGlobalsAddressSpace());
   InfoPtr->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
   CGM.getSanitizerMetadata()->disableSanitizerForGlobal(InfoPtr);
-  Args.push_back(Builder.CreateBitCast(InfoPtr, Int8PtrTy));
-  ArgTypes.push_back(Int8PtrTy);
+  Args.push_back(EmitCastToVoidPtr(InfoPtr));
+  ArgTypes.push_back(Args.back()->getType());
 }
 
 for (size_t i = 0, n = DynamicArgs.size(); i != n; ++i) {


Index: clang/test/CodeGenHIP/sanitize-undefined-null.hip
===
--- /dev/null
+++ clang/test/CodeGenHIP/sanitize-undefined-null.hip
@@ -0,0 +1,36 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -emit-llvm -disable-llvm-passes -fcuda-is-device -fsanitize=null \
+// RUN:   -o - %s | FileCheck --enable-var-scope %s
+
+// Check there are no assertions when trying to sanitize when globals have non-0
+// address spaces.
+
+#define __device__ _

[PATCH] D136176: Implement support for option 'fexcess-precision'.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 476913.
zahiraam marked 6 inline comments as done.

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

https://reviews.llvm.org/D136176

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/X86/fexcess-precision.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fexcess-precision.c

Index: clang/test/Driver/fexcess-precision.c
===
--- /dev/null
+++ clang/test/Driver/fexcess-precision.c
@@ -0,0 +1,30 @@
+// RUN: %clang -### -target i386 -fexcess-precision=fast -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang -### -target i386 -fexcess-precision=standard -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-STD %s
+
+// RUN: %clang -### -target x86_64 -fexcess-precision=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang -### -target x86_64 -fexcess-precision=standard -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STD %s
+
+// RUN: %clang -### -target i386 -fexcess-precision=none -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-ARG %s
+
+// RUN: %clang -### -target x86_64 -fexcess-precision=none -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=CHECK-ERR-ARG %s
+
+// RUN: %clang -### -target i386 -fexcess-precision=none -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=CHECK-ERR-ARG %s
+
+// RUN: %clang -### -target i386 -fexcess-precision=16 -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-I386 %s
+
+// RUN: %clang -### -target x86_64 -fexcess-precision=16 -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-X64 %s
+
+// CHECK-FAST: "-ffloat16-excess-precision=fast"
+// CHECK-STD: "-ffloat16-excess-precision=standard"
+// CHECK-ERR-I386: error: unsupported option '-fexcess-precision=16' for target 'i386'
+// CHECK-ERR-X64: error: unsupported option '-fexcess-precision=16' for target 'x86_64'
+// CHECK-ERR-ARG: unsupported argument 'none' to option '-fexcess-precision='
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -398,7 +398,7 @@
 // CHECK-WARNING-DAG: optimization flag '-falign-loops' is not supported
 // CHECK-WARNING-DAG: optimization flag '-falign-jumps' is not supported
 // CHECK-WARNING-DAG: optimization flag '-falign-jumps=100' is not supported
-// CHECK-WARNING-DAG: optimization flag '-fexcess-precision=100' is not supported
+// CHECK-WARNING-DAG: unsupported argument '100' to option '-fexcess-precision='
 // CHECK-WARNING-DAG: optimization flag '-fbranch-count-reg' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fno-default-inline' is not supported
Index: clang/test/CodeGen/X86/fexcess-precision.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/fexcess-precision.c
@@ -0,0 +1,286 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=standard \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=none \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: -emit-llvm -ffp-eval-method=source -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=standard \
+// RUN: -emit-llvm -ffp-eval-method=source -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=none \
+// RUN: -emit-llvm -ffp-eval-method=source -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: -emit-llvm -ffp-eval-method=double -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT-DBL %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=standard \
+// RUN: -emit-llvm -ffp-eval-method=double -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT-DBL %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=none \
+// RUN: -emit-llvm -ffp-eval-metho

[PATCH] D136176: Implement support for option 'fexcess-precision'.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/docs/UsersManual.rst:1732
+.. option:: -fexcess-precision:
+
+   By default, Clang uses excess precision to calculate ``_Float16``

rjmccall wrote:
> 
>The C and C++ standards allow floating-point expressions to be computed
>as if intermediate results had more precision (and/or a wider range) than 
> the
>type of the expression strictly allows.  This is called excess precision 
> arithmetic.
>Excess precision arithmetic can improve the accuracy of results (although 
> not
>always), and it can make computation significantly faster if the target 
> lacks
>direct hardware support for arithmetic in a particular type.  However, it 
> can
>also undermine strict floating-point reproducibility.
> 
>Under the standards, assignments and explicit casts force the operand to be
>converted to its formal type, discarding any excess precision.  Because 
> data
>can only flow between statements via an assignment, this means that the
>use of excess precision arithmetic is a reliable local property of a single
>statement, and results do not change based on optimization.  However, when
>excess precision arithmetic is in use, Clang does not guarantee strict
>reproducibility, and future compiler releases may recognize more 
> opportunities
>to use excess precision arithmetic, e.g. with floating-point builtins.
> 
>Clang does not use excess precision arithmetic for most types or on most 
> targets.
>For example, even on pre-SSE X86 targets where ``float`` and ``double``
>computations must be performed in the 80-bit X87 format, Clang rounds
>all intermediate results correctly for their type.  Clang currently uses 
> excess
>precision arithmetic by default only for the following types and targets:
> 
>* ``_Float16`` on X86 targets without ``AVX512-FP16``
> 
>The ``-fexcess-precision=`` option can be used to control the use 
> of excess
>precision arithmetic.  Valid values are:
> 
>* ``standard`` - The default.  Allow the use of excess precision 
> arithmetic under
>  the constraints of the C and C++ standards. Has no effect except on the 
> types
>  and targets listed above.
>* ``fast`` - Accepted for GCC compatibility, but currently treated as an 
> alias
>  for ``standard``.
>* ``16`` - Forces ``_Float16`` operations to be emitted without using 
> excess
>  precision arithmetic.
>
Thanks.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2995
+  StringRef Val = A->getValue();
+   if (TC.getTriple().getArch() == llvm::Triple::x86 && Val.equals("16"))
+D.Diag(diag::err_drv_unsupported_opt_for_target)

rjmccall wrote:
> zahiraam wrote:
> > andrew.w.kaylor wrote:
> > > Why is 16 only supported for x86? Is it only here for gcc compatibility?
> > Yes for gcc compatibility (although we are using here that value "none" to 
> > disable excess precision instead of using "16") and also because we are 
> > dealing with excess precision for _Float16 types only, so sticking to X86.
> `llvm::Triple::x86` is just i386, and I think you want to include x86_64, 
> right?
Yes. Thanks.


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

https://reviews.llvm.org/D136176

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

[PATCH] D138210: [clang] Require parameter pack to be last argument in concepts.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The error itself makes sense, but I don't quite understand why usages of 
`invalid` did not produce errors before. Any idea why that happened? Are 
there some other bugs hiding?
It seems that at some point when parsing this code:

  static_assert(invalid also here ;

we chose to silently recover by skipping until the semicolon, but never 
actually produced any errors. Is there a code path that should also be updated 
to handle that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138210

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

[PATCH] D138210: [clang] Require parameter pack to be last argument in concepts.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

E.g. gcc actually accepts the concept definition, but later fails with "wrong 
number of template arguments". I wonder why Clang has not been doing the latter 
before this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138210

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

[PATCH] D138441: [clang-format][docs] Fix invalid CSS syntax in versionbadge

[Emilia Dreamer via Phabricator via cfe-commits]

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

CSS uses colons, not the equals sign. The final semicolon is optional,
but preferred to be included. Really, the font property doesn't really
need to be there, but I suppose it was put there for a reason.

It's surprising how lenient browsers are when parsing


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138441

Files:
  clang/docs/ClangFormatStyleOptions.rst


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1,7 +1,7 @@
 .. raw:: html
 
   
-.versionbadge { background-color: #1c913d; height: 20px; display: 
inline-block; width: 120px; text-align: center; border-radius: 5px; color: 
#FF; font-family="Verdana,Geneva,DejaVu Sans,sans-serif" }
+.versionbadge { background-color: #1c913d; height: 20px; display: 
inline-block; width: 120px; text-align: center; border-radius: 5px; color: 
#FF; font-family: "Verdana,Geneva,DejaVu Sans,sans-serif"; }
   
 
 .. role:: versionbadge


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1,7 +1,7 @@
 .. raw:: html
 
   
-.versionbadge { background-color: #1c913d; height: 20px; display: inline-block; width: 120px; text-align: center; border-radius: 5px; color: #FF; font-family="Verdana,Geneva,DejaVu Sans,sans-serif" }
+.versionbadge { background-color: #1c913d; height: 20px; display: inline-block; width: 120px; text-align: center; border-radius: 5px; color: #FF; font-family: "Verdana,Geneva,DejaVu Sans,sans-serif"; }
   
 
 .. role:: versionbadge
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

[Tom Praschan via Phabricator via cfe-commits]

tom-anders updated this revision to Diff 476918.
tom-anders marked 7 inline comments as done.
tom-anders added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137894

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -302,6 +302,9 @@
 MATCHER_P(sym, Name, "") { return arg.Name == Name; }
 
 MATCHER_P(rangeIs, R, "") { return arg.Loc.range == R; }
+MATCHER_P(containerIs, C, "") {
+  return arg.Loc.containerName.value_or("") == C;
+}
 MATCHER_P(attrsAre, A, "") { return arg.Attributes == A; }
 MATCHER_P(hasID, ID, "") { return arg.ID == ID; }
 
@@ -1900,28 +1903,30 @@
 
   auto AST = TU.build();
   std::vector> ExpectedLocations;
-  for (const auto &R : T.ranges())
-ExpectedLocations.push_back(AllOf(rangeIs(R), attrsAre(0u)));
+  for (const auto &[R, Context] : T.rangesWithPayload())
+ExpectedLocations.push_back(
+AllOf(rangeIs(R), containerIs(Context), attrsAre(0u)));
   // $def is actually shorthand for both definition and declaration.
   // If we have cases that are definition-only, we should change this.
-  for (const auto &R : T.ranges("def"))
-ExpectedLocations.push_back(
-AllOf(rangeIs(R), attrsAre(ReferencesResult::Definition |
-   ReferencesResult::Declaration)));
-  for (const auto &R : T.ranges("decl"))
-ExpectedLocations.push_back(
-AllOf(rangeIs(R), attrsAre(ReferencesResult::Declaration)));
-  for (const auto &R : T.ranges("overridedecl"))
+  for (const auto &[R, Context] : T.rangesWithPayload("def"))
+ExpectedLocations.push_back(AllOf(rangeIs(R), containerIs(Context),
+  attrsAre(ReferencesResult::Definition |
+   ReferencesResult::Declaration)));
+  for (const auto &[R, Context] : T.rangesWithPayload("decl"))
+ExpectedLocations.push_back(AllOf(rangeIs(R), containerIs(Context),
+  attrsAre(ReferencesResult::Declaration)));
+  for (const auto &[R, Context] : T.rangesWithPayload("overridedecl"))
 ExpectedLocations.push_back(AllOf(
-rangeIs(R),
+rangeIs(R), containerIs(Context),
 attrsAre(ReferencesResult::Declaration | ReferencesResult::Override)));
-  for (const auto &R : T.ranges("overridedef"))
-ExpectedLocations.push_back(
-AllOf(rangeIs(R), attrsAre(ReferencesResult::Declaration |
-   ReferencesResult::Definition |
-   ReferencesResult::Override)));
+  for (const auto &[R, Context] : T.rangesWithPayload("overridedef"))
+ExpectedLocations.push_back(AllOf(rangeIs(R), containerIs(Context),
+  attrsAre(ReferencesResult::Declaration |
+   ReferencesResult::Definition |
+   ReferencesResult::Override)));
   for (const auto &P : T.points()) {
-EXPECT_THAT(findReferences(AST, P, 0, UseIndex ? TU.index().get() : nullptr)
+EXPECT_THAT(findReferences(AST, P, 0, UseIndex ? TU.index().get() : nullptr,
+   /*AddContext*/ true)
 .References,
 UnorderedElementsAreArray(ExpectedLocations))
 << "Failed for Refs at " << P << "\n"
@@ -1933,18 +1938,18 @@
   const char *Tests[] = {
   R"cpp(// Local variable
 int main() {
-  int $def[[foo]];
-  [[^foo]] = 2;
-  int test1 = [[foo]];
+  int $def(main)[[foo]];
+  $(main)[[^foo]] = 2;
+  int test1 = $(main)[[foo]];
 }
   )cpp",
 
   R"cpp(// Struct
 namespace ns1 {
-struct $def[[Foo]] {};
+struct $def(ns1)[[Foo]] {};
 } // namespace ns1
 int main() {
-  ns1::[[Fo^o]]* Params;
+  ns1::$(main)[[Fo^o]]* Params;
 }
   )cpp",
 
@@ -1952,51 +1957,51 @@
 class $decl[[Foo]];
 class $def[[Foo]] {};
 int main() {
-  [[Fo^o]] foo;
+  $(main)[[Fo^o]] foo;
 }
   )cpp",
 
   R"cpp(// Function
 int $def[[foo]](int) {}
 int main() {
-  auto *X = &[[^foo]];
-  [[foo]](42);
+

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

In D137894#3940600 , @sammccall wrote:

> Since this is a protocol extension, you might want to add an end-to-end test: 
> variant of `clang-tools-extra/clangd/test/xrefs.test` 
> (`xrefs-container.test`? to avoid complicating all the existing tests).

I'll look into adding this, but I'm not familiar with with these kind of tests 
yet, so I'll might need some time to figure it out - I already uploaded an 
updated version of the patch anyway to get some more feedback in the meantime.

> And one this lands, one of us should update 
> https://github.com/llvm/clangd-www/blob/main/extensions.md

Yeah I could do that, I think I can even commit directly to that repo? Or 
should I open a review on github/phabricator for this anyway?




Comment at: clang-tools-extra/clangd/Protocol.h:244
+
+  friend bool operator!=(const ReferenceLocation &LHS,
+ const ReferenceLocation &RHS) {

sammccall wrote:
> are these operators actually needed? especially `operator<` we usually get 
> away without
Correct, they are indeed not needed as of now.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1307
+llvm::Optional
+getContextStringForMainFileRef(const DeclContext *DeclCtx) {
+  for (auto *Ctx = DeclCtx; Ctx; Ctx = Ctx->getParent()) {

sammccall wrote:
> a few things to consider here:
>  - for e.g. lambdas, do you want `foo::bar::(anonymous class)::operator()`, 
> or `foo::bar`, or something else?
>  - there are a few other function-like decls, see Decl::isFunctionOrMethod().
>  - only functions? suppose you have `struct S { T member; };` and you're 
> looking up xrefs for T. Don't we want to return `S` as the context/container 
> name
>  - in general, I think want to get the same container for the index vs AST 
> path here. This suggests we should be sharing `getRefContainer` from 
> `SymbolCollector.cpp` rather than implementing something ifferent
Changed this to use `SymbolCollector::getRefContainer`, this should address 
most of the things you mentioned. Now AST and Index results should be 
consistent here



Comment at: clang-tools-extra/clangd/XRefs.cpp:1310
+if (const auto *FD = llvm::dyn_cast(Ctx))
+  return FD->getQualifiedNameAsString();
+if (const auto *RD = llvm::dyn_cast(Ctx))

sammccall wrote:
> again for stringifying, I think we want the index/non-index cases to be the 
> same, which suggests the logic to generate Scope+Name (i.e. 
> clangd::printQualifiedName)+Signature and then concatenating them.
> 
> Generating the signature is a bit complicated, so maybe leave it out (from 
> both AST + index codepath) from initial patch?
There's `CodeCompletionString::getSignature`, we could use that one probably?

But anyway, the `Signature` field for index container results is actually empty 
right now for some reason (not sure if this a bug or a feature), so I removed 
Signature from index container results as well for now. Let's look into this in 
a follow-up patch.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1458
+  assert(Ref != RefIndexForContainer.end());
+  // Name is not useful here, because it's just gonna be the name of 
the
+  // function the cursor is on. Scope is interesting though, since this

sammccall wrote:
> To me, this doesn't seem like a sufficient reason to have irregular behavior 
> for different refs.
Ok, changed it to Scope + Name here as well. If users complain about this, we 
can still change it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137894

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

[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this, I know it was a slog! I think this is pretty 
close to ready, though you should definitely add a release note about the 
changes.




Comment at: clang/include/clang/Parse/Parser.h:1605-1606
   // C99 6.9: External Definitions.
   DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &Attrs,
+  ParsedAttributes &DeclSpecAttrs,
   ParsingDeclSpec *DS = nullptr);

Should we rename `Attrs` to `DeclaratorAttrs` or something along those lines, 
so it's easier for someone to distinguish which attributes go where? (Same 
suggestion applies throughout the review.)



Comment at: clang/lib/Parse/ParseObjc.cpp:67-69
+for (const auto &PA : Attrs)
+  if (PA.isGNUAttribute())
+Diag(Tok.getLocation(), diag::err_objc_unexpected_attr);

llvm::for_each?



Comment at: clang/lib/Parse/ParseOpenMP.cpp:1997
 
+  ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+

We can sink this down to the only place it's used in the function, right?



Comment at: clang/lib/Parse/Parser.cpp:735-737
+  ParsedAttributes DeclSpecAttrs(AttrFactory);
+  while (MaybeParseCXX11Attributes(attrs) ||
+ MaybeParseGNUAttributes(DeclSpecAttrs))

We probably could use some comments here explaining why we parse some into one 
container and others into another container.



Comment at: clang/lib/Parse/Parser.cpp:1096-1097
+ParsingDeclSpec &DS, AccessSpecifier AS) {
+  DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
+  DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
+  DS.takeAttributesFrom(DeclSpecAttrs);

How sure are you that this isn't overwriting existing range data that's 
potentially relevant? e.g., where declaration specifiers and attributes are 
mixed together such that the attribute range doesn't cover all of the 
declaration specifiers?



Comment at: clang/test/Parser/attr-order.cpp:20
 __attribute__((cdecl)) __declspec(dllexport) [[noreturn]] void f(); // 
expected-error {{an attribute list cannot appear here}}
-__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void g(); // 
expected-error {{an attribute list cannot appear here}}
+__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void g();
 

This makes me happy!!


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

https://reviews.llvm.org/D137979

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

[PATCH] D138377: add clang_Type_getFullyQualifiedName

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In terms of testing, I'd recommend adding `-test-print-qualified-type` and see 
if you can thread a qualified vs unqualified parameter through `PrintType()` 
and friends to try to share as much code as possible. You'll probably need new 
wrapper functions that calls `PrintType` with the correct argument value 
depending on which test is being run. WDYT?




Comment at: clang/tools/libclang/CXType.cpp:318-325
+  CXTranslationUnit TU = GetTU(CT);
+  SmallString<64> Str;
+  llvm::raw_svector_ostream OS(Str);
+  PrintingPolicy PP(cxtu::getASTUnit(TU)->getASTContext().getLangOpts());
+
+  std::string qname = clang::TypeName::getFullyQualifiedName(
+  T, cxtu::getASTUnit(TU)->getASTContext(), PP);

Simplifying a bit and fixing up a naming convention nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138377

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

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:1250-1251
   bool ADL = UseArgumentDependentLookup(SS, Result, 
NextToken.is(tok::l_paren));
-  if (Result.isSingleResult() && !ADL && !FirstDecl->isCXXClassMember())
+  if (Result.isSingleResult() && !ADL &&
+  (!FirstDecl->isCXXClassMember() || isa(FirstDecl)))
 return NameClassification::NonType(Result.getRepresentativeDecl());

This change looks correct to me -- enum constants are class members but are 
certainly not forming an overload set!

Did you audit the other places we call `isCXXClassMember()` to ensure they're 
sensible regarding enum constant declarations?


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

https://reviews.llvm.org/D138091

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

[PATCH] D138446: [clang-format][docs] Add ability to link to specific config options

[Emilia Dreamer via Phabricator via cfe-commits]

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

This allows for the creation of permalinks to specific clang-format
options, for better sharing of a specific option and its options.

(I'm adding the usual clang-format reviewers on this patch because
I don't know any other reviewers that well, perhaps someone with
docs experience should be added instead...)

Note that I wanted to make minimal changes to make this happen and thus
landed on an unideal setup, but to me, it seems like the best out of
worse ones.

I could have made every style option a subheading, which would add
automatically the logic for permalinks and the little paragraph icon for
sharing.

However, this meant that the links themselves would be suboptimal, as
they'd include the whole text of the heading, including the type and
versionbadge, which is needless noise and could change, breaking the
concept of a "permalink". The format of the page could be changed to
put the option names on their own in a heading, and the other info below
it in a paragraph.

As Sphinx seems unwilling to fix 
https://github.com/sphinx-doc/sphinx/issues/1961,
there isn't a succinct way to change the "id" html field used for
sections

I could have used an add-on 
(https://github.com/GeeTransit/sphinx-better-subsection),
or made one myself, but I wanted to avoid extra dependencies for no
reason. (plus, I don't know how to make one myself.)

I could have used raw HTML for each heading, but that would immensely
pollute the rst file, which, while it is generated, is currently still
human-readable and it'd be nice for it to stay that way.

Also note that sphinx treats references as case-insensitive, which means
that they will all be lowercased in the resulting HTML. I envisioned
the ability to simply add #OptionName after the URL to get placed right
at the desired config option, which isn't possible without things such
as inline `raw` HTML.

To reconcile that, I added the ¶ paragraph buttons that can be used to
generate the link to the desired section, but since headings are not
actually used, they are faked and literally just a link following each
option, which means they stylistically don't match all other headings.

Also note that this sort-of assumes HTML output. I know Sphinx can
output other formats but I do not know if they are used. A non-html
output could embed unusable ¶ signs everywhere.

I'm okay with this patch being rejected in its current solution, or if
any of the above listed alternatives are better, they could be pursued
instead. In case the downsides of this solution are too much, I will
just create a feature request issue for this and maybe let someone more
experienced with Sphinx handle it, since this is still a feature I would
like to have. (and I do not want to deal with Sphinx at all after
battling with it for a whole day to produce a mediocre result.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138446

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py

Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -98,12 +98,10 @@
 self.version = version
 
   def __str__(self):
+s = ".. _%s:\n\n**%s** (``%s``) " % (self.name, self.name, to_yaml_type(self.type))
 if self.version:
-  s = '**%s** (``%s``) :versionbadge:`clang-format %s`\n%s' % (self.name, to_yaml_type(self.type), self.version,
- doxygen2rst(indent(self.comment, 2)))
-else:
-  s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
- doxygen2rst(indent(self.comment, 2)))
+  s += ':versionbadge:`clang-format %s` ' % self.version
+s += ':ref:`¶ <%s>`\n%s' % (self.name, doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
 if self.nested_struct:
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -136,8 +136,9 @@
 enumeration member (with a prefix, e.g. ``LS_Auto``), and as a value usable in
 the configuration (without a prefix: ``Auto``).
 
+.. _BasedOnStyle:
 
-**BasedOnStyle** (``String``)
+**BasedOnStyle** (``String``) :ref:`¶ `
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -178,10 +179,14 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``Integer``) :versionbadge:`clang-format 3.3`
+.. _AccessModifierOffset:
+
+**AccessModi

[PATCH] D137818: Add support for querying SubstTemplateTypeParm types

[Anders Langlands via Phabricator via cfe-commits]

anderslanglands updated this revision to Diff 476935.
anderslanglands added a comment.

Move getReplacementType to LLVM_16 block


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137818

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/test/Index/print-type.cpp
  clang/tools/libclang/CXType.cpp
  clang/tools/libclang/libclang.map

Index: clang/tools/libclang/libclang.map
===
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -412,6 +412,7 @@
 clang_CXXMethod_isDeleted;
 clang_CXXMethod_isCopyAssignmentOperator;
 clang_CXXMethod_isMoveAssignmentOperator;
+clang_Type_getReplacementType;
 };
 
 # Example of how to add a new symbol version entry.  If you do add a new symbol
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -118,13 +118,13 @@
 TKCASE(Attributed);
 TKCASE(BTFTagAttributed);
 TKCASE(Atomic);
+TKCASE(SubstTemplateTypeParm);
 default:
   return CXType_Unexposed;
   }
 #undef TKCASE
 }
 
-
 CXType cxtype::MakeCXType(QualType T, CXTranslationUnit TU) {
   CXTypeKind TK = CXType_Invalid;
 
@@ -635,6 +635,7 @@
 TKIND(OCLQueue);
 TKIND(OCLReserveID);
 TKIND(Atomic);
+TKIND(SubstTemplateTypeParm);
   }
 #undef TKIND
   return cxstring::createRef(s);
@@ -1355,3 +1356,11 @@
   const auto *AT = T->castAs();
   return MakeCXType(AT->getValueType(), GetTU(CT));
 }
+
+CXType clang_Type_getReplacementType(CXType CT) {
+  QualType T = GetQualType(CT);
+
+  const auto *ST =
+  !T.isNull() ? T->getAs() : nullptr;
+  return MakeCXType(ST ? ST->getReplacementType() : QualType(), GetTU(CT));
+}
Index: clang/test/Index/print-type.cpp
===
--- clang/test/Index/print-type.cpp
+++ clang/test/Index/print-type.cpp
@@ -171,7 +171,7 @@
 // CHECK: VarDecl=autoI:54:6 (Definition) [type=int] [typekind=Auto] [canonicaltype=int] [canonicaltypekind=Int] [isPOD=1]
 // CHECK: IntegerLiteral= [type=int] [typekind=Int] [isPOD=1]
 // CHECK: VarDecl=autoTbar:55:6 (Definition) [type=int] [typekind=Auto] [canonicaltype=int] [canonicaltypekind=Int] [isPOD=1]
-// CHECK: CallExpr=tbar:36:3 [type=int] [typekind=Unexposed] [canonicaltype=int] [canonicaltypekind=Int] [args= [int] [Int]] [isPOD=1]
+// CHECK: CallExpr=tbar:36:3 [type=int] [typekind=SubstTemplateTypeParm] [canonicaltype=int] [canonicaltypekind=Int] [args= [int] [Int]] [isPOD=1]
 // CHECK: UnexposedExpr=tbar:36:3 [type=int (*)(int)] [typekind=Pointer] [canonicaltype=int (*)(int)] [canonicaltypekind=Pointer] [isPOD=1] [pointeetype=int (int)] [pointeekind=FunctionProto]
 // CHECK: DeclRefExpr=tbar:36:3 RefName=[55:17 - 55:21] RefName=[55:21 - 55:26] [type=int (int)] [typekind=FunctionProto] [canonicaltype=int (int)] [canonicaltypekind=FunctionProto] [isPOD=0]
 // CHECK: IntegerLiteral= [type=int] [typekind=Int] [isPOD=1]
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -2789,7 +2789,9 @@
 
   CXType_ExtVector = 176,
   CXType_Atomic = 177,
-  CXType_BTFTagAttributed = 178
+  CXType_BTFTagAttributed = 178,
+  /* Represents a type that has been substituted for a template type parameter. */
+  CXType_SubstTemplateTypeParm = 179
 };
 
 /**
@@ -3447,6 +3449,13 @@
  */
 CINDEX_LINKAGE CXType clang_Type_getValueType(CXType CT);
 
+/**
+ * Gets the replacement type for a SubstTemplateTypeParm type.
+ *
+ * If any other type kind is passed in, an invalid type is returned.
+ */
+CINDEX_LINKAGE CXType clang_Type_getReplacementType(CXType CT);
+
 /**
  * Return the offset of the field represented by the Cursor.
  *
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -806,6 +806,10 @@
   ``clang_Cursor_getTemplateArgumentType``, ``clang_Cursor_getTemplateArgumentValue`` and 
   ``clang_Cursor_getTemplateArgumentUnsignedValue`` now work on struct, class,
   and partial template specialization cursors in addition to function cursors.
+- Added ``CXType_SubstTemplateTypeParm`` to ``CXTypeKind``, which identifies a type that 
+  is a replacement for a template type parameter (previously reported a ``CXType_Unexposed``).
+- Introduced the new function ``clang_Type_getReplacementType`` which gets the type replacing
+  the template type parameter when type kind is ``CXType_SubstTemplateTypeParm``.
 
 Static Analyzer
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136603: [analyzer] getBinding should auto-detect type only if it was not given

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

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

Sounds reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136603

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

[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

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

xazax.hun added a comment.

I did not spend too much time thinking about this yet, but this sounds scary. I 
wonder if we should target the underlying problem instead, i.e., making sure we 
never have dead symbols as representatives for eq. classes. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138037

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

[PATCH] D137346: [-Wunsafe-buffer-usage] Initial commit - Transition away from raw buffer accesses.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: njames93, sammccall, LegalizeAdulthood, klimek.
aaron.ballman added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:34
+// through the handler class.
+void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler);
+

Do we need the interface to accept a non-const reference?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11731-11732
+// Unsafe buffer usage diagnostics.
+def warn_unsafe_buffer_usage : Warning<"unchecked operation on raw buffer in 
expression">,
+  InGroup>, DefaultIgnore;
 } // end of sema component.

The diagnostic wording isn't wrong, but I am a bit worried about complex 
expressions. Consider something like `void func(a, b, c + d, e++, f(&g));` -- 
if you got this warning on that line of code, how likely would you be to spot 
what caused the diagnostic? I think we need to be sure that issuing this 
warning *always* passes in an extra `SourceRange` for the part of the 
expression that's caused the issue so users will at least get some underlined 
squiggles to help them out.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:50
+  M.addMatcher(
+  stmt(forEachDescendant(
+stmt(

Errr, this looks expensive to me, but maybe I'm forgetting (I can't recall if 
it's ancestor or descendant that's more expensive, I think it's ancestor 
though). @njames93 @LegalizeAdulthood @klimek @sammccall -- do you have 
concerns here or know of a better approach?



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:57
+  )
+  // FIXME: Idiomatically there should be a forCallable(equalsNode(D))
+  // here, to make sure that the statement actually belongs to the

Heh, this is the sort of thing I was worried about, especially when you 
consider things like lambda bodies or class definitions with member functions 
being declared in a function, etc.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2152-2154
+  void handleUnsafeOperation(const Stmt *Operation) override {
+S.Diag(Operation->getBeginLoc(), diag::warn_unsafe_buffer_usage);
+  }

I think this interface needs an additional `SourceRange` parameter that can be 
passed in as a streaming argument, along these lines. This way you'll get 
squiggles for the problematic part of the expression.



Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:1
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+

No need to pin to a specific language mode.


Repository:
  rC Clang

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

https://reviews.llvm.org/D137346

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

[PATCH] D137348: [-Wunsafe-buffer-usage] Introduce an abstraction for fixable code patterns.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def:1
+//=- UnsafeBufferUsageGadgets.def - List of ways to use a buffer --*- C++ 
-*-=//
+//

Is this file named properly if it is also going to handle `SAFE_GADGET`?



Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def:9
+
+#ifndef GADGET
+#define GADGET(name)

You have some comments below about what gadgets are and what makes one safe or 
unsafe; should those comments be hoisted into this .def file?



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:17-21
+static auto hasPointerType() {
+  return anyOf(hasType(pointerType()),
+   hasType(autoType(hasDeducedType(
+   hasUnqualifiedDesugaredType(pointerType());
 }

Something to think about: ObjC pointers are pointer-like but not actual 
pointers; should those be covered as well (via `isAnyPointerType()`)?



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:37
+#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
+#undef GADGETS
+  };

NoQ wrote:
> Whoops typo!
We don't even need the `#undef` because the .def file already does that for us. 
Same observation applies below.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:63
+
+  Gadget(Kind K) : K(K) {}
+

Should this be an explicit constructor? (Same question applies below to derived 
classes as well.)



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:113
+  : UnsafeGadget(Kind::Increment),
+Op(Result.Nodes.getNodeAs("op")) {}
+

Should we make a `static constexpr const char *` for these strings so we can 
give it a name and ensure it's used consistently?


Repository:
  rC Clang

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

https://reviews.llvm.org/D137348

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

[PATCH] D137379: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:168-169
+  static Matcher matcher() {
+// FIXME: What if the index is integer literal 0? Should this be
+// a safe gadget in this case?
+return stmt(

xazax.hun wrote:
> As per some of the discussions, in the future the compiler might be able to 
> recognize certain safe patterns, e.g., when there is a simple for loop with 
> known bounds, or when both the index and the array size is statically known.
> 
> I think here we need to make a very important design decision: Do we want the 
> gadgets to have the right "safety" category when it is created (e.g., we have 
> to be able to decide if a gadget is safe or not using matchers), or do we 
> want some mechanisms to be able to promote an unsafe gadget to be a safe one? 
> (E.g., do we want to be able to prove some unsafe gadgets safe using dataflow 
> analysis in a later pass?)
(FWIW, this is a great question and I really appreciate you asking it!)



Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:16
+void testArraySubscripts(int *p, int **pp) {
+  foo(p[0], // expected-warning{{unchecked operation on raw buffer 
in expression}}
+  pp[0][0], // expected-warning2{{unchecked operation on raw 
buffer in expression}}

One test case I'd like to see is: `sizeof(p[0])` -- should code in an 
unevaluated context be warned?



Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:43
+}
+
+void testArraySubscriptsWithAuto(int *p, int **pp) {

Can you also add tests for function declarations like:
```
void foo(int not_really_an_array[10]) { ... }

template 
void bar(int (&actually_an_array)[N]) { ... }

// Using a dependent type but we know it's a pointer.
template 
void baz(Ty *ptr) { ... }

// Using a dependent type where we have no idea if it's a pointer.
template 
void quux(Ty ptr) { ... }
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137379

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

[PATCH] D138289: [clang][Parse] Remove constant expression from if condition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found that this change breaks clang/test/Sema/PR28181.c which was 
introduced to fix https://bugs.llvm.org/show_bug.cgi?id=28181 which was a 
regression caused by 
https://github.com/llvm/llvm-project/commit/5a5319062300166a747807339349766341a2c2b2


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

https://reviews.llvm.org/D138289

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

[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd marked 5 inline comments as done.
compnerd added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:1605-1606
   // C99 6.9: External Definitions.
   DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &Attrs,
+  ParsedAttributes &DeclSpecAttrs,
   ParsingDeclSpec *DS = nullptr);

aaron.ballman wrote:
> Should we rename `Attrs` to `DeclaratorAttrs` or something along those lines, 
> so it's easier for someone to distinguish which attributes go where? (Same 
> suggestion applies throughout the review.)
Yes, `DeclAttrs` would be much nicer, but I wanted to get everything right 
before doing that and then forgot.  Thanks for the reminder.



Comment at: clang/lib/Parse/Parser.cpp:1096-1097
+ParsingDeclSpec &DS, AccessSpecifier AS) {
+  DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
+  DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
+  DS.takeAttributesFrom(DeclSpecAttrs);

aaron.ballman wrote:
> How sure are you that this isn't overwriting existing range data that's 
> potentially relevant? e.g., where declaration specifiers and attributes are 
> mixed together such that the attribute range doesn't cover all of the 
> declaration specifiers?
I don't believe it can - the range hasn't been setup yet when coming into this 
function, only the storage has been allocated.  This is the first place where 
we are initializing the source range.  If there are other unstated invariants, 
it would be nice to have some sort of assertion for them.


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

https://reviews.llvm.org/D137979

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

[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 476949.
compnerd marked an inline comment as done.
compnerd added a comment.

Address review feedback


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

https://reviews.llvm.org/D137979

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/attr-order.cpp
  clang/test/Parser/cxx-attributes.cpp
  clang/test/SemaCXX/attr-unavailable.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -247,14 +247,25 @@
 
   // Includes attributes.
   Visitor.runOverAnnotated(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   int x;]])cpp");
 
   // Includes attributes and comments together.
   Visitor.runOverAnnotated(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
+  // Comment.
+  int x;]])cpp");
+
+  // Includes attributes through macro expansion.
+  Visitor.runOverAnnotated(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
+  int x;]])cpp");
+
+  // Includes attributes through macro expansion with comments.
+  Visitor.runOverAnnotated(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
   // Comment.
   int x;]])cpp");
 }
@@ -402,14 +413,25 @@
 
   // Includes attributes.
   Visit(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   int x;]])cpp");
 
   // Includes attributes and comments together.
   Visit(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
+  // Comment.
+  int x;]])cpp");
+
+  // Includes attributes through macro expansion.
+  Visitor.runOverAnnotated(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
+  int x;]])cpp");
+
+  // Includes attributes through macro expansion with comments.
+  Visitor.runOverAnnotated(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
   // Comment.
   int x;]])cpp");
 }
Index: clang/test/SemaCXX/attr-unavailable.cpp
===
--- clang/test/SemaCXX/attr-unavailable.cpp
+++ clang/test/SemaCXX/attr-unavailable.cpp
@@ -42,18 +42,18 @@
 // delayed process for 'deprecated'.
 //  and 
 enum DeprecatedEnum { DE_A, DE_B } __attribute__((deprecated)); // expected-note {{'DeprecatedEnum' has been explicitly marked deprecated here}}
-__attribute__((deprecated)) typedef enum DeprecatedEnum DeprecatedEnum;
 typedef enum DeprecatedEnum AnotherDeprecatedEnum; // expected-warning {{'DeprecatedEnum' is deprecated}}
 
+__attribute__((deprecated)) typedef enum DeprecatedEnum DeprecatedEnum;
 __attribute__((deprecated))
 DeprecatedEnum testDeprecated(DeprecatedEnum X) { return X; }
 
 
 enum UnavailableEnum { UE_A, UE_B } __attribute__((unavailable)); // expected-note {{'UnavailableEnum' has been explicitly marked unavailable here}}
-__attribute__((unavailable)) typedef enum UnavailableEnum UnavailableEnum;
 typedef enum UnavailableEnum AnotherUnavailableEnum; // expected-error {{'UnavailableEnum' is unavailable}}
+ //
 
-
+__attribute__((unavailable)) typedef enum UnavailableEnum UnavailableEnum;
 __attribute__((unavailable))
 UnavailableEnum testUnavailable(UnavailableEnum X) { return X; }
 
Index: clang/test/Parser/cxx-attributes.cpp
===
--- clang/test/Parser/cxx-attributes.cpp
+++ clang/test/Parser/cxx-attributes.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+// GH#58229 - rejects-valid
+__attribute__((__visibility__("default"))) [[nodiscard]] int f();
+[[nodiscard]] __attribute__((__visibility__("default"))) int f();
+
 class c {
   virtual void f1(const char* a, ...)
 __attribute__ (( __format__(__printf__,2,3) )) = 0;
Index: clang/test/Parser/attr-order.cpp
===
--- clang/test/Parser/attr-order.cpp
+++ clang/test/Parser/attr-order.cpp
@@ -17,8 +17,8 @@
 __declspec(dllexport) [[noreturn]] __attribute__((cdecl)) void d(); // expected-error {{an attribute list cannot appear here}}
 __declspec(dllexport) __attribute__((cdecl)) [[noreturn]] void e(); // expected-error {{an attribute list cannot appear here}}
 __attribute

[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Parse/Parser.cpp:1096-1097
+ParsingDeclSpec &DS, AccessSpecifier AS) {
+  DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
+  DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
+  DS.takeAttributesFrom(DeclSpecAttrs);

compnerd wrote:
> aaron.ballman wrote:
> > How sure are you that this isn't overwriting existing range data that's 
> > potentially relevant? e.g., where declaration specifiers and attributes are 
> > mixed together such that the attribute range doesn't cover all of the 
> > declaration specifiers?
> I don't believe it can - the range hasn't been setup yet when coming into 
> this function, only the storage has been allocated.  This is the first place 
> where we are initializing the source range.  If there are other unstated 
> invariants, it would be nice to have some sort of assertion for them.
Ah, so we could perhaps assert that the range is invalid on entry and leave a 
comment explaining why we're asserting/what's going on?


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

https://reviews.llvm.org/D137979

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

[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from the question about whether we want to add an assert or not (I 
don't insist on adding one) and adding a release note (which doesn't need 
further review unless you want it). Thank you for these changes!


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

https://reviews.llvm.org/D137979

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