[PATCH] D102614: [index] Add support for type of pointers to class members

2021-05-24 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie updated this revision to Diff 347581.
OikawaKirie added a comment.

Update the test case to avoid a crash in the Windows version of the 
`c-index-test` tool.


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

https://reviews.llvm.org/D102614

Files:
  clang/lib/Index/USRGeneration.cpp
  clang/test/Index/USR/MemberFunctionPtr.cpp


Index: clang/test/Index/USR/MemberFunctionPtr.cpp
===
--- /dev/null
+++ clang/test/Index/USR/MemberFunctionPtr.cpp
@@ -0,0 +1,33 @@
+// RUN: c-index-test -index-file %s | FileCheck %s
+
+struct C {
+  int X;
+  void f(char);
+};
+
+void f(int C::*) {}
+// CHECK: name: f | USR: c:@F@f#$@S@C::*I#
+void f(void (C::*)(char)) {}
+// CHECK: name: f | USR: c:@F@f#$@S@C::*Fv(#C)#
+
+typedef int C::*Xtd;
+void ftd(Xtd) {}
+// CHECK: name: ftd | USR: c:@F@ftd#$@S@C::*I#
+typedef void (C::*Ftd)(char);
+void ftd(Ftd) {}
+// CHECK: name: ftd | USR: c:@F@ftd#$@S@C::*Fv(#C)#
+
+using Xus = int C::*;
+void fus(Xus) {}
+// CHECK: name: fus | USR: c:@F@fus#$@S@C::*I#
+using Fus = void (C::*)(char);
+void fus(Fus) {}
+// CHECK: name: fus | USR: c:@F@fus#$@S@C::*Fv(#C)#
+
+template  struct S;
+template  struct S {
+  static const bool V = true;
+  // CHECK: name: V | USR: c:@SP>2#T#T@S>#t0.1::*t0.0@V
+  void f() {}
+  // CHECK: name: f | USR: c:@SP>2#T#T@S>#t0.1::*t0.0@F@f#
+};
Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -893,6 +893,12 @@
   T = AT->getElementType();
   continue;
 }
+if (const MemberPointerType *MPT = T->getAs()) {
+  VisitType(QualType(MPT->getClass(), 0));
+  Out << "::*";
+  T = MPT->getPointeeType();
+  continue;
+}
 
 // Unhandled type.
 Out << ' ';


Index: clang/test/Index/USR/MemberFunctionPtr.cpp
===
--- /dev/null
+++ clang/test/Index/USR/MemberFunctionPtr.cpp
@@ -0,0 +1,33 @@
+// RUN: c-index-test -index-file %s | FileCheck %s
+
+struct C {
+  int X;
+  void f(char);
+};
+
+void f(int C::*) {}
+// CHECK: name: f | USR: c:@F@f#$@S@C::*I#
+void f(void (C::*)(char)) {}
+// CHECK: name: f | USR: c:@F@f#$@S@C::*Fv(#C)#
+
+typedef int C::*Xtd;
+void ftd(Xtd) {}
+// CHECK: name: ftd | USR: c:@F@ftd#$@S@C::*I#
+typedef void (C::*Ftd)(char);
+void ftd(Ftd) {}
+// CHECK: name: ftd | USR: c:@F@ftd#$@S@C::*Fv(#C)#
+
+using Xus = int C::*;
+void fus(Xus) {}
+// CHECK: name: fus | USR: c:@F@fus#$@S@C::*I#
+using Fus = void (C::*)(char);
+void fus(Fus) {}
+// CHECK: name: fus | USR: c:@F@fus#$@S@C::*Fv(#C)#
+
+template  struct S;
+template  struct S {
+  static const bool V = true;
+  // CHECK: name: V | USR: c:@SP>2#T#T@S>#t0.1::*t0.0@V
+  void f() {}
+  // CHECK: name: f | USR: c:@SP>2#T#T@S>#t0.1::*t0.0@F@f#
+};
Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -893,6 +893,12 @@
   T = AT->getElementType();
   continue;
 }
+if (const MemberPointerType *MPT = T->getAs()) {
+  VisitType(QualType(MPT->getClass(), 0));
+  Out << "::*";
+  T = MPT->getPointeeType();
+  continue;
+}
 
 // Unhandled type.
 Out << ' ';
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

2021-05-24 Thread Thomas Lively via Phabricator via cfe-commits
tlively added a comment.

It would be good to add dedicated tests for table.get and table.set as well.




Comment at: llvm/lib/CodeGen/MachineOperand.cpp:1174
+  } else
+OS << ", opaque ";
   if (getAlign() != getBaseAlign())

pmatos wrote:
> tlively wrote:
> > Is there a test that demonstrates this printing? Also, I'm not sure 
> > specifically calling out zero-sized operands in the text dump is that 
> > useful. Can zero-sized operands still be given an alignment? If so, it 
> > might even be good to continue printing the alignment for them.
> The reason I changed this here is due to the call of getSize() crashing for 
> zeroSized arguments. I have not added test for the printing. I don't think 
> zero-sized operands can still be given an alignment. We are actually setting 
> the alignment to
Looks like this comment was cut off! I wonder if it would be better to skip 
printing `opaque` for now since we have no test of it/ Also, if another target 
comes along with a zero-sized type, I don't know whether "opaque" will 
necessarily describe it.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:558-559
 
+  // If this is a funcref call, to avoid hidden GC roots, we need to clear the
+  // table slot with ref.null upon call_indirect return.
+  if (IsIndirect && IsFuncrefCall) {

Can you add the .wat of the code being generated here to the comment?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:19
  [],
  "table.get\t$res, $table",
  "table.get\t$table",

pmatos wrote:
> tlively wrote:
> > wingo wrote:
> > > Not something for this patch, but this is certainly bogus: surely we mean 
> > > `table.get\t$table, $i` and we have an i32 index argument?
> > Agree about the i32 index argument, but it is correct to have the result as 
> > part of the string for the register-based output format.
> Not sure I understand why this should be `$i` instead of `$table`. Isn't 
> `$table` represented as an index anyway? A `table32_op` is defined as 
> `Operand` so I don't quite understand what we gain by changing this.
I would still expect to see a register for the result and immediate inputs for 
the table symbol and the table slot index here.



Comment at: llvm/test/CodeGen/WebAssembly/externref-undef.ll:13-14
+; CHECK-LABEL: return_extern_undef:
+; CHECK-NEXT: functype   return_extern_undef () -> ()
+; CHECK-NEXT: end_function
+

tlively wrote:
> pmatos wrote:
> > tlively wrote:
> > > I would expect this to declare an externref local and return it 
> > > uninitialized or to return a ref.null. It would be good to at least add a 
> > > TODO comment for fixing this.
> > Returning a `ref.null` or an uninitialized externref would make more sense 
> > if the return type would be `%externref`. However, in this case this is an 
> > `%extern` value, which really is an opaque type and should never really 
> > happen.
> Ah, that makes sense.
It would be good to add this explanation as a comment in the test file.



Comment at: llvm/test/CodeGen/WebAssembly/funcref-call.ll:15
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: table.set __funcref_call_table
+; CHECK-NEXT: local.get 0

Missing a table element index here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95425

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


[PATCH] D103036: [cfe][inline-asm] Support target-specific escaped character in inline asm

2021-05-24 Thread Min-Yih Hsu 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 rG6685a3f3e4c4: [cfe] Support target-specific escaped 
character in inline asm (authored by myhsu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103036

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/test/CodeGen/m68k-asm.c

Index: clang/test/CodeGen/m68k-asm.c
===
--- /dev/null
+++ clang/test/CodeGen/m68k-asm.c
@@ -0,0 +1,21 @@
+// REQUIRES: m68k-registered-target
+// RUN: %clang -target m68k -S %s -o - | FileCheck %s
+
+// Test special escaped character in inline assembly
+void escaped() {
+  // '.' -> '.'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move%.l #66, %%d1" ::);
+  // '#' -> '#'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l %#66, %%d1" ::);
+  // '/' -> '%'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l #66, %/d1" ::);
+  // '$' -> 's'
+  // CHECK: muls %d0, %d1
+  __asm__ ("mul%$ %%d0, %%d1" ::);
+  // '&' -> 'd'
+  // CHECK: move.l %d0, %d1
+  __asm__ ("move.l %%%&0, %%d1" ::);
+}
Index: clang/lib/Basic/Targets/M68k.h
===
--- clang/lib/Basic/Targets/M68k.h
+++ clang/lib/Basic/Targets/M68k.h
@@ -47,6 +47,7 @@
   std::string convertConstraint(const char *&Constraint) const override;
   bool validateAsmConstraint(const char *&Name,
  TargetInfo::ConstraintInfo &info) const override;
+  llvm::Optional handleAsmEscapedChar(char EscChar) const override;
   const char *getClobbers() const override;
   BuiltinVaListKind getBuiltinVaListKind() const override;
   bool setCPU(const std::string &Name) override;
Index: clang/lib/Basic/Targets/M68k.cpp
===
--- clang/lib/Basic/Targets/M68k.cpp
+++ clang/lib/Basic/Targets/M68k.cpp
@@ -191,6 +191,30 @@
   return false;
 }
 
+llvm::Optional
+M68kTargetInfo::handleAsmEscapedChar(char EscChar) const {
+  char C;
+  switch (EscChar) {
+  case '.':
+  case '#':
+C = EscChar;
+break;
+  case '/':
+C = '%';
+break;
+  case '$':
+C = 's';
+break;
+  case '&':
+C = 'd';
+break;
+  default:
+return llvm::None;
+  }
+
+  return std::string(1, C);
+}
+
 std::string M68kTargetInfo::convertConstraint(const char *&Constraint) const {
   if (*Constraint == 'C')
 // Two-character constraint; add "^" hint for later parsing
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -646,6 +646,8 @@
   continue;
 }
 
+const TargetInfo &TI = C.getTargetInfo();
+
 // Escaped "%" character in asm string.
 if (CurPtr == StrEnd) {
   // % at end of string is invalid (no escape).
@@ -656,6 +658,11 @@
 char EscapedChar = *CurPtr++;
 switch (EscapedChar) {
 default:
+  // Handle target-specific escaped characters.
+  if (auto MaybeReplaceStr = TI.handleAsmEscapedChar(EscapedChar)) {
+CurStringPiece += *MaybeReplaceStr;
+continue;
+  }
   break;
 case '%': // %% -> %
 case '{': // %{ -> {
@@ -688,7 +695,6 @@
   EscapedChar = *CurPtr++;
 }
 
-const TargetInfo &TI = C.getTargetInfo();
 const SourceManager &SM = C.getSourceManager();
 const LangOptions &LO = C.getLangOpts();
 
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1091,6 +1091,12 @@
 return std::string(1, *Constraint);
   }
 
+  /// Replace some escaped characters with another string based on
+  /// target-specific rules
+  virtual llvm::Optional handleAsmEscapedChar(char C) const {
+return llvm::None;
+  }
+
   /// Returns a string of target-specific clobbers, in LLVM format.
   virtual const char *getClobbers() const = 0;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 6685a3f - [cfe] Support target-specific escaped character in inline asm

2021-05-24 Thread Min-Yih Hsu via cfe-commits

Author: Min-Yih Hsu
Date: 2021-05-24T21:39:21-07:00
New Revision: 6685a3f3e4c497a3a0fd06aa4e77cb442325d1ba

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

LOG: [cfe] Support target-specific escaped character in inline asm

GCC allows each target to define a set of non-letter and non-digit
escaped characters for inline assembly that will be replaced by another
string (They call this "punctuation" characters. The existing "%%" and
"%{" -- replaced by '%' and '{' at the end -- can be seen as special
cases shared by all targets).
This patch implements this feature by adding a new hook in `TargetInfo`.

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

Added: 
clang/test/CodeGen/m68k-asm.c

Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/Stmt.cpp
clang/lib/Basic/Targets/M68k.cpp
clang/lib/Basic/Targets/M68k.h

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index 29b957607f3e5..d59bad30e7428 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1091,6 +1091,12 @@ class TargetInfo : public virtual 
TransferrableTargetInfo,
 return std::string(1, *Constraint);
   }
 
+  /// Replace some escaped characters with another string based on
+  /// target-specific rules
+  virtual llvm::Optional handleAsmEscapedChar(char C) const {
+return llvm::None;
+  }
+
   /// Returns a string of target-specific clobbers, in LLVM format.
   virtual const char *getClobbers() const = 0;
 

diff  --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index d30df296dbd57..47693ef9fee3e 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/lib/AST/Stmt.cpp
@@ -646,6 +646,8 @@ unsigned 
GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl&Pieces,
   continue;
 }
 
+const TargetInfo &TI = C.getTargetInfo();
+
 // Escaped "%" character in asm string.
 if (CurPtr == StrEnd) {
   // % at end of string is invalid (no escape).
@@ -656,6 +658,11 @@ unsigned 
GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl&Pieces,
 char EscapedChar = *CurPtr++;
 switch (EscapedChar) {
 default:
+  // Handle target-specific escaped characters.
+  if (auto MaybeReplaceStr = TI.handleAsmEscapedChar(EscapedChar)) {
+CurStringPiece += *MaybeReplaceStr;
+continue;
+  }
   break;
 case '%': // %% -> %
 case '{': // %{ -> {
@@ -688,7 +695,6 @@ unsigned 
GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl&Pieces,
   EscapedChar = *CurPtr++;
 }
 
-const TargetInfo &TI = C.getTargetInfo();
 const SourceManager &SM = C.getSourceManager();
 const LangOptions &LO = C.getLangOpts();
 

diff  --git a/clang/lib/Basic/Targets/M68k.cpp 
b/clang/lib/Basic/Targets/M68k.cpp
index 9fcd58ee6401a..31cb36d37636c 100644
--- a/clang/lib/Basic/Targets/M68k.cpp
+++ b/clang/lib/Basic/Targets/M68k.cpp
@@ -191,6 +191,30 @@ bool M68kTargetInfo::validateAsmConstraint(
   return false;
 }
 
+llvm::Optional
+M68kTargetInfo::handleAsmEscapedChar(char EscChar) const {
+  char C;
+  switch (EscChar) {
+  case '.':
+  case '#':
+C = EscChar;
+break;
+  case '/':
+C = '%';
+break;
+  case '$':
+C = 's';
+break;
+  case '&':
+C = 'd';
+break;
+  default:
+return llvm::None;
+  }
+
+  return std::string(1, C);
+}
+
 std::string M68kTargetInfo::convertConstraint(const char *&Constraint) const {
   if (*Constraint == 'C')
 // Two-character constraint; add "^" hint for later parsing

diff  --git a/clang/lib/Basic/Targets/M68k.h b/clang/lib/Basic/Targets/M68k.h
index be2462bbd7acd..a42ca674ef9cc 100644
--- a/clang/lib/Basic/Targets/M68k.h
+++ b/clang/lib/Basic/Targets/M68k.h
@@ -47,6 +47,7 @@ class LLVM_LIBRARY_VISIBILITY M68kTargetInfo : public 
TargetInfo {
   std::string convertConstraint(const char *&Constraint) const override;
   bool validateAsmConstraint(const char *&Name,
  TargetInfo::ConstraintInfo &info) const override;
+  llvm::Optional handleAsmEscapedChar(char EscChar) const 
override;
   const char *getClobbers() const override;
   BuiltinVaListKind getBuiltinVaListKind() const override;
   bool setCPU(const std::string &Name) override;

diff  --git a/clang/test/CodeGen/m68k-asm.c b/clang/test/CodeGen/m68k-asm.c
new file mode 100644
index 0..bfaf2d93ef2d2
--- /dev/null
+++ b/clang/test/CodeGen/m68k-asm.c
@@ -0,0 +1,21 @@
+// REQUIRES: m68k-registered-target
+// RUN: %clang -target m68k -S %s -o - | FileCheck %s
+
+// Test special escaped character in inline assembly
+void escaped() {
+  // '.' -> '.'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move%.l #66, %%d1" ::);
+  // '#' -> '#'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l %#66, %%d1" ::);
+  // '/' -> '%'
+  

[PATCH] D103036: [cfe][inline-asm] Support target-specific escaped character in inline asm

2021-05-24 Thread Min-Yih Hsu via Phabricator via cfe-commits
myhsu updated this revision to Diff 347574.
myhsu marked 3 inline comments as done.
myhsu added a comment.

Fix formatting


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

https://reviews.llvm.org/D103036

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/test/CodeGen/m68k-asm.c

Index: clang/test/CodeGen/m68k-asm.c
===
--- /dev/null
+++ clang/test/CodeGen/m68k-asm.c
@@ -0,0 +1,21 @@
+// REQUIRES: m68k-registered-target
+// RUN: %clang -target m68k -S %s -o - | FileCheck %s
+
+// Test special escaped character in inline assembly
+void escaped() {
+  // '.' -> '.'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move%.l #66, %%d1" ::);
+  // '#' -> '#'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l %#66, %%d1" ::);
+  // '/' -> '%'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l #66, %/d1" ::);
+  // '$' -> 's'
+  // CHECK: muls %d0, %d1
+  __asm__ ("mul%$ %%d0, %%d1" ::);
+  // '&' -> 'd'
+  // CHECK: move.l %d0, %d1
+  __asm__ ("move.l %%%&0, %%d1" ::);
+}
Index: clang/lib/Basic/Targets/M68k.h
===
--- clang/lib/Basic/Targets/M68k.h
+++ clang/lib/Basic/Targets/M68k.h
@@ -47,6 +47,7 @@
   std::string convertConstraint(const char *&Constraint) const override;
   bool validateAsmConstraint(const char *&Name,
  TargetInfo::ConstraintInfo &info) const override;
+  llvm::Optional handleAsmEscapedChar(char EscChar) const override;
   const char *getClobbers() const override;
   BuiltinVaListKind getBuiltinVaListKind() const override;
   bool setCPU(const std::string &Name) override;
Index: clang/lib/Basic/Targets/M68k.cpp
===
--- clang/lib/Basic/Targets/M68k.cpp
+++ clang/lib/Basic/Targets/M68k.cpp
@@ -191,6 +191,30 @@
   return false;
 }
 
+llvm::Optional
+M68kTargetInfo::handleAsmEscapedChar(char EscChar) const {
+  char C;
+  switch (EscChar) {
+  case '.':
+  case '#':
+C = EscChar;
+break;
+  case '/':
+C = '%';
+break;
+  case '$':
+C = 's';
+break;
+  case '&':
+C = 'd';
+break;
+  default:
+return llvm::None;
+  }
+
+  return std::string(1, C);
+}
+
 std::string M68kTargetInfo::convertConstraint(const char *&Constraint) const {
   if (*Constraint == 'C')
 // Two-character constraint; add "^" hint for later parsing
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -646,6 +646,8 @@
   continue;
 }
 
+const TargetInfo &TI = C.getTargetInfo();
+
 // Escaped "%" character in asm string.
 if (CurPtr == StrEnd) {
   // % at end of string is invalid (no escape).
@@ -656,6 +658,11 @@
 char EscapedChar = *CurPtr++;
 switch (EscapedChar) {
 default:
+  // Handle target-specific escaped characters.
+  if (auto MaybeReplaceStr = TI.handleAsmEscapedChar(EscapedChar)) {
+CurStringPiece += *MaybeReplaceStr;
+continue;
+  }
   break;
 case '%': // %% -> %
 case '{': // %{ -> {
@@ -688,7 +695,6 @@
   EscapedChar = *CurPtr++;
 }
 
-const TargetInfo &TI = C.getTargetInfo();
 const SourceManager &SM = C.getSourceManager();
 const LangOptions &LO = C.getLangOpts();
 
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1091,6 +1091,12 @@
 return std::string(1, *Constraint);
   }
 
+  /// Replace some escaped characters with another string based on
+  /// target-specific rules
+  virtual llvm::Optional handleAsmEscapedChar(char C) const {
+return llvm::None;
+  }
+
   /// Returns a string of target-specific clobbers, in LLVM format.
   virtual const char *getClobbers() const = 0;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D102338: [Sema] Always search the full function scope context if a potential availability violation is encountered

2021-05-24 Thread Logan Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa5a3efa82a77: [Sema] Always search the full function scope 
context if a potential… (authored by logan-5).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102338

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/unguarded-availability.m

Index: clang/test/SemaObjC/unguarded-availability.m
===
--- clang/test/SemaObjC/unguarded-availability.m
+++ clang/test/SemaObjC/unguarded-availability.m
@@ -125,6 +125,14 @@
   (void) ^{
 func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   };
+
+  if (@available(macos 10.12, *))
+(void) ^{
+  func_10_12();
+  (void) ^{
+func_10_12();
+  };
+};
 }
 
 void test_params(int_10_12 x); // expected-warning {{'int_10_12' is only available on macOS 10.12 or newer}} expected-note{{annotate 'test_params' with an availability attribute to silence this warning}}
@@ -233,6 +241,36 @@
   (void)(^ {
 func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   });
+
+  if (@available(macos 10.12, *)) {
+void([]() {
+  func_10_12();
+  void([] () {
+func_10_12();
+  });
+  struct DontWarn {
+void f() {
+  func_10_12();
+}
+  };
+});
+  }
+
+  if (@available(macos 10.12, *)) {
+struct DontWarn {
+  void f() {
+func_10_12();
+void([] () {
+  func_10_12();
+});
+struct DontWarn2 {
+  void f() {
+func_10_12();
+  }
+};
+  }
+};
+  }
 }
 
 #endif
@@ -259,9 +297,14 @@
 @end
 
 void with_local_struct() {
-  struct local { // expected-note{{annotate 'local' with an availability attribute}}
-new_int x; // expected-warning{{'new_int' is only available}}
+  struct local { 
+new_int x; // expected-warning{{'new_int' is only available}} expected-note{{enclose 'new_int' in an @available check}}
   };
+  if (@available(macos 10.12, *)) {
+struct DontWarn {
+  new_int x;
+};
+  }
 }
 
 // rdar://33156429:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19650,12 +19650,10 @@
   if (Spec != AvailSpecs.end())
 Version = Spec->getVersion();
 
-  // The use of `@available` in the enclosing function should be analyzed to
+  // The use of `@available` in the enclosing context should be analyzed to
   // warn when it's used inappropriately (i.e. not if(@available)).
-  if (getCurFunctionOrMethodDecl())
-getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
-  else if (getCurBlock() || getCurLambda())
-getCurFunction()->HasPotentialAvailabilityViolations = true;
+  if (FunctionScopeInfo *Context = getCurFunctionAvailabilityContext())
+Context->HasPotentialAvailabilityViolations = true;
 
   return new (Context)
   ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -666,13 +666,6 @@
 SemaRef.Context.getTargetInfo().getPlatformMinVersion());
   }
 
-  bool TraverseDecl(Decl *D) {
-// Avoid visiting nested functions to prevent duplicate warnings.
-if (!D || isa(D))
-  return true;
-return Base::TraverseDecl(D);
-  }
-
   bool TraverseStmt(Stmt *S) {
 if (!S)
   return true;
@@ -686,8 +679,6 @@
 
   bool TraverseIfStmt(IfStmt *If);
 
-  bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
-
   // for 'case X:' statements, don't bother looking at the 'X'; it can't lead
   // to any useful diagnostics.
   bool TraverseCaseStmt(CaseStmt *CS) { return TraverseStmt(CS->getSubStmt()); }
@@ -919,6 +910,17 @@
   DiagnoseUnguardedAvailability(*this, D).IssueDiagnostics(Body);
 }
 
+FunctionScopeInfo *Sema::getCurFunctionAvailabilityContext() {
+  if (FunctionScopes.empty())
+return nullptr;
+
+  // Conservatively search the entire current function scope context for
+  // availability violations. This ensures we always correctly analyze nested
+  // classes, blocks, lambdas, etc. that may or may not be inside if(@available)
+  // checks themselves.
+  return FunctionScopes.front();
+}
+
 void Sema::DiagnoseAvailabilityOfDecl(NamedDecl *D,
   ArrayRef Locs,
   const ObjCInterfaceDecl *UnknownObjCClass,
@@ -941,11 +943,8 @@
 // We need to know the @available context in the current function to
 // diag

[clang] a5a3efa - [Sema] Always search the full function scope context if a potential availability violation is encountered

2021-05-24 Thread Logan Smith via cfe-commits

Author: Logan Smith
Date: 2021-05-24T21:13:30-07:00
New Revision: a5a3efa82a77ab7a1c9787ef97b547a4a81f2440

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

LOG: [Sema] Always search the full function scope context if a potential 
availability violation is encountered

This fixes both https://bugs.llvm.org/show_bug.cgi?id=50309 and 
https://bugs.llvm.org/show_bug.cgi?id=50310.

Previously, lambdas inside functions would mark their own bodies for later 
analysis when encountering a potentially unavailable decl, without taking into 
consideration that the entire lambda itself might be correctly guarded inside 
an @available check. The same applied to inner class member functions. Blocks 
happened to work as expected already, since Sema::getEnclosingFunction() skips 
through block scopes.

This patch instead simply and conservatively marks the entire outermost 
function scope for search, and removes some special-case logic that prevented 
DiagnoseUnguardedAvailabilityViolations from traversing down into lambdas and 
nested functions. This correctly accounts for arbitrarily nested lambdas, inner 
classes, and blocks that may be inside appropriate @available checks at any 
ancestor level. It also treats all potential availability violations inside 
functions consistently, without being overly sensitive to the current 
DeclContext, which previously caused issues where e.g. nested struct members 
were warned about twice.

DiagnoseUnguardedAvailabilityViolations now has more work to do in some cases, 
particularly in functions with many (possibly deeply) nested lambdas and 
classes, but the big-O is the same, and the simplicity of the approach and the 
fact that it fixes at least two bugs feels like a strong win.

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

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaAvailability.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaObjC/unguarded-availability.m

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2ac6471c4f04a..706293fa929ce 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1882,6 +1882,10 @@ class Sema final {
   /// Retrieve the current captured region, if any.
   sema::CapturedRegionScopeInfo *getCurCapturedRegion();
 
+  /// Retrieve the current function, if any, that should be analyzed for
+  /// potential availability violations.
+  sema::FunctionScopeInfo *getCurFunctionAvailabilityContext();
+
   /// WeakTopLevelDeclDecls - access to \#pragma weak-generated Decls
   SmallVectorImpl &WeakTopLevelDecls() { return WeakTopLevelDecl; }
 

diff  --git a/clang/lib/Sema/SemaAvailability.cpp 
b/clang/lib/Sema/SemaAvailability.cpp
index 74c4b9e16f74e..d69600aaf94ea 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -666,13 +666,6 @@ class DiagnoseUnguardedAvailability
 SemaRef.Context.getTargetInfo().getPlatformMinVersion());
   }
 
-  bool TraverseDecl(Decl *D) {
-// Avoid visiting nested functions to prevent duplicate warnings.
-if (!D || isa(D))
-  return true;
-return Base::TraverseDecl(D);
-  }
-
   bool TraverseStmt(Stmt *S) {
 if (!S)
   return true;
@@ -686,8 +679,6 @@ class DiagnoseUnguardedAvailability
 
   bool TraverseIfStmt(IfStmt *If);
 
-  bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
-
   // for 'case X:' statements, don't bother looking at the 'X'; it can't lead
   // to any useful diagnostics.
   bool TraverseCaseStmt(CaseStmt *CS) { return TraverseStmt(CS->getSubStmt()); 
}
@@ -919,6 +910,17 @@ void Sema::DiagnoseUnguardedAvailabilityViolations(Decl 
*D) {
   DiagnoseUnguardedAvailability(*this, D).IssueDiagnostics(Body);
 }
 
+FunctionScopeInfo *Sema::getCurFunctionAvailabilityContext() {
+  if (FunctionScopes.empty())
+return nullptr;
+
+  // Conservatively search the entire current function scope context for
+  // availability violations. This ensures we always correctly analyze nested
+  // classes, blocks, lambdas, etc. that may or may not be inside 
if(@available)
+  // checks themselves.
+  return FunctionScopes.front();
+}
+
 void Sema::DiagnoseAvailabilityOfDecl(NamedDecl *D,
   ArrayRef Locs,
   const ObjCInterfaceDecl 
*UnknownObjCClass,
@@ -941,11 +943,8 @@ void Sema::DiagnoseAvailabilityOfDecl(NamedDecl *D,
 // We need to know the @available context in the current function to
 // diagnose this use, let DiagnoseUnguardedAvailabilityViolations do that
 // when we're done parsing the current function.
-if (getCurFunctionOrMethodDecl()) {
-  getEnclosingFunction()->HasPotentialAvailabilityViolations = t

[PATCH] D102995: errorUnsupported should be non-fatal

2021-05-24 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

"fatal" in the comment means don't diagnose any additional errors and 
immediately stop. We attempt to recover to detect more errors, but the emitted 
binary code is likely incorrect. I don't think we can just emit a warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102995

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


[PATCH] D102995: errorUnsupported should be non-fatal

2021-05-24 Thread MJ via Phabricator via cfe-commits
majiang31312 updated this revision to Diff 347546.
majiang31312 added a comment.

Try Arc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102995

Files:
  clang/test/CodeGen/x86_64-mno-sse.c
  clang/test/CodeGen/x86_64-mno-sse2.c
  llvm/lib/Target/X86/X86ISelLowering.cpp


Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -96,8 +96,8 @@
 static void errorUnsupported(SelectionDAG &DAG, const SDLoc &dl,
  const char *Msg) {
   MachineFunction &MF = DAG.getMachineFunction();
-  DAG.getContext()->diagnose(
-  DiagnosticInfoUnsupported(MF.getFunction(), Msg, dl.getDebugLoc()));
+  DAG.getContext()->diagnose(DiagnosticInfoUnsupported(
+  MF.getFunction(), Msg, dl.getDebugLoc(), DS_Warning));
 }
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
Index: clang/test/CodeGen/x86_64-mno-sse2.c
===
--- clang/test/CodeGen/x86_64-mno-sse2.c
+++ clang/test/CodeGen/x86_64-mno-sse2.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse2 -S -o /dev/null 
-verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+double f1(void) { // expected-warning {{SSE2 register return with SSE2 
disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void f2(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   g = f1();
 }
 void take_double(double);
@@ -15,6 +15,6 @@
 }
 
 double return_double();
-void call_double(double *a) { // expected-error {{SSE2 register return with 
SSE2 disabled}}
+void call_double(double *a) { // expected-warning {{SSE2 register return with 
SSE2 disabled}}
   *a = return_double();
 }
Index: clang/test/CodeGen/x86_64-mno-sse.c
===
--- clang/test/CodeGen/x86_64-mno-sse.c
+++ clang/test/CodeGen/x86_64-mno-sse.c
@@ -1,15 +1,14 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse -target-feature 
-sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE register return with SSE disabled}}
+double f1(void) { // expected-warning {{SSE register return with SSE disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE register return with SSE disabled}}
+void f2(void) { // expected-warning {{SSE register return with SSE disabled}}
   g = f1();
 }
 void take_double(double);
 void pass_double(void) {
-  // FIXME: Still asserts.
-  //take_double(1.5);
+  take_double(1.5);
 }


Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -96,8 +96,8 @@
 static void errorUnsupported(SelectionDAG &DAG, const SDLoc &dl,
  const char *Msg) {
   MachineFunction &MF = DAG.getMachineFunction();
-  DAG.getContext()->diagnose(
-  DiagnosticInfoUnsupported(MF.getFunction(), Msg, dl.getDebugLoc()));
+  DAG.getContext()->diagnose(DiagnosticInfoUnsupported(
+  MF.getFunction(), Msg, dl.getDebugLoc(), DS_Warning));
 }
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
Index: clang/test/CodeGen/x86_64-mno-sse2.c
===
--- clang/test/CodeGen/x86_64-mno-sse2.c
+++ clang/test/CodeGen/x86_64-mno-sse2.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+double f1(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void f2(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   g = f1();
 }
 void take_double(double);
@@ -15,6 +15,6 @@
 }
 
 double return_double();
-void call_double(double *a) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void call_double(double *a) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   *a = return_double();
 }
Index: clang/test/CodeGen/x86_64-mno-sse.c
===
--- clang/test/CodeGen/x86_64-mno-sse.c
+++ clang/test/CodeGen/x86_64-mno-sse.c
@@ -1,15 +1,14 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse -target-feature -sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {

[PATCH] D102995: errorUnsupported should be non-fatal

2021-05-24 Thread MJ via Phabricator via cfe-commits
majiang31312 updated this revision to Diff 347545.
majiang31312 set the repository for this revision to rG LLVM Github Monorepo.
majiang31312 added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

CI failed to patch?   Retry with Repository 'rG 
 LLVM Github Monorepo'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102995

Files:
  clang/test/CodeGen/x86_64-mno-sse.c
  clang/test/CodeGen/x86_64-mno-sse2.c
  llvm/lib/Target/X86/X86ISelLowering.cpp


Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -96,8 +96,8 @@
 static void errorUnsupported(SelectionDAG &DAG, const SDLoc &dl,
  const char *Msg) {
   MachineFunction &MF = DAG.getMachineFunction();
-  DAG.getContext()->diagnose(
-  DiagnosticInfoUnsupported(MF.getFunction(), Msg, dl.getDebugLoc()));
+  DAG.getContext()->diagnose(DiagnosticInfoUnsupported(
+  MF.getFunction(), Msg, dl.getDebugLoc(), DS_Warning));
 }
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
Index: clang/test/CodeGen/x86_64-mno-sse2.c
===
--- clang/test/CodeGen/x86_64-mno-sse2.c
+++ clang/test/CodeGen/x86_64-mno-sse2.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse2 -S -o /dev/null 
-verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+double f1(void) { // expected-warning {{SSE2 register return with SSE2 
disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void f2(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   g = f1();
 }
 void take_double(double);
@@ -15,6 +15,6 @@
 }
 
 double return_double();
-void call_double(double *a) { // expected-error {{SSE2 register return with 
SSE2 disabled}}
+void call_double(double *a) { // expected-warning {{SSE2 register return with 
SSE2 disabled}}
   *a = return_double();
 }
Index: clang/test/CodeGen/x86_64-mno-sse.c
===
--- clang/test/CodeGen/x86_64-mno-sse.c
+++ clang/test/CodeGen/x86_64-mno-sse.c
@@ -1,15 +1,14 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse -target-feature 
-sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE register return with SSE disabled}}
+double f1(void) { // expected-warning {{SSE register return with SSE disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE register return with SSE disabled}}
+void f2(void) { // expected-warning {{SSE register return with SSE disabled}}
   g = f1();
 }
 void take_double(double);
 void pass_double(void) {
-  // FIXME: Still asserts.
-  //take_double(1.5);
+  take_double(1.5);
 }


Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -96,8 +96,8 @@
 static void errorUnsupported(SelectionDAG &DAG, const SDLoc &dl,
  const char *Msg) {
   MachineFunction &MF = DAG.getMachineFunction();
-  DAG.getContext()->diagnose(
-  DiagnosticInfoUnsupported(MF.getFunction(), Msg, dl.getDebugLoc()));
+  DAG.getContext()->diagnose(DiagnosticInfoUnsupported(
+  MF.getFunction(), Msg, dl.getDebugLoc(), DS_Warning));
 }
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
Index: clang/test/CodeGen/x86_64-mno-sse2.c
===
--- clang/test/CodeGen/x86_64-mno-sse2.c
+++ clang/test/CodeGen/x86_64-mno-sse2.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cc1 -triple x86_64-linux -target-feature -sse2 -S -o /dev/null -verify %s
 // REQUIRES: x86-registered-target
 
-double f1(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+double f1(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   return 1.4;
 }
 extern double g;
-void f2(void) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void f2(void) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   g = f1();
 }
 void take_double(double);
@@ -15,6 +15,6 @@
 }
 
 double return_double();
-void call_double(double *a) { // expected-error {{SSE2 register return with SSE2 disabled}}
+void call_double(double *a) { // expected-warning {{SSE2 register return with SSE2 disabled}}
   *a = return_double();
 }
Index: clang/test/CodeGen/x86_64-mno-sse.c
===
--- clang/test/CodeGen/x86

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

2021-05-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

reverted in d881319cc5606baa7668405a296d0960a83a1e4c 
 for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102693

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


[clang] d881319 - Revert "Do not create LLVM IR `constant`s for objects with dynamic initialisation"

2021-05-24 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2021-05-24T21:22:07-04:00
New Revision: d881319cc5606baa7668405a296d0960a83a1e4c

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

LOG: Revert "Do not create LLVM IR `constant`s for objects with dynamic 
initialisation"

This reverts commit 13dd65b3a1a3ac049b5f3a9712059f7c61649bea.
Breaks check-clang on macOS, see https://reviews.llvm.org/D102693

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 
clang/test/CodeGenCXX/clang-sections-1.cpp
clang/test/CodeGenCXX/const-dynamic-init.cpp



diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e08e8d8346c03..601f4f2502f0a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13047,6 +13047,43 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 }
   }
 
+  // Apply section attributes and pragmas to global variables.
+  bool GlobalStorage = var->hasGlobalStorage();
+  if (GlobalStorage && var->isThisDeclarationADefinition() &&
+  !inTemplateInstantiation()) {
+PragmaStack *Stack = nullptr;
+int SectionFlags = ASTContext::PSF_Read;
+if (var->getType().isConstQualified())
+  Stack = &ConstSegStack;
+else if (!var->getInit()) {
+  Stack = &BSSSegStack;
+  SectionFlags |= ASTContext::PSF_Write;
+} else {
+  Stack = &DataSegStack;
+  SectionFlags |= ASTContext::PSF_Write;
+}
+if (const SectionAttr *SA = var->getAttr()) {
+  if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec)
+SectionFlags |= ASTContext::PSF_Implicit;
+  UnifySection(SA->getName(), SectionFlags, var);
+} else if (Stack->CurrentValue) {
+  SectionFlags |= ASTContext::PSF_Implicit;
+  auto SectionName = Stack->CurrentValue->getString();
+  var->addAttr(SectionAttr::CreateImplicit(
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
+var->dropAttr();
+}
+
+// Apply the init_seg attribute if this has an initializer.  If the
+// initializer turns out to not be dynamic, we'll end up ignoring this
+// attribute.
+if (CurInitSeg && var->getInit())
+  var->addAttr(InitSegAttr::CreateImplicit(Context, 
CurInitSeg->getString(),
+   CurInitSegLoc,
+   
AttributeCommonInfo::AS_Pragma));
+  }
 
   if (!var->getType()->isStructureType() && var->hasInit() &&
   isa(var->getInit())) {
@@ -13096,6 +13133,14 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
   }
   }
 
+  // All the following checks are C++ only.
+  if (!getLangOpts().CPlusPlus) {
+// If this variable must be emitted, add it as an initializer for the
+// current module.
+if (Context.DeclMustBeEmitted(var) && !ModuleScopes.empty())
+  Context.addModuleInitializer(ModuleScopes.back().Module, var);
+return;
+  }
 
   QualType type = var->getType();
 
@@ -13103,14 +13148,11 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 getCurFunction()->addByrefBlockVar(var);
 
   Expr *Init = var->getInit();
-  bool GlobalStorage = var->hasGlobalStorage();
   bool IsGlobal = GlobalStorage && !var->isStaticLocal();
   QualType baseType = Context.getBaseElementType(type);
-  bool HasConstInit = true;
 
   // Check whether the initializer is sufficiently constant.
-  if (getLangOpts().CPlusPlus && !type->isDependentType() && Init &&
-  !Init->isValueDependent() &&
+  if (!type->isDependentType() && Init && !Init->isValueDependent() &&
   (GlobalStorage || var->isConstexpr() ||
var->mightBeUsableInConstantExpressions(Context))) {
 // If this variable might have a constant initializer or might be usable in
@@ -13118,6 +13160,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 // do this lazily, because the result might depend on things that change
 // later, such as which constexpr functions happen to be defined.
 SmallVector Notes;
+bool HasConstInit;
 if (!getLangOpts().CPlusPlus11) {
   // Prior to C++11, in contexts where a constant initializer is required,
   // the set of valid constant initializers is described by syntactic rules
@@ -13182,57 +13225,6 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 }
   }
 
-  // Apply section attributes and pragmas to global variables.
-  if (GlobalStorage && var->isThisDeclarationADefinition() &&
-  !inTemplateInstantiation()) {
-PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Read;
-if (var->getType().isConstQualified()) {
-  if (HasConstInit)
-Stack = &ConstSegStac

[PATCH] D101777: [clang] p1099 1/5: [NFC] Break out BaseUsingDecl from UsingDecl

2021-05-24 Thread David Rector via Phabricator via cfe-commits
davrec accepted this revision.
davrec added a comment.

Looks good, thanks for doing this!


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

https://reviews.llvm.org/D101777

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


[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

2021-05-24 Thread Haowei Wu via Phabricator via cfe-commits
haowei added a comment.

This CL breaks our mac builders as well. Could you either fix the test or 
revert the change please?

Error message:

   TEST 'Clang :: CodeGenCXX/const-dynamic-init.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   /opt/staging/llvm_build/bin/clang -cc1 
-internal-isystem /opt/staging/llvm_build/lib/clang/13.0.0/include 
-nostdsysteminc -triple x86_64-apple-darwin19.6.0 -emit-llvm -o - 
/opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp | 
/opt/staging/llvm_build/bin/FileCheck 
/opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  /opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp:3:24: error: 
argument to 'section' attribute is not valid for this target: mach-o section 
specifier requires a segment and section separated by a comma
  __attribute__((section("A")))
 ^
  /opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp:9:24: error: 
argument to 'section' attribute is not valid for this target: mach-o section 
specifier requires a segment and section separated by a comma
  __attribute__((section("B")))
 ^
  /opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp:15:24: error: 
argument to 'section' attribute is not valid for this target: mach-o section 
specifier requires a segment and section separated by a comma
  __attribute__((section("C")))
 ^
  /opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp:19:24: error: 
argument to 'section' attribute is not valid for this target: mach-o section 
specifier requires a segment and section separated by a comma
  __attribute__((section("D")))
 ^
  /opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp:23:24: error: 
argument to 'section' attribute is not valid for this target: mach-o section 
specifier requires a segment and section separated by a comma
  __attribute__((section("E")))
 ^
  5 errors generated.
  FileCheck error: '' is empty.
  FileCheck command line:  /opt/staging/llvm_build/bin/FileCheck 
/opt/llvm-project/clang/test/CodeGenCXX/const-dynamic-init.cpp
  
  --
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102693

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


[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

2021-05-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This breaks tests on macOS: http://45.33.8.238/macm1/10125/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102693

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


[clang] de6164e - PR50456: Properly handle multiple escaped newlines in a '*/'.

2021-05-24 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2021-05-24T16:21:03-07:00
New Revision: de6164ec4da0cfea1b0d0e472c432ea1be4d9c29

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

LOG: PR50456: Properly handle multiple escaped newlines in a '*/'.

Added: 


Modified: 
clang/lib/Lex/Lexer.cpp
clang/test/Lexer/block_cmt_end.c

Removed: 




diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 75c0fb65f5b1..d31987a432b8 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -2443,56 +2443,70 @@ static bool isEndOfBlockCommentWithEscapedNewLine(const 
char *CurPtr,
   Lexer *L) {
   assert(CurPtr[0] == '\n' || CurPtr[0] == '\r');
 
-  // Back up off the newline.
-  --CurPtr;
+  // Position of the first trigraph in the ending sequence.
+  const char *TrigraphPos = 0;
+  // Position of the first whitespace after a '\' in the ending sequence.
+  const char *SpacePos = 0;
 
-  // If this is a two-character newline sequence, skip the other character.
-  if (CurPtr[0] == '\n' || CurPtr[0] == '\r') {
-// \n\n or \r\r -> not escaped newline.
-if (CurPtr[0] == CurPtr[1])
-  return false;
-// \n\r or \r\n -> skip the newline.
+  while (true) {
+// Back up off the newline.
 --CurPtr;
-  }
 
-  // If we have horizontal whitespace, skip over it.  We allow whitespace
-  // between the slash and newline.
-  bool HasSpace = false;
-  while (isHorizontalWhitespace(*CurPtr) || *CurPtr == 0) {
---CurPtr;
-HasSpace = true;
-  }
+// If this is a two-character newline sequence, skip the other character.
+if (CurPtr[0] == '\n' || CurPtr[0] == '\r') {
+  // \n\n or \r\r -> not escaped newline.
+  if (CurPtr[0] == CurPtr[1])
+return false;
+  // \n\r or \r\n -> skip the newline.
+  --CurPtr;
+}
 
-  // If we have a slash, we know this is an escaped newline.
-  if (*CurPtr == '\\') {
-if (CurPtr[-1] != '*') return false;
-  } else {
-// It isn't a slash, is it the ?? / trigraph?
-if (CurPtr[0] != '/' || CurPtr[-1] != '?' || CurPtr[-2] != '?' ||
-CurPtr[-3] != '*')
+// If we have horizontal whitespace, skip over it.  We allow whitespace
+// between the slash and newline.
+while (isHorizontalWhitespace(*CurPtr) || *CurPtr == 0) {
+  SpacePos = CurPtr;
+  --CurPtr;
+}
+
+// If we have a slash, this is an escaped newline.
+if (*CurPtr == '\\') {
+  --CurPtr;
+} else if (CurPtr[0] == '/' && CurPtr[-1] == '?' && CurPtr[-2] == '?') {
+  // This is a trigraph encoding of a slash.
+  TrigraphPos = CurPtr - 2;
+  CurPtr -= 3;
+} else {
   return false;
+}
 
-// This is the trigraph ending the comment.  Emit a stern warning!
-CurPtr -= 2;
+// If the character preceding the escaped newline is a '*', then after line
+// splicing we have a '*/' ending the comment.
+if (*CurPtr == '*')
+  break;
+
+if (*CurPtr != '\n' && *CurPtr != '\r')
+  return false;
+  }
 
+  if (TrigraphPos) {
 // If no trigraphs are enabled, warn that we ignored this trigraph and
 // ignore this * character.
 if (!L->getLangOpts().Trigraphs) {
   if (!L->isLexingRawMode())
-L->Diag(CurPtr, diag::trigraph_ignored_block_comment);
+L->Diag(TrigraphPos, diag::trigraph_ignored_block_comment);
   return false;
 }
 if (!L->isLexingRawMode())
-  L->Diag(CurPtr, diag::trigraph_ends_block_comment);
+  L->Diag(TrigraphPos, diag::trigraph_ends_block_comment);
   }
 
   // Warn about having an escaped newline between the */ characters.
   if (!L->isLexingRawMode())
-L->Diag(CurPtr, diag::escaped_newline_block_comment_end);
+L->Diag(CurPtr + 1, diag::escaped_newline_block_comment_end);
 
   // If there was space between the backslash and newline, warn about it.
-  if (HasSpace && !L->isLexingRawMode())
-L->Diag(CurPtr, diag::backslash_newline_space);
+  if (SpacePos && !L->isLexingRawMode())
+L->Diag(SpacePos, diag::backslash_newline_space);
 
   return true;
 }

diff  --git a/clang/test/Lexer/block_cmt_end.c 
b/clang/test/Lexer/block_cmt_end.c
index 1d00137644c3..7d24817042f4 100644
--- a/clang/test/Lexer/block_cmt_end.c
+++ b/clang/test/Lexer/block_cmt_end.c
@@ -32,3 +32,14 @@ foo
 // rdar://6060752 - We should not get warnings about trigraphs in comments:
 // ''
 /*  */
+
+// PR50456: multiple escaped newlines in one */.
+/*
+ *\
+??/
+??/  
+\  
+/
+// expected-warning@-5 {{escaped newline}}
+// expected-warning@-4 {{separated by space}}
+// expected-warning@-6 {{trigraph ends block comment}}



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

[PATCH] D102650: Old work on P0388. For reference in D102645. Not for review / commit.

2021-05-24 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

FWIW, there's some way to post reviews in draft form (I think via arc --draft 
or something like that) - which would ensure that emails for the review aren't 
sent to the -commits mailing list for something you don't want folks on the 
-commits list to review just yet, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102650

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


Re: [clang] b89e09a - Silence "Undefined or garbage value returned to caller" static analysis warning. NFCI.

2021-05-24 Thread David Blaikie via cfe-commits
If this is meant to be NFC, I guess the function is never meant to return
null? (it's meant to be an invariant of the code that at least one of the
builders is valid) - if that's the case, I'm not sure if this is good
change.

If there is a bug in the future that causes there to be an no valid builder
- this change will cause that failure mode to be delayed and more distant
from the problem, compared to the failure that this code would have under
msan if it attempted to return garbage, I think? (hmm, well, I guess not
/much/ better, even the msan failure is delayed until the value is used in
an "interesting" way - not directly on the return - but the msan error is
clearer/might lead someone back to the variable declaration (where we could
have a comment about the intended invariant of the code) compared to them
thinking null was a valid return value from the function and adding null
handling in the caller instead)

On Mon, May 17, 2021 at 6:09 AM Simon Pilgrim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Simon Pilgrim
> Date: 2021-05-17T14:08:27+01:00
> New Revision: b89e09a19f9b60dfa9477b24a404a7ae5522f898
>
> URL:
> https://github.com/llvm/llvm-project/commit/b89e09a19f9b60dfa9477b24a404a7ae5522f898
> DIFF:
> https://github.com/llvm/llvm-project/commit/b89e09a19f9b60dfa9477b24a404a7ae5522f898.diff
>
> LOG: Silence "Undefined or garbage value returned to caller" static
> analysis warning. NFCI.
>
> Added:
>
>
> Modified:
> clang/lib/Driver/Driver.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> index 1086887a8de5..25af909d9bd2 100644
> --- a/clang/lib/Driver/Driver.cpp
> +++ b/clang/lib/Driver/Driver.cpp
> @@ -3449,7 +3449,7 @@ class OffloadingActionBuilder final {
>return nullptr;
>
>  // Let builders add host linking actions.
> -Action* HA;
> +Action* HA = nullptr;
>  for (DeviceActionBuilder *SB : SpecializedBuilders) {
>if (!SB->isValid())
>  continue;
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D102689: [C++4OpenCL] Allow address space conversion in reinterpret_cast

2021-05-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D102689#2778072 , @rsmith wrote:

> In D102689#2778011 , @rjmccall 
> wrote:
>
>> The C++ standard does not appear to have similar wording.  On the other 
>> hand, the C++ standard says that e.g. "The result of the expression `(T) 
>> cast-expression` is of type `T`", and similarly for the other casts, which 
>> is clearly just wrong if `T` is a reference type; the wording clarifies that 
>> the expression is an l-value or x-value if the type is a reference but 
>> doesn't remove the reference-ness of the expression type as it must, unless 
>> that's done by some other clause at a distance.
>
> It is indeed done by a different clause at a distance:
>
> **[expr.type]**
>
>> 1. If an expression initially has the type “reference to `T`”, the type is 
>> adjusted to `T` prior to any further analysis. The expression designates the 
>> object or function denoted by the reference, and the expression is an lvalue 
>> or an xvalue, depending on the expression.
>
>
>
>> 2. If a prvalue initially has the type “cv `T`”, where `T` is a 
>> cv-unqualified non-class, non-array type, the type of the expression is 
>> adjusted to `T` prior to any further analysis.

Ah, thank you.

> In D102689#2778011 , @rjmccall 
> wrote:
>
>> C++ has a very strange feature of allowing qualifiers on class types in 
>> particular circumstances, so we might need to be permissive about that, 
>> although that's tricky for address spaces because (unlike with `const` or 
>> `volatile`) we cannot in fact allocate a temporary in an arbitrary address 
>> space.
>
> Right, specifically C++ preserves top-level `const` and `volatile` 
> qualifications on prvalues only if they're of class or array type. If an 
> address-space-qualified temporary makes sense, then I think extending this 
> behavior to address spaces makes sense too:
>
> - For array prvalues, an `AS1 int[3]` should decay to an `AS1 int*`, not to a 
> plain `int*`, because it seems clear the intent is for the array to be 
> created in the given address space.
> - For class prvalues, I think it's important that we preserve the address 
> space, because (for example) a function `AS1 MyClass f();` might take a 
> return value slot as an `AS1` pointer, so a prvalue `f()` should retain the 
> address space qualification.

Hmm.  That's an interesting question, because while I can see how it's an 
interesting thing to be able to express, it might also lead to 
otherwise-unnecessary template errors, e.g. if a template type argument is 
inferred to be `__global MyClass`.  On the other hand, that kind of inference 
can easily lead to other problems with e.g. locals or fields; making it work in 
slightly more cases isn't necessarily worthwhile.

> While we can't allocate an address-space-qualified temporary right now, would 
> it be a good idea to prepare for the possibility that we will one day be able 
> to, in some cases? For example, in a system with separate control and data 
> stacks, we might want to put some data on the control stack; in an exotic 
> heterogeneous compute system we might be able to allocate memory in the host 
> and device stack from a single function.  If so, following the C++ rule for 
> all qualifiers might be a good direction. (We'll presumably need to look at 
> the other qualifiers and see if any of them would have problems with this 
> treatment.)

Hmm, okay, I cede the point.  I have some concerns here (C compatibility and 
addressability), but we don't have to design AS-qualified return values right 
now.  I agree that we should be stripping qualifiers except on class and array 
types.

I believe array types are actually ruled out for casts because there are no 
conversions to array type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102689

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


[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

2021-05-24 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur updated this revision to Diff 347508.
Meinersbur added a comment.

- Merge branch 'arcpatch-D102180' into HEAD
- Use SmallVector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102180

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/tile_codegen.cpp
  clang/test/OpenMP/tile_codegen_for_dependent.cpp
  clang/test/OpenMP/tile_codegen_tile_for.cpp

Index: clang/test/OpenMP/tile_codegen_tile_for.cpp
===
--- /dev/null
+++ clang/test/OpenMP/tile_codegen_tile_for.cpp
@@ -0,0 +1,253 @@
+// Check code generation
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+
+// Check same results after serialization round-trip
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-pch -o %t %s
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -include-pch %t -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+// expected-no-diagnostics
+
+// Account for multiple transformations of a loop before consumed by
+// #pragma omp for.
+
+#ifndef HEADER
+#define HEADER
+
+// placeholder for loop body code.
+extern "C" void body(...) {}
+
+
+// IR-LABEL: @func(
+// IR-NEXT:  [[ENTRY:.*]]:
+// IR-NEXT:%[[START_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[END_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[STEP_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IV:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP:.+]] = alloca i32, align 4
+// IR-NEXT:%[[I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_1:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_2:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_3:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTFLOOR_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_6:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_8:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_12:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_14:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTFLOOR_0_IV__FLOOR_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_LB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_UB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_STRIDE:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IS_LAST:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTFLOOR_0_IV__FLOOR_0_IV_I18:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTTILE_0_IV__FLOOR_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTTILE_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP0:.+]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @2)
+// IR-NEXT:store i32 %[[START:.+]], i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[END:.+]], i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[STEP:.+]], i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:%[[TMP1:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP1]], i32* %[[I]], align 4
+// IR-NEXT:%[[TMP2:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP2]], i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[TMP3:.+]] = load i32, i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP3]], i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP4:.+]] = load i32, i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP4]], i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[TMP5:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP6:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[SUB:.+]] = sub i32 %[[TMP5]], %[[TMP6]]
+// IR-NEXT:%[[SUB4:.+]] = sub i32 %[[SUB]], 1
+// IR-NEXT:%[[TMP7:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[ADD:.+]] = add i32 %[[SUB4]], %[[TMP7]]
+// IR-NEXT:%[[TMP8:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[DIV:.+]] = udiv i32 %[[ADD]], %[[TMP8]]
+// IR-NEXT:%[[SUB5:.+]] = sub i32 %[[DIV]], 1
+// IR-NEXT:store i32 %[[SUB5]], i32* %[[DOTCAPTURE_EXPR_3]], align 4
+// IR-NEXT:store i32 0, i32* %[[DOTFLOOR_0_IV_I]], align 4
+// IR-NEXT:%[[TMP9:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_3]], align 4
+// IR-NEXT:%[[ADD7:.+]] = add i32 %[[TMP9]], 1
+// IR-NEXT:store i32 %[[ADD7]], i32* %[[DOTCAPTURE_EXPR_6]], align 4
+// IR-NEXT:%[[TMP10:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_6]], align 4
+// IR-NEXT:%[[SUB9:.+]] = sub i32 %[[TMP10]], -3
+// IR-NEXT:%[[DIV10:.+]] = udiv i32 %[[SUB9]], 4
+// IR-NEXT:%[[SUB11:.+]] = sub i32 %[[DIV10]], 1
+// IR-NEXT:store i32 %[[SUB11]], i32* %

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2021-05-24 Thread Ahmed Mahdy via Phabricator via cfe-commits
aybassiouny updated this revision to Diff 347504.
aybassiouny added a comment.

Add an assert


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

https://reviews.llvm.org/D99031

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2589,6 +2589,25 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespacesLambdaRegression) {
+  // Make sure compact namespaces are not confused with lambdas
+  FormatStyle CompactNamespacesStyle{getLLVMStyle()};
+  CompactNamespacesStyle.CompactNamespaces = true;
+  CompactNamespacesStyle.AllowShortLambdasOnASingleLine = 
FormatStyle::SLS_None;
+  CompactNamespacesStyle.BreakBeforeBraces = FormatStyle::BS_Custom;
+  CompactNamespacesStyle.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("namespace out { namespace in {\n"
+   "}} // namespace out::in",
+   CompactNamespacesStyle);
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   CompactNamespacesStyle));
+}
+
 TEST_F(FormatTest, FormatsExternC) {
   verifyFormat("extern \"C\" {\nint a;");
   verifyFormat("extern \"C\" {}");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3493,8 +3493,12 @@
   return true;
 }
 static bool isAllmanLambdaBrace(const FormatToken &Tok) {
+  assert(!Tok.is(tok::l_brace) || !Tok.is(BK_Block) ||
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) ||
+ (Tok.Previous && Tok.Previous->Previous));
   return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
-  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
+  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) &&
+  !Tok.Previous->Previous->is(tok::kw_namespace));
 }
 
 static bool isAllmanBraceIncludedBreakableLambda(


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2589,6 +2589,25 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespacesLambdaRegression) {
+  // Make sure compact namespaces are not confused with lambdas
+  FormatStyle CompactNamespacesStyle{getLLVMStyle()};
+  CompactNamespacesStyle.CompactNamespaces = true;
+  CompactNamespacesStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  CompactNamespacesStyle.BreakBeforeBraces = FormatStyle::BS_Custom;
+  CompactNamespacesStyle.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("namespace out { namespace in {\n"
+   "}} // namespace out::in",
+   CompactNamespacesStyle);
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   CompactNamespacesStyle));
+}
+
 TEST_F(FormatTest, FormatsExternC) {
   verifyFormat("extern \"C\" {\nint a;");
   verifyFormat("extern \"C\" {}");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3493,8 +3493,12 @@
   return true;
 }
 static bool isAllmanLambdaBrace(const FormatToken &Tok) {
+  assert(!Tok.is(tok::l_brace) || !Tok.is(BK_Block) ||
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) ||
+ (Tok.Previous && Tok.Previous->Previous));
   return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
-  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
+  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) &&
+  !Tok.Previous->Previous->is(tok::kw_namespace));
 }
 
 static bool isAllmanBraceIncludedBreakableLambda(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103048: [IR] make -stack-alignment= into a module attr

2021-05-24 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers created this revision.
nickdesaulniers added reviewers: tejohnson, craig.topper, RKSimon.
Herald added subscribers: dexonsmith, pengfei, hiraditya, qcolombet.
nickdesaulniers requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Similar to D102742 , specifying the stack 
alignment via CodegenOpts means
that this flag gets dropped during LTO, unless the command line is
respecified as a plugin opt. Instead, encode this information as a
module level attribute so that we don't have to expose this llvm
internal flag when linking the Linux kernel with LTO.

Curiously, using ModFlagBehavior::Error doesn't error if one of two
modules being linked together doesn't have such a module level
attribute.

Looks like external dependencies might need a fix:

- https://github.com/llvm-hs/llvm-hs
- https://github.com/halide/Halide

Link: https://github.com/ClangBuiltLinux/linux/issues/1377


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103048

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stackrealign-main.c
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/Generic/ForceStackAlign.ll
  llvm/test/CodeGen/X86/base-pointer-and-cmpxchg.ll
  llvm/test/CodeGen/X86/base-pointer-and-mwaitx.ll
  llvm/test/CodeGen/X86/dynamic-allocas-VLAs-stack-align.ll
  llvm/test/CodeGen/X86/dynamic-allocas-VLAs.ll
  llvm/test/CodeGen/X86/force-align-stack-alloca.ll
  llvm/test/CodeGen/X86/hipe-cc.ll
  llvm/test/CodeGen/X86/hipe-cc64.ll
  llvm/test/CodeGen/X86/movtopush-stack-align.ll
  llvm/test/CodeGen/X86/movtopush.ll
  llvm/test/CodeGen/X86/pr11468.ll
  llvm/test/CodeGen/X86/unaligned-spill-folding.ll
  llvm/test/CodeGen/X86/x86-64-baseptr.ll
  llvm/test/CodeGen/X86/x86-64-xmm-spill-unaligned.ll
  llvm/test/Linker/stack-alignment.ll

Index: llvm/test/Linker/stack-alignment.ll
===
--- /dev/null
+++ llvm/test/Linker/stack-alignment.ll
@@ -0,0 +1,22 @@
+; RUN: split-file %s %t
+; TODO: it seems that ModFlagBehavior::Error doesn't error when a module is
+;   missing a module level attribute. IRLinker::linkModuleFlagsMetadata
+;   seems to think this is ok. Debugging stack mismatch errors is not a fun
+;   time.
+; RUN not llvm-link %t/main.ll %t/none.ll 2>&1 | FileCheck --check-prefix=NONE %s
+; RUN: llvm-link %t/main.ll %t/8.ll
+; RUN: not llvm-link %t/main.ll %t/16.ll 2>&1 | FileCheck --check-prefix=CHECK-16 %s
+
+;--- main.ll
+; NONE: error: linking module flags 'override-stack-alignment': IDs have conflicting values
+; CHECK-16: error: linking module flags 'override-stack-alignment': IDs have conflicting values
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"override-stack-alignment", i32 8}
+
+;--- none.ll
+;--- 8.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"override-stack-alignment", i32 8}
+;--- 16.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"override-stack-alignment", i32 16}
Index: llvm/test/CodeGen/X86/x86-64-xmm-spill-unaligned.ll
===
--- llvm/test/CodeGen/X86/x86-64-xmm-spill-unaligned.ll
+++ llvm/test/CodeGen/X86/x86-64-xmm-spill-unaligned.ll
@@ -2,7 +2,7 @@
 ; elements (here: XMM spills) are accessed using instructions that tolerate
 ; unaligned access.
 ;
-; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mcpu=x86-64 -mattr=+sse,+sse-unaligned-mem -stack-alignment=8 --frame-pointer=all < %s | FileCheck %s
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mcpu=x86-64 -mattr=+sse,+sse-unaligned-mem --frame-pointer=all < %s | FileCheck %s
 
 define dso_local preserve_allcc void @func() #0 {
 ; CHECK-LABEL: func:
@@ -13,3 +13,5 @@
 }
 
 attributes #0 = { nounwind }
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"override-stack-alignment", i32 8}
Index: llvm/test/CodeGen/X86/x86-64-baseptr.ll
===
--- llvm/test/CodeGen/X86/x86-64-baseptr.ll
+++ llvm/test/CodeGen/X86/x86-64-baseptr.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=x86_64-pc-linux -stackrealign -stack-alignment=32 < %s | FileCheck %s
-; RUN: llc -mtriple=x86_64-pc-linux-gnux32 -stackrealign -stack-alignment=32 < %s | FileCheck -check-prefix=X32ABI %s
+; RUN: llc -mtriple=x86_64-pc-linux -stackrealign < %s | FileCheck %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnux32 -stackrealign < %s | FileCheck -check-prefix=X32ABI %s
 
 ; This should run with NaCl as well ( -mtriple=x86_64-pc-nacl ) but currently doesn't due to PR22655
 
@@ -65,3 +65,5 @@
 }
 
 attributes #0 = { nounwind "frame-pointer"="all"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"override-stack-alignment", i32 32}
In

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

2021-05-24 Thread Momchil Velikov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG13dd65b3a1a3: Do not create LLVM IR `constant`s for objects 
with dynamic initialisation (authored by chill).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102693

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/clang-sections-1.cpp
  clang/test/CodeGenCXX/const-dynamic-init.cpp

Index: clang/test/CodeGenCXX/const-dynamic-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/const-dynamic-init.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((section("A")))
+const int a = 1;
+const int *f() { return &a; }
+// CHECK: @_ZL1a = internal constant i32 1, section "A"
+
+int init();
+__attribute__((section("B")))
+const int b = init();
+// Even if it's const-qualified, it must not be LLVM IR `constant` since it's
+// dynamically initialised.
+// CHECK: @_ZL1b = internal global i32 0, section "B"
+
+__attribute__((section("C")))
+int c = 2;
+// CHECK: @c = {{.*}}global i32 2, section "C"
+
+__attribute__((section("D")))
+int d = init();
+// CHECK: @d = {{.*}}global i32 0, section "D"
+
+__attribute__((section("E")))
+int e;
+// CHECK: @e = {{.*}}global i32 0, section "E", align 4
Index: clang/test/CodeGenCXX/clang-sections-1.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-sections-1.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s --check-prefix=LLVM
+// RUN: %clang_cc1 -triple x86_64-linux -S -o - %s | FileCheck %s --check-prefix=ASM
+// Actually, any ELF target would do
+// REQUIRES: x86_64-linux
+
+#pragma clang section bss = "B$$" data = "d@t@" rodata = "r0d@t@"
+
+const int a = 1;
+const int *f() { return &a; }
+
+int init();
+const int b = init();
+
+int c = 2;
+
+int d = init();
+
+int e;
+
+// LLVM: @_ZL1a = internal constant i32 1, align 4 #[[#A:]]
+// LLVM: @_ZL1b = internal global i32 0, align 4 #[[#A]]
+// LLVM: @c = {{.*}}global i32 2, align 4 #[[#A]]
+// LLVM: @d = {{.*}}global i32 0, align 4 #[[#A]]
+// LLVM: @e = {{.*}}global i32 0, align 4 #[[#A]]
+
+// LLVM: attributes #[[#A]] = { "bss-section"="B$$" "data-section"="d@t@" "rodata-section"="r0d@t@" }
+
+// ASM:   .section "r0d@t@","a",@progbits
+// ASM-NOT:   .section
+// ASM-LABEL: _ZL1a:
+// ASM-NEXT:  .long 1
+
+// ASM:   .section "B$$","aw",@nobits
+// ASM-NOT:   .section
+// ASM-LABEL: _ZL1b:
+// ASM-NEXT: .long 0
+
+// ASM:   .section "d@t@","aw",@progbits
+// ASM-NOT:   .section
+// ASM-LABEL: c:
+// ASM:   .long 2
+
+// ASM:   .section "B$$","aw",@nobits
+// ASM-NOT:   .section
+// ASM-LABEL: d:
+// ASM:   .long 0
+
+// ASM-NOT:   .section
+// ASM-LABEL: e:
+// ASM.long 0
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13047,43 +13047,6 @@
 }
   }
 
-  // Apply section attributes and pragmas to global variables.
-  bool GlobalStorage = var->hasGlobalStorage();
-  if (GlobalStorage && var->isThisDeclarationADefinition() &&
-  !inTemplateInstantiation()) {
-PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Read;
-if (var->getType().isConstQualified())
-  Stack = &ConstSegStack;
-else if (!var->getInit()) {
-  Stack = &BSSSegStack;
-  SectionFlags |= ASTContext::PSF_Write;
-} else {
-  Stack = &DataSegStack;
-  SectionFlags |= ASTContext::PSF_Write;
-}
-if (const SectionAttr *SA = var->getAttr()) {
-  if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec)
-SectionFlags |= ASTContext::PSF_Implicit;
-  UnifySection(SA->getName(), SectionFlags, var);
-} else if (Stack->CurrentValue) {
-  SectionFlags |= ASTContext::PSF_Implicit;
-  auto SectionName = Stack->CurrentValue->getString();
-  var->addAttr(SectionAttr::CreateImplicit(
-  Context, SectionName, Stack->CurrentPragmaLocation,
-  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
-  if (UnifySection(SectionName, SectionFlags, var))
-var->dropAttr();
-}
-
-// Apply the init_seg attribute if this has an initializer.  If the
-// initializer turns out to not be dynamic, we'll end up ignoring this
-// attribute.
-if (CurInitSeg && var->getInit())
-  var->addAttr(InitSegAttr::CreateImplicit(Context, CurInitSeg->getString(),
-   CurInitSegLoc,
-   AttributeCommonInfo::AS_Pragma));
-  }
 
   if (!var->getType()->isStructureType() && var->hasInit() &&
   isa(var->getInit()))

[clang] 13dd65b - Do not create LLVM IR `constant`s for objects with dynamic initialisation

2021-05-24 Thread Momchil Velikov via cfe-commits

Author: Momchil Velikov
Date: 2021-05-24T22:04:15+01:00
New Revision: 13dd65b3a1a3ac049b5f3a9712059f7c61649bea

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

LOG: Do not create LLVM IR `constant`s for objects with dynamic initialisation

When a const-qualified object has a section attribute, that
section is set to read-only and clang outputs a LLVM IR constant
for that object. This is incorrect for dynamically initialised
objects.

For example:

int init() { return 15; }

__attribute__((section("SA")))
const int a = init();

a is allocated to a read-only section and is left
unintialised (zero-initialised).

This patch adds checks if an initialiser is a constant expression
and allocates objects to sections as follows:

* const-qualified objects
  - no initialiser or constant initialiser: .rodata
  - dynamic initializer: .bss
* non const-qualified objects
  - no initialiser or dynamic initialiser: .bss
  - constant initialiser: .data

(".rodata", ".data", and ".bss" names used just for explanatory
purpose)

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

Added: 
clang/test/CodeGenCXX/clang-sections-1.cpp
clang/test/CodeGenCXX/const-dynamic-init.cpp

Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 601f4f2502f0..e08e8d8346c0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13047,43 +13047,6 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 }
   }
 
-  // Apply section attributes and pragmas to global variables.
-  bool GlobalStorage = var->hasGlobalStorage();
-  if (GlobalStorage && var->isThisDeclarationADefinition() &&
-  !inTemplateInstantiation()) {
-PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Read;
-if (var->getType().isConstQualified())
-  Stack = &ConstSegStack;
-else if (!var->getInit()) {
-  Stack = &BSSSegStack;
-  SectionFlags |= ASTContext::PSF_Write;
-} else {
-  Stack = &DataSegStack;
-  SectionFlags |= ASTContext::PSF_Write;
-}
-if (const SectionAttr *SA = var->getAttr()) {
-  if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec)
-SectionFlags |= ASTContext::PSF_Implicit;
-  UnifySection(SA->getName(), SectionFlags, var);
-} else if (Stack->CurrentValue) {
-  SectionFlags |= ASTContext::PSF_Implicit;
-  auto SectionName = Stack->CurrentValue->getString();
-  var->addAttr(SectionAttr::CreateImplicit(
-  Context, SectionName, Stack->CurrentPragmaLocation,
-  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
-  if (UnifySection(SectionName, SectionFlags, var))
-var->dropAttr();
-}
-
-// Apply the init_seg attribute if this has an initializer.  If the
-// initializer turns out to not be dynamic, we'll end up ignoring this
-// attribute.
-if (CurInitSeg && var->getInit())
-  var->addAttr(InitSegAttr::CreateImplicit(Context, 
CurInitSeg->getString(),
-   CurInitSegLoc,
-   
AttributeCommonInfo::AS_Pragma));
-  }
 
   if (!var->getType()->isStructureType() && var->hasInit() &&
   isa(var->getInit())) {
@@ -13133,14 +13096,6 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
   }
   }
 
-  // All the following checks are C++ only.
-  if (!getLangOpts().CPlusPlus) {
-// If this variable must be emitted, add it as an initializer for the
-// current module.
-if (Context.DeclMustBeEmitted(var) && !ModuleScopes.empty())
-  Context.addModuleInitializer(ModuleScopes.back().Module, var);
-return;
-  }
 
   QualType type = var->getType();
 
@@ -13148,11 +13103,14 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 getCurFunction()->addByrefBlockVar(var);
 
   Expr *Init = var->getInit();
+  bool GlobalStorage = var->hasGlobalStorage();
   bool IsGlobal = GlobalStorage && !var->isStaticLocal();
   QualType baseType = Context.getBaseElementType(type);
+  bool HasConstInit = true;
 
   // Check whether the initializer is sufficiently constant.
-  if (!type->isDependentType() && Init && !Init->isValueDependent() &&
+  if (getLangOpts().CPlusPlus && !type->isDependentType() && Init &&
+  !Init->isValueDependent() &&
   (GlobalStorage || var->isConstexpr() ||
var->mightBeUsableInConstantExpressions(Context))) {
 // If this variable might have a constant initializer or might be usable in
@@ -13160,7 +13118,6 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
 // do this lazily, because the result might depend on things that change
 // later, such as which constexpr functions 

[PATCH] D97340: [HIP] Support Spack packages

2021-05-24 Thread Harmen Stoppels via Phabricator via cfe-commits
haampie added a comment.
Herald added a subscriber: foad.

Hi Yaxunl,

> The patch should not cause circular dependency on HIP or device library.

I'm not saying this patch introduces a circular dependency, I'm saying you are 
trying to solve an already existing circular dependency (clang needs device 
libs at runtime, but device libs need llvm to compile).

Let's talk about my PR here: https://github.com/spack/spack/pull/23859. It's 
building rocm-device-libs as part of llvm-amdgpu by configuring llvm with 
`-DLLVM_EXTERNAL_PROJECTS=device-libs`.

If you checkout that pr, run `spack install hip@4.2.0`, what you get is get is:

  $ spack find -p llvm-amdgpu@4.2.0
  llvm-amdgpu@4.2.0  
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi

And indeed the bitcode is there:

  $ find 
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi
 -iname '*.bc'
  
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/oclc_isa_version_1033.bc
  
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/ocml.bc
  
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/hip.bc
  ...

Now when I used this `--print-rocm-search-dirs` flag on clang without other 
flags, what I see is:

  $ 
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/bin/clang++
 --print-rocm-search-dirs -x hip hi.cc 
  ROCm installation search path (Spack 4.2.0): 
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0
  ROCm installation search path: 
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/lib/clang/12.0.0
  ROCm installation search path: /opt/rocm
  ...
  clang-12: error: cannot find ROCm device library. Provide its path via 
--rocm-path or --rocm-device-lib-path, or pass -nogpulib to build without ROCm 
device library.

Now can you make it such that clang will search the llvm prefix itself?

In fact, if you just reduces ALL 3 search paths to just CMAKE_INSTALL_PREFIX 
path, it will find the correct bitcode files independent of whether you're 
using spack or whether you're bundling the rocm packages the traditional way in 
with the shared prefix in /opt/rocm... So, what I'd like to see is this:

  $ 
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/bin/clang++
 --print-rocm-search-dirs -x hip hi.cc 
  ROCm installation search path: 
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/

and

  $ /opt/rocm/bin/clang++ --print-rocm-search-dirs -x hip hi.cc 
  ROCm installation search path: /opt/rocm

Doesn't that make a whole lot more sense than informing llvm about spack?

Finally, it doesn't settle locating hipcc (nor `.hipVersion), but that 
shouldn't be a real issue from the point of view of a spack user, since spack 
already sets the `-I` and `-L` flags for you when you make your package depend 
on `hip`.

Finally,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97340

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


[PATCH] D102689: [C++4OpenCL] Allow address space conversion in reinterpret_cast

2021-05-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D102689#2778011 , @rjmccall wrote:

> The C++ standard does not appear to have similar wording.  On the other hand, 
> the C++ standard says that e.g. "The result of the expression `(T) 
> cast-expression` is of type `T`", and similarly for the other casts, which is 
> clearly just wrong if `T` is a reference type; the wording clarifies that the 
> expression is an l-value or x-value if the type is a reference but doesn't 
> remove the reference-ness of the expression type as it must, unless that's 
> done by some other clause at a distance.

It is indeed done by a different clause at a distance:

**[expr.type]**

> 1. If an expression initially has the type “reference to `T`”, the type is 
> adjusted to `T` prior to any further analysis. The expression designates the 
> object or function denoted by the reference, and the expression is an lvalue 
> or an xvalue, depending on the expression.



> 2. If a prvalue initially has the type “cv `T`”, where `T` is a 
> cv-unqualified non-class, non-array type, the type of the expression is 
> adjusted to `T` prior to any further analysis.



In D102689#2778011 , @rjmccall wrote:

> C++ has a very strange feature of allowing qualifiers on class types in 
> particular circumstances, so we might need to be permissive about that, 
> although that's tricky for address spaces because (unlike with `const` or 
> `volatile`) we cannot in fact allocate a temporary in an arbitrary address 
> space.

Right, specifically C++ preserves top-level `const` and `volatile` 
qualifications on prvalues only if they're of class or array type. If an 
address-space-qualified temporary makes sense, then I think extending this 
behavior to address spaces makes sense too:

- For array prvalues, an `AS1 int[3]` should decay to an `AS1 int*`, not to a 
plain `int*`, because it seems clear the intent is for the array to be created 
in the given address space.
- For class prvalues, I think it's important that we preserve the address 
space, because (for example) a function `AS1 MyClass f();` might take a return 
value slot as an `AS1` pointer, so a prvalue `f()` should retain the address 
space qualification.

While we can't allocate an address-space-qualified temporary right now, would 
it be a good idea to prepare for the possibility that we will one day be able 
to, in some cases? For example, in a system with separate control and data 
stacks, we might want to put some data on the control stack; in an exotic 
heterogeneous compute system we might be able to allocate memory in the host 
and device stack from a single function.  If so, following the C++ rule for all 
qualifiers might be a good direction. (We'll presumably need to look at the 
other qualifiers and see if any of them would have problems with this 
treatment.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102689

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Jano Simas via Phabricator via cfe-commits
janosimas updated this revision to Diff 347489.
janosimas added a comment.

Fix a mistake in my last patch.
I added a `--` in a command that should not have it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

Files:
  clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
  clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
@@ -1,11 +1,11 @@
 // REQUIRES: shell
 // RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
 // RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
 // RUN: mkdir -p %T/compilation-database-test/
 // RUN: echo '[{"directory": "%T", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %T/compilation-database-test/compile_commands.json
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
 struct A {
   virtual void f() {}
   virtual void g() {}
Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -17,9 +17,9 @@
 detect clang-tidy regressions in the lines touched by a specific patch.
 Example usage for git/svn users:
 
-  git diff -U0 HEAD^ | clang-tidy-diff.py -p1
+  git diff -U0 HEAD^ | clang-tidy-diff.py -strip 1
   svn diff --diff-cmd=diff -x-U0 | \
-  clang-tidy-diff.py -fix -checks=-*,modernize-use-override
+  clang-tidy-diff.py -- -fix -checks=-*,modernize-use-override
 
 """
 
@@ -119,12 +119,13 @@
   parser = argparse.ArgumentParser(description=
'Run clang-tidy against changed files, and '
'output diagnostics only for modified '
-   'lines.')
+   'lines.'
+   '\nclang-tidy arguments should be passed after a \'--\' .')
   parser.add_argument('-clang-tidy-binary', metavar='PATH',
   default='clang-tidy',
   help='path to clang-tidy binary')
-  parser.add_argument('-p', metavar='NUM', default=0,
-  help='strip the smallest prefix containing P slashes')
+  parser.add_argument('-strip', metavar='NUM', default=0,
+  help='strip the smallest prefix containing NUM slashes')
   parser.add_argument('-regex', metavar='PATTERN', default=None,
   help='custom pattern selecting file paths to check '
   '(case sensitive, overrides -iregex)')
@@ -136,34 +137,14 @@
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('-timeout', type=int, default=None,
   help='timeout per each file in seconds.')
-  parser.add_argument('-fix', action='store_true', default=False,
-  help='apply suggested fixes')
-  parser.add_argument('-checks',
-  help='checks filter, when not specified, use clang-tidy '
-  'default',
-  default='')
-  parser.add_argument('-use-color', action='store_true',
-  help='Use colors in output')
-  parser.add_argument('-path', dest='build_path',
-  help='Path used to read a compile command database.')
   if yaml:
 parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
 help='Create a yaml file to store suggested fixes in, '
 'which can be applied with clang-apply-replacements.')
-  parser.add_argument('-extra-arg', dest='extra_arg',
-  action='append', default=[],
-  help='Additional argument to append to the compiler '
-  'comm

[PATCH] D102689: [C++4OpenCL] Allow address space conversion in reinterpret_cast

2021-05-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a reviewer: rsmith.
rjmccall added a comment.

Top-level qualifiers aren't normally meaningful on pr-values.

The C standard says that casts are to the unqualified type:

  N2454 6.5.4p5: Preceding an expression by a parenthesized type name converts 
the value of the expression to the unqualified version of the named type.

The C++ standard does not appear to have similar wording.  On the other hand, 
the C++ standard says that e.g. "The result of the expression `(T) 
cast-expression` is of type `T`", and similarly for the other casts, which is 
clearly just wrong if `T` is a reference type; the wording clarifies that the 
expression is an l-value or x-value if the type is a reference but doesn't 
remove the reference-ness of the expression type as it must, unless that's done 
by some other clause at a distance.  CC'ing Richard.

It makes sense to me that qualifiers on the cast type ought to be uniformly and 
silently ignored, including address space qualifiers.  C++ has a very strange 
feature of allowing qualifiers on class types in particular circumstances, so 
we might need to be permissive about that, although that's tricky for address 
spaces because (unlike with `const` or `volatile`) we cannot in fact allocate a 
temporary in an arbitrary address space.

I agree that this should be done uniformly, not just in OpenCL, unless OpenCL 
wants rules that differ from what we think they ought to be in general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102689

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


[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

2021-05-24 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie added a comment.

In D101763#2777821 , @steakhal wrote:

> It seems like everything passes. Yeey, good job!
> Shall I commit this tomorrow on your behalf?

I do not have commit access to the code base. It would be appreciated if you 
could commit it on my behalf (Ella Ma ).
Thanks to all reviewers again for your suggestions.


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

https://reviews.llvm.org/D101763

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


[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

2021-05-24 Thread Jan Kratochvil via Phabricator via cfe-commits
jankratochvil updated this revision to Diff 347478.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101236

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/Shell/Expr/no_unique_address.cpp
  lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
@@ -44,18 +44,18 @@
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} struct Struct definition
 // CHECK: | |-FieldDecl {{.*}} A 'int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 5
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 5
 // CHECK: | |-FieldDecl {{.*}} B 'int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 7
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 7
 // CHECK: | |-FieldDecl {{.*}} C 'unsigned int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 3
 // CHECK: | |-FieldDecl {{.*}} D 'unsigned int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 15
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 15
 // CHECK: | |-FieldDecl {{.*}} E 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 1
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 1
 // CHECK: | |-FieldDecl {{.*}} F 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 2
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 2
 // CHECK: | |-FieldDecl {{.*}} G 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 3
 // CHECK: | `-FieldDecl {{.*}} H 'char'
-// CHECK: |   `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: |   {{.}}-IntegerLiteral {{.*}} 'int' 3
Index: lldb/test/Shell/Expr/no_unique_address.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/no_unique_address.cpp
@@ -0,0 +1,15 @@
+// Test LLDB does not assert on miscalculated field offsets.
+
+// RUN: %clang_host %s -g -c -o %t.o
+// RUN: %lldb %t.o -o "p c.c" -o exit | FileCheck %s
+
+// CHECK: (lldb) p c.c
+// CHECK-NEXT: (char) $0 = '\0'
+
+struct A {};
+struct B {
+  [[no_unique_address]] A a;
+};
+struct C : B {
+  char c;
+} c;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7196,6 +7196,7 @@
 if (bit_width)
   field->setBitWidth(bit_width);
 SetMemberOwningModule(field, record_decl);
+field->addAttr(NoUniqueAddressAttr::CreateImplicit(ast->getASTContext()));
 
 if (name.empty()) {
   // Determine whether this field corresponds to an anonymous struct or
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6357,6 +6357,78 @@
 ToD->getTemplateSpecializationKind());
 }
 
+struct LLDBMinimalImport : ASTImporterOptionSpecificTestBase {
+  LLDBMinimalImport() {
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport,
+ const std::shared_ptr &SharedState) {
+  auto retval = new ASTImporter(ToContext, ToFileManager, FromContext,
+FromFileManager, true /*MinimalImport*/,
+// We use the regular lookup.
+/*SharedState=*/nullptr);
+  retval->setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+  return retval;
+};
+  }
+};
+
+TEST_P(LLDBMinimalImport, LLDBImportNoUniqueAddress) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+  R"(
+// lldb/test/API/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
+template  struct DWrapper {};
+typedef DWrapper DW;
+struct B {
+  DW spd;
+} b;
+struct E {
+  B &b_ref = b;
+  static DW f() { return {}; }
+} e;
+  )",
+  Lang_CXX11);
+
+  auto *FromE = FirstDeclMatcher().match(FromTU, varDecl(hasName("e")));
+
+#if 0
+  // Set up a stub external storage.
+  FromTU->setHasExternalLexicalStorage(true);
+  // Set up DeclContextBits.HasLazyExternalLexicalLookups to true.
+  FromTU->setMustBuildLookupTable();
+  struct TestExternalASTSource : ExternalASTSource {};
+  FromTU->getASTContext().setExternalSource(new TestExternalASTSource());
+#endif
+
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+struct E;
+extern E e;
+  )",
+  Lang_CXX11);
+  auto *ToE = FirstDeclMatcher().match(ToTU, varDecl(hasName("e")));
+
+#if 1
+  // Set up a 

[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

2021-05-24 Thread Jan Kratochvil via Phabricator via cfe-commits
jankratochvil added a comment.

  cat >1.cpp < struct DWrapper {};
  typedef DWrapper DW;
  struct B {
DW spd;
  } b;
  struct E {
B &b_ref = b;
static DW f() { return {}; }
  } e;
  EOH
  $ (set -ex;./bin/clang -c -o 1.o 1.cpp -Wall -g;./bin/lldb -b ./1.o -o 'p E' 
-o 'p e')
  (lldb) p E
  (lldb) p e
  lldb: .../clang/lib/AST/Decl.cpp:4188: bool 
clang::FieldDecl::isZeroSize(const clang::ASTContext &) const: Assertion 
`isInvalidDecl() && "valid field has incomplete type"' failed.

So importing just the type `E` is insufficient but I somehow cannot import the 
variable `e`:

auto *FromE = FirstDeclMatcher().match(FromTU, 
varDecl(hasName("e")));
  ...
// ASTTests: .../llvm/include/llvm/Support/Casting.h:269: typename 
cast_retty::ret_type llvm::cast(Y *) [X = clang::DeclContext, Y = 
clang::Decl]: Assertion `isa(Val) && "cast() argument of incompatible 
type!"' failed.
llvm::Error Err = findFromTU(FromE)->Importer->ImportDefinition(FromE);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101236

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


[PATCH] D102443: [PowerPC] Added multiple PowerPC builtins

2021-05-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

The Clang part of this looks fine to me; I can't review the backend changes, 
but you have partial approval.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102443

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


[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

2021-05-24 Thread Jan Kratochvil via Phabricator via cfe-commits
jankratochvil updated this revision to Diff 347475.
jankratochvil added a comment.

Update of the testcase but it still does not reproduce the LLDB problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101236

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/Shell/Expr/no_unique_address.cpp
  lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
@@ -44,18 +44,18 @@
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} struct Struct definition
 // CHECK: | |-FieldDecl {{.*}} A 'int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 5
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 5
 // CHECK: | |-FieldDecl {{.*}} B 'int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 7
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 7
 // CHECK: | |-FieldDecl {{.*}} C 'unsigned int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 3
 // CHECK: | |-FieldDecl {{.*}} D 'unsigned int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 15
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 15
 // CHECK: | |-FieldDecl {{.*}} E 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 1
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 1
 // CHECK: | |-FieldDecl {{.*}} F 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 2
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 2
 // CHECK: | |-FieldDecl {{.*}} G 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 3
 // CHECK: | `-FieldDecl {{.*}} H 'char'
-// CHECK: |   `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: |   {{.}}-IntegerLiteral {{.*}} 'int' 3
Index: lldb/test/Shell/Expr/no_unique_address.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/no_unique_address.cpp
@@ -0,0 +1,15 @@
+// Test LLDB does not assert on miscalculated field offsets.
+
+// RUN: %clang_host %s -g -c -o %t.o
+// RUN: %lldb %t.o -o "p c.c" -o exit | FileCheck %s
+
+// CHECK: (lldb) p c.c
+// CHECK-NEXT: (char) $0 = '\0'
+
+struct A {};
+struct B {
+  [[no_unique_address]] A a;
+};
+struct C : B {
+  char c;
+} c;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7196,6 +7196,7 @@
 if (bit_width)
   field->setBitWidth(bit_width);
 SetMemberOwningModule(field, record_decl);
+field->addAttr(NoUniqueAddressAttr::CreateImplicit(ast->getASTContext()));
 
 if (name.empty()) {
   // Determine whether this field corresponds to an anonymous struct or
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6357,6 +6357,78 @@
 ToD->getTemplateSpecializationKind());
 }
 
+struct LLDBMinimalImport : ASTImporterOptionSpecificTestBase {
+  LLDBMinimalImport() {
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport,
+ const std::shared_ptr &SharedState) {
+  auto retval = new ASTImporter(ToContext, ToFileManager, FromContext,
+FromFileManager, true /*MinimalImport*/,
+// We use the regular lookup.
+/*SharedState=*/nullptr);
+  retval->setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+  return retval;
+};
+  }
+};
+
+TEST_P(LLDBMinimalImport, LLDBImportNoUniqueAddress) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+  R"(
+// lldb/test/API/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
+template  struct DWrapper {};
+typedef DWrapper DW;
+struct B {
+  DW spd;
+} b;
+struct E {
+  B &b_ref = b;
+  static DW f() { return {}; }
+} e;
+  )",
+  Lang_CXX11);
+
+  auto *FromE = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("E")));
+
+#if 0
+  // Set up a stub external storage.
+  FromTU->setHasExternalLexicalStorage(true);
+  // Set up DeclContextBits.HasLazyExternalLexicalLookups to true.
+  FromTU->setMustBuildLookupTable();
+  struct TestExternalASTSource : ExternalASTSource {};
+  FromTU->getASTContext().setExternalSource(new TestExternalASTSource());
+#endif
+
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+struct E;
+  )",
+  

[PATCH] D101192: Add support for #elifdef and #elifndef

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D101192#2763498 , @mibintc wrote:

> let 'er rip

Thanks @mibintc!

@rsmith -- I'll land this sometime later this week unless I hear further 
comments from you. If I land it and you still have feedback, I can address it 
post commit.


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

https://reviews.llvm.org/D101192

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D49864#274 , @janosimas wrote:

> I manage to upload it, copy-pasting the content to the text box. I have no 
> idea why it is not working with the upload.
>
> Updates from the last change
>
> - Modified the tests to match the input
> - Addend file context

Can confirm that the new diff looks to have not been munged somehow. Sorry that 
you had the weird Phab issues, hopefully they're transitory and won't crop up 
again on the review.

The changes seem reasonable to me, but you should wait for @alexfh to sign off 
on the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

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


[PATCH] D103040: Print default template argument if manually specified in typedef declaration.

2021-05-24 Thread Pratyush Das via Phabricator via cfe-commits
reikdas created this revision.
reikdas added a reviewer: rsmith.
reikdas requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If a default template type argument is manually specified to be of the default 
type, then it is committed when printing the template.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103040

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/SemaTemplate/class-template-id.cpp
  clang/test/SemaTemplate/default-arguments-ast-print.cpp


Index: clang/test/SemaTemplate/default-arguments-ast-print.cpp
===
--- clang/test/SemaTemplate/default-arguments-ast-print.cpp
+++ clang/test/SemaTemplate/default-arguments-ast-print.cpp
@@ -10,3 +10,15 @@
   // CHECK: int Foo::method1()
   return 10;
 }
+
+int test_typedef() {
+  typedef Foo TypedefArg;
+  // CHECK: typedef Foo TypedefArg;
+  return 10;
+}
+
+int test_typedef2() {
+  typedef Foo TypedefArg;
+  // CHECK: typedef Foo TypedefArg;
+  return 10;
+}
Index: clang/test/SemaTemplate/class-template-id.cpp
===
--- clang/test/SemaTemplate/class-template-id.cpp
+++ clang/test/SemaTemplate/class-template-id.cpp
@@ -9,9 +9,9 @@
   if (ptr)
 return ptr; // okay
   else if (ptr2)
-return ptr2; // expected-error{{cannot initialize return object of type 
'A *' with an lvalue of type 'const A *'}}
+return ptr2; // expected-error{{cannot initialize return object of type 
'A *' (aka 'A *') with an lvalue of type 'const A 
*'}}
   else {
-return ptr3; // expected-error{{cannot initialize return object of type 
'A *' with an lvalue of type 'A *'}}
+return ptr3; // expected-error{{cannot initialize return object of type 
'A *' (aka 'A *') with an lvalue of type 'A *'}}
   }
 }
 
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1447,8 +1447,7 @@
 T->getTemplateName().print(OS, Policy);
   }
 
-  const TemplateParameterList *TPL = TD ? TD->getTemplateParameters() : 
nullptr;
-  printTemplateArgumentList(OS, T->template_arguments(), Policy, TPL);
+  printTemplateArgumentList(OS, T->template_arguments(), Policy);
   spaceBeforePlaceHolder(OS);
 }
 


Index: clang/test/SemaTemplate/default-arguments-ast-print.cpp
===
--- clang/test/SemaTemplate/default-arguments-ast-print.cpp
+++ clang/test/SemaTemplate/default-arguments-ast-print.cpp
@@ -10,3 +10,15 @@
   // CHECK: int Foo::method1()
   return 10;
 }
+
+int test_typedef() {
+  typedef Foo TypedefArg;
+  // CHECK: typedef Foo TypedefArg;
+  return 10;
+}
+
+int test_typedef2() {
+  typedef Foo TypedefArg;
+  // CHECK: typedef Foo TypedefArg;
+  return 10;
+}
Index: clang/test/SemaTemplate/class-template-id.cpp
===
--- clang/test/SemaTemplate/class-template-id.cpp
+++ clang/test/SemaTemplate/class-template-id.cpp
@@ -9,9 +9,9 @@
   if (ptr)
 return ptr; // okay
   else if (ptr2)
-return ptr2; // expected-error{{cannot initialize return object of type 'A *' with an lvalue of type 'const A *'}}
+return ptr2; // expected-error{{cannot initialize return object of type 'A *' (aka 'A *') with an lvalue of type 'const A *'}}
   else {
-return ptr3; // expected-error{{cannot initialize return object of type 'A *' with an lvalue of type 'A *'}}
+return ptr3; // expected-error{{cannot initialize return object of type 'A *' (aka 'A *') with an lvalue of type 'A *'}}
   }
 }
 
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1447,8 +1447,7 @@
 T->getTemplateName().print(OS, Policy);
   }
 
-  const TemplateParameterList *TPL = TD ? TD->getTemplateParameters() : nullptr;
-  printTemplateArgumentList(OS, T->template_arguments(), Policy, TPL);
+  printTemplateArgumentList(OS, T->template_arguments(), Policy);
   spaceBeforePlaceHolder(OS);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103039: [AST] fully-qualify template args of outer types in getFullyQualifiedType

2021-05-24 Thread Victor Kuznetsov via Phabricator via cfe-commits
victordk created this revision.
victordk added reviewers: ilya-biryukov, saugustine.
victordk requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Template args of outer types were not fully-qualified when calling 
getFullyQualifiedType() for inner types.

For simplicity the patch is a copy-paste of the same call from 
getFullyQualifiedType().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103039

Files:
  clang/lib/AST/QualTypeNames.cpp
  clang/unittests/Tooling/QualTypeNamesTest.cpp


Index: clang/unittests/Tooling/QualTypeNamesTest.cpp
===
--- clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ clang/unittests/Tooling/QualTypeNamesTest.cpp
@@ -93,12 +93,14 @@
   "Foo::non_dependent_type";
   Visitor.ExpectedQualTypeNames["AnEnumVar"] = "EnumScopeClass::AnEnum";
   Visitor.ExpectedQualTypeNames["AliasTypeVal"] = "A::B::C::InnerAlias";
+  Visitor.ExpectedQualTypeNames["AliasInnerTypeVal"] =
+  "OuterTemplateClass::Inner";
   Visitor.ExpectedQualTypeNames["CheckM"] = "const A::B::Class0 *";
   Visitor.ExpectedQualTypeNames["CheckN"] = "const X *";
   Visitor.runOver(
   "int CheckInt;\n"
   "template \n"
-  "class OuterTemplateClass { };\n"
+  "class OuterTemplateClass { public: struct Inner {}; };\n"
   "namespace A {\n"
   " namespace B {\n"
   "   class Class0 { };\n"
@@ -107,6 +109,7 @@
   " template \n"
   " using InnerAlias = OuterTemplateClass;\n"
   " InnerAlias AliasTypeVal;\n"
+  " InnerAlias::Inner AliasInnerTypeVal;\n"
   "   }\n"
   "   template class Template0;"
   "   template class Template1;"
@@ -165,8 +168,7 @@
   "  enum AnEnum { ZERO, ONE };\n"
   "};\n"
   "EnumScopeClass::AnEnum AnEnumVar;\n",
-  TypeNameVisitor::Lang_CXX11
-);
+  TypeNameVisitor::Lang_CXX11);
 
   TypeNameVisitor Complex;
   Complex.ExpectedQualTypeNames["CheckTX"] = "B::TX";
Index: clang/lib/AST/QualTypeNames.cpp
===
--- clang/lib/AST/QualTypeNames.cpp
+++ clang/lib/AST/QualTypeNames.cpp
@@ -356,11 +356,19 @@
const TypeDecl *TD,
bool FullyQualify,
bool WithGlobalNsPrefix) {
+  const Type *TypePtr = TD->getTypeForDecl();
+  if (isa(TypePtr) ||
+  isa(TypePtr)) {
+// We are asked to fully qualify and we have a Record Type (which
+// may point to a template specialization) or Template
+// Specialization Type. We need to fully qualify their arguments.
+
+TypePtr = getFullyQualifiedTemplateType(Ctx, TypePtr, WithGlobalNsPrefix);
+  }
+
   return NestedNameSpecifier::Create(
-  Ctx,
-  createOuterNNS(Ctx, TD, FullyQualify, WithGlobalNsPrefix),
-  false /*No TemplateKeyword*/,
-  TD->getTypeForDecl());
+  Ctx, createOuterNNS(Ctx, TD, FullyQualify, WithGlobalNsPrefix),
+  false /*No TemplateKeyword*/, TypePtr);
 }
 
 /// Return the fully qualified type, including fully-qualified


Index: clang/unittests/Tooling/QualTypeNamesTest.cpp
===
--- clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ clang/unittests/Tooling/QualTypeNamesTest.cpp
@@ -93,12 +93,14 @@
   "Foo::non_dependent_type";
   Visitor.ExpectedQualTypeNames["AnEnumVar"] = "EnumScopeClass::AnEnum";
   Visitor.ExpectedQualTypeNames["AliasTypeVal"] = "A::B::C::InnerAlias";
+  Visitor.ExpectedQualTypeNames["AliasInnerTypeVal"] =
+  "OuterTemplateClass::Inner";
   Visitor.ExpectedQualTypeNames["CheckM"] = "const A::B::Class0 *";
   Visitor.ExpectedQualTypeNames["CheckN"] = "const X *";
   Visitor.runOver(
   "int CheckInt;\n"
   "template \n"
-  "class OuterTemplateClass { };\n"
+  "class OuterTemplateClass { public: struct Inner {}; };\n"
   "namespace A {\n"
   " namespace B {\n"
   "   class Class0 { };\n"
@@ -107,6 +109,7 @@
   " template \n"
   " using InnerAlias = OuterTemplateClass;\n"
   " InnerAlias AliasTypeVal;\n"
+  " InnerAlias::Inner AliasInnerTypeVal;\n"
   "   }\n"
   "   template class Template0;"
   "   template class Template1;"
@@ -165,8 +168,7 @@
   "  enum AnEnum { ZERO, ONE };\n"
   "};\n"
   "EnumScopeClass::AnEnum AnEnumVar;\n",
-  TypeNameVisitor::Lang_CXX11
-);
+  TypeNameVisitor::Lang_CXX11);
 
   TypeNameVisitor Complex;
   Complex.ExpectedQualTypeNames["CheckTX"] = "B::TX";
Index: clang/lib/AST/QualTypeNames.cpp
===
--- clang/lib/AST/QualTypeNames.cpp
+++ clang/lib/AST/QualTypeNames.cpp
@@ -356,11 +356,19 @@
const TypeDecl *TD,
bool FullyQualif

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:22
+namespace {
+auto hasAnyWhitelistedName(const std::string &Names) {
+  const std::vector NameList =





Comment at: 
clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:23-24
+auto hasAnyWhitelistedName(const std::string &Names) {
+  const std::vector NameList =
+  utils::options::parseStringList(Names);
+  return hasAnyName(std::vector(NameList.begin(), NameList.end()));

It's unfortunate that this is parsing the list of names each time -- I think we 
should parse the string list one time and pass a container in to this function 
rather than doing the split every time we encounter a constructor.



Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:102
   if (const auto *Conversion =
-  Result.Nodes.getNodeAs("conversion")) {
+  Result.Nodes.getNodeAs("conversion")) {
 if (Conversion->isOutOfLine())

Unintended formatting change?



Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h:28
+  : ClangTidyCheck(Name, Context),
+ConstructorWhitelist(Options.get("ConstructorWhitelist", "")) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;

The community has recently been switching away from "whitelist" and "blacklist" 
terminology, so I'd recommend changing this to `IgnoredConstructors`.

Should conversion operators also have an allow list? (The core guideline 
doesn't mention it, but I'm wondering about code that wants an implicit 
conversion in a particular case and how they should silence the diagnostic in 
that case.)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp:11
+// RUN: ]}'
+
+struct A {

It looks like there are no tests for conversion operators -- those should be 
added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

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


[PATCH] D95070: Fix crash when emitting NullReturn guards for functions returning BOOL

2021-05-24 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs closed this revision.
jroelofs added a comment.

This landed in 1deee5cacbb76578367186d7ff2937b6fa79b827 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95070

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


[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

2021-05-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal added a comment.

It seems like everything passes. Yeey, good job!
Shall I commit this tomorrow on your behalf?


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

https://reviews.llvm.org/D101763

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


[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from some minor nits.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3961
+def warn_deprecated_ignored_on_using : Warning<
+  "%0 currently has no effect on using-declarations">,
+  InGroup;





Comment at: clang/lib/Parse/ParseDeclCXX.cpp:501
   return ParseUsingDeclaration(Context, TemplateInfo, UsingLoc, DeclEnd,
-   AS_none);
+   attrs, AS_none);
 }

Might as well clang-format this (same for the other clang-format issues where 
you've changed the code).



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:693
+Tok.getLocation(),
+CharSourceRange::getTokenRange(MisplacedAttrs.Range))
+<< FixItHint::CreateRemoval(MisplacedAttrs.Range);

You can ignore this clang-format message though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91630

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


[PATCH] D86881: Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin

2021-05-24 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

Hey, thanks for following up on this PR. I've done some more digging and I 
think we can remove this Darwin-specific workaround in the near future. I'm 
hoping to provide an update in the next few weeks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86881

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Jano Simas via Phabricator via cfe-commits
janosimas added a comment.

> Hmm, I'm not certain what's going on there. When I try to upload a patch for 
> the review, Phab lets me upload it (but I didn't try to submit the changes). 
> At what stage are you getting the failure? Is it when uploading the patch 
> itself, or at some other point?

Immediately when I try to upload a diff file.
I fill the form with the file I want to upload, the repo and when I try to 
submit I get the error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Jano Simas via Phabricator via cfe-commits
janosimas updated this revision to Diff 347459.
janosimas added a comment.

I manage to upload it, copy-pasting the content to the text box. I have no idea 
why it is not working with the upload.

Updates from the last change

- Modified the tests to match the input
- Addend file context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

Files:
  clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
  clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
@@ -1,11 +1,11 @@
 // REQUIRES: shell
 // RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
-// RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
+// RUN: clang-tidy -- -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
 // RUN: mkdir -p %T/compilation-database-test/
 // RUN: echo '[{"directory": "%T", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %T/compilation-database-test/compile_commands.json
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -- -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
 struct A {
   virtual void f() {}
   virtual void g() {}
Index: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -17,9 +17,9 @@
 detect clang-tidy regressions in the lines touched by a specific patch.
 Example usage for git/svn users:
 
-  git diff -U0 HEAD^ | clang-tidy-diff.py -p1
+  git diff -U0 HEAD^ | clang-tidy-diff.py -strip 1
   svn diff --diff-cmd=diff -x-U0 | \
-  clang-tidy-diff.py -fix -checks=-*,modernize-use-override
+  clang-tidy-diff.py -- -fix -checks=-*,modernize-use-override
 
 """
 
@@ -119,12 +119,13 @@
   parser = argparse.ArgumentParser(description=
'Run clang-tidy against changed files, and '
'output diagnostics only for modified '
-   'lines.')
+   'lines.'
+   '\nclang-tidy arguments should be passed after a \'--\' .')
   parser.add_argument('-clang-tidy-binary', metavar='PATH',
   default='clang-tidy',
   help='path to clang-tidy binary')
-  parser.add_argument('-p', metavar='NUM', default=0,
-  help='strip the smallest prefix containing P slashes')
+  parser.add_argument('-strip', metavar='NUM', default=0,
+  help='strip the smallest prefix containing NUM slashes')
   parser.add_argument('-regex', metavar='PATTERN', default=None,
   help='custom pattern selecting file paths to check '
   '(case sensitive, overrides -iregex)')
@@ -136,34 +137,14 @@
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('-timeout', type=int, default=None,
   help='timeout per each file in seconds.')
-  parser.add_argument('-fix', action='store_true', default=False,
-  help='apply suggested fixes')
-  parser.add_argument('-checks',
-  help='checks filter, when not specified, use clang-tidy '
-  'default',
-  default='')
-  parser.add_argument('-use-color', action='store_true',
-  help='Use colors in output')
-  parser.add_argument('-path', dest='build_path',
-  help='Path used to read a compile command database.')
   if yaml:
 parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
 help='Create a yaml file to store suggested fixes in, '
 'which can be appli

[PATCH] D102338: [Sema] Always search the full function scope context if a potential availability violation is encountered

2021-05-24 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

I think this is good to go, I haven't observed any issues with this patch so 
far in my testing. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102338

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


[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

FWIW, this looks like it's also partially covering `-Wnon-virtual-dtor`, but 
that doesn't seem to have the checks for a protected destructor.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:110-111
+diag(MatchedClassOrStruct->getLocation(),
+ "destructor of %0 is private and prevents using the type. Consider "
+ "making it public and virtual or protected and non-virtual")
+<< MatchedClassOrStruct;

clang-tidy diagnostics should not contain full sentences with punctuation in 
them (they follow the Clang diagnostic rules).

One thing you could do to resolve this is to issue one diagnostic for the 
warning, and two diagnostics for notes with fixits attached to them. e.g.,

warning: `destructor of %0 is 'private' and prevents using the type`
note 1: `consider making it public and virtual` (fixit to insert `public:` 
before the declaration, insert `virtual` in the declaration, 
and insert `private:` after the declaration)
note 2: `consider making it protected` (fixit to insert `protected:` before the 
declaration and insert `private:` after the declaration)

If you don't want to do the fixits, you could reword the diagnostic to combine 
the sentences, but it's a bit lengthy.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:135-137
+   "destructor of %0 is %select{public and non-virtual|protected and "
+   "virtual}1. It should either be public and virtual or protected and "
+   "non-virtual")

Same issue here as above.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp:8
+private:
+  virtual ~PrivateVirtualBaseStruct(){};
+};

All of these empty dtors have a semi-colon after their closing brace that can 
be removed.

I'd also like to see a test that `= default` works fine for the dtor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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


[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

2021-05-24 Thread Artem Belevich via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a75c06cd9d9: [CUDA]  Work around compatibility issue with 
libstdc++ 11.1.0 (authored by tra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

Files:
  clang/lib/Headers/cuda_wrappers/complex


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 


Index: clang/lib/Headers/cuda_wrappers/complex
===
--- clang/lib/Headers/cuda_wrappers/complex
+++ clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9a75c06 - [CUDA] Work around compatibility issue with libstdc++ 11.1.0

2021-05-24 Thread Artem Belevich via cfe-commits

Author: Artem Belevich
Date: 2021-05-24T11:07:09-07:00
New Revision: 9a75c06cd9d94d3fd13c47a01044da97b98cf26b

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

LOG: [CUDA]  Work around compatibility issue with libstdc++ 11.1.0

libstdc++ redeclares __failed_assertion multiple times and that results in the
function declared with conflicting set of attributes when we include 
with __host__ __device__ attributes force-applied to all functions.

In order to work around the issue, we rename __failed_assertion within the
region with forced attributes.

See https://bugs.llvm.org/show_bug.cgi?id=50383 for the details.

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

Added: 


Modified: 
clang/lib/Headers/cuda_wrappers/complex

Removed: 




diff  --git a/clang/lib/Headers/cuda_wrappers/complex 
b/clang/lib/Headers/cuda_wrappers/complex
index 11d40a82a8f6a..e6805b6044e98 100644
--- a/clang/lib/Headers/cuda_wrappers/complex
+++ b/clang/lib/Headers/cuda_wrappers/complex
@@ -72,8 +72,16 @@
 #define _GLIBCXX_USE_C99_COMPLEX 0
 #define _GLIBCXX_USE_C99_COMPLEX_TR1 0
 
+// Work around a compatibility issue with libstdc++ 11.1.0
+// https://bugs.llvm.org/show_bug.cgi?id=50383
+#pragma push_macro("__failed_assertion")
+#if _GLIBCXX_RELEASE == 11
+#define __failed_assertion __cuda_failed_assertion
+#endif
+
 #include_next 
 
+#pragma pop_macro("__failed_assertion")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX_TR1")
 #pragma pop_macro("_GLIBCXX_USE_C99_COMPLEX")
 



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


[PATCH] D102975: [HIP] Check compatibility of -fgpu-sanitize with offload arch

2021-05-24 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.
Herald added a subscriber: foad.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.h:114-115
+  /// specified and valid.
+  std::tuple, Optional,
+ Optional>>
+  getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const;

I'd use a struct instead. It makes things more readable in all the other other 
places where we don't have the exact return type spelled out.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:473
+
+  auto &FeatureMap = std::get<2>(ParsedTargetID).getValue();
+  // Sanitizer is not supported with xnack-.

What if FeatureMap is `None`?
Considering that it's derived from user input, this should be properly checked 
and diagnosed, if we didn't get the feature map.
At the very least there should be an assert, if we ensure somewhere else that 
we always get a valid FeatureMap here.


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

https://reviews.llvm.org/D102975

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


[PATCH] D101630: [HIP] Fix device-only compilation

2021-05-24 Thread Artem Belevich via Phabricator via cfe-commits
tra added a subscriber: echristo.
tra added a comment.

In D101630#2777346 , @yaxunl wrote:

> In D101630#2748513 , @tra wrote:
>
>> How about this:
>> If the user explicitly specified `--cuda-host-only` or `--cuda-device-only`, 
>> then by default only allow producing the natural output format, unless a 
>> bundled output is requested by an option. This should keep existing users 
>> working.
>> If the compilation is done without explicitly requested sub-compilation(s), 
>> then bundle the output by default. This should keep the GPU-unaware tools 
>> like ccache happy as they would always get the single output they expect.
>>
>> WDYT?
>
> `--cuda-host-only` always have one output, therefore there is no point of 
> bundle its output. We only need to decide the proper behavior of 
> `--cuda-device-only`.

It still fits my proposal of requiring a single sub-compilation and not 
bundling the output.
The point was that such behavior is consistent regardless of whether we're 
compiling CUDA or HIP for the host or for device.

> How about keeping the original default behavior of not bundling if users do 
> not specify output file, 
> whereas bundle the output if users specifying output file.

I think it will make things worse. Compiler output should not change depending 
on whether `-o` is used.

> Since specifying output file indicates users  requesting one output. 
> -f[no-]hip-bundle-device-output override the default behavior.

I disagree. When user specifies the output, the intent is to specify the 
**location** of the outputs, not its contents or format.

Telling compiler to produce a different output format should not depend on 
specifying (or not) the output location.

I think our options are:

- Always bundle --cuda-device-only outputs by default. This is consistent for 
HIP compilation, but deviates from CUDA, which can't do bundling. Also, 
single-target subcompilation deviates from both CUDA and regular C++ 
compilation, which is what most users would be familiar with and which would 
probably be the most sensible default for a single sub-compilation. It can be 
overridden with an option, but it goes against the principle that it's 
specialized use case that should need extra options. The most common use case 
should not need them.

- Only bundle multiple sub-compilations' output by default. This would preserve 
the sensible single sub-compilation behavior. The downside is that it makes the 
output format depend on whether compiler ends up doing one or many 
sub-compilations. E.g. `--offload-arch=A -S` would produce ASM and 
`--offload-arch=A --offload-arch=B -S` would produce a bundle. If the user 
can't control some of the compiler options, Such approach would make output 
format unpredictable. E.g. passing `--offload-arch=foo` to compiler on godbolt 
would all of a sudden produce bundled output instead of assembly text or a 
sensible error message that you're trying to produce multiple outputs.

- Keep the current behavior (insist on single sub-compilation) as the default, 
allow overriding it for HIP with the flag. IMO that's the most consistent 
option and I still think it's the one most suitable to keep as the default.

I can see the benefit of always bundling for HIP, but I also believe that 
keeping things simple, consistent and predictable is important. Considering 
that we're tinkering in a relatively obscure niche of the compiler, it probably 
does not matter all that much, but it should not stop us from trying to figure 
out the best approach in a principled way.

I think we could benefit from a second opinion on which approach would make 
more sense for clang. 
Summoning @jdoerfert and @echristo.


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

https://reviews.llvm.org/D101630

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


[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

2021-05-24 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie updated this revision to Diff 347446.
OikawaKirie added a comment.

Re-submit the patch to re-build the code.

Sorry for the spam of submitting an incomplete diff which triggers a patch 
apply failure.


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

https://reviews.llvm.org/D101763

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp

Index: clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %host_cxx %s -fPIC -shared -o %t/mock_open.so
+
+// RUN: echo "void bar(); void foo() { bar(); bar(); }" > %t/trigger.c
+// RUN: echo "void bar() {}" > %t/importee.c
+// RUN: echo '[{"directory":"%t", "command":"cc -c %t/importee.c", "file": "%t/importee.c"}]' > %t/compile_commands.json
+// RUN: %clang_extdef_map -p %t "%t/importee.c" > %t/externalDefMap.txt
+
+// Add an empty invocation list to make the on-demand parsing fail and load it again.
+// RUN: echo '' > %t/invocations.yaml
+
+// RUN: cd %t && \
+// RUN: LD_PRELOAD=%t/mock_open.so \
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN:   %t/trigger.c | FileCheck %s
+
+// REQUIRES: shell, system-linux
+
+// CHECK: {{Opening file invocations.yaml: 1}}
+// CHECK-NOT: {{Opening file invocations.yaml: 2}}
+
+#define _GNU_SOURCE 1
+#include 
+#include 
+
+#include 
+#include 
+#include 
+using namespace std;
+
+extern "C" int open(const char *name, int flag, ...) {
+  // Log how many times the invocation list is opened.
+  if ("invocations.yaml" == string(name)) {
+static unsigned N = 0;
+cout << "Opening file invocations.yaml: " << ++N << endl;
+  }
+
+  // The original open function will be called to open the files.
+  using open_t = int (*)(const char *, int, mode_t);
+  static open_t o_open = nullptr;
+  if (!o_open)
+o_open = reinterpret_cast(dlsym(RTLD_NEXT, "open"));
+  assert(o_open && "Cannot find function `open'.");
+
+  va_list vl;
+  va_start(vl, flag);
+  auto mode = va_arg(vl, mode_t);
+  va_end(vl);
+  return o_open(name, flag, mode);
+}
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -92,6 +92,10 @@
 
   std::string message(int Condition) const override {
 switch (static_cast(Condition)) {
+case index_error_code::success:
+  // There should not be a success error. Jump to unreachable directly.
+  // Add this case to make the compiler stop complaining.
+  break;
 case index_error_code::unspecified:
   return "An unknown error has occurred.";
 case index_error_code::missing_index_file:
@@ -667,12 +671,15 @@
   /// Lazily initialize the invocation list member used for on-demand parsing.
   if (InvocationList)
 return llvm::Error::success();
+  if (index_error_code::success != PreviousParsingResult)
+return llvm::make_error(PreviousParsingResult);
 
   llvm::ErrorOr> FileContent =
   llvm::MemoryBuffer::getFile(InvocationListFilePath);
-  if (!FileContent)
-return llvm::make_error(
-index_error_code::invocation_list_file_not_found);
+  if (!FileContent) {
+PreviousParsingResult = index_error_code::invocation_list_file_not_found;
+return llvm::make_error(PreviousParsingResult);
+  }
   std::unique_ptr ContentBuffer = std::move(*FileContent);
   assert(ContentBuffer && "If no error was produced after loading, the pointer "
   "should not be nullptr.");
@@ -680,8 +687,13 @@
   llvm::Expected ExpectedInvocationList =
   parseInvocationList(ContentBuffer->getBuffer(), PathStyle);
 
-  if (!ExpectedInvocationList)
-return ExpectedInvocationList.takeError();
+  // Handle the error to store the code for next call to this function.
+  if (!ExpectedInvocationList) {
+llvm::handleAllErrors(
+ExpectedInvocationList.takeError(),
+[&](const IndexError &E) { PreviousParsingResult = E.getCode(); });
+return llvm::make_error(PreviousParsingResult);
+  }
 
   InvocationList = *ExpectedInvocationList;
 
Index: clang/include/clang/CrossTU/CrossTranslationUnit.h
===
--- clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -38,6 +38,7 @@
 namespace cross_tu {
 
 enum class index_error_code {
+  success = 0,
   unspecified = 1,
   m

[PATCH] D103036: [cfe][inline-asm] Support target-specific escaped character in inline asm

2021-05-24 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

LGTM; please fix up minor nits. Thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103036

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


[PATCH] D103036: [cfe][inline-asm] Support target-specific escaped character in inline asm

2021-05-24 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:1094-1095
 
+  // Replace some escaped characters with another string based on
+  // target-specific rules
+  virtual llvm::Optional handleAsmEscapedChar(char C) const {

Use triple slashes so that a Doxygen comment can be produced.  End sentence 
with a period.



Comment at: clang/lib/AST/Stmt.cpp:671
 }
+// Handle target-specific escaped characters
+if (auto MaybeReplaceStr = TI.handleAsmEscapedChar(EscapedChar)) {

nickdesaulniers wrote:
> Do you want to move this addition into the `default` case above?
Terminate sentence with period.



Comment at: clang/lib/AST/Stmt.cpp:671-675
+// Handle target-specific escaped characters
+if (auto MaybeReplaceStr = TI.handleAsmEscapedChar(EscapedChar)) {
+  CurStringPiece += *MaybeReplaceStr;
+  continue;
+}

Do you want to move this addition into the `default` case above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103036

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


[PATCH] D98895: [X86][Draft] Disable long double type for -mno-x87 option

2021-05-24 Thread Andrew Savonichev via Phabricator via cfe-commits
asavonic updated this revision to Diff 347443.
asavonic added a comment.

Added LIT run lines for i686 and windows targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/x86-no-x87.c

Index: clang/test/Sema/x86-no-x87.c
===
--- /dev/null
+++ clang/test/Sema/x86-no-x87.c
@@ -0,0 +1,73 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-linux-gnu -target-feature -x87
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -target-feature -x87
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-windows-msvc -target-feature -x87 -DNOERROR
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-linux-gnu -DNOERROR
+
+#ifdef NOERROR
+// expected-no-diagnostics
+#endif
+
+typedef long double long_double;
+
+#ifndef NOERROR
+// expected-error@+3{{long double is not supported on this target}}
+// expected-error@+2{{long double is not supported on this target}}
+#endif
+double ld_args(long_double x, long_double y);
+
+long_double ld_ret(double x, double y);
+
+double args(double x, double y) {
+  return ld_args(x, y);
+}
+
+double ret(double x, double y) {
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  return ld_ret(x, y);
+}
+
+double binop(double x, double y) {
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  double z = (long_double)x * (long_double)y;
+  return z;
+}
+
+void assign1(long_double *ret, double x) {
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  *ret = x;
+}
+
+struct st_long_double {
+  long_double ld;
+};
+
+void assign2() {
+  struct st_long_double st;
+  st.ld = 0.42;
+}
+
+void assign3() {
+  struct st_long_double st;
+  st.ld = 42;
+}
+
+void assign4(double d) {
+  struct st_long_double st;
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  st.ld = d;
+}
+
+void assign5() {
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  long_double ld = 0.42;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14002,6 +14002,26 @@
 }
   }
 
+  if (!Context.getTargetInfo().hasLongDoubleType()) {
+QualType LHSTy = LHSExpr->getType();
+QualType RHSTy = RHSExpr->getType();
+
+// Allow assignment of a literal, because it compiles to just a
+// store of a constant.
+bool IsLiteralAssign =
+(Opc == BO_Assign) &&
+(isa(RHSExpr) || isa(RHSExpr));
+
+bool HaveLongDoubleOp =
+(LHSTy.getCanonicalType() == Context.LongDoubleTy) ||
+(RHSTy.getCanonicalType() == Context.LongDoubleTy);
+
+if (HaveLongDoubleOp && !IsLiteralAssign) {
+  Diag(OpLoc, diag::err_type_unsupported) << "long double";
+  return ExprError();
+}
+  }
+
   switch (Opc) {
   case BO_Assign:
 ResultTy = CheckAssignmentOperands(LHS.get(), RHS, OpLoc, QualType());
@@ -14840,6 +14860,13 @@
   if (Opc != UO_AddrOf && Opc != UO_Deref)
 CheckArrayAccess(Input.get());
 
+  if (!Context.getTargetInfo().hasLongDoubleType() &&
+  (resultType.getCanonicalType() == Context.LongDoubleTy ||
+   InputExpr->getType().getCanonicalType() == Context.LongDoubleTy)) {
+Diag(OpLoc, diag::err_type_unsupported) << "long double";
+return ExprError();
+  }
+
   auto *UO =
   UnaryOperator::Create(Context, Input.get(), Opc, resultType, VK, OK,
 OpLoc, CanOverflow, CurFPFeatureOverrides());
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7270,6 +7270,11 @@
 }
   }
 
+  if (!Context.getTargetInfo().hasLongDoubleType() &&
+  NewVD->getType().getCanonicalType() == Context.LongDoubleTy) {
+Diag(NewVD->getLocation(), diag::err_type_unsupported) << "long double";
+  }
+
   // Handle attributes prior to checking for duplicates in MergeVarDecl
   ProcessDeclAttributes(S, NewVD, D);
 
@@ -14223,6 +14228,17 @@
   CheckParmsForFunctionDef(FD->parameters(),
/*CheckParameterNames=*/true);
 
+  if (!Context.getTargetInfo().hasLongDoubleType()) {
+if (ResultType.getCanonicalType() == Context.LongDoubleTy) {
+  Diag(FD->getLocation(), diag::err_type_unsupported) << "long double";
+}
+for (ParmVarDecl *Param : FD->parameters()) {
+  if (Param->getType().getCanonicalType() == Contex

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

2021-05-24 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie updated this revision to Diff 347441.
OikawaKirie added a comment.

Re-submit the patch to re-build the code.


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

https://reviews.llvm.org/D101763

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp

Index: clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %host_cxx %s -fPIC -shared -o %t/mock_open.so
+
+// RUN: echo "void bar(); void foo() { bar(); bar(); }" > %t/trigger.c
+// RUN: echo "void bar() {}" > %t/importee.c
+// RUN: echo '[{"directory":"%t", "command":"cc -c %t/importee.c", "file": "%t/importee.c"}]' > %t/compile_commands.json
+// RUN: %clang_extdef_map -p %t "%t/importee.c" > %t/externalDefMap.txt
+
+// Add an empty invocation list to make the on-demand parsing fail and load it again.
+// RUN: echo '' > %t/invocations.yaml
+
+// RUN: cd %t && \
+// RUN: LD_PRELOAD=%t/mock_open.so \
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN:   %t/trigger.c | FileCheck %s
+
+// REQUIRES: shell, system-linux
+
+// CHECK: {{Opening file invocations.yaml: 1}}
+// CHECK-NOT: {{Opening file invocations.yaml: 2}}
+
+#define _GNU_SOURCE 1
+#include 
+#include 
+
+#include 
+#include 
+#include 
+using namespace std;
+
+extern "C" int open(const char *name, int flag, ...) {
+  // Log how many times the invocation list is opened.
+  if ("invocations.yaml" == string(name)) {
+static unsigned N = 0;
+cout << "Opening file invocations.yaml: " << ++N << endl;
+  }
+
+  // The original open function will be called to open the files.
+  using open_t = int (*)(const char *, int, mode_t);
+  static open_t o_open = nullptr;
+  if (!o_open)
+o_open = reinterpret_cast(dlsym(RTLD_NEXT, "open"));
+  assert(o_open && "Cannot find function `open'.");
+
+  va_list vl;
+  va_start(vl, flag);
+  auto mode = va_arg(vl, mode_t);
+  va_end(vl);
+  return o_open(name, flag, mode);
+}
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -92,6 +92,10 @@
 
   std::string message(int Condition) const override {
 switch (static_cast(Condition)) {
+case index_error_code::success:
+  // There should not be a success error. Jump to unreachable directly.
+  // Add this case to make the compiler stop complaining.
+  break;
 case index_error_code::unspecified:
   return "An unknown error has occurred.";
 case index_error_code::missing_index_file:
@@ -667,12 +671,15 @@
   /// Lazily initialize the invocation list member used for on-demand parsing.
   if (InvocationList)
 return llvm::Error::success();
+  if (index_error_code::success != PreviousParsingResult)
+return llvm::make_error(PreviousParsingResult);
 
   llvm::ErrorOr> FileContent =
   llvm::MemoryBuffer::getFile(InvocationListFilePath);
-  if (!FileContent)
-return llvm::make_error(
-index_error_code::invocation_list_file_not_found);
+  if (!FileContent) {
+PreviousParsingResult = index_error_code::invocation_list_file_not_found;
+return llvm::make_error(PreviousParsingResult);
+  }
   std::unique_ptr ContentBuffer = std::move(*FileContent);
   assert(ContentBuffer && "If no error was produced after loading, the pointer "
   "should not be nullptr.");
@@ -680,8 +687,13 @@
   llvm::Expected ExpectedInvocationList =
   parseInvocationList(ContentBuffer->getBuffer(), PathStyle);
 
-  if (!ExpectedInvocationList)
-return ExpectedInvocationList.takeError();
+  // Handle the error to store the code for next call to this function.
+  if (!ExpectedInvocationList) {
+llvm::handleAllErrors(
+ExpectedInvocationList.takeError(),
+[&](const IndexError &E) { PreviousParsingResult = E.getCode(); });
+return llvm::make_error(PreviousParsingResult);
+  }
 
   InvocationList = *ExpectedInvocationList;
 
Index: clang/include/clang/CrossTU/CrossTranslationUnit.h
===
--- clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -38,6 +38,7 @@
 namespace cross_tu {
 
 enum class index_error_code {
+  success = 0,
   unspecified = 1,
   missing_index_file,
   invalid_index_format,
@@ -253,6 +254,7 @@
 /// In case of on-deman

[PATCH] D98895: [X86][Draft] Disable long double type for -mno-x87 option

2021-05-24 Thread Andrew Savonichev via Phabricator via cfe-commits
asavonic added inline comments.



Comment at: clang/test/Sema/x86-no-x87.c:48-61
+void assign2() {
+  struct st_long_double st;
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  st.ld = 0.42;
+}

pengfei wrote:
> asavonic wrote:
> > pengfei wrote:
> > > These seems pass with GCC. https://godbolt.org/z/qM4nWhThx
> > Right. Assignment of a literal is compiled to just `mov` without any x87 
> > instructions, so it is not diagnosed by GCC. On the other hand, assignment 
> > of a variable does trigger the error:
> > 
> > void assign4(double d) {
> >   struct st_long_double st;
> >   st.ld = d; // error: long double is not supported on this target
> > }
> > 
> > We can update the patch to do the same for some cases, but it does not look 
> > very consistent, and makes assumptions on how the code is optimized and 
> > compiled.
> > 
> > GCC has an advantage here, because it emits the diagnostic at a lower level 
> > after at lease some optimizations are done. For example, the following code 
> > does not trigger an error for GCC because of inlining:
> > 
> > double get_const() {
> >   return 0.42;
> > }
> > void assign5(struct st_long_double *st) {
> >   st->ld = get_const();
> > }
> > 
> I see. Another concern is about the 32 bits. @LiuChen3 had tested in D100091 
> that GCC doesn't error for 32 bits. Do we need to consider it here?
Yes, it should be considered. Both x86 and x86_64 SysV ABI targets require x87 
to be used for long double. Other targets have different requirements for long 
double: double precision for Windows, double and quad precision for Android x86 
and x86_64 respectively.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

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


[PATCH] D103036: [cfe][inline-asm] Support target-specific escaped character in inline asm

2021-05-24 Thread Min-Yih Hsu via Phabricator via cfe-commits
myhsu created this revision.
myhsu added reviewers: nickdesaulniers, nathanchance, m_zuckerman, rnk.
myhsu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC allows each target to define a set of non-letter and non-digit escaped 
characters for inline assembly that will be replaced by another string (They 
call this "punctuation" characters. The existing "%%" and "%{" -- replaced by 
'%' and '{' at the end -- can be seen as special cases shared by all targets).
This patch implements this feature by adding a new hook in `TargetInfo`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103036

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/test/CodeGen/m68k-asm.c

Index: clang/test/CodeGen/m68k-asm.c
===
--- /dev/null
+++ clang/test/CodeGen/m68k-asm.c
@@ -0,0 +1,21 @@
+// REQUIRES: m68k-registered-target
+// RUN: %clang -target m68k -S %s -o - | FileCheck %s
+
+// Test special escaped character in inline assembly
+void escaped() {
+  // '.' -> '.'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move%.l #66, %%d1" ::);
+  // '#' -> '#'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l %#66, %%d1" ::);
+  // '/' -> '%'
+  // CHECK: move.l #66, %d1
+  __asm__ ("move.l #66, %/d1" ::);
+  // '$' -> 's'
+  // CHECK: muls %d0, %d1
+  __asm__ ("mul%$ %%d0, %%d1" ::);
+  // '&' -> 'd'
+  // CHECK: move.l %d0, %d1
+  __asm__ ("move.l %%%&0, %%d1" ::);
+}
Index: clang/lib/Basic/Targets/M68k.h
===
--- clang/lib/Basic/Targets/M68k.h
+++ clang/lib/Basic/Targets/M68k.h
@@ -47,6 +47,7 @@
   std::string convertConstraint(const char *&Constraint) const override;
   bool validateAsmConstraint(const char *&Name,
  TargetInfo::ConstraintInfo &info) const override;
+  llvm::Optional handleAsmEscapedChar(char EscChar) const override;
   const char *getClobbers() const override;
   BuiltinVaListKind getBuiltinVaListKind() const override;
   bool setCPU(const std::string &Name) override;
Index: clang/lib/Basic/Targets/M68k.cpp
===
--- clang/lib/Basic/Targets/M68k.cpp
+++ clang/lib/Basic/Targets/M68k.cpp
@@ -191,6 +191,30 @@
   return false;
 }
 
+llvm::Optional
+M68kTargetInfo::handleAsmEscapedChar(char EscChar) const {
+  char C;
+  switch (EscChar) {
+  case '.':
+  case '#':
+C = EscChar;
+break;
+  case '/':
+C = '%';
+break;
+  case '$':
+C = 's';
+break;
+  case '&':
+C = 'd';
+break;
+  default:
+return llvm::None;
+  }
+
+  return std::string(1, C);
+}
+
 std::string M68kTargetInfo::convertConstraint(const char *&Constraint) const {
   if (*Constraint == 'C')
 // Two-character constraint; add "^" hint for later parsing
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -646,6 +646,8 @@
   continue;
 }
 
+const TargetInfo &TI = C.getTargetInfo();
+
 // Escaped "%" character in asm string.
 if (CurPtr == StrEnd) {
   // % at end of string is invalid (no escape).
@@ -666,6 +668,11 @@
   CurStringPiece += "${:uid}";
   continue;
 }
+// Handle target-specific escaped characters
+if (auto MaybeReplaceStr = TI.handleAsmEscapedChar(EscapedChar)) {
+  CurStringPiece += *MaybeReplaceStr;
+  continue;
+}
 
 // Otherwise, we have an operand.  If we have accumulated a string so far,
 // add it to the Pieces list.
@@ -688,7 +695,6 @@
   EscapedChar = *CurPtr++;
 }
 
-const TargetInfo &TI = C.getTargetInfo();
 const SourceManager &SM = C.getSourceManager();
 const LangOptions &LO = C.getLangOpts();
 
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1091,6 +1091,12 @@
 return std::string(1, *Constraint);
   }
 
+  // Replace some escaped characters with another string based on
+  // target-specific rules
+  virtual llvm::Optional handleAsmEscapedChar(char C) const {
+return llvm::None;
+  }
+
   /// Returns a string of target-specific clobbers, in LLVM format.
   virtual const char *getClobbers() const = 0;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

2021-05-24 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh updated this revision to Diff 347428.
svenvh edited the summary of this revision.
svenvh added a comment.

Fix formatting issues, rebase patch, add test.


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

https://reviews.llvm.org/D97869

Files:
  clang/test/Headers/lit.local.cfg
  clang/test/Headers/opencl-builtins.cl
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -122,6 +122,8 @@
 
 void EmitClangOpenCLBuiltins(llvm::RecordKeeper &Records,
  llvm::raw_ostream &OS);
+void EmitClangOpenCLBuiltinTests(llvm::RecordKeeper &Records,
+ llvm::raw_ostream &OS);
 
 void EmitClangDataCollectors(llvm::RecordKeeper &Records,
  llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -63,6 +63,7 @@
   GenClangCommentCommandInfo,
   GenClangCommentCommandList,
   GenClangOpenCLBuiltins,
+  GenClangOpenCLBuiltinTests,
   GenArmNeon,
   GenArmFP16,
   GenArmBF16,
@@ -194,6 +195,8 @@
"documentation comments"),
 clEnumValN(GenClangOpenCLBuiltins, "gen-clang-opencl-builtins",
"Generate OpenCL builtin declaration handlers"),
+clEnumValN(GenClangOpenCLBuiltinTests, "gen-clang-opencl-builtin-tests",
+   "Generate OpenCL builtin declaration tests"),
 clEnumValN(GenArmNeon, "gen-arm-neon", "Generate arm_neon.h for clang"),
 clEnumValN(GenArmFP16, "gen-arm-fp16", "Generate arm_fp16.h for clang"),
 clEnumValN(GenArmBF16, "gen-arm-bf16", "Generate arm_bf16.h for clang"),
@@ -371,6 +374,9 @@
   case GenClangOpenCLBuiltins:
 EmitClangOpenCLBuiltins(Records, OS);
 break;
+  case GenClangOpenCLBuiltinTests:
+EmitClangOpenCLBuiltinTests(Records, OS);
+break;
   case GenClangSyntaxNodeList:
 EmitClangSyntaxNodeList(Records, OS);
 break;
Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -228,6 +228,64 @@
   // same entry ().
   MapVector SignatureListMap;
 };
+
+// OpenCL builtin test generator.  This class processes the same TableGen input
+// as BuiltinNameEmitter, but generates a .cl file that contains a call to each
+// builtin function described in the .td input.
+class OpenCLBuiltinTestEmitter {
+public:
+  OpenCLBuiltinTestEmitter(RecordKeeper &Records, raw_ostream &OS)
+  : Records(Records), OS(OS) {}
+
+  // Entrypoint to generate the functions for testing all OpenCL builtin
+  // functions.
+  void Emit();
+
+private:
+  struct TypeFlags {
+TypeFlags() : IsConst(false), IsVolatile(false), IsPointer(false) {}
+bool IsConst : 1;
+bool IsVolatile : 1;
+bool IsPointer : 1;
+StringRef AddrSpace;
+  };
+
+  // Return a string representation of the given type, such that it can be
+  // used as a type in OpenCL C code.
+  std::string getTypeString(const Record *Type, TypeFlags Flags,
+int VectorSize) const;
+
+  // Return the type(s) and vector size(s) for the given type.  For
+  // non-GenericTypes, the resulting vectors will contain 1 element.  For
+  // GenericTypes, the resulting vectors typically contain multiple elements.
+  void getTypeLists(Record *Type, TypeFlags &Flags,
+std::vector &TypeList,
+std::vector &VectorList) const;
+
+  // Expand the TableGen Records representing a builtin function signature into
+  // one or more function signatures.  Return them as a vector of a vector of
+  // strings, with each string containing an OpenCL C type and optional
+  // qualifiers.
+  //
+  // The Records may contain GenericTypes, which expand into multiple
+  // signatures.  Repeated occurrences of GenericType in a signature expand to
+  // the same types.  For example [char, FGenType, FGenType] expands to:
+  //   [char, float, float]
+  //   [char, float2, float2]
+  //   [char, float3, float3]
+  //   ...
+  void
+  expandTypesInSignature(const std::vector &Signature,
+ SmallVectorImpl> &Types);
+
+  // Contains OpenCL builtin functions and related information, stored as
+  // Record instances. They are coming from the associated TableGen file.
+  RecordKeeper &Records;
+
+  // The output file.
+  raw_ostream &OS;
+};
+
 } // namespace
 
 void BuiltinNameEmitter::Emit() {
@@ -861,7 +919,229 @@
   OS << "\n} // OCL2Qual\n";
 }
 
+std::string OpenCLBuiltinTestEmitter::g

[PATCH] D101964: Added support for -Wstack-usage flag and Framesize reporting fix

2021-05-24 Thread Ryan Santhirarajan via Phabricator via cfe-commits
rsanthir.quic abandoned this revision.
rsanthir.quic added a comment.

Moving forward with simpler approach here:
https://reviews.llvm.org/D102782


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101964

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


[PATCH] D101965: Added Support for Warning Flag -Wstack-usage=

2021-05-24 Thread Ryan Santhirarajan via Phabricator via cfe-commits
rsanthir.quic abandoned this revision.
rsanthir.quic added a comment.

Moving forward with simpler approach here:
https://reviews.llvm.org/D102782


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101965

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


[PATCH] D97699: [analyzer] Add InvalidPtrChecker

2021-05-24 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze added a comment.
Herald added a subscriber: manas.

@NoQ can you please have another look at this? I think it will be a useful 
checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97699

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


[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

2021-05-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

Could you please reupload your diff to retrigger the bots?
I'd like to make sure that everything passes, even the tests that do not belong 
to you.
Unfortunately, I don't know any better way of retriggering the pre-merge checks 
- I don't have permission to manually retrigger builds.


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

https://reviews.llvm.org/D101763

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


[PATCH] D101775: Fix for Bug 50033 - -fno-temp-file is not respected when creating a pch in clang 12

2021-05-24 Thread Zachary Henkel via Phabricator via cfe-commits
zahen added a comment.

@dexonsmith anything else required before this can land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101775

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


[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-24 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley added inline comments.



Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4570
 mu_.AssertHeld();
-mu_.Unlock();
-  }  // should this be a warning?
+mu_.Unlock(); // should this be a warning?
+  }

This function should have a warning because is not marked with UNLOCK_FUNCTION. 
 The lock was held on entry, and not held on exit.  So the original location of 
the comment, on the closing curly brace, was correct.  



Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4629
+  mu_.AssertHeld(); // expected-note {{the other acquisition of mutex 
'mu_' is here}}
+// FIXME: should instead warn because it's unclear whether we need to 
release or not.
+int b = a;

I'm not wild about having FIXMEs in test code.  I would prefer to have the 
patch actually issue a warning here (but see below).

HOWEVER, does the current code issue a warning in this situation?  If not, 
leave it as-is and keep the FIXME.  You can't make the analysis more strict 
without checking with the C++ compiler team at Google (which I am no longer 
on), because it may break the build on our end.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102026

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


[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-24 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 347419.
feg208 added a comment.

clang-tidy and clang-format changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16367,6 +16367,162 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\" },\n"
+   "{-1, 93463, \"world\" },\n"
+   "{ 7, 5,\"!!\" }\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\" }, // first line\n"
+   "{-1, 93463, \"world\" }, // second line\n"
+   "{ 7, 5,\"!!\" }  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\" }, // first line\n"
+   "{ -1, 93463, 22,   \"my\" }, // second line\n"
+   "{  7, 5,  1, \"goodness\" }  // third line\n"
+   "{234, 5,  1, \"gracious\" }  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\" },\n"
+   "{-1, 93463, \"world\" },\n"
+   "{ 7, 5,\"!!\" }\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\" },\n"
+   "{int{-1}, 93463, \"world\" },\n"
+   "{ int{7}, 5,\"!!\" }\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\" },\n"
+   "{-1, 93463, \"world\" },\n"
+   "{ 7, 5,\"!!\" },\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\" },\n"
+   "{-1, 93463, \"world\" },\n"
+   "{ 7, 5,\"!!\" },\n"
+   "};\n",
+   Style);
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\" },\n"
+   "test{-1, 93463, \"world\" },\n"
+   "test{ 7, 5,\"!!\" },\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\" },\n"
+   "#if X\n"
+   "{-1, 93463, \"world\" },\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\" }\n"
+   "};\n",
+   Style);
+
+  verifyFormat("test demo[] = {\n"
+   "{ 7,23,\n"
+   "\"hello world i am a very long line that really, in any\"\n"
+   "\"just world, ought to be split over multiple lines\" },\n"
+   "{-1, 93463, \"world\" },\n"
+   "{56, 5,\"!!\" }\n"
+   "};\n",
+   Style);
+
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "{\"x\", \"dy\"} },\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"} },\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\" },\n"
+  "{-1, 93463,  "
+  " \"world\" },\n"
+  "{ 7, 5,  "
+  "\"!!\" },\n"
+  "};",
+  format("test demo[] = {{

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

2021-05-24 Thread Fabian Thurnheer via Phabricator via cfe-commits
DNS320 added a comment.

Dear reviewers,

Since this work was conducted as part of a bachelor's thesis, which has to be
handed in on the 18th of June 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Marco & Fabian


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

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


[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-24 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley added a comment.

I have a few concerns.  First, this patch introduces an inconsistency between 
managed and unmanaged locks.  For unmanaged locks, we warn, and //then assume 
the lock is held exclusively//.  For managed locks, we don't warn, but //assume 
it is held shared//.  The warn/not-warn is consistent with more relaxed 
handling of managed locks, but exclusive/shared difference could lead to 
confusion.  Both mechanisms are sound; they're just not consistent.  Any 
thoughts?

Second, the mixing of AssertHeld() and Lock() on different control flow 
branches looks very weird to me.  I can see one obvious case where you might 
want to do that, e.g.  if (mu_.is_locked()) mu_.AssertHeld(); else mu_.Lock().  
However, if a function locks mu_ on one branch, and assert mu_ on the other, 
then that usually indicates a problem.  It means that the lock-state going into 
that function was unknown; the programmer didn't know whether mu_ was locked or 
not, and that's never good.  The //only// valid condition in that case is to 
check whether mu_ is locked -- any other condition would be unsound.  The 
correct way to deal with this situation is to mark the is_locked() method with 
an attribute, e.g. TEST_LOCKED_FUNCTION, rather than relying on the AssertHeld 
mechanism.  That's a lot more work, but I would prefer to at least have a TODO 
comment indicating that the AssertHeld is a workaround, and restrict the mixing 
of assert/lock to managed locks in the mean time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102026

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


[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-24 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment.

In D101868#2776633 , 
@HazardyKnusperkeks wrote:

> In D101868#2775550 , @feg208 wrote:
>
>> This reworks substantially this commit. I recognize there are lacking/broken
>> tests but I just would like to ensure that the general direction doesn't
>> seem likely to end in tears
>
> I'm not deep enough in clang-format and currently have not enough time to 
> check that in depth. But why are you right aligning?

So the basic approach here is to add the column aligning at the point that the 
lines are broken to make sure that we can align to the indented, now broken, 
columns. The right alignment was the easier path (at the moment) since the 
spaces attached to tokens in the change list proceeded the token and since I 
was calculating the column size with spaces based on a pointer attached to the 
token in the same column. To left align at each column I'd need to look at the 
adjacent column to determine the right number of spaces.

> I would love to have a right aligning for numbers (or even better aligning 
> around the decimal dot) in all my code, but strings I wouldn't want to right 
> align.

I think we could certainly do that. I guess at this point (and this is somewhat 
motivated by the fact that I don't, and probably shouldn't, have commit rights 
the to llvm repo) I think if we want to move this commit forward we ought to 
agree on a set of changes and subsequent tests that we can all be comfortable 
with that would make this a viable bit of code. I don't think it has deep 
problems in the sense the prior one did and so should be amenable to laundry 
listing what we need and I'll move it forward. I think this set of tests should 
be added/fixed:

A test where the line is broken in the middle and/or first column (line 
breaking is really the sticky bit)
Fixing the test where the 100 column limit doesn't format correctly
Adding a test with several arrays to format and varying the other alignment 
options to make sure that doesn't confuse anything

I guess a final question I have would be, if we get this list sorted who 
can/would be capable of committing this change to the repo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D49864#2777300 , @janosimas wrote:

> I found the issue with my diff and I was able to get the expected file.
>
> I'm still having issues uploading the file, this is the message I get in the 
> browser console:
>
>   POST https://reviews.llvm.org/differential/revision/update/49864/ 500

Hmm, I'm not certain what's going on there. When I try to upload a patch for 
the review, Phab lets me upload it (but I didn't try to submit the changes). At 
what stage are you getting the failure? Is it when uploading the patch itself, 
or at some other point?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

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


[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

2021-05-24 Thread Giorgis Georgakoudis via Phabricator via cfe-commits
ggeorgakoudis added a comment.

In D102107#2776237 , @jdoerfert wrote:

> This allows us to remove the switch in the device runtime, right?

Yes, with a complication: for combined directives of worksharing loops 
(distributed parallel for) clang emits the lower bound and upper bound as 
distinct parameters besides the global tid, bound tid and the captured 
variables (aggregated in the struct of this patch). We will need to have a flag 
in parallel_51 to unwrap the global args array for this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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


[PATCH] D101630: [HIP] Fix device-only compilation

2021-05-24 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 347400.
yaxunl added a comment.

fixed option. bundle output if users specify output by -o or -E


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

https://reviews.llvm.org/D101630

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/clang-offload-bundler.c
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-output-file-name.hip
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-rdc-device-only.hip
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -117,6 +117,9 @@
 /// The index of the host input in the list of inputs.
 static unsigned HostInputIndex = ~0u;
 
+/// Whether not having host target is allowed.
+static bool AllowNoHost = false;
+
 /// Path to the current binary.
 static std::string BundlerExecutable;
 
@@ -839,9 +842,10 @@
   }
 
   // Get the file handler. We use the host buffer as reference.
-  assert(HostInputIndex != ~0u && "Host input index undefined??");
+  assert((HostInputIndex != ~0u || AllowNoHost) &&
+ "Host input index undefined??");
   Expected> FileHandlerOrErr =
-  CreateFileHandler(*InputBuffers[HostInputIndex]);
+  CreateFileHandler(*InputBuffers[AllowNoHost ? 0 : HostInputIndex]);
   if (!FileHandlerOrErr)
 return FileHandlerOrErr.takeError();
 
@@ -1108,6 +1112,7 @@
   // have exactly one host target.
   unsigned Index = 0u;
   unsigned HostTargetNum = 0u;
+  bool HIPOnly = true;
   llvm::DenseSet ParsedTargets;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
@@ -1149,12 +1154,21 @@
   HostInputIndex = Index;
 }
 
+if (Kind != "hip" && Kind != "hipv4")
+  HIPOnly = false;
+
 ++Index;
   }
 
+  // HIP uses clang-offload-bundler to bundle device-only compilation results
+  // for multiple GPU archs, therefore allow no host target if all entries
+  // are for HIP.
+  AllowNoHost = HIPOnly;
+
   // Host triple is not really needed for unbundling operation, so do not
   // treat missing host triple as error if we do unbundling.
-  if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) {
+  if ((Unbundle && HostTargetNum > 1) ||
+  (!Unbundle && HostTargetNum != 1 && !AllowNoHost)) {
 reportError(createStringError(errc::invalid_argument,
   "expecting exactly one host target but got " +
   Twine(HostTargetNum)));
Index: clang/test/Driver/hip-rdc-device-only.hip
===
--- clang/test/Driver/hip-rdc-device-only.hip
+++ clang/test/Driver/hip-rdc-device-only.hip
@@ -6,7 +6,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -c -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip -fhip-bundle-device-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITBC %s
 
 // With `-emit-llvm`, the output should be the same as the aforementioned line
@@ -16,14 +16,14 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -c -emit-llvm -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip -fhip-bundle-device-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITBC %s
 
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip -fhip-bundle-device-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITLL %s
 
 // With `-emit-llvm`, the output should be the same as the aforementioned line
@@ -33,7 +33,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -emit-llvm -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip -fhip-bundle-device-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITLL %s
 
 // With `-save-temps`, commane lines for each steps are dumped. For assembly
@@ -44,9 +44,17 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_mul

[PATCH] D101630: [HIP] Fix device-only compilation

2021-05-24 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added a comment.

In D101630#2748513 , @tra wrote:

> How about this:
> If the user explicitly specified `--cuda-host-only` or `--cuda-device-only`, 
> then by default only allow producing the natural output format, unless a 
> bundled output is requested by an option. This should keep existing users 
> working.
> If the compilation is done without explicitly requested sub-compilation(s), 
> then bundle the output by default. This should keep the GPU-unaware tools 
> like ccache happy as they would always get the single output they expect.
>
> WDYT?

`--cuda-host-only` always have one output, therefore there is no point of 
bundle its output. We only need to decide the proper behavior of 
`--cuda-device-only`.

How about keeping the original default behavior of not bundling if users do not 
specify output file, whereas bundle the output if users specifying output file. 
Since specifying output file indicates users requesting one output. 
-f[no-]hip-bundle-device-output override the default behavior.


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

https://reviews.llvm.org/D101630

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


[PATCH] D102241: [clang] p1099 4/5: using enum EnumTag

2021-05-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan updated this revision to Diff 347406.

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

https://reviews.llvm.org/D102241

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Index/IndexSymbol.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Index/IndexSymbol.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/ast-dump-using-enum.cpp
  clang/test/SemaCXX/cxx20-using-enum.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1428,6 +1428,22 @@
   usingDecl(hasAnyUsingShadowDecl(hasName("a");
 }
 
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesUsingEnumDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("namespace X { enum x {}; } using enum X::x;", usingEnumDecl()));
+}
+
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesShadowUsingDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("namespace f { enum a {b}; } using enum f::a;",
+  usingEnumDecl(hasAnyUsingShadowDecl(hasName("b");
+}
+
 TEST_P(ASTMatchersTest, UsingDirectiveDecl_MatchesUsingNamespace) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -844,7 +844,15 @@
   testImport("namespace foo { int bar; }"
  "void declToImport() { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- functionDecl(hasDescendant(usingDecl(;
+ functionDecl(hasDescendant(usingDecl(hasName("bar");
+}
+
+TEST_P(ImportDecl, ImportUsingEnumDecl) {
+  MatchVerifier Verifier;
+  testImport("namespace foo { enum bar { baz, toto, quux }; }"
+ "void declToImport() { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ functionDecl(hasDescendant(usingEnumDecl(hasName("bar");
 }
 
 /// \brief Matches shadow declarations introduced into a scope by a
@@ -862,10 +870,16 @@
 
 TEST_P(ImportDecl, ImportUsingShadowDecl) {
   MatchVerifier Verifier;
+  // from using-decl
   testImport("namespace foo { int bar; }"
  "namespace declToImport { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- namespaceDecl(has(usingShadowDecl(;
+ namespaceDecl(has(usingShadowDecl(hasName("bar");
+  // from using-enum-decl
+  testImport("namespace foo { enum bar {baz, toto, quux }; }"
+ "namespace declToImport { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ namespaceDecl(has(usingShadowDecl(hasName("baz");
 }
 
 TEST_P(ImportExpr, ImportUnresolvedLookupExpr) {
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6540,6 +6540,7 @@
   }
 
   case Decl::Using:
+  case Decl::UsingEnum:
 return MakeCursorOverloadedDeclRef(cast(D), D->getLocation(),
TU);
 
Index: clang/test/SemaCXX/cxx20-using-enum.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-using-enum.cpp
@@ -0,0 +1,233 @@
+// RUN: %clang_cc1 -fsyntax-only

[PATCH] D102241: [clang] p1099 4/5: using enum EnumTag

2021-05-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan updated this revision to Diff 347405.
urnathan added a comment.

Address davrec's comments


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

https://reviews.llvm.org/D102241

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Index/IndexSymbol.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Index/IndexSymbol.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/ast-dump-using-enum.cpp
  clang/test/SemaCXX/cxx20-using-enum.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1428,6 +1428,22 @@
   usingDecl(hasAnyUsingShadowDecl(hasName("a");
 }
 
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesUsingEnumDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("namespace X { enum x {}; } using enum X::x;", usingEnumDecl()));
+}
+
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesShadowUsingDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("namespace f { enum a {b}; } using enum f::a;",
+  usingEnumDecl(hasAnyUsingShadowDecl(hasName("b");
+}
+
 TEST_P(ASTMatchersTest, UsingDirectiveDecl_MatchesUsingNamespace) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -844,7 +844,15 @@
   testImport("namespace foo { int bar; }"
  "void declToImport() { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- functionDecl(hasDescendant(usingDecl(;
+ functionDecl(hasDescendant(usingDecl(hasName("bar");
+}
+
+TEST_P(ImportDecl, ImportUsingEnumDecl) {
+  MatchVerifier Verifier;
+  testImport("namespace foo { enum bar { baz, toto, quux }; }"
+ "void declToImport() { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ functionDecl(hasDescendant(usingEnumDecl(hasName("bar");
 }
 
 /// \brief Matches shadow declarations introduced into a scope by a
@@ -862,10 +870,16 @@
 
 TEST_P(ImportDecl, ImportUsingShadowDecl) {
   MatchVerifier Verifier;
+  // from using-decl
   testImport("namespace foo { int bar; }"
  "namespace declToImport { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- namespaceDecl(has(usingShadowDecl(;
+ namespaceDecl(has(usingShadowDecl(hasName("bar");
+  // from using-enum-decl
+  testImport("namespace foo { enum bar {baz, toto, quux }; }"
+ "namespace declToImport { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ namespaceDecl(has(usingShadowDecl(hasName("baz");
 }
 
 TEST_P(ImportExpr, ImportUnresolvedLookupExpr) {
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6540,6 +6540,7 @@
   }
 
   case Decl::Using:
+  case Decl::UsingEnum:
 return MakeCursorOverloadedDeclRef(cast(D), D->getLocation(),
TU);
 
Index: clang/test/SemaCXX/cxx20-using-enum.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-using-enum.c

[PATCH] D100276: [clang] p1099 3/5: using Enum::member

2021-05-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan updated this revision to Diff 347403.
urnathan added a comment.

rebased


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

https://reviews.llvm.org/D100276

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
  clang/test/SemaCXX/enum-scoped.cpp

Index: clang/test/SemaCXX/enum-scoped.cpp
===
--- clang/test/SemaCXX/enum-scoped.cpp
+++ clang/test/SemaCXX/enum-scoped.cpp
@@ -301,8 +301,8 @@
   int E::*p; // expected-error {{does not point into a class}}
   using E::f; // expected-error {{no member named 'f'}}
 
-  using E::a; // expected-error {{using declaration cannot refer to a scoped enumerator}}
-  E b = a; // expected-error {{undeclared}}
+  using E::a; // expected-warning {{using declaration naming a scoped enumerator is a C++20 extension}}
+  E b = a;
 }
 
 namespace test11 {
Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
===
--- clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7.cpp
@@ -1,4 +1,11 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s
 
 enum class EC { ec };
-using EC::ec; // expected-error {{using declaration cannot refer to a scoped enumerator}}
+using EC::ec;
+#if __cplusplus < 202002
+// expected-warning@-2 {{using declaration naming a scoped enumerator is a C++20 extension}}
+#else
+// expected-no-diagnostics
+#endif
Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+
+// p1099 'using SCOPEDENUM::MEMBER;'
+
+namespace Zero {
+namespace Bob {
+enum class Kevin {
+  Stuart,
+  AlsoStuart
+#if __cplusplus >= 202002L
+// expected-note@-3{{target of using declaration}}
+// expected-note@-3{{target of using declaration}}
+#endif
+};
+} // namespace Bob
+
+using Bob::Kevin::Stuart;
+#if __cplusplus < 202002L
+// expected-warning@-2{{using declaration naming a scoped enumerator is a C++20 extension}}
+#else
+using Bob::Kevin::Stuart;
+
+auto b = Stuart;
+
+namespace Foo {
+int Stuart;   // expected-note{{conflicting declaration}}
+using Bob::Kevin::Stuart; // expected-error{{target of using declaration conflicts}}
+
+using Bob::Kevin::AlsoStuart; // expected-note{{using declaration}}
+int AlsoStuart;   // expected-error{{declaration conflicts with target}}
+} // namespace Foo
+#endif
+
+} // namespace Zero
+
+namespace One {
+
+// derived from [namespace.udecl]/3
+enum class button { up,
+down };
+struct S {
+  using button::up;
+#if __cplusplus < 202002L
+  // expected-warning@-2{{a C++20 extension}}
+  // expected-error@-3{{using declaration in class}}
+#else
+  button b = up;
+#endif
+};
+
+#if __cplusplus >= 202002L
+// some more
+struct T : S {
+  button c = up;
+};
+#endif
+enum E2 { e2 };
+} // namespace One
+
+namespace Two {
+enum class E1 { e1 };
+
+struct S {
+  using One::e2;
+#if __cplusplus < 202002L
+  // expected-error@-2{{using declaration in class}}
+#else
+  One::E2 c = e2;
+#endif
+};
+
+} // namespace Two
+
+namespace Three {
+
+enum E3 { e3 };
+struct e3;
+
+struct S {
+  using Three::e3; // expected-error{{using declaration in class}}
+
+  enum class E4 { e4 };
+  enum E5 { e5 };
+};
+
+using S::e5;
+using S::E4::e4;
+#if __cplusplus < 202002L
+// expected-error@-3{{using declaration cannot refer to class member}}
+// expected-note@-4{{use a constexpr variable instead}}
+// expected-warning@-4{{a C++20 extension}}
+// expected-error@-5{{using declaration cannot refer to class member}}
+// expected-note@-6{{use a constexpr variable instead}}
+#else
+auto a = e4;
+auto b = e5;
+#endif
+} // namespace Three
+
+namespace Four {
+
+template 
+struct TPL {
+  enum class E1 { e1 };
+  struct IN {
+enum class E2 { e2 };
+  };
+
+protected:
+  enum class E3 { e3 }; // expected-note{{declared protected here}}
+};
+
+using TPL::E1::e1;
+#if __cplusplus < 202002L
+// expected-warning@-2{{a C++20 extension}}
+// expected-error@-3{{using declaration cannot refer to class member}}
+// expected-note@-4{{use a constexpr variable instead}}
+#else
+using TPL::IN::E2::e2;
+
+auto a = e1;
+auto b = e2;
+#endif
+
+enum class E4 { e4 };
+template 
+struct DER : TPL {
+  using TPL::E1::e1;
+#if __cplusplu

[PATCH] D101777: [clang] p1099 1/5: [NFC] Break out BaseUsingDecl from UsingDecl

2021-05-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan marked 2 inline comments as done.
urnathan added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:3390
   ConstructorUsingShadowDecl(ASTContext &C, DeclContext *DC, SourceLocation 
Loc,
- UsingDecl *Using, NamedDecl *Target,
+ BaseUsingDecl *Using, NamedDecl *Target,
  bool TargetInVirtualBase)

davrec wrote:
> `BaseUsingDecl` here should remain `UsingDecl`.
> Additionally, I would add a `UsingDecl *getIntroducer() { return 
> cast(UsingShadowDecl::getIntroducer()); }` method here.
Done, this required a further reordering of class definitions, so that 
UsingDecl is complete at this point.


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

https://reviews.llvm.org/D101777

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


[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

2021-05-24 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

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


[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

2021-05-24 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann marked an inline comment as done.
mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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


[PATCH] D102906: [clang-tidy] Remark was added to clang tooling Diagnostic

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

In D102906#2777296 , @ivanmurashko 
wrote:

> Hi @aaron.ballman
>
>> Thanks! Do you need someone to commit on your behalf?
>
> I will appreciate your assistance in the diff landing.

Thank you for the patch! I've landed on your behalf in 
7f2f0247f855b143c12cd33d1f53d14bc63f3e82 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102906

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


[PATCH] D101777: [clang] p1099 1/5: [NFC] Break out BaseUsingDecl from UsingDecl

2021-05-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan updated this revision to Diff 347401.
urnathan added a comment.

Addressing Davrec's comments


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

https://reviews.llvm.org/D101777

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Sema/SemaAccess.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6540,7 +6540,7 @@
   }
 
   case Decl::Using:
-return MakeCursorOverloadedDeclRef(cast(D), D->getLocation(),
+return MakeCursorOverloadedDeclRef(cast(D), D->getLocation(),
TU);
 
   case Decl::UsingShadow:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1870,7 +1870,7 @@
   Diag(KWLoc, diag::err_using_decl_conflict_reverse);
   Diag(Shadow->getTargetDecl()->getLocation(),
diag::note_using_decl_target);
-  Diag(Shadow->getUsingDecl()->getLocation(), diag::note_using_decl) << 0;
+  Diag(Shadow->getIntroducer()->getLocation(), diag::note_using_decl) << 0;
   // Recover by ignoring the old declaration.
   PrevDecl = PrevClassTemplate = nullptr;
 }
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3732,7 +3732,7 @@
   // A shadow declaration that's created by a resolved using declaration
   // is not hidden by the same using declaration.
   if (isa(ND) && isa(D) &&
-  cast(ND)->getUsingDecl() == D)
+  cast(ND)->getIntroducer() == D)
 continue;
 
   // We've found a declaration that hides this one.
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7000,7 +7000,7 @@
   : S(S), UseLoc(UseLoc) {
 bool DiagnosedMultipleConstructedBases = false;
 CXXRecordDecl *ConstructedBase = nullptr;
-UsingDecl *ConstructedBaseUsing = nullptr;
+BaseUsingDecl *ConstructedBaseIntroducer = nullptr;
 
 // Find the set of such base class subobjects and check that there's a
 // unique constructed subobject.
@@ -7024,18 +7024,18 @@
   //   of type B, the program is ill-formed.
   if (!ConstructedBase) {
 ConstructedBase = DConstructedBase;
-ConstructedBaseUsing = D->getUsingDecl();
+ConstructedBaseIntroducer = D->getIntroducer();
   } else if (ConstructedBase != DConstructedBase &&
  !Shadow->isInvalidDecl()) {
 if (!DiagnosedMultipleConstructedBases) {
   S.Diag(UseLoc, diag::err_ambiguous_inherited_constructor)
   << Shadow->getTargetDecl();
-  S.Diag(ConstructedBaseUsing->getLocation(),
-   diag::note_ambiguous_inherited_constructor_using)
+  S.Diag(ConstructedBaseIntroducer->getLocation(),
+ diag::note_ambiguous_inherited_constructor_using)
   << ConstructedBase;
   DiagnosedMultipleConstructedBases = true;
 }
-S.Diag(D->getUsingDecl()->getLocation(),
+S.Diag(D->getIntroducer()->getLocation(),
diag::note_ambiguous_inherited_constructor_using)
 << DConstructedBase;
   }
@@ -11633,7 +11633,7 @@
 
 /// Determines whether to create a using shadow decl for a particular
 /// decl, given the set of decls existing prior to this using lookup.
-bool Sema::CheckUsingShadowDecl(UsingDecl *Using, NamedDecl *Orig,
+bool Sema::CheckUsingShadowDecl(BaseUsingDecl *BUD, NamedDecl *Orig,
 const LookupResult &Previous,
 UsingShadowDecl *&PrevShadow) {
   // Diagnose finding a decl which is not from a base class of the
@@ -11655,35 +11655,36 @@
   // specialization.  The UsingShadowDecl in D then points directly
   // to A::foo, which will look well-formed when we instantiate.
   // The right solution is to not collapse the shadow-decl chain.
-  if (!getLangOpts().CPlusPlus11 && CurContext->isRecord()) {
-DeclContext *OrigDC = Orig->getDeclContext();
-
-// Handle enums and anonymous structs.
-if (isa(OrigDC)) OrigDC = OrigDC->getParent();
-CXXRecordDecl *OrigRec = cast(OrigDC);
-while (OrigRec->isAnonymousStructOrUnion())
-  OrigRec = cast(OrigRec->getDeclContext());
-
-i

[clang-tools-extra] 7f2f024 - Remark was added to clang tooling Diagnostic

2021-05-24 Thread Aaron Ballman via cfe-commits

Author: Ivan Murashko
Date: 2021-05-24T11:21:44-04:00
New Revision: 7f2f0247f855b143c12cd33d1f53d14bc63f3e82

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

LOG: Remark was added to clang tooling Diagnostic

The diff adds Remark to Diagnostic::Level for clang tooling. That makes
Remark diagnostic level ready to use in clang-tidy checks: the
clang-diagnostic-module-import becomes visible as a part of the change.

Added: 
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/A.h

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/module.modulemap
clang-tools-extra/test/clang-tidy/infrastructure/remarks.cpp

Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang/include/clang/Tooling/Core/Diagnostic.h
clang/include/clang/Tooling/DiagnosticsYaml.h
clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 08d5c0d6704ba..1457f145f552c 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -446,6 +446,9 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
   case DiagnosticsEngine::Warning:
 CheckName = "clang-diagnostic-warning";
 break;
+  case DiagnosticsEngine::Remark:
+CheckName = "clang-diagnostic-remark";
+break;
   default:
 CheckName = "clang-diagnostic-unknown";
 break;
@@ -460,7 +463,10 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
   Level = ClangTidyError::Error;
   LastErrorRelatesToUserCode = true;
   LastErrorPassesLineFilter = true;
+} else if (DiagLevel == DiagnosticsEngine::Remark) {
+  Level = ClangTidyError::Remark;
 }
+
 bool IsWarningAsError = DiagLevel == DiagnosticsEngine::Warning &&
 Context.treatAsError(CheckName);
 Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(),

diff  --git 
a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/A.h 
b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/A.h
new file mode 100644
index 0..b7c2ac02d8d2f
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/A.h
@@ -0,0 +1 @@
+// A

diff  --git 
a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/module.modulemap
 
b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/module.modulemap
new file mode 100644
index 0..37a855090ec8a
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/remarks/module.modulemap
@@ -0,0 +1 @@
+module A { header "A.h" }

diff  --git a/clang-tools-extra/test/clang-tidy/infrastructure/remarks.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/remarks.cpp
new file mode 100644
index 0..0e9fb46e3e1cb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/remarks.cpp
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: cp -r %S/Inputs/remarks %t
+// RUN: cp %s %t/t.cpp
+
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-module-import' t.cpp -- \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -fsyntax-only \
+// RUN: -I%S/Inputs/remarks \
+// RUN: -working-directory=%t \
+// RUN: -Rmodule-build -Rmodule-import t.cpp 2>&1 |\
+// RUN: FileCheck %s -implicit-check-not "remark:"
+
+#include "A.h"
+// CHECK: remark: importing module 'A' from {{.*}} 
[clang-diagnostic-module-import]
+

diff  --git a/clang/include/clang/Tooling/Core/Diagnostic.h 
b/clang/include/clang/Tooling/Core/Diagnostic.h
index f68e0a682cd21..4553380bcf00e 100644
--- a/clang/include/clang/Tooling/Core/Diagnostic.h
+++ b/clang/include/clang/Tooling/Core/Diagnostic.h
@@ -67,6 +67,7 @@ struct DiagnosticMessage {
 /// fixes to be applied.
 struct Diagnostic {
   enum Level {
+Remark = DiagnosticsEngine::Remark,
 Warning = DiagnosticsEngine::Warning,
 Error = DiagnosticsEngine::Error
   };

diff  --git a/clang/include/clang/Tooling/DiagnosticsYaml.h 
b/clang/include/clang/Tooling/DiagnosticsYaml.h
index 31bc2989b883e..3f257d84f8136 100644
--- a/clang/include/clang/Tooling/DiagnosticsYaml.h
+++ b/clang/include/clang/Tooling/DiagnosticsYaml.h
@@ -106,6 +106,7 @@ template <> struct 
ScalarEnumerationTraits {
   static void enumeration(IO &IO, clang::tooling::Diagnostic::Level &Value) {
 IO.enumCase(Value, "Warning", clang::tooling::Diagnostic::Warning);
 IO.enumCase(Value, "Error", clang::tooling::Diagnostic::Error);
+IO.enumCase(Value, "Remark", clang::tooling::Diagnostic::Remark);
   }
 };
 

diff 

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

2021-05-24 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100972

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


[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

2021-05-24 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Jano Simas via Phabricator via cfe-commits
janosimas added a comment.

I found the issue with my diff and I was able to get the expected file.

I'm still having issues uploading the file, this is the message I get in the 
browser console:

  POST https://reviews.llvm.org/differential/revision/update/49864/ 500


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

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


[PATCH] D102906: [clang-tidy] Remark was added to clang tooling Diagnostic

2021-05-24 Thread Ivan Murashko via Phabricator via cfe-commits
ivanmurashko marked an inline comment as done.
ivanmurashko added a comment.

Hi @aaron.ballman

> Thanks! Do you need someone to commit on your behalf?

I will appreciate your assistance in the landing the diff.

> If so, what name and email address would you like used for patch attribution?

It would be good to use mine - Ivan Murashko (ivan.murashko at gmail.com) for 
that




Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:72
+Error = DiagnosticsEngine::Error,
+Remark = DiagnosticsEngine::Remark
   };

aaron.ballman wrote:
> DmitryPolukhin wrote:
> > nit, I would move remark first to have list by increasing severity. It 
> > looks like this enum values is not used outside of the sources so I hope it 
> > is safe.
> We have diagnostic notes as well and it's not clear to me how to order remark 
> and note should we go that way in the future. So I'm not opposed to the 
> suggestion, but the order likely doesn't matter all that much.
@DmitryPolukhin fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102906

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


[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D49864#2776570 , @janosimas wrote:

> Hi, sorry for taking so long for such a small change.
> I did the changes and generated a diff with the requested context.
> It's a huge file with a lot more diff than my changes, is that right?

That sounds incorrect. The generated diff should be basically the same size as 
what's already in the review.

> When I try to upload it, I'm getting `Unhandled Exception ("Exception")`, no 
> other error message.
> Any suggestions there?

That sounds like a Phabricator issue, but perhaps it has to do with an 
unexpectedly large patch file. I'd recommend first getting the patch file to 
the point it looks reasonable, and then if the Phab issues persist, we can 
investigate them at that point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

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


[PATCH] D102906: [clang-tidy] Remark was added to clang tooling Diagnostic

2021-05-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thanks! Do you need someone to commit on your behalf? If so, what name and 
email address would you like used for patch attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102906

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


[PATCH] D103025: [analyzer] Handle NTTP invocation in CallContext.getCalleeDecl()

2021-05-24 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource added a comment.

The alternative solution, that I think may be preferable is to remove following 
functions from CheckerContext and use CallEvent.getDecl() instead:

  const FunctionDecl *getCalleeDecl(const CallExpr *CE) const;
  StringRef getCalleeName(const FunctionDecl *FunDecl) const;
  const IdentifierInfo *getCalleeIdentifier(const CallExpr *CE) const;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103025

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


[PATCH] D103025: [analyzer] Handle NTTP invocation in CallContext.getCalleeDecl()

2021-05-24 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource created this revision.
tomasz-kaminski-sonarsource added reviewers: NoQ, vsavchenko, steakhal.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
tomasz-kaminski-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes a crash in MallocChecker for the situation when operator new 
(delete) is invoked via NTTP  and makes the behavior of 
CallContext.getCalleeDecl(Expr) identical to CallEvent.getDecl().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103025

Files:
  clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
  clang/test/Analysis/NewDelete-checker-test.cpp


Index: clang/test/Analysis/NewDelete-checker-test.cpp
===
--- clang/test/Analysis/NewDelete-checker-test.cpp
+++ clang/test/Analysis/NewDelete-checker-test.cpp
@@ -421,3 +421,36 @@
   Derived *p = (Derived *)allocate();
   delete p;
 }
+
+template
+void* allocate_via_nttp(size_t n) {
+  return allocate_fn(n);
+}
+
+template
+void deallocate_via_nttp(void* ptr) {
+  deallocate_fn(ptr);
+}
+
+void testNTTPNewNTTPDelete() {
+  void* p = allocate_via_nttp<::operator new>(10);
+  deallocate_via_nttp<::operator delete>(p);
+} // no warn
+
+void testNTTPNewDirectDelete() {
+  void* p = allocate_via_nttp<::operator new>(10);
+  ::operator delete(p);
+} // no warn
+
+void testDirectNewNTTPDelete() {
+  void* p = ::operator new(10);
+  deallocate_via_nttp<::operator delete>(p);
+}
+
+void not_free(void*) {
+}
+
+void testLeakBecauseNTTPIsNotDeallocation() {
+  void* p = ::operator new(10);
+  deallocate_via_nttp(p);
+}  // leak-warning{{Potential leak of memory pointed to by 'p'}}
Index: clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -19,6 +19,10 @@
 using namespace ento;
 
 const FunctionDecl *CheckerContext::getCalleeDecl(const CallExpr *CE) const {
+  const FunctionDecl *D = CE->getDirectCallee();
+  if (D)
+return D;
+
   const Expr *Callee = CE->getCallee();
   SVal L = Pred->getSVal(Callee);
   return L.getAsFunctionDecl();


Index: clang/test/Analysis/NewDelete-checker-test.cpp
===
--- clang/test/Analysis/NewDelete-checker-test.cpp
+++ clang/test/Analysis/NewDelete-checker-test.cpp
@@ -421,3 +421,36 @@
   Derived *p = (Derived *)allocate();
   delete p;
 }
+
+template
+void* allocate_via_nttp(size_t n) {
+  return allocate_fn(n);
+}
+
+template
+void deallocate_via_nttp(void* ptr) {
+  deallocate_fn(ptr);
+}
+
+void testNTTPNewNTTPDelete() {
+  void* p = allocate_via_nttp<::operator new>(10);
+  deallocate_via_nttp<::operator delete>(p);
+} // no warn
+
+void testNTTPNewDirectDelete() {
+  void* p = allocate_via_nttp<::operator new>(10);
+  ::operator delete(p);
+} // no warn
+
+void testDirectNewNTTPDelete() {
+  void* p = ::operator new(10);
+  deallocate_via_nttp<::operator delete>(p);
+}
+
+void not_free(void*) {
+}
+
+void testLeakBecauseNTTPIsNotDeallocation() {
+  void* p = ::operator new(10);
+  deallocate_via_nttp(p);
+}  // leak-warning{{Potential leak of memory pointed to by 'p'}}
Index: clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -19,6 +19,10 @@
 using namespace ento;
 
 const FunctionDecl *CheckerContext::getCalleeDecl(const CallExpr *CE) const {
+  const FunctionDecl *D = CE->getDirectCallee();
+  if (D)
+return D;
+
   const Expr *Callee = CE->getCallee();
   SVal L = Pred->getSVal(Callee);
   return L.getAsFunctionDecl();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

2021-05-24 Thread Felix Berger via Phabricator via cfe-commits
flx created this revision.
flx added reviewers: hokein, ymandel, aaron.ballman.
Herald added a subscriber: xazax.hun.
flx requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This fixes false positive cases where a reference is initialized outside of a
block statement and then its initializing variable is modified. Another case is
when the looped over container is modified.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103021

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

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -1,9 +1,19 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
+template 
+struct Iterator {
+  void operator++();
+  const T &operator*() const;
+  bool operator!=(const Iterator &) const;
+  typedef const T &const_reference;
+};
+
 struct ExpensiveToCopyType {
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType &reference() const;
+  Iterator begin() const;
+  Iterator end() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -508,3 +518,41 @@
   // CHECK-FIXES: const auto& UnnecessaryCopy = Ref.reference();
   Orig.constMethod();
 }
+
+void negativeloopedOverObjectIsModified() {
+  ExpensiveToCopyType Orig;
+  for (const auto &Element : Orig) {
+const auto Copy = Element;
+Orig.nonConstMethod();
+Copy.constMethod();
+  }
+
+  auto Labmda = []() {
+ExpensiveToCopyType Orig;
+for (const auto &Element : Orig) {
+  const auto Copy = Element;
+  Orig.nonConstMethod();
+  Copy.constMethod();
+}
+  };
+}
+
+void negativeReferenceIsInitializedOutsideOfBlock() {
+  ExpensiveToCopyType Orig;
+  const auto &E2 = Orig;
+  if (1 != 2 * 3) {
+const auto C2 = E2;
+Orig.nonConstMethod();
+C2.constMethod();
+  }
+
+  auto Lambda = []() {
+ExpensiveToCopyType Orig;
+const auto &E2 = Orig;
+if (1 != 2 * 3) {
+  const auto C2 = E2;
+  Orig.nonConstMethod();
+  C2.constMethod();
+}
+  };
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -35,11 +35,12 @@
 
 private:
   void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
-  bool IssueFix, const VarDecl *ObjectArg,
+  const Stmt &Body, bool IssueFix,
+  const VarDecl *ObjectArg,
   ASTContext &Context);
   void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
-  const Stmt &BlockStmt, bool IssueFix,
-  ASTContext &Context);
+  const Stmt &BlockStmt, const Stmt &Body,
+  bool IssueFix, ASTContext &Context);
   const std::vector AllowedTypes;
 };
 
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -82,6 +82,7 @@
 // object arg or variable that is referenced is immutable as well.
 static bool isInitializingVariableImmutable(const VarDecl &InitializingVar,
 const Stmt &BlockStmt,
+const Stmt &Body,
 ASTContext &Context) {
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
@@ -92,12 +93,14 @@
   if (!isa(T))
 return true;
 
+  // Search the whole function body for decl statements of the initialization
+  // variable not just the current block statement.
   auto Matches =
   match(findAll(declStmt(has(varDecl(equalsNode(&InitializingVar
 .bind("declStmt")),
-BlockStmt, Context);
-  // The reference or pointer is not initialized in the BlockStmt. We assume
-  // its pointee is not modified then.
+Body, Context);
+  // The reference or pointer is not initialized anywhere witin the function. We
+  // assume its p

[PATCH] D103018: [clang-tidy] performance-unnecessary-copy-initialization: Look at the canonical type when checking for aliases.

2021-05-24 Thread Felix Berger via Phabricator via cfe-commits
flx created this revision.
flx added reviewers: hokein, ymandel, aaron.ballman.
Herald added subscribers: jeroen.dobbelaere, xazax.hun.
flx requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This fixes a false positive case where for instance a pointer is obtained and 
declared using `auto`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103018

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType &reference() const;
+  const ExpensiveToCopyType *pointer() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -500,6 +501,25 @@
   Orig.nonConstMethod();
 }
 
+void negativeAliasNonCanonicalPointerType() {
+  ExpensiveToCopyType Orig;
+  // The use of auto here hides that the type is a pointer type. The check 
needs
+  // to look at the canonical type to detect the aliasing through this pointer.
+  const auto Pointer = Orig.pointer();
+  const auto NecessaryCopy = Pointer->reference();
+  Orig.nonConstMethod();
+}
+
+void negativeAliasTypedefedType() {
+  typedef const ExpensiveToCopyType &ReferenceType;
+  ExpensiveToCopyType Orig;
+  // The typedef hides the fact that this is a reference type. The check needs
+  // to look at the canonical type to detect the aliasing.
+  ReferenceType Ref = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
+
 void positiveCopiedFromGetterOfReferenceToConstVar() {
   ExpensiveToCopyType Orig;
   const auto &Ref = Orig.reference();
Index: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -86,7 +86,7 @@
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
 
-  QualType T = InitializingVar.getType();
+  QualType T = InitializingVar.getType().getCanonicalType();
   // The variable is a value type and we know it is only used as const. Safe
   // to reference it and avoid the copy.
   if (!isa(T))


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType &reference() const;
+  const ExpensiveToCopyType *pointer() const;
   void nonConstMethod();
   bool constMethod() const;
 };
@@ -500,6 +501,25 @@
   Orig.nonConstMethod();
 }
 
+void negativeAliasNonCanonicalPointerType() {
+  ExpensiveToCopyType Orig;
+  // The use of auto here hides that the type is a pointer type. The check needs
+  // to look at the canonical type to detect the aliasing through this pointer.
+  const auto Pointer = Orig.pointer();
+  const auto NecessaryCopy = Pointer->reference();
+  Orig.nonConstMethod();
+}
+
+void negativeAliasTypedefedType() {
+  typedef const ExpensiveToCopyType &ReferenceType;
+  ExpensiveToCopyType Orig;
+  // The typedef hides the fact that this is a reference type. The check needs
+  // to look at the canonical type to detect the aliasing.
+  ReferenceType Ref = Orig.reference();
+  const auto NecessaryCopy = Ref.reference();
+  Orig.nonConstMethod();
+}
+
 void positiveCopiedFromGetterOfReferenceToConstVar() {
   ExpensiveToCopyType Orig;
   const auto &Ref = Orig.reference();
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -86,7 +86,7 @@
   if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
 return false;
 
-  QualType T = InitializingVar.getType();
+  QualType T = InitializingVar.getType().getCanonicalType();
   // The variable is a value type and we know it is only used as const. Safe
   // to reference it and avoid the copy.
   if (!isa(T))
___
cfe-commits mail

[clang] 5ccc79d - [OpenCL][Docs] Minor update to OpenCL 3.0

2021-05-24 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2021-05-24T14:19:22+01:00
New Revision: 5ccc79dc38b2df18cca5a9b4d66dcd4603f948e9

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

LOG: [OpenCL][Docs] Minor update to OpenCL 3.0

Added: 


Modified: 
clang/docs/OpenCLSupport.rst

Removed: 




diff  --git a/clang/docs/OpenCLSupport.rst b/clang/docs/OpenCLSupport.rst
index dbfb67f90ae1..2a6b9a0b7346 100644
--- a/clang/docs/OpenCLSupport.rst
+++ b/clang/docs/OpenCLSupport.rst
@@ -340,7 +340,7 @@ Missing features or with limited support
 .. _opencl_300:
 
 OpenCL C 3.0 Usage
-
+==
 
 OpenCL C 3.0 language standard makes most OpenCL C 2.0 features optional. 
Optional
 functionality in OpenCL C 3.0 is indicated with the presence of feature-test 
macros
@@ -357,7 +357,7 @@ user should specify both (extension and feature) in 
command-line flag:
 
 
 OpenCL C 3.0 Implementation Status
-
+--
 
 The following table provides an overview of features in OpenCL C 3.0 and their
 implementation status.
@@ -371,7 +371,7 @@ implementation status.
 
+--+--+--+---+
 | Predefined macros| Feature macros
   | :good:`done` | https://reviews.llvm.org/D95776 
  |
 
+--+--+--+---+
-| Feature optionality  | Generic address space 
   | :none:`unclaimed`| 
  |
+| Feature optionality  | Generic address space 
   | :none:`worked on`| https://reviews.llvm.org/D95778 
(partial frontend)|
 
+--+--+--+---+
 | Feature optionality  | Builtin function overloads with generic 
address space| :part:`worked on`| https://reviews.llvm.org/D92004   
|
 
+--+--+--+---+
@@ -389,6 +389,8 @@ implementation status.
 
+--+--+--+---+
 | Feature optionality  | Work group collective functions   
   | :part:`worked on`| https://reviews.llvm.org/D92004 
  |
 
+--+--+--+---+
+| Feature optionality  | Image types   
   | :part:`unclaimed`| 
  |
++--+--+--+---+
 | New functionality| RGBA vector components
   | :good:`done` | https://reviews.llvm.org/D99969 
  |
 
+--+--+--+---+
 | New functionality| Subgroup functions
   | :part:`worked on`| https://reviews.llvm.org/D92004 
  |



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


[PATCH] D102850: [C++4OpenCL] Fix overloading resolution of addrspace constructors

2021-05-24 Thread Ole Strohm via Phabricator via cfe-commits
olestrohm added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:9870
 
+  if (S.getLangOpts().OpenCL) {
+if (const auto *CD1 = 
dyn_cast_or_null(Cand1.Function)) {

Anastasia wrote:
> olestrohm wrote:
> > Anastasia wrote:
> > > I think we should remove the OpenCL check since it is not OpenCL specific 
> > > rule and you are using common helpers indeed!
> > > 
> > > I also wonder if this should be applied to all member functions not only 
> > > ctors since `isBetterOverloadCandidate` should be used for everything?
> > > 
> > > However it seems that other members are already handled somehow 
> > > https://godbolt.org/z/MrWKPKed7. Do you know where this handling comes 
> > > from?
> > It's handled in SemaOverload.cpp:1 in 
> > `OverloadCandidateSet::BestViableFunction` which checks if the given 
> > function is viable or not. For member functions the address space they're 
> > defined in and the address space of the variable have to match. Therefore 
> > only one of them is valid at a time and they never get checked against each 
> > other in `isBetterOverloadCandidate`.
> I am quite sure that for the example above both `__private` and `__generic 
> `overloads are viable. In fact, if you remove the `__private` overload the 
> `__generic` one gets picked.
> 
> So I think the logic should be checking for compatibility rather than the 
> exact match i.e. using `isAddressSpaceSupersetOf` although it might not be 
> calling that helper directly but through the other Qualifier logic.
Ah, I think I checked it wrong last time. Going through it again it's actually 
determined by `QualType::isMoreQualifiedThan` in the end. I'll look into it 
more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102850

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


[clang] 626e964 - [OpenCL] Fix test by adding SPIR triple

2021-05-24 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2021-05-24T13:03:50+01:00
New Revision: 626e9641a2f5fde638b86d4e043f82fc58b908f8

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

LOG: [OpenCL] Fix test by adding SPIR triple

Added: 


Modified: 
clang/test/SemaOpenCL/unsupported.cl

Removed: 




diff  --git a/clang/test/SemaOpenCL/unsupported.cl 
b/clang/test/SemaOpenCL/unsupported.cl
index 36f60259f8a7..75175c88fe26 100644
--- a/clang/test/SemaOpenCL/unsupported.cl
+++ b/clang/test/SemaOpenCL/unsupported.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -verify %s
-// RUN: %clang_cc1 -verify %s -DBITFIELDS_EXT
+// RUN: %clang_cc1 -verify %s -DBITFIELDS_EXT -triple spir
 
 #ifdef BITFIELDS_EXT
 #pragma OPENCL EXTENSION __cl_clang_bitfields : enable



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


[PATCH] D101843: [OpenCL] Add clang extension for bitfields

2021-05-24 Thread Anastasia Stulova 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 rG237c6924bd46: [OpenCL] Add clang extension for bit-fields. 
(authored by Anastasia).
Herald added subscribers: foad, ldrumm.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D101843?vs=344784&id=347348#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101843

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Misc/amdgcn.languageOptsOpenCL.cl
  clang/test/Misc/nvptx.languageOptsOpenCL.cl
  clang/test/Misc/r600.languageOptsOpenCL.cl
  clang/test/SemaOpenCL/unsupported.cl

Index: clang/test/SemaOpenCL/unsupported.cl
===
--- clang/test/SemaOpenCL/unsupported.cl
+++ clang/test/SemaOpenCL/unsupported.cl
@@ -1,7 +1,15 @@
 // RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -DBITFIELDS_EXT
 
-struct {
-  int a : 1; // expected-error {{bit-fields are not supported in OpenCL}}
+#ifdef BITFIELDS_EXT
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+#endif
+
+struct test {
+  int a : 1;
+#ifndef BITFIELDS_EXT
+// expected-error@-2 {{bit-fields are not supported in OpenCL}}
+#endif
 };
 
 void no_vla(int n) {
Index: clang/test/Misc/r600.languageOptsOpenCL.cl
===
--- clang/test/Misc/r600.languageOptsOpenCL.cl
+++ clang/test/Misc/r600.languageOptsOpenCL.cl
@@ -39,6 +39,11 @@
 #endif
 #pragma OPENCL EXTENSION __cl_clang_non_portable_kernel_param_types : enable
 
+#ifndef __cl_clang_bitfields
+#error "Missing __cl_clang_bitfields define"
+#endif
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+
 #ifdef cl_khr_fp16
 #error "Incorrect cl_khr_fp16 define"
 #endif
Index: clang/test/Misc/nvptx.languageOptsOpenCL.cl
===
--- clang/test/Misc/nvptx.languageOptsOpenCL.cl
+++ clang/test/Misc/nvptx.languageOptsOpenCL.cl
@@ -33,6 +33,11 @@
 #endif
 #pragma OPENCL EXTENSION __cl_clang_non_portable_kernel_param_types : enable
 
+#ifndef __cl_clang_bitfields
+#error "Missing __cl_clang_bitfields define"
+#endif
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+
 #ifdef cl_khr_fp16
 #error "Incorrect cl_khr_fp16 define"
 #endif
Index: clang/test/Misc/amdgcn.languageOptsOpenCL.cl
===
--- clang/test/Misc/amdgcn.languageOptsOpenCL.cl
+++ clang/test/Misc/amdgcn.languageOptsOpenCL.cl
@@ -29,6 +29,11 @@
 #endif
 #pragma OPENCL EXTENSION __cl_clang_non_portable_kernel_param_types : enable
 
+#ifndef __cl_clang_bitfields
+#error "Missing __cl_clang_bitfields define"
+#endif
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+
 #ifndef cl_khr_fp16
 #error "Missing cl_khr_fp16 define"
 #endif
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16815,8 +16815,10 @@
   Record->setInvalidDecl();
   InvalidDecl = true;
 }
-// OpenCL v1.2 s6.9.c: bitfields are not supported.
-if (BitWidth) {
+// OpenCL v1.2 s6.9.c: bitfields are not supported, unless Clang extension
+// is enabled.
+if (BitWidth && !getOpenCLOptions().isAvailableOption(
+"__cl_clang_bitfields", LangOpts)) {
   Diag(Loc, diag::err_opencl_bitfields);
   InvalidDecl = true;
 }
Index: clang/lib/Basic/Targets/NVPTX.h
===
--- clang/lib/Basic/Targets/NVPTX.h
+++ clang/lib/Basic/Targets/NVPTX.h
@@ -136,6 +136,7 @@
 Opts["__cl_clang_function_pointers"] = true;
 Opts["__cl_clang_variadic_functions"] = true;
 Opts["__cl_clang_non_portable_kernel_param_types"] = true;
+Opts["__cl_clang_bitfields"] = true;
 
 Opts["cl_khr_fp64"] = true;
 Opts["__opencl_c_fp64"] = true;
Index: clang/lib/Basic/Targets/AMDGPU.h
===
--- clang/lib/Basic/Targets/AMDGPU.h
+++ clang/lib/Basic/Targets/AMDGPU.h
@@ -288,6 +288,7 @@
 Opts["__cl_clang_variadic_functions"] = true;
 Opts["__cl_clang_function_pointers"] = true;
 Opts["__cl_clang_non_portable_kernel_param_types"] = true;
+Opts["__cl_clang_bitfields"] = true;
 
 bool IsAMDGCN = isAMDGCN(getTriple());
 
Index: clang/include/clang/Basic/OpenCLExtensions.def
===
--- clang/include/clang/Basic/OpenCLExtensions.def
+++ clang/include/clang/Basic/OpenCLExtensions.def
@@ -88,6 +88,7 @@
 OPENCL_EXTENSION(__cl_clang_function_pointers, true, 100)
 OPENCL_EXTENSI

[clang] 237c692 - [OpenCL] Add clang extension for bit-fields.

2021-05-24 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2021-05-24T12:42:17+01:00
New Revision: 237c6924bd46ec0e33da71f9616caf9bf9965b23

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

LOG: [OpenCL] Add clang extension for bit-fields.

Allow use of bit-fields as a clang extension
in OpenCL. The extension can be enabled using
pragma directives.

This fixes PR45339!

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/OpenCLExtensions.def
clang/lib/Basic/Targets/AMDGPU.h
clang/lib/Basic/Targets/NVPTX.h
clang/lib/Sema/SemaDecl.cpp
clang/test/Misc/amdgcn.languageOptsOpenCL.cl
clang/test/Misc/nvptx.languageOptsOpenCL.cl
clang/test/Misc/r600.languageOptsOpenCL.cl
clang/test/SemaOpenCL/unsupported.cl

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 235bcac8f00f4..eb9fe0c0ac0d7 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1741,6 +1741,34 @@ OpenCL Features
 
 Clang supports internal OpenCL extensions documented below.
 
+``__cl_clang_bitfields``
+
+
+With this extension it is possible to enable bitfields in structs
+or unions using the OpenCL extension pragma mechanism detailed in
+`the OpenCL Extension Specification, section 1.2
+`_.
+
+Use of bitfields in OpenCL kernels can result in reduced portability as struct
+layout is not guaranteed to be consistent when compiled by 
diff erent compilers.
+If structs with bitfields are used as kernel function parameters, it can result
+in incorrect functionality when the layout is 
diff erent between the host and
+device code.
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  #pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+  struct with_bitfield {
+unsigned int i : 5; // compiled - no diagnostic generated
+  };
+
+  #pragma OPENCL EXTENSION __cl_clang_bitfields : disable
+  struct without_bitfield {
+unsigned int i : 5; // error - bitfields are not supported
+  };
+
 ``__cl_clang_function_pointers``
 
 

diff  --git a/clang/include/clang/Basic/OpenCLExtensions.def 
b/clang/include/clang/Basic/OpenCLExtensions.def
index a0f01a2af9c37..a053a0e9adb59 100644
--- a/clang/include/clang/Basic/OpenCLExtensions.def
+++ b/clang/include/clang/Basic/OpenCLExtensions.def
@@ -88,6 +88,7 @@ OPENCL_EXTENSION(cl_clang_storage_class_specifiers, true, 100)
 OPENCL_EXTENSION(__cl_clang_function_pointers, true, 100)
 OPENCL_EXTENSION(__cl_clang_variadic_functions, true, 100)
 OPENCL_EXTENSION(__cl_clang_non_portable_kernel_param_types, true, 100)
+OPENCL_EXTENSION(__cl_clang_bitfields, true, 100)
 
 // AMD OpenCL extensions
 OPENCL_EXTENSION(cl_amd_media_ops, true, 100)

diff  --git a/clang/lib/Basic/Targets/AMDGPU.h 
b/clang/lib/Basic/Targets/AMDGPU.h
index 2729c515bb650..fe5c61c6ba2bb 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -288,6 +288,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : 
public TargetInfo {
 Opts["__cl_clang_variadic_functions"] = true;
 Opts["__cl_clang_function_pointers"] = true;
 Opts["__cl_clang_non_portable_kernel_param_types"] = true;
+Opts["__cl_clang_bitfields"] = true;
 
 bool IsAMDGCN = isAMDGCN(getTriple());
 

diff  --git a/clang/lib/Basic/Targets/NVPTX.h b/clang/lib/Basic/Targets/NVPTX.h
index c429d5501337a..c7db3cdaaf10a 100644
--- a/clang/lib/Basic/Targets/NVPTX.h
+++ b/clang/lib/Basic/Targets/NVPTX.h
@@ -136,6 +136,7 @@ class LLVM_LIBRARY_VISIBILITY NVPTXTargetInfo : public 
TargetInfo {
 Opts["__cl_clang_function_pointers"] = true;
 Opts["__cl_clang_variadic_functions"] = true;
 Opts["__cl_clang_non_portable_kernel_param_types"] = true;
+Opts["__cl_clang_bitfields"] = true;
 
 Opts["cl_khr_fp64"] = true;
 Opts["__opencl_c_fp64"] = true;

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6d6b8b5e797da..601f4f2502f0a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16815,8 +16815,10 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, 
QualType T,
   Record->setInvalidDecl();
   InvalidDecl = true;
 }
-// OpenCL v1.2 s6.9.c: bitfields are not supported.
-if (BitWidth) {
+// OpenCL v1.2 s6.9.c: bitfields are not supported, unless Clang extension
+// is enabled.
+if (BitWidth && !getOpenCLOptions().isAvailableOption(
+"__cl_clang_bitfields", LangOpts)) {
   Diag(Loc, diag::err_opencl_bitfields);
   InvalidDecl = true;
 }

diff  --git a

[PATCH] D101156: [Clang] Support a user-defined __dso_handle

2021-05-24 Thread Andrew Savonichev via Phabricator via cfe-commits
asavonic added a comment.

I went ahead and implemented a fix for EmitGlobalVarDefinition. Please let me 
know what approach is preferable: Diff 339986 or Diff 347341.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101156

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


[PATCH] D102850: [C++4OpenCL] Fix overloading resolution of addrspace constructors

2021-05-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:9870
 
+  if (S.getLangOpts().OpenCL) {
+if (const auto *CD1 = 
dyn_cast_or_null(Cand1.Function)) {

olestrohm wrote:
> Anastasia wrote:
> > I think we should remove the OpenCL check since it is not OpenCL specific 
> > rule and you are using common helpers indeed!
> > 
> > I also wonder if this should be applied to all member functions not only 
> > ctors since `isBetterOverloadCandidate` should be used for everything?
> > 
> > However it seems that other members are already handled somehow 
> > https://godbolt.org/z/MrWKPKed7. Do you know where this handling comes from?
> It's handled in SemaOverload.cpp:1 in 
> `OverloadCandidateSet::BestViableFunction` which checks if the given function 
> is viable or not. For member functions the address space they're defined in 
> and the address space of the variable have to match. Therefore only one of 
> them is valid at a time and they never get checked against each other in 
> `isBetterOverloadCandidate`.
I am quite sure that for the example above both `__private` and `__generic 
`overloads are viable. In fact, if you remove the `__private` overload the 
`__generic` one gets picked.

So I think the logic should be checking for compatibility rather than the exact 
match i.e. using `isAddressSpaceSupersetOf` although it might not be calling 
that helper directly but through the other Qualifier logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102850

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


[PATCH] D101156: [Clang] Support a user-defined __dso_handle

2021-05-24 Thread Andrew Savonichev via Phabricator via cfe-commits
asavonic updated this revision to Diff 347341.
asavonic edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101156

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/dso-handle-custom.cpp


Index: clang/test/CodeGenCXX/dso-handle-custom.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dso-handle-custom.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s 
-o - | FileCheck %s --check-prefixes CHECK,CHECK-DEFAULT
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s 
-o - -DHIDDEN | FileCheck %s --check-prefixes CHECK,CHECK-HIDDEN
+
+class A {
+public:
+  ~A();
+} a;
+
+// CHECK-DEFAULT: @__dso_handle = global i8* bitcast (i8** @__dso_handle to 
i8*), align 8
+// CHECK-HIDDEN: @__dso_handle = hidden global i8* bitcast (i8** @__dso_handle 
to i8*), align 8
+// CHECK: define internal void @__cxx_global_var_init()
+// CHECK:   call i32 @__cxa_atexit({{.*}}, {{.*}}, i8* bitcast (i8** 
@__dso_handle to i8*))
+
+#ifdef HIDDEN
+void *__dso_handle __attribute__((__visibility__("hidden"))) = &__dso_handle;
+#else
+void *__dso_handle = &__dso_handle;
+#endif
+
+void use(void *);
+void use_dso_handle() {
+  use(__dso_handle);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4337,6 +4337,23 @@
   }
 
   llvm::Type* InitType = Init->getType();
+
+  bool ReplaceInitWithGV = false;
+  llvm::GlobalValue *OrigEntry = GetGlobalValue(getMangledName(D));
+  if (Init == OrigEntry && OrigEntry->getValueType() != InitType) {
+// Type of the initializer does not match the type of the global -
+// GetAddrOfGlobalVar is going to recreate it, replace all uses of
+// the original entry and erase the original entry from the
+// module.
+//
+// However, if Init is an address of the global itself, then it is
+// not a use, and thus it will not be replaced. It will become a
+// dangling pointer after GetAddrOfGlobalVar. Replace it once we
+// get a new GV.
+//
+ReplaceInitWithGV = true;
+  }
+
   llvm::Constant *Entry =
   GetAddrOfGlobalVar(D, InitType, ForDefinition_t(!IsTentative));
 
@@ -4404,6 +4421,9 @@
 getCUDARuntime().handleVarRegistration(D, *GV);
   }
 
+  if (ReplaceInitWithGV)
+Init = llvm::ConstantExpr::getBitCast(GV, GV->getValueType());
+
   GV->setInitializer(Init);
   if (emitter)
 emitter->finalize(GV);


Index: clang/test/CodeGenCXX/dso-handle-custom.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dso-handle-custom.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-DEFAULT
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s -o - -DHIDDEN | FileCheck %s --check-prefixes CHECK,CHECK-HIDDEN
+
+class A {
+public:
+  ~A();
+} a;
+
+// CHECK-DEFAULT: @__dso_handle = global i8* bitcast (i8** @__dso_handle to i8*), align 8
+// CHECK-HIDDEN: @__dso_handle = hidden global i8* bitcast (i8** @__dso_handle to i8*), align 8
+// CHECK: define internal void @__cxx_global_var_init()
+// CHECK:   call i32 @__cxa_atexit({{.*}}, {{.*}}, i8* bitcast (i8** @__dso_handle to i8*))
+
+#ifdef HIDDEN
+void *__dso_handle __attribute__((__visibility__("hidden"))) = &__dso_handle;
+#else
+void *__dso_handle = &__dso_handle;
+#endif
+
+void use(void *);
+void use_dso_handle() {
+  use(__dso_handle);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4337,6 +4337,23 @@
   }
 
   llvm::Type* InitType = Init->getType();
+
+  bool ReplaceInitWithGV = false;
+  llvm::GlobalValue *OrigEntry = GetGlobalValue(getMangledName(D));
+  if (Init == OrigEntry && OrigEntry->getValueType() != InitType) {
+// Type of the initializer does not match the type of the global -
+// GetAddrOfGlobalVar is going to recreate it, replace all uses of
+// the original entry and erase the original entry from the
+// module.
+//
+// However, if Init is an address of the global itself, then it is
+// not a use, and thus it will not be replaced. It will become a
+// dangling pointer after GetAddrOfGlobalVar. Replace it once we
+// get a new GV.
+//
+ReplaceInitWithGV = true;
+  }
+
   llvm::Constant *Entry =
   GetAddrOfGlobalVar(D, InitType, ForDefinition_t(!IsTentative));
 
@@ -4404,6 +4421,9 @@
 getCUDARuntime().handleVarRegistration(D, *GV);
   }
 
+  if (ReplaceInitWithGV)
+Init = llvm::ConstantExpr::getBitCast(GV, GV-

[PATCH] D100630: [Debug-Info][DBX] DW_TAG_rvalue_reference_type should not be generated when dwarf version is smaller than 4

2021-05-24 Thread ChenZheng via Phabricator via cfe-commits
shchenz added a comment.

Thanks for your review! @dblaikie @aprantl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100630

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


  1   2   >