[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

[Brooks Moses via Phabricator via cfe-commits]

brooksmoses added a comment.

So, I have bad news: This causes OpenJDK to segfault.  The relevant code is 
here:
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/arena.cpp#L311

  void Arena::destruct_contents() {
if (UseMallocOnly && _first != NULL) {
  char* end = _first->next() ? _first->top() : _hwm;
  free_malloced_objects(_first, _first->bottom(), end, _hwm);
}
// reset size before chop to avoid a rare racing condition
// that can have total arena memory exceed total chunk memory
set_size_in_bytes(0);
_first->chop();
reset();
  }

I've also seen a segfault in Verilator that root-causes to this patch, though I 
haven't yet tracked that down to the source code.

I hate to say it, but is this a significant enough problem to call for a 
(temporary, I hope) rollback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17993

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

[clang] e91b234 - [mac/arm] Fix test/Driver/darwin-sdk-version.c on arm macs

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2020-11-20T22:32:31-05:00
New Revision: e91b2344ad7294c47d8b10b3a84e962bc3ed4160

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

LOG: [mac/arm] Fix test/Driver/darwin-sdk-version.c on arm macs

Two invocations in this test used `-m64`, which on an arm mac means
arm64 in the triple, not x86_64.

Added: 


Modified: 
clang/test/Driver/darwin-sdk-version.c

Removed: 




diff  --git a/clang/test/Driver/darwin-sdk-version.c 
b/clang/test/Driver/darwin-sdk-version.c
index 9c1eec0ee43f..e95103b7cfb5 100644
--- a/clang/test/Driver/darwin-sdk-version.c
+++ b/clang/test/Driver/darwin-sdk-version.c
@@ -30,8 +30,8 @@
 // RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
 
 // CHECK: -target-sdk-version=10.14
-// INFER_SDK_VERSION: "-triple" "x86_64-apple-macosx10.10.0"
+// INFER_SDK_VERSION: "-triple" "{{arm64|x86_64}}-apple-macosx10.10.0"
 // INFER_SDK_VERSION-SAME: -target-sdk-version=10.10
-// INFER_DEPLOYMENT_TARGET_VERSION: "-triple" "x86_64-apple-macosx10.8.0"
+// INFER_DEPLOYMENT_TARGET_VERSION: "-triple" 
"{{arm64|x86_64}}-apple-macosx10.8.0"
 // NO_VERSION-NOT: target-sdk-version
 // ERROR: warning: SDK settings were ignored as 'SDKSettings.json' could not 
be parsed



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

[clang] c473184 - [mac/arm] Fix clang/test/Sema/wchar.c on mac/arm hosts

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2020-11-20T21:49:17-05:00
New Revision: c47318491439642ed47cb9c9098333a8199fa54a

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

LOG: [mac/arm] Fix clang/test/Sema/wchar.c on mac/arm hosts

Part of PR46644.

Added: 


Modified: 
clang/test/Sema/wchar.c

Removed: 




diff  --git a/clang/test/Sema/wchar.c b/clang/test/Sema/wchar.c
index 3170e363ff77..83ea6372168d 100644
--- a/clang/test/Sema/wchar.c
+++ b/clang/test/Sema/wchar.c
@@ -6,7 +6,14 @@ typedef __WCHAR_TYPE__ wchar_t;
 #if defined(_WIN32) || defined(_M_IX86) || defined(__CYGWIN__) \
  || defined(_M_X64) || defined(__ORBIS__) || defined(SHORT_WCHAR)
   #define WCHAR_T_TYPE unsigned short
-#elif defined(__arm) || defined(__aarch64__) || defined(__MVS__)
+#elif defined(__aarch64__)
+  // See AArch64TargetInfo constructor -- unsigned on non-darwin non-OpenBSD 
non-NetBSD.
+  #if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+#define WCHAR_T_TYPE int
+  #else
+#define WCHAR_T_TYPE unsigned int
+  #endif
+#elif defined(__arm) || defined(__MVS__)
   #define WCHAR_T_TYPE unsigned int
 #elif defined(__sun)
   #if defined(__LP64__)
@@ -14,7 +21,7 @@ typedef __WCHAR_TYPE__ wchar_t;
   #else
 #define WCHAR_T_TYPE long
   #endif
-#else /* Solaris. */
+#else /* Solaris, Linux, non-arm64 macOS, ... */
   #define WCHAR_T_TYPE int
 #endif
  



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

[PATCH] D91904: [mac/arm] Fix rtti codegen tests when running on an arm mac

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rjmccall.
Herald added subscribers: kristof.beyls, dschuff.
thakis requested review of this revision.
Herald added a subscriber: aheejin.

[mac/arm] Fix rtti codegen tests when running on an arm mac

shouldRTTIBeUnique() returns false for iOS64CXXABI, which causes
RTTI objects to be emitted hidden. Update two tests that didn't
expect this to happen for the default triple.

Also rename iOS64CXXABI to AppleARM64CXXABI, since it's used for
arm64-apple-macos triples too.

Part of PR46644.


https://reviews.llvm.org/D91904

Files:
  clang/include/clang/Basic/TargetCXXABI.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/weak-extern-typeinfo.cpp
  clang/test/SemaCXX/typeid-ref.cpp

Index: clang/test/SemaCXX/typeid-ref.cpp
===
--- clang/test/SemaCXX/typeid-ref.cpp
+++ clang/test/SemaCXX/typeid-ref.cpp
@@ -6,7 +6,7 @@
 struct X { };
 
 void f() {
-  // CHECK: @_ZTS1X = linkonce_odr {{(dso_local )?}}constant
-  // CHECK: @_ZTI1X = linkonce_odr {{(dso_local )?}}constant 
+  // CHECK: @_ZTS1X = linkonce_odr {{(dso_local|hidden )?}}constant
+  // CHECK: @_ZTI1X = linkonce_odr {{(dso_local|hidden )?}}constant
   (void)typeid(X&);
 }
Index: clang/test/CodeGenCXX/weak-extern-typeinfo.cpp
===
--- clang/test/CodeGenCXX/weak-extern-typeinfo.cpp
+++ clang/test/CodeGenCXX/weak-extern-typeinfo.cpp
@@ -31,17 +31,17 @@
 void V1::foo() { }
 void V2::foo() { }
 
-// CHECK: @_ZTS1A = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI1A = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTS1B = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI1B = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTS1C = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTS2T1 = linkonce_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI2T1 = linkonce_odr {{(dso_local )?}}constant
-// CHECK: @_ZTS1T = linkonce_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI1T = linkonce_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI1C = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTS2V1 = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI2V1 = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTS2V2 = weak_odr {{(dso_local )?}}constant
-// CHECK: @_ZTI2V2 = weak_odr {{(dso_local )?}}constant
+// CHECK: @_ZTS1A = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI1A = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTS1B = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI1B = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTS1C = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTS2T1 = linkonce_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI2T1 = linkonce_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTS1T = linkonce_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI1T = linkonce_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI1C = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTS2V1 = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI2V1 = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTS2V2 = weak_odr {{(dso_local|hidden )?}}constant
+// CHECK: @_ZTI2V2 = weak_odr {{(dso_local|hidden )?}}constant
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -486,9 +486,9 @@
CharUnits cookieSize) override;
 };
 
-class iOS64CXXABI : public ARMCXXABI {
+class AppleARM64CXXABI : public ARMCXXABI {
 public:
-  iOS64CXXABI(CodeGen::CodeGenModule ) : ARMCXXABI(CGM) {
+  AppleARM64CXXABI(CodeGen::CodeGenModule ) : ARMCXXABI(CGM) {
 Use32BitVTableOffsetABI = true;
   }
 
@@ -551,8 +551,8 @@
   case TargetCXXABI::WatchOS:
 return new ARMCXXABI(CGM);
 
-  case TargetCXXABI::iOS64:
-return new iOS64CXXABI(CGM);
+  case TargetCXXABI::AppleARM64:
+return new AppleARM64CXXABI(CGM);
 
   case TargetCXXABI::Fuchsia:
 return new FuchsiaCXXABI(CGM);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -76,11 +76,11 @@
 
 static CGCXXABI *createCXXABI(CodeGenModule ) {
   switch (CGM.getTarget().getCXXABI().getKind()) {
+  case TargetCXXABI::AppleARM64:
   case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericAArch64:
   case TargetCXXABI::GenericARM:
   case TargetCXXABI::iOS:
-  case TargetCXXABI::iOS64:
   case TargetCXXABI::WatchOS:
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ 

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Shane via Phabricator via cfe-commits]

smhc updated this revision to Diff 306815.
smhc added a comment.

Updated after review comments.


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

https://reviews.llvm.org/D90282

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: "^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: "^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: "sooo|so|soo|$invalidregex["} \
+// RUN:  ]}'
+
+int testFunc(int a, char **b);
+int testFunc(int ab, char **ba);
+int testFunc(int abc, char **cba);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'abc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'cba'
+// CHECK-FIXES: {{^}}int testFunc(int Abc, char **Cba);{{$}}
+int testFunc(int dE, char **eD);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'dE'
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: invalid case style for parameter 'eD'
+// CHECK-FIXES: {{^}}int testFunc(int DE, char **ED);{{$}}
+int testFunc(int Abc, char **Cba);
+
+class fo {
+};
+
+class fofo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'fofo'
+  // CHECK-FIXES: {{^}}class Fofo {{{$}}
+};
+
+class foo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'foo'
+  // CHECK-FIXES: {{^}}class Foo {{{$}}
+};
+
+class fooo {
+};
+
+class afooo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'afooo'
+  // CHECK-FIXES: {{^}}class Afooo {{{$}}
+};
+
+struct soo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'soo'
+  // CHECK-FIXES: {{^}}struct Soo {{{$}}
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -35,60 +35,60 @@
 
 The following options are describe below:
 
- - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`
+ - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`, :option:`AbstractClassIgnoredRegexp`
  - :option:`AggressiveDependentMemberLookup`
- - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`
- - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`
- - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`
- - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`
- - :option:`ConstantCase`, :option:`ConstantPrefix`, :option:`ConstantSuffix`
- - :option:`ConstantMemberCase`, :option:`ConstantMemberPrefix`, :option:`ConstantMemberSuffix`
- - :option:`ConstantParameterCase`, :option:`ConstantParameterPrefix`, :option:`ConstantParameterSuffix`
- - :option:`ConstantPointerParameterCase`, :option:`ConstantPointerParameterPrefix`, :option:`ConstantPointerParameterSuffix`
- - :option:`ConstexprFunctionCase`, :option:`ConstexprFunctionPrefix`, :option:`ConstexprFunctionSuffix`
- - :option:`ConstexprMethodCase`, :option:`ConstexprMethodPrefix`, :option:`ConstexprMethodSuffix`
- - :option:`ConstexprVariableCase`, :option:`ConstexprVariablePrefix`, :option:`ConstexprVariableSuffix`
- - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`
- - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`
- - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`
+ - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`, :option:`ClassIgnoredRegexp`
+ - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`, :option:`ClassConstantIgnoredRegexp`
+ - :option:`ClassMemberCase`, 

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

And for `drivers/hid/usbhid/hid-core.c` it looks like multiple labels are 
modeled as individual blocks in the control flow graph.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91895

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D91859#2408064 , @sammccall wrote:

> I'm not totally following the discussion on the more complete fix (LMK if you 
> want me to dig into that), but if the build is broken in some configurations 
> we should likely prioritize fixing that over ensuring the fix is conceptually 
> complete.

Yes, this is basically an expanded context from a discussion me and Kadir had 
over the last couple of days. Sorry for confusion, the "complete" means "it's 
still not working for me without these changes". And the configuration we're 
looking at is simply shared libs build.

>> Otherwise by the time unittests are compiled the include paths are not set 
>> appropriately and we will see the errors with Protobuf version mismatch 
>> through transitive includes etc.
>
> I don't think that's how CMake works, the whole CMakeLists tree is parsed 
> before anything is compiled, so it shouldn't race like that.

This may look counter-intuitive but this is how it works on my machine. 
`MarshallingTests.cpp` entry in `compile_commands.json` does not have gRPC 
include path. (`rg "MarshallingTests.cpp" compile_commands.json | rg grpc` is 
empty). When I put `FindGRPC` before `add_directory(unittests)` the compile 
commands have the right include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91868: [clangd] Mention when CXXThis is implicit in exposed AST.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/DumpAST.cpp:233
   return CCO->getConstructor()->getNameAsString();
+if (const auto *CTE = dyn_cast(S)) {
+  bool Const = CTE->getType()->getPointeeType().isLocalConstQualified();

kadircet wrote:
> should we ensure we always return within the branch (as we do within the rest 
> of the branches to make sure we don't accumulate details by mistake)? e.g:
> ```
> if(CXXThisExpr) {
>   details = {}
>   if (const) details += "const";
>   if (implicit) details += "implicit";
>   return join(",", details);
> }
> ```
Actually I was trying to *avoid* returning `""` in several places.
I think of these functions as a collection of special cases where we have 
something to say - if we don't find anything, we fall off the end.
(`getDetail` for DeducedType is the other example)

Can change it if you think it's important though.



Comment at: clang-tools-extra/clangd/unittests/DumpASTTests.cpp:79
   {R"cpp(
-template  int root() {
-  (void)root();
+namespace root {
+template  int tmpl() {

kadircet wrote:
> is this change intentional ?
Yeah, the problem is that I wanted the root in the new test to be a member 
function, so I needed to switch from findDecl("root") to 
findUnqualifiedDecl("root"). But that fails if there are two decls with that 
name, as there were here: the primary template root and the specialization 
root. Thus the change to wrap the whole thing in a root namespace. 
Maybe there's a neater way to solve this, I'm not sure it matters much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91868

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

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Testing this on the kernel with `-Wimplicit-fallthrough`, there's some 
additional cases that are still warning:

  // mm/vmscan.c
  1371 mapping = page_mapping(page);

 
  1372   case PAGE_CLEAN: // -Wimplicit-fallthrough 


  
  1373 ; /* try to free the page below */   

 
  1374   } 

  // fs/nfs/nfs4client.c
   611   nfs4_schedule_path_down_recovery(pos); 

 
   612 default: // -Wimplicit-fallthrough   


  
   613   goto out;

  // drivers/hid/usbhid/hid-core.c
   490   case -ESHUTDOWN:  /* unplug */ 

 
   491 unplug = 1; 
   492   case -EILSEQ:   /* protocol error or unplug */ // 
-Wimplicit-fallthrough...how?!  

   
   493   case -EPROTO:   /* protocol error or unplug */ 

 
   494   case -ECONNRESET: /* unlink */ 

 
   495   case -ENOENT:  

 
   496   case -EPIPE:/* report not available */ 

 
   497 break;   

  // security/keys/process_keys.c
  782   default:

 
  783 if (need_perm != KEY_AUTHTOKEN_OVERRIDE &&

 
  784 need_perm != KEY_DEFER_PERM_CHECK)

 
  785   goto invalid_key;   

 
  786   case 0: // -Wimplicit-fallthrough...how?!   

   
  787 break;

 
  788   }

Ah, I think I need to reconsider the case that there is more than one 
successor, or at least differentiate which successor is the fallthrough block, 
rather than only checking blocks that have 1 successor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91895

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

[PATCH] D91029: [clangd] Implement clang-tidy options from config

[Nathan James via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/TidyProvider.h:20
+/// The base class of all clang-tidy config providers for clangd.
+class TidyProvider {
+public:

sammccall wrote:
> we're using inheritance for a couple of purposes here.
> 
> First, giving a common interface to the various types of provider.
> This makes sense, though we use 
> `unique_function` for this as the type as it's 
> essentially just one function.
> This would give us the option of using lambdas instead of having 
> TestClangTidyProvider...
> 
> The other purpose is composition: ConfigTidyProvider inherits from 
> ThreadSafeFileTidyProvider in order to provide both sets of config.
> This is awkward and constraining, e.g. we can't now enable config but not 
> `.clang-tidy` (which is the desired config inside google), and more 
> immediately the handling of overrides is awkward.
> It seems more natural to have several units and compose them explicitly. The 
> most simple way to compose them is have them be functions that mutate a 
> ClangTidyOptions, instead of producing one.
> 
> So in its minimal form this could look like:
> ```
> using TidyProvider = unique_function /*Filename*/llvm::StringRef)>;
> TidyProvider combine(vector); 
> ClangTidyOptionsProvider asClangTidyOptionsProvider(TidyProvider);
> 
> TidyProvider provideEnvironment();
> TidyProvider provideDefaultChecks();
> TidyProvider disableUnusableChecks(ArrayRef OverrideChecks={});
> TidyProvider provideClangdConfig();
> TidyProvider provideClangTidyFiles(ThreadsafeFS&);
> 
> // In ClangdMain:
> TidyProvider Tidy = combine({provideEnvironment(), provideClangTidyFiles(FS), 
> provideClangdConfig(), provideDefaultChecks(), disableUnusableChecks());
> ```
> 
> What do you think?
That is a much nicer interface


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91029

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

[PATCH] D91029: [clangd] Implement clang-tidy options from config

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 306796.
njames93 marked 7 inline comments as done.
njames93 added a comment.

Messed up branches, fixed that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91029

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/TidyProvider.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -17,6 +17,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
+#include "../TidyProvider.h"
 #include "Compiler.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
@@ -58,8 +59,7 @@
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
-  llvm::Optional ClangTidyChecks;
-  llvm::Optional ClangTidyWarningsAsErrors;
+  mutable TidyProvider ClangTidyProvider = {};
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -59,8 +59,8 @@
 FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = 
   Inputs.Opts = ParseOptions();
-  Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
-  Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
+  if (ClangTidyProvider)
+Inputs.ClangTidyProvider = ClangTidyProvider;
   Inputs.Index = ExternalIndex;
   if (Inputs.Index)
 Inputs.Opts.SuggestMissingIncludes = true;
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -24,6 +24,7 @@
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -250,7 +251,8 @@
   TestTU TU;
   // this check runs the preprocessor, we need to make sure it does not break
   // our recording logic.
-  TU.ClangTidyChecks = "modernize-use-trailing-return-type";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("modernize-use-trailing-return-type");
   TU.Code = "inline int foo() {}";
 
   auto AST = TU.build();
@@ -406,7 +408,7 @@
   "replay-preamble-module", "");
   TestTU TU;
   // This check records inclusion directives replayed by clangd.
-  TU.ClangTidyChecks = "replay-preamble-check";
+  TU.ClangTidyProvider = fixedTidyProvider("replay-preamble-check");
   llvm::Annotations Test(R"cpp(
 $hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
 $hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -14,6 +14,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "index/MemIndex.h"
 #include "support/Path.h"
 #include "clang/Basic/Diagnostic.h"
@@ -129,7 +130,7 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ClangTidyChecks = "-*,google-explicit-constructor";
+  TU.ClangTidyProvider = fixedTidyProvider("-*,google-explicit-constructor");
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   ElementsAre(
@@ -201,8 +202,9 @@
   auto TU = TestTU::withCode(Test.code());
   // Enable alias clang-tidy checks, these check emits the same diagnostics
   // (except the check name).
-  TU.ClangTidyChecks = "-*, readability-uppercase-literal-suffix, "
-   "hicpp-uppercase-literal-suffix";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("-*, readability-uppercase-literal-suffix, "
+"hicpp-uppercase-literal-suffix");
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
   TU.build().getDiagnostics(),
@@ -245,9 +247,9 @@
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 306795.
sammccall added a comment.
Herald added subscribers: arphaman, kbarton, nemanjai.

Drop hasAttrName matcher.

Update uses of hasAncestor(isImplicit()) to hasAncestor(decl(isImplicit())),
as the former no longer compiles due to lack of deduction.

Implement and test up/down traversals involving attrs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

Files:
  clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchFinder.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/ParentMapContext.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTest.h
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "ASTMatchersTest.h"
+#include "clang/AST/Attrs.inc"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
@@ -3959,6 +3960,24 @@
 // Nested names: a, a::A and a::A::B.
 std::make_unique>("x", 3)));
 }
+
+TEST(Attr, AttrsAsDescendants) {
+  StringRef Fragment = "namespace a { struct [[clang::warn_unused_result]] "
+   "F{}; [[noreturn]] void foo(); }";
+  EXPECT_TRUE(matches(Fragment, namespaceDecl(hasDescendant(attr();
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Fragment,
+  namespaceDecl(hasName("a"),
+forEachDescendant(attr(unless(isImplicit())).bind("x"))),
+  std::make_unique>("x", 2)));
+}
+
+TEST(Attr, ParentsOfAttrs) {
+  StringRef Fragment =
+  "namespace a { struct [[clang::warn_unused_result]] F{}; }";
+  EXPECT_TRUE(matches(Fragment, attr(hasAncestor(namespaceDecl();
+}
+
 template  class VerifyMatchOnNode : public BoundNodesCallback {
 public:
   VerifyMatchOnNode(StringRef Id, const internal::Matcher ,
Index: clang/unittests/ASTMatchers/ASTMatchersTest.h
===
--- clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -298,7 +298,7 @@
   // Some tests use typeof, which is a gnu extension.  Using an explicit
   // unknown-unknown triple is good for a large speedup, because it lets us
   // avoid constructing a full system triple.
-  std::vector Args = {"-std=gnu++98", "-target",
+  std::vector Args = {"-std=gnu++11", "-target",
"i386-unknown-unknown"};
   if (!runToolOnCodeWithArgs(Factory->create(), Code, Args)) {
 return testing::AssertionFailure() << "Parsing error in \"" << Code << "\"";
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1875,6 +1875,19 @@
  nestedNameSpecifier()));
 }
 
+TEST_P(ASTMatchersTest, Attr) {
+  if (GetParam().isCXX11OrLater()) {
+EXPECT_TRUE(matches("struct [[clang::warn_unused_result]] F{};", attr()));
+  }
+  if (GetParam().isCXX17OrLater()) {
+EXPECT_TRUE(matches("struct [[nodiscard]] F{};", attr()));
+  }
+  EXPECT_TRUE(matches("int x(int * __attribute__((nonnull)) );", attr()));
+  // On windows, some nodes have an implicit visibility attribute.
+  EXPECT_TRUE(
+  notMatches("struct F{}; int x(int *);", attr(unless(isImplicit();
+}
+
 TEST_P(ASTMatchersTest, NullStmt) {
   EXPECT_TRUE(matches("void f() {int i;;}", nullStmt()));
   EXPECT_TRUE(notMatches("void f() {int i;}", nullStmt()));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -120,6 +120,7 @@
   VERIFY_NAME(CallExpr);
   VERIFY_NAME(Type);
   VERIFY_NAME(ConstantArrayType);
+  VERIFY_NAME(NonNullAttr);
 #undef VERIFY_NAME
 }
 
@@ -148,6 +149,13 @@
  

[PATCH] D91029: [clangd] Implement clang-tidy options from config

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 306794.
njames93 marked an inline comment as done.
njames93 added a comment.

Reworked everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91029

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/TidyProvider.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -17,6 +17,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
+#include "../TidyProvider.h"
 #include "Compiler.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
@@ -58,8 +59,7 @@
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
-  llvm::Optional ClangTidyChecks;
-  llvm::Optional ClangTidyWarningsAsErrors;
+  mutable TidyProvider ClangTidyProvider = {};
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -59,8 +59,8 @@
 FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = 
   Inputs.Opts = ParseOptions();
-  Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
-  Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
+  if (ClangTidyProvider)
+Inputs.ClangTidyProvider = ClangTidyProvider;
   Inputs.Index = ExternalIndex;
   if (Inputs.Index)
 Inputs.Opts.SuggestMissingIncludes = true;
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -24,6 +24,7 @@
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -250,7 +251,8 @@
   TestTU TU;
   // this check runs the preprocessor, we need to make sure it does not break
   // our recording logic.
-  TU.ClangTidyChecks = "modernize-use-trailing-return-type";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("modernize-use-trailing-return-type");
   TU.Code = "inline int foo() {}";
 
   auto AST = TU.build();
@@ -406,7 +408,7 @@
   "replay-preamble-module", "");
   TestTU TU;
   // This check records inclusion directives replayed by clangd.
-  TU.ClangTidyChecks = "replay-preamble-check";
+  TU.ClangTidyProvider = fixedTidyProvider("replay-preamble-check");
   llvm::Annotations Test(R"cpp(
 $hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
 $hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -14,6 +14,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "index/MemIndex.h"
 #include "support/Path.h"
 #include "clang/Basic/Diagnostic.h"
@@ -129,7 +130,7 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ClangTidyChecks = "-*,google-explicit-constructor";
+  TU.ClangTidyProvider = fixedTidyProvider("-*,google-explicit-constructor");
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   ElementsAre(
@@ -201,8 +202,9 @@
   auto TU = TestTU::withCode(Test.code());
   // Enable alias clang-tidy checks, these check emits the same diagnostics
   // (except the check name).
-  TU.ClangTidyChecks = "-*, readability-uppercase-literal-suffix, "
-   "hicpp-uppercase-literal-suffix";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("-*, 

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: aaron.ballman, dexonsmith, erik.pilkington, vsavchenko.
Herald added subscribers: martong, Charusso, JDevlieghere, kristof.beyls.
NoQ requested review of this revision.

Patch by Sean Dooher! I'll be addressing the review comments.

The point is to markup a section of code (a set of functions) that should be 
isolated for security, basically like a TCB. Such section of code, being 
privileged in some specific manner, would not be allowed to exercise arbitrary 
behavior, so calling a function that's outside the set from a function that's 
inside the set is not allowed; they can only call each other. This is 
ultimately supposed to achieve security of the system with respect to that 
privilege through audit of the TCB.

The patch adds an attribute `enforce_tcb` to define a TCB and a warning 
`-Wtcb-enforcement` for violating the enforcement. Additionally it adds an 
attribute `enforce_tcb_leaf` that allows opting out of enforcement for 
individual harmless functions: such "leaf" functions are allowed to be called 
from the respective TCB but aren't forced into the TCB themselves.


https://reviews.llvm.org/D91898

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-enforce-tcb.c

Index: clang/test/Sema/attr-enforce-tcb.c
===
--- /dev/null
+++ clang/test/Sema/attr-enforce-tcb.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+#define PLACE_IN_TCB(NAME) __attribute__ ((enforce_tcb(NAME)))
+#define PLACE_IN_TCB_LEAF(NAME) __attribute__ ((enforce_tcb_leaf(NAME)))
+
+void foo1 (void) PLACE_IN_TCB("bar");
+void foo2 (void) PLACE_IN_TCB("bar");
+void foo3 (void); // not in any TCB
+void foo4 (void) PLACE_IN_TCB("bar2");
+void foo5 (void) PLACE_IN_TCB_LEAF("bar");
+void foo6 (void) PLACE_IN_TCB("bar2") PLACE_IN_TCB("bar");
+void foo7 (void) PLACE_IN_TCB("bar3");
+void foo8 (void) PLACE_IN_TCB("bar") PLACE_IN_TCB("bar2");
+void foo9 (void);
+
+void
+foo1()
+{
+foo2(); // OK - function in same TCB
+foo3(); // expected-warning {{TCB [bar] has been violated by calling a function [foo3]}}
+foo4(); // expected-warning {{TCB [bar] has been violated by calling a function [foo4]}}
+foo5(); // OK - in leaf node
+foo6(); // OK - in multiple TCBs, one of which is the same
+foo7(); // expected-warning {{TCB [bar] has been violated by calling a function [foo7]}}
+(void) __builtin_clz(5); // OK - builtins are excluded
+}
+
+// Normal use without any attributes works
+void
+foo3()
+{
+foo9(); // OK
+}
+
+void
+foo5()
+{
+// all calls should be okay, function in TCB leaf
+foo2();
+foo3();
+foo4();
+}
+
+
+void
+foo6()
+{
+foo1(); // expected-warning {{TCB [bar2] has been violated by calling a function [foo1]}}
+foo4(); // expected-warning {{TCB [bar] has been violated by calling a function [foo4]}}
+foo8(); // OK
+foo7(); // #1
+// expected-warning@#1 {{TCB [bar] has been violated by calling a function [foo7] that is not in the TCB.}}
+// expected-warning@#1 {{TCB [bar2] has been violated by calling a function [foo7] that is not in the TCB.}}
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7336,6 +7336,14 @@
   D->addAttr(::new (S.Context) CFGuardAttr(S.Context, AL, Arg));
 }
 
+template
+static void handleEnforceTCBAttr(Sema , Decl *D, const ParsedAttr ) {
+  StringRef Argument;
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Argument))
+return;
+  D->addAttr(Attr::Create(S.Context, Argument, AL));
+}
+
 //===--===//
 // Top Level Sema Entry Points
 //===--===//
@@ -8015,6 +8023,14 @@
   case ParsedAttr::AT_UseHandle:
 handleHandleAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_EnforceTCB:
+handleEnforceTCBAttr(S, D, AL);
+break;
+
+  case ParsedAttr::AT_EnforceTCBLeaf:
+handleEnforceTCBAttr(S, D, AL);
+break;
   }
 }
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -75,6 +75,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/AtomicOrdering.h"
@@ -4568,6 +4569,8 @@
   if (!FnInfo)
 return false;
 
+  CheckTCBEnforcement(TheCall, FDecl);
+
   

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D89743#2409001 , @sammccall wrote:

> We didn't talk about overloading isImplicit to apply to attrs too, but it 
> doesn't seem like that was controversial (and it does help with the tests).

I spoke too soon about this...
This prevents `hasAncestor(isImplicit())` from compiling, because `hasAncestor` 
needs to deduce the node type from its argument to call 
`ASTMatchFinder::matchesAncestorOf()`.
This occurs in a few places in tree and many places in our private codebase...
The workaround is `hasAncestor(decl(isImplicit()))` which is reasonable, except 
that "is contained in *any* implicit node" is probably actually the intent. 
Well, at least it's not a regression.

In addition, while digging into this, I realized Attrs are not traversed in the 
parent map, and not supported by the parent/child/ancestor/descendant 
traversals.
So I'm fixing that... and adding some tests.

I'll need to send this for another round, even without the name matcher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

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

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

Marking as "requested changes" to get this off my queue, but if my lambda 
suggestion doesn't work well for some reason (maybe it significantly increases 
compile-time), I'm open to sticking to the documentation @Bigcheese suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

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

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:156-159
+template 
+FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
+  return FlagToValueNormalizer{std::move(Value)};
 }

dexonsmith wrote:
> Please declare this `static`.
BTW, I suspect we can save some compile time (once there are lots of these) 
with:
```
template <
  class T,
  std::enable_if_t<
  sizeof(T) <= sizeof(uint64_t) &&
  std::is_trivially_convertible::value &&
  std::is_trivially_convertible::value,
  bool> = false>
FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
  return FlagToValueNormalizer{std::move(Value)};
}

template <
  class T,
  std::enable_if_t<
  !(sizeof(T) <= sizeof(uint64_t) &&
std::is_trivially_convertible::value &&
std::is_trivially_convertible::value),
  bool> = false>
FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
  return FlagToValueNormalizer{std::move(Value)};
}
```
It'd be worth trying if/once the compile-time begins to climb to catch the 
enums (that aren't strongly typed) in a single instantiation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83694

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> How should I check for support? Is it going to be e.g. 
> __has_feature(__builtin_clear_padding)?

I'm not sure `__has_feature` will work but `__has_builtin` should work on both 
Clang and GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh added inline comments.



Comment at: clang/test/Frontend/noderef.c:71
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access

aaron.ballman wrote:
> Can you add tests for the weird situations where the expression actually is 
> evaluated? e.g.,
> ```
> struct S {
>   virtual ~S(); // Make S polymorphic
> };
> 
> S NODEREF *s;
> typeid(*s); // Actually evaluates *s at runtime
> 
> struct T {
>   unsigned i;
> };
> 
> T t1;
> T NODEREF *t2 = 
> 
> sizeof(int[++t2->i]); // Actually evaluates t2->i at runtime
> ```
Oh jeez, good point. I'm extremely happy that other people have done the hard 
work of building the `ExpressionEvaluationContext` logic... I've added tests 
for the cases you described.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh updated this revision to Diff 306792.
thejh marked an inline comment as done.
thejh added a comment.

As requested by @aaron.ballman, added tests for edgecases where
sizeof()/typeid() can cause memory access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Frontend/noderef.c
  clang/test/Frontend/noderef.cpp


Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -6,6 +6,11 @@
 
 #define NODEREF __attribute__((noderef))
 
+// Stub out types for 'typeid' to work.
+namespace std {
+class type_info {};
+} // namespace std
+
 void Normal() {
   int NODEREF i;// expected-warning{{'noderef' can only be used on an 
array or pointer type}}
   int NODEREF *i_ptr;   // expected-note 2 {{i_ptr declared here}}
@@ -102,6 +107,18 @@
   return child->func();   // expected-warning{{dereferencing 
child; was declared with a 'noderef' type}}
 }
 
+std::type_info TypeIdPolymorphic(NODEREF A *a) { // expected-note{{a declared 
here}}
+  return typeid(*a); // 
expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+class SimpleClass {
+  int a;
+};
+
+std::type_info TypeIdNonPolymorphic(NODEREF SimpleClass *simple) {
+  return typeid(*simple);
+}
+
 template 
 class B {
   Ty NODEREF *member;
Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -57,12 +57,19 @@
   p = &*(p + 1);
 
   // Struct member access
-  struct S NODEREF *s;  // expected-note 2 {{s declared here}}
+  struct S NODEREF *s; // expected-note 3 {{s declared here}}
   x = s->a;   // expected-warning{{dereferencing s; was declared with a 
'noderef' type}}
   x = (*s).b; // expected-warning{{dereferencing s; was declared with a 
'noderef' type}}
   p = >a;
   p = &(*s).b;
 
+  // Most things in sizeof() can't actually access memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+  x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was 
declared with a 'noderef' type}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord  = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.


Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -6,6 +6,11 @@
 
 #define NODEREF __attribute__((noderef))
 
+// Stub out types for 'typeid' to work.
+namespace std {
+class type_info {};
+} // namespace std
+
 void Normal() {
   int NODEREF i;// expected-warning{{'noderef' can only be used on an array or pointer type}}
   int NODEREF *i_ptr;   // expected-note 2 {{i_ptr declared here}}
@@ -102,6 +107,18 @@
   return child->func();   // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
 }
 
+std::type_info TypeIdPolymorphic(NODEREF A *a) { // expected-note{{a declared here}}
+  return typeid(*a); // expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+class SimpleClass {
+  int a;
+};
+
+std::type_info TypeIdNonPolymorphic(NODEREF SimpleClass *simple) {
+  return typeid(*simple);
+}
+
 template 
 class B {
   Ty NODEREF *member;
Index: 

[PATCH] D91669: Don’t break before nested block param when prior param is not a block

[Keith Smiley 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 rG244022a3cd75: Don’t break before nested block param when 
prior param is not a block (authored by segiddins, committed by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91669

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -18,6 +18,7 @@
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -51,18 +52,24 @@
 return *Result;
   }
 
-  void verifyFormat(StringRef Code) {
+  void _verifyFormat(const char *File, int Line, StringRef Code) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code)));
   }
 
-  void verifyIncompleteFormat(StringRef Code) {
+  void _verifyIncompleteFormat(const char *File, int Line, StringRef Code) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(), format(test::messUp(Code), SC_ExpectIncomplete));
   }
 
   FormatStyle Style;
 };
 
+#define verifyIncompleteFormat(...)
\
+  _verifyIncompleteFormat(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
+
 TEST(FormatTestObjCStyle, DetectsObjCInHeaders) {
   auto Style = getStyle("LLVM", "a.h", "none",
 "@interface\n"
@@ -1453,6 +1460,39 @@
   "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
   " u = v;\n"
   "   }]");
+
+  verifyFormat("[self block:^(void) {\n"
+   "  doStuff();\n"
+   "} completionHandler:^(void) {\n"
+   "  doStuff();\n"
+   "  [self block:^(void) {\n"
+   "doStuff();\n"
+   "  } completionHandler:^(void) {\n"
+   "doStuff();\n"
+   "  }];\n"
+   "}];");
+
+  Style.ColumnLimit = 0;
+  verifyFormat("[[SessionService sharedService] "
+   "loadWindowWithCompletionBlock:^(SessionWindow *window) {\n"
+   "  if (window) {\n"
+   "[self windowDidLoad:window];\n"
+   "  } else {\n"
+   "[self errorLoadingWindow];\n"
+   "  }\n"
+   "}];");
+  verifyFormat("[controller test:^{\n"
+   "  doStuff();\n"
+   "} withTimeout:5 completionHandler:^{\n"
+   "  doStuff();\n"
+   "}];");
+  verifyFormat(
+  "[self setupTextFieldSignals:@[\n"
+  "  self.documentWidthField,\n"
+  "  self.documentHeightField,\n"
+  "] solver:^(NSTextField *textField) {\n"
+  "  return [self.representedObject 
solveEquationForTextField:textField];\n"
+  "}];");
 }
 
 TEST_F(FormatTestObjC, IfNotUnlikely) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -400,7 +400,9 @@
 return true;
   if (Current.is(TT_SelectorName) && !Previous.is(tok::at) &&
   State.Stack.back().ObjCSelectorNameFound &&
-  State.Stack.back().BreakBeforeParameter)
+  State.Stack.back().BreakBeforeParameter &&
+  (Style.ObjCBreakBeforeNestedBlockParam ||
+   !Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)))
 return true;
 
   unsigned NewLineColumn = getNewLineColumn(State);


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -18,6 +18,7 @@
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -51,18 +52,24 @@
 return *Result;
   }
 
-  void verifyFormat(StringRef Code) {
+  void _verifyFormat(const char *File, int Line, StringRef Code) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code)));
   }
 
-  void verifyIncompleteFormat(StringRef Code) {
+  void _verifyIncompleteFormat(const char *File, int Line, StringRef Code) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(), 

[clang] 244022a - Don’t break before nested block param when prior param is not a block

[Keith Smiley via cfe-commits]

Author: Samuel Giddins
Date: 2020-11-20T15:16:04-08:00
New Revision: 244022a3cd75b51dcf1d2a5a822419492ce79e47

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

LOG: Don’t break before nested block param when prior param is not a block

Add ScopedTrace to verify methods in FormatTestObjC
Add tests from D17700

Reviewed By: keith, kastiglione

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp
clang/unittests/Format/FormatTestObjC.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index d99107cb8b2c..d811f0f9e531 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -400,7 +400,9 @@ bool ContinuationIndenter::mustBreak(const LineState 
) {
 return true;
   if (Current.is(TT_SelectorName) && !Previous.is(tok::at) &&
   State.Stack.back().ObjCSelectorNameFound &&
-  State.Stack.back().BreakBeforeParameter)
+  State.Stack.back().BreakBeforeParameter &&
+  (Style.ObjCBreakBeforeNestedBlockParam ||
+   !Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)))
 return true;
 
   unsigned NewLineColumn = getNewLineColumn(State);

diff  --git a/clang/unittests/Format/FormatTestObjC.cpp 
b/clang/unittests/Format/FormatTestObjC.cpp
index a34a0832286b..c33f93bcf99d 100644
--- a/clang/unittests/Format/FormatTestObjC.cpp
+++ b/clang/unittests/Format/FormatTestObjC.cpp
@@ -18,6 +18,7 @@
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -51,18 +52,24 @@ class FormatTestObjC : public ::testing::Test {
 return *Result;
   }
 
-  void verifyFormat(StringRef Code) {
+  void _verifyFormat(const char *File, int Line, StringRef Code) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code)));
   }
 
-  void verifyIncompleteFormat(StringRef Code) {
+  void _verifyIncompleteFormat(const char *File, int Line, StringRef Code) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(), format(test::messUp(Code), SC_ExpectIncomplete));
   }
 
   FormatStyle Style;
 };
 
+#define verifyIncompleteFormat(...)
\
+  _verifyIncompleteFormat(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
+
 TEST(FormatTestObjCStyle, DetectsObjCInHeaders) {
   auto Style = getStyle("LLVM", "a.h", "none",
 "@interface\n"
@@ -1453,6 +1460,39 @@ TEST_F(FormatTestObjC, BreakLineBeforeNestedBlockParam) {
   "   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {\n"
   " u = v;\n"
   "   }]");
+
+  verifyFormat("[self block:^(void) {\n"
+   "  doStuff();\n"
+   "} completionHandler:^(void) {\n"
+   "  doStuff();\n"
+   "  [self block:^(void) {\n"
+   "doStuff();\n"
+   "  } completionHandler:^(void) {\n"
+   "doStuff();\n"
+   "  }];\n"
+   "}];");
+
+  Style.ColumnLimit = 0;
+  verifyFormat("[[SessionService sharedService] "
+   "loadWindowWithCompletionBlock:^(SessionWindow *window) {\n"
+   "  if (window) {\n"
+   "[self windowDidLoad:window];\n"
+   "  } else {\n"
+   "[self errorLoadingWindow];\n"
+   "  }\n"
+   "}];");
+  verifyFormat("[controller test:^{\n"
+   "  doStuff();\n"
+   "} withTimeout:5 completionHandler:^{\n"
+   "  doStuff();\n"
+   "}];");
+  verifyFormat(
+  "[self setupTextFieldSignals:@[\n"
+  "  self.documentWidthField,\n"
+  "  self.documentHeightField,\n"
+  "] solver:^(NSTextField *textField) {\n"
+  "  return [self.representedObject 
solveEquationForTextField:textField];\n"
+  "}];");
 }
 
 TEST_F(FormatTestObjC, IfNotUnlikely) {



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

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 306787.
nickdesaulniers added a comment.

- check the successor's terminator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91895

Files:
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -151,23 +151,23 @@
 #define MY_CASE2(X, Y, U, V) case X: Y; case U: V
 
 int fallthrough_macro1(int n) {
-  MY_SWITCH(n, 13, n *= 2, 14, break)  // expected-warning{{unannotated fall-through between switch labels}}
+  MY_SWITCH(n, 13, n *= 2, 14, break)
 
   switch (n + 1) {
 MY_CASE(33, n += 2);
-MY_CASE(44, break);  // expected-warning{{unannotated fall-through between switch labels}}
-MY_CASE(55, n += 3);
+MY_CASE(44, n += 3);  // expected-warning{{unannotated fall-through between switch labels}}
+MY_CASE(55, break);
   }
 
   switch (n + 3) {
 MY_CASE(333, return 333);
-MY_CASE2(444, n += 44, , break);  // expected-warning{{unannotated fall-through between switch labels}}
+MY_CASE2(444, n += 44, , break);
 MY_CASE(555, n += 33);
   }
 
-  MY_SWITCH2(n + 4, MY_CASE(17, n *= 3), MY_CASE(19, break))  // expected-warning{{unannotated fall-through between switch labels}}
+  MY_SWITCH2(n + 4, MY_CASE(17, n *= 3), MY_CASE(19, break))
 
-  MY_SWITCH2(n + 5, MY_CASE(21, break), MY_CASE2(23, n *= 7, 25, break))  // expected-warning{{unannotated fall-through between switch labels}}
+  MY_SWITCH2(n + 5, MY_CASE(21, break), MY_CASE2(23, n *= 7, 25, break))
 
   return n;
 }
@@ -237,9 +237,15 @@
   [[clang::fallthrough]]; // no diagnostics
 case 1:
   x++;
+case 2: // no diagnostics
+  break;
+case 3:
+  x++;
 default: // \
 expected-warning{{unannotated fall-through between switch labels}} \
+expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} \
 expected-note{{insert 'break;' to avoid fall-through}}
+  x++;
   break;
   }
 }
@@ -257,9 +263,15 @@
   [[clang::fallthrough]]; // no diagnostics
 case 1:
   x++;
+case 2: // no diagnostics
+  break;
+case 3:
+  x++;
 default: // \
 expected-warning{{unannotated fall-through between switch labels}} \
+expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} \
 expected-note{{insert 'break;' to avoid fall-through}}
+  x++;
   break;
 }
   };
Index: clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
@@ -7,11 +7,17 @@
 case 0:
   x++;
   [[clang::fallthrough]]; // no diagnostics
+case 2:
+  x++;
+case 3: // no diagnostics
+  break;
 case 1:
   x++;
 default: // \
 expected-warning{{unannotated fall-through between switch labels}} \
+expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} \
 expected-note{{insert 'break;' to avoid fall-through}}
+  x++;
   break;
 }
   };
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1149,6 +1149,15 @@
   continue; // Fallthrough annotation, good.
 }
 
+// If next statment in P's successor is a break with no other
+// statements, no issue.
+if (P->succ_size() == 1) {
+  const CFGBlock::AdjacentBlock  = *P->succ_begin();
+  const Stmt *Term = Succ->getTerminatorStmt();
+  if (Succ->size() == 0 && Term && isa(Term))
+continue;
+}
+
 if (!LastStmt) { // This block contains no executable statements.
   // Traverse its predecessors.
   std::copy(P->pred_begin(), P->pred_end(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1156
+  const CFGBlock::AdjacentBlock  = *P->succ_begin();
+  const Stmt *Term = B.getTerminatorStmt();
+  if (Succ->size() == 0 && Term && isa(Term))

ah, we want `Succ`'s terminator stmt, not B's.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91895

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

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: rsmith, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nickdesaulniers requested review of this revision.

Clang differs slightly than GCC for the pattern:

switch (x) {

  case 0:
++x;
  default:
break;

}

Clang will warn, GCC will not.  This is making it excessively painful to
enable -Wimplicit-fallthrough for Linux kernel builds with Clang, see
below link in which 140 patches were sent to try to fix up these
differences in kernel code. Kernel and GCC developers point out that
Clang should not warn in this case.

Link: https://github.com/ClangBuiltLinux/linux/issues/636
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432
Link: 
https://lore.kernel.org/lkml/CANiq72=E_gEVvqUUTSqU4zegC2=yZSTM4b=4G-iofp6d3=u...@mail.gmail.com/T/#t
Signed-off-by: Nick Desaulniers 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91895

Files:
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -151,23 +151,23 @@
 #define MY_CASE2(X, Y, U, V) case X: Y; case U: V
 
 int fallthrough_macro1(int n) {
-  MY_SWITCH(n, 13, n *= 2, 14, break)  // expected-warning{{unannotated fall-through between switch labels}}
+  MY_SWITCH(n, 13, n *= 2, 14, break)
 
   switch (n + 1) {
 MY_CASE(33, n += 2);
-MY_CASE(44, break);  // expected-warning{{unannotated fall-through between switch labels}}
-MY_CASE(55, n += 3);
+MY_CASE(44, n += 3);  // expected-warning{{unannotated fall-through between switch labels}}
+MY_CASE(55, break);
   }
 
   switch (n + 3) {
 MY_CASE(333, return 333);
-MY_CASE2(444, n += 44, , break);  // expected-warning{{unannotated fall-through between switch labels}}
+MY_CASE2(444, n += 44, , break);
 MY_CASE(555, n += 33);
   }
 
-  MY_SWITCH2(n + 4, MY_CASE(17, n *= 3), MY_CASE(19, break))  // expected-warning{{unannotated fall-through between switch labels}}
+  MY_SWITCH2(n + 4, MY_CASE(17, n *= 3), MY_CASE(19, break))
 
-  MY_SWITCH2(n + 5, MY_CASE(21, break), MY_CASE2(23, n *= 7, 25, break))  // expected-warning{{unannotated fall-through between switch labels}}
+  MY_SWITCH2(n + 5, MY_CASE(21, break), MY_CASE2(23, n *= 7, 25, break))
 
   return n;
 }
@@ -237,9 +237,15 @@
   [[clang::fallthrough]]; // no diagnostics
 case 1:
   x++;
+case 2: // no diagnostics
+  break;
+case 3:
+  x++;
 default: // \
 expected-warning{{unannotated fall-through between switch labels}} \
+expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} \
 expected-note{{insert 'break;' to avoid fall-through}}
+  x++;
   break;
   }
 }
@@ -257,9 +263,15 @@
   [[clang::fallthrough]]; // no diagnostics
 case 1:
   x++;
+case 2: // no diagnostics
+  break;
+case 3:
+  x++;
 default: // \
 expected-warning{{unannotated fall-through between switch labels}} \
+expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} \
 expected-note{{insert 'break;' to avoid fall-through}}
+  x++;
   break;
 }
   };
Index: clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough-blocks.cpp
@@ -7,11 +7,17 @@
 case 0:
   x++;
   [[clang::fallthrough]]; // no diagnostics
+case 2:
+  x++;
+case 3: // no diagnostics
+  break;
 case 1:
   x++;
 default: // \
 expected-warning{{unannotated fall-through between switch labels}} \
+expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} \
 expected-note{{insert 'break;' to avoid fall-through}}
+  x++;
   break;
 }
   };
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1149,6 +1149,15 @@
   continue; // Fallthrough annotation, good.
 }
 
+// If next statment in P's successor is a break with no other
+// statements, no issue.
+if (P->succ_size() == 1) {
+  const CFGBlock::AdjacentBlock  = *P->succ_begin();
+  const Stmt *Term = B.getTerminatorStmt();
+  if (Succ->size() == 0 && Term && isa(Term))
+continue;
+}
+
 if (!LastStmt) { // This block contains no executable 

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Olivier Giroux via Phabricator via cfe-commits]

__simt__ added a comment.

I've resumed looking at the library code.

How should I check for support? Is it going to be e.g. 
`__has_feature(__builtin_clear_padding)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D91821: Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1055
+  // semi-colon.
+  EndLoc = PP.getLastCachedTokenLocation();
+}

It seems to me that either `EndLoc` was miscomputed and should already point to 
the last token that we skipped over, in which case we should fix 
`ParseDecltypeSpecifier` to return the correct source location, or `EndLoc` is 
correct and we should revert back to the corresponding token and only annotate 
that portion of the token stream as the decltype specifier. (Or maybe a third 
option: `EndLoc` is meaningless when the type specifier is erroneous, in which 
case this code is still wrong because it's using that value in the case where 
backtracking is not enabled.)

Rolling back the token stream to the token corresponding to `EndLoc` seems 
likely to give us the best error recovery here, assuming that `EndLoc` is 
currently set to the token that we think is the last one that's intended to be 
part of the decltype specifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91821

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

[PATCH] D91868: [clangd] Mention when CXXThis is implicit in exposed AST.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

This looks like an improvement to me as well, thanks!




Comment at: clang-tools-extra/clangd/DumpAST.cpp:233
   return CCO->getConstructor()->getNameAsString();
+if (const auto *CTE = dyn_cast(S)) {
+  bool Const = CTE->getType()->getPointeeType().isLocalConstQualified();

should we ensure we always return within the branch (as we do within the rest 
of the branches to make sure we don't accumulate details by mistake)? e.g:
```
if(CXXThisExpr) {
  details = {}
  if (const) details += "const";
  if (implicit) details += "implicit";
  return join(",", details);
}
```



Comment at: clang-tools-extra/clangd/unittests/DumpASTTests.cpp:79
   {R"cpp(
-template  int root() {
-  (void)root();
+namespace root {
+template  int tmpl() {

is this change intentional ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91868

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

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Zequan Wu via Phabricator via cfe-commits]

zequanwu marked an inline comment as done.
zequanwu added inline comments.



Comment at: clang/test/CXX/cpp/cpp.predefined/p2.cpp:1
+// RUN: %clang_cc1 %s -verify
+// expected-no-diagnostics

rnk wrote:
> Let's expand on this:
> - test that we don't set the macro when compiling C (`-x c`)
> - test that we don't set the macro when `-mthread-model single` is passed
> 
> Pass an extra macro def on the command line to control the preprocessor test 
> expectations similar to p1.cpp
I found split-file is handy to separate different test expectations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

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

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK, I'm going to drop hasAttrName from the patch (it's a relatively tiny piece 
of the code/tests) and land this, and put that back up as another review for 
discussion. And thanks, it's been interesting and I learned a bunch!
We didn't talk about overloading isImplicit to apply to attrs too, but it 
doesn't seem like that was controversial (and it does help with the tests).

In D89743#2408747 , @aaron.ballman 
wrote:

>>> Maybe `attr(equivalentAttrName("..."))` or so, possibly constrasting to 
>>> `attr(exactAttrName("..."))`?
>
> I think this design is the most flexible and will be the most understandable 
> in the long-run because it makes it clear which name-matching behavior is 
> being used. But do we want to stick `attr` in the name of this to limit it to 
> just attributes or do we think this is a generalized concept that should 
> apply to matching other names as well? e.g., 
> `hasType(equivalentName("long"))` to match either `long`, `long int` or 
> `signed long int` and `hasType(exactName(`long int`))` to only match `long 
> int` and not the other forms? Or do you think this is a bridge too far 
> because it would be reasonable to expect `hasType(equivalentName("long"))` to 
> match typedef names because those are equivalent (or perhaps more 
> problematically, would the user expect `hasType(equivalentName("struct 
> foo::baz"))` to match as an equivalent name for the parameter type 
> (spelled with a `using` declaration) in `void foo(blah b)`)? (I think I've 
> just about talked myself into keeping `attr` in the name so we don't try to 
> use this for other kinds of identifiers with equivalent names.)

Putting attr in the name is the *safe* option - as we've seen, going for a 
generic name and hoping it can work consistently for all types over time is 
risky.
(The cost of a confusing "overload" set is also high because the polymorphism 
mechanism is poorly understood - most users probably can't easily look up the 
variant they're using and ignore the others).
On the other hand, when it is consistent, I think the uniformity is nice - so a 
generic name isn't without merit, it's just a bit of a gamble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

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

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 306777.
zequanwu added a comment.

Update tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/CXX/cpp/cpp.predefined/p2.cpp
  clang/test/Preprocessor/init-aarch64.c

Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -233,6 +233,7 @@
 // AARCH64-NEXT: #define __SIZE_TYPE__ long unsigned int
 // AARCH64-NEXT: #define __SIZE_WIDTH__ 64
 // AARCH64_CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// AARCH64_CXX: #define __STDCPP_THREADS__ 1
 // AARCH64-NEXT: #define __STDC_HOSTED__ 1
 // AARCH64-NEXT: #define __STDC_UTF_16__ 1
 // AARCH64-NEXT: #define __STDC_UTF_32__ 1
Index: clang/test/CXX/cpp/cpp.predefined/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/cpp/cpp.predefined/p2.cpp
@@ -0,0 +1,17 @@
+// RUN: split-file %s %t.dir
+// RUN: %clang_cc1 -verify %t.dir/defined.cpp
+// RUN: %clang_cc1 -verify -mthread-model posix %t.dir/defined.cpp
+// RUN: %clang_cc1 -verify -mthread-model single %t.dir/not-defined.cpp
+// RUN: %clang_cc1 -verify -x c %t.dir/not-defined.cpp
+
+//--- defined.cpp
+// expected-no-diagnostics
+#ifndef __STDCPP_THREADS__
+#error __STDCPP_THREADS__ is not defined in posix thread model.
+#endif
+
+//--- not-defined.cpp
+// expected-no-diagnostics
+#ifdef __STDCPP_THREADS__
+#error __STDCPP_THREADS__ is defined in single thread model.
+#endif
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -403,6 +403,12 @@
 Builder.defineMacro("__STDCPP_DEFAULT_NEW_ALIGNMENT__",
 Twine(TI.getNewAlign() / TI.getCharWidth()) +
 TI.getTypeConstantSuffix(TI.getSizeType()));
+
+//   -- __STDCPP_­THREADS__
+//  Defined, and has the value integer literal 1, if and only if a
+//  program can have more than one thread of execution.
+if (LangOpts.getThreadModel() == LangOptions::ThreadModelKind::POSIX)
+  Builder.defineMacro("__STDCPP_THREADS__", "1");
   }
 
   // In C11 these are environment macros. In C++11 they are only defined
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1037,12 +1037,6 @@
   Opts.StrictVTablePointers = Args.hasArg(OPT_fstrict_vtable_pointers);
   Opts.ForceEmitVTables = Args.hasArg(OPT_fforce_emit_vtables);
   Opts.UnwindTables = Args.hasArg(OPT_munwind_tables);
-  Opts.ThreadModel =
-  std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
-  if (Opts.ThreadModel != "posix" && Opts.ThreadModel != "single")
-Diags.Report(diag::err_drv_invalid_value)
-<< Args.getLastArg(OPT_mthread_model)->getAsString(Args)
-<< Opts.ThreadModel;
   Opts.TrapFuncName = std::string(Args.getLastArgValue(OPT_ftrap_function_EQ));
   Opts.UseInitArray = !Args.hasArg(OPT_fno_use_init_array);
 
@@ -3553,6 +3547,16 @@
   Args.hasFlag(OPT_fexperimental_relative_cxx_abi_vtables,
OPT_fno_experimental_relative_cxx_abi_vtables,
/*default=*/false);
+
+  std::string ThreadModel =
+  std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
+  if (ThreadModel != "posix" && ThreadModel != "single")
+Diags.Report(diag::err_drv_invalid_value)
+<< Args.getLastArg(OPT_mthread_model)->getAsString(Args) << ThreadModel;
+  Opts.setThreadModel(
+  llvm::StringSwitch(ThreadModel)
+  .Case("posix", LangOptions::ThreadModelKind::POSIX)
+  .Case("single", LangOptions::ThreadModelKind::Single));
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -49,7 +49,7 @@
   const PreprocessorOptions 
   CodeGenOptions CodeGenOpts;
   const TargetOptions TargetOpts;
-  const LangOptions LangOpts;
+  LangOptions LangOpts;
   std::unique_ptr VMContext;
   std::unique_ptr M;
   std::unique_ptr Builder;
@@ -147,7 +147,7 @@
 // The debug info output isn't affected by CodeModel and
 // 

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> Let's make sure that we follow the same semantics that GCC does, particularly 
> w.r.t. union, bitfields, and padding at the end of an object (whether it's in 
> an array or not).

Agreed. I'm planning to run some tests tomorrow once the nightly build has 
updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D83697: [clang][cli] Port Frontend option flags to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

This is close, just a few nits. Also, since this has the first uses of 
`IsNegative`, it'd be great to have a couple of tests for one of the flags it's 
used on.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137
 
+static llvm::Optional
+normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned TableIndex,

Nit: clang/Basic/LLVM.h pulls Optional in `namespace clang` so you don't need 
the `llvm::` qualifier.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:138-139
+static llvm::Optional
+normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned TableIndex,
+const ArgList , DiagnosticsEngine ) {
+  if (Args.hasArg(Opt))

Please drop parameter names for `TableIndex` and `Diags`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83697

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Let's make sure that we follow the same semantics that GCC does, particularly 
w.r.t. union, bitfields, and padding at the end of an object (whether it's in 
an array or not).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Driver/Options.td:473
+MarshallingInfoFlag<"DependencyOutputOpts.OutputFormat", 
"DependencyOutputFormat::Make">,
+Normalizer<"makeFlagToValueNormalizer(DependencyOutputFormat::NMake)">;
 def Mach : Flag<["-"], "Mach">, Group;

jansvoboda11 wrote:
> The original patch used the following normalizer:
> `(normalizeFlagToValue DependencyOutputFormat::NMake>)`.
> 
> I wanted to remove the parenthesis and redundant type argument, hence 
> `makeFlagToValueNormalizer`. This could be useful later on when normalizing 
> flags to non-literal types.
This seems a lot cleaner, thanks!



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137-143
+template 
 void denormalizeSimpleFlag(SmallVectorImpl ,
const char *Spelling,
CompilerInvocation::StringAllocator SA,
-   unsigned TableIndex, unsigned Value) {
+   unsigned TableIndex, T Value) {
   Args.push_back(Spelling);
 }

On the switch from `T` to `unsigned`, it'd be nice to avoid a separate 
instantiation for each enumeration, since the code is identical... it'll just 
bloat compile time for no reason. Maybe:
```
/// The tblgen-erated code passes in a fifth parameter of an arbitrary type, but
/// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
/// unnecessary template instantiations and just ignore it with a variadic
/// argument.
static void denormalizeSimpleFlag(
SmallVectorImpl , const char *Spelling,
CompilerInvocation::StringAllocator, unsigned, /*T*/...) {
  Args.push_back(Spelling);
}
```
Note that it'd also be nice to make this `static` and drop unused parameter 
names, as I've done here.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:145-154
+template  struct FlagToValueNormalizer {
+  T Value;
+
+  llvm::Optional operator()(OptSpecifier Opt, unsigned TableIndex,
+   const ArgList , DiagnosticsEngine ) {
+if (Args.hasArg(Opt))
+  return Value;

Please put this in an anonymous namespace.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:148-149
+
+  llvm::Optional operator()(OptSpecifier Opt, unsigned TableIndex,
+   const ArgList , DiagnosticsEngine ) {
+if (Args.hasArg(Opt))

Please drop the parameter names for `TableIndex` and `Diags`, since they aren't 
used.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:156-159
+template 
+FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
+  return FlagToValueNormalizer{std::move(Value)};
 }

Please declare this `static`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83694

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

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 306773.
flx added a comment.

Fix formatting and comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91893

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -476,3 +476,17 @@
   auto Copy = Orig.reference();
   Update(Copy);
 }
+
+void negativeCopiedFromReferenceToModifiedVar() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig;
+  const auto NecessaryCopy = Ref;
+  Orig.nonConstMethod();
+}
+
+void negativeCopiedFromGetterOfReferenceToModifiedVar() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -43,6 +43,12 @@
   return referenceType(pointee(qualType(isConstQualified(;
 }
 
+// Returns QualType matcher for pointers to const.
+AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isPointerToConst) {
+  using namespace ast_matchers;
+  return pointerType(pointee(qualType(isConstQualified(;
+}
+
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
   NameList) {
   return llvm::any_of(NameList, [](const std::string ) {
Index: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -58,7 +58,7 @@
   SmallPtrSet DeclRefs;
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   auto ConstReferenceOrValue =
-  qualType(anyOf(referenceType(pointee(qualType(isConstQualified(,
+  qualType(anyOf(matchers::isReferenceToConst(),
  unless(anyOf(referenceType(), pointerType(),
   substTemplateTypeParmType();
   auto ConstReferenceOrValueOrReplaced = qualType(anyOf(
@@ -71,6 +71,20 @@
   Matches =
   match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  // References and pointers to const assignments.
+  Matches =
+  match(findAll(declStmt(
+has(varDecl(hasType(qualType(matchers::isReferenceToConst())),
+hasInitializer(ignoringImpCasts(DeclRefToVar)),
+Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  Matches =
+  match(findAll(declStmt(has(varDecl(
+hasType(qualType(matchers::isPointerToConst())),
+hasInitializer(ignoringImpCasts(unaryOperator(
+hasOperatorName("&"), hasUnaryOperand(DeclRefToVar,
+Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
   return DeclRefs;
 }
 
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -19,6 +19,14 @@
 namespace performance {
 namespace {
 
+using namespace ::clang::ast_matchers;
+using llvm::StringRef;
+using utils::decl_ref_expr::isOnlyUsedAsConst;
+
+static constexpr StringRef kObjectArg = "objectArg";
+static constexpr StringRef kInitFunctionCall = "initFunctionCall";
+static constexpr StringRef kOldVarDecl = "oldVarDecl";
+
 void recordFixes(const VarDecl , ASTContext ,
  DiagnosticBuilder ) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
@@ -29,10 +37,91 @@
   }
 }
 
-} // namespace
+AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningMethodCall) {
+  // Match method call expressions where the `this` argument is only used as
+  // const, this will be checked in `check()` part. This returned const
+  // reference is highly likely to outlive the local const reference of the
+  // variable being declared. The assumption is that the const reference being
+  // returned either points to a global static variable or to a member of the
+  // called object.
+  return cxxMemberCallExpr(
+  

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

[Felix Berger via Phabricator via cfe-commits]

flx created this revision.
flx added reviewers: aaron.ballman, hokein.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
flx requested review of this revision.

Extend the check to not only look at the variable the unnecessarily copied
variable is initialized from, but also ensure that any variable the old variable
references is not modified.

Extend DeclRefExprUtils to also count references and pointers to const assigned
from the DeclRef we check for const usages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91893

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -476,3 +476,17 @@
   auto Copy = Orig.reference();
   Update(Copy);
 }
+
+void negativeCopiedFromReferenceToModifiedVar() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig;
+  const auto NecessaryCopy = Ref;
+  Orig.nonConstMethod();
+}
+
+void negativeCopiedFromGetterOfReferenceToModifiedVar() {
+  ExpensiveToCopyType Orig;
+  const auto  = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -43,6 +43,12 @@
   return referenceType(pointee(qualType(isConstQualified(;
 }
 
+// Returns QualType matcher for pointers to const.
+AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isPointerToConst) {
+  using namespace ast_matchers;
+  return pointerType(pointee(qualType(isConstQualified(;
+}
+
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
   NameList) {
   return llvm::any_of(NameList, [](const std::string ) {
Index: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -58,7 +58,7 @@
   SmallPtrSet DeclRefs;
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   auto ConstReferenceOrValue =
-  qualType(anyOf(referenceType(pointee(qualType(isConstQualified(,
+  qualType(anyOf(matchers::isReferenceToConst(),
  unless(anyOf(referenceType(), pointerType(),
   substTemplateTypeParmType();
   auto ConstReferenceOrValueOrReplaced = qualType(anyOf(
@@ -71,6 +71,20 @@
   Matches =
   match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  // References and pointers to const assignments.
+  Matches =
+  match(findAll(declStmt(
+has(varDecl(hasType(qualType(matchers::isReferenceToConst())),
+hasInitializer(ignoringImpCasts(DeclRefToVar)),
+Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  Matches =
+  match(findAll(declStmt(has(varDecl(
+hasType(qualType(matchers::isPointerToConst())),
+hasInitializer(ignoringImpCasts(unaryOperator(
+hasOperatorName("&"), hasUnaryOperand(DeclRefToVar,
+Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
   return DeclRefs;
 }
 
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -7,7 +7,6 @@
 //===--===//
 
 #include "UnnecessaryCopyInitialization.h"
-
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
@@ -19,6 +18,14 @@
 namespace performance {
 namespace {
 
+using namespace ::clang::ast_matchers;
+using llvm::StringRef;
+using utils::decl_ref_expr::isOnlyUsedAsConst;
+
+static constexpr StringRef kObjectArg = "objectArg";
+static constexpr StringRef kInitFunctionCall = "initFunctionCall";
+static constexpr StringRef kOldVarDecl = "oldVarDecl";
+
 void recordFixes(const VarDecl , ASTContext ,
  DiagnosticBuilder ) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
@@ 

[PATCH] D91844: [llvm][clang] Add checks for the smart pointers with the possibility to be null

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

Is it possible to split these up into separate patches for unrelated code?




Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1346-1353
   if (!Ptr) {
 // Search in reverse order so that the most-derived type is handled first.
 ArrayRef> Bases = Search->getSuperClasses();
 for (const auto  : llvm::reverse(Bases)) {
   if ((Ptr = createArgument(Arg, Attr, Base.first)))
 break;
 }

Can we just add a single assertion here? It looks to me like every caller wants 
a valid return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91844

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

[PATCH] D91565: Guard init_priority attribute within libc++

[Abhina Sree via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c7e24c4b689: Guard init_priority attribute within libc++ 
(authored by zibi, committed by abhina.sreeskantharajan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/test/SemaCXX/init-priority-attr.cpp
  libcxx/include/__config
  libcxx/src/experimental/memory_resource.cpp
  libcxx/src/iostream.cpp

Index: libcxx/src/iostream.cpp
===
--- libcxx/src/iostream.cpp
+++ libcxx/src/iostream.cpp
@@ -77,7 +77,7 @@
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams __attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we create
 // the standard streams, otherwise it may not be alive during program termination
Index: libcxx/src/experimental/memory_resource.cpp
===
--- libcxx/src/experimental/memory_resource.cpp
+++ libcxx/src/experimental/memory_resource.cpp
@@ -76,16 +76,6 @@
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support any GCC attributes.
-# define _LIBCPP_INIT_PRIORITY_MAX
-#else
-# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
-#endif
-
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1421,6 +1421,12 @@
 #define _LIBCPP_HAS_NO_FGETPOS_FSETPOS
 #endif
 
+#if __has_attribute(init_priority)
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG
Index: clang/test/SemaCXX/init-priority-attr.cpp
===
--- clang/test/SemaCXX/init-priority-attr.cpp
+++ clang/test/SemaCXX/init-priority-attr.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -verify=unknown %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -DSYSTEM -verify=unknown-system %s
 
 #if defined(SYSTEM)
 #5 "init-priority-attr.cpp" 3 // system header
@@ -23,25 +25,35 @@
 extern Two koo[];
 
 Two foo __attribute__((init_priority(101))) ( 5, 6 );
+ // unknown-system-no-diagnostics
+ // unknown-warning@-2 {{unknown attribute 'init_priority' ignored}}
 
 Two goo __attribute__((init_priority(2,3))) ( 5, 6 ); // expected-error {{'init_priority' attribute takes one argument}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two coo[2]  __attribute__((init_priority(100)));
 #if !defined(SYSTEM)
-// expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+  // expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+  // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two boo[2]  __attribute__((init_priority(65536)));
 #if !defined(SYSTEM)
-// expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+ // expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+ // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two koo[4]  __attribute__((init_priority(1.13))); // expected-error {{'init_priority' attribute requires an integer constant}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two func()  __attribute__((init_priority(1001))); // expected-error {{'init_priority' attribute only applies to variables}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
+
 
 int i  __attribute__((init_priority(1001))); // expected-error {{can only use 'init_priority' attribute on file-scope definitions of objects of class type}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' 

[clang] 2c7e24c - Guard init_priority attribute within libc++

[Abhina Sreeskantharajan via cfe-commits]

Author: Zbigniew Sarbinowski
Date: 2020-11-20T15:53:26-05:00
New Revision: 2c7e24c4b6893a93ddb2b2cca91eaf5bf7956965

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

LOG: Guard init_priority attribute within libc++

Not all platforms support priority attribute. I'm moving conditional definition 
of this attribute to `include/__config`.

Reviewed By: #libc, aaron.ballman

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

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/test/SemaCXX/init-priority-attr.cpp
libcxx/include/__config
libcxx/src/experimental/memory_resource.cpp
libcxx/src/iostream.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 62c97cb0440c..8555bd351747 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -381,6 +381,9 @@ def TargetELF : TargetSpec {
   let ObjectFormats = ["ELF"];
 }
 
+def TargetSupportsInitPriority : TargetSpec {
+  let CustomCode = [{ !Target.getTriple().isOSzOS() }];
+}
 // Attribute subject match rules that are used for #pragma clang attribute.
 //
 // A instance of AttrSubjectMatcherRule represents an individual match rule.
@@ -2221,7 +2224,7 @@ def WorkGroupSizeHint :  InheritableAttr {
   let Documentation = [Undocumented];
 }
 
-def InitPriority : InheritableAttr {
+def InitPriority : InheritableAttr, 
TargetSpecificAttr {
   let Spellings = [GCC<"init_priority", /*AllowInC*/0>];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 8e04ea84b240..1781543cc4f3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -79,7 +79,7 @@ relative ordering of values is important. For example:
 initialization being the opposite.
 
 This attribute is only supported for C++ and Objective-C++ and is ignored in
-other language modes.
+other language modes. Currently, this attribute is not implemented on z/OS.
   }];
 }
 

diff  --git a/clang/test/SemaCXX/init-priority-attr.cpp 
b/clang/test/SemaCXX/init-priority-attr.cpp
index 5b5e3b9eb940..8c0a17682bb0 100644
--- a/clang/test/SemaCXX/init-priority-attr.cpp
+++ b/clang/test/SemaCXX/init-priority-attr.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -DSYSTEM 
-verify %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -verify=unknown %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -DSYSTEM 
-verify=unknown-system %s
 
 #if defined(SYSTEM)
 #5 "init-priority-attr.cpp" 3 // system header
@@ -23,25 +25,35 @@ extern Two coo[];
 extern Two koo[];
 
 Two foo __attribute__((init_priority(101))) ( 5, 6 );
+ // unknown-system-no-diagnostics
+ // unknown-warning@-2 {{unknown attribute 'init_priority' ignored}}
 
 Two goo __attribute__((init_priority(2,3))) ( 5, 6 ); // expected-error 
{{'init_priority' attribute takes one argument}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two coo[2]  __attribute__((init_priority(100)));
 #if !defined(SYSTEM)
-// expected-error@-2 {{'init_priority' attribute requires integer constant 
between 101 and 65535 inclusive}}
+  // expected-error@-2 {{'init_priority' attribute requires integer constant 
between 101 and 65535 inclusive}}
+  // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two boo[2]  __attribute__((init_priority(65536)));
 #if !defined(SYSTEM)
-// expected-error@-2 {{'init_priority' attribute requires integer constant 
between 101 and 65535 inclusive}}
+ // expected-error@-2 {{'init_priority' attribute requires integer constant 
between 101 and 65535 inclusive}}
+ // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two koo[4]  __attribute__((init_priority(1.13))); // expected-error 
{{'init_priority' attribute requires an integer constant}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two func()  __attribute__((init_priority(1001))); // expected-error 
{{'init_priority' attribute only applies to variables}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
+
 
 int i  __attribute__((init_priority(1001))); // expected-error {{can only use 
'init_priority' attribute on file-scope definitions of objects of class type}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 int main() {
-  Two foo __attribute__((init_priority(1001)));// 

[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1443
 
+  if (Arg *A =
+  Args.getLastArg(OPT_mabi_EQ_vec_default, OPT_mabi_EQ_vec_extabi)) {

ZarkoCA wrote:
> Xiangling_L wrote:
> > Should we also check if target feature altivec[`-target-feature +altivec`] 
> > is enabled when using these two options? If so, we should also add related 
> > testcases.
> Both of these options require that -maltivec is also selected which sets 
> `-target-feature +altivec`.
> Both of these options require that -maltivec is also selected which sets 
> `-target-feature +altivec`.

It seems the scenario you pointed out are driver invocation use case. But when 
users invoke clang -cc1 not driver with -mabi=ext-abi/default, should we also 
check if the user specify `-target-feature +altivec` as well? For now, based on 
my observation, if invoking cc1 without `-target-feature +altivec` but with 
`-mabi=ext-abi` only like `// RUN: %clang_cc1 -mabi=vec-extabi -triple 
powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s`, the error is `error: 
unknown type name 'vector'`. Should we explicitly say `-target-feature 
+altivec` is required as what we do for driver?



Comment at: clang/test/Driver/aix-vec-extabi.c:2
+// RUN:  %clang -### -target powerpc-unknown-aix -S -maltivec -mabi=vec-extabi 
%s 2>&1 | \
+// RUN:  FileCheck %s --check-prefix=CHECK
+

minor: We can omit `--check-prefix=CHECK`



Comment at: llvm/include/llvm/Target/TargetOptions.h:179
 
+/// AIXExtendedAltivecABI - This flag returns true when -vec-extabi is
+/// specified. The code generator is then able to use both volatile and

minor: 
s/AIXExtendedAltivecABI/EnableAIXExtendedAltivecABI


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

https://reviews.llvm.org/D89684

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

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/tools/driver/CMakeLists.txt:123
+
+check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS)

ro wrote:
> MaskRay wrote:
> > ro wrote:
> > > MaskRay wrote:
> > > > GNU ld reports a warning instead of an error when an unknown `-z` is 
> > > > seen. The warning remains a warning even with `--fatal-warnings`.
> > > > GNU ld reports a warning instead of an error when an unknown `-z` is 
> > > > seen. The warning remains a warning even with `--fatal-warnings`.
> > > 
> > > Thanks for reminding me about that misfeature of GNU `ld`.  I guess 
> > > `check_linker_flags` needs to be updated to handle that.
> > > In the case at hand, it won't matter either way: the flag is only passed 
> > > to `ld`, which on Solaris is guaranteed to be the native linker.  Once 
> > > (if at all) I get around to completing D85309, I can deal with that.  For 
> > > now, other targets won't see linker warnings about this flag, other than 
> > > when the flag is used at build time.
> > OK. Then I guess you can condition this when the OS is Solaris?
> > OK. Then I guess you can condition this when the OS is Solaris?
> 
> I fear not: `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` is tested inside an `if` in 
> `Solaris.cpp`: this code is also compiled on non-Solaris hosts.  Why are you 
> worried about the definition being always present?
It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong result 
for GNU ld, even if it is not used for non-Solaris. We should make the value 
correct in other configurations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/test/CXX/cpp/cpp.predefined/p2.cpp:1
+// RUN: %clang_cc1 %s -verify
+// expected-no-diagnostics

Let's expand on this:
- test that we don't set the macro when compiling C (`-x c`)
- test that we don't set the macro when `-mthread-model single` is passed

Pass an extra macro def on the command line to control the preprocessor test 
expectations similar to p1.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

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

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

[Alexander Yermolovich via Phabricator via cfe-commits]

ayermolo added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:35
 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
+CODEGENOPT(Dwarf64   , 1, 0) ///< -gdwarf64.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.

dblaikie wrote:
> ayermolo wrote:
> > dblaikie wrote:
> > > ayermolo wrote:
> > > > dblaikie wrote:
> > > > > ayermolo wrote:
> > > > > > ikudrin wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > Is there any precedent to draw from for this flag name? (Does 
> > > > > > > > GCC support DWARF64? Does it support it under this flag name or 
> > > > > > > > some other? (similarly with other gcc-like compilers (Intel's? 
> > > > > > > > Whoever else... )))
> > > > > > > It looks like we are pioneering in that area. To me, the proposed 
> > > > > > > name looks consonant with other debug-related switches.
> > > > > > I didn't see any dwarf64 flags in gcc:
> > > > > > https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html
> > > > > > 
> > > > > > I tried to follow clang convention for other dwarf flags.
> > > > > Huh - tried making really big binaries or anything (or checking the 
> > > > > GCC source) to see if it does it implicitly under some conditions?
> > > > > Hmm - looks like this maybe came up at the Linux Plumbers Conference 
> > > > > & the suggested flag was -fdwarf64/32: 
> > > > > https://linuxplumbersconf.org/event/7/contributions/746/attachments/578/1018/DWARF5-64.pdf
> > > > >  (this avoids the "does g imply debug info" and avoids the subtle 
> > > > > distinction between "-gdwarf64 and -gdwarf-N" the presence of the '-' 
> > > > > changing the meaning of the number quite significantly). Though 
> > > > > hardly authoritative
> > > > > https://linuxplumbersconf.org/event/7/sessions/90/attachments/583/1201/dwarf-bof-notes-aug24-lpc-2020.txt
> > > > >  - seems some other options were (are?) under consideration too. 
> > > > > Might be worth touching base with the folks involved in those 
> > > > > discussions to see where they're at with regard to naming/support?
> > > > > 
> > > > > (they also touch on the "all units must agree" issue - so not sure if 
> > > > > the same folks involved in those discussions have also been included 
> > > > > in the discussions around debug info 32/64 sorting as another 
> > > > > approach that may avoid the "all units must agree" constraint (I 
> > > > > assume that's the reason they had that constraint))
> > > > In the DWARFV5-64 pdf it says 64 bit support has no patches and is 
> > > > after DWARF5. Although it's not clear if they are talking about DWARF64 
> > > > support for V5 or in general.
> > > > 
> > > > I have not hacked our build system to use gcc for builds that can 
> > > > overflow debug_info. I scanned through gcc code and was only able to 
> > > > find references to dwarf 64 in go library, and in dwarf2out.c. In 
> > > > latter it relies on DWARF_OFFSET_SIZE macro. 
> > > > 
> > > > I don't quite understand the "all [CU] units must agree" part either. 
> > > > From DWARF perspective we are free to match on CU level DWARF32/64, and 
> > > > consumer are free not to do anything beyond that. So if overflow 
> > > > occurs, will so be it. What we are trying to do in linker with sorting 
> > > > is being "nice" to the users, and kind of going beyond what spec 
> > > > requires.
> > > > 
> > > > Sounds like no conclusion was reached on their side, but only one of 
> > > > them -gdwarf64 follows naming convention of other debug flags.
> > > > > * -fdwarf64/-fdwarf32
> > > > >   * or -gdwarf32 or -gdwarf64
> > > > >   * or -gdbdwarf=32/64
> > > > 
> > > > 
> > > > 
> > > > 
> > > > only one of them -gdwarf64 follows naming convention of other debug 
> > > > flags.
> > > 
> > > There are many debug flags that don't use the '-g' prefix. 
> > > (-fdebug-types-section comes to mind, but I think - this was discussed in 
> > > depth earlier this year with regards to the -gsplit-dwarf flag, for 
> > > instance: https://www.mail-archive.com/gcc@gcc.gnu.org/msg92495.html - 
> > > though at least the DWARF64 flag doesn't have the legacy that 
> > > -gsplit-dwarf has that complicates things further there)
> > Ah, thanks for the context. My takeaway it's a mess. :)
> > Personally I find it more confusing that there are debug options that start 
> > with -f and -g, rather then that some -g enable debug output. When I look 
> > at documentation I just want to have see all the related options grouped in 
> > one area/one prefix, but that's just how my brain works.
> > That being said I don't have particular strong opinion about naming 
> > convention of this flag. Judging from that conversation, maybe there is 
> > some preference for -f, but mainly it was a big push against changing an 
> > option after it was introduced and proliferated. 
> Yep, bit of a mess - hence the concern about making it messier/trying to 
> drive in that 

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D89743#2406017 , @sammccall wrote:

> In D89743#2363201 , @aaron.ballman 
> wrote:
>
>> In D89743#2360258 , @sammccall 
>> wrote:
>>
>>> (while this is useful to you, let's keep discussing, but I'm also happy to 
>>> stop and land this with your preferred API/semantics - just LMK if changes 
>>> are needed)
>>
>> Let's hold off on landing for just a little bit. I don't think changes are 
>> needed yet, but the discussion may change my opinion and we might as well 
>> avoid churn. However, if you need to land part of this while we discuss 
>> other parts, LMK.
>
> Sorry I haven't had more to say on this. I would like to land at least the 
> attr-in-DynTypedNode part, as this unblocks some stuff unrelated to matchers 
> in clangd (someone just filed a dupe bug, which reminded me...)

That part LGTM and is fine to land.

> Happy to cut out all of the matcher part, or land just attr() without any 
> attr matchers, or all of it, or make another round of changes...

I think the `attr()` matcher is definitely reasonable as it stands and can be 
landed as well.

>>> In D89743#2360032 , @aaron.ballman 
>>> wrote:
>>>
 
>>
>> My mental model is that there is no declaration of an attribute (in the 
>> usual sense, because users cannot specify their own attributes without 
>> changing the compiler), and so there's not a referential model like there 
>> are with a DeclRefExpr that refers back to a specific declaration. Instead, 
>> to me, an attribute is a bit more like a builtin type --  you may have 
>> multiple ways to spell the type (`char` vs `signed char` or `unsigned char`, 
>> `long int` vs `long`, etc), but it's the same type under the hood.
>
> Ah, this also makes sense! Again, the wrinkle is IIUC `attr::Kind` is like a 
> builtin type, while `Attr` is more like VectorType - it specifies a builtin 
> type, but also some other stuff.

Yeah, that's a fair point.

>> However, that brings up an interesting observation: you can't do 
>> `hasType(hasName("long"))` and instead have to do 
>> `hasType(asString("long"))`. I don't recall the background about why there 
>> is `asString()` for matching string types vs `hasName()` -- you wouldn't 
>> happen to remember that context, would you?
>
> That was before my time I'm afraid.
> It seems the implementation checks whether QualType::getAsString() exactly 
> matches the argument. `asString("long")` matches a type specified as `long` 
> or `long int`. But `asString("long int")` doesn't match anything! (Sugar 
> types muddle things a bit but don't fundamentally change this).
> My guess is this implementation was just a question of practicality: parsing 
> the matcher argument as a type isn't feasible, but having the node producing 
> a single string to compare to is easy. And it generalizes to complicated 
> template types.

That's my guess as well -- also, I wasn't aware of the `long` vs `long int` 
behavioral difference with `asString()`, that's.. neat.

> It's reasonable to think of these as doing different things `hasName` is 
> querying a property, while `asString` is doing a specialized comparison of 
> the whole thing.
> I'm not *sure* that's why the different names where chosen though. And the 
> fact that `hasName` allows qualifiers definitely feels a bit bolted on here.
>
>> For instance, is the correct pattern to allow `attr(asString("aligned"))` to 
>> map back to an `AlignedAttr` regardless of how the attribute was spelled in 
>> source, similar to how `asString("long")` matches `long int`?
>
> If we're following precedents (not sure we have to):
>
> - per your mental model, `asString` would pick one fixed name for each 
> attribute kind - which may not be the one used!
> - ... but also `asString` should roughly be "node as string" which logically 
> includes arguments and should use the right name (since the node records it)
> - `hasName` should match against the `name` attribute of Attr, possibly with 
> optional qualifiers
> - ... but there's no precedent for rejecting synonymous names per se, and 
> this may be confusing behavior
>
> So I don't think either of these are a *great* fit, assuming we think 
> "attribute named XYZ or any synonym" is the most important operation (which I 
> agree with).

I agree that this is the most common use-case.

> I can't think of a pithy name, but I also think it's OK to be a bit verbose 
> and explicit here - attributes aren't the most commonly used bit of the AST.
>
>> Maybe `attr(equivalentAttrName("..."))` or so, possibly constrasting to 
>> `attr(exactAttrName("..."))`?

I think this design is the most flexible and will be the most understandable in 
the long-run because it makes it clear which name-matching behavior is being 
used. But do we want to stick `attr` in the name of this to limit it to just 
attributes 

[PATCH] D91861: [clang][cli] Split DefaultAnyOf into a default value and ImpliedByAnyOf

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4038-4043
   if (((FLAGS)::CC1Option) &&  
\
-  (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {
\
+  (ALWAYS_EMIT ||  
\
+   (EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE && !(IMPLIED_CHECK { 
\
 DENORMALIZER(Args, SPELLING, NEG_SPELLING, SA, TABLE_INDEX,
\
  EXTRACTOR(this->KEYPATH));
\
   }

dexonsmith wrote:
> I'm not entirely sure if the comment applies here, since a `bool` option is 
> simpler, but it would be good to have tests to demonstrate correct behaviour 
> for options with the following scenarios:
> - option != default, it can be implied but the antecedents are false
> - option == default, it can be implied but the antecedents are false
> - option != default, it can be implied and the antecedents are true
> - option == default, it can be implied and the antecedents are true
(Maybe the tests already exist in tree; if so, please just point me at them)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91861

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

[PATCH] D91861: [clang][cli] Split DefaultAnyOf into a default value and ImpliedByAnyOf

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I like this direction; the logic in the `.td` files seems much cleaner. The 
`MARSHALLING` macro logic seems a bit harder to follow and there may be a bug, 
but I'm not sure. See comments inline.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3762-3764
+this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);  
\
+if (IMPLIED_CHECK) 
\
+  this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);
\

This flip to the logic is interesting. I see it now matches the non-flag case; 
I can't remember if there was a subtle reason it was different before though.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4027-4031
+  if (((FLAGS)::CC1Option) &&  
\
+  (ALWAYS_EMIT ||  
\
+   (EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE && !(IMPLIED_CHECK { 
\
 DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   
\
   }

I'm not sure this logic is quite right. It looks to me like if an option can 
very be implied, it will never be seriazed to `-cc1`, even if its current value 
is not an implied one.

Or have I understood the conditions under which `IMPLIED_CHECK` returns `true`?

IIUC, then this logic seems closer:
```
if (((FLAGS)::CC1Option) &&  \
(ALWAYS_EMIT ||  \
 (EXTRACTOR(this->KEYPATH) !=\
  (IMPLIED_CHECK ? (DEFAULT_VALUE)   \
 : (MERGER(DEFAULT_VALUE, IMPLIED_VALUE)) {  \
  DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   \
}
```

It would be great to see tests in particular for a bitset (or similar) option 
where the merger does a union.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4038-4043
   if (((FLAGS)::CC1Option) &&  
\
-  (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {
\
+  (ALWAYS_EMIT ||  
\
+   (EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE && !(IMPLIED_CHECK { 
\
 DENORMALIZER(Args, SPELLING, NEG_SPELLING, SA, TABLE_INDEX,
\
  EXTRACTOR(this->KEYPATH));
\
   }

I'm not entirely sure if the comment applies here, since a `bool` option is 
simpler, but it would be good to have tests to demonstrate correct behaviour 
for options with the following scenarios:
- option != default, it can be implied but the antecedents are false
- option == default, it can be implied but the antecedents are false
- option != default, it can be implied and the antecedents are true
- option == default, it can be implied and the antecedents are true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91861

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

[clang-tools-extra] de5b0b7 - [clangd] semanticTokens: fields are 'property', not 'member'

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-11-20T20:53:12+01:00
New Revision: de5b0b776fd7de72078256e003ede4fb5c37cdcb

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

LOG: [clangd] semanticTokens: fields are 'property', not 'member'

This isn't obvious, but vscode maps member as 'entity.name.function.member',
so it's really for member functions.

Fixes https://github.com/clangd/vscode-clangd/issues/105

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index a9c885c7275e..1a78e7a8c0da 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -561,7 +561,8 @@ llvm::StringRef toSemanticTokenType(HighlightingKind Kind) {
 // FIXME: better function/member with static modifier?
 return "function";
   case HighlightingKind::Field:
-return "member";
+// Not "member": https://github.com/clangd/vscode-clangd/issues/105
+return "property";
   case HighlightingKind::Class:
 return "class";
   case HighlightingKind::Enum:



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

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

[Wei Mi via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:978-982
   Opts.DebugInfoForProfiling = Args.hasFlag(
   OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profiling, false);
+  Opts.PseudoProbeForProfiling =
+  Args.hasFlag(OPT_fpseudo_probe_for_profiling,
+   OPT_fno_pseudo_probe_for_profiling, false);

hoy wrote:
> wmi wrote:
> > Should it emit an error if DebugInfoForProfiling and 
> > PseudoProbeForProfiling are enabled at the same time? I see that from the 
> > other patch related with pseudoprobe discriminator, these two flags are 
> > incompatible.
> Yes, we should. It's done in D91756 passbuilder.h : 
> https://reviews.llvm.org/D91756#change-Bsibk2p32T1c for both `clang` and 
> `opt`.
> 
Ok, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86502

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

[PATCH] D89046: [AST] Build recovery expression by default for all language.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/CodeGen/builtins-ppc-error.c:51
 void testCTF(int index) {
-  vec_ctf(vsi, index); //expected-error {{argument to 
'__builtin_altivec_vcfsx' must be a constant integer}}
-  vec_ctf(vui, index); //expected-error {{argument to 
'__builtin_altivec_vcfsx' must be a constant integer}}
+  vec_ctf(vsi, index); //expected-error {{argument to 
'__builtin_altivec_vcfsx' must be a constant integer}} expected-error 
{{argument to '__builtin_altivec_vcfux' must be a constant integer}}
+  vec_ctf(vui, index); //expected-error {{argument to 
'__builtin_altivec_vcfsx' must be a constant integer}} expected-error 
{{argument to '__builtin_altivec_vcfux' must be a constant integer}}

hokein wrote:
> sammccall wrote:
> > hmm, this doesn't exactly look right to me - we know the type of `vsi` so 
> > we should only be considering the signed case I thought.
> > 
> > However the existing diagnostic for `vui` indicates that it's considering 
> > the **signed** case, so I guess this is already broken/bad.
> yeah, we should know this is the signed case.
> 
> after the macro expansion, the code looks like 
> 
> ```
> _Generic((vsi), vector int
>: (vector float)__builtin_altivec_vcfsx((vector int)(vsi), 
> (index)), vector unsigned int
>: (vector float)__builtin_altivec_vcfux((vector unsigned 
> int)(vsi), (index))); 
> ```
> 
> it is a `GenericSelectionExpr` which contains a switch-like structure (see 
> the AST below), so when we traverse it, we will traverse both 
> `__builtin_altivec_vcfsx` and `__builtin_altivec_vcfux`.
> 
> ```
> `-GenericSelectionExpr 0x9527238  '__vector float' 
> contains-errors
> |-ImplicitCastExpr 0x9527220  '__vector int' 
> 
> | `-ParenExpr 0x9526980  '__vector int' lvalue
> |   `-DeclRefExpr 0x9526960  '__vector int' lvalue Var 0x9526568 
> 'vsi' '__vector int' non_odr_use_unevaluated
> |-VectorType 0x91923c0 '__vector int' altivec 4
> | `-BuiltinType 0x91296e0 'int'
> |-case  '__vector int' selected
> | |-VectorType 0x91923c0 '__vector int' altivec 4
> | | `-BuiltinType 0x91296e0 'int'
> | `-CStyleCastExpr 0x9526db8  '__vector float' 
> contains-errors 
> |   `-RecoveryExpr 0x9526d70  '' 
> contains-errors lvalue
> | |-DeclRefExpr 0x9526bc0  '' Function 
> 0x95269e8 '__builtin_altivec_vcfsx' '__attribute__((__vector_size__(4 * 
> sizeof(float float (__attribute__((__vector_size__(4 * sizeof(int 
> int, int)'
> | |-CStyleCastExpr 0x9526c68  '__vector int' 
> | | `-ImplicitCastExpr 0x9526c50  '__vector int' 
>  part_of_explicit_cast
> | |   `-ParenExpr 0x9526c30  '__vector int' lvalue
> | | `-DeclRefExpr 0x9526be0  '__vector int' lvalue Var 
> 0x9526568 'vsi' '__vector int'
> | `-ParenExpr 0x9526cb0  'const int' lvalue
> |   `-DeclRefExpr 0x9526c90  'const int' lvalue ParmVar 
> 0x95267c8 'index' 'const int'
> `-case  '__vector unsigned int'
>   |-VectorType 0x9192700 '__vector unsigned int' altivec 4
>   | `-BuiltinType 0x9129780 'unsigned int'
>   `-CStyleCastExpr 0x95271f8  '__vector float' 
> contains-errors 
> `-RecoveryExpr 0x95271b0  '' 
> contains-errors lvalue
>   |-DeclRefExpr 0x9527000  '' Function 
> 0x9526e28 '__builtin_altivec_vcfux' '__attribute__((__vector_size__(4 * 
> sizeof(float float (__attribute__((__vector_size__(4 * sizeof(unsigned 
> int unsigned int, int)'
>   |-CStyleCastExpr 0x95270a8  '__vector unsigned int' 
> 
>   | `-ImplicitCastExpr 0x9527090  '__vector int' 
>  part_of_explicit_cast
>   |   `-ParenExpr 0x9527070  '__vector int' lvalue
>   | `-DeclRefExpr 0x9527020  '__vector int' lvalue Var 
> 0x9526568 'vsi' '__vector int'
>   `-ParenExpr 0x95270f0  'const int' lvalue
> `-DeclRefExpr 0x95270d0  'const int' lvalue ParmVar 
> 0x95267c8 'index' 'const int'
> ```
> 
> 
> > However the existing diagnostic for vui indicates that it's considering the 
> > signed case, so I guess this is already broken/bad.
> 
> hmm, `vsi` and `vui` have the same type `vector signed int`, so I think there 
> is a typo for `vui`, it should be `vector unsigned int`?
> it is a GenericSelectionExpr which contains a switch-like structure (see the 
> AST below), so when we traverse it, we will traverse both 
> __builtin_altivec_vcfsx and __builtin_altivec_vcfux.

OK, that makes sense I guess.

> I think there is a typo for vui, it should be vector unsigned int?

Yep, that definitely looks to be the case.



Comment at: clang/test/Sema/__try.c:55
+// expected-error{{too few arguments to function call, expected 1, have 
0}} \
+// expected-error{{expected ';' after expression}}
   }

hokein wrote:
> sammccall wrote:
> > this seems bad, am I missing something?
> agree, 

[PATCH] D91565: Guard init_priority attribute within libc++

[Zbigniew Sarbinowski via Phabricator via cfe-commits]

zibi marked an inline comment as done.
zibi added inline comments.



Comment at: clang/test/SemaCXX/init-priority-attr.cpp:26
 Two foo __attribute__((init_priority(101))) ( 5, 6 );
+#if defined(__MVS__)
+ #if defined(SYSTEM)

aaron.ballman wrote:
> Rather than using the preprocessor, you can assign a prefix to be checked to 
> `-verify`. e.g., `-verify=unknown` would allow you to do `// unknown-warning 
> {{unknown attribute 'init_priority' ignored}}` or `unknown-no-diagnostics` 
> that is only checked when `-verify=unknown`.
> 
> I wonder if it would be cleaner to use that solution here instead of the 
> preprocessor by adding a new RUN line that's specific to AIX, and setting 
> some non-AIX triples for the other run lines.
That simplifies a lot in the expanse of duplicating the run.
Thank you Aaron.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

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

[PATCH] D91565: Guard init_priority attribute within libc++

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for the fixes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

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

[PATCH] D91565: Guard init_priority attribute within libc++

[Zbigniew Sarbinowski via Phabricator via cfe-commits]

zibi updated this revision to Diff 306753.
zibi added a comment.

Make test cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/test/SemaCXX/init-priority-attr.cpp
  libcxx/include/__config
  libcxx/src/experimental/memory_resource.cpp
  libcxx/src/iostream.cpp

Index: libcxx/src/iostream.cpp
===
--- libcxx/src/iostream.cpp
+++ libcxx/src/iostream.cpp
@@ -77,7 +77,7 @@
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams __attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we create
 // the standard streams, otherwise it may not be alive during program termination
Index: libcxx/src/experimental/memory_resource.cpp
===
--- libcxx/src/experimental/memory_resource.cpp
+++ libcxx/src/experimental/memory_resource.cpp
@@ -76,16 +76,6 @@
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support any GCC attributes.
-# define _LIBCPP_INIT_PRIORITY_MAX
-#else
-# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
-#endif
-
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1435,6 +1435,12 @@
 #define _LIBCPP_HAS_NO_FGETPOS_FSETPOS
 #endif
 
+#if __has_attribute(init_priority)
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG
Index: clang/test/SemaCXX/init-priority-attr.cpp
===
--- clang/test/SemaCXX/init-priority-attr.cpp
+++ clang/test/SemaCXX/init-priority-attr.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -fsyntax-only -DSYSTEM -verify %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -verify=unknown %s
+// RUN: %clang_cc1 -triple=s390x-none-zos -fsyntax-only -DSYSTEM -verify=unknown-system %s
 
 #if defined(SYSTEM)
 #5 "init-priority-attr.cpp" 3 // system header
@@ -23,25 +25,35 @@
 extern Two koo[];
 
 Two foo __attribute__((init_priority(101))) ( 5, 6 );
+ // unknown-system-no-diagnostics
+ // unknown-warning@-2 {{unknown attribute 'init_priority' ignored}}
 
 Two goo __attribute__((init_priority(2,3))) ( 5, 6 ); // expected-error {{'init_priority' attribute takes one argument}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two coo[2]  __attribute__((init_priority(100)));
 #if !defined(SYSTEM)
-// expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+  // expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+  // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two boo[2]  __attribute__((init_priority(65536)));
 #if !defined(SYSTEM)
-// expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+ // expected-error@-2 {{'init_priority' attribute requires integer constant between 101 and 65535 inclusive}}
+ // unknown-warning@-3 {{unknown attribute 'init_priority' ignored}}
 #endif
 
 Two koo[4]  __attribute__((init_priority(1.13))); // expected-error {{'init_priority' attribute requires an integer constant}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 Two func()  __attribute__((init_priority(1001))); // expected-error {{'init_priority' attribute only applies to variables}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
+
 
 int i  __attribute__((init_priority(1001))); // expected-error {{can only use 'init_priority' attribute on file-scope definitions of objects of class type}}
+// unknown-warning@-1 {{unknown attribute 'init_priority' ignored}}
 
 int main() {
-  Two foo __attribute__((init_priority(1001)));	// expected-error {{can only use 'init_priority' attribute on file-scope definitions of objects of class type}}
+  Two foo __attribute__((init_priority(1001))); // expected-error {{can 

[PATCH] D91805: [OPENMP]Use the real pointer value as base, not indexed value.

[Alexey Bataev 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 rGc964f3081415: [OPENMP]Use the real pointer value as base, 
not indexed value. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91805

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp


Index: clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
===
--- clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
+++ clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
@@ -38,10 +38,17 @@
 // CHECK-DAG: [[MAPS1:@.+]] = private unnamed_addr constant [2 x i64] [i64 32, 
i64 281474976710673]
 // CHECK: @main
 int main(void) {
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* 
%{{.+}}, i32 0, i32 0
+// CHECK: [[BPTRC0:%.+]] = bitcast i8** [[BPTR0]] to %struct.MyObject***
+// CHECK: store %struct.MyObject** @objects, %struct.MyObject*** [[BPTRC0]],
 // CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* @{{.+}}, 
i64 -1, i32 1, i8** %{{.+}}, i8** %{{.+}}, i64* getelementptr inbounds ([1 x 
i64], [1 x i64]* [[SIZES0]], i32 0, i32 0), i64* getelementptr inbounds ([1 x 
i64], [1 x i64]* [[MAPS0]], i32 0, i32 0), i8** null, i8** null)
-#pragma omp target enter data map(to : objects [0:1])
+#pragma omp target enter data map(to : objects [1:1])
+// CHECK: [[OBJ:%.+]] = load %struct.MyObject*, %struct.MyObject** @objects,
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* 
%{{.+}}, i32 0, i32 0
+// CHECK: [[BPTRC0:%.+]] = bitcast i8** [[BPTR0]] to %struct.MyObject**
+// CHECK: store %struct.MyObject* [[OBJ]], %struct.MyObject** [[BPTRC0]],
 // CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* @{{.+}}, 
i64 -1, i32 2, i8** %{{.+}}, i8** %{{.+}}, i64* %{{.+}}, i64* getelementptr 
inbounds ([2 x i64], [2 x i64]* [[MAPS1]], i32 0, i32 0), i8** null, i8** null)
-#pragma omp target enter data map(to : objects[0].arr [0:1])
+#pragma omp target enter data map(to : objects[1].arr [0:1])
 
   return 0;
 }
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7904,8 +7904,11 @@
 IsCaptureFirstInfo = false;
 FirstPointerInComplexData = false;
   } else if (FirstPointerInComplexData) {
-BP = CGF.EmitOMPSharedLValue(I->getAssociatedExpression())
- .getAddress(CGF);
+QualType Ty = Components.rbegin()
+  ->getAssociatedDeclaration()
+  ->getType()
+  .getNonReferenceType();
+BP = CGF.EmitLoadOfPointer(BP, Ty->castAs());
 FirstPointerInComplexData = false;
   }
 }


Index: clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
===
--- clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
+++ clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
@@ -38,10 +38,17 @@
 // CHECK-DAG: [[MAPS1:@.+]] = private unnamed_addr constant [2 x i64] [i64 32, i64 281474976710673]
 // CHECK: @main
 int main(void) {
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* %{{.+}}, i32 0, i32 0
+// CHECK: [[BPTRC0:%.+]] = bitcast i8** [[BPTR0]] to %struct.MyObject***
+// CHECK: store %struct.MyObject** @objects, %struct.MyObject*** [[BPTRC0]],
 // CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* @{{.+}}, i64 -1, i32 1, i8** %{{.+}}, i8** %{{.+}}, i64* getelementptr inbounds ([1 x i64], [1 x i64]* [[SIZES0]], i32 0, i32 0), i64* getelementptr inbounds ([1 x i64], [1 x i64]* [[MAPS0]], i32 0, i32 0), i8** null, i8** null)
-#pragma omp target enter data map(to : objects [0:1])
+#pragma omp target enter data map(to : objects [1:1])
+// CHECK: [[OBJ:%.+]] = load %struct.MyObject*, %struct.MyObject** @objects,
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* %{{.+}}, i32 0, i32 0
+// CHECK: [[BPTRC0:%.+]] = bitcast i8** [[BPTR0]] to %struct.MyObject**
+// CHECK: store %struct.MyObject* [[OBJ]], %struct.MyObject** [[BPTRC0]],
 // CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* @{{.+}}, i64 -1, i32 2, i8** %{{.+}}, i8** %{{.+}}, i64* %{{.+}}, i64* getelementptr inbounds ([2 x i64], [2 x i64]* [[MAPS1]], i32 0, i32 0), i8** null, i8** null)
-#pragma omp target enter data map(to : objects[0].arr [0:1])
+#pragma omp target enter data map(to : objects[1].arr [0:1])
 
   return 0;
 }
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- 

[clang] c964f30 - [OPENMP]Use the real pointer value as base, not indexed value.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-11-20T11:34:14-08:00
New Revision: c964f308141578f24932c68a03af5fae7f876011

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

LOG: [OPENMP]Use the real pointer value as base, not indexed value.

After fix for PR48174 the base pointer for pointer-based
array-sections/array-subscripts will be emitted as `[idx]`, but
actually it should be just `ptr`, i.e. the address stored in the ponter
to point correctly to the beginning of the array. Currently it may lead
to a crash in the runtime.

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 84733369a4a1..c15f6350b95e 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7904,8 +7904,11 @@ class MappableExprsHandler {
 IsCaptureFirstInfo = false;
 FirstPointerInComplexData = false;
   } else if (FirstPointerInComplexData) {
-BP = CGF.EmitOMPSharedLValue(I->getAssociatedExpression())
- .getAddress(CGF);
+QualType Ty = Components.rbegin()
+  ->getAssociatedDeclaration()
+  ->getType()
+  .getNonReferenceType();
+BP = CGF.EmitLoadOfPointer(BP, Ty->castAs());
 FirstPointerInComplexData = false;
   }
 }

diff  --git 
a/clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp 
b/clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
index 14960191af03..77d416a685aa 100644
--- a/clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
+++ b/clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
@@ -38,10 +38,17 @@ MyObject *objects;
 // CHECK-DAG: [[MAPS1:@.+]] = private unnamed_addr constant [2 x i64] [i64 32, 
i64 281474976710673]
 // CHECK: @main
 int main(void) {
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* 
%{{.+}}, i32 0, i32 0
+// CHECK: [[BPTRC0:%.+]] = bitcast i8** [[BPTR0]] to %struct.MyObject***
+// CHECK: store %struct.MyObject** @objects, %struct.MyObject*** [[BPTRC0]],
 // CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* @{{.+}}, 
i64 -1, i32 1, i8** %{{.+}}, i8** %{{.+}}, i64* getelementptr inbounds ([1 x 
i64], [1 x i64]* [[SIZES0]], i32 0, i32 0), i64* getelementptr inbounds ([1 x 
i64], [1 x i64]* [[MAPS0]], i32 0, i32 0), i8** null, i8** null)
-#pragma omp target enter data map(to : objects [0:1])
+#pragma omp target enter data map(to : objects [1:1])
+// CHECK: [[OBJ:%.+]] = load %struct.MyObject*, %struct.MyObject** @objects,
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* 
%{{.+}}, i32 0, i32 0
+// CHECK: [[BPTRC0:%.+]] = bitcast i8** [[BPTR0]] to %struct.MyObject**
+// CHECK: store %struct.MyObject* [[OBJ]], %struct.MyObject** [[BPTRC0]],
 // CHECK: call void @__tgt_target_data_begin_mapper(%struct.ident_t* @{{.+}}, 
i64 -1, i32 2, i8** %{{.+}}, i8** %{{.+}}, i64* %{{.+}}, i64* getelementptr 
inbounds ([2 x i64], [2 x i64]* [[MAPS1]], i32 0, i32 0), i8** null, i8** null)
-#pragma omp target enter data map(to : objects[0].arr [0:1])
+#pragma omp target enter data map(to : objects[1].arr [0:1])
 
   return 0;
 }



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

[PATCH] D91565: Guard init_priority attribute within libc++

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision as: libc++.
ldionne added a comment.

LGTM from libc++'s perspective. You should wait until the Clang part is LGTM'd 
before committing, of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

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

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: alexfh, aaron.ballman.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
Herald added a project: clang.
njames93 requested review of this revision.

Add methods for emitting diagnostics with no location as well as a special 
diagnostic for configuration errors.
These show up in the errors as [clang-tidy-config].
The reason to use a custom name rather than the check name is to distinguish 
the error isn't the same category as the check that reported it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91885

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -10,7 +10,9 @@
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context) {
+diag("DiagWithNoLoc");
+  }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override {
 Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
   }
@@ -26,9 +28,10 @@
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", );
-  EXPECT_EQ(2ul, Errors.size());
-  EXPECT_EQ("type specifier", Errors[0].Message.Message);
-  EXPECT_EQ("variable", Errors[1].Message.Message);
+  EXPECT_EQ(3ul, Errors.size());
+  EXPECT_EQ("DiagWithNoLoc", Errors[0].Message.Message);
+  EXPECT_EQ("type specifier", Errors[1].Message.Message);
+  EXPECT_EQ("variable", Errors[2].Message.Message);
 }
 
 } // namespace test
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
@@ -5,11 +5,11 @@
 // RUN:   {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
 // RUN:   {key: readability-identifier-naming.StructCase, value: CAMEL}, \
 // RUN:   {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
-// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
 
-// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
-// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
+// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'? [clang-tidy-config]
+// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'? [clang-tidy-config]
 // Don't try to suggest an alternative for 'CAMEL'
-// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}}
+// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase' [clang-tidy-config]
 // This fails on the EditDistance, but as it matches ignoring case suggest the correct value
-// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}}
+// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'? [clang-tidy-config]
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

[PATCH] D91644: [OPENMP]Honor constantness of captured variables.

[Alexey Bataev 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 rG8f51dc49673c: [OPENMP]Honor constantness of captured 
variables. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91644

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/OpenMP/distribute_firstprivate_messages.cpp
  clang/test/OpenMP/openmp_capture_const_var_ast_print.cpp
  clang/test/OpenMP/teams_distribute_firstprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp

Index: clang/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
+++ clang/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
@@ -39,8 +39,8 @@
   S3 =(const S3 );
   
 public:
-  S3() : a(0) {} // expected-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
-  S3(S3 ) : a(s3.a) {} // expected-note {{candidate constructor not viable: 1st argument ('const S3') would lose const qualifier}}
+  S3() : a(0) {} // expected-note 2 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
+  S3(S3 ) : a(s3.a) {} // expected-note 2 {{candidate constructor not viable: 1st argument ('const S3') would lose const qualifier}}
 };
 const S3 c;
 const S3 ca[5];
@@ -110,7 +110,7 @@
   for (i = 0; i < argc; ++i) foo();
 
 #pragma omp target
-#pragma omp teams distribute simd firstprivate (k, a, b, c, d, f) // expected-error {{firstprivate variable with incomplete type 'S1'}} expected-error {{incomplete type 'S1' where a complete type is required}}
+#pragma omp teams distribute simd firstprivate (k, a, b, c, d, f) // expected-error {{firstprivate variable with incomplete type 'S1'}} expected-error {{incomplete type 'S1' where a complete type is required}} expected-error {{no matching constructor for initialization of 'S3'}}
   for (i = 0; i < argc; ++i) foo();
 
 #pragma omp target
Index: clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
@@ -38,8 +38,8 @@
   S3 =(const S3 );
   
 public:
-  S3() : a(0) {} // expected-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
-  S3(S3 ) : a(s3.a) {} // expected-note {{candidate constructor not viable: 1st argument ('const S3') would lose const qualifier}}
+  S3() : a(0) {} // expected-note 2 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
+  S3(S3 ) : a(s3.a) {} // expected-note 2 {{candidate constructor not viable: 1st argument ('const S3') would lose const qualifier}}
 };
 const S3 c;
 const S3 ca[5];
@@ -109,7 +109,7 @@
   for (i = 0; i < argc; ++i) foo();
 
 #pragma omp target
-#pragma omp teams distribute parallel for simd firstprivate (a, b, c, d, f) // expected-error {{firstprivate variable with incomplete type 'S1'}} expected-error {{incomplete type 'S1' where a complete type is required}}
+#pragma omp teams distribute parallel for simd firstprivate (a, b, c, d, f) // expected-error {{firstprivate variable with incomplete type 'S1'}} expected-error {{incomplete type 'S1' where a complete type is required}} expected-error {{no matching constructor for initialization of 'S3'}}
   for (i = 0; i < argc; ++i) foo();
 
 #pragma omp target
Index: clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
@@ -37,8 +37,8 @@
   S3 =(const S3 );
   
 public:
-  S3() : a(0) {} // expected-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
-  S3(S3 ) : a(s3.a) {} // expected-note {{candidate constructor not viable: 1st argument ('const S3') would lose const qualifier}}
+  S3() : a(0) {} // expected-note 2 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
+  S3(S3 ) : a(s3.a) {} // expected-note 2 {{candidate constructor not viable: 1st argument ('const S3') would lose const qualifier}}
 };
 const S3 c;
 const S3 ca[5];
@@ -108,7 +108,7 @@
   for (i = 0; i < argc; ++i) foo();
 
 #pragma omp target
-#pragma omp teams distribute parallel for firstprivate (a, b, c, d, f) // expected-error {{firstprivate variable with incomplete type 'S1'}} expected-error {{incomplete 

[clang] 8f51dc4 - [OPENMP]Honor constantness of captured variables.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-11-20T11:11:47-08:00
New Revision: 8f51dc49673c494cc1d118979b596288e938af13

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

LOG: [OPENMP]Honor constantness of captured variables.

Fixes bug reported via Stackoverflow:
https://stackoverflow.com/questions/64179168/clang-overload-resolution-failure-with-templates-and-openmp-collapse

Need to honor constantness of private/target variables to  make the code
compilable.

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

Added: 
clang/test/OpenMP/openmp_capture_const_var_ast_print.cpp

Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/OpenMP/distribute_firstprivate_messages.cpp
clang/test/OpenMP/teams_distribute_firstprivate_messages.cpp
clang/test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp

clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
clang/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7a8124fadfd7..60a685bfdf15 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17468,7 +17468,11 @@ bool Sema::tryCaptureVariable(
   if (IsTargetCap || IsOpenMPPrivateDecl == OMPC_private ||
   (IsGlobal && !IsGlobalCap)) {
 Nested = !IsTargetCap;
+bool HasConst = DeclRefType.isConstQualified();
 DeclRefType = DeclRefType.getUnqualifiedType();
+// Don't lose diagnostics about assignments to const.
+if (HasConst)
+  DeclRefType.addConst();
 CaptureType = Context.getLValueReferenceType(DeclRefType);
 break;
   }

diff  --git a/clang/test/OpenMP/distribute_firstprivate_messages.cpp 
b/clang/test/OpenMP/distribute_firstprivate_messages.cpp
index 346d1a834a86..25a5e9db0eb8 100644
--- a/clang/test/OpenMP/distribute_firstprivate_messages.cpp
+++ b/clang/test/OpenMP/distribute_firstprivate_messages.cpp
@@ -28,8 +28,8 @@ class S3 {
   S3 =(const S3 );
 
 public:
-  S3() : a(0) {} // expected-note {{candidate constructor not viable: requires 
0 arguments, but 1 was provided}}
-  S3(S3 ) : a(s3.a) {} // expected-note {{candidate constructor not viable: 
1st argument ('const S3') would lose const qualifier}}
+  S3() : a(0) {} // expected-note 2 {{candidate constructor not viable: 
requires 0 arguments, but 1 was provided}}
+  S3(S3 ) : a(s3.a) {} // expected-note 2 {{candidate constructor not 
viable: 1st argument ('const S3') would lose const qualifier}}
 };
 const S3 c;
 const S3 ca[5];
@@ -95,7 +95,7 @@ int main(int argc, char **argv) {
   for (i = 0; i < argc; ++i) foo();
   #pragma omp target
   #pragma omp teams
-  #pragma omp distribute firstprivate (a, b, c, d, f) // expected-error 
{{firstprivate variable with incomplete type 'S1'}} expected-warning {{Type 
'const S2' is not trivially copyable and not guaranteed to be mapped 
correctly}} expected-warning {{Type 'const S3' is not trivially copyable and 
not guaranteed to be mapped correctly}} expected-error {{incomplete type 'S1' 
where a complete type is required}}
+  #pragma omp distribute firstprivate (a, b, c, d, f) // expected-error 
{{firstprivate variable with incomplete type 'S1'}} expected-warning {{Type 
'const S2' is not trivially copyable and not guaranteed to be mapped 
correctly}} expected-warning {{Type 'const S3' is not trivially copyable and 
not guaranteed to be mapped correctly}} expected-error {{incomplete type 'S1' 
where a complete type is required}} expected-error {{no matching constructor 
for initialization of 'S3'}}
   for (i = 0; i < argc; ++i) foo();
   #pragma omp target
   #pragma omp teams

diff  --git a/clang/test/OpenMP/openmp_capture_const_var_ast_print.cpp 
b/clang/test/OpenMP/openmp_capture_const_var_ast_print.cpp
new file mode 100644
index ..4d4b0b00cbaf
--- /dev/null
+++ b/clang/test/OpenMP/openmp_capture_const_var_ast_print.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify 
%s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only 
-verify %s -ast-print | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+struct vector {
+  vector() = default;
+  int at(int) { return 0; }
+  int at(int) const { return 1; }
+};
+
+// CHECK: template  void test(const vector begin_vec) {
+// CHECK:#pragma omp 

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:978-982
   Opts.DebugInfoForProfiling = Args.hasFlag(
   OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profiling, false);
+  Opts.PseudoProbeForProfiling =
+  Args.hasFlag(OPT_fpseudo_probe_for_profiling,
+   OPT_fno_pseudo_probe_for_profiling, false);

wmi wrote:
> Should it emit an error if DebugInfoForProfiling and PseudoProbeForProfiling 
> are enabled at the same time? I see that from the other patch related with 
> pseudoprobe discriminator, these two flags are incompatible.
Yes, we should. It's done in D91756 passbuilder.h : 
https://reviews.llvm.org/D91756#change-Bsibk2p32T1c for both `clang` and `opt`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86502

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

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

[Wei Mi via Phabricator via cfe-commits]

wmi added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:978-982
   Opts.DebugInfoForProfiling = Args.hasFlag(
   OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profiling, false);
+  Opts.PseudoProbeForProfiling =
+  Args.hasFlag(OPT_fpseudo_probe_for_profiling,
+   OPT_fno_pseudo_probe_for_profiling, false);

Should it emit an error if DebugInfoForProfiling and PseudoProbeForProfiling 
are enabled at the same time? I see that from the other patch related with 
pseudoprobe discriminator, these two flags are incompatible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86502

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D87974#2408119 , @jwakely wrote:

> As of a few hours ago, GCC has `__builtin_clear_padding`, see 
> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fclear_005fpadding
>  for the docs.

Great. Then I'll update this to be named `__builtin_clear_padding`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Looks fine to me, but I'm not confident enough to give an approval.




Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:733-735
+  static AnalysisKey Key;
+
+public:

This is already public


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

https://reviews.llvm.org/D91327

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

[PATCH] D91565: Guard init_priority attribute within libc++

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/init-priority-attr.cpp:26
 Two foo __attribute__((init_priority(101))) ( 5, 6 );
+#if defined(__MVS__)
+ #if defined(SYSTEM)

Rather than using the preprocessor, you can assign a prefix to be checked to 
`-verify`. e.g., `-verify=unknown` would allow you to do `// unknown-warning 
{{unknown attribute 'init_priority' ignored}}` or `unknown-no-diagnostics` that 
is only checked when `-verify=unknown`.

I wonder if it would be cleaner to use that solution here instead of the 
preprocessor by adding a new RUN line that's specific to AIX, and setting some 
non-AIX triples for the other run lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Yitzhak Mandelbaum 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 rG88e62085624e: [libTooling] Update Transformers `node` 
combinator to include the trailing… (authored by ymandel).

Changed prior to commit:
  https://reviews.llvm.org/D91872?vs=306707=306729#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91872

Files:
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/lib/Tooling/Transformer/RangeSelector.cpp
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -1353,7 +1353,7 @@
 
   // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct':
   testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField),
-changeTo(cat("safe_int ", name("theField",
+changeTo(cat("safe_int ", name("theField"), ";"))),
NonTemplatesInput + TemplatesInput,
NonTemplatesExpected + TemplatesInput);
 
@@ -1378,7 +1378,7 @@
 
   // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct':
   testRule(makeRule(traverse(TK_AsIs, MatchedField),
-changeTo(cat("safe_int ", name("theField",
+changeTo(cat("safe_int ", name("theField"), ";"))),
 
NonTemplatesInput + TemplatesInput,
NonTemplatesExpected + IncorrectTemplatesExpected);
@@ -1589,7 +1589,7 @@
Changes[0].getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
-  EXPECT_EQ(format(*UpdatedCode), format(R"cc(;)cc"));
+  EXPECT_EQ(format(*UpdatedCode), "");
 
   ASSERT_EQ(Changes[1].getFilePath(), "input.cc");
   EXPECT_THAT(Changes[1].getInsertedHeaders(), IsEmpty());
@@ -1598,8 +1598,7 @@
   Source, Changes[1].getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
-  EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h"
-;)cc"));
+  EXPECT_EQ(format(*UpdatedCode), format("#include \"input.h\"\n"));
 }
 
 TEST_F(TransformerTest, AddIncludeMultipleFiles) {
Index: clang/lib/Tooling/Transformer/RangeSelector.cpp
===
--- clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@
 Expected Node = getNode(Result.Nodes, ID);
 if (!Node)
   return Node.takeError();
-return Node->get() != nullptr && Node->get() == nullptr
+return (Node->get() != nullptr ||
+(Node->get() != nullptr && Node->get() == nullptr))
? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
*Result.Context)
: CharSourceRange::getTokenRange(Node->getSourceRange());
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon, if any (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match 
result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting
Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -148,7 +148,7 @@
   if (Options.get("Skip", "false") == "true")
 return None;
   return tooling::makeRule(clang::ast_matchers::functionDecl(),
-   change(cat("void nothing()")), cat("no message"));
+   changeTo(cat("void nothing();")), cat("no 
message"));
 }
 
 class ConfigurableCheck : public TransformerClangTidyCheck {


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -1353,7 +1353,7 @@
 
   

[clang] 88e6208 - [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-11-20T18:11:50Z
New Revision: 88e62085624e7ec55bd4a41c6d77acdcb7f1d113

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

LOG: [libTooling] Update Transformer's `node` combinator to include the 
trailing semicolon for decls.

Currently, `node` only includes the semicolon for (some) statements. However,
declarations have the same issue of (potentially) trailing semicolons, so `node`
should behave the same for them.

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

Added: 


Modified: 
clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
clang/include/clang/Tooling/Transformer/RangeSelector.h
clang/lib/Tooling/Transformer/RangeSelector.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp 
b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
index 76de7711cee9..536d6f8ef275 100644
--- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -148,7 +148,7 @@ Optional noSkip(const LangOptions ,
   if (Options.get("Skip", "false") == "true")
 return None;
   return tooling::makeRule(clang::ast_matchers::functionDecl(),
-   change(cat("void nothing()")), cat("no message"));
+   changeTo(cat("void nothing();")), cat("no 
message"));
 }
 
 class ConfigurableCheck : public TransformerClangTidyCheck {

diff  --git a/clang/include/clang/Tooling/Transformer/RangeSelector.h 
b/clang/include/clang/Tooling/Transformer/RangeSelector.h
index e070c0e7e2e6..c6ca193123d1 100644
--- a/clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ b/clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@ inline RangeSelector between(RangeSelector R1, RangeSelector 
R2) {
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon, if any (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match 
result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting

diff  --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp 
b/clang/lib/Tooling/Transformer/RangeSelector.cpp
index ce6f5fb9b444..0f3138db218a 100644
--- a/clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@ RangeSelector transformer::node(std::string ID) {
 Expected Node = getNode(Result.Nodes, ID);
 if (!Node)
   return Node.takeError();
-return Node->get() != nullptr && Node->get() == nullptr
+return (Node->get() != nullptr ||
+(Node->get() != nullptr && Node->get() == nullptr))
? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
*Result.Context)
: CharSourceRange::getTokenRange(Node->getSourceRange());

diff  --git a/clang/unittests/Tooling/TransformerTest.cpp 
b/clang/unittests/Tooling/TransformerTest.cpp
index 773420c015cd..e4fcc210782f 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -1353,7 +1353,7 @@ void instantiate()
 
   // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct':
   testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField),
-changeTo(cat("safe_int ", name("theField",
+changeTo(cat("safe_int ", name("theField"), ";"))),
NonTemplatesInput + TemplatesInput,
NonTemplatesExpected + TemplatesInput);
 
@@ -1378,7 +1378,7 @@ void instantiate()
 
   // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct':
   testRule(makeRule(traverse(TK_AsIs, MatchedField),
-changeTo(cat("safe_int ", name("theField",
+changeTo(cat("safe_int ", name("theField"), ";"))),
 
NonTemplatesInput + TemplatesInput,
NonTemplatesExpected + IncorrectTemplatesExpected);
@@ -1589,7 +1589,7 @@ TEST_F(TransformerTest, MultipleFiles) {
Changes[0].getReplacements());
   ASSERT_TRUE(static_cast(UpdatedCode))
   << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
-  EXPECT_EQ(format(*UpdatedCode), format(R"cc(;)cc"));
+  EXPECT_EQ(format(*UpdatedCode), "");
 
   ASSERT_EQ(Changes[1].getFilePath(), "input.cc");
   

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 306724.
zequanwu added a comment.
Herald added a subscriber: dexonsmith.

Add ThreadModel to LangOptions and remove it from CodegenOption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/CXX/cpp/cpp.predefined/p2.cpp
  clang/test/Preprocessor/init-aarch64.c

Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -233,6 +233,7 @@
 // AARCH64-NEXT: #define __SIZE_TYPE__ long unsigned int
 // AARCH64-NEXT: #define __SIZE_WIDTH__ 64
 // AARCH64_CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// AARCH64_CXX: #define __STDCPP_THREADS__ 1
 // AARCH64-NEXT: #define __STDC_HOSTED__ 1
 // AARCH64-NEXT: #define __STDC_UTF_16__ 1
 // AARCH64-NEXT: #define __STDC_UTF_32__ 1
Index: clang/test/CXX/cpp/cpp.predefined/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/cpp/cpp.predefined/p2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -verify
+// expected-no-diagnostics
+
+#ifndef __STDCPP_THREADS__
+#error __STDCPP_THREADS__ is not defined
+#endif
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -403,6 +403,12 @@
 Builder.defineMacro("__STDCPP_DEFAULT_NEW_ALIGNMENT__",
 Twine(TI.getNewAlign() / TI.getCharWidth()) +
 TI.getTypeConstantSuffix(TI.getSizeType()));
+
+//   -- __STDCPP_­THREADS__
+//  Defined, and has the value integer literal 1, if and only if a
+//  program can have more than one thread of execution.
+if (LangOpts.getThreadModel() == LangOptions::ThreadModelKind::POSIX)
+  Builder.defineMacro("__STDCPP_THREADS__", "1");
   }
 
   // In C11 these are environment macros. In C++11 they are only defined
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1037,12 +1037,11 @@
   Opts.StrictVTablePointers = Args.hasArg(OPT_fstrict_vtable_pointers);
   Opts.ForceEmitVTables = Args.hasArg(OPT_fforce_emit_vtables);
   Opts.UnwindTables = Args.hasArg(OPT_munwind_tables);
-  Opts.ThreadModel =
+  std::string ThreadModel =
   std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
-  if (Opts.ThreadModel != "posix" && Opts.ThreadModel != "single")
+  if (ThreadModel != "posix" && ThreadModel != "single")
 Diags.Report(diag::err_drv_invalid_value)
-<< Args.getLastArg(OPT_mthread_model)->getAsString(Args)
-<< Opts.ThreadModel;
+<< Args.getLastArg(OPT_mthread_model)->getAsString(Args) << ThreadModel;
   Opts.TrapFuncName = std::string(Args.getLastArgValue(OPT_ftrap_function_EQ));
   Opts.UseInitArray = !Args.hasArg(OPT_fno_use_init_array);
 
Index: clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -49,7 +49,7 @@
   const PreprocessorOptions 
   CodeGenOptions CodeGenOpts;
   const TargetOptions TargetOpts;
-  const LangOptions LangOpts;
+  LangOptions LangOpts;
   std::unique_ptr VMContext;
   std::unique_ptr M;
   std::unique_ptr Builder;
@@ -147,7 +147,7 @@
 // The debug info output isn't affected by CodeModel and
 // ThreadModel, but the backend expects them to be nonempty.
 CodeGenOpts.CodeModel = "default";
-CodeGenOpts.ThreadModel = "single";
+LangOpts.setThreadModel(LangOptions::ThreadModelKind::Single);
 CodeGenOpts.DebugTypeExtRefs = true;
 // When building a module MainFileName is the name of the modulemap file.
 CodeGenOpts.MainFileName =
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -453,10 +453,14 @@
   const clang::TargetOptions ,
   const LangOptions ,
   const HeaderSearchOptions ) {
-  Options.ThreadModel =
-  llvm::StringSwitch(CodeGenOpts.ThreadModel)
-  .Case("posix", llvm::ThreadModel::POSIX)
-  .Case("single", llvm::ThreadModel::Single);
+  switch 

[PATCH] D91373: [OpenMP5.0] Support more kinds of lvalues in map clauses

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D91373#2408175 , @jacobdweightman 
wrote:

> Ping



1. Provide full diff, see https://www.llvm.org/docs/Phabricator.html for the 
guidance.
2. Need some more test with ast printing and codegen.
3. Would be good if you could run (at least locally) some tests to check that 
the changes really work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91373

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

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:127-128
 getFileStyleFromOptions(const ClangTidyCheck::OptionsView ) {
   SmallVector, 0> Styles(
   SK_Count);
   SmallString<64> StyleString;

Making this change removes the need to NamingStyle to be copy 
constructable/assignable.



Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:61
+  ? llvm::Regex()
+  : llvm::Regex("^" + IgnoredRegexpStr + "$");
+}

Potentially save an allocation.

Also is it wise to warn a user on an invalid regex, unfortunately clang-tidy 
doesnt support diags with no source location but could still output to 
llvm::errs.


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

https://reviews.llvm.org/D90282

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

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added inline comments.



Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:589
   sort(FrameData.Allocas, [&](const auto , const auto ) {
-return GetAllocaSize(Iter1) > GetAllocaSize(Iter2);
+return TypeSize::isKnownGT(GetAllocaSize(Iter1), GetAllocaSize(Iter2));
   });

We don't need to check whether the scalable flag matches here because we are 
sorting a container that (potentially) have both types of vectors.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

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

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 306715.
fpetrogalli edited the summary of this revision.
fpetrogalli added a comment.

I have added assertions around before `TypeSize` comparisons where it made 
sense to do so, but not in the lambda used in the `sort` invocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

Files:
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  
llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll

Index: llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
@@ -0,0 +1,42 @@
+; RUN: opt -mtriple aarch64-gnu-linux -mattr=+sve -instcombine -S < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; If this check fails please read
+; clang/test/CodeGen/aarch64-sve-intrinsics/README for instructions on
+; how to resolve it.
+
+; WARN-NOT: warning
+
+; CHECK-LABEL: @debug_local_scalable(
+define  @debug_local_scalable( %tostore) {
+  %vx = alloca , align 16
+  call void @llvm.dbg.declare(metadata * %vx, metadata !5, metadata !DIExpression()), !dbg !15
+  store  %tostore, * %vx, align 16
+  %ret = call  @f(* %vx)
+  ret  %ret
+}
+
+declare  @f(*)
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "/tmp/test.c", directory: "/tmp/")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !DILocalVariable(name: "vx", scope: !6, file: !7, line: 26, type: !8)
+!6 = distinct !DISubprogram(name: "debug_local_scalable", scope: null, file: !1, line: 25, scopeLine: 25, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!7 = !DIFile(filename: "test.c", directory: "/tmp/")
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "svfloat64_t", file: !9, line: 56, baseType: !10)
+!9 = !DIFile(filename: "arm_sve.h", directory: "/tmp/")
+!10 = !DIDerivedType(tag: DW_TAG_typedef, name: "__SVFloat64_t", file: !1, baseType: !11)
+!11 = !DICompositeType(tag: DW_TAG_array_type, baseType: !12, flags: DIFlagVector, elements: !13)
+!12 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float)
+!13 = !{!14}
+!14 = !DISubrange(lowerBound: 0, upperBound: !DIExpression(DW_OP_constu, 1, DW_OP_bregx, 46, 0, DW_OP_mul, DW_OP_constu, 1, DW_OP_minus))
+!15 = !DILocation(line: 26, column: 15, scope: !6)
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1368,16 +1368,22 @@
 /// least n bits.
 static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   const DataLayout  = DII->getModule()->getDataLayout();
-  uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy);
-  if (auto FragmentSize = DII->getFragmentSizeInBits())
-return ValueSize >= *FragmentSize;
+  TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy);
+  if (Optional FragmentSize = DII->getFragmentSizeInBits()) {
+assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
+   "Both sizes should agree on the scalable flag.");
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+  }
   // We can't always calculate the size of the DI variable (e.g. if it is a
   // VLA). Try to use the size of the alloca that the dbg intrinsic describes
   // intead.
   if (DII->isAddressOfVariable())
 if (auto *AI = dyn_cast_or_null(DII->getVariableLocation()))
-  if (auto FragmentSize = AI->getAllocationSizeInBits(DL))
-return ValueSize >= *FragmentSize;
+  if (Optional FragmentSize = AI->getAllocationSizeInBits(DL)) {
+assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
+   "Both sizes should agree on the scalable flag.");
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+  }
   // Could not determine size of variable. Conservatively return false.
   return false;
 }
Index: llvm/lib/Transforms/Utils/Debugify.cpp
===
--- llvm/lib/Transforms/Utils/Debugify.cpp
+++ llvm/lib/Transforms/Utils/Debugify.cpp
@@ -44,8 

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

From a ThinLTO perspective, no specific concerns as the 
buildModuleSimplificationPipeline is invoked in both the pre and post LTO link 
pipelines, so they both get an equivalent change. But there is an issue for 
regular LTO, noted below.




Comment at: llvm/test/Other/new-pm-lto-defaults.ll:70
 ; CHECK-O2-NEXT: Starting llvm::Module pass manager run.
-; CHECK-O2-NEXT: Running pass: AlwaysInlinerPass
 ; CHECK-O2-NEXT: Starting CGSCC pass manager run.

Note there is no corresponding add of an additional InlinerPass like in the 
other files. The reason is that PassBuilder::buildLTODefaultPipeline doesn't 
invoke buildModuleSimplificationPipeline, or even buildInlinerPipeline (it has 
a separate pipeline setup for compile time reasons due to the monolithic nature 
of the post-LTO link compilation), but rather directly adds 
ModuleInlinerWrapperPass. So you'll want to add the additional 
ModuleInlinerWrapperPass invocation there as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91567

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Frontend/noderef.c:71
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access

Can you add tests for the weird situations where the expression actually is 
evaluated? e.g.,
```
struct S {
  virtual ~S(); // Make S polymorphic
};

S NODEREF *s;
typeid(*s); // Actually evaluates *s at runtime

struct T {
  unsigned i;
};

T t1;
T NODEREF *t2 = 

sizeof(int[++t2->i]); // Actually evaluates t2->i at runtime
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D91872#2408321 , @ymandel wrote:

> In D91872#2408278 , @aaron.ballman 
> wrote:
>
>> Drive-by question from the peanut gallery, sorry if this is an ignorant one 
>> -- not all declarations have a trailing semicolon; is that handled properly? 
>> e.g., `int x;` has a trailing semicolon but `int x, y;` only has a trailing 
>> semicolon for one of the two declarations. Relatedly, in `int f(int x);`, 
>> the declaration of `f` has a trailing semicolon, but the declaration of `x` 
>> does not.
>
> No, it's a good question -- the comments and the patch description should 
> have been clearer.  I've updated both. Also, I found a test that needed to be 
> fixed (and also, hopefully, illustrates why the new behavior is preferred. 
> The old behavior left the semicolon out of the replacement (which looks 
> weird) because it was working around the fact that the source being removed 
> didn't include the trailing semicolon, while the new version can just specify 
> the replacement correctly).
>
> Thanks!

Thanks, this is more clear now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91872

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D91872#2408278 , @aaron.ballman 
wrote:

> Drive-by question from the peanut gallery, sorry if this is an ignorant one 
> -- not all declarations have a trailing semicolon; is that handled properly? 
> e.g., `int x;` has a trailing semicolon but `int x, y;` only has a trailing 
> semicolon for one of the two declarations. Relatedly, in `int f(int x);`, the 
> declaration of `f` has a trailing semicolon, but the declaration of `x` does 
> not.

No, it's a good question -- the comments and the patch description should have 
been clearer.  I've updated both. Also, I found a test that needed to be fixed 
(and also, hopefully, illustrates why the new behavior is preferred. The old 
behavior left the semicolon out of the replacement (which looks weird) because 
it was working around the fact that the source being removed didn't include the 
trailing semicolon, while the new version can just specify the replacement 
correctly).

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91872

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 306707.
ymandel added a comment.

Clarified that semicolons are only removed if present. Fixed test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91872

Files:
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/lib/Tooling/Transformer/RangeSelector.cpp


Index: clang/lib/Tooling/Transformer/RangeSelector.cpp
===
--- clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@
 Expected Node = getNode(Result.Nodes, ID);
 if (!Node)
   return Node.takeError();
-return Node->get() != nullptr && Node->get() == nullptr
+return (Node->get() != nullptr ||
+(Node->get() != nullptr && Node->get() == nullptr))
? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
*Result.Context)
: CharSourceRange::getTokenRange(Node->getSourceRange());
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon, if any (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match 
result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting
Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -148,7 +148,7 @@
   if (Options.get("Skip", "false") == "true")
 return None;
   return tooling::makeRule(clang::ast_matchers::functionDecl(),
-   change(cat("void nothing()")), cat("no message"));
+   changeTo(cat("void nothing();")), cat("no 
message"));
 }
 
 class ConfigurableCheck : public TransformerClangTidyCheck {


Index: clang/lib/Tooling/Transformer/RangeSelector.cpp
===
--- clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@
 Expected Node = getNode(Result.Nodes, ID);
 if (!Node)
   return Node.takeError();
-return Node->get() != nullptr && Node->get() == nullptr
+return (Node->get() != nullptr ||
+(Node->get() != nullptr && Node->get() == nullptr))
? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
*Result.Context)
: CharSourceRange::getTokenRange(Node->getSourceRange());
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon, if any (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting
Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -148,7 +148,7 @@
   if (Options.get("Skip", "false") == "true")
 return None;
   return tooling::makeRule(clang::ast_matchers::functionDecl(),
-   change(cat("void nothing()")), cat("no message"));
+   changeTo(cat("void nothing();")), cat("no message"));
 }
 
 class ConfigurableCheck : public TransformerClangTidyCheck {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This was caught with the GNUstep runtime's test suite, which apparently had not 
been run with anything newer than clang 8 until recently.  With this patch, all 
of the runtime's tests now pass again (a few others failed in 10 but appear to 
have been fixed in 11 or 12).  We've now configured our CI to test with the 
nightly builds on Linux, so should catch these things sooner in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91874

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

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

[David Chisnall via Phabricator via cfe-commits]

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

Methods synthesized from declared properties were being added to the
method lists twice.  This came from the change to list them in the
class's method list, which missed removing the place in CGObjCGNU that
added them again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91874

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/gnu-method-only-once.m


Index: clang/test/CodeGenObjC/gnu-method-only-once.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnu-method-only-once.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm 
-fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm 
-fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+
+// Clang 9 or 10 changed the handling of method lists so that methods provided
+// from synthesised properties showed up in the method list, where previously
+// CGObjCGNU had to collect them and merge them.  One of the places where this
+// merging happened was missed in the move and so we ended up emitting two
+// copies of method metadata for everything that appeared in the 
+
+// This class has only instance properties and only one pair of synthesized
+// methods from the property and so we should synthesize only one method list,
+// with precisely two methods on it.
+@interface X
+@property (retain) id iProp;
+@end
+
+@implementation X
+@synthesize iProp;
+@end
+
+// Check that the method list has precisely 2 methods.
+// CHECK-NEW: @.objc_method_list = internal global { i8*, i32, i64, [2 x
+// CHECK-OLD: @.objc_method_list = internal global { i8*, i32, [2 x
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3512,19 +3512,6 @@
   ClassMethods.insert(ClassMethods.begin(), OID->classmeth_begin(),
   OID->classmeth_end());
 
-  // Collect the same information about synthesized properties, which don't
-  // show up in the instance method lists.
-  for (auto *propertyImpl : OID->property_impls())
-if (propertyImpl->getPropertyImplementation() ==
-ObjCPropertyImplDecl::Synthesize) {
-  auto addPropertyMethod = [&](const ObjCMethodDecl *accessor) {
-if (accessor)
-  InstanceMethods.push_back(accessor);
-  };
-  addPropertyMethod(propertyImpl->getGetterMethodDecl());
-  addPropertyMethod(propertyImpl->getSetterMethodDecl());
-}
-
   llvm::Constant *Properties = GeneratePropertyList(OID, ClassDecl);
 
   // Collect the names of referenced protocols


Index: clang/test/CodeGenObjC/gnu-method-only-once.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnu-method-only-once.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+
+// Clang 9 or 10 changed the handling of method lists so that methods provided
+// from synthesised properties showed up in the method list, where previously
+// CGObjCGNU had to collect them and merge them.  One of the places where this
+// merging happened was missed in the move and so we ended up emitting two
+// copies of method metadata for everything that appeared in the 
+
+// This class has only instance properties and only one pair of synthesized
+// methods from the property and so we should synthesize only one method list,
+// with precisely two methods on it.
+@interface X
+@property (retain) id iProp;
+@end
+
+@implementation X
+@synthesize iProp;
+@end
+
+// Check that the method list has precisely 2 methods.
+// CHECK-NEW: @.objc_method_list = internal global { i8*, i32, i64, [2 x
+// CHECK-OLD: @.objc_method_list = internal global { i8*, i32, [2 x
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3512,19 +3512,6 @@
   ClassMethods.insert(ClassMethods.begin(), OID->classmeth_begin(),
   OID->classmeth_end());
 
-  // Collect the same information about synthesized properties, which don't
-  // show up in the instance method lists.
-  for (auto *propertyImpl : OID->property_impls())
-if (propertyImpl->getPropertyImplementation() ==
-ObjCPropertyImplDecl::Synthesize) {
-  auto addPropertyMethod = [&](const ObjCMethodDecl *accessor) {
-if (accessor)
-  InstanceMethods.push_back(accessor);
-  };
-  

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

[Francesco Petrogalli via Phabricator via cfe-commits]

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

Thank you for the review @sdesmalen

1. I have replied to the comment about `auto` with my reason for removing it.
2. I removed the C test, as the LL file is enough.
3. The third comment requires more investigation, I'll get back to you to see 
the use of the `TypeSize` comparisons.

Thank you!




Comment at: llvm/lib/IR/IntrinsicInst.cpp:56
+Optional DbgVariableIntrinsic::getFragmentSizeInBits() const {
+  if (Optional Fragment =
+  getExpression()->getFragmentInfo())

sdesmalen wrote:
> Change back to `auto` ?
I'd like to keep this, because it seems to me the use of auto in this case 
didn't fit in the cases mentioned in 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable.
 There is no direct type deduction from the context around the initialization 
of the variable `Fragment`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Drive-by question from the peanut gallery, sorry if this is an ignorant one -- 
not all declarations have a trailing semicolon; is that handled properly? e.g., 
`int x;` has a trailing semicolon but `int x, y;` only has a trailing semicolon 
for one of the two declarations. Relatedly, in `int f(int x);`, the declaration 
of `f` has a trailing semicolon, but the declaration of `x` does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91872

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

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[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.

LGTM, thank you for the new check!


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

https://reviews.llvm.org/D89651

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: tdl-g.
Herald added a project: clang.
ymandel requested review of this revision.

Currently, `node` only includes the semicolon for (some) statements. However,
declarations have the same issue of trailing semicolons, so `node` should behave
the same for them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91872

Files:
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/lib/Tooling/Transformer/RangeSelector.cpp


Index: clang/lib/Tooling/Transformer/RangeSelector.cpp
===
--- clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@
 Expected Node = getNode(Result.Nodes, ID);
 if (!Node)
   return Node.takeError();
-return Node->get() != nullptr && Node->get() == nullptr
+return (Node->get() != nullptr ||
+(Node->get() != nullptr && Node->get() == nullptr))
? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
*Result.Context)
: CharSourceRange::getTokenRange(Node->getSourceRange());
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match 
result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting


Index: clang/lib/Tooling/Transformer/RangeSelector.cpp
===
--- clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@
 Expected Node = getNode(Result.Nodes, ID);
 if (!Node)
   return Node.takeError();
-return Node->get() != nullptr && Node->get() == nullptr
+return (Node->get() != nullptr ||
+(Node->get() != nullptr && Node->get() == nullptr))
? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
*Result.Context)
: CharSourceRange::getTokenRange(Node->getSourceRange());
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90928: [OpenCL] Check for extension string extension lookup

[Erik Tomusk via Phabricator via cfe-commits]

erik2020 added a comment.

In D90928#2405796 , @Anastasia wrote:

> Do you think we could improve testing? I presume there is something that 
> triggers a failure without your change...

I'm not really sure how to test this code. Best I can tell, there's no way for 
the `clang` executable to call these functions with invalid strings. I only ran 
into the seg faults when I was programmatically setting options using the clang 
API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90928

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

[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 306696.
ZarkoCA marked an inline comment as done.
ZarkoCA added a comment.

Addressed comments and added a test to check whether the driver passes these 
options.


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

https://reviews.llvm.org/D89684

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/altivec.c
  clang/test/CodeGen/ppc64-vector.c
  clang/test/Driver/aix-vec-extabi.c
  clang/test/Preprocessor/aix-vec_extabi.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/aix-vec-abi.ll

Index: llvm/test/CodeGen/PowerPC/aix-vec-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-vec-abi.ll
@@ -0,0 +1,12 @@
+; RUN: not --crash llc < %s -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr8 2>&1 | FileCheck %s --check-prefix=DFLTERROR
+; RUN: not --crash llc < %s -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr8 2>&1 | FileCheck %s --check-prefix=DFLTERROR
+
+; RUN: not --crash llc < %s -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr8 -vec-extabi 2>&1 | FileCheck %s --check-prefix=VEXTERROR
+; RUN: not --crash llc < %s -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr8 -vec-extabi 2>&1 | FileCheck %s --check-prefix=VEXTERROR
+
+define void @vec_callee(<4 x i32> %vec1) {
+ret void 
+}
+
+; DFLTERROR:  LLVM ERROR: the default Altivec AIX ABI is not yet supported
+; VEXTERROR:  LLVM ERROR: the extended Altivec AIX ABI is not yet supported
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -6968,6 +6968,16 @@
   const Align PtrAlign = IsPPC64 ? Align(8) : Align(4);
   const MVT RegVT = IsPPC64 ? MVT::i64 : MVT::i32;
 
+  if (ValVT.isVector() && !State.getMachineFunction()
+   .getTarget()
+   .Options.EnableAIXExtendedAltivecABI)
+report_fatal_error("the default Altivec AIX ABI is not yet supported");
+
+  if (ValVT.isVector() && State.getMachineFunction()
+  .getTarget()
+  .Options.EnableAIXExtendedAltivecABI)
+report_fatal_error("the extended Altivec AIX ABI is not yet supported");
+
   assert((!ValVT.isInteger() ||
   (ValVT.getFixedSizeInBits() <= RegVT.getFixedSizeInBits())) &&
  "Integer argument exceeds register size: should have been legalized");
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -58,6 +58,7 @@
 CGOPT(bool, EnableNoNaNsFPMath)
 CGOPT(bool, EnableNoSignedZerosFPMath)
 CGOPT(bool, EnableNoTrappingFPMath)
+CGOPT(bool, EnableAIXExtendedAltivecABI)
 CGOPT(DenormalMode::DenormalModeKind, DenormalFPMath)
 CGOPT(DenormalMode::DenormalModeKind, DenormalFP32Math)
 CGOPT(bool, EnableHonorSignDependentRoundingFPMath)
@@ -282,6 +283,11 @@
   cl::init(false));
   CGBINDOPT(DontPlaceZerosInBSS);
 
+  static cl::opt EnableAIXExtendedAltivecABI(
+  "vec-extabi", cl::desc("Enable the AIX Extended Altivec ABI."),
+  cl::init(false));
+  CGBINDOPT(EnableAIXExtendedAltivecABI);
+
   static cl::opt EnableGuaranteedTailCallOpt(
   "tailcallopt",
   cl::desc(
@@ -516,6 +522,7 @@
   getEnableHonorSignDependentRoundingFPMath();
   if (getFloatABIForCalls() != FloatABI::Default)
 Options.FloatABIType = getFloatABIForCalls();
+  Options.EnableAIXExtendedAltivecABI = getEnableAIXExtendedAltivecABI();
   Options.NoZerosInBSS = getDontPlaceZerosInBSS();
   Options.GuaranteedTailCallOpt = getEnableGuaranteedTailCallOpt();
   Options.StackAlignmentOverride = getOverrideStackAlignment();
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -124,6 +124,7 @@
 TargetOptions()
 : UnsafeFPMath(false), NoInfsFPMath(false), NoNaNsFPMath(false),
   NoTrappingFPMath(true), NoSignedZerosFPMath(false),
+  EnableAIXExtendedAltivecABI(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
   EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
@@ -175,6 +176,12 @@
 /// 

[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

[Zarko Todorovski via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1443
 
+  if (Arg *A =
+  Args.getLastArg(OPT_mabi_EQ_vec_default, OPT_mabi_EQ_vec_extabi)) {

Xiangling_L wrote:
> Should we also check if target feature altivec[`-target-feature +altivec`] is 
> enabled when using these two options? If so, we should also add related 
> testcases.
Both of these options require that -maltivec is also selected which sets 
`-target-feature +altivec`.



Comment at: clang/test/CodeGen/altivec.c:2
 // RUN: %clang_cc1 -target-feature +altivec -triple powerpc-unknown-unknown 
-emit-llvm %s -o - | FileCheck %s
-
+// RUN: %clang_cc1 -target-feature +altivec -mabi=vec-extabi -triple 
powerpc-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -target-feature +altivec -mabi=vec-extabi -triple 
powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s

Xiangling_L wrote:
> ZarkoCA wrote:
> > Xiangling_L wrote:
> > > Based on the code added in `BackendUtil:551`, should we also add a case 
> > > for compiling a source to assembly?
> > Added in lines 15-18
> > Added in lines 15-18
> 
> Sorry, I should make my point clearer. Based on current testcases, there are 
> two things:
> 
> 1. line 15-18 are actually duplication to 10,11, 13. 14. Because all of them 
> are testing if the driver will emit error when not specifying -maltivec with 
> -mabi=vec-default/-mabi=vec-extabi, i.e compiling from .c to .ll and .c to .s 
> won't affect how driver works,
> 
> 2. `BackendUtil:551` The code I mentioned is actually affecting how BE 
> behaves when we enable AIX altivec in the FE[or driver]. So the testcase I am 
> looking for is something like:
> 
> ```
> // RUN:  %clang -target powerpc-unknown-aix -S -maltivec -mabi=vec-extabi %s  
> | FileCheck  %s
> // CHECK: LLVM ERROR: the extended Altivec AIX ABI is not yet supported
> ```
As far as I understand, testing the assembly path is a bit tricky mainly due to 
how Altivec is determined to be on for all powerpc targets.  

The tests in this file won't trigger the Altivec ABI errors because they are 
calling convention ABIs and there is no parameter passing or returns.  In fact, 
they will generate assembly because the default CPU has the Altivec attribute 
enabled.

I wrote tests that will use the Altivec calling convention ABI in those cases 
we  trigger earlier errors such as "vector type is unimplemented on AIX". 

But, I did a test which shows that the driver passes the `-mabi=vec-extabi` 
option. 



Comment at: llvm/test/CodeGen/PowerPC/aix-func-dsc-gen.ll:2
-; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mtriple powerpc-ibm-aix-xcoff 
-filetype=obj -o %t.o < %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=-altivec -mtriple 
powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
 ; RUN: llvm-readobj  --symbols %t.o | FileCheck %s
 

Xiangling_L wrote:
> I am not sure if this is for all testcases where you add `-mattr=-altivec`, 
> but I tried the first three. They all passed without this option. Could you 
> double check this?
You're right,it doesn't look like it's needed any longer. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684

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

[PATCH] D84345: [AMDGPU] Set the default globals address space to 1

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG51e09e1d5aa4: [AMDGPU] Set the default globals address space 
to 1 (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D84345?vs=289235=306693#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84345

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTT.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp

Index: llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
===
--- llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
+++ llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
@@ -27,6 +27,10 @@
  "-f80:32-n8:16:32-S32");
   EXPECT_EQ(DL3, "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128"
  "-n32:64-S128");
+
+  // Check that AMDGPU targets add -G1 if it's not present.
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "r600"), "e-p:32:32-G1");
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64", "amdgcn"), "e-p:64:64-G1");
 }
 
 TEST(DataLayoutUpgradeTest, NoDataLayoutUpgrade) {
@@ -46,6 +50,13 @@
   EXPECT_EQ(DL2, "e-p:32:32");
   EXPECT_EQ(DL3, "e-m:e-i64:64-n32:64");
   EXPECT_EQ(DL4, "e-m:o-i64:64-i128:128-n32:64-S128");
+
+  // Check that AMDGPU targets don't add -G1 if there is already a -G flag.
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G2", "r600"), "e-p:32:32-G2");
+  EXPECT_EQ(UpgradeDataLayoutString("G2", "r600"), "G2");
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G2", "amdgcn"), "e-p:64:64-G2");
+  EXPECT_EQ(UpgradeDataLayoutString("G2-e-p:64:64", "amdgcn"), "G2-e-p:64:64");
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G0", "amdgcn"), "e-p:64:64-G0");
 }
 
 TEST(DataLayoutUpgradeTest, EmptyDataLayout) {
@@ -54,6 +65,10 @@
   "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128", "");
   EXPECT_EQ(DL1, "");
   EXPECT_EQ(DL2, "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128");
+
+  // Check that AMDGPU targets add G1 if it's not present.
+  EXPECT_EQ(UpgradeDataLayoutString("", "r600"), "G1");
+  EXPECT_EQ(UpgradeDataLayoutString("", "amdgcn"), "G1");
 }
 
 } // end namespace
Index: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -342,15 +342,15 @@
 static StringRef computeDataLayout(const Triple ) {
   if (TT.getArch() == Triple::r600) {
 // 32-bit pointers.
-  return "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
- "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5";
+return "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
+   "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1";
   }
 
   // 32-bit private, local, and region pointers. 64-bit global, constant and
   // flat, non-integral buffer fat pointers.
-return "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
+  return "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
  "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
- "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
+ "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1"
  "-ni:7";
 }
 
Index: llvm/lib/IR/AutoUpgrade.cpp
===
--- llvm/lib/IR/AutoUpgrade.cpp
+++ llvm/lib/IR/AutoUpgrade.cpp
@@ -4380,11 +4380,17 @@
 }
 
 std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
-  StringRef AddrSpaces = "-p270:32:32-p271:32:32-p272:64:64";
+  Triple T(TT);
+  // For AMDGPU we uprgrade older DataLayouts to include the default globals
+  // address space of 1.
+  if (T.isAMDGPU() && !DL.contains("-G") && !DL.startswith("G")) {
+return DL.empty() ? std::string("G1") : (DL + "-G1").str();
+  }
 
+  std::string AddrSpaces = "-p270:32:32-p271:32:32-p272:64:64";
   // If X86, and the datalayout matches the expected format, add pointer size
   // address spaces to the datalayout.
-  if (!Triple(TT).isX86() || DL.contains(AddrSpaces))
+  if (!T.isX86() || DL.contains(AddrSpaces))
 return std::string(DL);
 
   SmallVector Groups;
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -215,7 +215,7 @@
 GV->setAlignment(Align(8));
 Ident = GV;
   }
-  return Ident;
+  return 

[clang] 51e09e1 - [AMDGPU] Set the default globals address space to 1

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-11-20T15:46:53Z
New Revision: 51e09e1d5aa43296cf8baf26a74793fd86b0b0d2

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

LOG: [AMDGPU] Set the default globals address space to 1

This will ensure that passes that add new global variables will create them
in address space 1 once the passes have been updated to no longer default
to the implicit address space zero.
This also changes AutoUpgrade.cpp to add -G1 to the DataLayout if it wasn't
already to present to ensure bitcode backwards compatibility.

Reviewed by: arsenm

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

Added: 


Modified: 
clang/lib/Basic/Targets/AMDGPU.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGVTT.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGen/target-data.c
clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/IR/AutoUpgrade.cpp
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AMDGPU.cpp 
b/clang/lib/Basic/Targets/AMDGPU.cpp
index 9b88dff7c4af..91c1e83f61cb 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -31,12 +31,12 @@ namespace targets {
 
 static const char *const DataLayoutStringR600 =
 "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
-"-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5";
+"-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1";
 
 static const char *const DataLayoutStringAMDGCN =
 "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
 "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
-"-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
+"-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1"
 "-ni:7";
 
 const LangASMap AMDGPUTargetInfo::AMDGPUDefIsGenMap = {

diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index c41650ad2b3b..5bd484eb1464 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -2508,12 +2508,16 @@ void CodeGenFunction::InitializeVTablePointer(const 
VPtr ) {
 
   // Finally, store the address point. Use the same LLVM types as the field to
   // support optimization.
+  unsigned GlobalsAS = CGM.getDataLayout().getDefaultGlobalsAddressSpace();
+  unsigned ProgAS = CGM.getDataLayout().getProgramAddressSpace();
   llvm::Type *VTablePtrTy =
   llvm::FunctionType::get(CGM.Int32Ty, /*isVarArg=*/true)
-  ->getPointerTo()
-  ->getPointerTo();
-  VTableField = Builder.CreateBitCast(VTableField, 
VTablePtrTy->getPointerTo());
-  VTableAddressPoint = Builder.CreateBitCast(VTableAddressPoint, VTablePtrTy);
+  ->getPointerTo(ProgAS)
+  ->getPointerTo(GlobalsAS);
+  VTableField = Builder.CreatePointerBitCastOrAddrSpaceCast(
+  VTableField, VTablePtrTy->getPointerTo(GlobalsAS));
+  VTableAddressPoint = Builder.CreatePointerBitCastOrAddrSpaceCast(
+  VTableAddressPoint, VTablePtrTy);
 
   llvm::StoreInst *Store = Builder.CreateStore(VTableAddressPoint, 
VTableField);
   TBAAAccessInfo TBAAInfo = CGM.getTBAAVTablePtrAccessInfo(VTablePtrTy);

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index ced45eca4ba6..84733369a4a1 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3075,11 +3075,12 @@ void CGOpenMPRuntime::createOffloadEntry(
   llvm::GlobalValue::InternalLinkage, StrPtrInit, StringName);
   Str->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 
-  llvm::Constant *Data[] = {llvm::ConstantExpr::getBitCast(ID, CGM.VoidPtrTy),
-llvm::ConstantExpr::getBitCast(Str, CGM.Int8PtrTy),
-llvm::ConstantInt::get(CGM.SizeTy, Size),
-llvm::ConstantInt::get(CGM.Int32Ty, Flags),
-llvm::ConstantInt::get(CGM.Int32Ty, 0)};
+  llvm::Constant *Data[] = {
+  llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(ID, CGM.VoidPtrTy),
+  llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(Str, CGM.Int8PtrTy),
+  llvm::ConstantInt::get(CGM.SizeTy, Size),
+  llvm::ConstantInt::get(CGM.Int32Ty, Flags),
+  llvm::ConstantInt::get(CGM.Int32Ty, 0)};
   std::string EntryName = getName({"omp_offloading", "entry", ""});
   llvm::GlobalVariable *Entry = createGlobalStruct(
   CGM, getTgtOffloadEntryQTy(), /*IsConstant=*/true, Data,

diff  --git a/clang/lib/CodeGen/CGVTT.cpp b/clang/lib/CodeGen/CGVTT.cpp
index e79f3f3dd8bc..564d9f354e64 100644
--- 

[PATCH] D91373: [OpenMP5.0] Support more kinds of lvalues in map clauses

[Jacob Weightman via Phabricator via cfe-commits]

jacobdweightman added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91373

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

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis added a comment.

In D75229#2406758 , @jfb wrote:

> Most of the tests as written should be failing right now, at least on macOS 
> and Linux, because they likely should be identified as POSIX, right?

Yes, now it fails on my system too. What would be the solution?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D75229

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

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis updated this revision to Diff 306690.
abelkocsis added a comment.

Fix, `MacroName == "__unix__"` seems really necessary. Without that, it throws 
a warning for me on Linux.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D75229

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-in-multithreaded-program.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-con37-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std_thread.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std_thread.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.c
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) { return 0; };
+enum {
+  thrd_success = 0,
+};
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+int func(void *data) {
+  while (!flag) {
+  }
+  return 0;
+}
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+  thrd_t tid;
+
+  if (thrd_success != thrd_create(, func, NULL)) {
+  }
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std_thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std_thread.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+void threadFunction() {}
+
+namespace std {
+class thread {
+public:
+  thread() noexcept;
+  template 
+  explicit thread(Function &, Args &&... args);
+  thread(const thread &) = delete;
+  thread(thread &&) noexcept;
+};
+} // namespace std
+
+int main() {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+
+  std::thread threadObj(threadFunction);
+
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.c
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: bugprone-signal-in-multithreaded-program.ThreadList, value: "thrd_create"}]}'
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) { return 0; };
+enum {
+  thrd_success = 0,
+};
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+int func(void *data) {
+  while (!flag) {
+  }
+  return 0;
+}
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+  thrd_t tid;
+
+  if (thrd_success != thrd_create(, func, NULL)) {
+  }
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std_thread.cpp

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:1756
 /// a function.
-enum FunctionDefinitionKind {
-  FDK_Declaration,
-  FDK_Definition,
-  FDK_Defaulted,
-  FDK_Deleted
+enum class FunctionDefinitionKind : unsigned char {
+  Declaration,

We don't gain a whole lot by making this `unsigned char` since we're not 
storing it anywhere -- leave as the default `int` and change the 
`static_cast<>`s?



Comment at: clang/lib/Sema/SemaDecl.cpp:9163
 switch (D.getFunctionDefinitionKind()) {
-  case FDK_Declaration:
-  case FDK_Definition:
+  case FunctionDefinitionKind::Declaration:
+  case FunctionDefinitionKind::Definition:

Might as well hit these formatting fixes since we're touching the lines anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Shane via Phabricator via cfe-commits]

smhc updated this revision to Diff 306687.
smhc added a comment.

removed notes on short length threshold


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

https://reviews.llvm.org/D90282

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignore-regexp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignore-regexp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignore-regexp.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: "^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: "^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: "sooo|so|soo|$o["} \
+// RUN:  ]}'
+
+int testFunc(int a, char **b);
+int testFunc(int ab, char **ba);
+int testFunc(int abc, char **cba);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'abc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'cba'
+// CHECK-FIXES: {{^}}int testFunc(int Abc, char **Cba);{{$}}
+int testFunc(int Abc, char **Cba);
+
+class fo {
+};
+
+class fofo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'fofo'
+  // CHECK-FIXES: {{^}}class Fofo {{{$}}
+};
+
+class foo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'foo'
+  // CHECK-FIXES: {{^}}class Foo {{{$}}
+};
+
+class fooo {
+};
+
+class afooo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'afooo'
+  // CHECK-FIXES: {{^}}class Afooo {{{$}}
+};
+
+struct soo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'soo'
+  // CHECK-FIXES: {{^}}struct Soo {{{$}}
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -35,60 +35,60 @@
 
 The following options are describe below:
 
- - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`
+ - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`, :option:`AbstractClassIgnoredRegexp`
  - :option:`AggressiveDependentMemberLookup`
- - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`
- - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`
- - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`
- - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`
- - :option:`ConstantCase`, :option:`ConstantPrefix`, :option:`ConstantSuffix`
- - :option:`ConstantMemberCase`, :option:`ConstantMemberPrefix`, :option:`ConstantMemberSuffix`
- - :option:`ConstantParameterCase`, :option:`ConstantParameterPrefix`, :option:`ConstantParameterSuffix`
- - :option:`ConstantPointerParameterCase`, :option:`ConstantPointerParameterPrefix`, :option:`ConstantPointerParameterSuffix`
- - :option:`ConstexprFunctionCase`, :option:`ConstexprFunctionPrefix`, :option:`ConstexprFunctionSuffix`
- - :option:`ConstexprMethodCase`, :option:`ConstexprMethodPrefix`, :option:`ConstexprMethodSuffix`
- - :option:`ConstexprVariableCase`, :option:`ConstexprVariablePrefix`, :option:`ConstexprVariableSuffix`
- - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`
- - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`
- - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`
+ - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`, :option:`ClassIgnoredRegexp`
+ - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`, :option:`ClassConstantIgnoredRegexp`
+ - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`, :option:`ClassMemberIgnoredRegexp`
+ - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`, :option:`ClassMethodIgnoredRegexp`
+ - :option:`ConstantCase`, :option:`ConstantPrefix`, 

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Shane via Phabricator via cfe-commits]

smhc updated this revision to Diff 306686.
smhc added a comment.

Updated diff to use 'IgnoredRegexp'
I see that a regexp has been used elsewhere for some other checks (macro usage, 
AllowedRegexp).

(as a side note, I see that AllowedRegexp string is getting compiled on every 
match rather than pre-compiling, the lack of a copy constructor on llvm::Regex 
makes it trickier to work with)


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

https://reviews.llvm.org/D90282

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignore-regexp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignore-regexp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignore-regexp.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: "^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: "^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: "sooo|so|soo|$o["} \
+// RUN:  ]}'
+
+int testFunc(int a, char **b);
+int testFunc(int ab, char **ba);
+int testFunc(int abc, char **cba);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'abc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'cba'
+// CHECK-FIXES: {{^}}int testFunc(int Abc, char **Cba);{{$}}
+int testFunc(int Abc, char **Cba);
+
+class fo {
+};
+
+class fofo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'fofo'
+  // CHECK-FIXES: {{^}}class Fofo {{{$}}
+};
+
+class foo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'foo'
+  // CHECK-FIXES: {{^}}class Foo {{{$}}
+};
+
+class fooo {
+};
+
+class afooo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'afooo'
+  // CHECK-FIXES: {{^}}class Afooo {{{$}}
+};
+
+struct soo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'soo'
+  // CHECK-FIXES: {{^}}struct Soo {{{$}}
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -20,7 +20,8 @@
  - ``aNy_CasE``.
 
 It also supports a fixed prefix and suffix that will be prepended or appended
-to the identifiers, regardless of the casing.
+to the identifiers, regardless of the casing. A threshold for the length of
+the identifer may be specified to suppress the checks for short names.
 
 Many configuration options are available, in order to be able to create
 different rules for different kinds of identifiers. In general, the rules are
@@ -35,60 +36,60 @@
 
 The following options are describe below:
 
- - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`
+ - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`, :option:`AbstractClassIgnoredRegexp`
  - :option:`AggressiveDependentMemberLookup`
- - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`
- - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`
- - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`
- - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`
- - :option:`ConstantCase`, :option:`ConstantPrefix`, :option:`ConstantSuffix`
- - :option:`ConstantMemberCase`, :option:`ConstantMemberPrefix`, :option:`ConstantMemberSuffix`
- - :option:`ConstantParameterCase`, :option:`ConstantParameterPrefix`, :option:`ConstantParameterSuffix`
- - :option:`ConstantPointerParameterCase`, :option:`ConstantPointerParameterPrefix`, :option:`ConstantPointerParameterSuffix`
- - :option:`ConstexprFunctionCase`, :option:`ConstexprFunctionPrefix`, :option:`ConstexprFunctionSuffix`
- - :option:`ConstexprMethodCase`, :option:`ConstexprMethodPrefix`, :option:`ConstexprMethodSuffix`
- - :option:`ConstexprVariableCase`, :option:`ConstexprVariablePrefix`, :option:`ConstexprVariableSuffix`
- - :option:`EnumCase`, 

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

This patch needs to be retitled to what this is actually doing: changing the 
getTypeAllocationSizeInBits and getFragmentSizeInBits to return a TypeSize 
instead of `unsigned`.
It would be even better if you can split those up into two patches with 
separate tests for each one of them.




Comment at: clang/test/CodeGen/aarch64-sve-acle-rel-note.c:1
+// REQUIRES: aarch64-registered-target
+// RUN: %clang --target=aarch64-linux-gnu -march=armv8-a+sve -S -emit-llvm -o 
- -c %s -Werror -Wall -g -O0 2>%t | FileCheck %s

I'm not entirely sure if it is acceptable to have a clang test that generates 
assembly, but in any case I think this test needs to be added in a separate 
patch.



Comment at: llvm/lib/IR/IntrinsicInst.cpp:56
+Optional DbgVariableIntrinsic::getFragmentSizeInBits() const {
+  if (Optional Fragment =
+  getExpression()->getFragmentInfo())

Change back to `auto` ?



Comment at: llvm/lib/Transforms/Utils/Local.cpp:1373
+  if (Optional FragmentSize = DII->getFragmentSizeInBits())
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
   // We can't always calculate the size of the DI variable (e.g. if it is a

Should the scalable flag match? Same question for all the other cases in this 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

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

[clang-tools-extra] e4f9b5d - [clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

[Chris Kennelly via cfe-commits]

Author: Chris Kennelly
Date: 2020-11-20T10:06:57-05:00
New Revision: e4f9b5d442a260dd78b3de581cec1e90567a2aac

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

LOG: [clang-tidy] Include std::basic_string_view in 
readability-redundant-string-init.

std::string_view("") produces a string_view instance that compares
equal to std::string_view(), but requires more complex initialization
(storing the address of the string literal, rather than zeroing).

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
index e5825bc4f0e3..24defc80f161 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -18,7 +18,8 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
-const char DefaultStringNames[] = "::std::basic_string";
+const char DefaultStringNames[] =
+"::std::basic_string_view;::std::basic_string";
 
 static ast_matchers::internal::Matcher
 hasAnyNameStdString(std::vector Names) {

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index cc9de109900b..5e78de2b0edc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -152,6 +152,11 @@ Changes in existing checks
 - Removed `google-runtime-references` check because the rule it checks does
   not exist in the Google Style Guide anymore.
 
+- Improved :doc:`readability-redundant-string-init
+  ` check.
+
+  Added `std::basic_string_view` to default list of ``string``-like types.
+
 Improvements to include-fixer
 -
 

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
index c4556887f89a..dc3dfacb15d5 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
@@ -19,12 +19,21 @@ Examples
   std::string a;
   std::string b;
 
+  // Initializing a string_view with an empty string literal produces an
+  // instance that compares equal to string_view().
+  std::string_view a = "";
+  std::string_view b("");
+
+  // becomes
+  std::string_view a;
+  std::string_view b;
+
 Options
 ---
 
 .. option:: StringNames
 
-Default is `::std::basic_string`.
+Default is `::std::basic_string;::std::basic_string_view`.
 
 Semicolon-delimited list of class names to apply this check to.
 By default `::std::basic_string` applies to ``std::string`` and

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
index c33d1a7d5f2a..ed3d90ca307e 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,7 +1,7 @@
 // RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t \
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-string-init.StringNames, \
-// RUN:   value: '::std::basic_string;our::TestString'}] \
+// RUN:   value: 
'::std::basic_string;::std::basic_string_view;our::TestString'}] \
 // RUN: }"
 // FIXME: Fix the checker to work in C++17 mode.
 
@@ -19,6 +19,20 @@ struct basic_string {
 };
 typedef basic_string string;
 typedef basic_string wstring;
+
+template , typename A = 
std::allocator>
+struct basic_string_view {
+  using size_type = decltype(sizeof(0));
+
+  basic_string_view();
+  basic_string_view(const basic_string_view &);
+  basic_string_view(const C *, size_type);
+  basic_string_view(const C *);
+  template 
+  basic_string_view(It, End);
+};
+typedef basic_string_view string_view;
+typedef basic_string_view wstring_view;
 }
 
 void f() {
@@ -48,6 +62,33 @@ void f() {
   std::string z;
 }
 
+void fview() {
+  std::string_view a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization 
[readability-redundant-string-init]
+  // CHECK-FIXES: std::string_view a;
+  

[PATCH] D91009: [clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

[Chris Kennelly 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 rGe4f9b5d442a2: [clang-tidy] Include std::basic_string_view in 
readability-redundant-string… (authored by ckennelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91009

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,7 +1,7 @@
 // RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t \
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-string-init.StringNames, \
-// RUN:   value: '::std::basic_string;our::TestString'}] \
+// RUN:   value: '::std::basic_string;::std::basic_string_view;our::TestString'}] \
 // RUN: }"
 // FIXME: Fix the checker to work in C++17 mode.
 
@@ -19,6 +19,20 @@
 };
 typedef basic_string string;
 typedef basic_string wstring;
+
+template , typename A = std::allocator>
+struct basic_string_view {
+  using size_type = decltype(sizeof(0));
+
+  basic_string_view();
+  basic_string_view(const basic_string_view &);
+  basic_string_view(const C *, size_type);
+  basic_string_view(const C *);
+  template 
+  basic_string_view(It, End);
+};
+typedef basic_string_view string_view;
+typedef basic_string_view wstring_view;
 }
 
 void f() {
@@ -48,6 +62,33 @@
   std::string z;
 }
 
+void fview() {
+  std::string_view a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization [readability-redundant-string-init]
+  // CHECK-FIXES: std::string_view a;
+  std::string_view b("");
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view b;
+  std::string_view c = R"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view c;
+  std::string_view d(R"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view d;
+  std::string_view e{""};
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view e;
+  std::string_view f = {""};
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: redundant string initialization
+  // CHECK-FIXES: std::string_view f;
+
+  std::string_view u = "u";
+  std::string_view w("w");
+  std::string_view x = R"(x)";
+  std::string_view y(R"(y)");
+  std::string_view z;
+}
+
 void g() {
   std::wstring a = L"";
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
@@ -69,6 +110,33 @@
   std::wstring z;
 }
 
+void gview() {
+  std::wstring_view a = L"";
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization [readability-redundant-string-init]
+  // CHECK-FIXES: std::wstring_view a;
+  std::wstring_view b(L"");
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring_view b;
+  std::wstring_view c = L"";
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring_view c;
+  std::wstring_view d(L"");
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring_view d;
+  std::wstring_view e{L""};
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring_view e;
+  std::wstring_view f = {L""};
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring_view f;
+
+  std::wstring_view u = L"u";
+  std::wstring_view w(L"w");
+  std::wstring_view x = LR"(x)";
+  std::wstring_view y(LR"(y)");
+  std::wstring_view z;
+}
+
 template 
 void templ() {
   std::string s = "";
@@ -274,7 +342,6 @@
   // CHECK-MESSAGES: [[@LINE-2]]:23: warning: redundant string initialization
   // CHECK-FIXES:  Foo(float)  {}
 
-
   // Check how it handles removing some redundant initializers while leaving
   // valid initializers intact.
   Foo(std::string Arg) : A(Arg), B(""), C("NonEmpty"), D(R"()"), E("") {}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
+++ 

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

As of a few hours ago, GCC has 
[`__builtin_clear_padding`](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fclear_005fpadding)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D91806: [SVE] Remove warnings from release notes example on SVE ACLE.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 306684.
fpetrogalli added a comment.

I have removed the C test, as the LL test is enough to test the changes I have 
done in `Local.cpp`.

I can confirm that with this patch, the release note ton'r raise any warnings 
when compiling with debug info:

  frapet01@man-08:~/projects/upstream-clang/build-clang$ cat ../release-notes.c
  #include 
  
  void VLA_add_arrays(double *x, double *y, double *out, unsigned N) {
for (unsigned i = 0; i < N; i += svcntd()) {
  svbool_t Pg = svwhilelt_b64(i, N);
  svfloat64_t vx = svld1(Pg, [i]);
  svfloat64_t vy = svld1(Pg, [i]);
  svfloat64_t vout = svadd_x(Pg, vx, vy);
  svst1(Pg, [i], vout);
}
  }
  frapet01@man-08:~/projects/upstream-clang/build-clang$ ./bin/clang 
--target=aarch64-gnu-linux -march=armv8-a+sve -c -g -S -O3 -o /dev/null 
../release-notes.c
  frapet01@man-08:~/projects/upstream-clang/build-clang$

When the patch will end up into a nightly build of compiler explorer, the 
compilation at this link should not raise any warnings: 
https://godbolt.org/z/zebzKM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

Files:
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  
llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll

Index: llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
@@ -0,0 +1,42 @@
+; RUN: opt -mtriple aarch64-gnu-linux -mattr=+sve -instcombine -S < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; If this check fails please read
+; clang/test/CodeGen/aarch64-sve-intrinsics/README for instructions on
+; how to resolve it.
+
+; WARN-NOT: warning
+
+; CHECK-LABEL: @debug_local_scalable(
+define  @debug_local_scalable( %tostore) {
+  %vx = alloca , align 16
+  call void @llvm.dbg.declare(metadata * %vx, metadata !5, metadata !DIExpression()), !dbg !15
+  store  %tostore, * %vx, align 16
+  %ret = call  @f(* %vx)
+  ret  %ret
+}
+
+declare  @f(*)
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "/tmp/test.c", directory: "/tmp/")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !DILocalVariable(name: "vx", scope: !6, file: !7, line: 26, type: !8)
+!6 = distinct !DISubprogram(name: "debug_local_scalable", scope: null, file: !1, line: 25, scopeLine: 25, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!7 = !DIFile(filename: "test.c", directory: "/tmp/")
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "svfloat64_t", file: !9, line: 56, baseType: !10)
+!9 = !DIFile(filename: "arm_sve.h", directory: "/tmp/")
+!10 = !DIDerivedType(tag: DW_TAG_typedef, name: "__SVFloat64_t", file: !1, baseType: !11)
+!11 = !DICompositeType(tag: DW_TAG_array_type, baseType: !12, flags: DIFlagVector, elements: !13)
+!12 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float)
+!13 = !{!14}
+!14 = !DISubrange(lowerBound: 0, upperBound: !DIExpression(DW_OP_constu, 1, DW_OP_bregx, 46, 0, DW_OP_mul, DW_OP_constu, 1, DW_OP_minus))
+!15 = !DILocation(line: 26, column: 15, scope: !6)
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1368,16 +1368,16 @@
 /// least n bits.
 static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   const DataLayout  = DII->getModule()->getDataLayout();
-  uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy);
-  if (auto FragmentSize = DII->getFragmentSizeInBits())
-return ValueSize >= *FragmentSize;
+  TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy);
+  if (Optional FragmentSize = DII->getFragmentSizeInBits())
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
   // We can't always calculate the size of the DI variable (e.g. if it is a
   // VLA). Try to use the size of the alloca that the dbg intrinsic describes
   // intead.
   if (DII->isAddressOfVariable())
 if (auto *AI = 

[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

[Sean Fertile via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D88676

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