[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added a comment.

https://github.com/llvm/llvm-project/issues/62110 (in addition to the 
self-friendship case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D147920: [clang] Add test for CWG399

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a subscriber: rsmith.
shafik added inline comments.



Comment at: clang/test/CXX/drs/dr3xx.cpp:1439
+
+namespace dr399 { // dr399: 11
+  // NB: reuse dr244 test 

Endill wrote:
> shafik wrote:
> > Endill wrote:
> > > Despite a couple of FIXME in CWG244 test (out of dozens of examples), it 
> > > claims full availability since Clang 11. I'd take a more conservative 
> > > approach, declaring partial support, but I think that declaring different 
> > > availability for the same test would bring unnecessary confusion. So I 
> > > followed CWG244 availability.
> > > 
> > > Alternative is to demote CWG244 to partial, but I'm not sure we should go 
> > > back on our claims for CWG support that has been out for so long.
> > I think the bugs are not awful, we should file bug reports if we don't 
> > already have them. Some of them seem like they should be not too bad to fix.
> > 
> > CC @aaron.ballman to get a second opinion
> If we are to file bug reports, I'm not sure what wording makes those examples 
> ill-formed. Is it [[ http://eel.is/c++draft/basic.lookup#qual.general-4.6 | 
> qual.general-4.6 ]]: `The type-name that is or contains Q shall refer to its 
> (original) lookup context (ignoring cv-qualification) under the 
> interpretation established by at least one (successful) lookup performed.`? I 
> interpret it as requiring names to the left and to the right of `~` to be 
> found in the same scope (lookup context; `namespace dr244` in our case). 
> Could it actually mean that they have to refer to the same type?
I am not sure maybe @rsmith might be able to help us here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147920

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Arthur Eubanks (out until mid-April) via Phabricator via cfe-commits]

aeubanks added a comment.

F27125556: a.tgz 

this is the original repro that crashed but maybe differently (you'll need to 
remove the plugin arguments since our clang has some custom plugins)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148189: [NFC][Clang] Fix static analyzer tool remark about missing user-defined assignment operator

[Soumi Manna via Phabricator via cfe-commits]

Manna created this revision.
Manna added reviewers: erichkeane, aaron.ballman.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a reviewer: NoQ.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

Reported by Coverity:

Copy without assign
This class has a user-defined copy constructor but no user-defined assignment 
operator. If the copy constructor is necessary to manage owned resources then a 
corresponding assignment operator is usually required. If an object of this 
type is assigned memory leaks and/or use-after-free errors may occur. Note that 
a compiler-generated assignment operator will perform only a bit-wise copy for 
any fields that do not have their own assignment operators defined.

Class has user-written copy constructor but no user-written assignment operator

copy_without_assign: Class ::DeclUseTracker has a user-written copy 
constructor ::DeclUseTracker::DeclUseTracker(::DeclUseTracker 
const &) =delete but no corresponding user-written assignment operator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148189

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp


Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -645,6 +645,7 @@
 public:
   DeclUseTracker() = default;
   DeclUseTracker(const DeclUseTracker &) = delete; // Let's avoid copies.
+  DeclUseTracker =(const DeclUseTracker &) = delete;
   DeclUseTracker(DeclUseTracker &&) = default;
   DeclUseTracker =(DeclUseTracker &&) = default;
 


Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -645,6 +645,7 @@
 public:
   DeclUseTracker() = default;
   DeclUseTracker(const DeclUseTracker &) = delete; // Let's avoid copies.
+  DeclUseTracker =(const DeclUseTracker &) = delete;
   DeclUseTracker(DeclUseTracker &&) = default;
   DeclUseTracker =(DeclUseTracker &&) = default;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.
Herald added a reviewer: rymiel.

@MyDeveloperDay We keep getting "spammed" by [libc++][format] patches. Can you 
delete the rule in H987 :

  "field": "differential.revision.title",
  "condition": "contains",
  "value": "[Format]"

or change "contains" to something equivalent to "starts with"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147176

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

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.
Herald added subscribers: bviyer, ekilmer, jplehr, thopre.

@Ericson2314  @phosek What's the state of this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

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

[PATCH] D148181: [Demangle] make llvm::demangle take a StringRef; NFC

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Unfortunately we cannot do this. `LLVMSupport` depends on `LLVMDemangle`. Due 
to library layering `LLVMDemangle` cannot depend on `LLVMSupport`.
The layering may look weird, but that's what we have.

Using `std::string_view` may possibly be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148181

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

[PATCH] D146809: [WIP][clang-repl] Implement Value pretty printing

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 513024.
junaire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp

Index: clang/test/Interpreter/pretty-print.cpp
===
--- /dev/null
+++ clang/test/Interpreter/pretty-print.cpp
@@ -0,0 +1,51 @@
+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl | FileCheck %s
+char c = 'a';
+c
+// CHECK: (char) 'a'
+
+int x = 42;
+x
+// CHECK-NEXT: (int) 42
+
+x - 2
+// CHECK-NEXT: (int) 40
+
+float f = 4.2f;
+f
+// CHECK-NEXT: (float) 4.2f
+
+double d = 4.21;
+d
+// CHECK-NEXT: (double) 4.210
+
+struct S{};
+S s;
+s
+// CHECK-NEXT: (S &) [[Addr:@0x.*]]
+
+S{}
+// CHECK-NEXT: (S) [[Addr:@0x.*]]
+
+struct SS { int* p; SS() { p = new int(42); } ~SS() { delete p; } };
+SS{}
+// CHECK-NEXT: (SS) [[Addr:@0x.*]]
+SS ss;
+ss
+// CHECK-NEXT: (SS &) [[Addr:@0x.*]]
+
+int arr[3] = {1,2,3};
+arr
+// CHECK-NEXT: (int[3]) { 1, 2, 3 }
+
+#include 
+
+auto p1 = std::make_shared(42);
+p1
+// CHECK-NEXT: (shared_ptr &) std::shared_ptr -> [[Addr:@0x.*]]
+
+%quit
+
Index: clang/lib/Interpreter/ValuePrinter.cpp
===
--- /dev/null
+++ clang/lib/Interpreter/ValuePrinter.cpp
@@ -0,0 +1,507 @@
+#include "InterpreterUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/Type.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Interpreter/Value.h"
+#include "clang/Parse/Parser.h"
+#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+using namespace clang;
+
+namespace rep_runtime_internal {
+REPL_EXTERNAL_VISIBILITY
+extern const char *const kEmptyCollection = "{}";
+} // namespace rep_runtime_internal
+
+static std::string PrintDeclType(const QualType , NamedDecl *D) {
+  std::string Str;
+  llvm::raw_string_ostream SS(Str);
+  if (QT.hasQualifiers())
+SS << QT.getQualifiers().getAsString() << " ";
+  SS << D->getQualifiedNameAsString();
+  return Str;
+}
+
+static std::string PrintQualType(ASTContext , QualType QT) {
+  std::string Str;
+  llvm::raw_string_ostream SS(Str);
+  PrintingPolicy Policy(Ctx.getPrintingPolicy());
+  // Print the Allocator in STL containers, for instance.
+  Policy.SuppressDefaultTemplateArgs = false;
+  Policy.SuppressUnwrittenScope = true;
+  // Print 'a >' rather than 'a>'.
+  Policy.SplitTemplateClosers = true;
+
+  struct LocalPrintingPolicyRAII {
+ASTContext 
+PrintingPolicy Policy;
+
+LocalPrintingPolicyRAII(ASTContext , PrintingPolicy )
+: Context(Ctx), Policy(Ctx.getPrintingPolicy()) {
+  Context.setPrintingPolicy(PP);
+}
+~LocalPrintingPolicyRAII() { Context.setPrintingPolicy(Policy); }
+  } X(Ctx, Policy);
+
+  const QualType NonRefTy = QT.getNonReferenceType();
+
+  if (const auto *TTy = llvm::dyn_cast(NonRefTy))
+SS << PrintDeclType(NonRefTy, TTy->getDecl());
+  else if (const auto *TRy = dyn_cast(NonRefTy))
+SS << PrintDeclType(NonRefTy, TRy->getDecl());
+  else {
+const QualType Canon = NonRefTy.getCanonicalType();
+if (Canon->isBuiltinType() && !NonRefTy->isFunctionPointerType() &&
+!NonRefTy->isMemberPointerType()) {
+  SS << Canon.getAsString(Ctx.getPrintingPolicy());
+} else if (const auto *TDTy = dyn_cast(NonRefTy)) {
+  // FIXME: TemplateSpecializationType & SubstTemplateTypeParmType checks
+  // are predominately to get STL containers to print nicer and might be
+  // better handled in GetFullyQualifiedName.
+  //
+  // std::vector::iterator is a TemplateSpecializationType
+  // std::vector::value_type is a SubstTemplateTypeParmType
+  //
+  QualType SSDesugar = TDTy->getLocallyUnqualifiedSingleStepDesugaredType();
+  if (llvm::isa(SSDesugar))
+SS << GetFullTypeName(Ctx, Canon);
+  else if (llvm::isa(SSDesugar))
+SS << GetFullTypeName(Ctx, NonRefTy);
+  else
+SS << PrintDeclType(NonRefTy, TDTy->getDecl());

[PATCH] D147263: Fix an assertion failure in unwrapSugar

[Akira Hatanaka 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 rG2ba88443b31a: Fix an assertion failure in unwrapSugar 
(authored by ahatanak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147263

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/sugar-common-types.cpp


Index: clang/test/SemaCXX/sugar-common-types.cpp
===
--- clang/test/SemaCXX/sugar-common-types.cpp
+++ clang/test/SemaCXX/sugar-common-types.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fenable-matrix -triple 
i686-pc-win32
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -x objective-c++ 
-fobjc-arc -fenable-matrix -triple i686-pc-win32
 
 enum class N {};
 
@@ -131,3 +131,14 @@
 auto t32 = 0 ? (UPX1){} : (UPY1){};
 N t33 = t32;  // expected-error {{lvalue of type 'C::type1' (aka 'int *')}}
 N t34 = *t32; // expected-error {{lvalue of type 'B1' (aka 'int')}}
+
+// See https://github.com/llvm/llvm-project/issues/61419
+namespace PR61419 {
+  template  struct pair {
+T0 first;
+T1 second;
+  };
+
+  extern const pair p;
+  id t = false ? p.first : p.second;
+} // namespace PR61419
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -13082,7 +13082,7 @@
 static auto unwrapSugar(SplitQualType , Qualifiers ) {
   SmallVector R;
   while (true) {
-QTotal += T.Quals;
+QTotal.addConsistentQualifiers(T.Quals);
 QualType NT = T.Ty->getLocallyUnqualifiedSingleStepDesugaredType();
 if (NT == QualType(T.Ty, 0))
   break;


Index: clang/test/SemaCXX/sugar-common-types.cpp
===
--- clang/test/SemaCXX/sugar-common-types.cpp
+++ clang/test/SemaCXX/sugar-common-types.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fenable-matrix -triple i686-pc-win32
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -x objective-c++ -fobjc-arc -fenable-matrix -triple i686-pc-win32
 
 enum class N {};
 
@@ -131,3 +131,14 @@
 auto t32 = 0 ? (UPX1){} : (UPY1){};
 N t33 = t32;  // expected-error {{lvalue of type 'C::type1' (aka 'int *')}}
 N t34 = *t32; // expected-error {{lvalue of type 'B1' (aka 'int')}}
+
+// See https://github.com/llvm/llvm-project/issues/61419
+namespace PR61419 {
+  template  struct pair {
+T0 first;
+T1 second;
+  };
+
+  extern const pair p;
+  id t = false ? p.first : p.second;
+} // namespace PR61419
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -13082,7 +13082,7 @@
 static auto unwrapSugar(SplitQualType , Qualifiers ) {
   SmallVector R;
   while (true) {
-QTotal += T.Quals;
+QTotal.addConsistentQualifiers(T.Quals);
 QualType NT = T.Ty->getLocallyUnqualifiedSingleStepDesugaredType();
 if (NT == QualType(T.Ty, 0))
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2ba8844 - Fix an assertion failure in unwrapSugar

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2023-04-12T16:45:56-07:00
New Revision: 2ba88443b31a437a3e80ac4bb6d8ca1675828d1b

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

LOG: Fix an assertion failure in unwrapSugar

An assertion in Qualifiers::addObjCLifetime fails when the ObjC lifetime
bits are already set.

Instead of calling operator+=, call addConsistentQualifiers, which
allows the lifetime bits to be set again as long the new value doesn't
conflict with the old value.

This fixes https://github.com/llvm/llvm-project/issues/61419.

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

Added: 


Modified: 
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/sugar-common-types.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6f36b056c7bb9..87c908ae5f561 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13082,7 +13082,7 @@ static QualType getCommonSugarTypeNode(ASTContext , 
const Type *X,
 static auto unwrapSugar(SplitQualType , Qualifiers ) {
   SmallVector R;
   while (true) {
-QTotal += T.Quals;
+QTotal.addConsistentQualifiers(T.Quals);
 QualType NT = T.Ty->getLocallyUnqualifiedSingleStepDesugaredType();
 if (NT == QualType(T.Ty, 0))
   break;

diff  --git a/clang/test/SemaCXX/sugar-common-types.cpp 
b/clang/test/SemaCXX/sugar-common-types.cpp
index 1932ccd9c7250..f4e545bb9bceb 100644
--- a/clang/test/SemaCXX/sugar-common-types.cpp
+++ b/clang/test/SemaCXX/sugar-common-types.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fenable-matrix -triple 
i686-pc-win32
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -x objective-c++ 
-fobjc-arc -fenable-matrix -triple i686-pc-win32
 
 enum class N {};
 
@@ -131,3 +131,14 @@ using UPY1 = C::type1;
 auto t32 = 0 ? (UPX1){} : (UPY1){};
 N t33 = t32;  // expected-error {{lvalue of type 'C::type1' (aka 'int *')}}
 N t34 = *t32; // expected-error {{lvalue of type 'B1' (aka 'int')}}
+
+// See https://github.com/llvm/llvm-project/issues/61419
+namespace PR61419 {
+  template  struct pair {
+T0 first;
+T1 second;
+  };
+
+  extern const pair p;
+  id t = false ? p.first : p.second;
+} // namespace PR61419



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

[PATCH] D140315: [AMDGCN] Update search path for device libraries

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D140315#4077611 , @scchan wrote:

> In D140315#4076614 , @mgorny wrote:
>
>> I'm sorry for reporting it this late (clang's been broken for over two 
>> weeks, so we couldn't periodically test it) but this change is causing test 
>> regressions on Gentoo/amd64:
>
> Thanks for reporting the issue.  I'm working on a patch to fix the test.

I don't think it's actually the test that was at fault, it doesn't look right 
that this diff uses `CLANG_INSTALL_LIBDIR_BASENAME` for an unrelated purpose to 
determining the basename of the libdir, especially considering the comments 
continue to say clang looks in `lib/amdgcn/bitcode` rather than 
`/amdgcn/bitcode`, and your original test specifically assumed that it 
would always be `lib`. Should the code perhaps just be updated to hardcode 
`"lib"` as it did before?

Either way, the test can still fail after D142506 
, `CLANG_INSTALL_LIBDIR_BASENAME` may be any 
arbitrary string and cannot be assumed to be either `lib` or `lib64`. With 
Debian-style multiarch I have `CLANG_INSTALL_LIBDIR=lib/x86_64-linux-gnu`, so 
after updating from Clang 15 to Clang 16 I am now also seeing this test fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140315

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

[PATCH] D147263: Fix an assertion failure in unwrapSugar

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 513010.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Simplify test case and move it to clang/test/SemaCXX/sugar-common-types.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147263

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/sugar-common-types.cpp


Index: clang/test/SemaCXX/sugar-common-types.cpp
===
--- clang/test/SemaCXX/sugar-common-types.cpp
+++ clang/test/SemaCXX/sugar-common-types.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fenable-matrix -triple 
i686-pc-win32
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -x objective-c++ 
-fobjc-arc -fenable-matrix -triple i686-pc-win32
 
 enum class N {};
 
@@ -131,3 +131,14 @@
 auto t32 = 0 ? (UPX1){} : (UPY1){};
 N t33 = t32;  // expected-error {{lvalue of type 'C::type1' (aka 'int *')}}
 N t34 = *t32; // expected-error {{lvalue of type 'B1' (aka 'int')}}
+
+// See https://github.com/llvm/llvm-project/issues/61419
+namespace PR61419 {
+  template  struct pair {
+T0 first;
+T1 second;
+  };
+
+  extern const pair p;
+  id t = false ? p.first : p.second;
+} // namespace PR61419
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -13082,7 +13082,7 @@
 static auto unwrapSugar(SplitQualType , Qualifiers ) {
   SmallVector R;
   while (true) {
-QTotal += T.Quals;
+QTotal.addConsistentQualifiers(T.Quals);
 QualType NT = T.Ty->getLocallyUnqualifiedSingleStepDesugaredType();
 if (NT == QualType(T.Ty, 0))
   break;


Index: clang/test/SemaCXX/sugar-common-types.cpp
===
--- clang/test/SemaCXX/sugar-common-types.cpp
+++ clang/test/SemaCXX/sugar-common-types.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fenable-matrix -triple i686-pc-win32
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -x objective-c++ -fobjc-arc -fenable-matrix -triple i686-pc-win32
 
 enum class N {};
 
@@ -131,3 +131,14 @@
 auto t32 = 0 ? (UPX1){} : (UPY1){};
 N t33 = t32;  // expected-error {{lvalue of type 'C::type1' (aka 'int *')}}
 N t34 = *t32; // expected-error {{lvalue of type 'B1' (aka 'int')}}
+
+// See https://github.com/llvm/llvm-project/issues/61419
+namespace PR61419 {
+  template  struct pair {
+T0 first;
+T1 second;
+  };
+
+  extern const pair p;
+  id t = false ? p.first : p.second;
+} // namespace PR61419
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -13082,7 +13082,7 @@
 static auto unwrapSugar(SplitQualType , Qualifiers ) {
   SmallVector R;
   while (true) {
-QTotal += T.Quals;
+QTotal.addConsistentQualifiers(T.Quals);
 QualType NT = T.Ty->getLocallyUnqualifiedSingleStepDesugaredType();
 if (NT == QualType(T.Ty, 0))
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

I have opened a bug at issuetracker.google.com/issues/277967395


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144331

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

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

Just a heads up, with this change, we are hitting issues in building gdb. 
Appreciate any ideas on what is wrong.

  aarch64-cros-linux-gnu-clang++ -x c++-I. -I. -I./config 
-DLOCALEDIR="\"/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode   
-I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber  
-I./../gnulib/import -I../gnulib/import -I./.. -I..  -DTUI=1-I./.. -pthread 
 -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch 
-Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter 
-Wunused-but-set-variable -Wno-sign-compare -Wno-error=maybe-uninitialized 
-Wno-mismatched-tags -Wno-error=deprecated-register -Wsuggest-override 
-Wimplicit-fallthrough=3 -Wduplicated-cond -Wshadow=local -Wdeprecated-copy 
-Wdeprecated-copy-dtor -Wredundant-move -Wmissing-declarations 
-Wmissing-prototypes -Wstrict-null-sentinel -Wformat -Wformat-nonliteral  -Os 
-pipe -march=armv8-a+crc+crypto -ftree-vectorize -g -ffunction-sections 
-fdata-sections -c -o minsyms.o -MT minsyms.o -MMD -MP -MF 
./.deps/minsyms.Tpo minsyms.c
   In file included from minsyms.c:56:
   In file included from ./../gdbsupport/parallel-for.h:25:
   In file included from /usr/bin/../include/c++/v1/thread:100:
   In file included from 
/usr/bin/../include/c++/v1/__format/formatter_integral.h:32:
   In file included from /usr/bin/../include/c++/v1/locale:203:
   /usr/bin/../include/c++/v1/__locale:600:5: error: '__abi_tag__' attribute 
only applies to structs, variables, functions, and namespaces
   _LIBCPP_INLINE_VISIBILITY
   ^
   /usr/bin/../include/c++/v1/__config:668:37: note: expanded from macro 
'_LIBCPP_INLINE_VISIBILITY'
   #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
   ^
   /usr/bin/../include/c++/v1/__config:647:26: note: expanded from macro 
'_LIBCPP_HIDE_FROM_ABI'
 
__attribute__((__abi_tag__(_LIBCPP_TOSTRING(_LIBCPP_VERSIONED_IDENTIFIER
^
   In file included from minsyms.c:56:
   In file included from ./../gdbsupport/parallel-for.h:25:
   In file included from /usr/bin/../include/c++/v1/thread:100:
   In file included from 
/usr/bin/../include/c++/v1/__format/formatter_integral.h:32:
   In file included from /usr/bin/../include/c++/v1/locale:203:
   /usr/bin/../include/c++/v1/__locale:601:37: error: expected ';' at end of 
declaration list
   char_type toupper(char_type __c) const
   ^
   /usr/bin/../include/c++/v1/__locale:607:48: error: too many arguments 
provided to function-like macro invocation
   const char_type* toupper(char_type* __low, const char_type* __high) const
  ^
   ./../include/safe-ctype.h:146:9: note: macro 'toupper' defined here
   #define toupper(c) do_not_use_toupper_with_safe_ctype
   ^
   In file included from minsyms.c:56:
   In file included from ./../gdbsupport/parallel-for.h:25:
   In file included from /usr/bin/../include/c++/v1/thread:100:
   In file included from 
/usr/bin/../include/c++/v1/__format/formatter_integral.h:32:
   In file included from /usr/bin/../include/c++/v1/locale:203:
   /usr/bin/../include/c++/v1/__locale:619:48: error: too many arguments 
provided to function-like macro invocation
   const char_type* tolower(char_type* __low, const char_type* __high) const
  ^
   ./../include/safe-ctype.h:148:9: note: macro 'tolower' defined here
   #define tolower(c) do_not_use_tolower_with_safe_ctype
   ^

Contents of __locale at line 600:

  _LIBCPP_INLINE_VISIBILITY
  char_type toupper(char_type __c) const
  {
  return do_toupper(__c);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144331

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

[PATCH] D148181: [Demangle] make llvm::demangle take a StringRef; NFC

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: llvm/lib/Demangle/Demangle.cpp:29
 
-std::string llvm::demangle(const std::string ) {
+std::string llvm::demangle(const StringRef MangledName) {
   std::string Result;

Should `const` be removed?  (i.e. is StringRef not inherently const?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148181

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

[PATCH] D148181: [Demangle] make llvm::demangle take a StringRef; NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

There's a few more callsites I'd like to clean up, tomorrow:

- llvm/tools/llvm-objdump/llvm-objdump.cpp
- llvm/tools/llvm-objdump/XCOFFDump.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148181

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

[PATCH] D148181: [Demangle] make llvm::demangle take a StringRef; NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: erichkeane, MaskRay.
Herald added subscribers: pmatos, asb, hiraditya, arichardson, sbc100, emaste.
Herald added a reviewer: jhenderson.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
nickdesaulniers requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, aheejin.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148181

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lld/COFF/Symbols.cpp
  lld/ELF/SymbolTable.cpp
  lld/ELF/Symbols.cpp
  lld/MachO/Symbols.cpp
  lld/wasm/Symbols.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
  llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objdump/ELFDump.cpp
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp

Index: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
===
--- llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
+++ llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
@@ -107,7 +107,7 @@
   std::string OutputName = "'";
   OutputName += Name;
   OutputName += "'";
-  std::string DemangledName(demangle(Name.str()));
+  std::string DemangledName(demangle(Name));
   if (Name != DemangledName) {
 OutputName += " aka ";
 OutputName += DemangledName;
Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -908,7 +908,7 @@
 }
 
 static std::string maybeDemangle(StringRef Name) {
-  return opts::Demangle ? demangle(std::string(Name)) : Name.str();
+  return opts::Demangle ? demangle(Name) : Name.str();
 }
 
 template 
Index: llvm/tools/llvm-objdump/ELFDump.cpp
===
--- llvm/tools/llvm-objdump/ELFDump.cpp
+++ llvm/tools/llvm-objdump/ELFDump.cpp
@@ -109,7 +109,7 @@
   if (!SymName)
 return SymName.takeError();
   if (Demangle)
-Fmt << demangle(std::string(*SymName));
+Fmt << demangle(*SymName);
   else
 Fmt << *SymName;
 }
Index: llvm/tools/llvm-nm/llvm-nm.cpp
===
--- llvm/tools/llvm-nm/llvm-nm.cpp
+++ llvm/tools/llvm-nm/llvm-nm.cpp
@@ -646,10 +646,10 @@
 
 static std::optional demangleXCOFF(StringRef Name) {
   if (Name.empty() || Name[0] != '.')
-return demangle(Name);
+return ::demangle(Name);
 
   Name = Name.drop_front();
-  std::optional DemangledName = demangle(Name);
+  std::optional DemangledName = ::demangle(Name);
   if (DemangledName)
 return "." + *DemangledName;
   return std::nullopt;
@@ -658,7 +658,7 @@
 static std::optional demangleMachO(StringRef Name) {
   if (!Name.empty() && Name[0] == '_')
 Name = Name.drop_front();
-  return demangle(Name);
+  return ::demangle(Name);
 }
 
 static bool symbolIsDefined(const NMSymbol ) {
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -441,7 +441,7 @@
 }
 
 void DiagnosticInfoDontCall::print(DiagnosticPrinter ) const {
-  DP << "call to " << demangle(getFunctionName().str())
+  DP << "call to " << demangle(getFunctionName())
  << " marked \"dontcall-";
   if (getSeverity() == DiagnosticSeverity::DS_Error)
 DP << "error\"";
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -26,9 +26,10 @@
  MangledName[1] == 'D';
 }
 
-std::string llvm::demangle(const std::string ) {
+std::string llvm::demangle(const StringRef MangledName) {
   std::string Result;
-  const char *S = MangledName.c_str();
+  std::string Copy = MangledName.str();
+  const char *S = Copy.data();
 
   if (nonMicrosoftDemangle(S, Result))
 return Result;
@@ -43,7 +44,7 @@
 return Result;
   }
 
-  return MangledName;
+  return Copy;
 }
 
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string ) {
Index: llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
===
--- llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
+++ llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp
@@ -229,7 +229,7 @@
 return true;
 
   highlight();
-  OS << llvm::demangle(Node.Fields.front().str());
+  OS << llvm::demangle(Node.Fields.front());
   restoreColor();
   return true;
 }
Index: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
===
--- 

[PATCH] D146987: [Assignment Tracking] Enable by default

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In D146987#4263048 , @MaskRay wrote:

> The latest reland 3820e9a2b29a2e268319ed6635da0d59e18d736d 
>  still 
> caused some issues to our internal workloads. Stay tuned...
>
> So far we have only found crashes with ThinLTO workloads.

More specifically, thinlto + split debug.

I'm not sure if my reduction is the same issue as @aeubanks has. Here's mine: 
F27123766: repro-D146987.ll 

Repro w/ `clang -O1 -c reduced.ll -o /dev/null`

I don't see any assertions going off for this one. The stack trace I have just 
points to `llvm::Value::stripPointerCasts`. Lemme know if you need more info, 
but I'm hoping that file reproduces for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148102: [clang] Specify attribute syntax & spelling with a single argument

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm updated this revision to Diff 512993.
rsandifo-arm added a comment.

Tweak initializer to avoid an MSVC error: https://godbolt.org/z/sMxsW8G8j


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148102

Files:
  clang/include/clang/Basic/AttributeCommonInfo.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2378,6 +2378,13 @@
   OS << "#endif // CLANG_ATTR_ACCEPTS_EXPR_PACK\n\n";
 }
 
+static void emitFormInitializer(raw_ostream ,
+const FlattenedSpelling ,
+StringRef SpellingIndex) {
+  OS << "{AttributeCommonInfo::AS_" << Spelling.variety() << ", "
+ << SpellingIndex << "}";
+}
+
 static void emitAttributes(RecordKeeper , raw_ostream ,
bool Header) {
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
@@ -2595,14 +2602,9 @@
   if (Header)
 OS << " = {}";
   if (Spellings.size() > 1) {
-OS << ", AttributeCommonInfo::Syntax Syntax";
+OS << ", Spelling S";
 if (Header)
-  OS << " = AttributeCommonInfo::AS_" << Spellings[0].variety();
-  }
-  if (!ElideSpelling) {
-OS << ", " << R.getName() << "Attr::Spelling S";
-if (Header)
-  OS << " = static_cast(SpellingNotCalculated)";
+  OS << " = " << SemanticToSyntacticMap[0];
   }
   OS << ")";
   if (Header) {
@@ -2618,15 +2620,31 @@
   else
 OS << "NoSemaHandlerAttribute";
 
-  if (Spellings.size() == 0)
+  if (Spellings.size() == 0) {
 OS << ", AttributeCommonInfo::AS_Implicit";
-  else if (Spellings.size() == 1)
-OS << ", AttributeCommonInfo::AS_" << Spellings[0].variety();
-  else
-OS << ", Syntax";
+  } else if (Spellings.size() == 1) {
+OS << ", ";
+emitFormInitializer(OS, Spellings[0], "0");
+  } else {
+OS << ", (\n";
+std::set Uniques;
+unsigned Idx = 0;
+for (auto I = Spellings.begin(), E = Spellings.end(); I != E;
+ ++I, ++Idx) {
+  const FlattenedSpelling  = *I;
+  const auto  = SemanticToSyntacticMap[Idx];
+  if (Uniques.insert(Name).second) {
+OS << "S == " << Name << " ? AttributeCommonInfo::Form";
+emitFormInitializer(OS, S, Name);
+OS << " :\n";
+  }
+}
+OS << "(llvm_unreachable(\"Unknown attribute spelling!\"), "
+   << " AttributeCommonInfo::Form";
+emitFormInitializer(OS, Spellings[0], "0");
+OS << "))";
+  }
 
-  if (!ElideSpelling)
-OS << ", S";
   OS << ");\n";
   OS << "  return Create";
   if (Implicit)
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3092,9 +3092,10 @@
   unsigned Syntax = Record.readInt();
   unsigned SpellingIndex = Record.readInt();
 
-  AttributeCommonInfo Info(AttrName, ScopeName, AttrRange, ScopeLoc,
-   AttributeCommonInfo::Kind(ParsedKind),
-   AttributeCommonInfo::Syntax(Syntax), SpellingIndex);
+  AttributeCommonInfo Info(
+  AttrName, ScopeName, AttrRange, ScopeLoc,
+  AttributeCommonInfo::Kind(ParsedKind),
+  {AttributeCommonInfo::Syntax(Syntax), SpellingIndex});
 
 #include "clang/Serialization/AttrPCHRead.inc"
 
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8981,8 +8981,7 @@
   ? S.ImplicitMSInheritanceAttrLoc
   : RD->getSourceRange();
 RD->addAttr(MSInheritanceAttr::CreateImplicit(
-S.getASTContext(), BestCase, Loc, AttributeCommonInfo::AS_Microsoft,
-MSInheritanceAttr::Spelling(IM)));
+S.getASTContext(), BestCase, Loc, MSInheritanceAttr::Spelling(IM)));
 S.Consumer.AssignInheritanceModel(RD);
   }
 }
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- 

[PATCH] D148091: [clang][CodeGen] Break up TargetInfo.cpp [3/6]

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 updated this revision to Diff 512991.
barannikov88 added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148091

Files:
  clang/lib/CodeGen/TargetInfo.cpp


Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5067,9 +5067,10 @@
llvm::Value *Address) const override;
 };
 
-class PPC64TargetCodeGenInfo : public DefaultTargetCodeGenInfo {
+class PPC64TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
-  PPC64TargetCodeGenInfo(CodeGenTypes ) : DefaultTargetCodeGenInfo(CGT) {}
+  PPC64TargetCodeGenInfo(CodeGenTypes )
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -5079,7 +5080,6 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
-
 }
 
 // Return true if the ABI requires Ty to be passed sign- or zero-
@@ -8600,10 +8600,10 @@
 
 namespace {
 
-class TCETargetCodeGenInfo : public DefaultTargetCodeGenInfo {
+class TCETargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   TCETargetCodeGenInfo(CodeGenTypes )
-: DefaultTargetCodeGenInfo(CGT) {}
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule ) const override;


Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5067,9 +5067,10 @@
llvm::Value *Address) const override;
 };
 
-class PPC64TargetCodeGenInfo : public DefaultTargetCodeGenInfo {
+class PPC64TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
-  PPC64TargetCodeGenInfo(CodeGenTypes ) : DefaultTargetCodeGenInfo(CGT) {}
+  PPC64TargetCodeGenInfo(CodeGenTypes )
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -5079,7 +5080,6 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
-
 }
 
 // Return true if the ABI requires Ty to be passed sign- or zero-
@@ -8600,10 +8600,10 @@
 
 namespace {
 
-class TCETargetCodeGenInfo : public DefaultTargetCodeGenInfo {
+class TCETargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   TCETargetCodeGenInfo(CodeGenTypes )
-: DefaultTargetCodeGenInfo(CGT) {}
+  : TargetCodeGenInfo(std::make_unique(CGT)) {}
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule ) const override;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148177: [Clang][AIX] Remove error for -fprofile-instr-generate/use on AIX

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA created this revision.
ZarkoCA added reviewers: w2yehia, qiongsiwu1, hubert.reinterpretcast.
ZarkoCA added a project: PowerPC.
Herald added subscribers: wlei, wenlei.
Herald added a project: All.
ZarkoCA requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Instrumented profiling now works on AIX and there is no dependency 
on LTO for PGO. Remove the error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148177

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-option.c


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -6,14 +6,6 @@
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
 
-// RUN: not %clang -fprofile-instr-generate --target=powerpc-ibm-aix %s 2>&1 | 
\
-// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
-// INVALID-AIX-PROFILE: error: unsupported option '-fprofile-instr-generate' 
for target
-
-// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 
2>&1 | \
-// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
-// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for 
target
-
 // RUN: not %clang --target=powerpc-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX-LONGDOUBLE128-ERR
 // AIX-LONGDOUBLE128-ERR: error: unsupported option '-mlong-double-128' for 
target 'powerpc-ibm-aix'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -730,15 +730,6 @@
 PGOGenerateArg = nullptr;
   }
 
-  if (TC.getTriple().isOSAIX()) {
-if (ProfileGenerateArg)
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << ProfileGenerateArg->getSpelling() << TC.getTriple().str();
-if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
-  }
-
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
 options::OPT_fprofile_instr_generate_EQ))


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -6,14 +6,6 @@
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
 
-// RUN: not %clang -fprofile-instr-generate --target=powerpc-ibm-aix %s 2>&1 | \
-// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
-// INVALID-AIX-PROFILE: error: unsupported option '-fprofile-instr-generate' for target
-
-// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 2>&1 | \
-// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
-// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for target
-
 // RUN: not %clang --target=powerpc-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX-LONGDOUBLE128-ERR
 // AIX-LONGDOUBLE128-ERR: error: unsupported option '-mlong-double-128' for target 'powerpc-ibm-aix'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -730,15 +730,6 @@
 PGOGenerateArg = nullptr;
   }
 
-  if (TC.getTriple().isOSAIX()) {
-if (ProfileGenerateArg)
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << ProfileGenerateArg->getSpelling() << TC.getTriple().str();
-if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
-  D.Diag(diag::err_drv_unsupported_opt_for_target)
-  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
-  }
-
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
 options::OPT_fprofile_instr_generate_EQ))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

@erichkeane let me know if the test I added is sufficient


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

https://reviews.llvm.org/D146329

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

[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 512983.
shafik added a comment.

- Added codegen test


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

https://reviews.llvm.org/D146329

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CodeGenCXX/defaulted_equality_ignore_unnamed_bitfields.cpp


Index: clang/test/CodeGenCXX/defaulted_equality_ignore_unnamed_bitfields.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/defaulted_equality_ignore_unnamed_bitfields.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 %s -triple x86_64-linux -emit-llvm -o - | FileCheck %s
+
+// Check that we don't attempt to compare the unnamed bitfields
+struct A {
+  unsigned x : 1;
+  unsigned   : 1;
+
+  friend bool operator==(A, A);
+};
+
+
+struct B {
+  unsigned x : 1;
+  unsigned   : 31;
+
+  friend bool operator==(B, B);
+};
+
+bool operator==(A, A) = default;
+bool operator==(B, B) = default;
+
+
+// If it compares the unnamed bitfields it will first compare the named 
bit-field
+// and then branch based on the result of that comparison.
+// We also won't see a lshr in order to clear the named bitfield.
+// CHECK-NOT: br
+// CHECK-NOT: lshr
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -226,3 +226,26 @@
   (void)(b == 0);
 }
 } // namespace p2085_2
+
+namespace GH61417 {
+struct A {
+  unsigned x : 1;
+  unsigned   : 0;
+  unsigned y : 1;
+
+  constexpr A() : x(0), y(0) {}
+  bool operator==(const A& rhs) const noexcept = default;
+};
+
+void f1() {
+  constexpr A a, b;
+  constexpr bool c = (a == b); // no diagnostic, we should not be comparing the
+   // unnamed bit-field which is indeterminate
+}
+
+void f2() {
+A a, b;
+bool c = (a == b); // no diagnostic nor crash during codegen attempting to
+   // access info for unnamed bit-field
+}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7755,6 +7755,10 @@
 
 //   followed by the non-static data members of C
 for (FieldDecl *Field : Record->fields()) {
+  // C++23 [class.bit]p2:
+  //   Unnamed bit-fields are not members ...
+  if (Field->isUnnamedBitfield())
+continue;
   // Recursively expand anonymous structs.
   if (Field->isAnonymousStructOrUnion()) {
 if (visitSubobjects(Results, Field->getType()->getAsCXXRecordDecl(),


Index: clang/test/CodeGenCXX/defaulted_equality_ignore_unnamed_bitfields.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/defaulted_equality_ignore_unnamed_bitfields.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 %s -triple x86_64-linux -emit-llvm -o - | FileCheck %s
+
+// Check that we don't attempt to compare the unnamed bitfields
+struct A {
+  unsigned x : 1;
+  unsigned   : 1;
+
+  friend bool operator==(A, A);
+};
+
+
+struct B {
+  unsigned x : 1;
+  unsigned   : 31;
+
+  friend bool operator==(B, B);
+};
+
+bool operator==(A, A) = default;
+bool operator==(B, B) = default;
+
+
+// If it compares the unnamed bitfields it will first compare the named bit-field
+// and then branch based on the result of that comparison.
+// We also won't see a lshr in order to clear the named bitfield.
+// CHECK-NOT: br
+// CHECK-NOT: lshr
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -226,3 +226,26 @@
   (void)(b == 0);
 }
 } // namespace p2085_2
+
+namespace GH61417 {
+struct A {
+  unsigned x : 1;
+  unsigned   : 0;
+  unsigned y : 1;
+
+  constexpr A() : x(0), y(0) {}
+  bool operator==(const A& rhs) const noexcept = default;
+};
+
+void f1() {
+  constexpr A a, b;
+  constexpr bool c = (a == b); // no diagnostic, we should not be comparing the
+   // unnamed bit-field which is indeterminate
+}
+
+void f2() {
+A a, b;
+bool c = (a == b); // no diagnostic nor crash during codegen attempting to
+   // access info for unnamed bit-field
+}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7755,6 +7755,10 @@
 
 //   followed by the non-static data members of C
 for (FieldDecl *Field : Record->fields()) {
+  // C++23 [class.bit]p2:
+  //   Unnamed bit-fields are not members ...
+  if 

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir created this revision.
benlangmuir added reviewers: akyrtzi, jansvoboda11, Bigcheese.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We only want to make PCH imports visible once for the the TU, not repeatedly 
after every subsequent import. This causes some incorrect behaviour with 
submodule visibility, and causes us to get extra module dependencies in the 
scanner. So far I have only seen obviously incorrect behaviour when building 
with -fmodule-name to cause a submodule to be textually included when using the 
PCH, though the old behaviour seems wrong regardless.

rdar://107449644


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148176

Files:
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Serialization/ASTReader.cpp
  clang/test/ClangScanDeps/modules-pch-imports.c
  clang/test/Modules/submodule-visibility-pch.c

Index: clang/test/Modules/submodule-visibility-pch.c
===
--- /dev/null
+++ clang/test/Modules/submodule-visibility-pch.c
@@ -0,0 +1,56 @@
+// Verify that the use of a PCH does not accidentally make modules from the PCH
+// visible to submodules when using -fmodules-local-submodule-visibility
+// and -fmodule-name to trigger a textual include.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// First check that it works with a header
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache \
+// RUN:   -fmodules-local-submodule-visibility -fimplicit-module-maps \
+// RUN:   -fmodule-name=Mod \
+// RUN:   %t/tu.c -include %t/prefix.h -I %t -verify
+
+// Now with a PCH
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache \
+// RUN:   -fmodules-local-submodule-visibility -fimplicit-module-maps \
+// RUN:   -x c-header %t/prefix.h -emit-pch -o %t/prefix.pch -I %t
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache \
+// RUN:   -fmodules-local-submodule-visibility -fimplicit-module-maps \
+// RUN:   -fmodule-name=Mod \
+// RUN:   %t/tu.c -include-pch %t/prefix.pch -I %t -verify
+
+//--- module.modulemap
+module ModViaPCH { header "ModViaPCH.h" }
+module ModViaInclude { header "ModViaInclude.h" }
+module Mod { header "Mod.h" }
+module SomeOtherMod { header "SomeOtherMod.h" }
+
+//--- ModViaPCH.h
+#define ModViaPCH 1
+
+//--- ModViaInclude.h
+#define ModViaInclude 2
+
+//--- SomeOtherMod.h
+// empty
+
+//--- Mod.h
+#include "SomeOtherMod.h"
+#ifdef ModViaPCH
+#error "Visibility violation ModViaPCH"
+#endif
+#ifdef ModViaInclude
+#error "Visibility violation ModViaInclude"
+#endif
+
+//--- prefix.h
+#include "ModViaPCH.h"
+
+//--- tu.c
+#include "ModViaInclude.h"
+#include "Mod.h"
+// expected-no-diagnostics
Index: clang/test/ClangScanDeps/modules-pch-imports.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-pch-imports.c
@@ -0,0 +1,108 @@
+// Check that a module from -fmodule-name= does not accidentally pick up extra
+// dependencies that come from a PCH.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: sed "s|DIR|%/t|g" %t/cdb_pch.json.template > %t/cdb_pch.json
+
+// Scan PCH
+// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json \
+// RUN:   -format experimental-full -mode preprocess-dependency-directives \
+// RUN:   > %t/deps_pch.json
+
+// Build PCH
+// RUN: %deps-to-rsp %t/deps_pch.json --module-name A > %t/A.rsp
+// RUN: %deps-to-rsp %t/deps_pch.json --module-name B > %t/B.rsp
+// RUN: %deps-to-rsp %t/deps_pch.json --tu-index 0 > %t/pch.rsp
+// RUN: %clang @%t/A.rsp
+// RUN: %clang @%t/B.rsp
+// RUN: %clang @%t/pch.rsp
+
+// Scan TU with PCH
+// RUN: clang-scan-deps -compilation-database %t/cdb.json \
+// RUN:   -format experimental-full -mode preprocess-dependency-directives \
+// RUN:   > %t/deps.json
+
+// RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// Verify that the only modular import in C is E and not the unrelated modules
+// A or B that come from the PCH.
+
+// CHECK:  {
+// CHECK-NEXT:  "modules": [
+// CHECK-NEXT: {
+// CHECK:"clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK:"module-name": "E"
+// CHECK:  }
+// CHECK-NEXT:   ]
+// CHECK:"clang-modulemap-file": "[[PREFIX]]/module.modulemap"
+// CHECK:"command-line": [
+// CHECK-NOT:  "-fmodule-file=
+// CHECK:  "-fmodule-file={{(E=)?}}[[PREFIX]]/{{.*}}/E-{{.*}}.pcm"
+// CHECK-NOT:  "-fmodule-file=
+// CHECK:]
+// CHECK:"name": "C"
+// CHECK:  }
+
+
+//--- cdb_pch.json.template
+[{
+  "file": "DIR/prefix.h",
+  "directory": "DIR",
+  "command": "clang -x c-header DIR/prefix.h -o DIR/prefix.h.pch -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache"

[clang] efc8b52 - Revert D146987 "[Assignment Tracking] Enable by default"

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2023-04-12T15:01:04-07:00
New Revision: efc8b52cbd942f4bd5ffe8f64da5fb8a3b7adc32

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

LOG: Revert D146987 "[Assignment Tracking] Enable by default"

This reverts commit 3820e9a2b29a2e268319ed6635da0d59e18d736d.

See https://reviews.llvm.org/D146987 for issues.

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/test/CodeGen/assignment-tracking/flag.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 7a2d9bf2a3915..eb3c535a4a554 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5815,7 +5815,7 @@ def experimental_assignment_tracking_EQ : Joined<["-"], 
"fexperimental-assignmen
   Group, CodeGenOpts<"EnableAssignmentTracking">,
   NormalizedValuesScope<"CodeGenOptions::AssignmentTrackingOpts">,
   Values<"disabled,enabled,forced">, 
NormalizedValues<["Disabled","Enabled","Forced"]>,
-  MarshallingInfoEnum, "Enabled">;
+  MarshallingInfoEnum, "Disabled">;
 
 } // let Flags = [CC1Option, NoDriverOption]
 

diff  --git a/clang/test/CodeGen/assignment-tracking/flag.cpp 
b/clang/test/CodeGen/assignment-tracking/flag.cpp
index 3bd974fe07c6c..aa1f054dae4d7 100644
--- a/clang/test/CodeGen/assignment-tracking/flag.cpp
+++ b/clang/test/CodeGen/assignment-tracking/flag.cpp
@@ -8,10 +8,10 @@
 // RUN: -emit-llvm  %s -o - -fexperimental-assignment-tracking=disabled 
-O1\
 // RUN: | FileCheck %s --check-prefixes=DISABLE
 
- Enabled by default:
+ Disabled by default:
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\
 // RUN: -emit-llvm  %s -o - -O1
\
-// RUN: | FileCheck %s --check-prefixes=ENABLE
+// RUN: | FileCheck %s --check-prefixes=DISABLE
 
  Disabled at O0 unless forced.
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\



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

[PATCH] D144304: [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment

[Ziqing Luo 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 rG762af11d4c5d: [-Wunsafe-buffer-usage] Add a Fixable for 
pointer pre-increment (authored by ziqingluo-90).

Changed prior to commit:
  https://reviews.llvm.org/D144304?vs=503928=512979#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144304

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void foo(int * , int *);
+
+void simple() {
+  int * p = new int[10];
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span p"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+  bool b = ++p;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:15}:"(p = p.subspan(1)).data()"
+  unsigned long n = (unsigned long) ++p;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:40}:"(p = p.subspan(1)).data()"
+  if (++p) {
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+  }
+  if (++p - ++p) {
+// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:16}:"(p = p.subspan(1)).data()"
+  }
+  foo(++p, p);
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:".data()"
+
+  // FIXME: Don't know how to fix the following cases:
+  // CHECK-NOT: fix-it:"{{.*}}":{
+  int * g = new int[10];
+  int * a[10];
+  a[0] = ++g;
+  foo(g++, g);
+  if (++(a[0])) {
+  }
+}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -139,6 +139,11 @@
   return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
 }
 
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(UnaryOperator, isPreInc) {
+  return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+}  
+
 // Returns a matcher that matches any expression 'e' such that `innerMatcher`
 // matches 'e' and 'e' is in an Unspecified Lvalue Context.
 static auto isInUnspecifiedLvalueContext(internal::Matcher innerMatcher) {
@@ -719,6 +724,46 @@
 };
 } // namespace
 
+
+// Representing a pointer type expression of the form `++Ptr` in an Unspecified
+// Pointer Context (UPC):
+class UPCPreIncrementGadget : public FixableGadget {
+private:
+  static constexpr const char *const UPCPreIncrementTag =
+"PointerPreIncrementUnderUPC";
+  const UnaryOperator *Node; // the `++Ptr` node
+
+public:
+  UPCPreIncrementGadget(const MatchFinder::MatchResult )
+: FixableGadget(Kind::UPCPreIncrement),
+  Node(Result.Nodes.getNodeAs(UPCPreIncrementTag)) {
+assert(Node != nullptr && "Expecting a non-null matching result");
+  }
+
+  static bool classof(const Gadget *G) {
+return G->getKind() == Kind::UPCPreIncrement;
+  }
+
+  static Matcher matcher() {
+// Note here we match `++Ptr` for any expression `Ptr` of pointer type.
+// Although currently we can only provide fix-its when `Ptr` is a DRE, we
+// can have the matcher be general, so long as `getClaimedVarUseSites` does
+// things right.
+return stmt(isInUnspecifiedPointerContext(expr(ignoringImpCasts(
+unaryOperator(isPreInc(),
+		  hasUnaryOperand(declRefExpr())
+		  ).bind(UPCPreIncrementTag);
+  }
+
+  virtual std::optional getFixits(const Strategy ) const override;
+
+  virtual const Stmt *getBaseStmt() const override { return Node; }
+
+  virtual DeclUseList getClaimedVarUseSites() const override {
+return {dyn_cast(Node->getSubExpr())};
+  }
+};
+
 // Representing a fixable expression of the form `*(ptr + 123)` or `*(123 +
 // ptr)`:
 class DerefSimplePtrArithFixableGadget : public FixableGadget {
@@ -1196,6 +1241,36 @@
   FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())};
 }
 
+
+std::optional UPCPreIncrementGadget::getFixits(const Strategy ) const {
+  DeclUseList DREs = getClaimedVarUseSites();
+
+  if (DREs.size() != 1)
+return std::nullopt; // In cases of `++Ptr` where `Ptr` is not a DRE, we
+ // give up
+  if (const VarDecl *VD = dyn_cast(DREs.front()->getDecl())) {
+

[clang] 762af11 - [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment

[Ziqing Luo via cfe-commits]

Author: Ziqing Luo
Date: 2023-04-12T14:51:46-07:00
New Revision: 762af11d4c5d0bd1e76f23a07087773db09ef17d

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

LOG: [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment

For a pointer type expression `e` of the form `++DRE`, if `e` is under
an Unspecified Pointer Context (UPC) and `DRE` is suppose to be
transformed to have std:span type, we generate fix-its that transform `e` to
`(DRE = DRE.subspan(1)).data()`.

For reference, `e` is in an UPC if `e` is
- an argument of a function call (except the callee has [[unsafe_buffer_usage]] 
attribute), or
- the operand of a cast-to-(Integer or Boolean) operation; or
- the operand of a pointer subtraction operation; or
- the operand of a pointer comparison operation;

We may extend the definition of UPC by adding more cases later.

Reviewed by: NoQ (Artem Dergachev)

Differential revision: https://reviews.llvm.org/D144304

Added: 
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp

Modified: 
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def 
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index a52b00bf8d4ce..a112b6d105025 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -35,6 +35,7 @@ FIXABLE_GADGET(DerefSimplePtrArithFixable)
 FIXABLE_GADGET(PointerDereference)
 FIXABLE_GADGET(UPCAddressofArraySubscript) // '[any]' in an Unspecified 
Pointer Context
 FIXABLE_GADGET(UPCStandalonePointer)
+FIXABLE_GADGET(UPCPreIncrement)// '++Ptr' in an Unspecified 
Pointer Context
 
 #undef FIXABLE_GADGET
 #undef WARNING_GADGET

diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6a597c8851002..ff818140b2d89 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -139,6 +139,11 @@ AST_MATCHER_P(CastExpr, castSubExpr, 
internal::Matcher, innerMatcher) {
   return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
 }
 
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(UnaryOperator, isPreInc) {
+  return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+}  
+
 // Returns a matcher that matches any expression 'e' such that `innerMatcher`
 // matches 'e' and 'e' is in an Unspecified Lvalue Context.
 static auto isInUnspecifiedLvalueContext(internal::Matcher innerMatcher) 
{
@@ -719,6 +724,46 @@ class Strategy {
 };
 } // namespace
 
+
+// Representing a pointer type expression of the form `++Ptr` in an Unspecified
+// Pointer Context (UPC):
+class UPCPreIncrementGadget : public FixableGadget {
+private:
+  static constexpr const char *const UPCPreIncrementTag =
+"PointerPreIncrementUnderUPC";
+  const UnaryOperator *Node; // the `++Ptr` node
+
+public:
+  UPCPreIncrementGadget(const MatchFinder::MatchResult )
+: FixableGadget(Kind::UPCPreIncrement),
+  Node(Result.Nodes.getNodeAs(UPCPreIncrementTag)) {
+assert(Node != nullptr && "Expecting a non-null matching result");
+  }
+
+  static bool classof(const Gadget *G) {
+return G->getKind() == Kind::UPCPreIncrement;
+  }
+
+  static Matcher matcher() {
+// Note here we match `++Ptr` for any expression `Ptr` of pointer type.
+// Although currently we can only provide fix-its when `Ptr` is a DRE, we
+// can have the matcher be general, so long as `getClaimedVarUseSites` does
+// things right.
+return stmt(isInUnspecifiedPointerContext(expr(ignoringImpCasts(
+   
unaryOperator(isPreInc(),
+   
  hasUnaryOperand(declRefExpr())
+   
  ).bind(UPCPreIncrementTag);
+  }
+
+  virtual std::optional getFixits(const Strategy ) const override;
+
+  virtual const Stmt *getBaseStmt() const override { return Node; }
+
+  virtual DeclUseList getClaimedVarUseSites() const override {
+return {dyn_cast(Node->getSubExpr())};
+  }
+};
+
 // Representing a fixable expression of the form `*(ptr + 123)` or `*(123 +
 // ptr)`:
 class DerefSimplePtrArithFixableGadget : public FixableGadget {
@@ -1196,6 +1241,36 @@ fixUPCAddressofArraySubscriptWithSpan(const 
UnaryOperator *Node) {
   FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())};
 }
 
+
+std::optional UPCPreIncrementGadget::getFixits(const Strategy ) 
const {
+  DeclUseList DREs = 

[PATCH] D146987: [Assignment Tracking] Enable by default

[Arthur Eubanks (out until mid-April) via Phabricator via cfe-commits]

aeubanks added a comment.

I'm seeing

  llc: ../../llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6214: void 
llvm::SelectionDAGBuilder::visitIntrinsicCall(const CallInst &, unsigned int): 
Assertion `AssignmentTrackingEnabled && "expected assignment tracking to be 
enabled"' failed.   
 

with this patch on the following IR
F27123156: b.ll.txt 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D141451#4258031 , @aaron.ballman 
wrote:

> Have you checked to see how the diagnostics work in practice on deep inlining 
> stacks? If it's usually only 1-2 extra notes, that's probably not going to 
> overwhelm anyone. But if it's significantly more, that could be distracting. 
> Do we want any options to control when to emit the notes? e.g., do we want to 
> give users a way to say "only emit the last N inlining decision notes"?

I don't have a good means of aggregating such data, but in my kernel experience 
we only ever emit about 2-4 notes before we stop inlining.  It is usually much 
less than the inclusion chain of headers reported.  Example (I've manually 
added a bad memcmp to kernel sources):

  In file included from drivers/nfc/nfcmrvl/fw_dnld.c:8:
  In file included from ./include/linux/module.h:13:
  In file included from ./include/linux/stat.h:19:
  In file included from ./include/linux/time.h:60:
  In file included from ./include/linux/time32.h:13:
  In file included from ./include/linux/timex.h:67:
  In file included from ./arch/x86/include/asm/timex.h:5:
  In file included from ./arch/x86/include/asm/processor.h:23:
  In file included from ./arch/x86/include/asm/msr.h:11:
  In file included from ./arch/x86/include/asm/cpumask.h:5:
  In file included from ./include/linux/cpumask.h:12:
  In file included from ./include/linux/bitmap.h:11:
  In file included from ./include/linux/string.h:254:
  ./include/linux/fortify-string.h:661:4: error: call to '__read_overflow2' 
declared with 'error' attribute: detected read beyond size of object (2nd 
parameter)
  __read_overflow2();
  ^
  note: In function 'fw_dnld_rx_work'
  note:   which inlined function 'process_state_reset'
  note:   which inlined function 'memcmp(void const*, void const* 
pass_dynamic_object_size0, unsigned long pass_dynamic_object_size0)'
  1 error generated.

Can you construct come "cuckoo-bananas" cases with this though? Yes: 
https://godbolt.org/z/9eP8vjeb8

> Also, how does this work with LTO when it makes different inlining decisions?

The concern with LTO is less about a change in inlining decisions, and more so 
about loss of fidelity with corresponding SourceLocations.

Testing the same kernel code same above, but with LTO enabled, we simply get a 
backend diagnostic from LLD, since the sources and thus SourceLocations are not 
retained:

  ld.lld: error: call to __read_overflow2 marked "dontcall-error": detected 
read beyond size of object (2nd parameter)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The latest reland 3820e9a2b29a2e268319ed6635da0d59e18d736d 
 still 
caused some issues to our internal workloads. Stay tuned...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[clang] aa1d269 - [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Paul Kirth via cfe-commits]

Author: Paul Kirth
Date: 2023-04-12T21:06:22Z
New Revision: aa1d2693c25622ea4a8ee2b622ba2a617e18ef88

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

LOG: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

ShadowCallStack implementation uses s2 register on RISC-V, but that
choice is problematic for reasons described in:

https://lists.riscv.org/g/sig-toolchains/message/544,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/370, and
https://github.com/google/android-riscv64/issues/72

The concern over the register choice was also brought up in
https://reviews.llvm.org/D84414.

https://reviews.llvm.org/D84414#2228666 said:

```
  "If the register choice is the only concern about this work, then I think
  we can probably land it as-is and fixup the register choice if we see
  major drawbacks later. Yes, it's an ABI issue, but on the other hand the
  shadow call stack is not a standard ABI anyway.""
```

Since we have now found a sufficient reason to fixup the register
choice, we should go ahead and update the implementation. We propose
using x3(gp) which is now the platform register in the RISC-V ABI.

Reviewed By: asb, hiraditya, mcgrathr, craig.topper

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

Added: 
llvm/test/CodeGen/RISCV/saverestore-scs.ll

Modified: 
clang/docs/ShadowCallStack.rst
clang/lib/Driver/SanitizerArgs.cpp
clang/test/Driver/sanitizer-ld.c
compiler-rt/test/shadowcallstack/lit.cfg.py
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/TargetParser/RISCVTargetParser.h
llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
llvm/lib/Target/RISCV/RISCVSubtarget.cpp
llvm/lib/TargetParser/RISCVTargetParser.cpp
llvm/test/CodeGen/RISCV/reserved-regs.ll
llvm/test/CodeGen/RISCV/shadowcallstack.ll

Removed: 




diff  --git a/clang/docs/ShadowCallStack.rst b/clang/docs/ShadowCallStack.rst
index b1ab4c6e8b239..04c04d259d7dc 100644
--- a/clang/docs/ShadowCallStack.rst
+++ b/clang/docs/ShadowCallStack.rst
@@ -57,24 +57,27 @@ compiled application or the operating system. Integrating 
the runtime into
 the operating system should be preferred since otherwise all thread creation
 and destruction would need to be intercepted by the application.
 
-The instrumentation makes use of the platform register ``x18``.  On some
-platforms, ``x18`` is reserved, and on others, it is designated as a scratch
-register.  This generally means that any code that may run on the same thread
-as code compiled with ShadowCallStack must either target one of the platforms
-whose ABI reserves ``x18`` (currently Android, Darwin, Fuchsia and Windows)
-or be compiled with the flag ``-ffixed-x18``. If absolutely necessary, code
-compiled without ``-ffixed-x18`` may be run on the same thread as code that
-uses ShadowCallStack by saving the register value temporarily on the stack
-(`example in Android`_) but this should be done with care since it risks
-leaking the shadow call stack address.
+The instrumentation makes use of the platform register ``x18`` on AArch64 and
+``x3`` (``gp``) on RISC-V. For simplicity we will refer to this as the
+``SCSReg``. On some platforms, ``SCSReg`` is reserved, and on others, it is
+designated as a scratch register.  This generally means that any code that may
+run on the same thread as code compiled with ShadowCallStack must either target
+one of the platforms whose ABI reserves ``SCSReg`` (currently Android, Darwin,
+Fuchsia and Windows) or be compiled with a flag to reserve that register (e.g.,
+``-ffixed-x18``). If absolutely necessary, code compiled without reserving the
+register may be run on the same thread as code that uses ShadowCallStack by
+saving the register value temporarily on the stack (`example in Android`_) but
+this should be done with care since it risks leaking the shadow call stack
+address.
 
 .. _`example in Android`: 
https://android-review.googlesource.com/c/platform/frameworks/base/+/803717
 
-Because of the use of register ``x18``, the ShadowCallStack feature is
-incompatible with any other feature that may use ``x18``. However, there
-is no inherent reason why ShadowCallStack needs to use register ``x18``
-specifically; in principle, a platform could choose to reserve and use another
-register for ShadowCallStack, but this would be incompatible with the AAPCS64.
+Because it requires a dedicated register, the ShadowCallStack feature is
+incompatible with any other feature that may use ``SCSReg``. However, there is
+no inherent reason why ShadowCallStack needs to use a specific register; in
+principle, a platform could choose to reserve and use another register for
+ShadowCallStack, but this would be incompatible with the ABI standards

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Paul Kirth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaa1d2693c256: [CodeGen][RISCV] Change Shadow Call Stack 
Register to X3 (authored by paulkirth).

Changed prior to commit:
  https://reviews.llvm.org/D146463?vs=512489=512972#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

Files:
  clang/docs/ShadowCallStack.rst
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/shadowcallstack/lit.cfg.py
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll
  llvm/test/CodeGen/RISCV/saverestore-scs.ll
  llvm/test/CodeGen/RISCV/shadowcallstack.ll

Index: llvm/test/CodeGen/RISCV/shadowcallstack.ll
===
--- llvm/test/CodeGen/RISCV/shadowcallstack.ll
+++ llvm/test/CodeGen/RISCV/shadowcallstack.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV32
-; RUN: llc -mtriple=riscv64 -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV64
 
 define void @f1() shadowcallstack {
@@ -34,9 +34,9 @@
 define i32 @f3() shadowcallstack {
 ; RV32-LABEL: f3:
 ; RV32:   # %bb.0:
-; RV32-NEXT:sw ra, 0(s2)
-; RV32-NEXT:addi s2, s2, 4
-; RV32-NEXT:.cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c #
+; RV32-NEXT:sw ra, 0(gp)
+; RV32-NEXT:addi gp, gp, 4
+; RV32-NEXT:.cfi_escape 0x16, 0x03, 0x02, 0x73, 0x7c #
 ; RV32-NEXT:addi sp, sp, -16
 ; RV32-NEXT:.cfi_def_cfa_offset 16
 ; RV32-NEXT:sw ra, 12(sp) # 4-byte Folded Spill
@@ -44,16 +44,16 @@
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:addi sp, sp, 16
-; RV32-NEXT:lw ra, -4(s2)
-; RV32-NEXT:addi s2, s2, -4
-; RV32-NEXT:.cfi_restore s2
+; RV32-NEXT:lw ra, -4(gp)
+; RV32-NEXT:addi gp, gp, -4
+; RV32-NEXT:.cfi_restore gp
 ; RV32-NEXT:ret
 ;
 ; RV64-LABEL: f3:
 ; RV64:   # %bb.0:
-; RV64-NEXT:sd ra, 0(s2)
-; RV64-NEXT:addi s2, s2, 8
-; RV64-NEXT:.cfi_escape 0x16, 0x12, 0x02, 0x82, 0x78 #
+; RV64-NEXT:sd ra, 0(gp)
+; RV64-NEXT:addi gp, gp, 8
+; RV64-NEXT:.cfi_escape 0x16, 0x03, 0x02, 0x73, 0x78 #
 ; RV64-NEXT:addi sp, sp, -16
 ; RV64-NEXT:.cfi_def_cfa_offset 16
 ; RV64-NEXT:sd ra, 8(sp) # 8-byte Folded Spill
@@ -61,9 +61,9 @@
 ; RV64-NEXT:call bar@plt
 ; RV64-NEXT:ld ra, 8(sp) # 8-byte Folded Reload
 ; RV64-NEXT:addi sp, sp, 16
-; RV64-NEXT:ld ra, -8(s2)
-; RV64-NEXT:addi s2, s2, -8
-; RV64-NEXT:.cfi_restore s2
+; RV64-NEXT:ld ra, -8(gp)
+; RV64-NEXT:addi gp, gp, -8
+; RV64-NEXT:.cfi_restore gp
 ; RV64-NEXT:ret
   %res = call i32 @bar()
   %res1 = add i32 %res, 1
@@ -73,72 +73,72 @@
 define i32 @f4() shadowcallstack {
 ; RV32-LABEL: f4:
 ; RV32:   # %bb.0:
-; RV32-NEXT:sw ra, 0(s2)
-; RV32-NEXT:addi s2, s2, 4
-; RV32-NEXT:.cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c #
+; RV32-NEXT:sw ra, 0(gp)
+; RV32-NEXT:addi gp, gp, 4
+; RV32-NEXT:.cfi_escape 0x16, 0x03, 0x02, 0x73, 0x7c #
 ; RV32-NEXT:addi sp, sp, -16
 ; RV32-NEXT:.cfi_def_cfa_offset 16
 ; RV32-NEXT:sw ra, 12(sp) # 4-byte Folded Spill
 ; RV32-NEXT:sw s0, 8(sp) # 4-byte Folded Spill
 ; RV32-NEXT:sw s1, 4(sp) # 4-byte Folded Spill
-; RV32-NEXT:sw s3, 0(sp) # 4-byte Folded Spill
+; RV32-NEXT:sw s2, 0(sp) # 4-byte Folded Spill
 ; RV32-NEXT:.cfi_offset ra, -4
 ; RV32-NEXT:.cfi_offset s0, -8
 ; RV32-NEXT:.cfi_offset s1, -12
-; RV32-NEXT:.cfi_offset s3, -16
+; RV32-NEXT:.cfi_offset s2, -16
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:mv s0, a0
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:mv s1, a0
 ; RV32-NEXT:call bar@plt
-; RV32-NEXT:mv s3, a0
+; RV32-NEXT:mv s2, a0
 ; RV32-NEXT:call bar@plt
 ; RV32-NEXT:add s0, s0, s1
-; RV32-NEXT:add a0, s3, a0
+; RV32-NEXT:add a0, s2, a0
 ; RV32-NEXT:add a0, s0, a0
 ; RV32-NEXT:lw ra, 12(sp) # 4-byte Folded Reload
 ; RV32-NEXT:lw s0, 8(sp) # 4-byte Folded Reload
 ; RV32-NEXT:lw s1, 4(sp) # 4-byte Folded Reload
-; RV32-NEXT:lw s3, 0(sp) # 4-byte Folded Reload
+; RV32-NEXT:lw s2, 0(sp) # 4-byte Folded Reload
 ; RV32-NEXT:addi sp, sp, 16
-; RV32-NEXT:lw ra, -4(s2)
-; RV32-NEXT:addi s2, s2, -4
-; RV32-NEXT:.cfi_restore s2
+; RV32-NEXT:lw ra, -4(gp)
+; RV32-NEXT:addi gp, gp, 

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 512971.
zequanwu edited the summary of this revision.
zequanwu added a comment.

- Filed an github issue:
- Added release note.
- Updated commit message and summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CoverageMappingGen.cpp


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,19 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If either of these locations is invalid, something elsewhere in the
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not 
valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");
+
+// However, we can still recover without crashing.
+// If either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +637,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +705,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -248,6 +248,10 @@
 - Fix false-positive diagnostic issued for consteval initializers of temporary
   objects.
   (`#60286 `_)
+- Work around with a clang coverage crash which happens when visiting 
+  expressions/statements with invalid source locations in non-assert builds. 
+  Assert builds may still see assertions triggered from this.
+  (`#62105 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,19 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If either of these locations is invalid, something elsewhere in the
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");
+
+// However, we can still recover without crashing.
+// If either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +637,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +705,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -248,6 +248,10 @@
 - Fix false-positive diagnostic issued for consteval initializers of temporary
   objects.
   (`#60286 

[PATCH] D148034: [clang][driver] Disable GP relaxation with RISC-V ShadowCallStack

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148034#4260376 , @asb wrote:

> Will `--[no-]relax-gp` make its way into a minor gcc point release or do we 
> need to wait for the next major release?
>
> In terms of this breaking GNU users - isn't it the case that without this 
> option, they may get silently broken code when using the shadow call stack? 
> Breaking loudly and early seems preferable, though of course it would be best 
> if it's easily fixable by e.g. updating to a newer released binutils.

Yes, `-fsanitize=shadow-call-stack` using gp users will get silently broken 
code if linked with GNU ld, unless GNU ld is specified, or `-Wl,--no-relax` or 
`-Wl,--no-relax-gp` is specified.
This is an instance of the guideline proposed in 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/371/commits/bb0df41a4f2626fa039173c2a975039905dce99c

> For such platforms, care must be taken to ensure all code (compiler generated 
> or otherwise) avoids using gp in a way incompatible with the platform 
> specific purpose, and that global pointer relaxation is disabled in the 
> toolchain.

Personally I think most `-fsanitize=shadow-call-stack`  users do not use GNU 
ld, so this incompatibility is actually minor.

`-fsanitize=shadow-call-stack` is already a quite specific configuration. For 
GNU ld users, I think placing the burden more on user education is fine (sorry, 
just that we don't have better options).

We have experience that even when the linker option `--push-state` has been 
available in GNU ld for ~5 years, we don't use it in Clang Driver, since using 
the option in the default configuration will break old GNU ld users.

> One slight tweak might be to avoid adding `--no-relax-gp` if linker 
> relaxation is already disabled, though it's not going to matter once binutils 
> gets support for --no-relax-gp.

Compile actions and link actions can be separate. Unfortunately Clang Driver 
for the link action does not have sufficient information whether linker 
relaxation has been disabled for all input relocatable object files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148034

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

[PATCH] D145435: [clang-format] Choose style (file) from within code for use in IDEs

[bers via Phabricator via cfe-commits]

bersbersbers added a comment.

Alright, thanks for your opinions. I will probably maintain this feature in a 
private working copy and continue to advertise it (compare, e.g., 
https://github.com/llvm/llvm-project/issues/60917 and 
https://stackoverflow.com/q/20540017), hoping to convince more people that this 
feature should be implemented publicly :) Until then, thank you all for your 
consideration!


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

https://reviews.llvm.org/D145435

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Nick Desaulniers via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:860
+
+  Diags.Report(diag::note_fe_backend_in) << 
llvm::demangle(D.getCaller().str());
+

erichkeane wrote:
> Could we instead just make `demangle` take a `string_view` here?  It takes it 
> by const-ref, which shows that it doesn't really seem to need it to be a 
> string, so I would imagine this would be a minor refactor (to add such an 
> overload).
Do you mean `StringRef` rather than `std::string_view`?  The progammer's manual 
provides guidance on the former 
(https://llvm.org/docs/ProgrammersManual.html#the-stringref-class) but not that 
latter.

Otherwise is this what you had in mind?
```
diff --git a/llvm/include/llvm/Demangle/Demangle.h 
b/llvm/include/llvm/Demangle/Demangle.h
index 6133d0b95bbf..c47222f883e9 100644
--- a/llvm/include/llvm/Demangle/Demangle.h
+++ b/llvm/include/llvm/Demangle/Demangle.h
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 /// This is a llvm local version of __cxa_demangle. Other than the name and
@@ -68,7 +69,7 @@ char *dlangDemangle(const char *MangledName);
 /// \param MangledName - reference to string to demangle.
 /// \returns - the demangled string, or a copy of the input string if no
 /// demangling occurred.
-std::string demangle(const std::string );
+std::string demangle(const std::string_view MangledName);
 
 bool nonMicrosoftDemangle(const char *MangledName, std::string );
 
diff --git a/llvm/lib/Demangle/Demangle.cpp b/llvm/lib/Demangle/Demangle.cpp
index 9d128424cabf..408112b9248e 100644
--- a/llvm/lib/Demangle/Demangle.cpp
+++ b/llvm/lib/Demangle/Demangle.cpp
@@ -26,9 +26,10 @@ static bool isDLangEncoding(const std::string ) {
  MangledName[1] == 'D';
 }
 
-std::string llvm::demangle(const std::string ) {
+std::string llvm::demangle(const std::string_view MangledName) {
   std::string Result;
-  const char *S = MangledName.c_str();
+  std::string Mangled(MangledName);
+  const char *S = Mangled.c_str();
 
   if (nonMicrosoftDemangle(S, Result))
 return Result;
@@ -43,7 +44,7 @@ std::string llvm::demangle(const std::string ) {
 return Result;
   }
 
-  return MangledName;
+  return Mangled;
 }
 
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string ) {
```



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12
   foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh 
no foo}}
+ // expected-note@* {{In function 'baz'}}
   if (x())

aaron.ballman wrote:
> Instead of allowing this note to appear anywhere in the file, I think it's 
> better to use "bookmark" comments. e.g.,
> ```
> void baz() { // #baz_defn
> }
> 
> void foo() {
>   baz(); // expected-note@#baz_defn {{whatever note text}}
> }
> ```
> so that we're testing where the diagnostic is emitted. (This is especially 
> important given that the changes are about location tracking.)
oh, that's new (to me)! will do



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:29
+   // expected-note@* {{In function 'd'}}
+   // expected-note@* {{which inlined function 'b'}}
+   // expected-note@* {{which inlined function 'a'}}

erichkeane wrote:
> Same comment re-bookmarks, but this diagnostic seems awkward.  
> 
> Should we instead say :
> 
> `call to 'foo' declared with 'error' attribute: oh no foo`
> `called by function 'a'`
> `inlined by function 'b'`
> `inlined by function 'd'`?
> 
ah, yeah, I have this backwards (in v1, I had fixed this in the alt impl 
https://reviews.llvm.org/D145994). Let me fix that up here now, too. Good catch!



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:2467-2468
+
+const Function *Callee = CI->getCalledFunction();
+if (Callee && (Callee->hasFnAttribute("dontcall-error") ||
+   Callee->hasFnAttribute("dontcall-warn"))) {

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > arsenm wrote:
> > > > Misses constexpr casts and aliases
> > > The base feature doesn't work with aliases (or ConstantExpr), in GCC or 
> > > Clang.  I should perhaps fix that first...
> > Perhaps I should use Call.getCalledOperand()->stripPointerCasts() for 
> > constantexpr case.
> I've added support for ConstantExpr to the base feature in D142058.  I should 
> still fix this in this PR and add a test case, so not yet marking this thread 
> as done.
marking this thread as done; see other thread below. If someone adds an alias, 
maybe they _intend_ to call the warning/error attributed function for [[good 
reason]].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

[PATCH] D148034: [clang][driver] Disable GP relaxation with RISC-V ShadowCallStack

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D148034#4262938 , @jrtc27 wrote:

> In D148034#4262934 , @craig.topper 
> wrote:
>
>> I'm not sure I want to suggest this, but could we disable the emission of 
>> the relocations that can be GP relaxed?
>
> That would also lose the ability to do x0-relative and LUI -> C.LUI 
> relaxation... though given the former only has a simm12 field that's rather 
> limited in use.

I assume x0-relative wouldn't be useful for Android/Linux since nothing should 
be mapped to the zero page.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148034

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

[PATCH] D148034: [clang][driver] Disable GP relaxation with RISC-V ShadowCallStack

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D148034#4262934 , @craig.topper 
wrote:

> I'm not sure I want to suggest this, but could we disable the emission of the 
> relocations that can be GP relaxed?

That would also lose the ability to do x0-relative and LUI -> C.LUI 
relaxation... though given the former only has a simm12 field that's rather 
limited in use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148034

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

[PATCH] D148034: [clang][driver] Disable GP relaxation with RISC-V ShadowCallStack

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I'm not sure I want to suggest this, but could we disable the emission of the 
relocations that can be GP relaxed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148034

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

[PATCH] D147121: [hwasan] remove requirment for PIE

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka removed 1 blocking reviewer(s): eugenis.
vitalybuka added a comment.
This revision is now accepted and ready to land.

Feel free to land. It works on my arm linux setup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147121

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

[PATCH] D146101: [clang-format] Add BracedInitializerIndentWidth option.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

HazardyKnusperkeks wrote:
> jp4a50 wrote:
> > owenpan wrote:
> > > HazardyKnusperkeks wrote:
> > > > MyDeveloperDay wrote:
> > > > > rymiel wrote:
> > > > > > owenpan wrote:
> > > > > > > jp4a50 wrote:
> > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > owenpan wrote:
> > > > > > > > > > jp4a50 wrote:
> > > > > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > > > > owenpan wrote:
> > > > > > > > > > > > > jp4a50 wrote:
> > > > > > > > > > > > > > owenpan wrote:
> > > > > > > > > > > > > > > owenpan wrote:
> > > > > > > > > > > > > > > > Using -1 to mean `ContinuationIndentWidth` is 
> > > > > > > > > > > > > > > > unnecessary and somewhat confusing IMO.
> > > > > > > > > > > > > > > Please disregard my comment above.
> > > > > > > > > > > > > > Just to make sure we are on the same page, does 
> > > > > > > > > > > > > > this mean that you are happy with the approach of 
> > > > > > > > > > > > > > using `-1` as a default value to indicate that 
> > > > > > > > > > > > > > `ContinuationIndentWidth` should be used?
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > I did initially consider using 
> > > > > > > > > > > > > > `std::optional` and using empty optional 
> > > > > > > > > > > > > > to indicate that `ContinuationIndentWidth` should 
> > > > > > > > > > > > > > be used but I saw that `PPIndentWidth` was using 
> > > > > > > > > > > > > > `-1` to default to using `IndentWidth` so I 
> > > > > > > > > > > > > > followed that precedent.
> > > > > > > > > > > > > Yep! I would prefer the `optional`, but as you 
> > > > > > > > > > > > > pointed out, we already got `PPIndentWidth`using `-1`.
> > > > > > > > > > > > From the C++ side I totally agree. One could use 
> > > > > > > > > > > > `value_or()`, which would make the code much more 
> > > > > > > > > > > > readable.
> > > > > > > > > > > > And just because `PPIndentWidth` is using -1 is no 
> > > > > > > > > > > > reason to repeat that, we could just as easily change 
> > > > > > > > > > > > `PPIndentWidht` to an optional.
> > > > > > > > > > > > 
> > > > > > > > > > > > But how would it look in yaml?
> > > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > > 
> > > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the 
> > > > > > > > > > > YAML then the optional would simply not be set during 
> > > > > > > > > > > parsing.
> > > > > > > > > > > 
> > > > > > > > > > > Of course, if we were to change `PPIndentWidth` to work 
> > > > > > > > > > > that way too, it would technically be a breaking change 
> > > > > > > > > > > because users may have *explicitly* specified `-1` in 
> > > > > > > > > > > their YAML.
> > > > > > > > > > > And just because `PPIndentWidth` is using -1 is no reason 
> > > > > > > > > > > to repeat that, we could just as easily change 
> > > > > > > > > > > `PPIndentWidht` to an optional.
> > > > > > > > > > 
> > > > > > > > > > We would have to deal with backward compatibility to avoid 
> > > > > > > > > > regressions though.
> > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > 
> > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the 
> > > > > > > > > > YAML then the optional would simply not be set during 
> > > > > > > > > > parsing.
> > > > > > > > > > 
> > > > > > > > > > Of course, if we were to change `PPIndentWidth` to work 
> > > > > > > > > > that way too, it would technically be a breaking change 
> > > > > > > > > > because users may have *explicitly* specified `-1` in their 
> > > > > > > > > > YAML.
> > > > > > > > > 
> > > > > > > > > You need an explicit entry, because you need to be able to 
> > > > > > > > > write the empty optional on `--dump-config`.
> > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > 
> > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > 

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via cfe-commits]

kpdev42 updated this revision to Diff 512945.
kpdev42 added a comment.

Thanks for pointing out the bug @Michael137 . It seems that clang assigns 
arbitrary offsets for non_unique_address so analyzing them brings me nowhere. 
In this patch I tried assigning no_unique_address to every empty structure and 
this fixed issue you found, making the code changes much smaller and simpler. 
The lldb test suite pass for me as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/no_unique_address/Makefile
  lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
  lldb/test/API/lang/cpp/no_unique_address/main.cpp

Index: lldb/test/API/lang/cpp/no_unique_address/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/main.cpp
@@ -0,0 +1,77 @@
+struct C
+{
+ long c,d;
+};
+
+struct Q
+{
+ long h;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct E
+{
+  [[no_unique_address]] D x;
+};
+
+struct Foo1 : B,E,C
+{
+ long a = 42,b = 52;
+} _f1;
+
+struct Foo2 : B,E
+{
+ long v = 42;
+} _f2;
+
+struct Foo3 : C,B,E
+{
+ long v = 42;
+} _f3;
+
+struct Foo4 : B,C,E,Q
+{
+ long v = 42;
+} _f4;
+
+struct Foo5 : B,C,E
+{
+ [[no_unique_address]] D x1;
+ [[no_unique_address]] D x2;
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+ [[no_unique_address]] D z1;
+ [[no_unique_address]] D z2;
+} _f5;
+
+struct Foo6 : B,E
+{
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+} _f6;
+
+namespace NS {
+template struct Wrap {};
+}
+
+struct Foo7 : NS::Wrap,B,E {
+  NS::Wrap w1;
+  B b1;
+  long v = 42;
+} _f7;
+
+int main() {
+  return 0; // Set breakpoint here.
+}
Index: lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -0,0 +1,33 @@
+"""
+Test that we correctly handle [[no_unique_address]] attribute.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestInlineNamespace(TestBase):
+
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set breakpoint here.", lldb.SBFileSpec("main.cpp"))
+
+
+self.expect_expr("_f3", result_type="Foo3")
+self.expect_expr("_f7", result_type="Foo7")
+
+self.expect_expr("_f1.a", result_type="long", result_value="42")
+self.expect_expr("_f1.b", result_type="long", result_value="52")
+self.expect_expr("_f2.v", result_type="long", result_value="42")
+self.expect_expr("_f3.v", result_type="long", result_value="42")
+self.expect_expr("_f4.v", result_type="long", result_value="42")
+self.expect_expr("_f5.v1", result_type="long", result_value="42")
+self.expect_expr("_f5.v2", result_type="long", result_value="52")
+self.expect_expr("_f6.v1", result_type="long", result_value="42")
+self.expect_expr("_f6.v2", result_type="long", result_value="52")
+self.expect_expr("_f7.v", result_type="long", result_value="42")
Index: lldb/test/API/lang/cpp/no_unique_address/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1566,6 +1566,16 @@
   return qualified_name;
 }
 
+static bool IsEmptyStruct(const DWARFDIE ) {
+  if (!die.HasChildren())
+return true;
+
+  // Empty templates are actually empty structures.
+  return llvm::all_of(die.children(), [](const DWARFDIE ) {
+return die.Tag() == DW_TAG_template_type_parameter;
+  });
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext ,
const DWARFDIE ,
@@ -1829,7 +1839,7 @@
 // has child classes or types that require the class to be created
 // for use as their decl contexts the class will be ready to accept
 // these child definitions.
-if (!die.HasChildren()) {
+if (IsEmptyStruct(die)) {
   // No children for this struct/union/class, lets finish it
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
@@ -1852,6 +1862,9 @@
 clang::RecordDecl *record_decl =

[PATCH] D148148: [clang] Bump AS_GNU to 1

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm updated this revision to Diff 512947.
rsandifo-arm added a comment.

Gah, sorry.  Due to a botched git operation, I posted a first cut with a stupid 
typo, rather than the version that passed testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148148

Files:
  clang/include/clang/Basic/AttributeCommonInfo.h


Index: clang/include/clang/Basic/AttributeCommonInfo.h
===
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -25,7 +25,7 @@
   /// The style used to specify an attribute.
   enum Syntax {
 /// __attribute__((...))
-AS_GNU,
+AS_GNU = 1,
 
 /// [[...]]
 AS_CXX11,
@@ -122,37 +122,32 @@
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Form FormUsed)
+  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
   : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc),
-AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
+ScopeLoc(ScopeLoc), AttrKind(AttrKind),
 SyntaxUsed(FormUsed.getSyntax()),
 SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+IsAlignas(FormUsed.isAlignas()) {
+assert(SyntaxUsed >= AS_GNU && SyntaxUsed <= AS_Implicit &&
+   "Invalid syntax!");
+  }
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
-  : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc), AttrKind(AttrKind),
-SyntaxUsed(FormUsed.getSyntax()),
-SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+  SourceLocation ScopeLoc, Form FormUsed)
+  : AttributeCommonInfo(
+AttrName, ScopeName, AttrRange, ScopeLoc,
+getParsedKind(AttrName, ScopeName, FormUsed.getSyntax()),
+FormUsed) {}
 
   AttributeCommonInfo(const IdentifierInfo *AttrName, SourceRange AttrRange,
   Form FormUsed)
-  : AttrName(AttrName), ScopeName(nullptr), AttrRange(AttrRange),
-ScopeLoc(),
-AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
-SyntaxUsed(FormUsed.getSyntax()),
-SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+  : AttributeCommonInfo(AttrName, nullptr, AttrRange, SourceLocation(),
+FormUsed) {}
 
   AttributeCommonInfo(SourceRange AttrRange, Kind K, Form FormUsed)
-  : AttrName(nullptr), ScopeName(nullptr), AttrRange(AttrRange), 
ScopeLoc(),
-AttrKind(K), SyntaxUsed(FormUsed.getSyntax()),
-SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+  : AttributeCommonInfo(nullptr, nullptr, AttrRange, SourceLocation(), K,
+FormUsed) {}
 
   AttributeCommonInfo(AttributeCommonInfo &&) = default;
   AttributeCommonInfo(const AttributeCommonInfo &) = default;


Index: clang/include/clang/Basic/AttributeCommonInfo.h
===
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -25,7 +25,7 @@
   /// The style used to specify an attribute.
   enum Syntax {
 /// __attribute__((...))
-AS_GNU,
+AS_GNU = 1,
 
 /// [[...]]
 AS_CXX11,
@@ -122,37 +122,32 @@
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Form FormUsed)
+  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
   : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc),
-AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
+ScopeLoc(ScopeLoc), AttrKind(AttrKind),
 SyntaxUsed(FormUsed.getSyntax()),
 SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+IsAlignas(FormUsed.isAlignas()) {
+assert(SyntaxUsed >= AS_GNU && SyntaxUsed <= AS_Implicit &&
+   "Invalid syntax!");
+  }
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
-  : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc), 

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Instantiation pattern is null for incomplete template types and using
specializaiton decl results in not seeing re-declarations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148158

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -144,6 +144,9 @@
   ElementsAre(Decl::CXXRecord));
 
   // Implicit instantiations references most relevant template.
+  EXPECT_THAT(
+  testWalk("template struct $explicit^Foo {};", "^Foo 
x();"),
+  ElementsAre(Decl::Kind::ClassTemplate));
   EXPECT_THAT(
   testWalk("template struct $explicit^Foo {};", "^Foo x;"),
   ElementsAre(Decl::CXXRecord));
@@ -154,9 +157,15 @@
   ElementsAre(Decl::ClassTemplateSpecialization));
   EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
-template struct $explicit^Foo { void x(); };)cpp",
+template struct $explicit^Foo {};)cpp",
"^Foo x;"),
   ElementsAre(Decl::ClassTemplatePartialSpecialization));
+  // Incomplete templates don't have a specific specialization associated.
+  EXPECT_THAT(testWalk(R"cpp(
+template struct $explicit^Foo {};
+template struct Foo {};)cpp",
+   "^Foo x();"),
+  ElementsAre(Decl::Kind::ClassTemplate));
   EXPECT_THAT(testWalk(R"cpp(
 template struct $explicit^Foo {};
 template struct Foo;)cpp",
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/Casting.h"
@@ -83,7 +84,7 @@
 // This is the underlying decl used by TemplateSpecializationType, can be
 // null when type is dependent if so fallback to primary template.
 CXXRecordDecl *TD = TST->getAsCXXRecordDecl();
-if (!TD)
+if (!TD || TD->getTemplateSpecializationKind() == TSK_Undeclared)
   return ND;
 // We ignore explicit instantiations. This might imply marking the wrong
 // declaration as used in specific cases, but seems like the right 
trade-off


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -144,6 +144,9 @@
   ElementsAre(Decl::CXXRecord));
 
   // Implicit instantiations references most relevant template.
+  EXPECT_THAT(
+  testWalk("template struct $explicit^Foo {};", "^Foo x();"),
+  ElementsAre(Decl::Kind::ClassTemplate));
   EXPECT_THAT(
   testWalk("template struct $explicit^Foo {};", "^Foo x;"),
   ElementsAre(Decl::CXXRecord));
@@ -154,9 +157,15 @@
   ElementsAre(Decl::ClassTemplateSpecialization));
   EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
-template struct $explicit^Foo { void x(); };)cpp",
+template struct $explicit^Foo {};)cpp",
"^Foo x;"),
   ElementsAre(Decl::ClassTemplatePartialSpecialization));
+  // Incomplete templates don't have a specific specialization associated.
+  EXPECT_THAT(testWalk(R"cpp(
+template struct $explicit^Foo {};
+template struct Foo {};)cpp",
+   "^Foo x();"),
+  ElementsAre(Decl::Kind::ClassTemplate));
   EXPECT_THAT(testWalk(R"cpp(
 template struct $explicit^Foo {};
 template struct Foo;)cpp",
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/Support/Casting.h"
@@ -83,7 +84,7 @@
 // This is the underlying decl used by TemplateSpecializationType, can be
 // null when type is dependent if so fallback to primary template.
 

[PATCH] D147121: [hwasan] remove requirment for PIE

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Sorry, I do not remember why this requirement is there. Must be related to 
shadow / allocator placement and kernel mapping conflicts, but hwasan is using 
dynamic shadow so that should not be an issue... LGTM as long as it works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147121

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

[PATCH] D141307: Add -f[no-]loop-versioning option

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski accepted this revision.
awarzynski added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141307

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

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added subscribers: thieta, saudi.
aganea added a comment.

+ @saudi @thieta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rG144562e678d9: [clangd] Treat preamble patch as main file for 
include-cleaner analysis (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D148143?vs=512898=512932#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148143

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/unittests/PreambleTests.cpp

Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -666,6 +666,7 @@
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
   Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+  Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
@@ -699,6 +700,8 @@
   {
 Annotations Code("#define [[FOO]] 1\n");
 // Check ranges for notes.
+// This also makes sure we don't generate missing-include diagnostics
+// because macros are redefined in preamble-patch.
 Annotations NewCode(R"(#define BARXYZ 1
 #define $foo1[[FOO]] 1
 void foo();
@@ -866,6 +869,27 @@
   }
 }
 
+TEST(PreamblePatch, PatchFileEntry) {
+  Annotations Code(R"cpp(#define FOO)cpp");
+  Annotations NewCode(R"cpp(
+#define BAR
+#define FOO)cpp");
+  {
+auto AST = createPatchedAST(Code.code(), Code.code());
+EXPECT_EQ(
+PreamblePatch::getPatchEntry(AST->tuPath(), AST->getSourceManager()),
+nullptr);
+  }
+  {
+auto AST = createPatchedAST(Code.code(), NewCode.code());
+auto *FE =
+PreamblePatch::getPatchEntry(AST->tuPath(), AST->getSourceManager());
+ASSERT_NE(FE, nullptr);
+EXPECT_THAT(FE->getName().str(),
+testing::EndsWith(PreamblePatch::HeaderName.str()));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -30,6 +30,7 @@
 #include "clang-include-cleaner/Record.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/Lexer.h"
@@ -37,8 +38,11 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 
+#include 
+#include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -135,6 +139,10 @@
   static PreamblePatch createMacroPatch(llvm::StringRef FileName,
 const ParseInputs ,
 const PreambleData );
+  /// Returns the FileEntry for the preamble patch of MainFilePath in SM, if
+  /// any.
+  static const FileEntry *getPatchEntry(llvm::StringRef MainFilePath,
+const SourceManager );
 
   /// Adjusts CI (which compiles the modified inputs) to be used with the
   /// baseline preamble. This is done by inserting an artifical include to the
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -738,6 +738,14 @@
   return PatchedDiags;
 }
 
+static std::string getPatchName(llvm::StringRef FileName) {
+  // This shouldn't coincide with any real file name.
+  llvm::SmallString<128> PatchName;
+  llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
+  PreamblePatch::HeaderName);
+  return PatchName.str().str();
+}
+
 PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
 const ParseInputs ,
 const PreambleData ,
@@ -776,11 +784,7 @@
 
   PreamblePatch PP;
   PP.Baseline = 
-  // This shouldn't coincide with any real file name.
-  llvm::SmallString<128> PatchName;
-  llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
-  PreamblePatch::HeaderName);
-  PP.PatchFileName = PatchName.str().str();
+  PP.PatchFileName = getPatchName(FileName);
   PP.ModifiedBounds = ModifiedScan->Bounds;
 
   llvm::raw_string_ostream Patch(PP.PatchContents);
@@ -921,5 +925,13 @@
 return Baseline->Macros;
   return PatchedMacros;
 }
+
+const FileEntry *PreamblePatch::getPatchEntry(llvm::StringRef MainFilePath,
+

[clang-tools-extra] 144562e - [clangd] Treat preamble patch as main file for include-cleaner analysis

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-04-12T21:03:21+02:00
New Revision: 144562e678d932dc2685f8a83aeb2229bc96d71a

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

LOG: [clangd] Treat preamble patch as main file for include-cleaner analysis

Since we redefine all macros in preamble-patch, and it's parsed after
consuming the preamble macros, we can get false missing-include diagnostics
while a fresh preamble is being rebuilt.

This patch makes sure preamble-patch is treated same as main file for
include-cleaner purposes.

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

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index ee645efa6e6e1..e645b1cce6a09 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "ParsedAST.h"
+#include "Preamble.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -112,12 +113,12 @@ static bool mayConsiderUnused(const Inclusion , 
ParsedAST ,
   return false;
 // Check if main file is the public interface for a private header. If so 
we
 // shouldn't diagnose it as unused.
-if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
   PHeader = PHeader.trim("<>\"");
   // Since most private -> public mappings happen in a verbatim way, we
   // check textually here. This might go wrong in presence of symlinks or
   // header mappings. But that's not 
diff erent than rest of the places.
-  if(AST.tuPath().endswith(PHeader))
+  if (AST.tuPath().endswith(PHeader))
 return false;
 }
   }
@@ -360,6 +361,7 @@ IncludeCleanerFindings 
computeIncludeCleanerFindings(ParsedAST ) {
   include_cleaner::Includes ConvertedIncludes =
   convertIncludes(SM, Includes.MainFileIncludes);
   const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
+  auto *PreamblePatch = PreamblePatch::getPatchEntry(AST.tuPath(), SM);
 
   std::vector Macros =
   collectMacroReferences(AST);
@@ -374,7 +376,7 @@ IncludeCleanerFindings 
computeIncludeCleanerFindings(ParsedAST ) {
 bool Satisfied = false;
 for (const auto  : Providers) {
   if (H.kind() == include_cleaner::Header::Physical &&
-  H.physical() == MainFile) {
+  (H.physical() == MainFile || H.physical() == PreamblePatch)) {
 Satisfied = true;
 continue;
   }

diff  --git a/clang-tools-extra/clangd/Preamble.cpp 
b/clang-tools-extra/clangd/Preamble.cpp
index 08662697a4a5c..ea060bed03922 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -738,6 +738,14 @@ static std::vector patchDiags(llvm::ArrayRef 
BaselineDiags,
   return PatchedDiags;
 }
 
+static std::string getPatchName(llvm::StringRef FileName) {
+  // This shouldn't coincide with any real file name.
+  llvm::SmallString<128> PatchName;
+  llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
+  PreamblePatch::HeaderName);
+  return PatchName.str().str();
+}
+
 PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
 const ParseInputs ,
 const PreambleData ,
@@ -776,11 +784,7 @@ PreamblePatch PreamblePatch::create(llvm::StringRef 
FileName,
 
   PreamblePatch PP;
   PP.Baseline = 
-  // This shouldn't coincide with any real file name.
-  llvm::SmallString<128> PatchName;
-  llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
-  PreamblePatch::HeaderName);
-  PP.PatchFileName = PatchName.str().str();
+  PP.PatchFileName = getPatchName(FileName);
   PP.ModifiedBounds = ModifiedScan->Bounds;
 
   llvm::raw_string_ostream Patch(PP.PatchContents);
@@ -921,5 +925,13 @@ const MainFileMacros ::mainFileMacros() 
const {
 return Baseline->Macros;
   return PatchedMacros;
 }
+
+const FileEntry *PreamblePatch::getPatchEntry(llvm::StringRef MainFilePath,
+  const SourceManager ) {
+  auto PatchFilePath = getPatchName(MainFilePath);
+  if (auto File = SM.getFileManager().getFile(PatchFilePath))
+return *File;
+  return nullptr;
+}
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/Preamble.h 
b/clang-tools-extra/clangd/Preamble.h
index 

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you! I think there's only a few small things left then:

- Add a release note about the fact that we worked around this issue in 
non-asserts builds but that assert builds may see new assertions triggered from 
this and to file an issue with a reproducer if you hit the assertion.
- File an issue with a simple reproducer for the case you know we're still not 
handling properly
- Update the commit message to clarify that we're no longer fixing anything 
with these changes but are instead trying to catch the issues are more loudly 
than with a crash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

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

[PATCH] D141307: Add -f[no-]loop-versioning option

[Mats Petersson via Phabricator via cfe-commits]

Leporacanthicus updated this revision to Diff 512929.
Leporacanthicus marked 3 inline comments as done.
Leporacanthicus added a comment.

- Add new function to help clarify the purpose of the options being used.
- Add descriptive comment to make function use clearer.
- Update tests to make them a more explicit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141307

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/CodeGenOptions.def
  flang/include/flang/Tools/CLOptions.inc
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Driver/version-loops.f90

Index: flang/test/Driver/version-loops.f90
===
--- /dev/null
+++ flang/test/Driver/version-loops.f90
@@ -0,0 +1,54 @@
+! Test that flang-new forwards the -f{no-,}version-loops-for-stride 
+! options correctly to flang-new -fc1 for different variants of optimisation
+! and explicit flags.
+
+! RUN: %flang -### %s -o %t 2>&1   -O3 \
+! RUN:   | FileCheck %s
+  
+! RUN: %flang -### %s -o %t 2>&1 -O2 \
+! RUN:   | FileCheck %s --check-prefix=CHECK-O2
+
+! RUN: %flang -### %s -o %t 2>&1  -O2 -fversion-loops-for-stride \
+! RUN:   | FileCheck %s --check-prefix=CHECK-O2-with
+  
+! RUN: %flang -### %s -o %t 2>&1  -O4 \
+! RUN:   | FileCheck %s --check-prefix=CHECK-O4
+  
+! RUN: %flang -### %s -o %t 2>&1  -Ofast \
+! RUN:   | FileCheck %s --check-prefix=CHECK-Ofast
+  
+! RUN: %flang -### %s -o %t 2>&1 -Ofast -fno-version-loops-for-stride \
+! RUN:   | FileCheck %s --check-prefix=CHECK-Ofast-no
+
+! RUN: %flang -### %s -o %t 2>&1 -O3 -fno-version-loops-for-stride \
+! RUN:   | FileCheck %s --check-prefix=CHECK-O3-no
+
+! CHECK: "{{.*}}flang-new" "-fc1"
+! CHECK-SAME: "-fversion-loops-for-stride"
+! CHECK-SAME: "-O3"
+
+! CHECK-O2: "{{.*}}flang-new" "-fc1"
+! CHECK-O2-NOT: "-fversion-loops-for-stride"
+! CHECK-O2-SAME: "-O2"  
+
+! CHECK-O2-with: "{{.*}}flang-new" "-fc1"
+! CHECK-O2-with-SAME: "-fversion-loops-for-stride"
+! CHECK-O2-with-SAME: "-O2"  
+  
+! CHECK-O4: "{{.*}}flang-new" "-fc1"
+! CHECK-O4-SAME: "-fversion-loops-for-stride"
+! CHECK-O4-SAME: "-O3"
+
+! CHECK-Ofast: "{{.*}}flang-new" "-fc1"
+! CHECK-Ofast-SAME: "-ffast-math"
+! CHECK-Ofast-SAME: "-fversion-loops-for-stride"
+! CHECK-Ofast-SAME: "-O3"
+
+! CHECK-Ofast-no: "{{.*}}flang-new" "-fc1"
+! CHECK-Ofast-no-SAME: "-ffast-math"
+! CHECK-Ofast-no-NOT: "-fversion-loops-for-stride"
+! CHECK-Ofast-no-SAME: "-O3"
+
+! CHECK-O3-no: "{{.*}}flang-new" "-fc1"
+! CHECK-O3-no-NOT: "-fversion-loops-for-stride"
+! CHECK-O3-no-SAME: "-O3"
Index: flang/test/Driver/frontend-forwarding.f90
===
--- flang/test/Driver/frontend-forwarding.f90
+++ flang/test/Driver/frontend-forwarding.f90
@@ -15,6 +15,8 @@
 ! RUN: -fassociative-math \
 ! RUN: -freciprocal-math \
 ! RUN: -fpass-plugin=Bye%pluginext \
+! RUN: -fversion-loops-for-stride \
+! RUN: -mllvm -print-before-all\
 ! RUN: -mllvm -print-before-all \
 ! RUN: -save-temps=obj \
 ! RUN: -P \
@@ -34,5 +36,6 @@
 ! CHECK: "-freciprocal-math"
 ! CHECK: "-fconvert=little-endian"
 ! CHECK: "-fpass-plugin=Bye
+! CHECK: "-fversion-loops-for-stride"  
 ! CHECK: "-mllvm" "-print-before-all"
 ! CHECK: "-save-temps=obj"
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -48,6 +48,8 @@
 ! HELP-NEXT: -fno-integrated-as  Disable the integrated assembler
 ! HELP-NEXT: -fno-signed-zeros  Allow optimizations that ignore the sign of floating point zeros
 ! HELP-NEXT: -fno-stack-arrays  Allocate array temporaries on the heap (default)
+! HELP-NEXT: -fno-version-loops-for-stride
+! HELP-NEXT:Do not create unit-strided loops (default)
 ! HELP-NEXT: -fopenacc  Enable OpenACC
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fpass-plugin= Load pass plugin from a dynamic shared object file (only with new pass manager).
@@ -55,6 +57,8 @@
 ! HELP-NEXT: -fstack-arrays Attempt to allocate array temporaries on the stack, no matter their size
 ! HELP-NEXT: -fsyntax-only  Run the preprocessor, parser and semantic analysis stages
 ! HELP-NEXT: -funderscoring Appends one trailing underscore to external names
+! HELP-NEXT: -fversion-loops-for-stride
+! HELP-NEXT:Create unit-strided versions of loops
 ! HELP-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: -gline-tables-only Emit 

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/attr-riscv-rvv-vector-bits.cpp:12
+
+template struct S { T var; };
+

craig.topper wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > craig.topper wrote:
> > > > erichkeane wrote:
> > > > > craig.topper wrote:
> > > > > > @erichkeane does this cover the dependent case or were you looking 
> > > > > > for something else?
> > > > > > 
> > > > > > Here are on the only mentions of template I see in SVE tests that 
> > > > > > use this attribute.
> > > > > > 
> > > > > > ```
> > > > > > clang/test$ ack template `ack arm_sve_vector -l`
> > > > > > CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
> > > > > > 37:template  struct S {};
> > > > > > 
> > > > > > SemaCXX/attr-arm-sve-vector-bits.cpp
> > > > > > 16:template struct S { T var; };
> > > > > > ```
> > > > > > 
> > > > > > Here is the result for this patch
> > > > > > 
> > > > > > ```
> > > > > > clang/test$ ack template `ack riscv_rvv_vector -l`
> > > > > > CodeGenCXX/riscv-mangle-rvv-fixed-vectors.cpp
> > > > > > 48:template  struct S {};
> > > > > > 
> > > > > > SemaCXX/attr-riscv-rvv-vector-bits.cpp
> > > > > > 12:template struct S { T var; };
> > > > > > ```
> > > > > Thats unfortunate, and I wish I'd thought of it at the time/been more 
> > > > > active reviewing the SVE stuff then.  Really what I'm looking for is:
> > > > > 
> > > > > ```
> > > > > template 
> > > > > struct Whatever {
> > > > >   using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > > > > };
> > > > > 
> > > > > void Func(Whatever<5>::Something MyVar){}
> > > > > 
> > > > > ```
> > > > That does not appear to work.
> > > > 
> > > > ```
> > > > $ ./bin/clang test.cpp --target=riscv64 -march=rv64gcv 
> > > > -mrvv-vector-bits=zvl
> > > > test.cpp:3:41: error: 'riscv_rvv_vector_bits' attribute requires an 
> > > > integer constant
> > > > using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > > > ```
> > > > 
> > > > It's not very useful as a template parameter. There's only one value 
> > > > that works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> > > Thats really unfortunate, but it makes me wonder what 
> > > `DependentVectorType ` is for in this case, or the handling of said 
> > > things.  Because I would expect:
> > > 
> > > ```
> > > template
> > > using RiscvVector = T __attribute__((risv_rvv_vector_bits(Size)));
> > > 
> > > RiscvVector> Foo;
> > > ```
> > > to be useful.  Even if not, I'd expect:
> > > ```
> > > template
> > > using RiscvVector = T 
> > > __attribute__((risv_rvv_vector_bits(TheRightAnswer)));
> > > RiscvVector Foo;
> > > ```
> > > to both work.
> > > 
> > > >>It's not very useful as a template parameter. There's only one value 
> > > >>that works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> > > This makes me wonder why this attribute takes an integer constant anyway, 
> > > if it is just a 'guess what the right answer is!' sorta thing.  Seems to 
> > > me this never should have taken a parameter.
> > > It's not very useful as a template parameter. There's only one value that 
> > > works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> > 
> > Can you help me understand why the argument exists then?
> > 
> > We're pretty inconsistent about attribute arguments properly handling 
> > things like constant expressions vs integer literals, but the trend lately 
> > is to accept a constant expression rather than only a literal because of 
> > how often users like to give names to literals and how much more constexpr 
> > code we're seeing in the wild.
> This is what's in ARM's ACLE documentation:
> 
> 
> 
> > The ACLE only defines the effect of the attribute if all of the following 
> > are true:
> > 1. the attribute is attached to a single SVE vector type (such as 
> > svint32_t) or to the SVE predicate
> > type svbool_t;
> > 2. the arguments “…” consist of a single nonzero integer constant 
> > expression (referred to as N below); and
> > 3. N==__ARM_FEATURE_SVE_BITS.
> > In other cases the implementation must do one of the following:
> > • ignore the attribute; a warning would then be appropriate, but is not 
> > required
> > • reject the program with a diagnostic
> > • extend requirement (3) above to support other values of N besides 
> > __ARM_FEATURE_SVE_BITS
> > • process the attribute in accordance with a later revision of the ACLE
> 
> 
> 
> 
> 
> So there's a bullet in there that allows an implementation to support other 
> values, but it is not required.
Thank you, the current design makes more sense to me now. I'm less concerned 
about whether we support dependent values for this attribute argument. If we 
start to support values of `N` other than `__ARM_FEATURE_SVE_BITS` then it 
might make sense to care about it at that point. But I don't think users are 
going to do stuff like:
```
template 
using fixed_int8m1_t __attribute__((riscv_rvv_vector_bits(N))) = vint8m1_t;

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D147256#4262002 , @zequanwu wrote:

> In D147256#4261949 , @hans wrote:
>
>>> Well, MSVC cl removes redundant dots so we shouldn't remove 
>>> llvm::sys::path::remove_dots.
>>
>> Could we do the `remove_dots` on the Clang side, where we can decide based 
>> on the LangOpts?
>
> Yes, we can do that. Is there a reason to favor there over here? At here, the 
> object path in llc output can also benefits from `remove_dots`, not just 
> clang.

The benefit is that if we do it in Clang, we can just look at the LangOpts and 
don't have to add a new target option, command line flag etc.

For llc, since it's an internal tool, I think it would be fine to just use the 
name as given.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

In D148066#4262379 , @reames wrote:

> I would be fine landing this as experimental before ratification.  I see no 
> real downside to doing that

My concern would be that as we don't gate CSR names on enabling the relevant 
extension, people could start using CSR names and encodings that could change, 
without opting in via `-menable-experimental-extensions`, perhaps not realising 
that they're using the unratified version. OTOH, you could argue it was user 
error from the start by not trying to specify all the needed extensions in the 
ISA naming string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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

[PATCH] D146101: [clang-format] Add BracedInitializerIndentWidth option.

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

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

jp4a50 wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > rymiel wrote:
> > > > > owenpan wrote:
> > > > > > jp4a50 wrote:
> > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > owenpan wrote:
> > > > > > > > > jp4a50 wrote:
> > > > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > > > owenpan wrote:
> > > > > > > > > > > > jp4a50 wrote:
> > > > > > > > > > > > > owenpan wrote:
> > > > > > > > > > > > > > owenpan wrote:
> > > > > > > > > > > > > > > Using -1 to mean `ContinuationIndentWidth` is 
> > > > > > > > > > > > > > > unnecessary and somewhat confusing IMO.
> > > > > > > > > > > > > > Please disregard my comment above.
> > > > > > > > > > > > > Just to make sure we are on the same page, does this 
> > > > > > > > > > > > > mean that you are happy with the approach of using 
> > > > > > > > > > > > > `-1` as a default value to indicate that 
> > > > > > > > > > > > > `ContinuationIndentWidth` should be used?
> > > > > > > > > > > > > 
> > > > > > > > > > > > > I did initially consider using 
> > > > > > > > > > > > > `std::optional` and using empty optional to 
> > > > > > > > > > > > > indicate that `ContinuationIndentWidth` should be 
> > > > > > > > > > > > > used but I saw that `PPIndentWidth` was using `-1` to 
> > > > > > > > > > > > > default to using `IndentWidth` so I followed that 
> > > > > > > > > > > > > precedent.
> > > > > > > > > > > > Yep! I would prefer the `optional`, but as you pointed 
> > > > > > > > > > > > out, we already got `PPIndentWidth`using `-1`.
> > > > > > > > > > > From the C++ side I totally agree. One could use 
> > > > > > > > > > > `value_or()`, which would make the code much more 
> > > > > > > > > > > readable.
> > > > > > > > > > > And just because `PPIndentWidth` is using -1 is no reason 
> > > > > > > > > > > to repeat that, we could just as easily change 
> > > > > > > > > > > `PPIndentWidht` to an optional.
> > > > > > > > > > > 
> > > > > > > > > > > But how would it look in yaml?
> > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > 
> > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the 
> > > > > > > > > > YAML then the optional would simply not be set during 
> > > > > > > > > > parsing.
> > > > > > > > > > 
> > > > > > > > > > Of course, if we were to change `PPIndentWidth` to work 
> > > > > > > > > > that way too, it would technically be a breaking change 
> > > > > > > > > > because users may have *explicitly* specified `-1` in their 
> > > > > > > > > > YAML.
> > > > > > > > > > And just because `PPIndentWidth` is using -1 is no reason 
> > > > > > > > > > to repeat that, we could just as easily change 
> > > > > > > > > > `PPIndentWidht` to an optional.
> > > > > > > > > 
> > > > > > > > > We would have to deal with backward compatibility to avoid 
> > > > > > > > > regressions though.
> > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > 
> > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > > 
> > > > > > > > > Of course, if we were to change `PPIndentWidth` to work that 
> > > > > > > > > way too, it would technically be a breaking change because 
> > > > > > > > > users may have *explicitly* specified `-1` in their YAML.
> > > > > > > > 
> > > > > > > > You need an explicit entry, because you need to be able to 
> > > > > > > > write the empty optional on `--dump-config`.
> > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > 
> > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > > 
> > > > > > > > > Of course, if we were to change 

[PATCH] D148112: [include-cleaner] Improve handling for templates

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

caea93cc4478cca28321cba4fa2871d5e6090299 
 should 
fix the issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148112

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

[clang-tools-extra] caea93c - [icnlude-cleaner] Fix dandling pointers in tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-04-12T20:30:33+02:00
New Revision: caea93cc4478cca28321cba4fa2871d5e6090299

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

LOG: [icnlude-cleaner] Fix dandling pointers in tests

Added: 


Modified: 
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 264bedf2e9ac7..ad5bb0aba3076 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -16,7 +16,6 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/TextDiagnostic.h"
 #include "clang/Testing/TestAST.h"
-#include "llvm/ADT/GenericUniformityImpl.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
@@ -42,8 +41,8 @@ using testing::ElementsAre;
 //   Referencing: int x = ^foo();
 // There must be exactly one referencing location marked.
 // Returns target decls.
-std::vector testWalk(llvm::StringRef TargetCode,
-  llvm::StringRef ReferencingCode) {
+std::vector testWalk(llvm::StringRef TargetCode,
+ llvm::StringRef ReferencingCode) {
   llvm::Annotations Target(TargetCode);
   llvm::Annotations Referencing(ReferencingCode);
 
@@ -63,7 +62,7 @@ std::vector testWalk(llvm::StringRef TargetCode,
   FileID TargetFile = SM.translateFile(
   llvm::cantFail(AST.fileManager().getFileRef("target.h")));
 
-  std::vector TargetDecls;
+  std::vector TargetDecls;
   // Perform the walk, and capture the offsets of the referenced targets.
   std::unordered_map> ReferencedOffsets;
   for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
@@ -76,7 +75,7 @@ std::vector testWalk(llvm::StringRef TargetCode,
   if (NDLoc.first != TargetFile)
 return;
   ReferencedOffsets[RT].push_back(NDLoc.second);
-  TargetDecls.push_back();
+  TargetDecls.push_back(ND.getKind());
 });
   }
   for (auto  : ReferencedOffsets)
@@ -131,52 +130,45 @@ TEST(WalkAST, TagType) {
   testWalk("struct $explicit^$implicit^S { static int x; };", "auto y = 
^S();");
 }
 
-MATCHER_P(HasKind, Kind, "") {
-  if (arg->getKind() == Kind)
-return true;
-  *result_listener << "Got kind: " << arg->getDeclKindName();
-  return false;
-}
-
 TEST(WalkAST, ClassTemplates) {
   // Explicit instantiation and (partial) specialization references primary
   // template.
   EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
"template struct ^Foo;"),
-  ElementsAre(HasKind(Decl::CXXRecord)));
+  ElementsAre(Decl::CXXRecord));
   EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
"template<> struct ^Foo {};"),
-  ElementsAre(HasKind(Decl::CXXRecord)));
+  ElementsAre(Decl::CXXRecord));
   EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
"template struct ^Foo {};"),
-  ElementsAre(HasKind(Decl::CXXRecord)));
+  ElementsAre(Decl::CXXRecord));
 
   // Implicit instantiations references most relevant template.
   EXPECT_THAT(
   testWalk("template struct $explicit^Foo {};", "^Foo x;"),
-  ElementsAre(HasKind(Decl::CXXRecord)));
+  ElementsAre(Decl::CXXRecord));
   EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
 template<> struct $explicit^Foo {};)cpp",
"^Foo x;"),
-  ElementsAre(HasKind(Decl::ClassTemplateSpecialization)));
+  ElementsAre(Decl::ClassTemplateSpecialization));
   EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
 template struct $explicit^Foo { void x(); };)cpp",
"^Foo x;"),
-  ElementsAre(HasKind(Decl::ClassTemplatePartialSpecialization)));
+  ElementsAre(Decl::ClassTemplatePartialSpecialization));
   EXPECT_THAT(testWalk(R"cpp(
 template struct $explicit^Foo {};
 template struct Foo;)cpp",
"^Foo x;"),
-  ElementsAre(HasKind(Decl::CXXRecord)));
+  ElementsAre(Decl::CXXRecord));
   // FIXME: This is broken due to
   // https://github.com/llvm/llvm-project/issues/42259.
   EXPECT_THAT(testWalk(R"cpp(
 template struct $explicit^Foo { Foo(T); };
 template<> struct Foo { Foo(int); };)cpp",
"^Foo x(3);"),
-  ElementsAre(HasKind(Decl::ClassTemplate)));
+  ElementsAre(Decl::ClassTemplate));
 }
 TEST(WalkAST, VarTemplates) {
   // Explicit instantiation and (partial) specialization references primary
@@ -188,10 +180,10 @@ TEST(WalkAST, 

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

[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-tools-extra/clangd/Preamble.h:144
+  /// any.
+  static const FileEntry *getPatchEntry(llvm::StringRef MainFilePath,
+const SourceManager );

up to you, but this could be tested I think (just make sure it points to 
something valid with the right name when you have a patch and not otherwise)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148143

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

Re: [PATCH] D148112: [include-cleaner] Improve handling for templates

[Kadir Çetinkaya via cfe-commits]

figured out the issue, preparing a fix

On Wed, Apr 12, 2023 at 7:46 PM Douglas Yung via Phabricator <
revi...@reviews.llvm.org> wrote:

> dyung added a comment.
>
> @kadircet, your change is causing 3 test failures on Windows bots, can you
> take a look and revert if you need time to investigate?
>
> https://lab.llvm.org/buildbot/#/builders/216/builds/19740
> https://lab.llvm.org/buildbot/#/builders/123/builds/18332
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D148112/new/
>
> https://reviews.llvm.org/D148112
>
>

-- 
Want to support earthquake

victims in Turkey & Syria?
- https://google.benevity.org/campaigns/13999
- https://google.benevity.org/campaigns/14045
- https://google.benevity.org/campaigns/14018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill updated this revision to Diff 512910.
Endill edited the summary of this revision.
Endill added a comment.

Remove parts of CWG1894 and CWG2199 tests that are resolved by CWG407


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

Files:
  clang/test/CXX/drs/dr18xx.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/drs/dr4xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -11171,7 +11171,7 @@
 https://wg21.link/cwg1894;>1894
 CD6
 typedef-names and using-declarations
-Unknown
+Clang 3.8
   
   
 https://wg21.link/cwg1895;>1895
@@ -13001,7 +13001,7 @@
 https://wg21.link/cwg2199;>2199
 CD6
 Typedefs and tags
-Unknown
+Clang 3.8
   
   
 https://wg21.link/cwg2200;>2200
Index: clang/test/CXX/drs/dr4xx.cpp
===
--- clang/test/CXX/drs/dr4xx.cpp
+++ clang/test/CXX/drs/dr4xx.cpp
@@ -124,6 +124,7 @@
 }
 
 namespace dr407 { // dr407: 3.8
+  // NB: reused by dr1894 and dr2199
   struct S;
   typedef struct S S;
   void f() {
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++1z -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++2b -triple x86_64-unknown-unknown %s -verify -fexceptions -Wno-deprecated-builtins -fcxx-exceptions -pedantic-errors
 
 #if __cplusplus < 201103L
 // expected-error@+1 {{variadic macro}}
@@ -188,3 +190,31 @@
   B ::operator=(B&&) = default; // expected-error {{would delete}} expected-note@-10{{inaccessible move assignment}}
 #endif
 }
+
+namespace dr2199 { // dr2199: 3.8
+   // NB: reusing part of dr407 test
+namespace A {
+  struct S {};
+}
+namespace B {
+  typedef int S;
+}
+namespace E {
+  typedef A::S S;
+  using A::S;
+  struct S s;
+}
+namespace F {
+  typedef A::S S;
+}
+namespace G {
+  using namespace A;
+  using namespace F;
+  struct S s;
+}
+namespace H {
+  using namespace F;
+  using namespace A;
+  struct S s;
+}
+}
Index: clang/test/CXX/drs/dr18xx.cpp
===
--- clang/test/CXX/drs/dr18xx.cpp
+++ clang/test/CXX/drs/dr18xx.cpp
@@ -168,3 +168,31 @@
   b = static_cast(b); // expected-error {{copy assignment operator is implicitly deleted}}
 #endif
 }
+
+namespace dr1894 { // dr1894: 3.8
+   // NB: reusing part of dr407 test
+namespace A {
+  struct S {};
+}
+namespace B {
+  typedef int S;
+}
+namespace E {
+  typedef A::S S;
+  using A::S;
+  struct S s;
+}
+namespace F {
+  typedef A::S S;
+}
+namespace G {
+  using namespace A;
+  using namespace F;
+  struct S s;
+}
+namespace H {
+  using namespace F;
+  using namespace A;
+  struct S s;
+}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added inline comments.



Comment at: clang/www/cxx_dr_status.html:13004
 Typedefs and tags
-Unknown
+Clang 3.8
   

erichkeane wrote:
> Endill wrote:
> > cor3ntin wrote:
> > > I would just say "Yes", i think clang 3.8 predates these issues
> > It's not the case: 2199 was filed 12.11.2015, but 3.8 was released on 
> > 08.03.2016.
> I don't think that is a policy we have anyway.  We frequently just say "this 
> has worked since Clang X.X" even with much more recent DRs than the release.
Yes, up until now I'd just throw tests into compiler explorer, and look when 
clang started behaving the way it does today. I say "Yes" upon hitting 3.0, 
which is the oldest available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D148031: [clang][driver][NFC] Add hasShadowCallStack to SanitizerArgs

[Paul Kirth 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 rG723f7d3a0891: [clang][driver][NFC] Add hasShadowCallStack to 
SanitizerArgs (authored by paulkirth).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148031

Files:
  clang/include/clang/Driver/SanitizerArgs.h


Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -119,6 +119,10 @@
 return MemtagMode;
   }
 
+  bool hasShadowCallStack() const {
+return Sanitizers.has(SanitizerKind::ShadowCallStack);
+  }
+
   bool requiresPIE() const;
   bool needsUnwindTables() const;
   bool needsLTO() const;


Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -119,6 +119,10 @@
 return MemtagMode;
   }
 
+  bool hasShadowCallStack() const {
+return Sanitizers.has(SanitizerKind::ShadowCallStack);
+  }
+
   bool requiresPIE() const;
   bool needsUnwindTables() const;
   bool needsLTO() const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 723f7d3 - [clang][driver][NFC] Add hasShadowCallStack to SanitizerArgs

[Paul Kirth via cfe-commits]

Author: Paul Kirth
Date: 2023-04-12T18:01:47Z
New Revision: 723f7d3a0891d341b635f31086ae268aadd6d3d6

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

LOG: [clang][driver][NFC] Add hasShadowCallStack to SanitizerArgs

Currently, we can't check if ShadowCallStack is present in Args the same
way we handle other sanitizers. This is preparatory work for planned
driver changes to how we handle ShadowCallStack.

Reviewed By: vitalybuka

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

Added: 


Modified: 
clang/include/clang/Driver/SanitizerArgs.h

Removed: 




diff  --git a/clang/include/clang/Driver/SanitizerArgs.h 
b/clang/include/clang/Driver/SanitizerArgs.h
index 61b7c7f809ae4..5066c4eaf92d8 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -119,6 +119,10 @@ class SanitizerArgs {
 return MemtagMode;
   }
 
+  bool hasShadowCallStack() const {
+return Sanitizers.has(SanitizerKind::ShadowCallStack);
+  }
+
   bool requiresPIE() const;
   bool needsUnwindTables() const;
   bool needsLTO() const;



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

[PATCH] D148148: [clang] Bump AS_GNU to 1

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:135
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,

rsandifo-arm wrote:
> erichkeane wrote:
> > Did these two ctors swap places somehow?  There's something goofy going on 
> > here?  Or is this just to make the delegation of ctors more sensible? 
> Ah, yeah, I should have mentioned that, sorry.  The most general constructor 
> was previously the second in the list.  Like you say, I moved it up to make 
> the delegation more obvious.  The other three keep their relative order.
Thanks for explaining, sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148148

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

[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

I would be fine landing this as experimental before ratification.  I see no 
real downside to doing that, and would essentially leave it the judgement of 
the patch author as to whether just waiting until ratification or doing the 
extra work for staging as experimental is worthwhile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/www/cxx_dr_status.html:13004
 Typedefs and tags
-Unknown
+Clang 3.8
   

Endill wrote:
> cor3ntin wrote:
> > I would just say "Yes", i think clang 3.8 predates these issues
> It's not the case: 2199 was filed 12.11.2015, but 3.8 was released on 
> 08.03.2016.
I don't think that is a policy we have anyway.  We frequently just say "this 
has worked since Clang X.X" even with much more recent DRs than the release.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D148148: [clang] Bump AS_GNU to 1

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:135
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,

erichkeane wrote:
> Did these two ctors swap places somehow?  There's something goofy going on 
> here?  Or is this just to make the delegation of ctors more sensible? 
Ah, yeah, I should have mentioned that, sorry.  The most general constructor 
was previously the second in the list.  Like you say, I moved it up to make the 
delegation more obvious.  The other three keep their relative order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148148

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

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added inline comments.



Comment at: clang/www/cxx_dr_status.html:13004
 Typedefs and tags
-Unknown
+Clang 3.8
   

cor3ntin wrote:
> I would just say "Yes", i think clang 3.8 predates these issues
It's not the case: 2199 was filed 12.11.2015, but 3.8 was released on 
08.03.2016.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Aside from the commenting issue pointed out by @shafik, I think we should move 
the test case into its own test file in SemaCXX and that `RUN` line should 
*not* use `-verify`. This way, we can test that we don't crash but we don't 
have to worry about spurious test failures due to different machines having 
different stack exhaustion points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142401

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

[PATCH] D148101: [clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:55
+
+/// The attibute has no source code manifestation and is only created
+/// implicitly.

rsandifo-arm wrote:
> erichkeane wrote:
> > If I recall, there was some pretty awful funny business in some attributes, 
> > which would explicitly use '0' instead of AS_GNU as implicit.  Did you run 
> > into any of these?
> > 
> > Would it make sense to make AS_Implicit 'first' here to catch those?  Or 
> > perhaps make '0' ill-formed (and assert?) and make this '1'?
> Thanks for the reviews!
> 
> Bumping the values to 1 sounds good to me.  I've created 
> https://reviews.llvm.org/D148148 for that.  I kept AS_GNU first due to:
> ```
> // Note TableGen depends on the order above.  Do not add or change the 
> order
> // without adding related code to TableGen/ClangAttrEmitter.cpp.
> ```
> 
> (I don't know whether that still applies, but it seemed better to keep the 
> tablegen-sensitive stuff at “one end” of the enum.)
Thanks!  I think that makes sense to me.  Just make sure the pre-commit CI 
still passes before committing this patch stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148101

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

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9792
+  "construct">;
+def error_loop_reduction_clause : Error<
+  "reduction clause not handled with '#pragma omp loop bind(teams)'">;

err_omp_...



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6146
+
+  const OpenMPDirectiveKind parentDirective =
+  DSAStack->getParentDirective();

ParentDirective



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6167-6168
+  // Spec restriction : bind(teams) and reduction not permitted.
+  if ((BindKind == OMPC_BIND_teams) &&
+  (C->getClauseKind() == llvm::omp::Clause::OMPC_reduction))
+Diag(SourceLocation(), diag::error_loop_reduction_clause);

Remove extra parens



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6173-6175
+  if (C->getClauseKind() != llvm::omp::Clause::OMPC_bind) {
+ClausesWithoutBind.push_back(C);
+  }

Remove extra braces



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6181-6189
+  DSAStack->setCurrentDirective(OMPD_for);
+  break;
+case OMPC_BIND_teams:
+  Kind = OMPD_distribute;
+  DSAStack->setCurrentDirective(OMPD_distribute);
+  break;
+case OMPC_BIND_thread:

ABataev wrote:
> 1. You're overriding the directive kind here and do not restore it then. It 
> may cause the compiler crash.
> 2. I think you need to here to create a new OpenMP region rather than 
> overriding the existing one.
I see why you're doing this now. Would be good to see some semantic tests, like 
nesting of regions, use of unsupported clauses (e.g. linear on loop 
bind(teams)), etc.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:6199-6202
+  if (UseClausesWithoutBind) {
+ClausesWithImplicit.append(ClausesWithoutBind.begin(),
+   ClausesWithoutBind.end());
+  } else {

Why need to drop bind clause here?



Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:51-56
+void thread_loop2() {
+  #pragma omp loop bind(thread)
+  for (int j = 0 ; j < NNN ; j++) {
+aaa[j] = j*NNN;
+  }
+}

koops wrote:
> ABataev wrote:
> > I think it should trigger the assert in setCurrentDirective
> I did not see any crash at this point. Can you please give me other examples 
> where I can see this crash?
Ah, you're overriding the loop directive kind.


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

https://reviews.llvm.org/D144634

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

[PATCH] D148148: [clang] Bump AS_GNU to 1

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:28
 /// __attribute__((...))
-AS_GNU,
+AS_GNU = 1,
 

Note for others: we're now at 11 items (10 items + starting at 1), stored in 4 
bits.  So this doesn't cause problems.  In the future, once we're sure the '0' 
case isn't being used/abused for a while (god help my downstream...), we can 
start re-using 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148148

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

[PATCH] D148101: [clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:55
+
+/// The attibute has no source code manifestation and is only created
+/// implicitly.

erichkeane wrote:
> If I recall, there was some pretty awful funny business in some attributes, 
> which would explicitly use '0' instead of AS_GNU as implicit.  Did you run 
> into any of these?
> 
> Would it make sense to make AS_Implicit 'first' here to catch those?  Or 
> perhaps make '0' ill-formed (and assert?) and make this '1'?
Thanks for the reviews!

Bumping the values to 1 sounds good to me.  I've created 
https://reviews.llvm.org/D148148 for that.  I kept AS_GNU first due to:
```
// Note TableGen depends on the order above.  Do not add or change the order
// without adding related code to TableGen/ClangAttrEmitter.cpp.
```

(I don't know whether that still applies, but it seemed better to keep the 
tablegen-sensitive stuff at “one end” of the enum.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148101

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

[PATCH] D148148: [clang] Bump AS_GNU to 1

[Erich Keane via Phabricator via cfe-commits]

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

This LGTM, though curious about the re-ordering.




Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:135
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,

Did these two ctors swap places somehow?  There's something goofy going on 
here?  Or is this just to make the delegation of ctors more sensible? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148148

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

[PATCH] D148148: [clang] Bump AS_GNU to 1

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm created this revision.
rsandifo-arm added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
rsandifo-arm requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Following a suggestion from Erich in https://reviews.llvm.org/D148101,
this patch bumps AS_GNU to 1 so that syntax 0 is invalid.  It also
asserts that the syntax is in range.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148148

Files:
  clang/include/clang/Basic/AttributeCommonInfo.h


Index: clang/include/clang/Basic/AttributeCommonInfo.h
===
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -25,7 +25,7 @@
   /// The style used to specify an attribute.
   enum Syntax {
 /// __attribute__((...))
-AS_GNU,
+AS_GNU = 1,
 
 /// [[...]]
 AS_CXX11,
@@ -122,37 +122,32 @@
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Form FormUsed)
+  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
   : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc),
-AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
+ScopeLoc(ScopeLoc), AttrKind(AttrKind),
 SyntaxUsed(FormUsed.getSyntax()),
 SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+IsAlignas(FormUsed.isAlignas()) {
+assert(SyntaxUsed >= AS_GNU && SyntaxUsed <= AS_Implicit &&
+   "Invalid syntax!");
+  }
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
-  : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc), AttrKind(AttrKind),
-SyntaxUsed(FormUsed.getSyntax()),
-SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+  SourceLocation ScopeLoc, Form FormUsed)
+  : AttributeCommonInfo(
+AttrName, ScopeName, AttrRange, ScopeLoc,
+getParsedKind(AttrName, ScopeName, FormUsed.getSyntax()),
+FormUsed) {}
 
   AttributeCommonInfo(const IdentifierInfo *AttrName, SourceRange AttrRange,
   Form FormUsed)
-  : AttrName(AttrName), ScopeName(nullptr), AttrRange(AttrRange),
-ScopeLoc(),
-AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
-SyntaxUsed(FormUsed.getSyntax()),
-SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+  : AttributeCommonInfo(AttrName, nullptr, AttrRange, ScopeLoc(),
+FormUsed) {}
 
   AttributeCommonInfo(SourceRange AttrRange, Kind K, Form FormUsed)
-  : AttrName(nullptr), ScopeName(nullptr), AttrRange(AttrRange), 
ScopeLoc(),
-AttrKind(K), SyntaxUsed(FormUsed.getSyntax()),
-SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+  : AttributeCommonInfo(nullptr, nullptr, AttrRange, ScopeLoc(), K,
+FormUsed) {}
 
   AttributeCommonInfo(AttributeCommonInfo &&) = default;
   AttributeCommonInfo(const AttributeCommonInfo &) = default;


Index: clang/include/clang/Basic/AttributeCommonInfo.h
===
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -25,7 +25,7 @@
   /// The style used to specify an attribute.
   enum Syntax {
 /// __attribute__((...))
-AS_GNU,
+AS_GNU = 1,
 
 /// [[...]]
 AS_CXX11,
@@ -122,37 +122,32 @@
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Form FormUsed)
+  SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
   : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
-ScopeLoc(ScopeLoc),
-AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
+ScopeLoc(ScopeLoc), AttrKind(AttrKind),
 SyntaxUsed(FormUsed.getSyntax()),
 SpellingIndex(FormUsed.getSpellingIndex()),
-IsAlignas(FormUsed.isAlignas()) {}
+IsAlignas(FormUsed.isAlignas()) {
+assert(SyntaxUsed >= AS_GNU && SyntaxUsed <= AS_Implicit &&
+   "Invalid syntax!");
+  }
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
   const IdentifierInfo *ScopeName, SourceRange AttrRange,
-  SourceLocation ScopeLoc, Kind AttrKind, Form 

[PATCH] D147307: [clang] Do not require GNUInlineAttr for inline builtins

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D147307

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

[PATCH] D148146: [clang] Make make_cxx_dr_status script runnable from anywhere

[Vlad Serebrennikov 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 rGd945b6496ec6: [clang] Make make_cxx_dr_status script 
runnable from anywhere (authored by Endill).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148146

Files:
  clang/www/make_cxx_dr_status


Index: clang/www/make_cxx_dr_status
===
--- clang/www/make_cxx_dr_status
+++ clang/www/make_cxx_dr_status
@@ -2,10 +2,11 @@
 import sys, os, re, urllib.request
 
 
-default_issue_list_path = 'cwg_index.html'
+clang_www_dir = os.path.dirname(__file__)
+default_issue_list_path = os.path.join(clang_www_dir, 'cwg_index.html')
 issue_list_url = "https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html;
-output = 'cxx_dr_status.html'
-dr_test_dir = '../test/CXX/drs'
+output = os.path.join(clang_www_dir, 'cxx_dr_status.html')
+dr_test_dir = os.path.join(clang_www_dir, '../test/CXX/drs')
 
 class DR:
   def __init__(self, section, issue, url, status, title):


Index: clang/www/make_cxx_dr_status
===
--- clang/www/make_cxx_dr_status
+++ clang/www/make_cxx_dr_status
@@ -2,10 +2,11 @@
 import sys, os, re, urllib.request
 
 
-default_issue_list_path = 'cwg_index.html'
+clang_www_dir = os.path.dirname(__file__)
+default_issue_list_path = os.path.join(clang_www_dir, 'cwg_index.html')
 issue_list_url = "https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html;
-output = 'cxx_dr_status.html'
-dr_test_dir = '../test/CXX/drs'
+output = os.path.join(clang_www_dir, 'cxx_dr_status.html')
+dr_test_dir = os.path.join(clang_www_dir, '../test/CXX/drs')
 
 class DR:
   def __init__(self, section, issue, url, status, title):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d945b64 - [clang] Make make_cxx_dr_status script runnable from anywhere

[Vlad Serebrennikov via cfe-commits]

Author: Vlad Serebrennikov
Date: 2023-04-12T20:46:05+03:00
New Revision: d945b6496ec6604f692804d501fc96583069d432

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

LOG: [clang] Make make_cxx_dr_status script runnable from anywhere

This script has hardcoded relative paths to `clang/test/CXX/drs`, 
`cwg_index.html`, and `cxx_dr_status.html`, which requires running it with 
`clang/www` CWD. This patch makes those paths relative to path of the script 
itself, so that it could be run from anywhere.

Reviewed By: #clang-language-wg, cor3ntin

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

Added: 


Modified: 
clang/www/make_cxx_dr_status

Removed: 




diff  --git a/clang/www/make_cxx_dr_status b/clang/www/make_cxx_dr_status
index 205922169ebe8..c6229b9bf2c92 100755
--- a/clang/www/make_cxx_dr_status
+++ b/clang/www/make_cxx_dr_status
@@ -2,10 +2,11 @@
 import sys, os, re, urllib.request
 
 
-default_issue_list_path = 'cwg_index.html'
+clang_www_dir = os.path.dirname(__file__)
+default_issue_list_path = os.path.join(clang_www_dir, 'cwg_index.html')
 issue_list_url = "https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html;
-output = 'cxx_dr_status.html'
-dr_test_dir = '../test/CXX/drs'
+output = os.path.join(clang_www_dir, 'cxx_dr_status.html')
+dr_test_dir = os.path.join(clang_www_dir, '../test/CXX/drs')
 
 class DR:
   def __init__(self, section, issue, url, status, title):



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

[PATCH] D148112: [include-cleaner] Improve handling for templates

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

@kadircet, your change is causing 3 test failures on Windows bots, can you take 
a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/216/builds/19740
https://lab.llvm.org/buildbot/#/builders/123/builds/18332


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148112

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

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 512898.
kadircet added a comment.

- Expose helper to get preamble patch entry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148143

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/unittests/PreambleTests.cpp

Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -666,6 +666,7 @@
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
   Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+  Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
@@ -699,6 +700,8 @@
   {
 Annotations Code("#define [[FOO]] 1\n");
 // Check ranges for notes.
+// This also makes sure we don't generate missing-include diagnostics
+// because macros are redefined in preamble-patch.
 Annotations NewCode(R"(#define BARXYZ 1
 #define $foo1[[FOO]] 1
 void foo();
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -30,6 +30,7 @@
 #include "clang-include-cleaner/Record.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/Lexer.h"
@@ -37,8 +38,11 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 
+#include 
+#include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -135,6 +139,10 @@
   static PreamblePatch createMacroPatch(llvm::StringRef FileName,
 const ParseInputs ,
 const PreambleData );
+  /// Returns the FileEntry for the preamble patch of MainFilePath in SM, if
+  /// any.
+  static const FileEntry *getPatchEntry(llvm::StringRef MainFilePath,
+const SourceManager );
 
   /// Adjusts CI (which compiles the modified inputs) to be used with the
   /// baseline preamble. This is done by inserting an artifical include to the
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -738,6 +738,14 @@
   return PatchedDiags;
 }
 
+static std::string getPatchName(llvm::StringRef FileName) {
+  // This shouldn't coincide with any real file name.
+  llvm::SmallString<128> PatchName;
+  llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
+  PreamblePatch::HeaderName);
+  return PatchName.str().str();
+}
+
 PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
 const ParseInputs ,
 const PreambleData ,
@@ -776,11 +784,7 @@
 
   PreamblePatch PP;
   PP.Baseline = 
-  // This shouldn't coincide with any real file name.
-  llvm::SmallString<128> PatchName;
-  llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
-  PreamblePatch::HeaderName);
-  PP.PatchFileName = PatchName.str().str();
+  PP.PatchFileName = getPatchName(FileName);
   PP.ModifiedBounds = ModifiedScan->Bounds;
 
   llvm::raw_string_ostream Patch(PP.PatchContents);
@@ -921,5 +925,13 @@
 return Baseline->Macros;
   return PatchedMacros;
 }
+
+const FileEntry *PreamblePatch::getPatchEntry(llvm::StringRef MainFilePath,
+  const SourceManager ) {
+  auto PatchFilePath = getPatchName(MainFilePath);
+  if (auto File = SM.getFileManager().getFile(PatchFilePath))
+return *File;
+  return nullptr;
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "ParsedAST.h"
+#include "Preamble.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -112,12 +113,12 @@
   return false;
 // Check if main file is the public interface for a private header. If so we
 // shouldn't diagnose it as unused.
-if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
   PHeader = PHeader.trim("<>\"");
   // Since most private -> public 

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/www/cxx_dr_status.html:13004
 Typedefs and tags
-Unknown
+Clang 3.8
   

I would just say "Yes", i think clang 3.8 predates these issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/CXX/drs/dr18xx.cpp:172-227
+namespace dr1894 { // dr1894: 3.8
+   // NB: reusing dr407 test
+  struct S;
+  typedef struct S S;
+  void f() {
+struct S *p;
+{

Endill wrote:
> cor3ntin wrote:
> > I think i would prefer reusing verbatim the example in 1894 - lines 207 and 
> > following basically - as the issue clearly state that what comes before is 
> > resolved by 407 so not really relevant here
> Does the same apply to 2199? It also contains examples resolved by 407.
Yes, i think so. That way it better shows what is actually resolved / tested - 
amd avoid to much duplication


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

Thank you for taking a look at this.




Comment at: clang/test/CXX/drs/dr18xx.cpp:172-227
+namespace dr1894 { // dr1894: 3.8
+   // NB: reusing dr407 test
+  struct S;
+  typedef struct S S;
+  void f() {
+struct S *p;
+{

cor3ntin wrote:
> I think i would prefer reusing verbatim the example in 1894 - lines 207 and 
> following basically - as the issue clearly state that what comes before is 
> resolved by 407 so not really relevant here
Does the same apply to 2199? It also contains examples resolved by 407.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D143364: [RISCV] Support scalar/fix-length vector NTLH intrinsic with different domain

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/CodeGen/RISCV/ntlh-intrinsics/riscv32-zihintntl.c:19
+typedef signed short v8ss __attribute__((vector_size(16)));
+typedef signed char v16sc __attribute__((vector_size(16)));
+v4si v4si1, v4si2;

What about the rvv builtin types for scalable vectors?



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:15347
+  if (NontemporalLevel == 1)
+NontemporalLevel = 5;
+

Should we default Nontemporal level to 5 before looking at 
riscv-nontemporal-domain? Then only 2-5 are legal values?



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:2635
+  static const std::pair TargetFlags[] 
=
+  {{MONontemporalBit0, "riscv-non-temporal-domain-bit-0"},
+   {MONontemporalBit1, "riscv-non-temporal-domain-bit-1"}};

This uses "non-temporal" and the metadata uses "nontemporal". I think we should 
be consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143364

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

[PATCH] D148146: [clang] Make make_cxx_dr_status script runnable from anywhere

[Corentin Jabot via Phabricator via cfe-commits]

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

Great idea, looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148146

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

[PATCH] D148146: [clang] Make make_cxx_dr_status script runnable from anywhere

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill created this revision.
Endill added reviewers: clang-language-wg, cor3ntin.
Herald added a project: All.
Endill requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This script has hardcoded relative paths to `clang/test/CXX/drs`, 
`cwg_index.html`, and `cxx_dr_status.html`, which requires running it with 
`clang/www` CWD. This patch makes those paths relative to path of the script 
itself, so that it could be run from anywhere.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148146

Files:
  clang/www/make_cxx_dr_status


Index: clang/www/make_cxx_dr_status
===
--- clang/www/make_cxx_dr_status
+++ clang/www/make_cxx_dr_status
@@ -2,10 +2,11 @@
 import sys, os, re, urllib.request
 
 
-default_issue_list_path = 'cwg_index.html'
+clang_www_dir = os.path.dirname(__file__)
+default_issue_list_path = os.path.join(clang_www_dir, 'cwg_index.html')
 issue_list_url = "https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html;
-output = 'cxx_dr_status.html'
-dr_test_dir = '../test/CXX/drs'
+output = os.path.join(clang_www_dir, 'cxx_dr_status.html')
+dr_test_dir = os.path.join(clang_www_dir, '../test/CXX/drs')
 
 class DR:
   def __init__(self, section, issue, url, status, title):


Index: clang/www/make_cxx_dr_status
===
--- clang/www/make_cxx_dr_status
+++ clang/www/make_cxx_dr_status
@@ -2,10 +2,11 @@
 import sys, os, re, urllib.request
 
 
-default_issue_list_path = 'cwg_index.html'
+clang_www_dir = os.path.dirname(__file__)
+default_issue_list_path = os.path.join(clang_www_dir, 'cwg_index.html')
 issue_list_url = "https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html;
-output = 'cxx_dr_status.html'
-dr_test_dir = '../test/CXX/drs'
+output = os.path.join(clang_www_dir, 'cxx_dr_status.html')
+dr_test_dir = os.path.join(clang_www_dir, '../test/CXX/drs')
 
 class DR:
   def __init__(self, section, issue, url, status, title):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/SemaCXX/attr-riscv-rvv-vector-bits.cpp:12
+
+template struct S { T var; };
+

aaron.ballman wrote:
> erichkeane wrote:
> > craig.topper wrote:
> > > erichkeane wrote:
> > > > craig.topper wrote:
> > > > > @erichkeane does this cover the dependent case or were you looking 
> > > > > for something else?
> > > > > 
> > > > > Here are on the only mentions of template I see in SVE tests that use 
> > > > > this attribute.
> > > > > 
> > > > > ```
> > > > > clang/test$ ack template `ack arm_sve_vector -l`
> > > > > CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
> > > > > 37:template  struct S {};
> > > > > 
> > > > > SemaCXX/attr-arm-sve-vector-bits.cpp
> > > > > 16:template struct S { T var; };
> > > > > ```
> > > > > 
> > > > > Here is the result for this patch
> > > > > 
> > > > > ```
> > > > > clang/test$ ack template `ack riscv_rvv_vector -l`
> > > > > CodeGenCXX/riscv-mangle-rvv-fixed-vectors.cpp
> > > > > 48:template  struct S {};
> > > > > 
> > > > > SemaCXX/attr-riscv-rvv-vector-bits.cpp
> > > > > 12:template struct S { T var; };
> > > > > ```
> > > > Thats unfortunate, and I wish I'd thought of it at the time/been more 
> > > > active reviewing the SVE stuff then.  Really what I'm looking for is:
> > > > 
> > > > ```
> > > > template 
> > > > struct Whatever {
> > > >   using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > > > };
> > > > 
> > > > void Func(Whatever<5>::Something MyVar){}
> > > > 
> > > > ```
> > > That does not appear to work.
> > > 
> > > ```
> > > $ ./bin/clang test.cpp --target=riscv64 -march=rv64gcv 
> > > -mrvv-vector-bits=zvl
> > > test.cpp:3:41: error: 'riscv_rvv_vector_bits' attribute requires an 
> > > integer constant
> > > using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > > ```
> > > 
> > > It's not very useful as a template parameter. There's only one value that 
> > > works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> > Thats really unfortunate, but it makes me wonder what `DependentVectorType 
> > ` is for in this case, or the handling of said things.  Because I would 
> > expect:
> > 
> > ```
> > template
> > using RiscvVector = T __attribute__((risv_rvv_vector_bits(Size)));
> > 
> > RiscvVector> Foo;
> > ```
> > to be useful.  Even if not, I'd expect:
> > ```
> > template
> > using RiscvVector = T __attribute__((risv_rvv_vector_bits(TheRightAnswer)));
> > RiscvVector Foo;
> > ```
> > to both work.
> > 
> > >>It's not very useful as a template parameter. There's only one value that 
> > >>works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> > This makes me wonder why this attribute takes an integer constant anyway, 
> > if it is just a 'guess what the right answer is!' sorta thing.  Seems to me 
> > this never should have taken a parameter.
> > It's not very useful as a template parameter. There's only one value that 
> > works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> 
> Can you help me understand why the argument exists then?
> 
> We're pretty inconsistent about attribute arguments properly handling things 
> like constant expressions vs integer literals, but the trend lately is to 
> accept a constant expression rather than only a literal because of how often 
> users like to give names to literals and how much more constexpr code we're 
> seeing in the wild.
This is what's in ARM's ACLE documentation:



> The ACLE only defines the effect of the attribute if all of the following are 
> true:
> 1. the attribute is attached to a single SVE vector type (such as svint32_t) 
> or to the SVE predicate
> type svbool_t;
> 2. the arguments “…” consist of a single nonzero integer constant expression 
> (referred to as N below); and
> 3. N==__ARM_FEATURE_SVE_BITS.
> In other cases the implementation must do one of the following:
> • ignore the attribute; a warning would then be appropriate, but is not 
> required
> • reject the program with a diagnostic
> • extend requirement (3) above to support other values of N besides 
> __ARM_FEATURE_SVE_BITS
> • process the attribute in accordance with a later revision of the ACLE





So there's a bullet in there that allows an implementation to support other 
values, but it is not required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088

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

[PATCH] D148136: [clang] Add test for CWG1894 and CWG2199

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/CXX/drs/dr18xx.cpp:172-227
+namespace dr1894 { // dr1894: 3.8
+   // NB: reusing dr407 test
+  struct S;
+  typedef struct S S;
+  void f() {
+struct S *p;
+{

I think i would prefer reusing verbatim the example in 1894 - lines 207 and 
following basically - as the issue clearly state that what comes before is 
resolved by 407 so not really relevant here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148136

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I've not done a complete review yet, but I got started on it and have a handful 
of comments.




Comment at: clang/include/clang/Basic/TokenKinds.def:945
+// Annotation for end of input in clang-repl.
+ANNOTATION(input_end)
+

Should we name this `repl_input_end` to make it clear this is generally 
expected to only be used for REPL input?



Comment at: clang/include/clang/Interpreter/Interpreter.h:59
+
+  Value LastValue;
 

I think I'm surprised to see this as a data member of `Interpreter` but mostly 
because my brain keeps thinking this interpreter is to interpret whole 
programs, so the idea of a "last value" is a bit odd from that context. The 
name is probably fine as-is, but I figured it may be worth mentioning just the 
same.



Comment at: clang/include/clang/Interpreter/Interpreter.h:65
   create(std::unique_ptr CI);
+  ASTContext () const;
   const CompilerInstance *getCompilerInstance() const;

Overload set for some better const correctness.



Comment at: clang/include/clang/Interpreter/Interpreter.h:72
+  llvm::Error ParseAndExecute(llvm::StringRef Code, Value *V = nullptr);
+  llvm::Expected CompileDtorCall(CXXRecordDecl *CXXRD);
 

Hopefully this function isn't intending to mutate the passed object?



Comment at: clang/include/clang/Interpreter/Interpreter.h:98-100
+  llvm::SmallVectorImpl () {
+return ValuePrintingInfo;
+  }

`const` overload here as well.



Comment at: clang/include/clang/Interpreter/Interpreter.h:102
+
+  Expr *SynthesizeExpr(clang::Expr *E);
+

Any chance we can make this const correct as well? (I'll stop asking in this 
review -- can you take a pass through it to add const correctness where it 
isn't obnoxiously viral to do so?)



Comment at: clang/include/clang/Interpreter/Value.h:17
+
+#include 
+

Unused include can be removed.



Comment at: clang/include/clang/Interpreter/Value.h:43
+
+class Interpreter;
+class QualType;

Duplicates line 27, can be removed.



Comment at: clang/include/clang/Interpreter/Value.h:44
+class Interpreter;
+class QualType;
+

Move this up to the rest of the forward declarations (and probably keep them in 
alphabetical order).



Comment at: clang/include/clang/Interpreter/Value.h:46
+
+#define REPL_BUILTIN_TYPES 
\
+  X(bool, Bool)
\

Is this expected to be a complete list of builtin types? e.g., should this have 
`char8_t` and `void` and `wchar_t`, etc? Should this be including 
`clang/include/clang/AST/BuiltinTypes.def` instead of manually maintaining the 
list?



Comment at: clang/include/clang/Interpreter/Value.h:77
+
+K_Void,
+K_PtrOrObj,

Fixing indentation



Comment at: clang/include/clang/Interpreter/Value.h:83
+  Value() = default;
+  Value(void /*Interpreter*/ *In, void /*QualType*/ *Ty);
+  Value(const Value );

Why do these take `void *` instead of the expected type?



Comment at: clang/include/clang/Interpreter/Value.h:121
+static T cast(const Value ) {
+  if (V.isPointerOrObjectType())
+return (T)(uintptr_t)V.getAs();

Do we have to worry about member function pointers where a single pointer value 
may not be sufficient?



Comment at: clang/include/clang/Interpreter/Value.h:143
+  /// Values referencing an object are treated as pointers to the object.
+  template  T castAs() const { return CastFwd::cast(*this); }
+

This doesn't match the usual pattern used in Clang where the `get` variant 
returns `nullptr` and the `cast` variant asserts if the cast would be invalid. 
Should we go with a similar approach?



Comment at: clang/include/clang/Interpreter/Value.h:160-162
+  // Interpreter, QualType are stored as void* to reduce dependencies.
+  void *Interp = nullptr;
+  void *OpaqueType = nullptr;

Why don't forward declares suffice if we're storing the information by pointer?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:7226
+  // assert(DeferredDeclsToEmit.empty() &&
+  //"Should have emitted all decls deferred to emit.");
   assert(NewBuilder->DeferredDecls.empty() &&

v.g.vassilev wrote:
> That should probably be a separate review with a testcase.
+1 -- the codegen code owners should weigh in on whether this is reasonable as 
a temporary measure or not.



Comment at: clang/lib/Interpreter/CMakeLists.txt:14-16
   Interpreter.cpp
+  Value.cpp
+  InterpreterUtils.cpp

Probably should be 

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/attr-riscv-rvv-vector-bits.cpp:12
+
+template struct S { T var; };
+

erichkeane wrote:
> craig.topper wrote:
> > erichkeane wrote:
> > > craig.topper wrote:
> > > > @erichkeane does this cover the dependent case or were you looking for 
> > > > something else?
> > > > 
> > > > Here are on the only mentions of template I see in SVE tests that use 
> > > > this attribute.
> > > > 
> > > > ```
> > > > clang/test$ ack template `ack arm_sve_vector -l`
> > > > CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
> > > > 37:template  struct S {};
> > > > 
> > > > SemaCXX/attr-arm-sve-vector-bits.cpp
> > > > 16:template struct S { T var; };
> > > > ```
> > > > 
> > > > Here is the result for this patch
> > > > 
> > > > ```
> > > > clang/test$ ack template `ack riscv_rvv_vector -l`
> > > > CodeGenCXX/riscv-mangle-rvv-fixed-vectors.cpp
> > > > 48:template  struct S {};
> > > > 
> > > > SemaCXX/attr-riscv-rvv-vector-bits.cpp
> > > > 12:template struct S { T var; };
> > > > ```
> > > Thats unfortunate, and I wish I'd thought of it at the time/been more 
> > > active reviewing the SVE stuff then.  Really what I'm looking for is:
> > > 
> > > ```
> > > template 
> > > struct Whatever {
> > >   using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > > };
> > > 
> > > void Func(Whatever<5>::Something MyVar){}
> > > 
> > > ```
> > That does not appear to work.
> > 
> > ```
> > $ ./bin/clang test.cpp --target=riscv64 -march=rv64gcv -mrvv-vector-bits=zvl
> > test.cpp:3:41: error: 'riscv_rvv_vector_bits' attribute requires an integer 
> > constant
> > using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > ```
> > 
> > It's not very useful as a template parameter. There's only one value that 
> > works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> Thats really unfortunate, but it makes me wonder what `DependentVectorType ` 
> is for in this case, or the handling of said things.  Because I would expect:
> 
> ```
> template
> using RiscvVector = T __attribute__((risv_rvv_vector_bits(Size)));
> 
> RiscvVector> Foo;
> ```
> to be useful.  Even if not, I'd expect:
> ```
> template
> using RiscvVector = T __attribute__((risv_rvv_vector_bits(TheRightAnswer)));
> RiscvVector Foo;
> ```
> to both work.
> 
> >>It's not very useful as a template parameter. There's only one value that 
> >>works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
> This makes me wonder why this attribute takes an integer constant anyway, if 
> it is just a 'guess what the right answer is!' sorta thing.  Seems to me this 
> never should have taken a parameter.
> It's not very useful as a template parameter. There's only one value that 
> works and that's whatever __RISCV_RVV_VLEN_BITS is set to.

Can you help me understand why the argument exists then?

We're pretty inconsistent about attribute arguments properly handling things 
like constant expressions vs integer literals, but the trend lately is to 
accept a constant expression rather than only a literal because of how often 
users like to give names to literals and how much more constexpr code we're 
seeing in the wild.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088

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

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

[Sunil K via Phabricator via cfe-commits]

koops added inline comments.



Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:51-56
+void thread_loop2() {
+  #pragma omp loop bind(thread)
+  for (int j = 0 ; j < NNN ; j++) {
+aaa[j] = j*NNN;
+  }
+}

ABataev wrote:
> I think it should trigger the assert in setCurrentDirective
I did not see any crash at this point. Can you please give me other examples 
where I can see this crash?


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

https://reviews.llvm.org/D144634

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

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/SemaCXX/attr-riscv-rvv-vector-bits.cpp:12
+
+template struct S { T var; };
+

craig.topper wrote:
> erichkeane wrote:
> > craig.topper wrote:
> > > @erichkeane does this cover the dependent case or were you looking for 
> > > something else?
> > > 
> > > Here are on the only mentions of template I see in SVE tests that use 
> > > this attribute.
> > > 
> > > ```
> > > clang/test$ ack template `ack arm_sve_vector -l`
> > > CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
> > > 37:template  struct S {};
> > > 
> > > SemaCXX/attr-arm-sve-vector-bits.cpp
> > > 16:template struct S { T var; };
> > > ```
> > > 
> > > Here is the result for this patch
> > > 
> > > ```
> > > clang/test$ ack template `ack riscv_rvv_vector -l`
> > > CodeGenCXX/riscv-mangle-rvv-fixed-vectors.cpp
> > > 48:template  struct S {};
> > > 
> > > SemaCXX/attr-riscv-rvv-vector-bits.cpp
> > > 12:template struct S { T var; };
> > > ```
> > Thats unfortunate, and I wish I'd thought of it at the time/been more 
> > active reviewing the SVE stuff then.  Really what I'm looking for is:
> > 
> > ```
> > template 
> > struct Whatever {
> >   using Something = char __attribute((riscv_rvv_vector_bits(N)));
> > };
> > 
> > void Func(Whatever<5>::Something MyVar){}
> > 
> > ```
> That does not appear to work.
> 
> ```
> $ ./bin/clang test.cpp --target=riscv64 -march=rv64gcv -mrvv-vector-bits=zvl
> test.cpp:3:41: error: 'riscv_rvv_vector_bits' attribute requires an integer 
> constant
> using Something = char __attribute((riscv_rvv_vector_bits(N)));
> ```
> 
> It's not very useful as a template parameter. There's only one value that 
> works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
Thats really unfortunate, but it makes me wonder what `DependentVectorType ` is 
for in this case, or the handling of said things.  Because I would expect:

```
template
using RiscvVector = T __attribute__((risv_rvv_vector_bits(Size)));

RiscvVector> Foo;
```
to be useful.  Even if not, I'd expect:
```
template
using RiscvVector = T __attribute__((risv_rvv_vector_bits(TheRightAnswer)));
RiscvVector Foo;
```
to both work.

>>It's not very useful as a template parameter. There's only one value that 
>>works and that's whatever __RISCV_RVV_VLEN_BITS is set to.
This makes me wonder why this attribute takes an integer constant anyway, if it 
is just a 'guess what the right answer is!' sorta thing.  Seems to me this 
never should have taken a parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088

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

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:379
+  (H.physical() == MainFile ||
+   H.physical()->getName().endswith(PreamblePatch::HeaderName))) {
 Satisfied = true;

Comparing strings here every time seems odd & slow.

Is it too fragile to add a function somewhere (Preamble.h?) to get the preamble 
patch file ID from a source manager? (By reconstructing the path and then 
looking it up)

That way it can be done outside this loop, and without encoding such details 
here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148143

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

[clang] 3704752 - [clang][NFC] Use range-for loop in TextDiagnostic.cpp

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-04-12T18:50:37+02:00
New Revision: 37047523a9fb5ffa74eaf94d9d52db831f99c062

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

LOG: [clang][NFC] Use range-for loop in TextDiagnostic.cpp

Added: 


Modified: 
clang/lib/Frontend/TextDiagnostic.cpp

Removed: 




diff  --git a/clang/lib/Frontend/TextDiagnostic.cpp 
b/clang/lib/Frontend/TextDiagnostic.cpp
index 809d5309d1af..95b5f257c63d 100644
--- a/clang/lib/Frontend/TextDiagnostic.cpp
+++ b/clang/lib/Frontend/TextDiagnostic.cpp
@@ -1168,11 +1168,10 @@ void TextDiagnostic::emitSnippetAndCaret(
   // Find the set of lines to include.
   const unsigned MaxLines = DiagOpts->SnippetLineLimit;
   std::pair Lines = {CaretLineNo, CaretLineNo};
-  for (SmallVectorImpl::iterator I = Ranges.begin(),
-  E = Ranges.end();
-   I != E; ++I)
-if (auto OptionalRange = findLinesForRange(*I, FID, SM))
+  for (auto  : Ranges) {
+if (auto OptionalRange = findLinesForRange(I, FID, SM))
   Lines = maybeAddRange(Lines, *OptionalRange, MaxLines);
+  }
 
   for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1; ++LineNo) {
 const char *BufStart = BufData.data();
@@ -1212,10 +1211,8 @@ void TextDiagnostic::emitSnippetAndCaret(
 std::string CaretLine(sourceColMap.columns(), ' ');
 
 // Highlight all of the characters covered by Ranges with ~ characters.
-for (SmallVectorImpl::iterator I = Ranges.begin(),
-E = Ranges.end();
- I != E; ++I)
-  highlightRange(*I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
+for (auto  : Ranges)
+  highlightRange(I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
 
 // Next, insert the caret itself.
 if (CaretLineNo == LineNo) {



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

[PATCH] D146030: [clang][Interp] Handle LambdaExprs

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146030

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

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/SemaCXX/attr-riscv-rvv-vector-bits.cpp:12
+
+template struct S { T var; };
+

erichkeane wrote:
> craig.topper wrote:
> > @erichkeane does this cover the dependent case or were you looking for 
> > something else?
> > 
> > Here are on the only mentions of template I see in SVE tests that use this 
> > attribute.
> > 
> > ```
> > clang/test$ ack template `ack arm_sve_vector -l`
> > CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
> > 37:template  struct S {};
> > 
> > SemaCXX/attr-arm-sve-vector-bits.cpp
> > 16:template struct S { T var; };
> > ```
> > 
> > Here is the result for this patch
> > 
> > ```
> > clang/test$ ack template `ack riscv_rvv_vector -l`
> > CodeGenCXX/riscv-mangle-rvv-fixed-vectors.cpp
> > 48:template  struct S {};
> > 
> > SemaCXX/attr-riscv-rvv-vector-bits.cpp
> > 12:template struct S { T var; };
> > ```
> Thats unfortunate, and I wish I'd thought of it at the time/been more active 
> reviewing the SVE stuff then.  Really what I'm looking for is:
> 
> ```
> template 
> struct Whatever {
>   using Something = char __attribute((riscv_rvv_vector_bits(N)));
> };
> 
> void Func(Whatever<5>::Something MyVar){}
> 
> ```
That does not appear to work.

```
$ ./bin/clang test.cpp --target=riscv64 -march=rv64gcv -mrvv-vector-bits=zvl
test.cpp:3:41: error: 'riscv_rvv_vector_bits' attribute requires an integer 
constant
using Something = char __attribute((riscv_rvv_vector_bits(N)));
```

It's not very useful as a template parameter. There's only one value that works 
and that's whatever __RISCV_RVV_VLEN_BITS is set to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088

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

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Since we redefine all macros in preamble-patch, and it's parsed after
consuming the preamble macros, we can get false missing-include diagnostics
while a fresh preamble is being rebuilt.

This patch makes sure preamble-patch is treated same as main file for
include-cleaner purposes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148143

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -666,6 +666,7 @@
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
   Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+  Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
@@ -699,6 +700,8 @@
   {
 Annotations Code("#define [[FOO]] 1\n");
 // Check ranges for notes.
+// This also makes sure we don't generate missing-include diagnostics
+// because macros are redefined in preamble-patch.
 Annotations NewCode(R"(#define BARXYZ 1
 #define $foo1[[FOO]] 1
 void foo();
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "ParsedAST.h"
+#include "Preamble.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -112,12 +113,12 @@
   return false;
 // Check if main file is the public interface for a private header. If so 
we
 // shouldn't diagnose it as unused.
-if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
   PHeader = PHeader.trim("<>\"");
   // Since most private -> public mappings happen in a verbatim way, we
   // check textually here. This might go wrong in presence of symlinks or
   // header mappings. But that's not different than rest of the places.
-  if(AST.tuPath().endswith(PHeader))
+  if (AST.tuPath().endswith(PHeader))
 return false;
 }
   }
@@ -374,7 +375,8 @@
 bool Satisfied = false;
 for (const auto  : Providers) {
   if (H.kind() == include_cleaner::Header::Physical &&
-  H.physical() == MainFile) {
+  (H.physical() == MainFile ||
+   H.physical()->getName().endswith(PreamblePatch::HeaderName))) {
 Satisfied = true;
 continue;
   }


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -666,6 +666,7 @@
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
   Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+  Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
@@ -699,6 +700,8 @@
   {
 Annotations Code("#define [[FOO]] 1\n");
 // Check ranges for notes.
+// This also makes sure we don't generate missing-include diagnostics
+// because macros are redefined in preamble-patch.
 Annotations NewCode(R"(#define BARXYZ 1
 #define $foo1[[FOO]] 1
 void foo();
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -11,6 +11,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "ParsedAST.h"
+#include "Preamble.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
@@ -112,12 +113,12 @@
   return false;
 // Check if main file is the public interface for a private header. If so we
 // shouldn't diagnose it as unused.
-if(auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
+if (auto PHeader = PI->getPublic(*FE); !PHeader.empty()) {
   PHeader = PHeader.trim("<>\"");
   // Since most private -> public mappings happen in a verbatim way, we
   // check textually here. This might go wrong in presence of symlinks or
   // header mappings. But that's not different than rest of the places.
-  if(AST.tuPath().endswith(PHeader))
+  if 

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D147256#4261949 , @hans wrote:

>> Well, MSVC cl removes redundant dots so we shouldn't remove 
>> llvm::sys::path::remove_dots.
>
> Could we do the `remove_dots` on the Clang side, where we can decide based on 
> the LangOpts?

Yes, we can do that. Is there a reason to favor there over here? At here, the 
object path in llc output can also benefits from `remove_dots`, not just clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147256

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 512869.
junaire added a comment.

fix ci


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/Mangle.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Interpreter/Value.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
@@ -33,6 +34,10 @@
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
 #endif
 
+int Global = 42;
+int getGlobal() { return Global; }
+void setGlobal(int val) { Global = val; }
+
 namespace {
 using Args = std::vector;
 static std::unique_ptr
@@ -276,8 +281,7 @@
   std::vector Args = {"-fno-delayed-template-parsing"};
   std::unique_ptr Interp = createInterpreter(Args);
 
-  llvm::cantFail(Interp->Parse("void* operator new(__SIZE_TYPE__, void* __p);"
-   "extern \"C\" int printf(const char*,...);"
+  llvm::cantFail(Interp->Parse("extern \"C\" int printf(const char*,...);"
"class A {};"
"struct B {"
"  template"
@@ -314,4 +318,55 @@
   free(NewA);
 }
 
+TEST(InterpreterTest, Value) {
+  std::unique_ptr Interp = createInterpreter();
+
+  Value V1;
+  llvm::cantFail(Interp->ParseAndExecute("int x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("x", ));
+  EXPECT_TRUE(V1.isValid());
+  EXPECT_EQ(V1.getInt(), 42);
+  EXPECT_TRUE(V1.getType()->isIntegerType());
+  EXPECT_EQ(V1.getKind(), Value::K_Int);
+  EXPECT_FALSE(V1.isManuallyAlloc());
+  EXPECT_FALSE(V1.isPointerOrObjectType());
+
+  Value V2;
+  llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
+  llvm::cantFail(Interp->ParseAndExecute("y", ));
+  EXPECT_TRUE(V2.isValid());
+  EXPECT_EQ(V2.getDouble(), 3.14);
+  EXPECT_TRUE(V2.getType()->isFloatingType());
+  EXPECT_EQ(V2.getKind(), Value::K_Double);
+  EXPECT_FALSE(V2.isManuallyAlloc());
+  EXPECT_FALSE(V2.isPointerOrObjectType());
+
+  Value V3;
+  llvm::cantFail(Interp->ParseAndExecute(
+  "struct S { int* p; S() { p = new int(42); } ~S() { delete p; }};"));
+  llvm::cantFail(Interp->ParseAndExecute("S{}", ));
+  EXPECT_TRUE(V3.isValid());
+  EXPECT_TRUE(V3.getType()->isRecordType());
+  EXPECT_EQ(V3.getKind(), Value::K_PtrOrObj);
+  EXPECT_TRUE(V3.isManuallyAlloc());
+  EXPECT_TRUE(V3.isPointerOrObjectType());
+
+  Value V4;
+  llvm::cantFail(Interp->ParseAndExecute("int getGlobal();"));
+  llvm::cantFail(Interp->ParseAndExecute("void setGlobal(int);"));
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V4.getInt(), 42);
+  EXPECT_TRUE(V4.getType()->isIntegerType());
+
+  Value V5;
+  // Change the global from the compiled code.
+  setGlobal(43);
+  llvm::cantFail(Interp->ParseAndExecute("getGlobal()", ));
+  EXPECT_EQ(V5.getInt(), 43);
+  EXPECT_TRUE(V5.getType()->isIntegerType());
+
+  // Change the global from the interpreted code.
+  llvm::cantFail(Interp->ParseAndExecute("setGlobal(44);"));
+  EXPECT_EQ(getGlobal(), 44);
+}
 } // end anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -22,3 +22,5 @@
 if(NOT WIN32)
   add_subdirectory(ExceptionTests)
 endif()
+
+export_executable_symbols(ClangReplInterpreterTests)
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index: clang/lib/Parse/Parser.cpp

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

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

Add assertion on source locations in `pushRegion`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,19 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If either of these locations is invalid, something elsewhere in the
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not 
valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");
+
+// However, we can still recover without crashing.
+// If either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +637,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +705,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,19 @@
   MostRecentLocation = *StartLoc;
 }
 
+// If either of these locations is invalid, something elsewhere in the
+// compiler has broken.
+assert((!StartLoc || StartLoc->isValid()) && "Start location is not valid");
+assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");
+
+// However, we can still recover without crashing.
+// If either location is invalid, set it to std::nullopt to avoid
+// letting users of RegionStack think that region has a valid start/end
+// location.
+if (StartLoc && StartLoc->isInvalid())
+  StartLoc = std::nullopt;
+if (EndLoc && EndLoc->isInvalid())
+  EndLoc = std::nullopt;
 RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
 return RegionStack.size() - 1;
@@ -624,7 +637,8 @@
 assert(RegionStack.size() >= ParentIndex && "parent not in stack");
 while (RegionStack.size() > ParentIndex) {
   SourceMappingRegion  = RegionStack.back();
-  if (Region.hasStartLoc()) {
+  if (Region.hasStartLoc() &&
+  (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
 SourceLocation StartLoc = Region.getBeginLoc();
 SourceLocation EndLoc = Region.hasEndLoc()
 ? Region.getEndLoc()
@@ -691,7 +705,7 @@
 assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
 assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
-}
+  }
   RegionStack.pop_back();
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits