[PATCH] D46522: [clang] cmake: resolve symlinks in ClangConfig.cmake

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

I'm interested in this, I've tried for a while to fix the Debian packaging but 
I'm completely new to the packaging toolchain, so I'm making slow headway.

Some of the problems are recorded in: 
https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=862328

My (possibly naive) take is that since the LLVM/Clang build/install tree works 
as-is with `find_package`, the bug must be in packaging. That is, if you have a 
local build tree in `/build/`, this configures without a hitch: `cmake 
-DCMAKE_PREFIX_PATH=/build/ -G Ninja .`.


Repository:
  rC Clang

https://reviews.llvm.org/D46522



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

[PATCH] D46522: [clang] cmake: resolve symlinks in ClangConfig.cmake

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

In https://reviews.llvm.org/D46522#1096847, @kimgr wrote:

> I'm interested in this, I've tried for a while to fix the Debian packaging 
> but I'm completely new to the packaging toolchain, so I'm making slow headway.


The Debian clang-5.0 1:5.0.2-2 package already includes this patch. I tried to 
upstream it, but there were some concerns about the real path not always being 
the desired value.

> My (possibly naive) take is that since the LLVM/Clang build/install tree 
> works as-is with `find_package`, the bug must be in packaging. That is, if 
> you have a local build tree in `/build/`, this configures without a hitch: 
> `cmake -DCMAKE_PREFIX_PATH=/build/ -G Ninja .` with a simple `CMakeLists.txt` 
> doing `find_package` for both LLVM and Clang.

On Debian, `cmake -DCMAKE_PREFIX_PATH=/usr/lib/llvm-5.0` would work as well, 
the problem was that the FindClang.cmake file was installed into an unexpected 
location (packaging issue).

Secondary to that, some additional symlinks were installed to ensure that users 
do not have to set `CMAKE_PREFIX_PATH` in order to find some LLVM/Clang 
version. That scenario was being addressed with this patch (and 
https://reviews.llvm.org/D46521).


Repository:
  rC Clang

https://reviews.llvm.org/D46522



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

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 146467.
EricWF added a comment.

@rsmith How does this look?

Turns out we have to fully parse the diagnostic text in order to substitute 
modifier indexes, which is a bit of a complication. This patch does exactly 
that.

There is still some work done to produce useful error messages when the 
substitution is ill-formed, but hopefully the design of this patch is, in 
general, OK>


https://reviews.llvm.org/D46740

Files:
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticSemaKinds.td
  test/TableGen/DiagnosticBase.inc
  test/TableGen/DiagnosticDocs.inc
  test/TableGen/emit-diag-docs.td
  test/TableGen/text-substitution.td
  test/lit.cfg.py
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -14,12 +14,14 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringToOffsetTable.h"
@@ -441,6 +443,654 @@
   }
 }
 
+namespace {
+enum PieceKind {
+  MultiPieceClass,
+  TextPieceClass,
+  PlaceholderPieceClass,
+  SelectPieceClass,
+  DiffPieceClass,
+  SubstitutionPieceClass,
+};
+
+struct DiagText;
+
+enum ModifierType {
+  MT_Unknown,
+  MT_Placeholder,
+  MT_Select,
+  MT_Sub,
+  MT_Plural,
+  MT_Diff,
+  MT_Ordinal,
+  MT_S,
+  MT_Q,
+  MT_ObjCClass,
+  MT_ObjCInstance,
+};
+
+static StringRef getModifierName(ModifierType MT) {
+  switch (MT) {
+  case MT_Select:
+return "select";
+  case MT_Sub:
+return "sub";
+  case MT_Diff:
+return "diff";
+  case MT_Plural:
+return "plural";
+  case MT_Ordinal:
+return "ordinal";
+  case MT_S:
+return "s";
+  case MT_Q:
+return "q";
+  case MT_Placeholder:
+return "";
+  case MT_ObjCClass:
+return "objcclass";
+  case MT_ObjCInstance:
+return "objcinstance";
+  case MT_Unknown:
+return "<>";
+  }
+}
+
+struct Piece {
+  // This type and its derived classes are move-only.
+  Piece(PieceKind Kind) : ClassKind(Kind) {}
+  Piece(Piece const &O) = delete;
+  Piece &operator=(Piece const &) = delete;
+  virtual ~Piece() {}
+
+  PieceKind getPieceClass() const { return ClassKind; }
+  static bool classof(const Piece *) { return true; }
+
+private:
+  PieceKind ClassKind;
+};
+
+struct MultiPiece : Piece {
+  MultiPiece() : Piece(MultiPieceClass) {}
+  MultiPiece(std::vector Pieces)
+  : Piece(MultiPieceClass), Pieces(std::move(Pieces)) {}
+
+  std::vector Pieces;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == MultiPieceClass;
+  }
+};
+
+struct TextPiece : Piece {
+  StringRef Role;
+  std::string Text;
+  TextPiece(StringRef Text, StringRef Role = "")
+  : Piece(TextPieceClass), Role(Role), Text(Text.str()) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == TextPieceClass;
+  }
+};
+
+struct PlaceholderPiece : Piece {
+  ModifierType Kind;
+  int Index;
+  PlaceholderPiece(ModifierType Kind, int Index)
+  : Piece(PlaceholderPieceClass), Kind(Kind), Index(Index) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PlaceholderPieceClass;
+  }
+};
+
+struct SelectPiece : Piece {
+  SelectPiece(ModifierType Kind) : Piece(SelectPieceClass), Kind(Kind) {}
+
+  ModifierType Kind;
+  std::vector Options;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SelectPieceClass;
+  }
+};
+
+struct DiffPiece : Piece {
+  DiffPiece() : Piece(DiffPieceClass) {}
+
+  Piece *Options[2] = {};
+  unsigned Indexes[2] = {};
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == DiffPieceClass;
+  }
+};
+
+struct SubstitutionPiece : Piece {
+  SubstitutionPiece() : Piece(SubstitutionPieceClass) {}
+
+  std::string Name;
+  std::vector Modifiers;
+  Piece *Substitution = nullptr;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SubstitutionPieceClass;
+  }
+};
+
+/// Diagnostic text, parsed into pieces.
+struct DiagText {
+private:
+  friend struct DiagnosticTextBuilder;
+  std::vector AllocatedPieces;
+  Piece *Root = nullptr;
+
+  template  T *create(Args &&... args) {
+static_assert(std::is_base_of::value, "must be piece");
+T *Mem = new T(std::forward(args)...);
+AllocatedPieces.push_back(Mem);
+return Mem;
+  }
+
+  DiagText() {}
+  Piece *parseDiagText(StringRef &Text, bool Nested = false);
+
+  DiagText(StringRef Text) : Root(nullptr) { Root = parseDi

[PATCH] D46522: [clang] cmake: resolve symlinks in ClangConfig.cmake

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

It works! Having to specify `CMAKE_PREFIX_PATH` is at least documentable, so I 
don't see that as a major drawback, especially if it makes docs for tools using 
Clang more portable.

Thank you for moving this along, now I can vastly simplify our CMake build.


Repository:
  rC Clang

https://reviews.llvm.org/D46522



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

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: test/TableGen/text-substitution.td:9
+
+// CHECK: AHHH
+def AGGG : Warning<"%select{one|two}0 is OK">, InGroup;

This test TBD. The `emit-diag-docs.td` test should hit all the major cases.


https://reviews.llvm.org/D46740



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

[PATCH] D46643: CodeGen: Emit string literal in constant address space

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 146468.
yaxunl marked an inline comment as done.
yaxunl added a comment.

Revised by John's comments. Also refactored to extract common code.


https://reviews.llvm.org/D46643

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/amdgcn-string-literal.cpp

Index: test/CodeGenCXX/amdgcn-string-literal.cpp
===
--- /dev/null
+++ test/CodeGenCXX/amdgcn-string-literal.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: @.str = private unnamed_addr addrspace(4) constant [6 x i8] c"g_str\00", align 1
+// CHECK: @g_str = addrspace(1) global i8* addrspacecast (i8 addrspace(4)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(4)* @.str, i32 0, i32 0) to i8*), align 8
+// CHECK: @g_array = addrspace(1) global [8 x i8] c"g_array\00", align 1
+// CHECK: @.str.1 = private unnamed_addr addrspace(4) constant [6 x i8] c"l_str\00", align 1
+// CHECK: @_ZZ1fvE7l_array = private unnamed_addr addrspace(4) constant [8 x i8] c"l_array\00", align 1
+
+const char* g_str = "g_str";
+char g_array[] = "g_array";
+
+void g(const char* p);
+
+// CHECK-LABEL: define void @_Z1fv()
+void f() {
+  const char* l_str = "l_str";
+  
+  // CHECK: call void @llvm.memcpy.p0i8.p4i8.i64
+  char l_array[] = "l_array";
+
+  g(g_str);
+  g(g_array);
+  g(l_str);
+  g(l_array);
+
+  const char* p = g_str;
+  g(p);
+}
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -785,6 +785,18 @@
  ForDefinition_t IsForDefinition
= NotForDefinition);
 
+  /// Return the AST address space of string literal, which is used to emit
+  /// the string literal as global variable in LLVM IR.
+  /// Note: This is not necessarily the address space of the string literal
+  /// in AST. For address space agnostic language, e.g. C++, string literal
+  /// in AST is always in default address space.
+  LangAS getStringLiteralAddressSpace() const;
+
+  /// Cast the string literal global variable to default address space when
+  /// necessary.
+  llvm::Constant *
+  castStringLiteralToDefaultAddressSpace(llvm::GlobalVariable *GV);
+
   /// Return the address of the given function. If Ty is non-null, then this
   /// function will use the specified type if it has to create it.
   llvm::Constant *GetAddrOfFunction(GlobalDecl GD, llvm::Type *Ty = nullptr,
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3044,6 +3044,38 @@
   return getTargetCodeGenInfo().getGlobalVarAddressSpace(*this, D);
 }
 
+LangAS CodeGenModule::getStringLiteralAddressSpace() const {
+  // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.
+  if (LangOpts.OpenCL)
+return LangAS::opencl_constant;
+  if (auto AS = getTarget().getConstantAddressSpace())
+return AS.getValue();
+  return LangAS::Default;
+}
+
+// In address space agnostic languages, string literals are in default address
+// space in AST. However, certain targets (e.g. amdgcn) request them to be
+// emitted in constant address space in LLVM IR. To be consistent with other
+// parts of AST, string literal global variables in constant address space
+// need to be casted to default address space before being put into address
+// map and referenced by other part of CodeGen.
+// In OpenCL, string literals are in constant address space in AST, therefore
+// they should not be casted to default address space.
+llvm::Constant *CodeGenModule::castStringLiteralToDefaultAddressSpace(
+llvm::GlobalVariable *GV) {
+  llvm::Constant *Cast = GV;
+  if (!LangOpts.OpenCL) {
+if (auto AS = getTarget().getConstantAddressSpace()) {
+  if (AS != LangAS::Default)
+Cast = getTargetCodeGenInfo().performAddrSpaceCast(
+*this, GV, AS.getValue(), LangAS::Default,
+GV->getValueType()->getPointerTo(
+getContext().getTargetAddressSpace(LangAS::Default)));
+}
+  }
+  return Cast;
+}
+
 template
 void CodeGenModule::MaybeHandleStaticInExternC(const SomeDecl *D,
llvm::GlobalValue *GV) {
@@ -4039,10 +4071,8 @@
 GenerateStringLiteral(llvm::Constant *C, llvm::GlobalValue::LinkageTypes LT,
   CodeGenModule &CGM, StringRef GlobalName,
   CharUnits Alignment) {
-  // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.
-  unsigned AddrSpace = 0;
-  if (CGM.getLangOpts().OpenCL)
-AddrSpace = CGM.getContext().getTargetAddressSpace(LangAS::opencl_constant);
+  unsigned AddrSpace = CGM.getContext().getTargetAddressSpace(
+  CGM.getStringLiteralAddressS

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 146470.
EricWF added a comment.

- Update tests.

The last thing I want to tackle is improving parsing errors to at least name 
the diagnostic being parsed. Naming *why* it's invalid if we don't provide its 
name.


https://reviews.llvm.org/D46740

Files:
  include/clang/Basic/Diagnostic.td
  test/TableGen/DiagnosticBase.inc
  test/TableGen/DiagnosticDocs.inc
  test/TableGen/emit-diag-docs.td
  test/TableGen/text-substitution.td
  test/lit.cfg.py
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -14,12 +14,14 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringToOffsetTable.h"
@@ -441,6 +443,717 @@
   }
 }
 
+namespace {
+enum PieceKind {
+  MultiPieceClass,
+  TextPieceClass,
+  PlaceholderPieceClass,
+  SelectPieceClass,
+  PluralPieceClass,
+  DiffPieceClass,
+  SubstitutionPieceClass,
+};
+
+enum ModifierType {
+  MT_Unknown,
+  MT_Placeholder,
+  MT_Select,
+  MT_Sub,
+  MT_Plural,
+  MT_Diff,
+  MT_Ordinal,
+  MT_S,
+  MT_Q,
+  MT_ObjCClass,
+  MT_ObjCInstance,
+};
+
+static StringRef getModifierName(ModifierType MT) {
+  switch (MT) {
+  case MT_Select:
+return "select";
+  case MT_Sub:
+return "sub";
+  case MT_Diff:
+return "diff";
+  case MT_Plural:
+return "plural";
+  case MT_Ordinal:
+return "ordinal";
+  case MT_S:
+return "s";
+  case MT_Q:
+return "q";
+  case MT_Placeholder:
+return "";
+  case MT_ObjCClass:
+return "objcclass";
+  case MT_ObjCInstance:
+return "objcinstance";
+  case MT_Unknown:
+return "<>";
+  }
+}
+
+struct Piece {
+  // This type and its derived classes are move-only.
+  Piece(PieceKind Kind) : ClassKind(Kind) {}
+  Piece(Piece const &O) = delete;
+  Piece &operator=(Piece const &) = delete;
+  virtual ~Piece() {}
+
+  PieceKind getPieceClass() const { return ClassKind; }
+  static bool classof(const Piece *) { return true; }
+
+private:
+  PieceKind ClassKind;
+};
+
+struct MultiPiece : Piece {
+  MultiPiece() : Piece(MultiPieceClass) {}
+  MultiPiece(std::vector Pieces)
+  : Piece(MultiPieceClass), Pieces(std::move(Pieces)) {}
+
+  std::vector Pieces;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == MultiPieceClass;
+  }
+};
+
+struct TextPiece : Piece {
+  StringRef Role;
+  std::string Text;
+  TextPiece(StringRef Text, StringRef Role = "")
+  : Piece(TextPieceClass), Role(Role), Text(Text.str()) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == TextPieceClass;
+  }
+};
+
+struct PlaceholderPiece : Piece {
+  ModifierType Kind;
+  int Index;
+  PlaceholderPiece(ModifierType Kind, int Index)
+  : Piece(PlaceholderPieceClass), Kind(Kind), Index(Index) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PlaceholderPieceClass;
+  }
+};
+
+struct SelectPiece : Piece {
+protected:
+  SelectPiece(PieceKind Kind, ModifierType ModKind)
+  : Piece(Kind), ModKind(ModKind) {}
+
+public:
+  SelectPiece(ModifierType ModKind) : SelectPiece(SelectPieceClass, ModKind) {}
+
+  ModifierType ModKind;
+  std::vector Options;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SelectPieceClass ||
+   P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct PluralPiece : SelectPiece {
+  PluralPiece() : SelectPiece(PluralPieceClass, MT_Plural) {}
+
+  std::vector OptionPrefixes;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct DiffPiece : Piece {
+  DiffPiece() : Piece(DiffPieceClass) {}
+
+  Piece *Options[2] = {};
+  unsigned Indexes[2] = {};
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == DiffPieceClass;
+  }
+};
+
+struct SubstitutionPiece : Piece {
+  SubstitutionPiece() : Piece(SubstitutionPieceClass) {}
+
+  std::string Name;
+  std::vector Modifiers;
+  Piece *Substitution = nullptr;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SubstitutionPieceClass;
+  }
+};
+
+/// Diagnostic text, parsed into pieces.
+
+
+struct DiagnosticTextBuilder {
+  DiagnosticTextBuilder(DiagnosticTextBuilder const &) = delete;
+  DiagnosticTextBuilder &operator=(DiagnosticTextBuilder const &) = delete;
+
+  DiagnosticTextBuilder(RecordKeeper &Records) {
+for (auto *S : Reco

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

@rsmith Can select indexes be negative? What's the correct type to represent 
them? Existing code seems to use `int`, but the LLVM style seems to suggest 
`unsigned` is more appropriate.




Comment at: utils/TableGen/ClangDiagnosticsEmitter.cpp:591
+  std::vector Modifiers;
+  Piece *Substitution = nullptr;
+

This is unused. Removing.


https://reviews.llvm.org/D46740



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

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[Elena Demikhovsky via Phabricator via cfe-commits]

delena added a comment.

In https://reviews.llvm.org/D46386#1096833, @rjmccall wrote:

> The actual semantic parts of the diff seem to have disappeared from the patch 
> posted to Phabricator, for what it's worth.


It is not disappeared by itself, I removed it. I understood that you don't see 
any added value in the entire memory model description inside.
Thank you.


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 146472.
aaron.ballman added a comment.

Addresses review comments.


https://reviews.llvm.org/D46112

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaType.cpp
  test/CodeGen/c11atomics.c
  test/Sema/atomic-type.c
  test/Sema/atomic-type.cpp

Index: test/Sema/atomic-type.cpp
===
--- test/Sema/atomic-type.cpp
+++ test/Sema/atomic-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S;
+
+void f(_Atomic S *s);
+
+struct S { int a, b; };
+
+void f(_Atomic S *s) {
+  S s2;
+  (void)__atomic_load(s, &s2, 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(S) *' invalid)}}
+  (void)__c11_atomic_load(s, 5);
+}
+
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,17 @@
 extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
 
 _Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2;
 _Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
 _Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
 _Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+  int i;
+  (void)__atomic_load(addr, &i, 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+  (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+  _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
   // we have to generate an atomic add, which returns the old value, and then a
   // non-atomic add.
   // CHECK: atomicrmw add i32* @i, i32 1 seq_cst
-  // CHECK: add i32 
+  // CHECK: add i32
   ++i;
   // CHECK: atomicrmw add i64* @l, i64 1 seq_cst
   // CHECK: add i64
@@ -475,6 +475,25 @@
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
 
+int test_atomic_void_ptr(_Atomic void *addr) {
+  // CHECK-LABEL: @test_atomic_void_ptr(
+  // CHECK:   [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+  // CHECK:   [[ATOMIC_RES:%.*]] = alloca i32, align 4
+  // CHECK:   store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+  // CHECK:   [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+  // CHECK:   [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+  // CHECK:   call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+  // CHECK:   [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+  // CHECK:   [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+  // CHECK:   [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+  // CHECK:   [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+  // CHECK:   store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+  // CHECK:   [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+  // CHECK:   ret i32 [[RES]]
+  (void)__atomic_is_lock_free(1, addr);
+  return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
 struct Empty {};
 
 struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
@@ -488,3 +507,25 @@
   // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %{{.*}}, i32 5)
   __c11_atomic_store(empty, value, 5);
 }
+
+typedef struct T T;
+void test_struct_copy(_Atomic(T) *t1, _Atomic(T) *t2);
+struct T { int a, b; };
+void test_struct_copy(_Atomic(T) *t1, _Atomic(T) *t2) {
+  // CHECK-LABEL: @test_struct_copy(
+  // CHECK:  [[T1:%.*]] = alloca %struct.T*, align 4
+  // CHECK:  [[T2:%.*]] = alloca %struct.T*, align 4
+  // CHECK:  [[TEMP:%.*]] = alloca %struct.T, align 8
+  // CHECK:  store %struct.T* %t1, %struct.T** [[T1]], align 4
+  // CHECK:  store %struct.T* %t2, %struct.T** [[T2]], align 4
+  // CHECK:  [[LOAD1:%.*]] = load %struct.T*, %struct.T** [[T1]], align 4
+  // CHECK:  [[LOAD2:%.*]] = load %struct.T*, %struct.T** [[T2]], align 4
+  // CHECK:  [[CAST1:%.*]] = bitcast %struct.T* [[LOAD2]] to i8*
+  // CHECK:  [[CAST2:%.*]] = bitcast %struct.T* [[TEMP]] to i8*
+  // CHECK:  call arm_aapcscc void @__atomic_load(i3

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 146474.
aaron.ballman added a comment.

Modifications based on review feedback.


https://reviews.llvm.org/D46112

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/Type.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaType.cpp
  test/CodeGen/c11atomics.c
  test/Sema/atomic-type.c
  test/SemaCXX/atomic-type.cpp

Index: test/SemaCXX/atomic-type.cpp
===
--- test/SemaCXX/atomic-type.cpp
+++ test/SemaCXX/atomic-type.cpp
@@ -2,17 +2,17 @@
 // RUN: %clang_cc1 -verify -pedantic %s -std=c++11
 
 template struct atomic {
-  _Atomic(T) value;
+  _Atomic(T) value; // expected-error {{field has incomplete type '_Atomic(user::inner)'}}
 
   void f() _Atomic; // expected-error {{expected ';' at end of declaration list}}
 };
 
 template struct user {
   struct inner { char n[sizeof(T)]; };
-  atomic i;
+  atomic i; // expected-note {{in instantiation}}
 };
 
-user u;
+user u; // expected-note {{in instantiation}}
 
 // Test overloading behavior of atomics.
 struct A { };
@@ -94,3 +94,15 @@
 bool PR21836(_Atomic(int) *x) {
 return *x;
 }
+
+struct T;
+
+void f(_Atomic T *t);
+
+struct T { int a, b; };
+
+void f(_Atomic T *t) {
+  T t2;
+  (void)__atomic_load(t, &t2, 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(T) *' invalid)}}
+  (void)__c11_atomic_load(t, 5);
+}
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,22 @@
 extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
 
 _Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2; // expected-error {{tentative definition has type '_Atomic(struct ErrorS)' that is never completed}}
 _Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
 _Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
 _Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+  int i;
+  (void)__atomic_load(addr, &i, 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+  (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+  _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
+
+typedef struct S S;
+void test_atomic_incomplete_struct_assignment(_Atomic S *s, _Atomic S *s2) {
+  *s = *s2; // expected-error {{incomplete type '_Atomic(S)' is not assignable}}
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
   // we have to generate an atomic add, which returns the old value, and then a
   // non-atomic add.
   // CHECK: atomicrmw add i32* @i, i32 1 seq_cst
-  // CHECK: add i32 
+  // CHECK: add i32
   ++i;
   // CHECK: atomicrmw add i64* @l, i64 1 seq_cst
   // CHECK: add i64
@@ -475,6 +475,25 @@
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
 
+int test_atomic_void_ptr(_Atomic void *addr) {
+  // CHECK-LABEL: @test_atomic_void_ptr(
+  // CHECK:   [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+  // CHECK:   [[ATOMIC_RES:%.*]] = alloca i32, align 4
+  // CHECK:   store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+  // CHECK:   [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+  // CHECK:   [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+  // CHECK:   call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+  // CHECK:   [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+  // CHECK:   [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+  // CHECK:   [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+  // CHECK:   [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+  // CHECK:   store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+  // CHECK:   [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+  // CHECK:   ret i32 [[RES]]
+  (void)__atomic_is_lock_free(1, addr);
+  return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
 struct Empty {};
 
 struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
@@ -488,3 +507,25 @@
   // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 ze

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D46112#1091981, @efriedma wrote:

> I think the request was that we check that a type is trivially copyable when 
> we perform an atomic operation?  I don't see the code for that anywhere.


Sorry about that -- I didn't notice that GNU was handled while C11 was not. 
That's been updated now.

> Also needs some test coverage for atomic operations which aren't calls, like 
> "typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2; };".

Thank you for pointing this out -- that uncovered an issue where we were not 
properly diagnosing the types as being incomplete. I've added a new test case 
and rolled the contents of Sema\atomic-type.cpp (which I added in an earlier 
patch) into SemaCXX\atomic-type.cpp (which already existed and I missed it).

I believe the change I made to `Type::isIncompleteType()` is correct, but was 
surprised by the new behavior in SemaCXX/atomic-type.cpp that resulted. It 
appears that the `RecordDecl` for "struct inner" has `IsCompleteDefinition` set 
to `false` while instantiating `struct atomic`, but I'm not familiar enough 
with the template instantiation process to know whether this is reasonable or 
not.


https://reviews.llvm.org/D46112



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

[PATCH] D46795: [clangd] Don't query index when completing inside classes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added subscribers: jkorous, MaskRay, klimek.

We used to query the index when completing after class qualifiers,
e.g. 'ClassName::^'. We should not do that for the same reasons we
don't query the index for member access expressions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46795

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -825,6 +825,67 @@
   UnorderedElementsAre("";
 }
 
+TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
+  // clang-format off
+  auto Completions = completions(R"cpp(
+struct Foo {
+  int SomeNameOfField;
+  typedef int SomeNameOfTypedefField;
+};
+
+Foo::^)cpp",
+{func("::SomeNameInTheIndex")}).items;
+  // clang-format on
+
+  EXPECT_THAT(Completions, AllOf(Contains(Labeled("SomeNameOfField")),
+ Contains(Labeled("SomeNameOfTypedefField")),
+ Not(Contains(Labeled("SomeNameInTheIndex");
+}
+
+TEST(CompletionTest, NoIndexCompletionsInsideDependentCode) {
+  {
+// clang-format off
+auto Completions = completions(R"cpp(
+  template 
+  void foo() {
+T::^
+  }
+  )cpp",
+  {func("::SomeNameInTheIndex")}).items;
+// clang-format on
+
+EXPECT_THAT(Completions, Not(Contains(Labeled("SomeNameInTheIndex";
+  }
+
+  {
+// clang-format off
+auto Completions = completions(R"cpp(
+  template 
+  void foo() {
+T::template Y::^
+  }
+  )cpp",
+  {func("::SomeNameInTheIndex")}).items;
+// clang-format on
+
+EXPECT_THAT(Completions, Not(Contains(Labeled("SomeNameInTheIndex";
+  }
+
+  {
+// clang-format off
+auto Completions = completions(R"cpp(
+  template 
+  void foo() {
+T::foo::^
+  }
+  )cpp",
+  {func("::SomeNameInTheIndex")}).items;
+// clang-format on
+
+EXPECT_THAT(Completions, Not(Contains(Labeled("SomeNameInTheIndex";
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -759,9 +759,9 @@
   return true;
 }
 
-// Should we perform index-based completion in this context?
+// Should we perform index-based completion in a context of the specified kind?
 // FIXME: consider allowing completion, but restricting the result types.
-bool allowIndex(enum CodeCompletionContext::Kind K) {
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
   switch (K) {
   case CodeCompletionContext::CCC_TopLevel:
   case CodeCompletionContext::CCC_ObjCInterface:
@@ -803,6 +803,33 @@
   llvm_unreachable("unknown code completion context");
 }
 
+// Should we allow index completions in the specified context?
+bool allowIndex(CodeCompletionContext &CC) {
+  if (!contextAllowsIndex(CC.getKind()))
+return false;
+  auto Scope = CC.getCXXScopeSpecifier();
+  if (!Scope)
+return true;
+  NestedNameSpecifier *NameSpec = (*Scope)->getScopeRep();
+  if (!NameSpec)
+return true;
+  // We only query the index when qualifier is a namespace.
+  // If it's a class, we rely solely on sema completions.
+  switch (NameSpec->getKind()) {
+  case NestedNameSpecifier::Global:
+  case NestedNameSpecifier::Namespace:
+  case NestedNameSpecifier::NamespaceAlias:
+return true;
+  case NestedNameSpecifier::Super:
+  case NestedNameSpecifier::TypeSpec:
+  case NestedNameSpecifier::TypeSpecWithTemplate:
+  // Note that Identifier can only appear in dependent scopes and they never
+  // refer to namespaces.
+  case NestedNameSpecifier::Identifier:
+return false;
+  }
+}
+
 } // namespace
 
 clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
@@ -918,7 +945,7 @@
   }
 
   SymbolSlab queryIndex() {
-if (!Opts.Index || !allowIndex(Recorder->CCContext.getKind()))
+if (!Opts.Index || !allowIndex(Recorder->CCContext))
   return SymbolSlab();
 trace::Span Tracer("Query index");
 SPAN_ATTACH(Tracer, "limit", Opts.Limit);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 146491.
EricWF edited the summary of this revision.
EricWF added a comment.

- Get argument renumbering work as @rsmith requested.
- Make the changes to existing diagnostics whitespace correct.
- Cleanup `DiagnosticSemaKinds.td` and `SemaOverload.cpp` to take advantage of 
this change.

@rsmith, @rjmccall: This change set is a lot larger now. Is it appropriate to 
commit the Sema cleanup with this patch?


https://reviews.llvm.org/D46740

Files:
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Basic/Diagnostic.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/anonymous-struct.cpp
  test/SemaCXX/cxx98-compat.cpp
  test/TableGen/DiagnosticBase.inc
  test/TableGen/DiagnosticDocs.inc
  test/TableGen/emit-diag-docs.td
  test/TableGen/text-substitution.td
  test/lit.cfg.py
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -14,12 +14,13 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringToOffsetTable.h"
@@ -441,6 +442,722 @@
   }
 }
 
+namespace {
+enum PieceKind {
+  MultiPieceClass,
+  TextPieceClass,
+  PlaceholderPieceClass,
+  SelectPieceClass,
+  PluralPieceClass,
+  DiffPieceClass,
+  SubstitutionPieceClass,
+};
+
+enum ModifierType {
+  MT_Unknown,
+  MT_Placeholder,
+  MT_Select,
+  MT_Sub,
+  MT_Plural,
+  MT_Diff,
+  MT_Ordinal,
+  MT_S,
+  MT_Q,
+  MT_ObjCClass,
+  MT_ObjCInstance,
+};
+
+static StringRef getModifierName(ModifierType MT) {
+  switch (MT) {
+  case MT_Select:
+return "select";
+  case MT_Sub:
+return "sub";
+  case MT_Diff:
+return "diff";
+  case MT_Plural:
+return "plural";
+  case MT_Ordinal:
+return "ordinal";
+  case MT_S:
+return "s";
+  case MT_Q:
+return "q";
+  case MT_Placeholder:
+return "";
+  case MT_ObjCClass:
+return "objcclass";
+  case MT_ObjCInstance:
+return "objcinstance";
+  case MT_Unknown:
+llvm_unreachable("invalid modifier type");
+  }
+}
+
+struct Piece {
+  // This type and its derived classes are move-only.
+  Piece(PieceKind Kind) : ClassKind(Kind) {}
+  Piece(Piece const &O) = delete;
+  Piece &operator=(Piece const &) = delete;
+  virtual ~Piece() {}
+
+  PieceKind getPieceClass() const { return ClassKind; }
+  static bool classof(const Piece *) { return true; }
+
+private:
+  PieceKind ClassKind;
+};
+
+struct MultiPiece : Piece {
+  MultiPiece() : Piece(MultiPieceClass) {}
+  MultiPiece(std::vector Pieces)
+  : Piece(MultiPieceClass), Pieces(std::move(Pieces)) {}
+
+  std::vector Pieces;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == MultiPieceClass;
+  }
+};
+
+struct TextPiece : Piece {
+  StringRef Role;
+  std::string Text;
+  TextPiece(StringRef Text, StringRef Role = "")
+  : Piece(TextPieceClass), Role(Role), Text(Text.str()) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == TextPieceClass;
+  }
+};
+
+struct PlaceholderPiece : Piece {
+  ModifierType Kind;
+  int Index;
+  PlaceholderPiece(ModifierType Kind, int Index)
+  : Piece(PlaceholderPieceClass), Kind(Kind), Index(Index) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PlaceholderPieceClass;
+  }
+};
+
+struct SelectPiece : Piece {
+protected:
+  SelectPiece(PieceKind Kind, ModifierType ModKind)
+  : Piece(Kind), ModKind(ModKind) {}
+
+public:
+  SelectPiece(ModifierType ModKind) : SelectPiece(SelectPieceClass, ModKind) {}
+
+  ModifierType ModKind;
+  std::vector Options;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SelectPieceClass ||
+   P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct PluralPiece : SelectPiece {
+  PluralPiece() : SelectPiece(PluralPieceClass, MT_Plural) {}
+
+  std::vector OptionPrefixes;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct DiffPiece : Piece {
+  DiffPiece() : Piece(DiffPieceClass) {}
+
+  Piece *Options[2] = {};
+  unsigned Indexes[2] = {};
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == DiffPieceClass;
+  }
+};
+
+struct SubstitutionPiece : Piece {
+  SubstitutionPiece() : Piece(SubstitutionPieceClass) {}
+
+  std::string Name;
+  std::vector Modifiers;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == Substit

[PATCH] D23041: Un-XFAIL GCC atomics.align

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

OK, here's what I would do:

(1) Split the vector, nullptr, and other weirdly failing tests into their own 
files.
(2) In each file, use `defined(TEST_COMPILER_GCC) && __GXX_ABI_VERSION > 1006` 
(or w/e version is broken) to define a macro when the test is expected to fail.
(3) Either `ifdef` out failing tests, or change the assertion to expect 
failure. The latter is probably preferable as it will allow us to detect if 
they test ever starts unexpectedly passing.
(4) Add tests for GCC with different `fabi-version` options. I would make these 
separate test files which wrap the actual test their targeting. For example:

  //
  // This file is dual licensed under the MIT and the University of Illinois 
Open
  // Source Licenses. See LICENSE.TXT for details.
  //
  
//===--===//
  //
  // REQUIRES: gcc, libatomic
  // UNSUPPORTED: libcpp-has-no-threads, c++98, c++03
  //
  // RUN: %cxx -o %t.one.exe %s %all_flags -latomic -fabi-version=2 && 
%t.one.exe
  // RUN: %cxx -o %t.two.exe %s %all_flags -latomic -fabi-version=6 && 
%t.two.exe
  // RUN: %cxx -o %t.three.exe %s %all_flags -latomic -fabi-version=9 && 
%t.three.exe
  
  #include "align-vector-types.sh.cpp"

How does that sound?


https://reviews.llvm.org/D23041



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

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

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

rsmith added inline comments.



Comment at: test/SemaCXX/atomic-type.cpp:5
 template struct atomic {
-  _Atomic(T) value;
+  _Atomic(T) value; // expected-error {{field has incomplete type 
'_Atomic(user::inner)'}}
 

This is a bug. You need to teach `RequireCompleteType` to look through atomic 
types when looking for a class type to instantiate.


https://reviews.llvm.org/D46112



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

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 146495.
EricWF added a comment.

Document `%sub` in `InternalsManual.rst`


https://reviews.llvm.org/D46740

Files:
  docs/InternalsManual.rst
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Basic/Diagnostic.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/anonymous-struct.cpp
  test/SemaCXX/cxx98-compat.cpp
  test/TableGen/DiagnosticBase.inc
  test/TableGen/DiagnosticDocs.inc
  test/TableGen/emit-diag-docs.td
  test/TableGen/text-substitution.td
  test/lit.cfg.py
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -14,12 +14,13 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringToOffsetTable.h"
@@ -441,6 +442,722 @@
   }
 }
 
+namespace {
+enum PieceKind {
+  MultiPieceClass,
+  TextPieceClass,
+  PlaceholderPieceClass,
+  SelectPieceClass,
+  PluralPieceClass,
+  DiffPieceClass,
+  SubstitutionPieceClass,
+};
+
+enum ModifierType {
+  MT_Unknown,
+  MT_Placeholder,
+  MT_Select,
+  MT_Sub,
+  MT_Plural,
+  MT_Diff,
+  MT_Ordinal,
+  MT_S,
+  MT_Q,
+  MT_ObjCClass,
+  MT_ObjCInstance,
+};
+
+static StringRef getModifierName(ModifierType MT) {
+  switch (MT) {
+  case MT_Select:
+return "select";
+  case MT_Sub:
+return "sub";
+  case MT_Diff:
+return "diff";
+  case MT_Plural:
+return "plural";
+  case MT_Ordinal:
+return "ordinal";
+  case MT_S:
+return "s";
+  case MT_Q:
+return "q";
+  case MT_Placeholder:
+return "";
+  case MT_ObjCClass:
+return "objcclass";
+  case MT_ObjCInstance:
+return "objcinstance";
+  case MT_Unknown:
+llvm_unreachable("invalid modifier type");
+  }
+}
+
+struct Piece {
+  // This type and its derived classes are move-only.
+  Piece(PieceKind Kind) : ClassKind(Kind) {}
+  Piece(Piece const &O) = delete;
+  Piece &operator=(Piece const &) = delete;
+  virtual ~Piece() {}
+
+  PieceKind getPieceClass() const { return ClassKind; }
+  static bool classof(const Piece *) { return true; }
+
+private:
+  PieceKind ClassKind;
+};
+
+struct MultiPiece : Piece {
+  MultiPiece() : Piece(MultiPieceClass) {}
+  MultiPiece(std::vector Pieces)
+  : Piece(MultiPieceClass), Pieces(std::move(Pieces)) {}
+
+  std::vector Pieces;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == MultiPieceClass;
+  }
+};
+
+struct TextPiece : Piece {
+  StringRef Role;
+  std::string Text;
+  TextPiece(StringRef Text, StringRef Role = "")
+  : Piece(TextPieceClass), Role(Role), Text(Text.str()) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == TextPieceClass;
+  }
+};
+
+struct PlaceholderPiece : Piece {
+  ModifierType Kind;
+  int Index;
+  PlaceholderPiece(ModifierType Kind, int Index)
+  : Piece(PlaceholderPieceClass), Kind(Kind), Index(Index) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PlaceholderPieceClass;
+  }
+};
+
+struct SelectPiece : Piece {
+protected:
+  SelectPiece(PieceKind Kind, ModifierType ModKind)
+  : Piece(Kind), ModKind(ModKind) {}
+
+public:
+  SelectPiece(ModifierType ModKind) : SelectPiece(SelectPieceClass, ModKind) {}
+
+  ModifierType ModKind;
+  std::vector Options;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SelectPieceClass ||
+   P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct PluralPiece : SelectPiece {
+  PluralPiece() : SelectPiece(PluralPieceClass, MT_Plural) {}
+
+  std::vector OptionPrefixes;
+  unsigned Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct DiffPiece : Piece {
+  DiffPiece() : Piece(DiffPieceClass) {}
+
+  Piece *Options[2] = {};
+  unsigned Indexes[2] = {};
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == DiffPieceClass;
+  }
+};
+
+struct SubstitutionPiece : Piece {
+  SubstitutionPiece() : Piece(SubstitutionPieceClass) {}
+
+  std::string Name;
+  std::vector Modifiers;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SubstitutionPieceClass;
+  }
+};
+
+/// Diagnostic text, parsed into pieces.
+
+
+struct DiagnosticTextBuilder {
+  DiagnosticTextBuilder(DiagnosticTextBuilder const &) = delete;
+  DiagnosticTextBuilder &operator=(DiagnosticTextBuilder const &) = delete;
+
+  DiagnosticTextBuilder(RecordKeeper &Records) {
+/

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

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

rsmith added a comment.

Thanks, this is great.

In https://reviews.llvm.org/D46740#1096859, @EricWF wrote:

> Turns out we have to fully parse the diagnostic text in order to substitute 
> modifier indexes, which is a bit of a complication. This patch does exactly 
> that.


I like that you were able to reuse the existing parsing code for documentation 
generation here.

In https://reviews.llvm.org/D46740#1096868, @EricWF wrote:

> @rsmith Can select indexes be negative? What's the correct type to represent 
> them? Existing code seems to use `int`, but the LLVM style seems to suggest 
> `unsigned` is more appropriate.


No, select indexes must be in the range 0... I don't think 
we actually have a convention of using `unsigned` for integers that can't be 
negative, and I suspect that if we thought about a policy we'd pick the 
opposite one :)

In https://reviews.llvm.org/D46740#1096972, @EricWF wrote:

> @rsmith, @rjmccall: This change set is a lot larger now. Is it appropriate to 
> commit the Sema cleanup with this patch?


I would ideally like to see one substitution added, and one set of diagnostics 
converted, as part of this patch, and for other diagnostics to be converted in 
separate changes after that.




Comment at: docs/InternalsManual.rst:337
+def note_ovl_candidate : Note<
+  "candidate %{select_ovl_candidate}3,2,1 not viable">;
+

Missing the `sub` specifier here?



Comment at: docs/InternalsManual.rst:341-342
+  ``"candidate %select{function|constructor}3%select{| template| %1}2 not 
viable"``.
+Class:
+  ``Integers``
+Description:

I don't think "Class:" makes sense here, since what this specifier consumes 
depends on its replacement string. "Class: varies" or similar might work, but 
maybe just drop it?



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3526-3544
 def note_ovl_candidate : Note<"candidate "
 "%select{function|function|constructor|"
-"function |function |constructor |"
 "is the implicit default constructor|"
 "is the implicit copy constructor|"
 "is the implicit move constructor|"
 "is the implicit copy assignment operator|"
 "is the implicit move assignment operator|"

Is there a reason this one wasn't changed to use `%sub`?



Comment at: test/TableGen/DiagnosticBase.inc:1
-// Define the diagnostic mappings.
-class DiagMapping;
-def MAP_IGNORE  : DiagMapping;
-def MAP_WARNING : DiagMapping;
-def MAP_ERROR   : DiagMapping;
-def MAP_FATAL   : DiagMapping;
+//===--- Diagnostic.td - C Language Family Diagnostic Handling 
===//
+//

This is the wrong filename.


https://reviews.llvm.org/D46740



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

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

rsmith added a comment.

In https://reviews.llvm.org/D46441#1096874, @AntonBikineev wrote:

> Please notice, that this is not the only place where the position of the 
> `this` pointer is assumed to always be first. As an example, another such 
> place is here .


Actually, that is just building the sequence of Clang types for the parameters. 
The mapping to LLVM arguments is still done via `ClangToLLVMArgMapping` when we 
build the LLVM IR function type. So at least that case is correct. I found 
this, which appears to be making a potentially-unsafe assumption: 
https://clang.llvm.org/doxygen/CGObjCGNU_8cpp_source.html#l00664 -- but almost 
all of CodeGen gets this right already.

>> I think this standard text is in error and UB would be appropriate, but we 
>> should at least discuss whether we want to take this additional liberty not 
>> technically granted by the standard.
> 
> Should we then file an issue to the Core?
>  BTW, gcc already performs this opt since 8.1: 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82899#c10

This seems like a pretty good argument for this being a wording defect. Yes, we 
should file a core issue.


https://reviews.llvm.org/D46441



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

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 4 inline comments as done.
EricWF added a comment.

In https://reviews.llvm.org/D46740#1097014, @rsmith wrote:

> Thanks, this is great.
>
> In https://reviews.llvm.org/D46740#1096859, @EricWF wrote:
>
> > Turns out we have to fully parse the diagnostic text in order to substitute 
> > modifier indexes, which is a bit of a complication. This patch does exactly 
> > that.
>
>
> I like that you were able to reuse the existing parsing code for 
> documentation generation here.


With heavy modifications  :-)




Comment at: docs/InternalsManual.rst:341-342
+  ``"candidate %select{function|constructor}3%select{| template| %1}2 not 
viable"``.
+Class:
+  ``Integers``
+Description:

rsmith wrote:
> I don't think "Class:" makes sense here, since what this specifier consumes 
> depends on its replacement string. "Class: varies" or similar might work, but 
> maybe just drop it?
Dropping it seems clearer.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3526-3544
 def note_ovl_candidate : Note<"candidate "
 "%select{function|function|constructor|"
-"function |function |constructor |"
 "is the implicit default constructor|"
 "is the implicit copy constructor|"
 "is the implicit move constructor|"
 "is the implicit copy assignment operator|"
 "is the implicit move assignment operator|"

rsmith wrote:
> Is there a reason this one wasn't changed to use `%sub`?
It refers to implicit special members differently ("is the implicit default 
constructor" -> "constructor (the implicit default constructor"). I think I 
mistakenly thought transforming it would be grammatically incorrect. But 
looking further it seems fine.



https://reviews.llvm.org/D46740



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

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

rsmith added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:2513
+  cast(D)->isCopyOrMoveConstructor())
+F->addParamAttr(1, llvm::Attribute::NoAlias);
+

rsmith wrote:
> It's not strictly correct to assume that you know the correspondence between 
> `llvm::Function` parameter indices and those of the `clang::FunctionDecl` 
> like this. That's up to the ABI, and for constructors in particular, there 
> may be a VTT parameter to deal with (and `__attribute__((pass_object_size))` 
> can also interfere). You can probably get away with assuming that the `this` 
> parameter has index 0 for now, but even that is likely to break at some point 
> down the line.
The best way to deal with this is to create the attribute in 
`CodeGenModule::ConstructAttributeList`, where you have access to the 
`ClangToLLVMArgMapping`.


https://reviews.llvm.org/D46441



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

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D46740#1097014, @rsmith wrote:

> Thanks, this is great.
>
> In https://reviews.llvm.org/D46740#1096859, @EricWF wrote:
>
> > Turns out we have to fully parse the diagnostic text in order to substitute 
> > modifier indexes, which is a bit of a complication. This patch does exactly 
> > that.
>
>
> I like that you were able to reuse the existing parsing code for 
> documentation generation here.


With heavy modifications  :-)


https://reviews.llvm.org/D46740



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

[PATCH] D46740: [Clang Tablegen][RFC] Allow Early Textual Substitutions in `Diagnostic` messages.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 146498.
EricWF marked 2 inline comments as done.
EricWF added a comment.

Address @rsmiths comments.

- Prefer `int` to `unsigned`.
- Substitute `note_ovl_candidate`.
- Fix issues in docs.


https://reviews.llvm.org/D46740

Files:
  docs/InternalsManual.rst
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Basic/Diagnostic.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/anonymous-struct.cpp
  test/SemaCXX/cxx98-compat.cpp
  test/TableGen/DiagnosticBase.inc
  test/TableGen/DiagnosticDocs.inc
  test/TableGen/emit-diag-docs.td
  test/TableGen/text-substitution.td
  test/lit.cfg.py
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -14,12 +14,13 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringToOffsetTable.h"
@@ -441,6 +442,722 @@
   }
 }
 
+namespace {
+enum PieceKind {
+  MultiPieceClass,
+  TextPieceClass,
+  PlaceholderPieceClass,
+  SelectPieceClass,
+  PluralPieceClass,
+  DiffPieceClass,
+  SubstitutionPieceClass,
+};
+
+enum ModifierType {
+  MT_Unknown,
+  MT_Placeholder,
+  MT_Select,
+  MT_Sub,
+  MT_Plural,
+  MT_Diff,
+  MT_Ordinal,
+  MT_S,
+  MT_Q,
+  MT_ObjCClass,
+  MT_ObjCInstance,
+};
+
+static StringRef getModifierName(ModifierType MT) {
+  switch (MT) {
+  case MT_Select:
+return "select";
+  case MT_Sub:
+return "sub";
+  case MT_Diff:
+return "diff";
+  case MT_Plural:
+return "plural";
+  case MT_Ordinal:
+return "ordinal";
+  case MT_S:
+return "s";
+  case MT_Q:
+return "q";
+  case MT_Placeholder:
+return "";
+  case MT_ObjCClass:
+return "objcclass";
+  case MT_ObjCInstance:
+return "objcinstance";
+  case MT_Unknown:
+llvm_unreachable("invalid modifier type");
+  }
+}
+
+struct Piece {
+  // This type and its derived classes are move-only.
+  Piece(PieceKind Kind) : ClassKind(Kind) {}
+  Piece(Piece const &O) = delete;
+  Piece &operator=(Piece const &) = delete;
+  virtual ~Piece() {}
+
+  PieceKind getPieceClass() const { return ClassKind; }
+  static bool classof(const Piece *) { return true; }
+
+private:
+  PieceKind ClassKind;
+};
+
+struct MultiPiece : Piece {
+  MultiPiece() : Piece(MultiPieceClass) {}
+  MultiPiece(std::vector Pieces)
+  : Piece(MultiPieceClass), Pieces(std::move(Pieces)) {}
+
+  std::vector Pieces;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == MultiPieceClass;
+  }
+};
+
+struct TextPiece : Piece {
+  StringRef Role;
+  std::string Text;
+  TextPiece(StringRef Text, StringRef Role = "")
+  : Piece(TextPieceClass), Role(Role), Text(Text.str()) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == TextPieceClass;
+  }
+};
+
+struct PlaceholderPiece : Piece {
+  ModifierType Kind;
+  int Index;
+  PlaceholderPiece(ModifierType Kind, int Index)
+  : Piece(PlaceholderPieceClass), Kind(Kind), Index(Index) {}
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PlaceholderPieceClass;
+  }
+};
+
+struct SelectPiece : Piece {
+protected:
+  SelectPiece(PieceKind Kind, ModifierType ModKind)
+  : Piece(Kind), ModKind(ModKind) {}
+
+public:
+  SelectPiece(ModifierType ModKind) : SelectPiece(SelectPieceClass, ModKind) {}
+
+  ModifierType ModKind;
+  std::vector Options;
+  int Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SelectPieceClass ||
+   P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct PluralPiece : SelectPiece {
+  PluralPiece() : SelectPiece(PluralPieceClass, MT_Plural) {}
+
+  std::vector OptionPrefixes;
+  int Index;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == PluralPieceClass;
+  }
+};
+
+struct DiffPiece : Piece {
+  DiffPiece() : Piece(DiffPieceClass) {}
+
+  Piece *Options[2] = {};
+  int Indexes[2] = {};
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == DiffPieceClass;
+  }
+};
+
+struct SubstitutionPiece : Piece {
+  SubstitutionPiece() : Piece(SubstitutionPieceClass) {}
+
+  std::string Name;
+  std::vector Modifiers;
+
+  static bool classof(const Piece *P) {
+return P->getPieceClass() == SubstitutionPieceClass;
+  }
+};
+
+/// Diagnostic text, parsed into pieces.
+
+
+struct DiagnosticTextBuilder {
+  DiagnosticTextBuilder(DiagnosticTextBuilder const &) = delete;
+  DiagnosticTextBuilder &operator=