[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added inline comments.



Comment at: clang/www/cxx_status.html:1485-1486
+Clang 17 (Partial)
+ We do not support outside of defaulted special memeber functions the 
change that constexpr functions no
+  longer have to be constexpr compatible but rather support a less 
restricted requirements for constexpr
+  functions. Which include allowing non-literal types as return values 
and paremeters, allow calling of

That sentence is impossible to parse for me. Is "outside of defaulted special 
member functions" supposed to be a subclause? The second sentence is also not 
great ("Which include [X], allow [Y]").

Also typos: "memeber" and "paremeters".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[clang] e955e4f - [clang] Replace None with std::nullopt in comments (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2023-05-04T22:42:52-07:00
New Revision: e955e4fba60e6d93b66903687c1dd7a34435d33c

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

LOG: [clang] Replace None with std::nullopt in comments (NFC)

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

Added: 


Modified: 
clang/include/clang/AST/ExprCXX.h
clang/include/clang/Basic/DiagnosticError.h
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
clang/include/clang/Lex/Lexer.h
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 724904b4d2041..16f54d753d42f 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2321,7 +2321,7 @@ class CXXNewExpr final
 
   /// This might return std::nullopt even if isArray() returns true,
   /// since there might not be an array size expression.
-  /// If the result is not-None, it will never wrap a nullptr.
+  /// If the result is not std::nullopt, it will never wrap a nullptr.
   std::optional getArraySize() {
 if (!isArray())
   return std::nullopt;
@@ -2335,7 +2335,7 @@ class CXXNewExpr final
 
   /// This might return std::nullopt even if isArray() returns true,
   /// since there might not be an array size expression.
-  /// If the result is not-None, it will never wrap a nullptr.
+  /// If the result is not std::nullopt, it will never wrap a nullptr.
   std::optional getArraySize() const {
 if (!isArray())
   return std::nullopt;

diff  --git a/clang/include/clang/Basic/DiagnosticError.h 
b/clang/include/clang/Basic/DiagnosticError.h
index 3660bd1b3b3d8..744f7fe19db79 100644
--- a/clang/include/clang/Basic/DiagnosticError.h
+++ b/clang/include/clang/Basic/DiagnosticError.h
@@ -35,8 +35,8 @@ class DiagnosticError : public 
llvm::ErrorInfo {
   }
 
   /// Extracts and returns the diagnostic payload from the given \c Error if
-  /// the error is a \c DiagnosticError. Returns none if the given error is not
-  /// a \c DiagnosticError.
+  /// the error is a \c DiagnosticError. Returns std::nullopt if the given 
error
+  /// is not a \c DiagnosticError.
   static std::optional take(llvm::Error ) {
 std::optional Result;
 Err = llvm::handleErrors(std::move(Err), [&](DiagnosticError ) {

diff  --git 
a/clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h 
b/clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
index 55c7bb32054bc..6639082bbf332 100644
--- a/clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
+++ b/clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
@@ -67,7 +67,7 @@ class SymbolGraphSerializer : public APISerializer {
   ///
   /// \returns an optional JSON Object representing the payload that libclang
   /// expects for providing symbol information for a single symbol. If this is
-  /// not a known symbol returns \c None.
+  /// not a known symbol returns \c std::nullopt.
   static std::optional serializeSingleSymbolSGF(StringRef USR,
 const APISet );
 

diff  --git a/clang/include/clang/Lex/Lexer.h b/clang/include/clang/Lex/Lexer.h
index 8c2923b0150a0..98d34b783f084 100644
--- a/clang/include/clang/Lex/Lexer.h
+++ b/clang/include/clang/Lex/Lexer.h
@@ -551,7 +551,7 @@ class Lexer : public PreprocessorLexer {
 
   /// Finds the token that comes right after the given location.
   ///
-  /// Returns the next token, or none if the location is inside a macro.
+  /// Returns the next token, or std::nullopt if the location is inside a 
macro.
   static std::optional findNextToken(SourceLocation Loc,
 const SourceManager ,
 const LangOptions );

diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 3b58a51195f80..7871fed519b98 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -332,7 +332,7 @@ class FixableGadget : public Gadget {
   bool isWarningGadget() const final { return false; }
 
   /// Returns a fixit that would fix the current gadget according to
-  /// the current strategy. Returns None if the fix cannot be produced;
+  /// the current strategy. Returns std::nullopt if the fix cannot be produced;
   /// returns an empty list if 

[PATCH] D149920: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 519744.
MaskRay marked an inline comment as done.
MaskRay added a comment.

fix a typo in a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149920

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll

Index: llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll
===
--- llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll
+++ llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll
@@ -15,6 +15,7 @@
 ;   __asm {
 ;   call static_func
 ;   call extern_func
+;   jmp extern_func
 ;   shr eax, 0
 ;   shr ebx, 0
 ;   shr ecx, 0
@@ -40,6 +41,7 @@
 ; CHECK-EMPTY:
 ; CHECK-NEXT:calll static_func
 ; CHECK-NEXT:calll extern_func@PLT
+; CHECK-NEXT:jmp extern_func@PLT
 ; CHECK-NEXT:shrl $0, %eax
 ; CHECK-NEXT:shrl $0, %ebx
 ; CHECK-NEXT:shrl $0, %ecx
@@ -52,7 +54,8 @@
 ; CHECK-NEXT:#NO_APP
 entry:
   %call = tail call i32 @static_func()
-  tail call void asm sideeffect inteldialect "call dword ptr ${0:P}\0A\09call dword ptr ${1:P}\0A\09shr eax, $$0\0A\09shr ebx, $$0\0A\09shr ecx, $$0\0A\09shr edx, $$0\0A\09shr edi, $$0\0A\09shr esi, $$0\0A\09shr ebp, $$0\0A\09shr esp, $$0", "*m,*m,~{eax},~{ebp},~{ebx},~{ecx},~{edi},~{edx},~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(ptr nonnull elementtype(i32 (...)) @static_func, ptr nonnull elementtype(i32 (...)) @extern_func) #0
+;; We test call, CALL, and jmp.
+  tail call void asm sideeffect inteldialect "call ${0:P}\0A\09CALL ${1:P}\0A\09jmp ${1:P}\0A\09shr eax, $$0\0A\09shr ebx, $$0\0A\09shr ecx, $$0\0A\09shr edx, $$0\0A\09shr edi, $$0\0A\09shr esi, $$0\0A\09shr ebp, $$0\0A\09shr esp, $$0", "*m,*m,~{eax},~{ebp},~{ebx},~{ecx},~{edi},~{edx},~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(ptr nonnull elementtype(i32 (...)) @static_func, ptr nonnull elementtype(i32 (...)) @extern_func) #0
   ret void
 }
 
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -33942,12 +33942,9 @@
 // "call dword ptr "
 auto TmpStr = AsmStr.substr(0, I);
 I = TmpStr.rfind(':');
-if (I == StringRef::npos)
-  return TmpStr;
-
-assert(I < TmpStr.size() && "Unexpected inline asm string!");
-auto Asm = TmpStr.drop_front(I + 1);
-return Asm;
+if (I != StringRef::npos)
+  TmpStr = TmpStr.substr(I + 1);
+return TmpStr.take_while(llvm::isAlpha);
   }
 
   return StringRef();
@@ -33955,12 +33952,13 @@
 
 bool X86TargetLowering::isInlineAsmTargetBranch(
 const SmallVectorImpl , unsigned OpNo) const {
-  StringRef InstrStr = getInstrStrFromOpNo(AsmStrs, OpNo);
-
-  if (InstrStr.contains("call"))
-return true;
-
-  return false;
+  // In a __asm block, __asm inst foo where inst is CALL or JMP should be
+  // changed from indirect TargetLowering::C_Memory to direct
+  // TargetLowering::C_Address.
+  // We don't need to special case LOOP* and Jcc, which cannot target a memory
+  // location.
+  StringRef Inst = getInstrStrFromOpNo(AsmStrs, OpNo);
+  return Inst.equals_insensitive("call") || Inst.equals_insensitive("jmp");
 }
 
 /// Provide custom lowering hooks for some operations.
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3624,15 +3624,13 @@
   /// legal.  It is frequently not legal in PIC relocation models.
   virtual bool isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const;
 
-  /// Return true if the operand with index OpNo corresponding to a target
-  /// branch, for example, in following case
+  /// On x86, return true if the operand with index OpNo is a CALL or JUMP
+  /// instruction, which can use either a memory constraint or an address
+  /// constraint. -fasm-blocks "__asm call foo" lowers to
+  /// call void asm sideeffect inteldialect "call ${0:P}", "*m..."
   ///
-  /// call void asm "lea r8, $0\0A\09call qword ptr ${1:P}\0A\09ret",
-  ///   "*m,*m,~{r8},~{dirflag},~{fpsr},~{flags}"
-  ///([9 x i32]* @Arr), void (...)* @sincos_asm)
-  ///
-  /// the operand $1 (sincos_asm) is target branch in inline asm, but the
-  /// operand $0 (Arr) is not.
+  /// This function is used by a hack to choose the address constraint,
+  /// lowering to a direct call.
   virtual bool
   isInlineAsmTargetBranch(const SmallVectorImpl ,
   unsigned OpNo) const {
Index: clang/test/CodeGen/ms-inline-asm-functions.c

[PATCH] D149920: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:33955
 const SmallVectorImpl , unsigned OpNo) const {
-  StringRef InstrStr = getInstrStrFromOpNo(AsmStrs, OpNo);
-
-  if (InstrStr.contains("call"))
-return true;
-
-  return false;
+  // In a __asm block, __asm inst foo where inst is CALL or FOO should be
+  // changed from indirect TargetLowering::C_Memory to direct

JMP?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149920

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

[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

[Sheng via Phabricator via cfe-commits]

0x59616e added inline comments.



Comment at: clang/test/CodeGen/mrtd.c:9
+// X86: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// M68k: define{{.*}} cc104 void @foo(i32 noundef %arg)
 void foo(int arg) {

0x59616e wrote:
> Just curious, why do we have to use such an arcane name instead of a more 
> lucid one , such as `m68k_rtdcc`.
I guess this involves more work ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149867

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

[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

[Sheng via Phabricator via cfe-commits]

0x59616e added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:559
+emitError |= DefaultCC == LangOptions::DCC_StdCall &&
+ !(Arch == llvm::Triple::m68k || Arch == llvm::Triple::x86);
 emitError |= (DefaultCC == LangOptions::DCC_VectorCall ||

nit: Is the expanded one better in terms of readability ?



Comment at: clang/test/CodeGen/mrtd.c:9
+// X86: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// M68k: define{{.*}} cc104 void @foo(i32 noundef %arg)
 void foo(int arg) {

Just curious, why do we have to use such an arcane name instead of a more lucid 
one , such as `m68k_rtdcc`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149867

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

[PATCH] D149612: [Sema] avoid merge error type

[Congcong Cai via Phabricator via cfe-commits]

HerrCai0907 added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2582
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
-T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
+if (getLangOpts().CPlusPlus) {
+  T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals,

erichkeane wrote:
> So I'm still not sure this is the 'right' away about it.  I think we should 
> instead properly handle the `containsErrors` case and just always create a 
> non-dependent sized array, except with the `RecoveryExpr` having the correct 
> type.
For CPP, dependent sized array is acceptable and necessary.

For C, I don't think SemaType is a correctly way to resolve TypoExpr, It should 
be done by `ActOnXXX`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

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

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133
 }
 
+Response HandlePartialClassTemplateSpec(

alexander-shaposhnikov wrote:
> alexander-shaposhnikov wrote:
> > HandlePartialClassTemplateSpec is from Erich's diff 
> > (https://reviews.llvm.org/D147722)
> @erichkeane : previously (see https://reviews.llvm.org/D147722) we were 
> producing a real layer of arguments (if the depth is greater than 1), to the 
> best of my knowledge this is incorrect since it'd trigger unexpected depth 
> adjustment, we should produce retained layers instead.
(depth adjustment during the substitution)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D149920: ms inline asm: recognize case-insensitive JMP and CALL as TargetLowering::C_Address

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 519734.
MaskRay retitled this revision from "ms inline asm: recognize "jmp" as 
TargetLowering::C_Address" to "ms inline asm: recognize case-insensitive JMP 
and CALL as TargetLowering::C_Address".
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

better description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149920

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll

Index: llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll
===
--- llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll
+++ llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll
@@ -15,6 +15,7 @@
 ;   __asm {
 ;   call static_func
 ;   call extern_func
+;   jmp extern_func
 ;   shr eax, 0
 ;   shr ebx, 0
 ;   shr ecx, 0
@@ -40,6 +41,7 @@
 ; CHECK-EMPTY:
 ; CHECK-NEXT:calll static_func
 ; CHECK-NEXT:calll extern_func@PLT
+; CHECK-NEXT:jmp extern_func@PLT
 ; CHECK-NEXT:shrl $0, %eax
 ; CHECK-NEXT:shrl $0, %ebx
 ; CHECK-NEXT:shrl $0, %ecx
@@ -52,7 +54,8 @@
 ; CHECK-NEXT:#NO_APP
 entry:
   %call = tail call i32 @static_func()
-  tail call void asm sideeffect inteldialect "call dword ptr ${0:P}\0A\09call dword ptr ${1:P}\0A\09shr eax, $$0\0A\09shr ebx, $$0\0A\09shr ecx, $$0\0A\09shr edx, $$0\0A\09shr edi, $$0\0A\09shr esi, $$0\0A\09shr ebp, $$0\0A\09shr esp, $$0", "*m,*m,~{eax},~{ebp},~{ebx},~{ecx},~{edi},~{edx},~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(ptr nonnull elementtype(i32 (...)) @static_func, ptr nonnull elementtype(i32 (...)) @extern_func) #0
+;; We test call, CALL, and jmp.
+  tail call void asm sideeffect inteldialect "call ${0:P}\0A\09CALL ${1:P}\0A\09jmp ${1:P}\0A\09shr eax, $$0\0A\09shr ebx, $$0\0A\09shr ecx, $$0\0A\09shr edx, $$0\0A\09shr edi, $$0\0A\09shr esi, $$0\0A\09shr ebp, $$0\0A\09shr esp, $$0", "*m,*m,~{eax},~{ebp},~{ebx},~{ecx},~{edi},~{edx},~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(ptr nonnull elementtype(i32 (...)) @static_func, ptr nonnull elementtype(i32 (...)) @extern_func) #0
   ret void
 }
 
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -33942,12 +33942,9 @@
 // "call dword ptr "
 auto TmpStr = AsmStr.substr(0, I);
 I = TmpStr.rfind(':');
-if (I == StringRef::npos)
-  return TmpStr;
-
-assert(I < TmpStr.size() && "Unexpected inline asm string!");
-auto Asm = TmpStr.drop_front(I + 1);
-return Asm;
+if (I != StringRef::npos)
+  TmpStr = TmpStr.substr(I + 1);
+return TmpStr.take_while(llvm::isAlpha);
   }
 
   return StringRef();
@@ -33955,12 +33952,13 @@
 
 bool X86TargetLowering::isInlineAsmTargetBranch(
 const SmallVectorImpl , unsigned OpNo) const {
-  StringRef InstrStr = getInstrStrFromOpNo(AsmStrs, OpNo);
-
-  if (InstrStr.contains("call"))
-return true;
-
-  return false;
+  // In a __asm block, __asm inst foo where inst is CALL or FOO should be
+  // changed from indirect TargetLowering::C_Memory to direct
+  // TargetLowering::C_Address.
+  // We don't need to special case LOOP* and Jcc, which cannot target a memory
+  // location.
+  StringRef Inst = getInstrStrFromOpNo(AsmStrs, OpNo);
+  return Inst.equals_insensitive("call") || Inst.equals_insensitive("jmp");
 }
 
 /// Provide custom lowering hooks for some operations.
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3624,15 +3624,13 @@
   /// legal.  It is frequently not legal in PIC relocation models.
   virtual bool isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const;
 
-  /// Return true if the operand with index OpNo corresponding to a target
-  /// branch, for example, in following case
+  /// On x86, return true if the operand with index OpNo is a CALL or JUMP
+  /// instruction, which can use either a memory constraint or an address
+  /// constraint. -fasm-blocks "__asm call foo" lowers to
+  /// call void asm sideeffect inteldialect "call ${0:P}", "*m..."
   ///
-  /// call void asm "lea r8, $0\0A\09call qword ptr ${1:P}\0A\09ret",
-  ///   "*m,*m,~{r8},~{dirflag},~{fpsr},~{flags}"
-  ///([9 x i32]* @Arr), void (...)* @sincos_asm)
-  ///
-  /// the operand $1 (sincos_asm) is target branch in inline asm, but the
-  /// operand $0 (Arr) is not.
+  /// This function is used by a hack to choose the address constraint,
+  /// 

[PATCH] D148092: [clang][CodeGen] Break up TargetInfo.cpp [4/6]

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D148092#4317438 , @efriedma wrote:

> Is there some alternative way we can write this?  Even if each of the 
> overrides is only technically used in one place, it's a repeating pattern, 
> and writing the casts inline makes it really hard to read.  (Maybe the 
> helpers can be somewhere else?)

Thanks for the suggestion. Does it look better now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148092

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

[PATCH] D148092: [clang][CodeGen] Break up TargetInfo.cpp [4/6]

[Sergei Barannikov via Phabricator via cfe-commits]

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

Add a templated overload to help derived classes avoid explicit casts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148092

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h

Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -52,6 +52,11 @@
   // by returning true from TargetInfo::checkCallingConvention for them.
   std::unique_ptr SwiftInfo;
 
+  // Returns ABI info helper for the target. This is for use by derived classes.
+  template  const T () const {
+return static_cast(*Info);
+  }
+
 public:
   TargetCodeGenInfo(std::unique_ptr Info);
   virtual ~TargetCodeGenInfo();
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2483,10 +2483,6 @@
 std::make_unique(CGT, /*SwiftErrorInRegister=*/true);
   }
 
-  const X86_64ABIInfo () const {
-return static_cast(TargetCodeGenInfo::getABIInfo());
-  }
-
   /// Disable tail call on x86-64. The epilogue code before the tail jump blocks
   /// autoreleaseRV/retainRV and autoreleaseRV/unsafeClaimRV optimizations.
   bool markARCOptimizedReturnCallsAsNoTail() const override { return true; }
@@ -2523,7 +2519,7 @@
   bool HasAVXType = false;
   for (CallArgList::const_iterator
  it = args.begin(), ie = args.end(); it != ie; ++it) {
-if (getABIInfo().isPassedUsingAVXType(it->Ty)) {
+if (getABIInfo().isPassedUsingAVXType(it->Ty)) {
   HasAVXType = true;
   break;
 }
@@ -6404,10 +6400,6 @@
 SwiftInfo = std::make_unique(CGT);
   }
 
-  const ARMABIInfo () const {
-return static_cast(TargetCodeGenInfo::getABIInfo());
-  }
-
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 return 13;
   }
@@ -6426,7 +6418,8 @@
   }
 
   unsigned getSizeOfUnwindException() const override {
-if (getABIInfo().isEABI()) return 88;
+if (getABIInfo().isEABI())
+  return 88;
 return TargetCodeGenInfo::getSizeOfUnwindException();
   }
 
@@ -6493,7 +6486,7 @@
 
 Fn->addFnAttr("interrupt", Kind);
 
-ARMABIKind ABI = cast(getABIInfo()).getABIKind();
+ARMABIKind ABI = getABIInfo().getABIKind();
 if (ABI == ARMABIKind::APCS)
   return;
 
@@ -7431,10 +7424,6 @@
 class SystemZTargetCodeGenInfo : public TargetCodeGenInfo {
   ASTContext 
 
-  const SystemZABIInfo () const {
-return static_cast(TargetCodeGenInfo::getABIInfo());
-  }
-
   // These are used for speeding up the search for a visible vector ABI.
   mutable bool HasVisibleVecABIFlag = false;
   mutable std::set SeenTypes;
@@ -7883,8 +7872,9 @@
 // it will be passed in a vector register. A wide (>16 bytes) vector will
 // be passed via "hidden" pointer where any extra alignment is not
 // required (per GCC).
-const Type *SingleEltTy =
-  getABIInfo().GetSingleElementType(QualType(Ty, 0)).getTypePtr();
+const Type *SingleEltTy = getABIInfo()
+  .GetSingleElementType(QualType(Ty, 0))
+  .getTypePtr();
 bool SingleVecEltStruct = SingleEltTy != Ty && SingleEltTy->isVectorType() &&
   Ctx.getTypeSize(SingleEltTy) == Ctx.getTypeSize(Ty);
 if (Ty->isVectorType() || SingleVecEltStruct)
@@ -11857,10 +11847,6 @@
 public:
   BPFTargetCodeGenInfo(CodeGenTypes )
   : TargetCodeGenInfo(std::make_unique(CGT)) {}
-
-  const BPFABIInfo () const {
-return static_cast(TargetCodeGenInfo::getABIInfo());
-  }
 };
 
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/Modules/Inputs/implicit-module-header-maps/a.h:1-3
+#ifdef FOO
+#error foo
+#endif

It would be better to use `split-file` to merge the segments of the tests. You 
can find the example in the Modules directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D146342: [-Wunsafe-buffer-usage] Move the whole analysis to the end of a translation unit

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added inline comments.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2335
+  // To analyze callables:
+  if (isa(Node)) {
+// For code in dependent contexts, we'll do this at instantiation time:

NoQ wrote:
> The intended way to make recursive visitors is to have 
> `TraverseFunctionDecl`, `TraverseBlockDecl`, etc., which automatically do the 
> `isa` check and fall through to the superclass if the method isn't defined. 
> Then they can all call a common method to analyze the body.
> 
> This doesn't necessarily lead to better code though, just to more traditional 
> code, so up to you^^ in this case it probably doesn't matter. But it's 
> definitely nice to separate the common part (the part that invokes all 
> individual analyses) into its own method. And then you can eg. isolate the 
> special `hasBody()` check in the `TraverseFunctionDecl()` method, so that 
> other code paths didn't have an extra runtime check.
yeah, I think overriding `VisitFunctionDecl` and others separately is a better 
form because I was clearly dealing with `FunctionDecl` and others case by case 
there.  So put the specific logic for `FunctionDecl` in `VisitFunctionDecl` 
makes it clear.

Thanks for your advice.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2343-2345
+  // `FunctionDecl->hasBody()` returns true if the function has a body
+  // somewhere defined.  But we want to know if this `Node` has a body
+  // child.  So we use `doesThisDeclarationHaveABody`:

NoQ wrote:
> I wonder if `ObjCMethodDecl` has the same problem. They too can have 
> prototypes and definitions separate from each other.
It looks like `hasBody()` works properly for  `ObjCMethodDecl`.  I'm adding a 
test for it.


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

https://reviews.llvm.org/D146342

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

[PATCH] D146342: [-Wunsafe-buffer-usage] Move the whole analysis to the end of a translation unit

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 519719.
ziqingluo-90 marked 3 inline comments as done.
ziqingluo-90 added a comment.

Addressed the comments:

- 1) about using a more traditional way of AST traversal; and
- 2) about the concern on `ObjcMethodDecl` traversal.


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

https://reviews.llvm.org/D146342

Files:
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-objc-method-traversal.mm

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-objc-method-traversal.mm
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-objc-method-traversal.mm
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-objc-root-class -Wno-return-type -Wunsafe-buffer-usage %s -verify %s
+
+// This test is to show that ObjC methods are traversed by the
+// end-of-TU analysis properly.  Particularly, a method body will not
+// be repeatedly visited whenever a method prototype of it is visited.
+
+@interface I
++ f:(int *) p; // duplicated method prototype declaration
+
++ f:(int *) p;
+
++ f:(int *) p;
+@end
+
+@implementation I
++ f:(int *) p { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  int tmp;
+  tmp = p[5];  // expected-note{{used in buffer access here}}
+}
+@end
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -13,6 +13,7 @@
 //===--===//
 
 #include "clang/Sema/AnalysisBasedWarnings.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
@@ -2316,44 +2317,53 @@
   public:
 CallableVisitor(Sema , bool EmitFixits) : S(S), EmitFixits(EmitFixits) {}
 
-// The only interface that clients of `CallableVisitor` should call:
-void traverseTU(const TranslationUnitDecl *TU) {
-  for (auto I = TU->decls_begin(); I != TU->decls_end(); ++I)
-TraverseDecl(*I);
-}
-
 bool TraverseDecl(Decl *Node) {
-  DiagnosticsEngine  = S.getDiagnostics();
-
   if (!Node)
 return true;
-  // Do not do any analysis if we are going to just ignore them.
+
+  // Do not analyze any `Decl` if we are going to just ignore them.
+  DiagnosticsEngine  = S.getDiagnostics();
+
   if (Diags.getSuppressSystemWarnings() &&
   S.SourceMgr.isInSystemHeader(Node->getLocation()))
 return true;
-  // To analyze callables:
-  if (isa(Node)) {
-// For code in dependent contexts, we'll do this at instantiation time:
-if (cast(Node)->isDependentContext())
-  return true;
+  // Continue to traverse descendants:
+  return RecursiveASTVisitor::TraverseDecl(Node);
+}
+
+bool VisitFunctionDecl(FunctionDecl *Node) {
+  if (cast(Node)->isDependentContext())
+return true; // Not to analyze dependent decl
+  // `FunctionDecl->hasBody()` returns true if the function has a body
+  // somewhere defined.  But we want to know if this `Node` has a body
+  // child.  So we use `doesThisDeclarationHaveABody`:
+  if (Node->doesThisDeclarationHaveABody()) {
+UnsafeBufferUsageReporter R(S);
 
-bool ThisNodeHasBody = false;
+checkUnsafeBufferUsage(Node, R, EmitFixits);
+  }
+  return true;
+}
 
-if (auto *FD = dyn_cast(Node))
-  // `FunctionDecl->hasBody()` returns true if the function has a body
-  // somewhere defined.  But we want to know if this `Node` has a body
-  // child.  So we use `doesThisDeclarationHaveABody`:
-  ThisNodeHasBody = FD->doesThisDeclarationHaveABody();
-else
-  ThisNodeHasBody = Node->hasBody();
-if (ThisNodeHasBody) {
-  UnsafeBufferUsageReporter R(S);
+bool VisitBlockDecl(BlockDecl *Node) {
+  if (cast(Node)->isDependentContext())
+return true; // Not to analyze dependent decl
 
-  checkUnsafeBufferUsage(Node, R, EmitFixits);
-}
+  UnsafeBufferUsageReporter R(S);
+
+  checkUnsafeBufferUsage(Node, R, EmitFixits);
+  return true;
+}
+
+bool VisitObjCMethodDecl(ObjCMethodDecl *Node) {
+  if (cast(Node)->isDependentContext())
+return true; // Not to analyze dependent decl
+  if (Node->hasBody()) {
+UnsafeBufferUsageReporter R(S);
+
+checkUnsafeBufferUsage(Node, R, EmitFixits);
   }
-  // Continue to traverse descendants:
-  return RecursiveASTVisitor::TraverseDecl(Node);
+  return true;
 }
 
 bool TraverseStmt(Stmt *Node, DataRecursionQueue *Queue = nullptr) {
@@ -2382,7 +2392,9 @@
   // Emit unsafe buffer usage warnings and fixits.
   if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, 

[PATCH] D149922: Fix nullptr dereference found by Coverity static analysis tool

[Soumi Manna via Phabricator via cfe-commits]

Manna created this revision.
Manna added a reviewer: erichkeane.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

Reported by Coverity:

  In clang::​ASTContext::​hasUniqueObjectRepresentations(clang::​QualType, 
bool): Return value of function which returns null is dereferenced without 
checking.

  (Ty->isMemberPointerType()) {
//returned_null: getAs returns nullptr.
//var_assigned: Assigning: MPT = nullptr return value from getAs.
  const auto *MPT = Ty->getAs();

 //dereference: Dereferencing a pointer that might be nullptr MPT when 
calling getMemberPointerInfo. (The virtual call resolves to 
 ::ItaniumCXXABI::getMemberPointerInfo.)
  return !ABI->getMemberPointerInfo(MPT).HasPadding;
}

This patch uses castAs instead of getAs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149922

Files:
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2860,7 +2860,7 @@
 return true;
 
   if (Ty->isMemberPointerType()) {
-const auto *MPT = Ty->getAs();
+const auto *MPT = Ty->castAs();
 return !ABI->getMemberPointerInfo(MPT).HasPadding;
   }
 


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2860,7 +2860,7 @@
 return true;
 
   if (Ty->isMemberPointerType()) {
-const auto *MPT = Ty->getAs();
+const auto *MPT = Ty->castAs();
 return !ABI->getMemberPointerInfo(MPT).HasPadding;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added a comment.

@davidtgoldblatt - thanks for the report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/test/SemaTemplate/concepts-out-of-line-def.cpp:348
+
+namespace MultilevelTemplateWithPartialSpecialization {
+template 

(new tests)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133
 }
 
+Response HandlePartialClassTemplateSpec(

alexander-shaposhnikov wrote:
> HandlePartialClassTemplateSpec is from Erich's diff 
> (https://reviews.llvm.org/D147722)
@erichkeane : previously (see https://reviews.llvm.org/D147722) we were 
producing a real layer of arguments (if the depth is greater than 1), to the 
best of my knowledge this is incorrect since it'd trigger unexpected depth 
adjustment, we should produce retained layers instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov updated this revision to Diff 519711.
alexander-shaposhnikov added a comment.

Remove dead code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaTemplate/concepts-friends.cpp
  clang/test/SemaTemplate/concepts-out-of-line-def.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -816,3 +816,101 @@
 static_assert(Parent::TakesBinary::i == 0);
 }
 
+namespace TemplateInsideNonTemplateClass {
+template concept C = true;
+
+template auto L = [] U>() {};
+
+struct Q {
+  template U> friend constexpr auto decltype(L)::operator()() const;
+};
+
+template 
+concept C1 = false;
+
+struct Foo {
+  template 
+  struct Bar {};
+
+  template 
+requires(C1)
+  struct Bar;
+};
+
+Foo::Bar BarInstance;
+} // namespace TemplateInsideNonTemplateClass
+
+namespace GH61959 {
+template 
+concept C = (sizeof(T0) >= 4);
+
+template
+struct Orig { };
+
+template
+struct Orig {
+  template requires C
+  void f() { }
+
+  template requires true
+  void f() { }
+};
+
+template  struct Mod {};
+
+template 
+struct Mod {
+  template  requires C
+  constexpr static int f() { return 1; }
+
+  template  requires C
+  constexpr static int f() { return 2; }
+};
+
+static_assert(Mod::f() == 1);
+static_assert(Mod::f() == 2);
+
+template
+struct Outer {
+  template
+  struct Inner {};
+
+  template
+  struct Inner {
+template
+void foo()  requires C && C && C{}
+template
+void foo()  requires true{}
+  };
+};
+
+void bar() {
+  Outer::Inner I;
+  I.foo();
+}
+} // namespace GH61959
+
+
+namespace TemplateInsideTemplateInsideTemplate {
+template
+concept C1 = false;
+
+template 
+struct W0 {
+  template 
+  struct W1 {
+template 
+struct F {
+  enum { value = 1 };
+};
+
+template 
+  requires C1
+struct F {
+  enum { value = 2 };
+};
+  };
+};
+
+static_assert(W0<0>::W1<1>::F::value == 1);
+} // TemplateInsideTemplateInsideTemplate
Index: clang/test/SemaTemplate/concepts-out-of-line-def.cpp
===
--- clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -127,3 +127,275 @@
 static_assert(S::specialization("str") == SPECIALIZATION_REQUIRES);
 
 } // namespace multiple_template_parameter_lists
+
+static constexpr int CONSTRAINED_METHOD_1 = 1;
+static constexpr int CONSTRAINED_METHOD_2 = 2;
+
+namespace constrained_members {
+
+template 
+struct S {
+  template 
+  static constexpr int constrained_method();
+};
+
+template <>
+template 
+constexpr int S<1>::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  template T4>
+  static constexpr int constrained_method();
+};
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_2);
+
+} // namespace constrained members
+
+namespace constrained_members_of_nested_types {
+
+template 
+struct S {
+  struct Inner0 {
+struct Inner1 {
+  template 
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template <>
+template 
+constexpr int S<1>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  struct Inner0 {
+struct Inner1 {
+  template T4>
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2; }
+

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov updated this revision to Diff 519708.
alexander-shaposhnikov added a comment.

Add more tests,
Fix HandlePartialClassTemplateSpec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaTemplate/concepts-friends.cpp
  clang/test/SemaTemplate/concepts-out-of-line-def.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -816,3 +816,101 @@
 static_assert(Parent::TakesBinary::i == 0);
 }
 
+namespace TemplateInsideNonTemplateClass {
+template concept C = true;
+
+template auto L = [] U>() {};
+
+struct Q {
+  template U> friend constexpr auto decltype(L)::operator()() const;
+};
+
+template 
+concept C1 = false;
+
+struct Foo {
+  template 
+  struct Bar {};
+
+  template 
+requires(C1)
+  struct Bar;
+};
+
+Foo::Bar BarInstance;
+} // namespace TemplateInsideNonTemplateClass
+
+namespace GH61959 {
+template 
+concept C = (sizeof(T0) >= 4);
+
+template
+struct Orig { };
+
+template
+struct Orig {
+  template requires C
+  void f() { }
+
+  template requires true
+  void f() { }
+};
+
+template  struct Mod {};
+
+template 
+struct Mod {
+  template  requires C
+  constexpr static int f() { return 1; }
+
+  template  requires C
+  constexpr static int f() { return 2; }
+};
+
+static_assert(Mod::f() == 1);
+static_assert(Mod::f() == 2);
+
+template
+struct Outer {
+  template
+  struct Inner {};
+
+  template
+  struct Inner {
+template
+void foo()  requires C && C && C{}
+template
+void foo()  requires true{}
+  };
+};
+
+void bar() {
+  Outer::Inner I;
+  I.foo();
+}
+} // namespace GH61959
+
+
+namespace TemplateInsideTemplateInsideTemplate {
+template
+concept C1 = false;
+
+template 
+struct W0 {
+  template 
+  struct W1 {
+template 
+struct F {
+  enum { value = 1 };
+};
+
+template 
+  requires C1
+struct F {
+  enum { value = 2 };
+};
+  };
+};
+
+static_assert(W0<0>::W1<1>::F::value == 1);
+} // TemplateInsideTemplateInsideTemplate
Index: clang/test/SemaTemplate/concepts-out-of-line-def.cpp
===
--- clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -127,3 +127,275 @@
 static_assert(S::specialization("str") == SPECIALIZATION_REQUIRES);
 
 } // namespace multiple_template_parameter_lists
+
+static constexpr int CONSTRAINED_METHOD_1 = 1;
+static constexpr int CONSTRAINED_METHOD_2 = 2;
+
+namespace constrained_members {
+
+template 
+struct S {
+  template 
+  static constexpr int constrained_method();
+};
+
+template <>
+template 
+constexpr int S<1>::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  template T4>
+  static constexpr int constrained_method();
+};
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_2);
+
+} // namespace constrained members
+
+namespace constrained_members_of_nested_types {
+
+template 
+struct S {
+  struct Inner0 {
+struct Inner1 {
+  template 
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template <>
+template 
+constexpr int S<1>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  struct Inner0 {
+struct Inner1 {
+  template T4>
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { 

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

[Amara Emerson via Phabricator via cfe-commits]

aemerson added a comment.

I have another attempt at fixing this in D149918 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143624

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

[PATCH] D148924: [clang] Show error if defaulted comparions operator function is volatile or has ref-qualifier &&.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D148924#4318381 , @massberg wrote:

> I have checked the paper P2002R1 and as far as I can tell it is fully 
> implemented when this patch has landed.

So I read the paper but since the paper does not have examples for each change 
nor does it have any links to DRs it is not obvious without spending some time 
to verify that we support each section. Changes like these really should come 
with examples in the paper to demonstrate behavior before and after.

I searched in the code and found that @rsmith labelled P2002R0 changes with 
comments and so a cursory look at those changes side by side with the paper 
seems to indicate that we probably covered most of the cases but I am not 100% 
sure. There is test coverage added with those changes as well but again 
relating each test to specific paper section is not trivial, at least for me.

The paper also uses the term constexpr compatible which has been replaced with 
constexpr suitable and not sure if that has any practical effect here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D146090#4319737 , @Mordante wrote:

> In D146090#4319685 , @shafik wrote:
>
>> Landed fix 7887af027ee5eff27bbc953074726ab8d9d9f592 
>> 
>>
>> There was also 058f04ea7dcbafbeed271fa75ee65e41409b4479 
>>  and 
>> b323b407f76d22bfc08b1430f7952c03eb504288 
>> 
>
> Thanks for fixing these!

No worries, unfortunate timing on my part that our commits crossed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: cmake/Modules/GetClangResourceDir.cmake:13
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(ret_dir bin/${CLANG_RESOURCE_DIR})
+  else()

tstellar wrote:
> paperchalice wrote:
> > tstellar wrote:
> > > Why is the bin prefix here?
> > See 
> > https://clang.llvm.org/doxygen/classclang_1_1driver_1_1Driver.html#acda8dfdf4f80efa84df98157e1779152
> > `GetResourcesPath` will return `/lib/` when 
> > `CLANG_RESOURCE_DIR` is empty, `/bin/CLANG_RESOURCE_DIR` otherwise.
> > 
> `GetResourcesPath` calls `sys::path::parent_path` twice on the path to the 
> clang executable which is going to give you `/usr ` on most Linux systems, so 
> it's returning `/CLANG_RESOURCE_DIR` not 
> `/bin/CLANG_RESOURCE_DIR`.
Sorry, you are correct.  I was looking at the wrong branch.  It's really 
strange that it does that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: cmake/Modules/GetClangResourceDir.cmake:13
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(ret_dir bin/${CLANG_RESOURCE_DIR})
+  else()

paperchalice wrote:
> tstellar wrote:
> > Why is the bin prefix here?
> See 
> https://clang.llvm.org/doxygen/classclang_1_1driver_1_1Driver.html#acda8dfdf4f80efa84df98157e1779152
> `GetResourcesPath` will return `/lib/` when 
> `CLANG_RESOURCE_DIR` is empty, `/bin/CLANG_RESOURCE_DIR` otherwise.
> 
`GetResourcesPath` calls `sys::path::parent_path` twice on the path to the 
clang executable which is going to give you `/usr ` on most Linux systems, so 
it's returning `/CLANG_RESOURCE_DIR` not 
`/bin/CLANG_RESOURCE_DIR`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added a comment.

Reverted in 
https://github.com/llvm/llvm-project/commit/3b9ed6e5323176550925f3b0a2c50ced1b61438d,
it'll take time to investigate this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[clang] 3b9ed6e - Revert "[Clang][Sema] Fix comparison of constraint expressions"

[Alexander Shaposhnikov via cfe-commits]

Author: Alexander Shaposhnikov
Date: 2023-05-05T00:02:26Z
New Revision: 3b9ed6e5323176550925f3b0a2c50ced1b61438d

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

LOG: Revert "[Clang][Sema] Fix comparison of constraint expressions"

This reverts commit 3a540229341e3c8dc6d8ee61309eafaf943ea254.
A new regression is discovered and needs to be investigated.

Added: 


Modified: 
clang/include/clang/AST/DeclTemplate.h
clang/include/clang/Sema/Template.h
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaTemplate/concepts-friends.cpp
clang/test/SemaTemplate/concepts-out-of-line-def.cpp
clang/test/SemaTemplate/concepts.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 7cd505218f2b9..3677335fa176f 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -2309,15 +2309,9 @@ class ClassTemplateDecl : public 
RedeclarableTemplateDecl {
 return static_cast(RedeclarableTemplateDecl::getCommonPtr());
   }
 
-  void setCommonPtr(Common *C) {
-RedeclarableTemplateDecl::Common = C;
-  }
-
 public:
-
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
-  friend class TemplateDeclInstantiator;
 
   /// Load any lazily-loaded specializations from the external source.
   void LoadLazySpecializations() const;

diff  --git a/clang/include/clang/Sema/Template.h 
b/clang/include/clang/Sema/Template.h
index 1de2cc6917b42..48e8b78311e12 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -232,21 +232,9 @@ enum class TemplateSubstitutionKind : char {
 /// Replaces the current 'innermost' level with the provided argument list.
 /// This is useful for type deduction cases where we need to get the entire
 /// list from the AST, but then add the deduced innermost list.
-void replaceInnermostTemplateArguments(Decl *AssociatedDecl, ArgList Args) 
{
-  assert((!TemplateArgumentLists.empty() || NumRetainedOuterLevels) &&
- "Replacing in an empty list?");
-
-  if (!TemplateArgumentLists.empty()) {
-assert((TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ||
-TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ==
-AssociatedDecl) &&
-   "Trying to change incorrect declaration?");
-TemplateArgumentLists[0].Args = Args;
-  } else {
---NumRetainedOuterLevels;
-TemplateArgumentLists.push_back(
-{{AssociatedDecl, /*Final=*/false}, Args});
-  }
+void replaceInnermostTemplateArguments(ArgList Args) {
+  assert(TemplateArgumentLists.size() > 0 && "Replacing in an empty 
list?");
+  TemplateArgumentLists[0].Args = Args;
 }
 
 /// Add an outermost level that we are not substituting. We have no

diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 1126c2c517fe4..7aa06b615fec2 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -722,7 +722,7 @@ CalculateTemplateDepthForConstraints(Sema , const 
NamedDecl *ND,
   ND, /*Final=*/false, /*Innermost=*/nullptr, /*RelativeToPrimary=*/true,
   /*Pattern=*/nullptr,
   /*ForConstraintInstantiation=*/true, SkipForSpecialization);
-  return MLTAL.getNumLevels();
+  return MLTAL.getNumSubstitutedLevels();
 }
 
 namespace {
@@ -753,44 +753,27 @@ namespace {
   };
 } // namespace
 
-static const Expr *SubstituteConstraintExpression(Sema , const NamedDecl *ND,
-  const Expr *ConstrExpr) {
-  MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
-  ND, /*Final=*/false, /*Innermost=*/nullptr,
-  /*RelativeToPrimary=*/true,
-  /*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
-  /*SkipForSpecialization*/ false);
-  if (MLTAL.getNumSubstitutedLevels() == 0)
-return ConstrExpr;
-
-  Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
-  std::optional ThisScope;
-  if (auto *RD = dyn_cast(ND->getDeclContext()))
-ThisScope.emplace(S, const_cast(RD), Qualifiers());
-  ExprResult SubstConstr =
-  S.SubstConstraintExpr(const_cast(ConstrExpr), MLTAL);
-  if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
-return nullptr;
-  return SubstConstr.get();
-}
-
 bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
  const Expr *OldConstr,
  const NamedDecl *New,
   

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Junchang Liu via Phabricator via cfe-commits]

paperchalice added inline comments.



Comment at: cmake/Modules/GetClangResourceDir.cmake:13
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(ret_dir bin/${CLANG_RESOURCE_DIR})
+  else()

tstellar wrote:
> Why is the bin prefix here?
See 
https://clang.llvm.org/doxygen/classclang_1_1driver_1_1Driver.html#acda8dfdf4f80efa84df98157e1779152
`GetResourcesPath` will return `/lib/` when 
`CLANG_RESOURCE_DIR` is empty, `/bin/CLANG_RESOURCE_DIR` otherwise.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[PATCH] D149904: Generic selection expressions that accept a type operand

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/AST/Expr.h:5686
   /// if and only if the generic selection expression is result-dependent.
-  unsigned NumAssocs, ResultIndex;
+  unsigned NumAssocs, ResultIndex, IsExprPredicate;
   enum : unsigned {

I realize this is a rarely used type, but should these be bitfielded somehow?  
Particularly now that you're adding a 'bool' field?



Comment at: clang/include/clang/AST/Expr.h:5691
 
+  unsigned getIndexOfControllingExpression() const {
+// If controlled by an expression, the first offset into the Stmt *

What is the value of these functions, if they only return 0 or 1?



Comment at: clang/include/clang/AST/Expr.h:5712
+// predicate expression.
+return (int)isExprPredicate();
+  }

Should these have asserts also?  Wouldn't saying this is index '0' be incorrect 
here if t his is the typePredicate?



Comment at: clang/include/clang/Parse/Parser.h:2551
+  bool isTypeIdForGenericSelection() {
+bool isAmbiguous;
+if (getLangOpts().CPlusPlus)

slight preference to put this declaration in the 'if' condition below, the 
split in location confused me for a bit trying to figure out where it was being 
used.



Comment at: clang/lib/Sema/SemaExpr.cpp:19851
+bool IsExpr = GSE->isExprPredicate();
+if (IsExpr)
+  ExOrTy = GSE->getControllingExpr();

I find myself wondering if a `getControllingOperand` here is valuable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149904

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

[PATCH] D149612: [Sema] avoid merge error type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2582
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
-T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
+if (getLangOpts().CPlusPlus) {
+  T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals,

So I'm still not sure this is the 'right' away about it.  I think we should 
instead properly handle the `containsErrors` case and just always create a 
non-dependent sized array, except with the `RecoveryExpr` having the correct 
type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

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

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Still needs a test..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: pmatos, asb, wingo, ecnelises, sunfish, 
jgravelle-google, dschuff.
Herald added a project: All.
sbc100 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, aheejin.
Herald added a project: clang.

This flag causes wasm-ld to preserve features section even in the face
of `--strip-all`.  This is useful for both clang (which can run
wasm-opt) after linking, and emcc (which performs a bunch of post-link
work).

Fixes: https://github.com/llvm/llvm-project/issues/60613
Fixes: https://github.com/llvm/llvm-project/issues/55781


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149917

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  lld/wasm/Config.h
  lld/wasm/Driver.cpp
  lld/wasm/Options.td
  lld/wasm/SyntheticSections.h

Index: lld/wasm/SyntheticSections.h
===
--- lld/wasm/SyntheticSections.h
+++ lld/wasm/SyntheticSections.h
@@ -408,7 +408,8 @@
   TargetFeaturesSection()
   : SyntheticSection(llvm::wasm::WASM_SEC_CUSTOM, "target_features") {}
   bool isNeeded() const override {
-return !config->stripAll && features.size() > 0;
+return features.size() > 0 &&
+   (!config->stripAll || config->preserveFeatures);
   }
   void writeBody() override;
 
Index: lld/wasm/Options.td
===
--- lld/wasm/Options.td
+++ lld/wasm/Options.td
@@ -132,6 +132,8 @@
 
 def strip_debug: F<"strip-debug">, HelpText<"Strip debugging information">;
 
+def preserve_features: F<"preserve-features">, HelpText<"Preserve features section even when --strip-all is given.  This is useful for compiler drivers such as clang or emcc that depend on the features section for post-link processing.">;
+
 defm threads
 : Eq<"threads", "Number of threads. '1' disables multi-threading. By "
 "default all available hardware threads are used">;
Index: lld/wasm/Driver.cpp
===
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -476,6 +476,7 @@
   args.hasFlag(OPT_merge_data_segments, OPT_no_merge_data_segments,
!config->relocatable);
   config->pie = args.hasFlag(OPT_pie, OPT_no_pie, false);
+  config->preserveFeatures = args.hasArg(OPT_preserve_features);
   config->printGcSections =
   args.hasFlag(OPT_print_gc_sections, OPT_no_print_gc_sections, false);
   config->saveTemps = args.hasArg(OPT_save_temps);
Index: lld/wasm/Config.h
===
--- lld/wasm/Config.h
+++ lld/wasm/Config.h
@@ -57,6 +57,7 @@
   std::optional is64;
   bool mergeDataSegments;
   bool pie;
+  bool preserveFeatures;
   bool printGcSections;
   bool relocatable;
   bool saveTemps;
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -126,14 +126,25 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
+  // When optimizing, if wasm-opt is available, run it.
+  std::string WasmOptPath;
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+WasmOptPath = ToolChain.GetProgramPath("wasm-opt");
+if (WasmOptPath == "wasm-opt") {
+  WasmOptPath = {};
+}
+  }
+
+  if (!WasmOptPath.empty()) {
+CmdArgs.push_back("--preserve-features");
+  }
+
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Linker, CmdArgs, Inputs, Output));
 
-  // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-auto WasmOptPath = ToolChain.GetProgramPath("wasm-opt");
-if (WasmOptPath != "wasm-opt") {
+if (!WasmOptPath.empty()) {
   StringRef OOpt = "s";
   if (A->getOption().matches(options::OPT_O4) ||
   A->getOption().matches(options::OPT_Ofast))
@@ -145,13 +156,13 @@
 
   if (OOpt != "0") {
 const char *WasmOpt = Args.MakeArgString(WasmOptPath);
-ArgStringList CmdArgs;
-CmdArgs.push_back(Output.getFilename());
-CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
-CmdArgs.push_back("-o");
-CmdArgs.push_back(Output.getFilename());
+ArgStringList OptArgs;
+OptArgs.push_back(Output.getFilename());
+OptArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+OptArgs.push_back("-o");
+OptArgs.push_back(Output.getFilename());
 C.addCommand(std::make_unique(
-JA, *this, ResponseFileSupport::AtFileCurCP(), WasmOpt, CmdArgs,
+JA, *this, ResponseFileSupport::AtFileCurCP(), WasmOpt, OptArgs,
 Inputs, Output));
   }
 }

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen planned changes to this revision.
samitolvanen added a comment.

Uploaded D149915  to remove the arch-specific 
KCFI passes. I'll update this patch once we clean that up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148385

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

[PATCH] D149912: [clangd] downgrade missing-includes diagnostic to Information level

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This change is just one option to be evaluated for making these diagnostics 
more user-friendly.

Mostly sending the patch now to attach some screenshots...
Effect in VSCode (blue underline, visible in problems view, filterable)
F27332814: image.png 

"Hint" severity in VSCode (ellipsis, not visible in problems view)
F27332825: image.png 
(This seems worse: the three-dots rendering does suggest "action here" rather 
than "issue here", and not being *able* to show these in the problems view 
seems unfortunate too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149912

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

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329
+static void
+builtinTransferScopeEnd(const CFGScopeEnd ,
+TypeErasedDataflowAnalysisState ) {

mboehme wrote:
> xazax.hun wrote:
> > mboehme wrote:
> > > mboehme wrote:
> > > > xazax.hun wrote:
> > > > > I think one question is whether we are interested in ScopeEnd or 
> > > > > LifetimeEnd, see the discussions in https://reviews.llvm.org/D15031 
> > > > > and https://reviews.llvm.org/D16403, specifically Devin's comment: 
> > > > > 
> > > > > >>! In D16403#799926, @dcoughlin wrote:
> > > > > >> @dcoughlin As a reviewer of both patches - could you tell us 
> > > > > >> what's the difference between them? And how are we going to 
> > > > > >> resolve this issue?
> > > > > > 
> > > > > > These two patches are emitting markers in the CFG for different 
> > > > > > things.
> > > > > > 
> > > > > > Here is how I would characterize the difference between the two 
> > > > > > patches.
> > > > > > 
> > > > > > - Despite its name, https://reviews.llvm.org/D15031, is really 
> > > > > > emitting markers for when the lifetime of a C++ object in an 
> > > > > > automatic variable ends.  For C++ objects with non-trivial 
> > > > > > destructors, this point is when the destructor is called. At this 
> > > > > > point the storage for the variable still exists, but what you can 
> > > > > > do with that storage is very restricted by the language because its 
> > > > > > contents have been destroyed. Note that even with the contents of 
> > > > > > the object have been destroyed, it is still meaningful to, say, 
> > > > > > take the address of the variable and construct a new object into it 
> > > > > > with a placement new. In other words, the same variable can have 
> > > > > > different objects, with different lifetimes, in it at different 
> > > > > > program points.
> > > > > > 
> > > > > > - In contrast, the purpose of this patch 
> > > > > > (https://reviews.llvm.org/D16403) is to add markers for when the 
> > > > > > storage duration for the variable begins and ends (this is, when 
> > > > > > the storage exists). Once the storage duration has ended, you can't 
> > > > > > placement new into the variables address, because another variable 
> > > > > > may already be at that address.
> > > > > > 
> > > > > > I don't think there is an "issue" to resolve here.  We should make 
> > > > > > sure the two patches play nicely with each other, though. In 
> > > > > > particular, we should make sure that the markers for when lifetime 
> > > > > > ends and when storage duration ends are ordered correctly.
> > > > > 
> > > > > 
> > > > > What I wanted to add, I wonder if we might not get ScopeEnd event for 
> > > > > temporaries and there might be other differences. The Clang 
> > > > > implementation of P1179 used LifetimeEnd instead of ScopeEnd, and I 
> > > > > believe probably most clients are more interested in LifetimeEnd 
> > > > > events rather than ScopeEnd.
> > > > > 
> > > > > I think I understand why you went with ScopeEnd for this specific 
> > > > > problem, but to avoid the confusion from having both in the Cfg 
> > > > > (because I anticipate someone will need LifetimeEnd at some point), I 
> > > > > wonder if we can make this work with LifetimeEnd. 
> > > > Hm, thanks for bringing it up.
> > > > 
> > > > Conincidentally, I realized while chasing another issue that 
> > > > `CFGScopeEnd` isn't the right construct here. I assumed that we would 
> > > > get a `CFGScopeEnd` for every variable, but I now realize that we only 
> > > > get a `CFGScopeEnd` for the first variable in the scope.
> > > > 
> > > > So `CFGLifetimeEnds` definitely looks like the right construct to use 
> > > > here, and indeed it's what I originally tried to use. Unfortuately, 
> > > > `CFG::BuildOptions::AddLifetime` cannot be used at the same time as 
> > > > `AddImplicitDtors`, which we already use. We don't actually need the 
> > > > `CFGElement`s added by `AddImplicitDtors`, but we do need 
> > > > `AddTemporaryDtors`, and that in turn can only be turned on if 
> > > > `AddImplicitDtors` is turned on.
> > > > 
> > > > It looks as if I will need to break one of these constraints. It looks 
> > > > as if the constraint that is easiest to break is that `AddLifetime` 
> > > > currently cannot be used at the same time as `AddImplicitDtors`. I'm 
> > > > not sure why this constraint currently exists (I'd appreciate any 
> > > > insights you or others may have), but I suspect it's because it's hard 
> > > > to ensure that the `CFGElement`s added by `AddImplicitDtors` are 
> > > > ordered correctly with respect to the `CFGLifetimeEnds` elements.
> > > > 
> > > > Anyway, this requires a change to `CFG` one way or another. I'll work 
> > > > on that next. I'll then make the required changes to this patch and 
> > > > will ask for another look before submitting.
> > > I've 

[PATCH] D149912: [clangd] downgrade missing-includes diagnostic to Information level

[Sam McCall via Phabricator via cfe-commits]

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

In practice, a Warning on every occurrence is very unpopular, even on a codebase
with clear rules about direct inclusion & moderately good compliance.

This change has various practical effects (in vscode for concreteness):

- makes the diagnostic decoration less striking (blue vs yellow)
- makes these diagnostics visually distinct from others when reading
- causes these diagnostics to sort last in the "problems" view
- allows these diagnostics to be easily filtered from the "problems" view


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149912

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/test/include-cleaner-batch-fix.test


Index: clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
===
--- clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -44,7 +44,7 @@
 # CHECK-NEXT: "line": 2
 # CHECK-NEXT:   }
 # CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "severity": 3,
 # CHECK-NEXT: "source": "clangd"
 # CHECK-NEXT:   },
 # CHECK-NEXT:   {
@@ -60,7 +60,7 @@
 # CHECK-NEXT: "line": 2
 # CHECK-NEXT:   }
 # CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "severity": 3,
 # CHECK-NEXT: "source": "clangd"
 # CHECK-NEXT:   },
 # CHECK-NEXT:   {
@@ -112,7 +112,7 @@
 # CHECK-NEXT: "version": 0
 # CHECK-NEXT:   }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///simple.cpp"},"range":{"start":{"line":2,"character":1},"end":{"line":2,"character":4}},"context":{"diagnostics":[{"range":{"start":
 {"line": 2, "character": 1}, "end": {"line": 2, "character": 
4}},"severity":2,"message":"No header providing \"Foo\" is directly included 
(fixes available)", "code": "missing-includes", "source": "clangd"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///simple.cpp"},"range":{"start":{"line":2,"character":1},"end":{"line":2,"character":4}},"context":{"diagnostics":[{"range":{"start":
 {"line": 2, "character": 1}, "end": {"line": 2, "character": 
4}},"severity":3,"message":"No header providing \"Foo\" is directly included 
(fixes available)", "code": "missing-includes", "source": "clangd"}]}}}
 #  CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -218,7 +218,23 @@
 D.Source = Diag::DiagSource::Clangd;
 D.File = AST.tuPath();
 D.InsideMainFile = true;
-D.Severity = DiagnosticsEngine::Warning;
+// We avoid the "warning" severity here in favor of LSP's "information".
+//
+// Users treat most warnings on code being edited as high-priority.
+// They don't think of include cleanups the same way: they want to edit
+// lines with existing violations without fixing them.
+// Diagnostics at the same level tend to be visually indistinguishable,
+// and a few missing includes can cause many diagnostics.
+// Marking these as "information" leaves them visible, but less intrusive.
+//
+// (These concerns don't apply to unused #include warnings: these are 
fewer,
+// they appear on infrequently-edited lines with few other warnings, and
+// the 'Unneccesary' tag often result in a different rendering)
+//
+// Usually clang's "note" severity usually has special semantics, being
+// translated into LSP RelatedInformation of a parent diagnostic.
+// But not here: these aren't processed by clangd's DiagnosticConsumer.
+D.Severity = DiagnosticsEngine::Note;
 D.Range = clangd::Range{
 offsetToPosition(Code,
  SymbolWithMissingInclude.SymRefRange.beginOffset()),


Index: clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
===
--- clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -44,7 +44,7 @@
 # CHECK-NEXT: "line": 2
 # CHECK-NEXT:   }
 # CHECK-NEXT: },
-# CHECK-NEXT: "severity": 2,
+# CHECK-NEXT: "severity": 3,
 # CHECK-NEXT: "source": "clangd"
 # CHECK-NEXT:   },
 # CHECK-NEXT:   {
@@ -60,7 +60,7 @@
 # CHECK-NEXT: "line": 2
 # 

[PATCH] D149612: [Sema] avoid merge error type

[Congcong Cai via Phabricator via cfe-commits]

HerrCai0907 updated this revision to Diff 519675.
HerrCai0907 added a comment.

use nullpt as SizeExpr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/merge-decls.c


Index: clang/test/Sema/merge-decls.c
===
--- clang/test/Sema/merge-decls.c
+++ clang/test/Sema/merge-decls.c
@@ -91,3 +91,7 @@
   int x[5];
   test7_f(); // expected-warning {{incompatible pointer types passing 'int 
(*)[5]' to parameter of type 'int (*)[10]}}
 }
+
+char d;
+char x[sizeof(d.data) == 8]; // expected-error {{member reference base type 
'char' is not a structure or union}}
+char x[sizeof(d.data) == 4]; // expected-error {{member reference base type 
'char' is not a structure or union}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2579,7 +2579,13 @@
   T = Context.getIncompleteArrayType(T, ASM, Quals);
 }
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
-T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
+if (getLangOpts().CPlusPlus) {
+  T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals,
+ Brackets);
+} else {
+  // DependentSizedArrayType cannot be handled by C
+  T = Context.getConstantArrayType(T, ConstVal, nullptr, ASM, Quals);
+}
   } else {
 ExprResult R =
 checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -369,6 +369,8 @@
   (`#62207 `_)
 - Fix lambdas and other anonymous function names not respecting 
``-fdebug-prefix-map``
   (`#62192 `_)
+- Fix crash when redefine variant with invalid type as another invalid type.
+  (`#62447 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/Sema/merge-decls.c
===
--- clang/test/Sema/merge-decls.c
+++ clang/test/Sema/merge-decls.c
@@ -91,3 +91,7 @@
   int x[5];
   test7_f(); // expected-warning {{incompatible pointer types passing 'int (*)[5]' to parameter of type 'int (*)[10]}}
 }
+
+char d;
+char x[sizeof(d.data) == 8]; // expected-error {{member reference base type 'char' is not a structure or union}}
+char x[sizeof(d.data) == 4]; // expected-error {{member reference base type 'char' is not a structure or union}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2579,7 +2579,13 @@
   T = Context.getIncompleteArrayType(T, ASM, Quals);
 }
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
-T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
+if (getLangOpts().CPlusPlus) {
+  T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals,
+ Brackets);
+} else {
+  // DependentSizedArrayType cannot be handled by C
+  T = Context.getConstantArrayType(T, ConstVal, nullptr, ASM, Quals);
+}
   } else {
 ExprResult R =
 checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -369,6 +369,8 @@
   (`#62207 `_)
 - Fix lambdas and other anonymous function names not respecting ``-fdebug-prefix-map``
   (`#62192 `_)
+- Fix crash when redefine variant with invalid type as another invalid type.
+  (`#62447 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149612: [Sema] avoid merge error type

[Congcong Cai via Phabricator via cfe-commits]

HerrCai0907 updated this revision to Diff 519671.
HerrCai0907 added a comment.

getConstantArrayType instal QualType()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/merge-decls.c


Index: clang/test/Sema/merge-decls.c
===
--- clang/test/Sema/merge-decls.c
+++ clang/test/Sema/merge-decls.c
@@ -91,3 +91,7 @@
   int x[5];
   test7_f(); // expected-warning {{incompatible pointer types passing 'int 
(*)[5]' to parameter of type 'int (*)[10]}}
 }
+
+char d;
+char x[sizeof(d.data) == 8]; // expected-error {{member reference base type 
'char' is not a structure or union}}
+char x[sizeof(d.data) == 4]; // expected-error {{member reference base type 
'char' is not a structure or union}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2579,7 +2579,13 @@
   T = Context.getIncompleteArrayType(T, ASM, Quals);
 }
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
-T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
+if (getLangOpts().CPlusPlus) {
+  T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals,
+ Brackets);
+} else {
+  // DependentSizedArrayType cannot be handled by C
+  T = Context.getConstantArrayType(T, ConstVal, ArraySize, ASM, Quals);
+}
   } else {
 ExprResult R =
 checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -369,6 +369,8 @@
   (`#62207 `_)
 - Fix lambdas and other anonymous function names not respecting 
``-fdebug-prefix-map``
   (`#62192 `_)
+- Fix crash when redefine variant with invalid type as another invalid type.
+  (`#62447 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/Sema/merge-decls.c
===
--- clang/test/Sema/merge-decls.c
+++ clang/test/Sema/merge-decls.c
@@ -91,3 +91,7 @@
   int x[5];
   test7_f(); // expected-warning {{incompatible pointer types passing 'int (*)[5]' to parameter of type 'int (*)[10]}}
 }
+
+char d;
+char x[sizeof(d.data) == 8]; // expected-error {{member reference base type 'char' is not a structure or union}}
+char x[sizeof(d.data) == 4]; // expected-error {{member reference base type 'char' is not a structure or union}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2579,7 +2579,13 @@
   T = Context.getIncompleteArrayType(T, ASM, Quals);
 }
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
-T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
+if (getLangOpts().CPlusPlus) {
+  T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals,
+ Brackets);
+} else {
+  // DependentSizedArrayType cannot be handled by C
+  T = Context.getConstantArrayType(T, ConstVal, ArraySize, ASM, Quals);
+}
   } else {
 ExprResult R =
 checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -369,6 +369,8 @@
   (`#62207 `_)
 - Fix lambdas and other anonymous function names not respecting ``-fdebug-prefix-map``
   (`#62192 `_)
+- Fix crash when redefine variant with invalid type as another invalid type.
+  (`#62447 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[David Goldblatt via Phabricator via cfe-commits]

davidtgoldblatt added a comment.

This version of the commit also introduces some breakages; as before I'm not 
sure if it's the code or the diff that's incorrect.

Repro:

  enum class Enum { E1 };
  
  template 
  inline constexpr bool some_concept = true;
  
  template 
  struct S {
template 
requires some_concept
void func(const T2 &);
  };
  
  template 
  struct S {
template 
requires some_concept
void func(const T2 &);
  };
  
  template 
  template 
  requires some_concept
  inline void S::func(const T2 &) {}

Error:

  repro.cpp:23:30: error: out-of-line definition of 'func' does not match any 
declaration in 'S'
  inline void S::func(const T2 &) {}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1121-1138
+static unsigned getNumDisplayWidth(unsigned N) {
+  if (N < 10)
+return 1;
+  if (N < 100)
+return 2;
+  if (N < 1'000)
+return 3;

kwk wrote:
> This function screamed at me to be generalized so I gave it a try: 
> https://gist.github.com/kwk/7e408065ea291e49fea4a83cf90a9cdf
I don't think I want floating point arithmetic in my compiler... Something like:

```
unsigned L, M;
for (L = 1U, M = 10U; N >= M && M != ~0U; ++L)
M = (M > ((~0U) / 10U)) ? (~0U) : (M * 10U);

return (L);
```

is the generalised form (without all the redundant parentheses I added during 
debugging).


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

https://reviews.llvm.org/D147875

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

[PATCH] D149514: Check if First argument in _builtin_assume_aligned_ is of pointer type

[Rishabh Bali via Phabricator via cfe-commits]

Ris-Bali added a comment.

I have made the requested changes. Please do let me know if other changes are 
required. Also I would like to thank everyone for their help and guidance.


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

https://reviews.llvm.org/D149514

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

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I retract my objection. I forgot that from clang-format's perspective, `#else` 
blocks in separate functions have the same indent and nesting level, and thus 
the same "scope", so the alignment issues may persist there. You will see 
misalignment in `#else` blocks over an entire source file whether you keep or 
revert this fix (i.e. pick your poison.) I suppose based on precedent we keep 
the misalignments I wanted to fix since I introduced misalignments in other 
cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134042

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

[PATCH] D149906: [Clang] Update release notes

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov updated this revision to Diff 519660.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149906

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -78,8 +78,13 @@
 
 C++20 Feature Support
 ^
-- Support for out-of-line definitions of constrained templates has been 
improved.
-  This partially fixes `#49620 
`_.
+- Implemented the rule introduced by `CA104 `_  for 
comparison of
+  constraint-expressions. Improved support for out-of-line definitions of 
constrained templates.
+  This fixes:
+  `#49620 `_,
+  `#60231 `_,
+  `#61414 `_,
+  `#61809 `_.
 - Lambda templates with a requires clause directly after the template 
parameters now parse
   correctly if the requires clause consists of a variable with a dependent 
type.
   (`#61278 `_)


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -78,8 +78,13 @@
 
 C++20 Feature Support
 ^
-- Support for out-of-line definitions of constrained templates has been improved.
-  This partially fixes `#49620 `_.
+- Implemented the rule introduced by `CA104 `_  for comparison of
+  constraint-expressions. Improved support for out-of-line definitions of constrained templates.
+  This fixes:
+  `#49620 `_,
+  `#60231 `_,
+  `#61414 `_,
+  `#61809 `_.
 - Lambda templates with a requires clause directly after the template parameters now parse
   correctly if the requires clause consists of a variable with a dependent type.
   (`#61278 `_)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149650: Give NullabilityKind a printing operator<

[Sam McCall 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 rG0a532207b869: Give NullabilityKind a printing 
operator (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

Files:
  clang/include/clang/Basic/Specifiers.h
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/test/SemaObjC/nullable-result.m

Index: clang/test/SemaObjC/nullable-result.m
===
--- clang/test/SemaObjC/nullable-result.m
+++ clang/test/SemaObjC/nullable-result.m
@@ -25,9 +25,9 @@
 }
 
 void test_dup(void) {
-  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate nullability specifier _Nullable_result}}
-  id _Nullable _Nullable_result b; // expected-error{{nullability specifier _Nullable_result conflicts with existing specifier '_Nullable'}}
-  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier '_Nonnull' conflicts with existing specifier _Nullable_result}}
+  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate nullability specifier '_Nullable_result'}}
+  id _Nullable _Nullable_result b; // expected-error{{nullability specifier '_Nullable_result' conflicts with existing specifier '_Nullable'}}
+  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier '_Nonnull' conflicts with existing specifier '_Nullable_result'}}
 }
 
 @interface NoContextSensitive
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -849,6 +849,20 @@
   llvm_unreachable("Unknown nullability kind.");
 }
 
+llvm::raw_ostream <<(llvm::raw_ostream , NullabilityKind NK) {
+  switch (NK) {
+  case NullabilityKind::NonNull:
+return OS << "NonNull";
+  case NullabilityKind::Nullable:
+return OS << "Nullable";
+  case NullabilityKind::NullableResult:
+return OS << "NullableResult";
+  case NullabilityKind::Unspecified:
+return OS << "Unspecified";
+  }
+  llvm_unreachable("Unknown nullability kind.");
+}
+
 diag::kind
 IdentifierTable::getFutureCompatDiagKind(const IdentifierInfo ,
  const LangOptions ) {
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -43,28 +43,12 @@
 
 const StreamingDiagnostic ::operator<<(const StreamingDiagnostic ,
  DiagNullabilityKind nullability) {
-  StringRef string;
-  switch (nullability.first) {
-  case NullabilityKind::NonNull:
-string = nullability.second ? "'nonnull'" : "'_Nonnull'";
-break;
-
-  case NullabilityKind::Nullable:
-string = nullability.second ? "'nullable'" : "'_Nullable'";
-break;
-
-  case NullabilityKind::Unspecified:
-string = nullability.second ? "'null_unspecified'" : "'_Null_unspecified'";
-break;
-
-  case NullabilityKind::NullableResult:
-assert(!nullability.second &&
-   "_Nullable_result isn't supported as context-sensitive keyword");
-string = "_Nullable_result";
-break;
-  }
-
-  DB.AddString(string);
+  DB.AddString(
+  ("'" +
+   getNullabilitySpelling(nullability.first,
+  /*isContextSensitive=*/nullability.second) +
+   "'")
+  .str());
   return DB;
 }
 
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -19,6 +19,9 @@
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
 
+namespace llvm {
+class raw_ostream;
+} // namespace llvm
 namespace clang {
 
   /// Define the meaning of possible values of the kind in ExplicitSpecifier.
@@ -333,6 +336,8 @@
 // parameters are assumed to only get null on error.
 NullableResult,
   };
+  /// Prints human-readable debug representation.
+  llvm::raw_ostream <<(llvm::raw_ostream&, NullabilityKind);
 
   /// Return true if \p L has a weaker nullability annotation than \p R. The
   /// ordering is: Unspecified < Nullable < NonNull.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0a53220 - Give NullabilityKind a printing operator<

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-05-04T23:24:51+02:00
New Revision: 0a532207b8696d81e46017f444bd2257347f129b

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

LOG: Give NullabilityKind a printing operator<<

This is more useful for debug/test than getNullabilitySpelling:
 - default form has uglifying underscores
 - non-default form crashes on NullableResult
 - both return unhelpfully verbose strings for Unspecified
 - operator<< works with gtest, formatv, etc

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

Added: 


Modified: 
clang/include/clang/Basic/Specifiers.h
clang/lib/Basic/Diagnostic.cpp
clang/lib/Basic/IdentifierTable.cpp
clang/test/SemaObjC/nullable-result.m

Removed: 




diff  --git a/clang/include/clang/Basic/Specifiers.h 
b/clang/include/clang/Basic/Specifiers.h
index a8c35fed9997e..06279a016a507 100644
--- a/clang/include/clang/Basic/Specifiers.h
+++ b/clang/include/clang/Basic/Specifiers.h
@@ -19,6 +19,9 @@
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
 
+namespace llvm {
+class raw_ostream;
+} // namespace llvm
 namespace clang {
 
   /// Define the meaning of possible values of the kind in ExplicitSpecifier.
@@ -333,6 +336,8 @@ namespace clang {
 // parameters are assumed to only get null on error.
 NullableResult,
   };
+  /// Prints human-readable debug representation.
+  llvm::raw_ostream <<(llvm::raw_ostream&, NullabilityKind);
 
   /// Return true if \p L has a weaker nullability annotation than \p R. The
   /// ordering is: Unspecified < Nullable < NonNull.

diff  --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 3474acbced195..b2106b0eb6611 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -43,28 +43,12 @@ using namespace clang;
 
 const StreamingDiagnostic ::operator<<(const StreamingDiagnostic ,
  DiagNullabilityKind nullability) {
-  StringRef string;
-  switch (nullability.first) {
-  case NullabilityKind::NonNull:
-string = nullability.second ? "'nonnull'" : "'_Nonnull'";
-break;
-
-  case NullabilityKind::Nullable:
-string = nullability.second ? "'nullable'" : "'_Nullable'";
-break;
-
-  case NullabilityKind::Unspecified:
-string = nullability.second ? "'null_unspecified'" : "'_Null_unspecified'";
-break;
-
-  case NullabilityKind::NullableResult:
-assert(!nullability.second &&
-   "_Nullable_result isn't supported as context-sensitive keyword");
-string = "_Nullable_result";
-break;
-  }
-
-  DB.AddString(string);
+  DB.AddString(
+  ("'" +
+   getNullabilitySpelling(nullability.first,
+  /*isContextSensitive=*/nullability.second) +
+   "'")
+  .str());
   return DB;
 }
 

diff  --git a/clang/lib/Basic/IdentifierTable.cpp 
b/clang/lib/Basic/IdentifierTable.cpp
index 97dc5cd2d9a6d..9be42c0f95ddc 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -849,6 +849,20 @@ StringRef clang::getNullabilitySpelling(NullabilityKind 
kind,
   llvm_unreachable("Unknown nullability kind.");
 }
 
+llvm::raw_ostream <<(llvm::raw_ostream , NullabilityKind NK) {
+  switch (NK) {
+  case NullabilityKind::NonNull:
+return OS << "NonNull";
+  case NullabilityKind::Nullable:
+return OS << "Nullable";
+  case NullabilityKind::NullableResult:
+return OS << "NullableResult";
+  case NullabilityKind::Unspecified:
+return OS << "Unspecified";
+  }
+  llvm_unreachable("Unknown nullability kind.");
+}
+
 diag::kind
 IdentifierTable::getFutureCompatDiagKind(const IdentifierInfo ,
  const LangOptions ) {

diff  --git a/clang/test/SemaObjC/nullable-result.m 
b/clang/test/SemaObjC/nullable-result.m
index 6125141e8c261..17cc1dd63810a 100644
--- a/clang/test/SemaObjC/nullable-result.m
+++ b/clang/test/SemaObjC/nullable-result.m
@@ -25,9 +25,9 @@ void test_conversion(void) {
 }
 
 void test_dup(void) {
-  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate 
nullability specifier _Nullable_result}}
-  id _Nullable _Nullable_result b; // expected-error{{nullability specifier 
_Nullable_result conflicts with existing specifier '_Nullable'}}
-  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier 
'_Nonnull' conflicts with existing specifier _Nullable_result}}
+  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate 
nullability specifier '_Nullable_result'}}
+  id _Nullable _Nullable_result b; // expected-error{{nullability specifier 
'_Nullable_result' conflicts with existing specifier '_Nullable'}}
+  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier 

[PATCH] D149650: Give NullabilityKind a printing operator<

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks Aaron!

The check is not going to be ready for upstreaming really soon - plenty of 
thorny issues to work out first, and checking in a half-working version would 
do more harm than good.
But that's definitely the goal once we have some deployment experience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

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

[PATCH] D149906: [Clang] Update release notes

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov created this revision.
alexander-shaposhnikov added reviewers: rsmith, erichkeane, ilya-biryukov.
alexander-shaposhnikov created this object with visibility "All Users".
Herald added a project: All.
alexander-shaposhnikov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update release notes.
This is a follow-up to https://reviews.llvm.org/D146178


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149906

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -78,8 +78,13 @@
 
 C++20 Feature Support
 ^
-- Support for out-of-line definitions of constrained templates has been 
improved.
-  This partially fixes `#49620 
`_.
+- Implemented the rule introduced by `CA104 `_  for 
comparison of
+  constraints expressions. Improved support for out-of-line definitions of 
constrained templates.
+  This fixes:
+  `#49620 `_,
+  `#60231 `_,
+  `#61414 `_,
+  `#61809 `_.
 - Lambda templates with a requires clause directly after the template 
parameters now parse
   correctly if the requires clause consists of a variable with a dependent 
type.
   (`#61278 `_)


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -78,8 +78,13 @@
 
 C++20 Feature Support
 ^
-- Support for out-of-line definitions of constrained templates has been improved.
-  This partially fixes `#49620 `_.
+- Implemented the rule introduced by `CA104 `_  for comparison of
+  constraints expressions. Improved support for out-of-line definitions of constrained templates.
+  This fixes:
+  `#49620 `_,
+  `#60231 `_,
+  `#61414 `_,
+  `#61809 `_.
 - Lambda templates with a requires clause directly after the template parameters now parse
   correctly if the requires clause consists of a variable with a dependent type.
   (`#61278 `_)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

LGTM still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149144

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

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

@tbaeder I have a small suggestion.




Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1121-1138
+static unsigned getNumDisplayWidth(unsigned N) {
+  if (N < 10)
+return 1;
+  if (N < 100)
+return 2;
+  if (N < 1'000)
+return 3;

This function screamed at me to be generalized so I gave it a try: 
https://gist.github.com/kwk/7e408065ea291e49fea4a83cf90a9cdf


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

https://reviews.llvm.org/D147875

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

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

After reviewing the GitHub issue, it looks like the IndentAndNestingLevel is 
sticky over the entire scope for `#else` blocks, so the regression only occurs 
within `#else` blocks in the same scope. The bug I fixed affected alignment in 
all `#else` blocks, regardless of scope.

I believe that if you choose to revert this @owenpan, you will see more bad 
alignments throughout `#else` blocks in an entire file. If you keep the fix, 
you will see bad alignment in `#else` blocks within the current scope. Pick 
your poison I guess. I personally prefer better alignment in separate `#else` 
blocks over the entire file, which is what prompted my bugfix in the first 
place. On those grounds I object to reverting the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134042

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

[PATCH] D149612: [Sema] avoid merge error type

[Congcong Cai via Phabricator via cfe-commits]

HerrCai0907 added a comment.

Dependent type for CPP is a common case due to template. But for `mergeTypes` 
in C,  dependent type cannot be handled correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

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

[PATCH] D149904: Generic selection expressions that accept a type operand

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: clang-language-wg, efriedma, jyknight, 
hubert.reinterpretcast, rsmith.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.
Herald added a reviewer: NoQ.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Extends `_Generic` selection expressions to accept a type operand in addition 
to an expression operand. The type operand form does not perform lvalue 
conversions on the operand, which allows the expression to select *qualified* 
types, making it more useful for generic programming in C.

C has a few operators that take either a type or an expression, such as 
`sizeof`. It is natural to extend that idea to `_Generic` so that it can also 
accept a type for the first operand. This type does not undergo any 
conversions, which allows it to match qualified types, incomplete types, and 
function types. C2x has the `typeof` operator as a way to get the type of an 
expression before lvalue conversion takes place, and so it keeps the 
qualification. This makes `typeof` a straightforward approach to determining a 
type operand for `_Generic` that considers qualifiers. This allows writing a 
helper macro like:

  #define EXPR_HAS_TYPE(Expr, Type) _Generic(typeof(Expr), Type : 1, default : 
0)

which can be called with an expression of any value category (no need to be an 
lvalue) and will test against (almost) any type. This is a conforming extension 
to C (it's defining the behavior of invalid syntax), and I believe this is a 
natural way to extend this functionality.

`_Generic` with a type operand will relax the requirements of what can be a 
valid association. Specifically, it allows incomplete types and non-object 
types (but still prevents use of variably-modified types). This relaxation only 
happens for the type operand form; the expression operand form continues to 
behave as it always has.

This extension allows incomplete and non-object types because the goal is to 
better enable type-generic programming in C, and so we want to allow any typed 
construct we can statically determine the type of. There is no reason to 
prevent matching against `void` or function types, but this does explain why we 
continue to prohibit variably-modified types.

Please see the RFC at 
https://discourse.llvm.org/t/rfc-generic-selection-expression-with-a-type-operand/70388
 for more details.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149904

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/Parser/generic-selection-type-extension-pedantic.c
  clang/test/Parser/generic-selection-type-extension.c
  clang/test/Sema/generic-selection-type-extension.c
  clang/test/SemaCXX/generic-selection.cpp

Index: clang/test/SemaCXX/generic-selection.cpp
===
--- clang/test/SemaCXX/generic-selection.cpp
+++ clang/test/SemaCXX/generic-selection.cpp
@@ -3,7 +3,7 @@
 template 
 struct A {
   enum {
-id = _Generic(T(), // expected-error {{controlling expression type 'char' not compatible with any generic association type}}
+id = _Generic(T{}, // expected-error {{controlling expression type 'char' not compatible with any generic association type}}
 int: 1, // expected-note {{compatible type 'int' specified here}}
 float: 2,
 U: 3) // expected-error {{type 'int' in generic association compatible with previously specified type 'int'}}
@@ -20,7 +20,7 @@
 template 
 struct B {
   enum {
-id = _Generic(T(),
+id = _Generic(T{},
 int: 1, // expected-note {{compatible type 'int' specified here}}
 int: 2, // expected-error {{type 'int' in generic association compatible with previously specified type 'int'}}
 U: 3)
@@ -37,7 +37,7 @@
 
 template  struct TypeMask {
   enum {
-   result = Or<_Generic(Args(), int: 1, long: 2, short: 4, float: 8)...>::result
+   result = Or<_Generic(Args{}, int: 1, long: 2, short: 4, float: 8)...>::result
   };
 };
 
Index: clang/test/Sema/generic-selection-type-extension.c
===
--- /dev/null
+++ clang/test/Sema/generic-selection-type-extension.c
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify -Wno-unused %s
+// RUN: %clang_cc1 

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbfbe13788815: [clang][dataflow] Eliminate intermediate 
`ReferenceValue`s from `Environment… (authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D149144?vs=519373=519644#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149144

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -404,14 +404,9 @@
 
 const StorageLocation *FooLoc =
 Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
-
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation  = FooVal->getReferentLoc();
-EXPECT_TRUE(isa());
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+const Value *FooReferentVal = Env.getValue(*FooLoc);
 EXPECT_TRUE(isa_and_nonnull(FooReferentVal));
   });
 }
@@ -495,11 +490,9 @@
 ASSERT_THAT(BazRefDecl, NotNull());
 ASSERT_THAT(BazPtrDecl, NotNull());
 
-const auto *FooLoc = cast(
+const auto *FooLoc = cast(
 Env.getStorageLocation(*FooDecl, SkipPast::None));
-const auto *FooVal = cast(Env.getValue(*FooLoc));
-const auto *FooReferentVal =
-cast(Env.getValue(FooVal->getReferentLoc()));
+const auto *FooReferentVal = cast(Env.getValue(*FooLoc));
 
 const auto *BarVal =
 cast(FooReferentVal->getChild(*BarDecl));
@@ -508,13 +501,17 @@
 
 const auto *FooRefVal =
 cast(BarReferentVal->getChild(*FooRefDecl));
-const StorageLocation  = FooRefVal->getReferentLoc();
-EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
+const auto  =
+cast(FooRefVal->getReferentLoc());
+EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooReferentLoc.getChild(*BarDecl)), IsNull());
 
 const auto *FooPtrVal =
 cast(BarReferentVal->getChild(*FooPtrDecl));
-const StorageLocation  = FooPtrVal->getPointeeLoc();
-EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+const auto  =
+cast(FooPtrVal->getPointeeLoc());
+EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooPtrPointeeLoc.getChild(*BarDecl)), IsNull());
 
 const auto *BazRefVal =
 cast(BarReferentVal->getChild(*BazRefDecl));
@@ -1059,16 +1056,9 @@
 
 const StorageLocation *FooLoc =
 Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
-
-const ReferenceValue *FooVal =
-dyn_cast(Env.getValue(*FooLoc));
-ASSERT_THAT(FooVal, NotNull());
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const StorageLocation  = FooVal->getReferentLoc();
-EXPECT_TRUE(isa());
-
-const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+const Value *FooReferentVal = Env.getValue(*FooLoc);
 EXPECT_TRUE(isa_and_nonnull(FooReferentVal));
   });
 }
@@ -1906,15 +1896,13 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const auto *FooVal =
-cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *FooLoc = Env.getStorageLocation(*FooDecl);
 
 const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux");
 ASSERT_THAT(QuxDecl, NotNull());
 
-const auto *QuxVal =
-cast(Env.getValue(*QuxDecl, SkipPast::None));
-EXPECT_EQ(>getReferentLoc(), >getReferentLoc());
+const auto *QuxLoc = Env.getStorageLocation(*QuxDecl);
+EXPECT_EQ(QuxLoc, FooLoc);
   });
 }
 
@@ -2594,9 +2582,8 @@
 
 const auto *FooVal =
 cast(Env.getValue(*FooDecl, SkipPast::None));
-const auto *BarVal =
-cast(Env.getValue(*BarDecl, SkipPast::None));
-EXPECT_EQ(>getReferentLoc(), >getPointeeLoc());
+const auto *BarLoc = Env.getStorageLocation(*BarDecl);
+EXPECT_EQ(BarLoc, >getPointeeLoc());
   });
 }
 
@@ -2644,17 +2631,20 @@
 void target(int *Foo) {
   do {
 int Bar = *Foo;
+// [[in_loop]]
   } while (false);
   (void)0;
-  /*[[p]]*/
+  // [[after_loop]]
 }
   )";
   runDataflow(
   Code,
   [](const llvm::StringMap> ,
  

[clang] bfbe137 - [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-05-04T20:57:30Z
New Revision: bfbe137888151dfd506df6b3319d08c4de0e00f5

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

LOG: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from 
`Environment::DeclToLoc`.

For the wider context of this change, see the RFC at
https://discourse.llvm.org/t/70086.

After this change, global and local variables of reference type are associated
directly with the `StorageLocation` of the referenced object instead of the
`StorageLocation` of a `ReferenceValue`.

Some tests that explicitly check for an existence of `ReferenceValue` for a
variable of reference type have been modified accordingly.

As discussed in the RFC, I have added an assertion to `Environment::join()` to
check that if both environments contain an entry for the same declaration in
`DeclToLoc`, they both map to the same `StorageLocation`. As discussed in
https://discourse.llvm.org/t/70086/5, this also necessitates removing
declarations from `DeclToLoc` when they go out of scope.

In the RFC, I proposed a gradual migration for this change, but it appears
that all of the callers of `Environment::setStorageLocation(const ValueDecl &,
SkipPast` are in the dataflow framework itself, and that there are only a few of
them.

As this is the function whose semantics are changing in a way that callers
potentially need to adapt to, I've decided to change the semantics of the
function directly.

The semantics of `getStorageLocation(const ValueDecl &, SkipPast SP` now no
longer depend on the behavior of the `SP` parameter. (There don't appear to be
any callers that use `SkipPast::ReferenceThenPointer`, so I've added an
assertion that forbids this usage.)

This patch adds a default argument for the `SP` parameter and removes the
explicit `SP` argument at the callsites that are touched by this change. A
followup patch will remove the argument from the remaining callsites,
allowing the `SkipPast` parameter to be removed entirely. (I don't want to do
that in this patch so that semantics-changing changes can be reviewed separately
from semantics-neutral changes.)

Reviewed By: ymandel, xazax.hun, gribozavr2

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 9c027ef4552ee..cd1703be0507b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -269,13 +269,24 @@ class Environment {
   ///
   /// Requirements:
   ///
-  ///  `D` must not be assigned a storage location in the environment.
+  ///  `D` must not already have a storage location in the environment.
+  ///
+  ///  If `D` has reference type, `Loc` must refer directly to the referenced
+  ///  object (if any), not to a `ReferenceValue`, and it is not permitted to
+  ///  later change `Loc` to refer to a `ReferenceValue.`
   void setStorageLocation(const ValueDecl , StorageLocation );
 
-  /// Returns the storage location assigned to `D` in the environment, applying
-  /// the `SP` policy for skipping past indirections, or null if `D` isn't
-  /// assigned a storage location in the environment.
-  StorageLocation *getStorageLocation(const ValueDecl , SkipPast SP) const;
+  /// Returns the storage location assigned to `D` in the environment, or null
+  /// if `D` isn't assigned a storage location in the environment.
+  ///
+  /// Note that if `D` has reference type, the storage location that is 
returned
+  /// refers directly to the referenced object, not a `ReferenceValue`.
+  ///
+  /// The `SP` parameter is deprecated and has no effect. In addition, it is
+  /// not permitted to pass `SkipPast::ReferenceThenPointer` for this 
parameter.
+  /// New uses of this function should use the default argument for `SP`.
+  StorageLocation *getStorageLocation(const ValueDecl ,
+  SkipPast SP = SkipPast::None) const;
 
   /// Assigns `Loc` as the storage location of `E` in the environment.
   ///
@@ -320,7 +331,11 @@ class Environment {
 
   /// Equivalent to `getValue(getStorageLocation(D, SP), SkipPast::None)` if 
`D`
   /// is assigned a storage location in the environment, otherwise returns 
null.
-  Value *getValue(const ValueDecl , SkipPast SP) const;
+  ///
+  /// The `SP` parameter is 

[PATCH] D149677: [clang][TypePrinter] Add option to skip over elaborated types

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua added a comment.

In D149677#4319646 , @aaron.ballman 
wrote:

> Will this new printing policy be used in tree?

Not initially, no. (I'm understanding "in tree" as "in the LLVM project".)

Some additional context: I'm working on a refactoring tool to, in part, 
deprecate an alias by replacing it with its definition. That functionality by 
itself could be provided through something like a clang-tidy, so I say 
"initially".




Comment at: clang/include/clang/AST/PrettyPrinter.h:143-145
+  /// Ignore qualifiers as specified by elaborated type sugar, instead letting
+  /// the underlying type handle printing the qualifiers.
+  unsigned IgnoreElaboratedQualifiers : 1;

aaron.ballman wrote:
> The name is somewhat confusing to me, because tag names are also elaborations 
> and those are handled by `SuppressTagKeyword` -- so what is the interaction 
> between those when the user specifies true for `IgnoreElaboratedQualifiers` 
> and false for `SuppressTagKeyword`?
Ah, I hadn't considered tag names. Let me see...

I tried this out with the template specialization test case I added. If I have

```
using Alias = struct a::S;
```

then printing the aliased type of `Alias` with true for 
`IgnoreElaboratedQualifiers` and false for `SuppressTagKeyword` gives 
`"S"`. (I'm guessing that printing the template 
specialization with `SuppressTagKeyword` disabled is weird or unexpected?) So 
it would seem that `SuppressTagKeyword` still functions, as long as the 
desugared type supports it?

Back to the topic of names, it seems like "qualifiers" is not sufficient, as it 
also ignores elaborated tag keywords. Perhaps `IgnoreElaboration`?

---
Note: I initially had the name as `IgnoreElaboratedTypes`, but there was also 
the [[ 
https://github.com/llvm/llvm-project/blob/1b05e74982242da81d257f660302585cee691f3b/clang/lib/AST/TypePrinter.cpp#L1559-L1568
 | tag definition printing logic ]] that I didn't fully understand, so I 
thought just saying "qualifiers" would be more correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149677

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

[PATCH] D149677: [clang][TypePrinter] Add option to skip over elaborated types

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 519643.
li.zhe.hua marked an inline comment as done.
li.zhe.hua added a comment.

Accept suggested edits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149677

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp
  clang/unittests/AST/TypePrinterTest.cpp

Index: clang/unittests/AST/TypePrinterTest.cpp
===
--- clang/unittests/AST/TypePrinterTest.cpp
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -97,6 +97,33 @@
  "const f *", Clean));
 }
 
+TEST(TypePrinter, IgnoreElaboratedQualifiers) {
+  llvm::StringLiteral Code = R"cpp(
+namespace shared {
+namespace a {
+template 
+struct S {};
+}  // namespace a
+namespace b {
+struct Foo {};
+}  // namespace b
+using Alias = a::S;
+}  // namespace shared
+  )cpp";
+
+  auto Matcher = typedefNameDecl(hasName("::shared::Alias"),
+ hasType(qualType().bind("id")));
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {}, Matcher, "a::S",
+  [](PrintingPolicy ) { Policy.FullyQualifiedName = true; }));
+  ASSERT_TRUE(PrintedTypeMatches(Code, {}, Matcher,
+ "shared::a::S",
+ [](PrintingPolicy ) {
+   Policy.IgnoreElaboratedQualifiers = true;
+   Policy.FullyQualifiedName = true;
+ }));
+}
+
 TEST(TypePrinter, TemplateIdWithNTTP) {
   constexpr char Code[] = R"cpp(
 template 
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1567,6 +1567,11 @@
 return;
   }
 
+  if (Policy.IgnoreElaboratedQualifiers) {
+printBefore(T->getNamedType(), OS);
+return;
+  }
+
   // The tag definition will take care of these.
   if (!Policy.IncludeTagDefinition)
   {
@@ -1586,6 +1591,12 @@
 raw_ostream ) {
   if (Policy.IncludeTagDefinition && T->getOwnedTagDecl())
 return;
+
+  if (Policy.IgnoreElaboratedQualifiers) {
+printAfter(T->getNamedType(), OS);
+return;
+  }
+
   ElaboratedTypePolicyRAII PolicyRAII(Policy);
   printAfter(T->getNamedType(), OS);
 }
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -60,14 +60,15 @@
   : Indentation(2), SuppressSpecifiers(false),
 SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
 SuppressScope(false), SuppressUnwrittenScope(false),
-SuppressInlineNamespace(true), SuppressInitializers(false),
-ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
+SuppressInlineNamespace(true), IgnoreElaboratedQualifiers(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
 SuppressTemplateArgsInCXXConstructors(false),
 SuppressDefaultTemplateArgs(true), Bool(LO.Bool),
 Nullptr(LO.CPlusPlus11 || LO.C2x), NullptrTypeInNamespace(LO.CPlusPlus),
-Restrict(LO.C99), Alignof(LO.CPlusPlus11),
-UnderscoreAlignof(LO.C11), UseVoidForZeroParams(!LO.CPlusPlus),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus),
 SplitTemplateClosers(!LO.CPlusPlus11), TerseOutput(false),
 PolishForDeclaration(false), Half(LO.Half),
 MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
@@ -139,6 +140,10 @@
   /// removed.
   unsigned SuppressInlineNamespace : 1;
 
+  /// Ignore qualifiers as specified by elaborated type sugar, instead letting
+  /// the underlying type handle printing the qualifiers.
+  unsigned IgnoreElaboratedQualifiers : 1;
+
   /// Suppress printing of variable initializers.
   ///
   /// This flag is used when printing the loop variable in a for-range
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

[Craig Topper via Phabricator via cfe-commits]

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

The instructions need a DecoderNamespace to separate them from c.fsdsp. See 
D149891  for how I've done it for Zcmt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819

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

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: clang-vendors.
aaron.ballman added a comment.

This should definitely have explicit documentation added to 
https://github.com/llvm/llvm-project/blob/main/clang/docs/UsersManual.rst?plain=1#L156
 and a release note.

Adding clang-vendors because of the potential that this is a breaking change 
for some folks doing diagnostic scraping; perhaps we should also mention this 
under potentially breaking changes in the release notes?




Comment at: clang/include/clang/Driver/Options.td:2544
+defm diagnostics_show_line_numbers : 
BoolFOption<"diagnostics-show-line-numbers",
+  DiagnosticOpts<"ShowLineNumbers">, DefaultTrue,
+  NegFlag,

I'm hopeful that defaulting this to true (which matches the GCC behavior) is 
reasonable, but I'm a bit worried about how downstreams will react to this. 
Specifically, I'm wondering about things like clang-cl integration in MSVC or 
clang use in Xcode, where diagnostics are being displayed other than on the 
command line.



Comment at: clang/test/FixIt/fixit-function-call.cpp:1
-// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -x c++ %s 2> %t
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits 
-fno-diagnostics-show-line-numbers -fcaret-diagnostics-max-lines 1 -x c++ %s 2> 
%t
 // RUN: FileCheck %s < %t

Just to double-check, parseable diagnostic output still works even if line 
numbers are shown, yes?


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

https://reviews.llvm.org/D147875

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

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a nit with the release notes. Thank you for the fix!




Comment at: clang/docs/ReleaseNotes.rst:109-113
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)

I think this should move down to `Improvements to Clang's diagnostics` instead 
of adding a new category for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

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

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329
+static void
+builtinTransferScopeEnd(const CFGScopeEnd ,
+TypeErasedDataflowAnalysisState ) {

xazax.hun wrote:
> mboehme wrote:
> > mboehme wrote:
> > > xazax.hun wrote:
> > > > I think one question is whether we are interested in ScopeEnd or 
> > > > LifetimeEnd, see the discussions in https://reviews.llvm.org/D15031 and 
> > > > https://reviews.llvm.org/D16403, specifically Devin's comment: 
> > > > 
> > > > >>! In D16403#799926, @dcoughlin wrote:
> > > > >> @dcoughlin As a reviewer of both patches - could you tell us what's 
> > > > >> the difference between them? And how are we going to resolve this 
> > > > >> issue?
> > > > > 
> > > > > These two patches are emitting markers in the CFG for different 
> > > > > things.
> > > > > 
> > > > > Here is how I would characterize the difference between the two 
> > > > > patches.
> > > > > 
> > > > > - Despite its name, https://reviews.llvm.org/D15031, is really 
> > > > > emitting markers for when the lifetime of a C++ object in an 
> > > > > automatic variable ends.  For C++ objects with non-trivial 
> > > > > destructors, this point is when the destructor is called. At this 
> > > > > point the storage for the variable still exists, but what you can do 
> > > > > with that storage is very restricted by the language because its 
> > > > > contents have been destroyed. Note that even with the contents of the 
> > > > > object have been destroyed, it is still meaningful to, say, take the 
> > > > > address of the variable and construct a new object into it with a 
> > > > > placement new. In other words, the same variable can have different 
> > > > > objects, with different lifetimes, in it at different program points.
> > > > > 
> > > > > - In contrast, the purpose of this patch 
> > > > > (https://reviews.llvm.org/D16403) is to add markers for when the 
> > > > > storage duration for the variable begins and ends (this is, when the 
> > > > > storage exists). Once the storage duration has ended, you can't 
> > > > > placement new into the variables address, because another variable 
> > > > > may already be at that address.
> > > > > 
> > > > > I don't think there is an "issue" to resolve here.  We should make 
> > > > > sure the two patches play nicely with each other, though. In 
> > > > > particular, we should make sure that the markers for when lifetime 
> > > > > ends and when storage duration ends are ordered correctly.
> > > > 
> > > > 
> > > > What I wanted to add, I wonder if we might not get ScopeEnd event for 
> > > > temporaries and there might be other differences. The Clang 
> > > > implementation of P1179 used LifetimeEnd instead of ScopeEnd, and I 
> > > > believe probably most clients are more interested in LifetimeEnd events 
> > > > rather than ScopeEnd.
> > > > 
> > > > I think I understand why you went with ScopeEnd for this specific 
> > > > problem, but to avoid the confusion from having both in the Cfg 
> > > > (because I anticipate someone will need LifetimeEnd at some point), I 
> > > > wonder if we can make this work with LifetimeEnd. 
> > > Hm, thanks for bringing it up.
> > > 
> > > Conincidentally, I realized while chasing another issue that 
> > > `CFGScopeEnd` isn't the right construct here. I assumed that we would get 
> > > a `CFGScopeEnd` for every variable, but I now realize that we only get a 
> > > `CFGScopeEnd` for the first variable in the scope.
> > > 
> > > So `CFGLifetimeEnds` definitely looks like the right construct to use 
> > > here, and indeed it's what I originally tried to use. Unfortuately, 
> > > `CFG::BuildOptions::AddLifetime` cannot be used at the same time as 
> > > `AddImplicitDtors`, which we already use. We don't actually need the 
> > > `CFGElement`s added by `AddImplicitDtors`, but we do need 
> > > `AddTemporaryDtors`, and that in turn can only be turned on if 
> > > `AddImplicitDtors` is turned on.
> > > 
> > > It looks as if I will need to break one of these constraints. It looks as 
> > > if the constraint that is easiest to break is that `AddLifetime` 
> > > currently cannot be used at the same time as `AddImplicitDtors`. I'm not 
> > > sure why this constraint currently exists (I'd appreciate any insights 
> > > you or others may have), but I suspect it's because it's hard to ensure 
> > > that the `CFGElement`s added by `AddImplicitDtors` are ordered correctly 
> > > with respect to the `CFGLifetimeEnds` elements.
> > > 
> > > Anyway, this requires a change to `CFG` one way or another. I'll work on 
> > > that next. I'll then make the required changes to this patch and will ask 
> > > for another look before submitting.
> > I've taken a look at what it would take to make `AddLifetime` coexist with 
> > `AddImplicitDtors`, and it's hard (because we need to get the ordering 
> > right, and that's non-trivial).
> > 
> > So 

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 519633.
carlosgalvezp edited the summary of this revision.
carlosgalvezp added a comment.

Update commit message with Github issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149899

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
  clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -120,6 +120,7 @@
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
   UseColor: false
+  SystemHeaders: false
   )",
"Options1"));
   ASSERT_TRUE(!!Options1);
@@ -134,6 +135,7 @@
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
   UseColor: true
+  SystemHeaders: true
   )",
"Options2"));
   ASSERT_TRUE(!!Options2);
@@ -154,6 +156,9 @@
Options.ExtraArgsBefore->end(), ","));
   ASSERT_TRUE(Options.UseColor.has_value());
   EXPECT_TRUE(*Options.UseColor);
+
+  ASSERT_TRUE(Options.SystemHeaders.has_value());
+  EXPECT_TRUE(*Options.SystemHeaders);
 }
 
 namespace {
@@ -249,6 +254,17 @@
  DiagKind(llvm::SourceMgr::DK_Error),
  DiagPos(Options.range().Begin),
  DiagRange(Options.range();
+
+  Options = llvm::Annotations(R"(
+SystemHeaders: [[NotABool]]
+  )");
+  ParsedOpt = ParseWithDiags(Options.code());
+  EXPECT_TRUE(!ParsedOpt);
+  EXPECT_THAT(Collector.getDiags(),
+  testing::ElementsAre(AllOf(DiagMessage("invalid boolean"),
+ DiagKind(llvm::SourceMgr::DK_Error),
+ DiagPos(Options.range().Begin),
+ DiagRange(Options.range();
 }
 
 namespace {
Index: clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ -0,0 +1,18 @@
+// RUN: clang-tidy -dump-config -system-headers=true | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -dump-config -system-headers=false | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: true' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+
+#include 
+// CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+// CHECK-NO-SYSTEM-HEADERS-NOT: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+
+// CHECK-CONFIG-NO-SYSTEM-HEADERS: SystemHeaders: false
+// CHECK-CONFIG-SYSTEM-HEADERS: SystemHeaders: true
+// CHECK-OPT-PRESENT: --system-headers
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
@@ -0,0 +1 @@
+class Foo { Foo(int); };
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,9 @@
 
 - 

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp created this revision.
Herald added subscribers: PiotrZSL, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

A previous patch update the clang-tidy documentation
incorrectly claiming that SystemHeaders can be provided
in the .clang-tidy configuration file.

This patch adds support for it, together with tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149899

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
  clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -120,6 +120,7 @@
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
   UseColor: false
+  SystemHeaders: false
   )",
"Options1"));
   ASSERT_TRUE(!!Options1);
@@ -134,6 +135,7 @@
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
   UseColor: true
+  SystemHeaders: true
   )",
"Options2"));
   ASSERT_TRUE(!!Options2);
@@ -154,6 +156,9 @@
Options.ExtraArgsBefore->end(), ","));
   ASSERT_TRUE(Options.UseColor.has_value());
   EXPECT_TRUE(*Options.UseColor);
+
+  ASSERT_TRUE(Options.SystemHeaders.has_value());
+  EXPECT_TRUE(*Options.SystemHeaders);
 }
 
 namespace {
@@ -249,6 +254,17 @@
  DiagKind(llvm::SourceMgr::DK_Error),
  DiagPos(Options.range().Begin),
  DiagRange(Options.range();
+
+  Options = llvm::Annotations(R"(
+SystemHeaders: [[NotABool]]
+  )");
+  ParsedOpt = ParseWithDiags(Options.code());
+  EXPECT_TRUE(!ParsedOpt);
+  EXPECT_THAT(Collector.getDiags(),
+  testing::ElementsAre(AllOf(DiagMessage("invalid boolean"),
+ DiagKind(llvm::SourceMgr::DK_Error),
+ DiagPos(Options.range().Begin),
+ DiagRange(Options.range();
 }
 
 namespace {
Index: clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ -0,0 +1,18 @@
+// RUN: clang-tidy -dump-config -system-headers=true | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -dump-config -system-headers=false | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
+// RUN: clang-tidy -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: true' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+
+#include 
+// CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+// CHECK-NO-SYSTEM-HEADERS-NOT: system_header.h:1:13: warning: single-argument constructors must be marked explicit
+
+// CHECK-CONFIG-NO-SYSTEM-HEADERS: SystemHeaders: false
+// CHECK-CONFIG-SYSTEM-HEADERS: SystemHeaders: true
+// CHECK-OPT-PRESENT: --system-headers
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/system-headers/system_header.h
@@ -0,0 +1 @@
+class Foo { 

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Owen Pan via Phabricator via cfe-commits]

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

I'll revert this patch due to the regression 
https://github.com/llvm/llvm-project/issues/61498 unless any of you (and 
@rymiel ) objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134042

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

[PATCH] D149514: Check if First argument in _builtin_assume_aligned_ is of pointer type

[Rishabh Bali via Phabricator via cfe-commits]

Ris-Bali updated this revision to Diff 519628.
Ris-Bali added a comment.

Rebase


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

https://reviews.llvm.org/D149514

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtin-assume-aligned.c


Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -71,6 +71,25 @@
   return a[0];
 }
 
+int test14(int *a, int b) {
+  a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{incompatible 
integer to pointer conversion passing 'int' to parameter of type 'const void *}}
+}
+
+int test15(int *b) {
+  int arr[3] = {1, 2, 3};
+  b = (int *)__builtin_assume_aligned(arr, 32);
+  return b[0];
+}
+
+int val(int x) {
+  return x;
+}
+
+int test16(int *b) {
+  b = (int *)__builtin_assume_aligned(val, 32);
+  return b[0];
+}
+
 void test_void_assume_aligned(void) __attribute__((assume_aligned(32))); // 
expected-warning {{'assume_aligned' attribute only applies to return values 
that are pointers}}
 int test_int_assume_aligned(void) __attribute__((assume_aligned(16))); // 
expected-warning {{'assume_aligned' attribute only applies to return values 
that are pointers}}
 void *test_ptr_assume_aligned(void) __attribute__((assume_aligned(64))); // 
no-warning
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8100,8 +8100,9 @@
   {
 ExprResult FirstArgResult =
 DefaultFunctionArrayLvalueConversion(FirstArg);
-if (FirstArgResult.isInvalid())
+if (checkBuiltinArgument(*this, TheCall, 0))
   return true;
+/// In-place updation of FirstArg by checkBuiltinArgument is ignored.
 TheCall->setArg(0, FirstArgResult.get());
   }
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2829,8 +2829,6 @@
   case Builtin::BI__builtin_assume_aligned: {
 const Expr *Ptr = E->getArg(0);
 Value *PtrValue = EmitScalarExpr(Ptr);
-if (PtrValue->getType() != VoidPtrTy)
-  PtrValue = EmitCastToVoidPtr(PtrValue);
 Value *OffsetValue =
   (E->getNumArgs() > 2) ? EmitScalarExpr(E->getArg(2)) : nullptr;
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -361,6 +361,9 @@
   (`#62207 `_)
 - Fix lambdas and other anonymous function names not respecting 
``-fdebug-prefix-map``
   (`#62192 `_)
+- Fix crash when attempting to pass a non-pointer type as first argument of
+  ``__builtin_assume_aligned``.
+  (`#62305 `_) 
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -71,6 +71,25 @@
   return a[0];
 }
 
+int test14(int *a, int b) {
+  a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *}}
+}
+
+int test15(int *b) {
+  int arr[3] = {1, 2, 3};
+  b = (int *)__builtin_assume_aligned(arr, 32);
+  return b[0];
+}
+
+int val(int x) {
+  return x;
+}
+
+int test16(int *b) {
+  b = (int *)__builtin_assume_aligned(val, 32);
+  return b[0];
+}
+
 void test_void_assume_aligned(void) __attribute__((assume_aligned(32))); // expected-warning {{'assume_aligned' attribute only applies to return values that are pointers}}
 int test_int_assume_aligned(void) __attribute__((assume_aligned(16))); // expected-warning {{'assume_aligned' attribute only applies to return values that are pointers}}
 void *test_ptr_assume_aligned(void) __attribute__((assume_aligned(64))); // no-warning
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8100,8 +8100,9 @@
   {
 ExprResult FirstArgResult =
 DefaultFunctionArrayLvalueConversion(FirstArg);
-if (FirstArgResult.isInvalid())
+if (checkBuiltinArgument(*this, TheCall, 0))
   return true;
+/// In-place updation of FirstArg by checkBuiltinArgument is ignored.
 TheCall->setArg(0, FirstArgResult.get());
   }
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D149612: [Sema] avoid merge error type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D149612#4314536 , @shafik wrote:

> In D149612#4314209 , @HerrCai0907 
> wrote:
>
>> in SemaType.cpp#BuildArrayType, It will generate `DependentSizedArrayType` 
>> which cannot be merged. 
>> So we can either add additional check in `MergeVarDeclTypes` or directly 
>> ignore to generate this type in `BuildArrayType`.
>> Which one is better?
>>
>>   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
>> T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, 
>> Brackets);
>
> I am not sure where the right place to fix this is, or when it is correct to 
> use `containsErrors()`
>
> Maybe @aaron.ballman or @rsmith might have some advice

so `containsErrors` on an expression just means it has a `RecoveryExpr` in it.  
MUCH of the time a `RecoveryExpr` is just created with a Dependent type, thus 
the Dependent Array type.  I would suggest rather than just returning 'no type' 
(via the empty qual type), if we find that the Array Size contains errors (I 
don't get the if CPlusPlus check?), we could either:
1- Make it an `IncompleteArrayType`.
2- Make it a `ConstantArrayType`, with a correctly-typed `RecoveryExpr` (that 
is, not dependent, still a size_t/etc expression, BUT with no value) for the 
bounds type.  I would think it is better to do this ~2584 though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149612

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

[PATCH] D149516: [Sema] `setInvalidDecl` for error deduction declaration

[Congcong Cai via Phabricator via cfe-commits]

HerrCai0907 added a comment.

ping @rsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149516

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

[PATCH] D144999: [RFC][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I wonder if we actually need to define a clang frontend flag for this; I 
suspect nobody will ever want to specify it, since the only non-canonical 
personality clang will ever generate that changes behavior is the pure-C 
destructor-only personality, `__gnu_personality_v0`. Otherwise, it could only 
arise from assembly or IR.

For a test-case, can just use `-mllvm -emit-compact-unwind-non-canonical`.




Comment at: llvm/lib/MC/MCAsmBackend.cpp:119
+
+bool MCAsmBackend::isDarwinCanonicalPersonality(const MCSymbol *Sym) const {
+  if (Sym && Sym->isMachO()) {

This could use a comment as to why these two are "canonical" and why 
"__gcc_personality_v0" is excluded despite being similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144999

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

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman added a comment.

Thank you very much for your patience and writing everything out. That makes a 
lot more sense. The resulting code definitely has a lot more desirable 
properties. I've updated the diff in accordance with your suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D149809: [Clang][Docs] Fix man page build

[Aiden Grossman via Phabricator via cfe-commits]

aidengrossman updated this revision to Diff 519622.
aidengrossman marked 6 inline comments as done.
aidengrossman added a comment.

Adjust based on reviewer suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

Files:
  clang/docs/CMakeLists.txt


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,60 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
   list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
   clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
-  add_dependencies(${docs_target} "gen-${output_file}")
+  foreach(target ${docs_targets})
+add_dependencies(${target} gen-${output_file})
+  endforeach()
 endfunction()
 
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
-  if (SPHINX_FOUND)
+  if (SPHINX_FOUND AND (${SPHINX_OUTPUT_HTML} OR ${SPHINX_OUTPUT_MAN}))
+# Copy rst files to build directory before generating the html
+# documentation.  Some of the rst files are generated, so they
+# only exist in the build directory.  Sphinx needs all files in
+# the same directory in order to generate the html, so we need to
+# copy all the non-gnerated rst files from the source to the build
+# directory before we run sphinx.
+add_custom_target(copy-clang-rst-docs
+  COMMAND "${CMAKE_COMMAND}" -E copy_directory
+  "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
+
+  COMMAND "${CMAKE_COMMAND}" -E copy_if_different
+  "${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
+  "${CMAKE_CURRENT_BINARY_DIR}"
+)
+
+set(docs_targets "")
+
 if (${SPHINX_OUTPUT_HTML})
   add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
-  # Copy rst files to build directory before generating the html
-  # documentation.  Some of the rst files are generated, so they
-  # only exist in the build directory.  Sphinx needs all files in
-  # the same directory in order to generate the html, so we need to
-  # copy all the non-gnerated rst files from the source to the build
-  # directory before we run sphinx.
-  add_custom_target(copy-clang-rst-docs
-COMMAND "${CMAKE_COMMAND}" -E copy_directory
-"${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
-
-COMMAND "${CMAKE_COMMAND}" -E copy_if_different
-"${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
-"${CMAKE_CURRENT_BINARY_DIR}"
-  )
-  add_dependencies(docs-clang-html copy-clang-rst-docs)
-
   add_custom_command(TARGET docs-clang-html POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
 
-  # Generated files
-  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
-  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
-  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
+  list(APPEND docs_targets "docs-clang-html")
 endif()
 if (${SPHINX_OUTPUT_MAN})
-  add_sphinx_target(man clang)
+  add_sphinx_target(man clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+  list(APPEND docs_targets "docs-clang-man")
 endif()
+
+# Generated files
+gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td "${docs_targets}")
+gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td "${docs_targets}")
+gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td "${docs_targets}")
+
+foreach(target ${docs_targets})
+  add_dependencies(${target} copy-clang-rst-docs)
+endforeach()
   endif()
 endif()


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,60 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source docs_targets)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: 

[PATCH] D149550: [clang][Interp] Fix compound assign operator evaluation order

[Shafik Yaghmour via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:683-685
+  // C++17 onwards require that we evaluate the RHS first.
+  // Compute RHS and save it in a temporary variable so we can
+  // load it again later.

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > tbaeder wrote:
> > > > > aaron.ballman wrote:
> > > > > > In C, the evaluation of the operands are unsequenced. C doesn't 
> > > > > > currently have constexpr functions, only constexpr objects, but 
> > > > > > that eliminates mutating operations like compound assignment... for 
> > > > > > the moment. Perhaps a FIXME comment for figuring out how to handle 
> > > > > > C?
> > > > > > 
> > > > > > (The situation I'm worried about in C is with UB dealing with 
> > > > > > unsequenced operations, like rejecting: 
> > > > > > https://godbolt.org/z/W11jchrKc)
> > > > > Could C make them sequenced when introducing constexpr functions? :)
> > > > Since we already emit a warning for this, we could in the future just 
> > > > check if the statement is in a constexpr function and emit an error 
> > > > instead? We're emitting the warning for c++ pre-17 as well but we don't 
> > > > make it an error, I guess because it's not UB there?
> > > If we're actually leaving the operations unsequenced before C++17, then 
> > > we should reject that code because it is UB: 
> > > http://eel.is/c++draft/basic#intro.execution-10
> > > 
> > > The wording in C++14 for assignment operations is:
> > > > In all cases, the assignment is sequenced after the value computation 
> > > > of the right and left operands, and before the value computation of the 
> > > > assignment expression.
> > > 
> > > So the left and right operands are unsequenced relative to one another.
> > > 
> > I was looking at the existing implementation when writing this patch: 
> > https://github.com/llvm/llvm-project/blob/1a7a00bdc99fa2b2ca19ecd2d1069991b3c1006b/clang/lib/AST/ExprConstant.cpp#L8545-L8568
> >  which always seems to evaluate RHS first (and actually abort for C++ <= 
> > 14, but for unrelated reasons, probably because this statement is just not 
> > supported in a constexpr context there).
> I think the existing implementation is incorrect to accept this in C++14 and 
> earlier:
> 
> https://eel.is/c++draft/basic.exec#intro.execution-10.sentence-4
> http://eel.is/c++draft/expr.const#5.8
I filed a bug on this a while ago that we don't catch unsequenced modifications 
in constant expressions contexts: 
https://github.com/llvm/llvm-project/issues/37768

I am almost sure I had a conversation with @rsmith about this.



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

https://reviews.llvm.org/D149550

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

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 519613.
aeubanks added a comment.

only diagnose if we're initializing an array


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/Modules/predefined.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- /dev/null
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
+
+void f() {
+ const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
+ const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+ const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+}
Index: clang/test/Modules/predefined.cpp
===
--- /dev/null
+++ clang/test/Modules/predefined.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -x c++ -std=c++20 -emit-module-interface a.h -o a.pcm -fms-extensions -verify
+// RUN: %clang_cc1 -std=c++20 a.cpp -fmodule-file=A=a.pcm -fms-extensions -fsyntax-only -verify
+
+//--- a.h
+
+// expected-no-diagnostics
+
+export module A;
+
+export template 
+void f() {
+char a[] = __func__;
+}
+
+//--- a.cpp
+
+// expected-warning@a.h:8 {{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+
+import A;
+
+void g() {
+f(); // expected-note {{in instantiation of function template specialization 'f' requested here}}
+}
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -593,6 +593,7 @@
   bool HasFunctionName = E->getFunctionName() != nullptr;
   Record.push_back(HasFunctionName);
   Record.push_back(E->getIdentKind()); // FIXME: stable encoding
+  Record.push_back(E->isTransparent());
   Record.AddSourceLocation(E->getLocation());
   if (HasFunctionName)
 Record.AddStmt(E->getFunctionName());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -582,6 +582,7 @@
   bool HasFunctionName = Record.readInt();
   E->PredefinedExprBits.HasFunctionName = HasFunctionName;
   E->PredefinedExprBits.Kind = Record.readInt();
+  E->PredefinedExprBits.IsTransparent = Record.readInt();
   E->setLocation(readSourceLocation());
   if (HasFunctionName)
 E->setFunctionName(cast(Record.readSubExpr()));
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -174,6 +174,8 @@
   E = GSE->getResultExpr();
 } else if (ChooseExpr *CE = dyn_cast(E)) {
   E = CE->getChosenSubExpr();
+} else if (PredefinedExpr *PE = dyn_cast(E)) {
+  E = PE->getFunctionName();
 } else {
   llvm_unreachable("unexpected expr in string literal init");
 }
@@ -8501,6 +8503,15 @@
 << Init->getSourceRange();
   }
 
+  if (S.getLangOpts().MicrosoftExt && Args.size() == 1 &&
+  isa(Args[0]) && Entity.getType()->isArrayType()) {
+// Produce a Microsoft compatibility warning when initializing from a
+// predefined expression since MSVC treats predefined expressions as string
+// literals.
+Expr *Init = Args[0];
+S.Diag(Init->getBeginLoc(), diag::ext_init_from_predefined) << Init;
+  }
+
   // OpenCL v2.0 s6.13.11.1. atomic variables can be initialized in global scope
   QualType ETy = Entity.getType();
   bool HasGlobalAS = ETy.hasAddressSpace() &&
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3581,7 

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.
Herald added subscribers: ekilmer, jplehr.



Comment at: cmake/Modules/GetClangResourceDir.cmake:13
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(ret_dir bin/${CLANG_RESOURCE_DIR})
+  else()

Why is the bin prefix here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[PATCH] D148381: [WIP][Clang] Add element_count attribute

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 519605.
void marked an inline comment as done.
void added a comment.

Use "Expected" for the SourceRange imports.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -61,6 +61,7 @@
 // CHECK-NEXT: DiagnoseAsBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: ElementCount (SubjectMatchRule_field)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8238,6 +8238,29 @@
   D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
 }
 
+static void handleElementCountAttr(Sema , Decl *D, const ParsedAttr ) {
+  // TODO: Probably needs more processing here. See Sema::AddAlignValueAttr.
+  SmallVector Names;
+  SmallVector Ranges;
+  for (unsigned ArgNo = 0; ArgNo < getNumAttributeArgs(AL); ++ArgNo) {
+if (!AL.isArgIdent(ArgNo)) {
+  S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+  << AL << AANT_ArgumentIdentifier;
+  return;
+}
+
+IdentifierLoc *IL = AL.getArgAsIdent(ArgNo);
+Names.push_back(IL->Ident);
+Ranges.push_back(IL->Loc);
+  }
+
+  ElementCountAttr *ECA = ::new (S.Context) ElementCountAttr(S.Context, AL,
+ Names.data(),
+ Names.size());
+  ECA->addCountFieldSourceRange(Ranges);
+  D->addAttr(ECA);
+}
+
 static void handleFunctionReturnThunksAttr(Sema , Decl *D,
const ParsedAttr ) {
   StringRef KindStr;
@@ -9136,6 +9159,9 @@
   case ParsedAttr::AT_FunctionReturnThunks:
 handleFunctionReturnThunksAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_ElementCount:
+handleElementCountAttr(S, D, AL);
+break;
 
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17692,6 +17692,44 @@
  "Broken injected-class-name");
 }
 
+static const FieldDecl *FindFieldWithElementCountAttr(const RecordDecl *RD) {
+  for (const Decl *D : RD->decls()) {
+if (const auto *FD = dyn_cast(D))
+  if (FD->hasAttr())
+return FD;
+
+if (const auto *SubRD = dyn_cast(D))
+  if (const FieldDecl *FD = FindFieldWithElementCountAttr(SubRD))
+return FD;
+  }
+
+  return nullptr;
+}
+
+static const IdentifierInfo *
+CheckElementCountAttr(const RecordDecl *RD, const FieldDecl *FD,
+  SourceRange ) {
+  const ElementCountAttr *ECA = FD->getAttr();
+  unsigned Idx = 0;
+
+  for (const IdentifierInfo *II : ECA->elementCountFields()) {
+Loc = ECA->getCountFieldSourceRange(Idx++);
+
+auto DeclIter = llvm::find_if(
+RD->fields(), [&](const FieldDecl *FD){
+  return II->getName() == FD->getName();
+});
+
+if (DeclIter == RD->field_end())
+  return II;
+
+if (auto *SubRD = DeclIter->getType()->getAsRecordDecl())
+  RD = SubRD;
+  }
+
+  return nullptr;
+}
+
 void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
 SourceRange BraceRange) {
   AdjustDeclIfTemplate(TagD);
@@ -17749,6 +17787,17 @@
  [](const FieldDecl *FD) { return FD->isBitField(); }))
   Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
   }
+
+  // Check the "element_count" attribute to ensure that the count field exists
+  // in the struct.
+  if (const RecordDecl *RD = dyn_cast(Tag)) {
+if (const FieldDecl *FD = FindFieldWithElementCountAttr(RD)) {
+  SourceRange SR;
+  if (const IdentifierInfo *II = CheckElementCountAttr(RD, FD, SR))
+Diag(SR.getBegin(), 

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D146090#4319685 , @shafik wrote:

> Landed fix 7887af027ee5eff27bbc953074726ab8d9d9f592 
> 
>
> There was also 058f04ea7dcbafbeed271fa75ee65e41409b4479 
>  and 
> b323b407f76d22bfc08b1430f7952c03eb504288 
> 

Thanks for fixing these!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D147823: [clang-repl] Reduce dynamic-library.cpp test to only load shared library

[Han Zhu via Phabricator via cfe-commits]

zhuhan0 abandoned this revision.
zhuhan0 added a comment.

Superseded by https://reviews.llvm.org/D148992.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147823

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

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/test/MC/RISCV/rv32zcmt-invalid.s:9
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 255]
+cm.jalt 256

This is wrong; the immediate must be in the range [32, 255]. This needs to be 
enforced in the assembler and the error needs to reflect that. We also need 
tests that check these cases (cm.jt with something in [32, 255] and cm.jalt 
with something in [0, 31]).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

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

[PATCH] D149737: [clang][ExtractAPI] Add semicolon to function declaration fragments

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14805dcb0d8a: [clang][ExtractAPI] Add semicolon to function 
declaration fragments (authored by chaitanyav).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149737

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/availability.c
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/macro_undefined.c

Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -67,7 +67,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -173,7 +173,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "functionSignature": {
Index: clang/test/ExtractAPI/global_record_multifile.c
===
--- clang/test/ExtractAPI/global_record_multifile.c
+++ clang/test/ExtractAPI/global_record_multifile.c
@@ -191,7 +191,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/global_record.c
===
--- clang/test/ExtractAPI/global_record.c
+++ clang/test/ExtractAPI/global_record.c
@@ -189,7 +189,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/availability.c
===
--- clang/test/ExtractAPI/availability.c
+++ clang/test/ExtractAPI/availability.c
@@ -76,7 +76,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -150,7 +150,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -234,7 +234,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -334,7 +334,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -416,7 +416,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -457,7 +457,7 @@
   Fragments.append(")", DeclarationFragments::FragmentKind::Text);
 
   // FIXME: Handle exception specifiers: throw, noexcept
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForEnumConstant(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 14805dc - [clang][ExtractAPI] Add semicolon to function declaration fragments

[NagaChaitanya Vellanki via cfe-commits]

Author: NagaChaitanya Vellanki
Date: 2023-05-04T11:46:43-07:00
New Revision: 14805dcb0d8abfd5cd015656313f3f98a22e0a1b

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

LOG: [clang][ExtractAPI] Add semicolon to function declaration fragments

Add missing semicolon at the end of function declarations to fragments

Reviewed By: dang

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

Added: 


Modified: 
clang/lib/ExtractAPI/DeclarationFragments.cpp
clang/test/ExtractAPI/availability.c
clang/test/ExtractAPI/global_record.c
clang/test/ExtractAPI/global_record_multifile.c
clang/test/ExtractAPI/macro_undefined.c

Removed: 




diff  --git a/clang/lib/ExtractAPI/DeclarationFragments.cpp 
b/clang/lib/ExtractAPI/DeclarationFragments.cpp
index da75a701b405e..1e52f221c7982 100644
--- a/clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ b/clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -457,7 +457,7 @@ DeclarationFragmentsBuilder::getFragmentsForFunction(const 
FunctionDecl *Func) {
   Fragments.append(")", DeclarationFragments::FragmentKind::Text);
 
   // FIXME: Handle exception specifiers: throw, noexcept
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForEnumConstant(

diff  --git a/clang/test/ExtractAPI/availability.c 
b/clang/test/ExtractAPI/availability.c
index 7d071909a092e..0c8cd3629f3fd 100644
--- a/clang/test/ExtractAPI/availability.c
+++ b/clang/test/ExtractAPI/availability.c
@@ -76,7 +76,7 @@ void e(void) __attribute__((availability(tvos, unavailable)));
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -150,7 +150,7 @@ void e(void) __attribute__((availability(tvos, 
unavailable)));
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -234,7 +234,7 @@ void e(void) __attribute__((availability(tvos, 
unavailable)));
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -334,7 +334,7 @@ void e(void) __attribute__((availability(tvos, 
unavailable)));
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -416,7 +416,7 @@ void e(void) __attribute__((availability(tvos, 
unavailable)));
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {

diff  --git a/clang/test/ExtractAPI/global_record.c 
b/clang/test/ExtractAPI/global_record.c
index 8516ac50be858..7ca89875d66ae 100644
--- a/clang/test/ExtractAPI/global_record.c
+++ b/clang/test/ExtractAPI/global_record.c
@@ -189,7 +189,7 @@ char unavailable __attribute__((unavailable));
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "docComment": {

diff  --git a/clang/test/ExtractAPI/global_record_multifile.c 
b/clang/test/ExtractAPI/global_record_multifile.c
index 0eacb8a94b77f..b1e9f9ed21d42 100644
--- a/clang/test/ExtractAPI/global_record_multifile.c
+++ b/clang/test/ExtractAPI/global_record_multifile.c
@@ -191,7 +191,7 @@ char unavailable __attribute__((unavailable));
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "docComment": {

diff  --git a/clang/test/ExtractAPI/macro_undefined.c 
b/clang/test/ExtractAPI/macro_undefined.c
index c7d3dd6cfea8c..f150c10fb2ed6 100644
--- a/clang/test/ExtractAPI/macro_undefined.c
+++ b/clang/test/ExtractAPI/macro_undefined.c
@@ -67,7 +67,7 @@ FUNC_GEN(bar, const int *, unsigned);
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -173,7 +173,7 @@ FUNC_GEN(bar, const int *, unsigned);
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "functionSignature": {



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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Landed fix 7887af027ee5eff27bbc953074726ab8d9d9f592 


There was also 058f04ea7dcbafbeed271fa75ee65e41409b4479 
 and 
b323b407f76d22bfc08b1430f7952c03eb504288 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D149884: [clang][deps] Teach dep directive scanner about _Pragma

[Ben Langmuir via Phabricator via cfe-commits]

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

While we cannot handle `_Pragma` used inside macros, we an handle this at the 
top level, and it some projects use the `_Pragma("once")` spelling like that, 
which was causing spurious failures in the scanner.

Limitations

- Cannot handle #define ONCE _Pragma("once"), same issue as using @import in a 
macro -- ideally we should diagnose this in obvious cases
- Our LangOpts are currently fixed, so we are not handling u"" strings or R"()" 
strings that require C11/C++11.

rdar://108629982


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149884

Files:
  clang/include/clang/Lex/Pragma.h
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/lib/Lex/Pragma.cpp
  clang/test/ClangScanDeps/_Pragma-once.c
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -503,6 +503,92 @@
   EXPECT_STREQ("#pragma clang module import\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, UnderscorePragma) {
+  SmallVector Out;
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(R"(_)", Out));
+  EXPECT_STREQ("\n", Out.data());
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(R"(_Pragma)", Out));
+  EXPECT_STREQ("\n", Out.data());
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(R"(_Pragma()", Out));
+  EXPECT_STREQ("\n", Out.data());
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(R"(_Pragma())", Out));
+  EXPECT_STREQ("\n", Out.data());
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(R"(_Pragma(")", Out));
+  EXPECT_STREQ("\n", Out.data());
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(R"(_Pragma("A"))", Out));
+  EXPECT_STREQ("\n", Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"x(_Pragma("push_macro(\"MACRO\")"))x", Out));
+  EXPECT_STREQ(R"x(_Pragma("push_macro(\"MACRO\")"))x"
+   "\n",
+   Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"x(_Pragma("pop_macro(\"MACRO\")"))x", Out));
+  EXPECT_STREQ(R"x(_Pragma("pop_macro(\"MACRO\")"))x"
+   "\n",
+   Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"x(_Pragma("include_alias(\"A\", \"B\")"))x", Out));
+  EXPECT_STREQ(R"x(_Pragma("include_alias(\"A\", \"B\")"))x"
+   "\n",
+   Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"x(_Pragma("include_alias(, )"))x", Out));
+  EXPECT_STREQ(R"x(_Pragma("include_alias(, )"))x"
+   "\n",
+   Out.data());
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives(R"(_Pragma("clang"))", Out));
+  EXPECT_STREQ("\n", Out.data());
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives(R"(_Pragma("clang module"))", Out));
+  EXPECT_STREQ("\n", Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"(_Pragma("clang module impor"))", Out));
+  EXPECT_STREQ("\n", Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"(_Pragma("clang module import"))", Out));
+  EXPECT_STREQ(R"(_Pragma("clang module import"))"
+   "\n",
+   Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"(_Pragma("clang \
+  module \
+  import"))",
+  Out));
+  EXPECT_STREQ(R"(_Pragma("clang \
+  module \
+  import"))"
+   "\n",
+   Out.data());
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"(_Pragma(L"clang module import"))", Out));
+  EXPECT_STREQ(R"(_Pragma(L"clang module import"))"
+   "\n",
+   Out.data());
+
+  // FIXME: u"" strings depend on using C11 language mode
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"(_Pragma(u"clang module import"))", Out));
+  EXPECT_STREQ("\n", Out.data());
+
+  // FIXME: R"()" strings depend on using C++ 11 language mode
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
+  R"(_Pragma(R"abc(clang module import)abc"))", Out));
+  EXPECT_STREQ("\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, Include) {
   SmallVector Out;
 
@@ -757,20 +843,26 @@
 #pragma once
 // another comment
 #include 
+_Pragma("once")
 )";
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
-  EXPECT_STREQ("#pragma once\n#include \n", Out.data());
-  ASSERT_EQ(Directives.size(), 3u);
+  EXPECT_STREQ("#pragma once\n#include \n_Pragma(\"once\")\n",
+   Out.data());
+  ASSERT_EQ(Directives.size(), 4u);
   

[PATCH] D149677: [clang][TypePrinter] Add option to skip over elaborated types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Will this new printing policy be used in tree?




Comment at: clang/include/clang/AST/PrettyPrinter.h:143-145
+  /// Ignore qualifiers as specified by elaborated type sugar, instead letting
+  /// the underlying type handle printing the qualifiers.
+  unsigned IgnoreElaboratedQualifiers : 1;

The name is somewhat confusing to me, because tag names are also elaborations 
and those are handled by `SuppressTagKeyword` -- so what is the interaction 
between those when the user specifies true for `IgnoreElaboratedQualifiers` and 
false for `SuppressTagKeyword`?



Comment at: clang/unittests/AST/TypePrinterTest.cpp:116-119
+  ASSERT_TRUE(PrintedTypeMatches(Code, {}, Matcher, //
+ "a::S",//
+ [](PrintingPolicy ) {
+   Policy.FullyQualifiedName = true; //




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149677

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

[clang] 7887af0 - Fix build bots due to missing a commit thay change 2b to 23

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2023-05-04T11:33:58-07:00
New Revision: 7887af027ee5eff27bbc953074726ab8d9d9f592

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

LOG: Fix build bots due to missing a commit thay change 2b to 23

Fixing build bot breaks due to b4692f29263006c7ea519c7b11c9082384f0af53

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
clang/test/CXX/class/class.compare/class.compare.default/p4.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7df963f7ba05..b496900fac06 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9431,15 +9431,15 @@ def ext_defaulted_comparison_constexpr_mismatch : 
Extension<
   "three-way comparison operator}0 that is "
   "declared %select{constexpr|consteval}2 but"
   "%select{|for which the corresponding implicit 'operator==' }0 "
-  "invokes a non-constexpr comparison function is a C++2b extension">,
-  InGroup>;
-def warn_cxx2b_compat_defaulted_comparison_constexpr_mismatch : Warning<
+  "invokes a non-constexpr comparison function is a C++23 extension">,
+  InGroup>;
+def warn_cxx23_compat_defaulted_comparison_constexpr_mismatch : Warning<
   "defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|"
   "three-way comparison operator}0 that is "
   "declared %select{constexpr|consteval}2 but"
   "%select{|for which the corresponding implicit 'operator==' }0 "
   "invokes a non-constexpr comparison function is incompatible with C++ "
-  "standards before C++2b">,
+  "standards before C++23">,
   InGroup, DefaultIgnore;
 def note_defaulted_comparison_not_constexpr : Note<
   "non-constexpr comparison function would be used to compare "

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 208e34a40e48..dcb3e676b0ba 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8830,7 +8830,7 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
 !Info.Constexpr) {
   Diag(FD->getBeginLoc(),
getLangOpts().CPlusPlus23
-   ? 
diag::warn_cxx2b_compat_defaulted_comparison_constexpr_mismatch
+   ? 
diag::warn_cxx23_compat_defaulted_comparison_constexpr_mismatch
: diag::ext_defaulted_comparison_constexpr_mismatch)
   << FD->isImplicit() << (int)DCK << FD->isConsteval();
   DefaultedComparisonAnalyzer(*this, RD, FD, DCK,

diff  --git a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp 
b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
index a64b8b895f61..166bd97e2731 100644
--- a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
+++ b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
@@ -2,7 +2,7 @@
 // Also covers modifications made by P2448R2 and extension warnings
 
 // RUN: %clang_cc1 -std=c++2a -verify %s
-// RUN: %clang_cc1 -std=c++2a -Wc++2b-default-comp-relaxed-constexpr 
-verify=expected,extension %s
+// RUN: %clang_cc1 -std=c++2a -Wc++23-default-comp-relaxed-constexpr 
-verify=expected,extension %s
 
 namespace std {
   struct strong_ordering {
@@ -131,10 +131,10 @@ struct E {
   A a;
   C c; // extension-note 2{{non-constexpr comparison function would be used to 
compare member 'c'}}
   A b;
-  friend constexpr bool operator==(const E&, const E&) = default; // 
extension-warning {{declared constexpr but invokes a non-constexpr comparison 
function is a C++2b extension}}
+  friend constexpr bool operator==(const E&, const E&) = default; // 
extension-warning {{declared constexpr but invokes a non-constexpr comparison 
function is a C++23 extension}}
   friend constexpr bool operator!=(const E&, const E&) = default;
 
-  friend constexpr std::strong_ordering operator<=>(const E&, const E&) = 
default; // extension-warning {{declared constexpr but invokes a non-constexpr 
comparison function is a C++2b extension}}
+  friend constexpr std::strong_ordering operator<=>(const E&, const E&) = 
default; // extension-warning {{declared constexpr but invokes a non-constexpr 
comparison function is a C++23 extension}}
   friend constexpr bool operator<(const E&, const E&) = default;
   friend constexpr bool operator<=(const E&, const E&) = default;
   friend constexpr bool operator>(const E&, const E&) = default;
@@ -142,10 +142,10 @@ struct E {
 };
 
 struct E2 : A, C { // extension-note 2{{non-constexpr comparison function 
would be used to compare base class 'C'}}
-  friend constexpr bool operator==(const E2&, const E2&) = default; // 

[clang] 058f04e - [clang] Fix another case where CPlusPlus2b is still used.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2023-05-04T14:27:33-04:00
New Revision: 058f04ea7dcbafbeed271fa75ee65e41409b4479

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

LOG: [clang] Fix another case where CPlusPlus2b is still used.

Added: 


Modified: 
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 5b7ee09ac4c7..208e34a40e48 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8829,7 +8829,7 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
 CheckConstexprParameterTypes(*this, FD, CheckConstexprKind::Diagnose) 
&&
 !Info.Constexpr) {
   Diag(FD->getBeginLoc(),
-   getLangOpts().CPlusPlus2b
+   getLangOpts().CPlusPlus23
? 
diag::warn_cxx2b_compat_defaulted_comparison_constexpr_mismatch
: diag::ext_defaulted_comparison_constexpr_mismatch)
   << FD->isImplicit() << (int)DCK << FD->isConsteval();



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

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found some related failures that need to be addressed, but the 
functional changes are all looking good to me. Be sure to also add a release 
note for the fix as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764

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

[clang] b323b40 - [clang] Fix build after https://reviews.llvm.org/D149553

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2023-05-04T14:23:46-04:00
New Revision: b323b407f76d22bfc08b1430f7952c03eb504288

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

LOG: [clang] Fix build after https://reviews.llvm.org/D149553

- `CXXPre2bCompat` is referenced somewhere after being removed.
- More warning messages on c++2b need refining

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f0ff7d0f3169..7df963f7ba05 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9440,7 +9440,7 @@ def 
warn_cxx2b_compat_defaulted_comparison_constexpr_mismatch : Warning<
   "%select{|for which the corresponding implicit 'operator==' }0 "
   "invokes a non-constexpr comparison function is incompatible with C++ "
   "standards before C++2b">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def note_defaulted_comparison_not_constexpr : Note<
   "non-constexpr comparison function would be used to compare "
   "%select{|member %1|base class %1}0">;



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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D146090#4319561 , @clementval 
wrote:

> This commit is failing couple of buildbots. Can you revert or fix?

Shafik is validating a fix for it right now, it should be fixed momentarily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

I am about to fix it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D149831: [clang][Interp] Fix ignoring SubstNonTypeTemplateParmExpr

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149831

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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

This commit is failing couple of buildbots. Can you revert or fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav added a comment.

@shafik @aaron.ballman I see some errors

  [1/2496] Building DiagnosticAnalysisKinds.inc...
  FAILED: tools/clang/include/clang/Basic/DiagnosticAnalysisKinds.inc 
/home/nvellanki/scratch/llvm-project/tools/clang/include/clang/Basic/DiagnosticAnalysisKinds.inc
 
  cd /home/nvellanki/scratch/llvm-project && 
/home/nvellanki/scratch/llvm-project/bin/clang-tblgen -gen-clang-diags-defs 
-clang-component=Analysis -I 
/home/nvellanki/scratch/llvm-project/clang/include/clang/Basic 
-I/home/nvellanki/scratch/llvm-project/clang/include 
-I/home/nvellanki/scratch/llvm-project/tools/clang/include 
-I/home/nvellanki/scratch/llvm-project/include 
-I/home/nvellanki/scratch/llvm-project/llvm/include 
/home/nvellanki/scratch/llvm-project/clang/include/clang/Basic/Diagnostic.td 
--write-if-changed -o 
tools/clang/include/clang/Basic/DiagnosticAnalysisKinds.inc -d 
tools/clang/include/clang/Basic/DiagnosticAnalysisKinds.inc.d
  Included from 
/home/nvellanki/scratch/llvm-project/clang/include/clang/Basic/Diagnostic.td:168:
  
/home/nvellanki/scratch/llvm-project/clang/include/clang/Basic/DiagnosticSemaKinds.td:9443:11:
 error: Variable not defined: 'CXXPre2bCompat'
InGroup, DefaultIgnore;
^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

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

[clang] b4692f2 - [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2023-05-04T11:07:16-07:00
New Revision: b4692f29263006c7ea519c7b11c9082384f0af53

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

LOG: [Clang] Updating handling of defaulted comparison operators to reflect 
changes from P2448R2

Prior to P2448R2 we were more aggressive in diagnosing ill-formed
constexpr functions. Many of these restrictions were relaxed and now it
is not required for defaulted comparison operators to call constexpr
functions.

This behavior is extended to before C++23 and diagnostic for it's use
can be enabled w/ -pedantic or -Wc++2b-default-comp-relaxed-constexpr

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

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8fda486228dbc..e0ad5f5e3ae53 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -100,6 +100,12 @@ C++23 Feature Support
   and `P2579R0 Mitigation strategies for P2036 `_.
   These proposals modify how variables captured in lambdas can appear in 
trailing return type
   expressions and how their types are deduced therein, in all C++ language 
versions.
+- Implemented partial support for `P2448R2: Relaxing some constexpr 
restrictions `_
+  Explicitly defaulted functions no longer have to be constexpr-compatible but 
merely constexpr suitable.
+  We do not support outside of defaulted special memeber functions the change 
that constexpr functions no
+  longer have to be constexpr compatible but rather support a less restricted 
requirements for constexpr
+  functions. Which include allowing non-literal types as return values and 
paremeters, allow calling of
+  non-constexpr functions and constructors.
 
 Resolutions to C++ Defect Reports
 ^

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7a20f2acbdede..f0ff7d0f3169b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9426,12 +9426,21 @@ def 
note_defaulted_comparison_cannot_deduce_undeduced_auto : Note<
   "%select{|member|base class}0 %1 declared here">;
 def note_defaulted_comparison_cannot_deduce_callee : Note<
   "selected 'operator<=>' for %select{|member|base class}0 %1 declared here">;
-def err_incorrect_defaulted_comparison_constexpr : Error<
+def ext_defaulted_comparison_constexpr_mismatch : Extension<
   "defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|"
-  "three-way comparison operator}0 "
-  "cannot be declared %select{constexpr|consteval}2 because "
-  "%select{it|the corresponding implicit 'operator=='}0 "
-  "invokes a non-constexpr comparison function">;
+  "three-way comparison operator}0 that is "
+  "declared %select{constexpr|consteval}2 but"
+  "%select{|for which the corresponding implicit 'operator==' }0 "
+  "invokes a non-constexpr comparison function is a C++2b extension">,
+  InGroup>;
+def warn_cxx2b_compat_defaulted_comparison_constexpr_mismatch : Warning<
+  "defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|"
+  "three-way comparison operator}0 that is "
+  "declared %select{constexpr|consteval}2 but"
+  "%select{|for which the corresponding implicit 'operator==' }0 "
+  "invokes a non-constexpr comparison function is incompatible with C++ "
+  "standards before C++2b">,
+  InGroup, DefaultIgnore;
 def note_defaulted_comparison_not_constexpr : Note<
   "non-constexpr comparison function would be used to compare "
   "%select{|member %1|base class %1}0">;

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a746d8c50a0cc..5b7ee09ac4c72 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8813,12 +8813,25 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
   //   the requirements for a constexpr function [...]
   // The only relevant requirements are that the parameter and return types are
   // literal types. The remaining conditions are checked by the analyzer.
+  //
+  // We support P2448R2 in language modes earlier than C++23 as an extension.
+  // The concept of 

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

[Shafik Yaghmour 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 rGb4692f292630: [Clang] Updating handling of defaulted 
comparison operators to reflect changes… (authored by shafik).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146090

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1480,7 +1480,14 @@
 
   Relaxing some constexpr restrictions
   https://wg21.link/P2448R2;>P2448R2
-  No
+  
+Clang 17 (Partial)
+	  We do not support outside of defaulted special memeber functions the change that constexpr functions no
+  longer have to be constexpr compatible but rather support a less restricted requirements for constexpr
+  functions. Which include allowing non-literal types as return values and paremeters, allow calling of
+  non-constexpr functions and constructors.
+
+  
 
 
   Using unknown pointers and references in constant expressions
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -242,6 +242,7 @@
 
 // - every constructor involved in initializing non-static data members and base
 //   class sub-objects shall be a constexpr constructor.
+// This will no longer be the case once we support P2448R2
 struct ConstexprBaseMemberCtors : Literal {
   Literal l;
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -41,6 +41,8 @@
   virtual constexpr int OutOfLineVirtual() const; // beforecxx20-error {{virtual function cannot be constexpr}}
 
   //  - its return type shall be a literal type;
+  // Once we support P2448R2 constexpr functions will be allowd to return non-literal types
+  // The destructor will also be allowed
   constexpr NonLiteral NonLiteralReturn() const { return {}; } // expected-error {{constexpr function's return type 'NonLiteral' is not a literal type}}
   constexpr void VoidReturn() const { return; }// beforecxx14-error {{constexpr function's return type 'void' is not a literal type}}
   constexpr ~T();  // beforecxx20-error {{destructor cannot be declared constexpr}}
@@ -49,6 +51,7 @@
   constexpr F NonLiteralReturn2; // ok until definition
 
   //  - each of its parameter types shall be a literal type;
+  // Once we support P2448R2 constexpr functions will be allowd to have parameters of non-literal types
   constexpr int NonLiteralParam(NonLiteral) const { return 0; } // expected-error {{constexpr function's 1st parameter type 'NonLiteral' is not a literal type}}
   typedef int G(NonLiteral) const;
   constexpr G NonLiteralParam2; // ok until definition
Index: clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -Wc++2b-default-comp-relaxed-constexpr -verify=expected,extension %s
 
 // This test is for [class.compare.default]p3 as modified and renumbered to p4
 // by P2002R0.
+// Also covers modifications made by P2448R2 and extension warnings
 
 namespace std {
   struct strong_ordering {
@@ -76,14 +78,13 @@
 }
 
 struct H {
-  bool operator==(const H&) const; // expected-note {{here}}
+  bool operator==(const H&) const; // extension-note {{non-constexpr comparison function declared here}}
   constexpr std::strong_ordering operator<=>(const H&) const { return std::strong_ordering::equal; }
 };
 
 struct I {
-  H h; // expected-note {{used to compare}}
-  // expected-error@+1 {{defaulted definition of three-way comparison operator cannot be declared constexpr because the corresponding implicit 'operator==' invokes a non-constexpr comparison function}}
-  constexpr std::strong_ordering operator<=>(const I&) const = default;
+  H h; // extension-note {{non-constexpr 

[PATCH] D149837: [clang][Interp] Fix ignoring CompoundLiteralExprs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:878
 
+(int){1};
+(int[]){1,2,3};

Can you also add a test like:
```
constexpr int oh_my(int x) {
  (int){ x++ };
  return x;
}
static_assert(oh_my(0) == 1);
```
and
```
constexpr int oh_my(int x) {
  int y{x++};
  return x;
}

static_assert(oh_my(0) == 1);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149837

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Thanks Duncan.

> Given that header maps are somewhat Apple-specific

Some non-obvious background: It began Apple specific, but Meta uses them at 
scale as well, pretty important for us to get this right.

> and unit test coverage is a bit lacking for these sorts of interactions, it'd 
> be nice for someone to check this with Apple-internal stuff.

+1, we got some feedback from Apple folks some time ago, trying to revive this 
so @ivanmurashko and others can kill more internal technical debt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D149650: Give NullabilityKind a printing operator<

[Aaron Ballman via Phabricator via cfe-commits]

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

In D149650#4319461 , @sammccall wrote:

> In D149650#4316138 , @aaron.ballman 
> wrote:
>
>> In D149650#4315211 , @sammccall 
>> wrote:
>>
>>> In D149650#4312389 , 
>>> @aaron.ballman wrote:
>>>
 I guess I'm not seeing much motivation for this change. We already have 
 `clang::getNullabilitySpelling()` and `const StreamingDiagnostic 
 ::operator<<(const StreamingDiagnostic , DiagNullabilityKind 
 nullability)` and now we're adding a third way to get this information. If 
 this is just for debug/testing purposes, can we use existing debug 
 formatters to convert the enumeration value into the enumerator name 
 instead?
>>>
>>> We're using NullabilityKind in our dataflow-based null-safety clang-tidy 
>>> check  that we hope 
>>> to upstream when it works.
>>
>> Ah, good to know! I think it would be reasonable to add this functionality 
>> once there's some agreement on adding the clang-tidy check.
>>
>>> (The idea is to use `_Nullable` and `_Nonnull` annotations on API 
>>> boundaries to gradually type pointers, and to provide a verification 
>>> clang-tidy check and libraries to infer annotations for existing code. The 
>>> actual clang-tidy check wrapper isn't in that repo yet, but it's trivial).
>>>
>>> It would be convenient if e.g. EXPECT_THAT(someVectorOfNK, 
>>> ElementsAre(Nullable, Unspecified)) 
>>> 
>>>  printed something useful when it fails, if we could write OS << NK and get 
>>> Unspecified rather than OS << getSpelling(NK) and get 
>>> _Unspecified_Nullability, etc.
>>> This doesn't concern clang core (ha, there are no unit tests...) but 
>>> there's no reasonable way to do this downstream without using a different 
>>> type.
>>
>> Hmmm, but does this need to be added to `Specifiers.h` instead of being kept 
>> packaged up with clang-tidy? For things like AST matchers, we usually ask 
>> that the matcher remain local to the clang-tidy check unless the same 
>> matcher is needed in multiple checks. This feels somewhat similar, despite 
>> it not being related to AST matching -- if the only need for this 
>> functionality exists out of the clang tree, can we keep it out of the clang 
>> tree until we find more needs?
>
> This can't be added downstream, the type needs to provide these.
> ADL extension points like operators need to be in namespace `clang`[1] so 
> that ADL will find them. If non-owners of the type define these, this ends up 
> in violating ODR when two such non-owners are linked together. Hacking around 
> this for the `operator<<` symbol itself (e.g. putting it inline in anonymous 
> namespaces) results in ODR violations in templates that call them (in this 
> case, gtest). Even defining them centrally in a separate header is risky, 
> including/not including the header produces similar violations.

Oh shoot, you're right. I hadn't thought about the fact that this is an ADL 
extension point specifically.

> There's never going to be a hard need for this - having an ergonomic way to 
> print things that composes with the rest of our infra is useful but it's 
> always possible to live without it.
> If we don't want such clang-as-a-library features upstream that's OK. We can 
> define our own `NullabilityKind` and marshal between them - I need to be able 
> to answer "why are we hacking around clang rather than improving it" in code 
> review :-)

I think you've convinced me this is reasonable to keep upstream despite not 
really being used in tree (yet). If it turns out that the clang-tidy check 
doesn't get upstreamed, we can remove this easily enough in the future if 
needed. I don't think we want to expose this sort of thing as a matter of 
course, but when the alternatives are significantly more dangerous, it is 
reasonable. If we find we're adding more clang-as a-library features upstream 
like this, we might want to revisit whether there's a better design approach 
for these needs.

LG!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added reviewers: Bigcheese, ChuanqiXu, jansvoboda11.
bruno added a comment.

Adding code owners and more relevant folks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D149850: [Clang][Modules] Support `requires cplusplus20` in a modulemap

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149850

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

[PATCH] D149650: Give NullabilityKind a printing operator<

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D149650#4316138 , @aaron.ballman 
wrote:

> In D149650#4315211 , @sammccall 
> wrote:
>
>> In D149650#4312389 , 
>> @aaron.ballman wrote:
>>
>>> I guess I'm not seeing much motivation for this change. We already have 
>>> `clang::getNullabilitySpelling()` and `const StreamingDiagnostic 
>>> ::operator<<(const StreamingDiagnostic , DiagNullabilityKind 
>>> nullability)` and now we're adding a third way to get this information. If 
>>> this is just for debug/testing purposes, can we use existing debug 
>>> formatters to convert the enumeration value into the enumerator name 
>>> instead?
>>
>> We're using NullabilityKind in our dataflow-based null-safety clang-tidy 
>> check  that we hope 
>> to upstream when it works.
>
> Ah, good to know! I think it would be reasonable to add this functionality 
> once there's some agreement on adding the clang-tidy check.
>
>> (The idea is to use `_Nullable` and `_Nonnull` annotations on API boundaries 
>> to gradually type pointers, and to provide a verification clang-tidy check 
>> and libraries to infer annotations for existing code. The actual clang-tidy 
>> check wrapper isn't in that repo yet, but it's trivial).
>>
>> It would be convenient if e.g. EXPECT_THAT(someVectorOfNK, 
>> ElementsAre(Nullable, Unspecified)) 
>> 
>>  printed something useful when it fails, if we could write OS << NK and get 
>> Unspecified rather than OS << getSpelling(NK) and get 
>> _Unspecified_Nullability, etc.
>> This doesn't concern clang core (ha, there are no unit tests...) but there's 
>> no reasonable way to do this downstream without using a different type.
>
> Hmmm, but does this need to be added to `Specifiers.h` instead of being kept 
> packaged up with clang-tidy? For things like AST matchers, we usually ask 
> that the matcher remain local to the clang-tidy check unless the same matcher 
> is needed in multiple checks. This feels somewhat similar, despite it not 
> being related to AST matching -- if the only need for this functionality 
> exists out of the clang tree, can we keep it out of the clang tree until we 
> find more needs?

This can't be added downstream, the type needs to provide these.
ADL extension points like operators need to be in namespace `clang`[1] so that 
ADL will find them. If non-owners of the type define these, this ends up in 
violating ODR when two such non-owners are linked together. Hacking around this 
for the `operator<<` symbol itself (e.g. putting it inline in anonymous 
namespaces) results in ODR violations in templates that call them (in this 
case, gtest). Even defining them centrally in a separate header is risky, 
including/not including the header produces similar violations.

There's never going to be a hard need for this - having an ergonomic way to 
print things that composes with the rest of our infra is useful but it's always 
possible to live without it.
If we don't want such clang-as-a-library features upstream that's OK. We can 
define our own `NullabilityKind` and marshal between them - I need to be able 
to answer "why are we hacking around clang rather than improving it" in code 
review :-)

[1] (or `llvm`, or `::`, or `::testing` - these all have the same issues).

>>> `StreamingDiagnostic ::operator<<`
>>
>> This actually wants the user-visible spelling, so I think it can just use 
>> getSpelling... if I make that change we're back to just two implementations 
>> :-)
>
> Oh, I really like those changes! Feel free to land that as an NFC change if 
> you'd like (the addition of quotes is a bug fix, but not really a functional 
> change).

Thanks - will do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

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

[PATCH] D149834: [clang][Interp] Fix ignoring TypeTraitExprs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:875
 1 ? 0 : 1;
+sizeof(A);
+alignof(A);

Let's make sure we still reject this:
```
constexpr int oh_my() {
  int x = 0;
  sizeof(int[x++]); // This would usually be evaluated because VLAs are terrible
  return x;
}
```
https://godbolt.org/z/E3jx6co46


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149834

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

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329
+static void
+builtinTransferScopeEnd(const CFGScopeEnd ,
+TypeErasedDataflowAnalysisState ) {

mboehme wrote:
> mboehme wrote:
> > xazax.hun wrote:
> > > I think one question is whether we are interested in ScopeEnd or 
> > > LifetimeEnd, see the discussions in https://reviews.llvm.org/D15031 and 
> > > https://reviews.llvm.org/D16403, specifically Devin's comment: 
> > > 
> > > >>! In D16403#799926, @dcoughlin wrote:
> > > >> @dcoughlin As a reviewer of both patches - could you tell us what's 
> > > >> the difference between them? And how are we going to resolve this 
> > > >> issue?
> > > > 
> > > > These two patches are emitting markers in the CFG for different things.
> > > > 
> > > > Here is how I would characterize the difference between the two patches.
> > > > 
> > > > - Despite its name, https://reviews.llvm.org/D15031, is really emitting 
> > > > markers for when the lifetime of a C++ object in an automatic variable 
> > > > ends.  For C++ objects with non-trivial destructors, this point is when 
> > > > the destructor is called. At this point the storage for the variable 
> > > > still exists, but what you can do with that storage is very restricted 
> > > > by the language because its contents have been destroyed. Note that 
> > > > even with the contents of the object have been destroyed, it is still 
> > > > meaningful to, say, take the address of the variable and construct a 
> > > > new object into it with a placement new. In other words, the same 
> > > > variable can have different objects, with different lifetimes, in it at 
> > > > different program points.
> > > > 
> > > > - In contrast, the purpose of this patch 
> > > > (https://reviews.llvm.org/D16403) is to add markers for when the 
> > > > storage duration for the variable begins and ends (this is, when the 
> > > > storage exists). Once the storage duration has ended, you can't 
> > > > placement new into the variables address, because another variable may 
> > > > already be at that address.
> > > > 
> > > > I don't think there is an "issue" to resolve here.  We should make sure 
> > > > the two patches play nicely with each other, though. In particular, we 
> > > > should make sure that the markers for when lifetime ends and when 
> > > > storage duration ends are ordered correctly.
> > > 
> > > 
> > > What I wanted to add, I wonder if we might not get ScopeEnd event for 
> > > temporaries and there might be other differences. The Clang 
> > > implementation of P1179 used LifetimeEnd instead of ScopeEnd, and I 
> > > believe probably most clients are more interested in LifetimeEnd events 
> > > rather than ScopeEnd.
> > > 
> > > I think I understand why you went with ScopeEnd for this specific 
> > > problem, but to avoid the confusion from having both in the Cfg (because 
> > > I anticipate someone will need LifetimeEnd at some point), I wonder if we 
> > > can make this work with LifetimeEnd. 
> > Hm, thanks for bringing it up.
> > 
> > Conincidentally, I realized while chasing another issue that `CFGScopeEnd` 
> > isn't the right construct here. I assumed that we would get a `CFGScopeEnd` 
> > for every variable, but I now realize that we only get a `CFGScopeEnd` for 
> > the first variable in the scope.
> > 
> > So `CFGLifetimeEnds` definitely looks like the right construct to use here, 
> > and indeed it's what I originally tried to use. Unfortuately, 
> > `CFG::BuildOptions::AddLifetime` cannot be used at the same time as 
> > `AddImplicitDtors`, which we already use. We don't actually need the 
> > `CFGElement`s added by `AddImplicitDtors`, but we do need 
> > `AddTemporaryDtors`, and that in turn can only be turned on if 
> > `AddImplicitDtors` is turned on.
> > 
> > It looks as if I will need to break one of these constraints. It looks as 
> > if the constraint that is easiest to break is that `AddLifetime` currently 
> > cannot be used at the same time as `AddImplicitDtors`. I'm not sure why 
> > this constraint currently exists (I'd appreciate any insights you or others 
> > may have), but I suspect it's because it's hard to ensure that the 
> > `CFGElement`s added by `AddImplicitDtors` are ordered correctly with 
> > respect to the `CFGLifetimeEnds` elements.
> > 
> > Anyway, this requires a change to `CFG` one way or another. I'll work on 
> > that next. I'll then make the required changes to this patch and will ask 
> > for another look before submitting.
> I've taken a look at what it would take to make `AddLifetime` coexist with 
> `AddImplicitDtors`, and it's hard (because we need to get the ordering right, 
> and that's non-trivial).
> 
> So I've instead decided to remove the behavior from this patch that removes 
> declarations from `Environment::DeclToLoc` when they go out of scope and have 
> instead