[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 updated this revision to Diff 364938.
benshi001 retitled this revision from "[AVR][clang] Search avr-libc installtion 
path according to avr-gcc's" to "[AVR][clang] Improve search for avr-libc 
installation path".
benshi001 edited the summary of this revision.

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

https://reviews.llvm.org/D107682

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,6 +453,11 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  std::string GCCRoot(GCCInstallation.getInstallPath());
+  std::string Path(GCCRoot + "/../../../avr");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
 // Return the first avr-libc installation that exists.


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,6 +453,11 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  std::string GCCRoot(GCCInstallation.getInstallPath());
+  std::string Path(GCCRoot + "/../../../avr");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
 // Return the first avr-libc installation that exists.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Matt Jacobson via Phabricator via cfe-commits 
mhjacobson added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:457
+  std::string GCCRoot(GCCInstallation.getInstallPath());
+  std::string Path(GCCRoot + "/../../../avr");
+  if (llvm::sys::fs::is_directory(Path))

Would it be worth encapsulating this logic in an accessor method on 
GCCInstallation?


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

https://reviews.llvm.org/D107682

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 updated this revision to Diff 364940.

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

https://reviews.llvm.org/D107682

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,9 +453,16 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  // Search avr-libc installation according to avr-gcc installation.
+  std::string GCCRoot(GCCInstallation.getInstallPath());
+  std::string Path(GCCRoot + "/../../../avr");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
+  // Search avr-libc installation from possible locations, and return the first
+  // one that exists, if there is no avr-gcc installed.
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
-// Return the first avr-libc installation that exists.
 if (llvm::sys::fs::is_directory(Path))
   return Optional(Path);
   }


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,9 +453,16 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  // Search avr-libc installation according to avr-gcc installation.
+  std::string GCCRoot(GCCInstallation.getInstallPath());
+  std::string Path(GCCRoot + "/../../../avr");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
+  // Search avr-libc installation from possible locations, and return the first
+  // one that exists, if there is no avr-gcc installed.
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
-// Return the first avr-libc installation that exists.
 if (llvm::sys::fs::is_directory(Path))
   return Optional(Path);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 marked an inline comment as done.
benshi001 added a comment.

Thanks. I have added some comments about our agreed logic, as discussed in 
https://reviews.llvm.org/D107672.

I think this way is clear enough.


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

https://reviews.llvm.org/D107682

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added a comment.

@mhjacobson Could you please check if my modification also works as expected on 
your platform ?


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

https://reviews.llvm.org/D107682

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

[PATCH] D104975: Implement P1949

2021-08-07 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin updated this revision to Diff 364941.
cor3ntin added a comment.

Update cxx_status


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/UnicodeCharSets.h
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/FixIt/fixit-unicode.c
  clang/test/Lexer/unicode.c
  clang/test/Parser/cxx11-user-defined-literals.cpp
  clang/test/Preprocessor/ucn-allowed-chars.c
  clang/test/Preprocessor/utf8-allowed-chars.c
  clang/www/cxx_status.html
  llvm/include/llvm/Support/UnicodeCharRanges.h

Index: llvm/include/llvm/Support/UnicodeCharRanges.h
===
--- llvm/include/llvm/Support/UnicodeCharRanges.h
+++ llvm/include/llvm/Support/UnicodeCharRanges.h
@@ -62,6 +62,14 @@
   /// Returns true if the character set contains the Unicode code point
   /// \p C.
   bool contains(uint32_t C) const {
+if (C < 127) {
+  for (const auto &R : Ranges) {
+if (R.Lower <= C && R.Upper >= C)
+  return true;
+if (R.Lower > C)
+  return false;
+  }
+}
 return std::binary_search(Ranges.begin(), Ranges.end(), C);
   }
 
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1311,7 +1311,7 @@
 
   C++ identifier syntax using UAX 31
   https://wg21.link/P1949R7";>P1949R7
-  No
+  Clang 14
 
 
   Mixed string literal concatenation
Index: clang/test/Preprocessor/utf8-allowed-chars.c
===
--- clang/test/Preprocessor/utf8-allowed-chars.c
+++ clang/test/Preprocessor/utf8-allowed-chars.c
@@ -23,46 +23,30 @@
 
 
 
-
-
-
-
 #if __cplusplus
-# if __cplusplus >= 201103L
-// C++11
-// expected-warning@9 {{using this character in an identifier is incompatible with C++98}}
-// expected-warning@10 {{using this character in an identifier is incompatible with C++98}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-warning@14 {{using this character in an identifier is incompatible with C++98}}
+// expected-error@11 {{not allowed in identifiers}}
+// expected-error@13 {{not allowed in identifiers}}
+// expected-error@20 {{expected unqualified-id}}
 // expected-error@21 {{expected unqualified-id}}
 
-# else
-// C++03
-// expected-error@9 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@10 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@14 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@21 {{non-ASCII characters are not allowed outside of literals and identifiers}} expected-warning@21 {{declaration does not declare anything}}
-
-# endif
 #else
 # if __STDC_VERSION__ >= 201112L
 // C11
 // expected-warning@9 {{using this character in an identifier is incompatible with C99}}
 // expected-warning@11 {{using this character in an identifier is incompatible with C99}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
+// expected-error@13 {{not allowed in identifiers}}
 // expected-warning@14 {{using this character in an identifier is incompatible with C99}}
 // expected-warning@20 {{starting an identifier with this character is incompatible with C99}}
 // expected-error@21 {{expected identifier}}
 
 # else
 // C99
-// expected-error@9 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@11 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@14 {{non-ASCII characters are not allowed outside of literals and identifiers}}
+// expected-error@9 {{not allowed in identifiers}}
+// expected-error@11 {{not allowed in identifiers}}
+// expected-error@13 {{allowed in identifiers}}
+// expected-error@14 {{not allowed in identifiers}}
 // expected-error@20 {{expected identifier}}
-// expected-error@21 {{non-ASCII characters are not allowed outside of literals and identifiers}} expected-warning@21 {{declaration does not declare anything}}
+// expected-error@21 {{not allowed in identifiers}} expected-warning@21 {{declaration does not declare anything}}
 
 # endif
 #endif
Index: clang/test/Preprocessor/ucn-allowed-chars.c
===
--- clang/test/Preprocessor/ucn-allowed-chars.c
+++ clang/test/Preprocessor/ucn-allowed-chars.c
@@ -16,7 +16,7 @@

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added inline comments.



Comment at: clang/test/Driver/avr-toolchain.c:9
 
-// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1B %s
-// CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree_opt_local/opt/local 2>&1 | FileCheck -check-prefix 
CC1B %s
+// CC1B: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*/opt/local/avr/include"}}

Please do not change existing tests, you can add new lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added a comment.

We need not add another `basic_avr_tree_opt_local`, since you added `/avr` to 
possible avr-libc pathes, you can test your change by specifying `--sysroot  
%S/Inputs/basic_avr_tree/usr/lib/` .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Matt Jacobson via Phabricator via cfe-commits 
mhjacobson added a comment.

In D107682#2932599 , @benshi001 wrote:

> @mhjacobson Could you please check if my modification also works as expected 
> on your platform ?

Hm... it doesn't, because there aren't enough `..`s.

Here's what my directory tree looks like.  My gcc install consists of:

  /opt/local/bin/avr-gcc-10.3.0
  /opt/local/lib/gcc/avr/10.3.0/libgcc.a
  /opt/local/libexec/gcc/avr/10.3.0/cc1

And avr-libc is at:

  /opt/local/avr/lib/libc.a
  /opt/local/avr/include/stdio.h

It would work if you appended four `..`s (which is effectively what GCC does, 
since it backs up to the "prefix", which in my case is `/opt/local/`).


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

https://reviews.llvm.org/D107682

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Matt Jacobson via Phabricator via cfe-commits 
mhjacobson added a comment.

In D107684#2932603 , @benshi001 wrote:

> We need not add another `basic_avr_tree_opt_local`, since you added `/avr` to 
> possible avr-libc pathes, you can test your change by specifying `--sysroot  
> %S/Inputs/basic_avr_tree/usr/lib/` .

Oh, good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Matt Jacobson via Phabricator via cfe-commits 
mhjacobson added a comment.

For other examples of using four `..`s, look in `Gnu.cpp`, where there are 
several places where `getParentLibPath()` is appended with `/../`.  
`AddMultilibPaths()`, `PushPPaths()`.

`getParentLibPath()`, in turn, is `GCCInstallPath/../../../`


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

https://reviews.llvm.org/D107682

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added a comment.

In D107684#2932605 , @mhjacobson 
wrote:

> In D107684#2932603 , @benshi001 
> wrote:
>
>> We need not add another `basic_avr_tree_opt_local`, since you added `/avr` 
>> to possible avr-libc pathes, you can test your change by specifying 
>> `--sysroot  %S/Inputs/basic_avr_tree/usr/lib/` .
>
> Oh, good idea.

Sorry, only spefifying `--sysroot  %S/Inputs/basic_avr_tree/usr/lib/` does not 
work, since the search for avr-ld also relies on `--sysroot`.

So I suggest adding a new dir name `avr` inside `Inputs/basic_avr_tree/`, and 
also put a copy of the fake avr-libc files at `Inputs/basic_avr_tree/avr`.

In this way the searchs for both avr-gcc and avr-libc should  work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Matt Jacobson via Phabricator via cfe-commits 
mhjacobson updated this revision to Diff 364942.
mhjacobson added a comment.

Simplify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

Files:
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -11,3 +11,7 @@
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck -check-prefix CC1C %s
 // CC1C-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// Test that the driver finds an avr-libc installation at `$SYSROOT/avr/`.
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree/usr/lib 2>&1 | FileCheck -check-prefix CC1D %s
+// CC1D: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -11,3 +11,7 @@
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck -check-prefix CC1C %s
 // CC1C-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// Test that the driver finds an avr-libc installation at `$SYSROOT/avr/`.
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree/usr/lib 2>&1 | FileCheck -check-prefix CC1D %s
+// CC1D: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Matt Jacobson via Phabricator via cfe-commits 
mhjacobson added a comment.

In D107684#2932609 , @benshi001 wrote:

> In D107684#2932605 , @mhjacobson 
> wrote:
>
>> In D107684#2932603 , @benshi001 
>> wrote:
>>
>>> We need not add another `basic_avr_tree_opt_local`, since you added `/avr` 
>>> to possible avr-libc pathes, you can test your change by specifying 
>>> `--sysroot  %S/Inputs/basic_avr_tree/usr/lib/` .
>>
>> Oh, good idea.
>
> Sorry, only spefifying `--sysroot  %S/Inputs/basic_avr_tree/usr/lib/` does 
> not work, since the search for avr-ld also relies on `--sysroot`.

For the purpose of the test, that doesn't matter, since we're just looking for 
the existence of the `-internal-isystem` argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

2021-08-07 Thread Kristina Bessonova via Phabricator via cfe-commits 
krisb accepted this revision.
krisb added a comment.
This revision is now accepted and ready to land.

@aorlov, thank you! LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92024

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added a comment.

@MaskRay

I think this test is OK. What is your opinion ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107693: [Parser] Fix attr infloop on "int x [[c"

2021-08-07 Thread Sam McCall via Phabricator via cfe-commits 
sammccall created this revision.
sammccall added a reviewer: aaron.ballman.
Herald added a subscriber: jdoerfert.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Similar to ad2d6bbb1435cef0a048c9aed3dcf9617640f222 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107693

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/cxx-attributes.cpp


Index: clang/test/Parser/cxx-attributes.cpp
===
--- clang/test/Parser/cxx-attributes.cpp
+++ clang/test/Parser/cxx-attributes.cpp
@@ -42,3 +42,6 @@
 [[,,maybe_unused,]] int Commas4; // ok
 [[foo bar]] int NoComma; // expected-error {{expected ','}} \
  // expected-warning {{unknown attribute 'foo' 
ignored}}
+// expected-error@+2 2 {{expected ']'}}
+// expected-error@+1 {{expected external declaration}}
+[[foo
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4385,7 +4385,7 @@
   llvm::SmallDenseMap SeenAttrs;
 
   bool AttrParsed = false;
-  while (!Tok.isOneOf(tok::r_square, tok::semi)) {
+  while (!Tok.isOneOf(tok::r_square, tok::semi, tok::eof)) {
 if (AttrParsed) {
   // If we parsed an attribute, a comma is required before parsing any
   // additional attributes.


Index: clang/test/Parser/cxx-attributes.cpp
===
--- clang/test/Parser/cxx-attributes.cpp
+++ clang/test/Parser/cxx-attributes.cpp
@@ -42,3 +42,6 @@
 [[,,maybe_unused,]] int Commas4; // ok
 [[foo bar]] int NoComma; // expected-error {{expected ','}} \
  // expected-warning {{unknown attribute 'foo' ignored}}
+// expected-error@+2 2 {{expected ']'}}
+// expected-error@+1 {{expected external declaration}}
+[[foo
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4385,7 +4385,7 @@
   llvm::SmallDenseMap SeenAttrs;
 
   bool AttrParsed = false;
-  while (!Tok.isOneOf(tok::r_square, tok::semi)) {
+  while (!Tok.isOneOf(tok::r_square, tok::semi, tok::eof)) {
 if (AttrParsed) {
   // If we parsed an attribute, a comma is required before parsing any
   // additional attributes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 updated this revision to Diff 364954.

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

https://reviews.llvm.org/D107682

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,9 +453,19 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  // Search avr-libc installation according to avr-gcc installation.
+  std::string GCCParent(GCCInstallation.getParentLibPath());
+  std::string Path(GCCParent + "/avr/");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+  Path = GCCParent + "/../avr/";
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
+  // Search avr-libc installation from possible locations, and return the first
+  // one that exists, if there is no avr-gcc installed.
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
-// Return the first avr-libc installation that exists.
 if (llvm::sys::fs::is_directory(Path))
   return Optional(Path);
   }


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,9 +453,19 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  // Search avr-libc installation according to avr-gcc installation.
+  std::string GCCParent(GCCInstallation.getParentLibPath());
+  std::string Path(GCCParent + "/avr/");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+  Path = GCCParent + "/../avr/";
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
+  // Search avr-libc installation from possible locations, and return the first
+  // one that exists, if there is no avr-gcc installed.
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
-// Return the first avr-libc installation that exists.
 if (llvm::sys::fs::is_directory(Path))
   return Optional(Path);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added a comment.

In D107682#2932607 , @mhjacobson 
wrote:

> For other examples of using four `..`s, look in `Gnu.cpp`, where there are 
> several places where `getParentLibPath()` is appended with `/../`.  
> `AddMultilibPaths()`, `PushPPaths()`.
>
> `getParentLibPath()`, in turn, is `GCCInstallPath/../../../`

Thanks for your help. I have updated my patch. Now it should work for both your 
platform and mine. It search `GCCRoot.getParentLibPath()/avr` first, otherwise 
`GCCRoot.getParentLibPath()/../avr`.

Please help me check.


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

https://reviews.llvm.org/D107682

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

2021-08-07 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 updated this revision to Diff 364955.
benshi001 edited the summary of this revision.
Herald added a subscriber: ormris.

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

https://reviews.llvm.org/D107682

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,9 +453,19 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  // Search avr-libc installation according to avr-gcc installation.
+  std::string GCCParent(GCCInstallation.getParentLibPath());
+  std::string Path(GCCParent + "/avr");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+  Path = GCCParent + "/../avr";
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
+  // Search avr-libc installation from possible locations, and return the first
+  // one that exists, if there is no avr-gcc installed.
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
-// Return the first avr-libc installation that exists.
 if (llvm::sys::fs::is_directory(Path))
   return Optional(Path);
   }


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -4,7 +4,7 @@
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
-// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
+// CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*../../../avr/include"}}
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck -check-prefix CC1B %s
 // CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -453,9 +453,19 @@
 }
 
 llvm::Optional AVRToolChain::findAVRLibcInstallation() const {
+  // Search avr-libc installation according to avr-gcc installation.
+  std::string GCCParent(GCCInstallation.getParentLibPath());
+  std::string Path(GCCParent + "/avr");
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+  Path = GCCParent + "/../avr";
+  if (llvm::sys::fs::is_directory(Path))
+return Optional(Path);
+
+  // Search avr-libc installation from possible locations, and return the first
+  // one that exists, if there is no avr-gcc installed.
   for (StringRef PossiblePath : PossibleAVRLibcLocations) {
 std::string Path = getDriver().SysRoot + PossiblePath.str();
-// Return the first avr-libc installation that exists.
 if (llvm::sys::fs::is_directory(Path))
   return Optional(Path);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 618543b - [clang][NFC] Fix a -Wparentheses warning.

2021-08-07 Thread Craig Topper via cfe-commits 

Author: Craig Topper
Date: 2021-08-07T08:56:31-07:00
New Revision: 618543bb120b5c92e692a4b22deb59012e047c9d

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

LOG: [clang][NFC] Fix a -Wparentheses warning.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 96bbc0250126..956b31e18ef8 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4747,7 +4747,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 if (StringRef(A->getValue()).getAsInteger(10, Value) || Value > 65536)
   TC.getDriver().Diag(diag::err_drv_invalid_int_value)
   << A->getAsString(Args) << A->getValue();
-else if (Value & Value - 1)
+else if (Value & (Value - 1))
   TC.getDriver().Diag(diag::err_drv_alignment_not_power_of_two)
   << A->getAsString(Args) << A->getValue();
 // Treat =0 as unspecified (use the target preference).



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

[PATCH] D107696: [CodeComplete] Basic code completion for attribute names.

2021-08-07 Thread Sam McCall via Phabricator via cfe-commits 
sammccall created this revision.
sammccall added reviewers: aaron.ballman, kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Only the bare name is completed, with no args.
For args to be useful we need arg names. These *are* in the tablegen but
not currently emitted in usable form, so left this as future work.

C++11, C2x, GNU, declspec, MS syntax is supported, with the appropriate
spellings of attributes suggested.
`#pragma clang attribute` is supported but not terribly useful as we
only reach completion if parens are balanced (i.e. the line is not truncated)

There's no filtering of which attributes might make sense in this
grammatical context (e.g. attached to a function). In code-completion context
this is hard to do, and will only work in few cases :-(

There's also no filtering by langopts: this is because currently the
only way of checking is to try to produce diagnostics, which requires a
valid ParsedAttr which is hard to get.
This should be fairly simple to fix but requires some tablegen changes
to expose the logic without the side-effect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107696

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/ParsedAttr.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/attr.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp

Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -541,6 +541,7 @@
 case CodeCompletionContext::CCC_MacroName:
 case CodeCompletionContext::CCC_PreprocessorExpression:
 case CodeCompletionContext::CCC_PreprocessorDirective:
+case CodeCompletionContext::CCC_Attribute:
 case CodeCompletionContext::CCC_TypeQualifiers: {
   //Only Clang results should be accepted, so we'll set all of the other
   //context bits to 0 (i.e. the empty set)
Index: clang/test/CodeCompletion/attr.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/attr.cpp
@@ -0,0 +1,53 @@
+int a [[gnu::used]];
+// RUN: %clang_cc1 -code-completion-at=%s:1:9 %s | FileCheck --check-prefix=STD %s
+// STD: COMPLETION: carries_dependency
+// STD: COMPLETION: clang::convergent
+// STD-NOT: COMPLETION: convergent
+// STD: COMPLETION: gnu::used
+// STD-NOT: COMPLETION: used
+// RUN: %clang_cc1 -code-completion-at=%s:1:14 %s | FileCheck --check-prefix=STD-NS %s
+// STD-NS-NOT: COMPLETION: carries_dependency
+// STD-NS-NOT: COMPLETION: clang::convergent
+// STD-NS-NOT: COMPLETION: convergent
+// STD-NS-NOT: COMPLETION: gnu::used
+// STD-NS: COMPLETION: used
+
+int b [[using gnu: used]];
+// RUN: %clang_cc1 -code-completion-at=%s:15:15 %s | FileCheck --check-prefix=STD-USING %s
+// STD-USING-NOT: COMPLETION: carries_dependency
+// STD-USING: COMPLETION: clang
+// STD-USING-NOT: COMPLETION: clang::
+// STD-USING-NOT: COMPLETION: gnu::
+// STD-USING: COMPLETION: gnu
+// RUN: %clang_cc1 -code-completion-at=%s:15:20 %s | FileCheck --check-prefix=STD-NS %s
+
+int c __attribute__((used));
+// RUN: %clang_cc1 -code-completion-at=%s:24:22 %s | FileCheck --check-prefix=GNU %s
+// GNU: COMPLETION: carries_dependency
+// GNU-NOT: COMPLETION: clang::convergent
+// GNU: COMPLETION: convergent
+// GNU-NOT: COMPLETION: gnu::used
+// GNU: COMPLETION: used
+
+#pragma clang attribute push (__attribute__((internal_linkage)), apply_to=variable)
+int d;
+#pragma clang attribute pop
+// RUN: %clang_cc1 -code-completion-at=%s:32:46 %s | FileCheck --check-prefix=PRAGMA %s
+// PRAGMA: internal_linkage
+
+int __declspec(thread) e;
+// RUN: %clang_cc1 -fms-extensions -code-completion-at=%s:38:16 %s | FileCheck --check-prefix=DS %s
+// DS-NOT: COMPLETION: clang::convergent
+// DS-NOT: COMPLETION: convergent
+// DS: COMPLETION: thread
+// DS-NOT: COMPLETION: used
+// DS: COMPLETION: uuid
+
+[uuid("123e4567-e89b-12d3-a456-426614174000")] struct f;
+// RUN: %clang_cc1 -fms-extensions -code-completion-at=%s:46:2 %s | FileCheck --check-prefix=MS %s
+// MS-NOT: COMPLETION: clang::convergent
+// MS-NOT: COMPLETION: convergent
+// MS-NOT: COMPLETION: thread
+// MS-NOT: COMPLETION: used
+// MS: COMPLETION: uuid
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.c

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

2021-08-07 Thread Nemanja Ivanovic via Phabricator via cfe-commits 
nemanjai added a comment.

In D107242#2932289 , @joerg wrote:

> I'm puzzled by this change. I don't think we have any case so far where the 
> compiler behavior changes with the host OS and I don't think it should. 
> What's the point / use case of this macro?

I agree that this is strange. And strange things need good documentation. The 
documentation for this extremely strange behaviour is woefully inadequate both 
in the code and in the commit message. Of course, I am not certain why this is 
needed but I assume it has something to do with the fact that it is the OS on 
AIX that owns the headers so the compiler needs to know whether it is running 
on an AIX host due to some non-portable code in the headers. Of course, this 
can't be the full explanation as it brings up many questions. This needs to be 
very clear to the reader - both in the commit message and in the code.

Another thing that I find extremely strange is that we set this only if we are 
on an AIX host **and** are targeting AIX. If this is truly something that has 
to do with the host (which I am not positive is the case as I'm not sure what 
`HOS` in the macro name means), then why is it not needed when cross-compiling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107242

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

[PATCH] D107138: [PowerPC] Implement cmplxl builtins

2021-08-07 Thread Nemanja Ivanovic via Phabricator via cfe-commits 
nemanjai added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-complex.c:1
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s

Conanap wrote:
> NeHuang wrote:
> > NeHuang wrote:
> > > `// REQUIRES: powerpc-registered-target`
> > Question: why do we need `-O2` for this builtin?
> > 
> it's not required, but removes a lot of the extra load and stores that make 
> the test cases longer unnecessarily. I can change it to O1 if preferred.
I prefer that front end tests should test what the front end does. The front 
end does not perform optimizations, the optimizer does. So the front end tests 
should not include optimization options (even though I do realize that adding 
-O reduces the size of the produced code).
There is no need to add unnecessary churn in front end tests if the optimizer 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107138

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

[clang] a382a74 - [clang] Fix libclang linking on Solaris

2021-08-07 Thread Rainer Orth via cfe-commits 

Author: Rainer Orth
Date: 2021-08-07T21:14:15+02:00
New Revision: a382a746275b4a1eb1867331efa2319c614305d8

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

LOG: [clang] Fix libclang linking on Solaris

Linking `libclang.so` is currently broken on Solaris:

  ld: fatal: option --version-script requires option -z 
gnu-version-script-compat to be specified

While Solaris `ld` supports a considerable subset of `--version-script`,
there are some elements of the syntax that aren't.

The fix is equivalent to D78510 .

Additionally, use of C-style comments is a GNU extension
that can easily be avoided by using `#` as comment character, which is
supported by GNU `ld`, `gold`, and `lld`.

Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`,
`x86_64-pc-linux-gnu`.

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

Added: 


Modified: 
clang/tools/libclang/CMakeLists.txt
clang/tools/libclang/libclang.map

Removed: 




diff  --git a/clang/tools/libclang/CMakeLists.txt 
b/clang/tools/libclang/CMakeLists.txt
index 8cc0fc622174a..7148bdccfba48 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -162,6 +162,11 @@ if(ENABLE_SHARED)
   endif()
   if (USE_VERSION_SCRIPT)
 target_link_options(libclang PRIVATE 
"-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
+# The Solaris 11.4 linker supports a subset of GNU ld version scripts,
+# but requires a special option to enable it.
+if (${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
+  target_link_options(libclang PRIVATE "-Wl,-z,gnu-version-script-compat")
+endif()
 # Ensure that libclang.so gets rebuilt when the linker script changes.
 set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
  OBJECT_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map)

diff  --git a/clang/tools/libclang/libclang.map 
b/clang/tools/libclang/libclang.map
index aee46b1448457..716e2474966d5 100644
--- a/clang/tools/libclang/libclang.map
+++ b/clang/tools/libclang/libclang.map
@@ -1,10 +1,8 @@
-/* If you add a symbol to this file, make sure to add it with the correct
- * version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
- * symbols with the version LLVM_14.
- * On platforms where versions scripts are not used, this file will be used to
- * generate a list of exports for libclang.so
- */
-
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
 
 LLVM_13 {
   global:
@@ -407,10 +405,9 @@ LLVM_13 {
   local: *;
 };
 
-/* Example of how to add a new symbol version entry.  If you do add a new 
symbol
- * version, please update the example to depend on the version you added.
- * LLVM_X {
- * global:
- *clang_newsymbol;
- * };
- */
+# Example of how to add a new symbol version entry.  If you do add a new symbol
+# version, please update the example to depend on the version you added.
+# LLVM_X {
+# global:
+#   clang_newsymbol;
+# };



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

[PATCH] D107559: [clang] Fix libclang linking on Solaris

2021-08-07 Thread Rainer Orth via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa382a746275b: [clang] Fix libclang linking on Solaris 
(authored by ro).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107559

Files:
  clang/tools/libclang/CMakeLists.txt
  clang/tools/libclang/libclang.map


Index: clang/tools/libclang/libclang.map
===
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -1,10 +1,8 @@
-/* If you add a symbol to this file, make sure to add it with the correct
- * version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
- * symbols with the version LLVM_14.
- * On platforms where versions scripts are not used, this file will be used to
- * generate a list of exports for libclang.so
- */
-
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
 
 LLVM_13 {
   global:
@@ -407,10 +405,9 @@
   local: *;
 };
 
-/* Example of how to add a new symbol version entry.  If you do add a new 
symbol
- * version, please update the example to depend on the version you added.
- * LLVM_X {
- * global:
- *clang_newsymbol;
- * };
- */
+# Example of how to add a new symbol version entry.  If you do add a new symbol
+# version, please update the example to depend on the version you added.
+# LLVM_X {
+# global:
+#   clang_newsymbol;
+# };
Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -162,6 +162,11 @@
   endif()
   if (USE_VERSION_SCRIPT)
 target_link_options(libclang PRIVATE 
"-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
+# The Solaris 11.4 linker supports a subset of GNU ld version scripts,
+# but requires a special option to enable it.
+if (${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
+  target_link_options(libclang PRIVATE "-Wl,-z,gnu-version-script-compat")
+endif()
 # Ensure that libclang.so gets rebuilt when the linker script changes.
 set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
  OBJECT_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map)


Index: clang/tools/libclang/libclang.map
===
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -1,10 +1,8 @@
-/* If you add a symbol to this file, make sure to add it with the correct
- * version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
- * symbols with the version LLVM_14.
- * On platforms where versions scripts are not used, this file will be used to
- * generate a list of exports for libclang.so
- */
-
+# If you add a symbol to this file, make sure to add it with the correct
+# version.  For example, if the LLVM main branch is LLVM 14.0.0, add new
+# symbols with the version LLVM_14.
+# On platforms where versions scripts are not used, this file will be used to
+# generate a list of exports for libclang.so
 
 LLVM_13 {
   global:
@@ -407,10 +405,9 @@
   local: *;
 };
 
-/* Example of how to add a new symbol version entry.  If you do add a new symbol
- * version, please update the example to depend on the version you added.
- * LLVM_X {
- * global:
- *clang_newsymbol;
- * };
- */
+# Example of how to add a new symbol version entry.  If you do add a new symbol
+# version, please update the example to depend on the version you added.
+# LLVM_X {
+# global:
+#   clang_newsymbol;
+# };
Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -162,6 +162,11 @@
   endif()
   if (USE_VERSION_SCRIPT)
 target_link_options(libclang PRIVATE "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
+# The Solaris 11.4 linker supports a subset of GNU ld version scripts,
+# but requires a special option to enable it.
+if (${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
+  target_link_options(libclang PRIVATE "-Wl,-z,gnu-version-script-compat")
+endif()
 # Ensure that libclang.so gets rebuilt when the linker script changes.
 set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
  OBJECT_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-08-07 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire added a comment.

Making a separate tool for this makes no sense. Especially as you are only 
proposing it to satisfy one (or are there more) vocal objector.

The objections to this make no sense. If you don't want to use it, then don't 
enable it. That principle applies whether "the way to enable it" is "enable 
this option" or "use this other tool instead". The latter is just more 
inconvenient. There is no other difference. Don't impose that on users just 
because of unreasonable objection.

Maintainership sometimes means discarding objections that make no sense.

It seems to me that there are two ways forward:

1. Land this (Yes. Do this. This is what makes sense)

2.

- Expose this feature in another tool with a very similar name
- Somehow tell users that the other tool exists and should be used instead
- Realize some years from now that we have two tools where we should have one 
and this is inconvenient for users
- Merge your new tool into clang-format
- Rejoice and wish this had been done in 2020

The outcome is the same, but it's a real disrespect to users to not just land 
this in clang-format now. The objection to doing so makes no sense, so it 
should be considered and dismissed (it has not been ignored).


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

https://reviews.llvm.org/D69764

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-07 Thread Florin Iucha via Phabricator via cfe-commits 
0x8000- marked an inline comment as done.
0x8000- added a comment.

Added regex for exception names and rebased onto current 'main' branch.




Comment at: 
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136
+CheckFactories.registerCheck(
+"readability-variable-length");
   }

aaron.ballman wrote:
> Is there a reason this should be restricted to variables? For example, 
> wouldn't the same functionality be useful for type names, or dare I say it, 
> even macro names? I'm wondering if this should be 
> `readability-identifier-length` to be more generic.
I consider those to be in separate namespaces. I suppose we could have a single 
checker with multiple rules, but on the other hand I don't want to combine too 
many things into one, just because they share the "compare length" dimension.



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:20
+
+const unsigned DefaultMinimumVariableNameLength = 3;
+const unsigned DefaultMinimumLoopCounterNameLength = 2;

aaron.ballman wrote:
> Should it be possible to enforce parameters differently than local variables? 
> (It seems a bit odd that we'd allow you to specify exception variable length 
> separate from loops but not function parameters separate from locals.)
Exceptions and Loops historically are "throw-away" / "don't care" / "one-off" 
variables. Parameter names are names, and they are the interface between the 
outside of the routine and the processing inside; other than historical, I 
don't see good arguments (sic) to allow single-letter parameter names.

Note that this check will be quite noisy on a legacy code base, and people will 
find little reason to invest the work to remove the warnings. But if somebody 
starts something new and want to enforce some quality attributes, it is the 
right tool at the right time. There will be new projects starting in 2021 and 
2022 that could benefit from this, I hope.



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:26
+
+const char ErrorMessage[] = "%select{|exception |loop }0 variable name %1 is "
+"too short, expected at least %2 characters";

aaron.ballman wrote:
> It looks like there will be whitespace issues with this. If the variable is a 
> loop or exception, it'll have an extra space (`loop  variable name`) and if 
> it's not, it will start with a leading space (` variable name`).
This was recommended by a previous reviewer. Any alternative suggestion here?


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

https://reviews.llvm.org/D97753

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

[PATCH] D107703: [AST][clangd] Expose documentation of Attrs on hover.

2021-08-07 Thread Sam McCall via Phabricator via cfe-commits 
sammccall created this revision.
sammccall added reviewers: aaron.ballman, kadircet.
Herald added subscribers: usaxena95, arphaman, mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

This adds a method to Attr to get at the documentation programmatically.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107703

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/AST/Attr.h
  clang/lib/AST/CMakeLists.txt
  clang/utils/TableGen/ClangAttrEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -61,6 +61,7 @@
llvm::raw_ostream &OS);
 void EmitClangAttrNodeTraverse(llvm::RecordKeeper &Records,
llvm::raw_ostream &OS);
+void EmitClangAttrDocTable(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
 void EmitClangDiagsDefs(llvm::RecordKeeper &Records, llvm::raw_ostream &OS,
 const std::string &Component);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -30,6 +30,7 @@
   GenClangAttrSubjectMatchRulesParserStringSwitches,
   GenClangAttrImpl,
   GenClangAttrList,
+  GenClangAttrDocTable,
   GenClangAttrSubjectMatchRuleList,
   GenClangAttrPCHRead,
   GenClangAttrPCHWrite,
@@ -115,6 +116,8 @@
"Generate clang attribute implementations"),
 clEnumValN(GenClangAttrList, "gen-clang-attr-list",
"Generate a clang attribute list"),
+clEnumValN(GenClangAttrDocTable, "gen-clang-attr-doc-table",
+   "Generate a table of attribute documentation"),
 clEnumValN(GenClangAttrSubjectMatchRuleList,
"gen-clang-attr-subject-match-rule-list",
"Generate a clang attribute subject match rule list"),
@@ -280,6 +283,9 @@
   case GenClangAttrList:
 EmitClangAttrList(Records, OS);
 break;
+  case GenClangAttrDocTable:
+EmitClangAttrDocTable(Records, OS);
+break;
   case GenClangAttrSubjectMatchRuleList:
 EmitClangAttrSubjectMatchRuleList(Records, OS);
 break;
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4210,6 +4210,41 @@
   getPragmaAttributeSupport(Records).generateParsingHelpers(OS);
 }
 
+void EmitClangAttrDocTable(RecordKeeper &Records, raw_ostream &OS) {
+  emitSourceFileHeader("Clang attribute documentation", OS);
+
+  OS << R"cpp(
+  #include "clang/AST/Attr.h"
+  #include "llvm/ADT/StringRef.h"
+  )cpp";
+  std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {
+if (!A->getValueAsBit("ASTNode"))
+  continue;
+std::vector Docs = A->getValueAsListOfDefs("Documentation");
+for (const auto *D : Docs) {
+  OS << "\nstatic const char AttrDoc_" << A->getName() << "[] = "
+ << "R\"reST("
+ << D->getValueAsOptionalString("Content").getValueOr("").trim()
+ << ")reST\";\n";
+  // Only look at the first documentation if there are several.
+  // (As of now, only one attribute has multiple documentation entries).
+  break;
+}
+  }
+  OS << R"cpp(
+  static const llvm::StringRef AttrDoc[] = {
+  #define ATTR(NAME) AttrDoc_##NAME,
+  #include "clang/Basic/AttrList.inc"
+  };
+
+  llvm::StringRef clang::Attr::getDocumentation(clang::attr::Kind K) {
+assert(K < llvm::array_lengthof(AttrDoc));
+return AttrDoc[K];
+  }
+  )cpp";
+}
+
 enum class SpellingKind {
   GNU,
   CXX11,
Index: clang/lib/AST/CMakeLists.txt
===
--- clang/lib/AST/CMakeLists.txt
+++ clang/lib/AST/CMakeLists.txt
@@ -13,6 +13,11 @@
   SOURCE Interp/Opcodes.td
   TARGET Opcodes)
 
+clang_tablegen(AttrDocTable.cpp -gen-clang-attr-doc-table
+  -I ${CMAKE_CURRENT_SOURCE_DIR}/../../include/
+  SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/../../include/clang/Basic/Attr.td
+  TARGET ClangAttrDocTable)
+
 add_clang_library(clangAST
   APValue.cpp
   ASTConcept.cpp
@@ -24,6 +29,7 @@
   ASTImporterLookupTable.cpp
   ASTStructuralEquivalence.cpp
   ASTTypeTraits.cpp
+  AttrDocTable.cpp
   AttrImpl.cpp
   Comment.cpp
   CommentBriefParser.cpp
Index: clang/include/clang/AST/Attr.h
===
--- clang/include/clang/AST/Attr.h
+++ clang/include/clang/AST/Attr.h
@@ -109,6 +109,8 @@
 
   // Pretty print this attribute.
   vo

[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

2021-08-07 Thread Sean Fertile via Phabricator via cfe-commits 
sfertile updated this revision to Diff 364996.
sfertile marked an inline comment as done.
sfertile added a comment.

Add a couple more struct layouts to the testing to show cases diagnostic is not 
issued.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107506

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+
+struct D {
+  double d;
+  struct A a;
+};
+#pragma align(reset)
+
+struct E {
+  int a : 28;
+  int   : 0;
+  int b : 16;
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != 
AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only warn if there is at least 1 bitfield member.
+if (llvm::any_of(RD->fields(),
+ [](const FieldDecl *FD) { return FD->isBitField(); }))
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+
+struct D {
+  double d;
+  struct A a;
+};
+#pragma align(reset)
+
+struct E {
+  int a : 28;
+  int   : 0;
+  int b : 16;
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only wa

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

2021-08-07 Thread liushuai wang via Phabricator via cfe-commits 
MTC added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:36
 template 
 void forEachField(const RecordDecl &Record, const T &Fields, Func &&Fn) {
   for (const FieldDecl *F : Fields) {

Btw, the parameter `Record` seems useless, could you clean it up in another 
patch?



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:439
+   [&](const FieldDecl *F) { 
+if (!HasRecordClasMemberSet.count(F))
+{

I believe `DenseSet::contains` is more appropriate here.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:454
+
+  if (AllFieldsToInit.empty())
+return;

`FieldsToInit` being empty implies `AllFieldsToInit` is empty. We have checked 
the emptiness of `FieldsToInit`.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:211
 
   // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;

Please update the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107641

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

2021-08-07 Thread Florin Iucha via Phabricator via cfe-commits 
0x8000- updated this revision to Diff 364997.
0x8000- added a comment.

Added separate check for parameter names.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s readability-variable-length %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-variable-length.IgnoredVariableNames, value: "^[xy]$"}]}' \
+// RUN: --
+
+struct myexcept
+{
+   int val;
+};
+
+struct simpleexcept
+{
+   int other;
+};
+
+void doIt();
+
+void tooShortVariableNames(int z)
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: parameter name 'z' is too short, expected at least 3 characters [readability-variable-length]
+{
+   int i = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'i' is too short, expected at least 3 characters [readability-variable-length]
+
+   int jj = z;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'jj' is too short, expected at least 3 characters [readability-variable-length]
+
+   for (int m = 0; m < 5; ++ m)
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& x)
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-variable-length]
+   {
+  doIt();
+   }
+}
+
+void longEnoughVariableNames(int n) // argument 'n' ignored by configuration
+{
+   int var = 5;
+
+   for (int i = 0; i < 42; ++ i) // 'i' is default allowed, for historical reasons
+   {
+  doIt();
+   }
+
+   for (int kk = 0; kk < 42; ++ kk)
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const simpleexcept& e) // ignored via default configuration
+   {
+  doIt();
+   }
+   catch (const myexcept& ex)
+   {
+  doIt();
+   }
+
+   int x = 5;  // ignored by configuration
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst
@@ -0,0 +1,76 @@
+.. title:: clang-tidy - readability-variable-length
+
+readability-variable-length
+===
+
+This check finds variables and function parameters whose length are too short.
+The desired name length is configurable.
+
+Special cases are supported for loop counters and for exception variable names.
+
+Options
+---
+
+The following options are described below:
+
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumVariableNameLength`, :option:`IgnoredVariableNames`
+ - :option:`MinimumExceptionNameLength`
+
+.. option:: MinimumLoopCounterNameLength
+
+Loop counter variables are expected to have a length of at least
+`MinimumLoopCounterNameLength` characters (default is `2`).
+
+.. code-block:: c++
+
+  // This warns that 'q' is too short.
+  for (int q = 0; q < size; ++ q) {
+ // ...
+  }
+
+.. option:: IgnoredLoopCounterNames
+
+Specifies a regular expression for counter names that are to be ignored.
+The default value is `^[ijk_]$`; the first three symbols for historical
+reasons and the last one since it is frequently used as a "dont't care"
+value, specifically in tools such as Google Benchmark.
+
+.. code-block:: c++
+
+  // This does not warn by default, for historical reasons.
+  for (int i = 0; i < size; ++ i) {
+  // ...
+  }
+
+.. option:: MinimumExceptionNameLength
+
+Exception clause variables are expected to have a length of at least
+`MinimumExceptionNameLength` (default is `2`).
+
+.. code-block:: c++
+
+  try {
+  // ...
+  }
+  // This warns that 'e' is too short.
+  catch (const std::exception& e) {
+  // ...
+  }
+
+.. option:: MinimumVariableNameLength
+
+All other variables are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`).
+
+.. code-block:: c++
+
+  int i = 42;// warns that 'i' is too sh