[clang] abbc2e9 - [CodeGen] Store ElementType in Address

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T08:59:44+01:00
New Revision: abbc2e997bbf8f60ccb18f04c477c28f3ad0a7a2

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

LOG: [CodeGen] Store ElementType in Address

Explicitly track the pointer element type in Address, rather than
deriving it from the pointer type, which will no longer be possible
with opaque pointers. This just adds the basic facility, for now
everything is still going through the deprecated constructors.

I had to adjust one place in the LValue implementation to satisfy
the new assertions: Global registers are represented as a
MetadataAsValue, which does not have a pointer type. We should
avoid using Address in this case.

This implements a part of D103465.

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

Added: 


Modified: 
clang/lib/CodeGen/Address.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGValue.h

Removed: 




diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index 2196c6bf11615..cd9d7cc53cc8b 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -23,18 +23,28 @@ namespace CodeGen {
 /// An aligned address.
 class Address {
   llvm::Value *Pointer;
+  llvm::Type *ElementType;
   CharUnits Alignment;
 
 protected:
-  Address(std::nullptr_t) : Pointer(nullptr) {}
+  Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
-  Address(llvm::Value *pointer, CharUnits alignment)
-  : Pointer(pointer), Alignment(alignment) {
+  Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
+  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
 assert(pointer != nullptr && "Pointer cannot be null");
+assert(elementType != nullptr && "Element type cannot be null");
+assert(llvm::cast(pointer->getType())
+   ->isOpaqueOrPointeeTypeMatches(elementType) &&
+   "Incorrect pointer element type");
 assert(!alignment.isZero() && "Alignment cannot be zero");
   }
 
+  // Deprecated: Use constructor with explicit element type instead.
+  Address(llvm::Value *Pointer, CharUnits Alignment)
+  : Address(Pointer, Pointer->getType()->getPointerElementType(),
+Alignment) {}
+
   static Address invalid() { return Address(nullptr); }
   bool isValid() const { return Pointer != nullptr; }
 
@@ -49,11 +59,9 @@ class Address {
   }
 
   /// Return the type of the values stored in this address.
-  ///
-  /// When IR pointer types lose their element type, we should simply
-  /// store it in Address instead for the convenience of writing code.
   llvm::Type *getElementType() const {
-return getType()->getElementType();
+assert(isValid());
+return ElementType;
   }
 
   /// Return the address space that this address resides in.
@@ -79,8 +87,13 @@ class ConstantAddress : public Address {
   ConstantAddress(std::nullptr_t) : Address(nullptr) {}
 
 public:
+  ConstantAddress(llvm::Constant *pointer, llvm::Type *elementType,
+  CharUnits alignment)
+  : Address(pointer, elementType, alignment) {}
+
+  // Deprecated: Use constructor with explicit element type instead.
   ConstantAddress(llvm::Constant *pointer, CharUnits alignment)
-: Address(pointer, alignment) {}
+  : Address(pointer, alignment) {}
 
   static ConstantAddress invalid() {
 return ConstantAddress(nullptr);
@@ -90,13 +103,10 @@ class ConstantAddress : public Address {
 return llvm::cast(Address::getPointer());
   }
 
-  ConstantAddress getBitCast(llvm::Type *ty) const {
-return ConstantAddress(llvm::ConstantExpr::getBitCast(getPointer(), ty),
-   getAlignment());
-  }
-
-  ConstantAddress getElementBitCast(llvm::Type *ty) const {
-return getBitCast(ty->getPointerTo(getAddressSpace()));
+  ConstantAddress getElementBitCast(llvm::Type *ElemTy) const {
+llvm::Constant *BitCast = llvm::ConstantExpr::getBitCast(
+getPointer(), ElemTy->getPointerTo(getAddressSpace()));
+return ConstantAddress(BitCast, ElemTy, getAlignment());
   }
 
   static bool isaImpl(Address addr) {
@@ -104,7 +114,7 @@ class ConstantAddress : public Address {
   }
   static ConstantAddress castImpl(Address addr) {
 return ConstantAddress(llvm::cast(addr.getPointer()),
-   addr.getAlignment());
+   addr.getElementType(), addr.getAlignment());
   }
 };
 

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 4332e74dbb244..81474136de264 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2610,7 +2610,7 @@ static LValue EmitGlobalNamedRegister(const VarDecl *VD, 
CodeGenModule &CGM) {
 
   llvm::Value *Ptr =
 llvm::MetadataAsValue::get(C

[PATCH] D115725: [CodeGen] Store ElementType in Address

[Nikita Popov 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 rGabbc2e997bbf: [CodeGen] Store ElementType in Address 
(authored by nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115725

Files:
  clang/lib/CodeGen/Address.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGValue.h

Index: clang/lib/CodeGen/CGValue.h
===
--- clang/lib/CodeGen/CGValue.h
+++ clang/lib/CodeGen/CGValue.h
@@ -441,11 +441,12 @@
 return R;
   }
 
-  static LValue MakeGlobalReg(Address Reg, QualType type) {
+  static LValue MakeGlobalReg(llvm::Value *V, CharUnits alignment,
+  QualType type) {
 LValue R;
 R.LVType = GlobalReg;
-R.V = Reg.getPointer();
-R.Initialize(type, type.getQualifiers(), Reg.getAlignment(),
+R.V = V;
+R.Initialize(type, type.getQualifiers(), alignment,
  LValueBaseInfo(AlignmentSource::Decl), TBAAAccessInfo());
 return R;
   }
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2610,7 +2610,7 @@
 
   llvm::Value *Ptr =
 llvm::MetadataAsValue::get(CGM.getLLVMContext(), M->getOperand(0));
-  return LValue::MakeGlobalReg(Address(Ptr, Alignment), VD->getType());
+  return LValue::MakeGlobalReg(Ptr, Alignment, VD->getType());
 }
 
 /// Determine whether we can emit a reference to \p VD from the current
Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -23,18 +23,28 @@
 /// An aligned address.
 class Address {
   llvm::Value *Pointer;
+  llvm::Type *ElementType;
   CharUnits Alignment;
 
 protected:
-  Address(std::nullptr_t) : Pointer(nullptr) {}
+  Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
-  Address(llvm::Value *pointer, CharUnits alignment)
-  : Pointer(pointer), Alignment(alignment) {
+  Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
+  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
 assert(pointer != nullptr && "Pointer cannot be null");
+assert(elementType != nullptr && "Element type cannot be null");
+assert(llvm::cast(pointer->getType())
+   ->isOpaqueOrPointeeTypeMatches(elementType) &&
+   "Incorrect pointer element type");
 assert(!alignment.isZero() && "Alignment cannot be zero");
   }
 
+  // Deprecated: Use constructor with explicit element type instead.
+  Address(llvm::Value *Pointer, CharUnits Alignment)
+  : Address(Pointer, Pointer->getType()->getPointerElementType(),
+Alignment) {}
+
   static Address invalid() { return Address(nullptr); }
   bool isValid() const { return Pointer != nullptr; }
 
@@ -49,11 +59,9 @@
   }
 
   /// Return the type of the values stored in this address.
-  ///
-  /// When IR pointer types lose their element type, we should simply
-  /// store it in Address instead for the convenience of writing code.
   llvm::Type *getElementType() const {
-return getType()->getElementType();
+assert(isValid());
+return ElementType;
   }
 
   /// Return the address space that this address resides in.
@@ -79,8 +87,13 @@
   ConstantAddress(std::nullptr_t) : Address(nullptr) {}
 
 public:
+  ConstantAddress(llvm::Constant *pointer, llvm::Type *elementType,
+  CharUnits alignment)
+  : Address(pointer, elementType, alignment) {}
+
+  // Deprecated: Use constructor with explicit element type instead.
   ConstantAddress(llvm::Constant *pointer, CharUnits alignment)
-: Address(pointer, alignment) {}
+  : Address(pointer, alignment) {}
 
   static ConstantAddress invalid() {
 return ConstantAddress(nullptr);
@@ -90,13 +103,10 @@
 return llvm::cast(Address::getPointer());
   }
 
-  ConstantAddress getBitCast(llvm::Type *ty) const {
-return ConstantAddress(llvm::ConstantExpr::getBitCast(getPointer(), ty),
-   getAlignment());
-  }
-
-  ConstantAddress getElementBitCast(llvm::Type *ty) const {
-return getBitCast(ty->getPointerTo(getAddressSpace()));
+  ConstantAddress getElementBitCast(llvm::Type *ElemTy) const {
+llvm::Constant *BitCast = llvm::ConstantExpr::getBitCast(
+getPointer(), ElemTy->getPointerTo(getAddressSpace()));
+return ConstantAddress(BitCast, ElemTy, getAlignment());
   }
 
   static bool isaImpl(Address addr) {
@@ -104,7 +114,7 @@
   }
   static ConstantAddress castImpl(Address addr) {
 return ConstantAddress(llvm::cast(addr.getPointer()),
-   addr.getAlignment());
+   

[PATCH] D115769: [clang-format] Remove spurious JSON binding when DisableFormat = true

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Thank you for the patch, this looks good but we do need a unit test in 
`clang/unittest/Format/FormatTestJson.cpp` you can then build them with `ninja 
FormatTests`, if you need help let me know


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115769

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

[PATCH] D115769: [clang-format] Remove spurious JSON binding when DisableFormat = true

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

The following will show the issue

  TEST_F(FormatTestJson, DisableJsonFormat) {
FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
verifyFormat("{}", Style);
verifyFormat("{\n"
 "  \"name\": 1\n"
 "}",
 Style);
Style.DisableFormat = true;
verifyFormat("{}", Style);
verifyFormat("{\n"
 "  \"name\": 1\n"
 "}",
 Style);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115769

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

[clang] b4f4655 - [CodeGen] Avoid some pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T09:29:27+01:00
New Revision: b4f46555d7462a88a8743026459ae40412ed4ed2

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

LOG: [CodeGen] Avoid some pointer element type accesses

Added: 


Modified: 
clang/lib/CodeGen/CGAtomic.cpp
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGBuilder.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGExpr.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index b68e6328acdf..a7a9ea2fbbca 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -1079,8 +1079,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   if (AS == LangAS::opencl_generic)
 return V;
   auto DestAS = getContext().getTargetAddressSpace(LangAS::opencl_generic);
-  auto T = V->getType();
-  auto *DestType = T->getPointerElementType()->getPointerTo(DestAS);
+  auto T = llvm::cast(V->getType());
+  auto *DestType = llvm::PointerType::getWithSamePointeeType(T, DestAS);
 
   return getTargetHooks().performAddrSpaceCast(
   *this, V, AS, LangAS::opencl_generic, DestType, false);

diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 2da2014345d8..7bb6dbb8a8ac 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -2721,8 +2721,7 @@ void CodeGenFunction::emitByrefStructureInit(const 
AutoVarEmission &emission) {
   Address addr = emission.Addr;
 
   // That's an alloca of the byref structure type.
-  llvm::StructType *byrefType = cast(
-cast(addr.getPointer()->getType())->getElementType());
+  llvm::StructType *byrefType = cast(addr.getElementType());
 
   unsigned nextHeaderIndex = 0;
   CharUnits nextHeaderOffset;

diff  --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index 4fad44a105cd..f3b759743d38 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -86,7 +86,8 @@ class CGBuilderTy : public CGBuilderBaseTy {
   llvm::LoadInst *CreateAlignedLoad(llvm::Type *Ty, llvm::Value *Addr,
 CharUnits Align,
 const llvm::Twine &Name = "") {
-assert(Addr->getType()->getPointerElementType() == Ty);
+assert(llvm::cast(Addr->getType())
+   ->isOpaqueOrPointeeTypeMatches(Ty));
 return CreateAlignedLoad(Ty, Addr, Align.getAsAlign(), Name);
   }
 
@@ -115,13 +116,15 @@ class CGBuilderTy : public CGBuilderBaseTy {
   /// Emit a load from an i1 flag variable.
   llvm::LoadInst *CreateFlagLoad(llvm::Value *Addr,
  const llvm::Twine &Name = "") {
-assert(Addr->getType()->getPointerElementType() == getInt1Ty());
+assert(llvm::cast(Addr->getType())
+   ->isOpaqueOrPointeeTypeMatches(getInt1Ty()));
 return CreateAlignedLoad(getInt1Ty(), Addr, CharUnits::One(), Name);
   }
 
   /// Emit a store to an i1 flag variable.
   llvm::StoreInst *CreateFlagStore(bool Value, llvm::Value *Addr) {
-assert(Addr->getType()->getPointerElementType() == getInt1Ty());
+assert(llvm::cast(Addr->getType())
+   ->isOpaqueOrPointeeTypeMatches(getInt1Ty()));
 return CreateAlignedStore(getInt1(Value), Addr, CharUnits::One());
   }
 

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 714386bbe4e1..a50f852f9051 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1502,8 +1502,7 @@ Value *CodeGenFunction::EmitMSVCBuiltinExpr(MSVCIntrin 
BuiltinID,
 Value *ArgValue = EmitScalarExpr(E->getArg(1));
 
 llvm::Type *ArgType = ArgValue->getType();
-llvm::Type *IndexType =
-IndexAddress.getPointer()->getType()->getPointerElementType();
+llvm::Type *IndexType = IndexAddress.getElementType();
 llvm::Type *ResultType = ConvertType(E->getType());
 
 Value *ArgZero = llvm::Constant::getNullValue(ArgType);
@@ -16389,8 +16388,7 @@ Value *CodeGenFunction::EmitAMDGPUBuiltinExpr(unsigned 
BuiltinID,
 llvm::Value *Result = Builder.CreateExtractValue(Tmp, 0);
 llvm::Value *Flag = Builder.CreateExtractValue(Tmp, 1);
 
-llvm::Type *RealFlagType
-  = FlagOutPtr.getPointer()->getType()->getPointerElementType();
+llvm::Type *RealFlagType = FlagOutPtr.getElementType();
 
 llvm::Value *FlagExt = Builder.CreateZExt(Flag, RealFlagType);
 Builder.CreateStore(FlagExt, FlagOutPtr);

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 81474136de26..1e36a3defd5b 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1777,16 +1777,14 @@ llvm::Value 
*CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
 

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE_ON_LINUX to emulate GCC --enable-default-pie

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/CMakeLists.txt:232
+if(CLANG_DEFAULT_PIE_ON_LINUX)
+  set(CLANG_DEFAULT_PIE_ON_LINUX 1)
+endif()

Can these 3 lines be removed after D115751?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113372

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

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Hi, the new test is failing on our arm/aarch64 quick bots. These build only the 
respective backend so you might need to add a requires x86 target.

https://lab.llvm.org/buildbot/#/builders/171/builds/7189

  
/home/tcwg-buildbot/worker/clang-armv7-quick/llvm/clang/test/CodeGen/avoidTBAAonASMstore.cpp:5:3:
 error: MS-style inline assembly is not available: No available targets are 
compatible with triple "i386-unknown-linux-gnu"
__asm { fnstcw word ptr[ControlWord]}
^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115320

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

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

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

[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429

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

[clang] c3b624a - [CodeGen] Avoid deprecated ConstantAddress constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T10:42:41+01:00
New Revision: c3b624a191e09f1f16ce32f35b59fc4ca22ec780

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

LOG: [CodeGen] Avoid deprecated ConstantAddress constructor

Change all uses of the deprecated constructor to pass the
element type explicitly and drop it.

For cases where the correct element type was not immediately
obvious to me or would require a slightly larger change I'm
falling back to explicitly calling getPointerElementType() for now.

Added: 


Modified: 
clang/lib/CodeGen/Address.h
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CGObjCGNU.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index cd9d7cc53cc8..0f2d7b1d9e45 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -91,10 +91,6 @@ class ConstantAddress : public Address {
   CharUnits alignment)
   : Address(pointer, elementType, alignment) {}
 
-  // Deprecated: Use constructor with explicit element type instead.
-  ConstantAddress(llvm::Constant *pointer, CharUnits alignment)
-  : Address(pointer, alignment) {}
-
   static ConstantAddress invalid() {
 return ConstantAddress(nullptr);
   }

diff  --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index d22f9dc3b68c..4a8e93e0f4b7 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -201,7 +201,8 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const 
VarDecl &D,
 DeclPtr = llvm::ConstantExpr::getAddrSpaceCast(DeclPtr, PTy);
   }
 
-  ConstantAddress DeclAddr(DeclPtr, getContext().getDeclAlign(&D));
+  ConstantAddress DeclAddr(DeclPtr, 
DeclPtr->getType()->getPointerElementType(),
+   getContext().getDeclAlign(&D));
 
   if (!T->isReferenceType()) {
 if (getLangOpts().OpenMP && !getLangOpts().OpenMPSimd &&

diff  --git a/clang/lib/CodeGen/CGExprConstant.cpp 
b/clang/lib/CodeGen/CGExprConstant.cpp
index ff900ed077e6..47eed6ea2aa8 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -899,7 +899,7 @@ static ConstantAddress 
tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
-return ConstantAddress(Addr, Align);
+return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
 
@@ -921,7 +921,7 @@ static ConstantAddress 
tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
   emitter.finalize(GV);
   GV->setAlignment(Align.getAsAlign());
   CGM.setAddrOfConstantCompoundLiteral(E, GV);
-  return ConstantAddress(GV, Align);
+  return ConstantAddress(GV, GV->getValueType(), Align);
 }
 
 static llvm::Constant *

diff  --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index e016644150b4..b2bf60d2c0fc 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -978,7 +978,9 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
 // Look for an existing one
 llvm::StringMap::iterator old = ObjCStrings.find(Str);
 if (old != ObjCStrings.end())
-  return ConstantAddress(old->getValue(), Align);
+  return ConstantAddress(
+  old->getValue(), old->getValue()->getType()->getPointerElementType(),
+  Align);
 
 bool isNonASCII = SL->containsNonAscii();
 
@@ -1000,7 +1002,7 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
   auto *ObjCStr = llvm::ConstantExpr::getIntToPtr(
   llvm::ConstantInt::get(Int64Ty, str), IdTy);
   ObjCStrings[Str] = ObjCStr;
-  return ConstantAddress(ObjCStr, Align);
+  return ConstantAddress(ObjCStr, IdTy->getPointerElementType(), Align);
 }
 
 StringRef StringClass = CGM.getLangOpts().ObjCConstantStringClass;
@@ -1114,7 +1116,7 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
 llvm::Constant *ObjCStr = llvm::ConstantExpr::getBitCast(ObjCStrGV, IdTy);
 ObjCStrings[Str] = ObjCStr;
 ConstantStrings.push_back(ObjCStr);
-return ConstantAddress(ObjCStr, Align);
+return ConstantAddress(ObjCStr, IdTy->getPointerElementType(), Align);
   }
 
   void PushProperty(ConstantArrayBuilder &PropertiesArray,
@@ -2476,7 +2478,7 @@ ConstantAddress CGObjCGNU::GenerateConstantString(const 
StringLiteral *SL) {
   // Look for an existing one
   llvm::StringMap::iterator old = ObjCStrings.find(Str);
   if (old != ObjCStrings.end())
-r

[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

[Amilendra Kodithuwakku via Phabricator via cfe-commits]

amilendra added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:391-392
 
+  if (!Arch.empty() && !isBranchProtectionSupportedArch(Arch))
+return false;
+

chill wrote:
> On empty `Arch` it'd continue down the function, but we'd like to return 
> failure.
I am having trouble getting the test `arm-branch-protection-attr-1.c` to work 
after these changes. `validateBranchProtection()` checks the combination of two 
parameters, the branch protection attribute and architecture.
If the architecture is empty, like below, shouldn't the function to continue 
checking further than simply returning false? 
```
__attribute__((target("branch-protection=bti"))) void btionly() {}
```
Or should I be using something else other than 
`CGM.getTarget().getTargetOpts().CPU` to get the architecture in 
`ARMTargetCodeGenInfo::setTargetAttributes`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115501

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

[clang-tools-extra] 5298333 - [clangd] Disable support for clang-tidy suppression blocks (NOLINTBEGIN)

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-12-15T10:58:30+01:00
New Revision: 529833377ccdf4381f8bc9961bfa96ec4f5e2eed

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

LOG: [clangd] Disable support for clang-tidy suppression blocks (NOLINTBEGIN)

The implementation is very inefficient and we pay the cost even when the 
feature is not used

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 307db29dc1966..4b96725de4417 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -395,7 +395,8 @@ ParsedAST::build(llvm::StringRef Filename, const 
ParseInputs &Inputs,
   if (IsInsideMainFile &&
   tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
  TidySuppressedErrors,
- /*AllowIO=*/false)) {
+ /*AllowIO=*/false,
+ /*EnableNolintBlocks=*/false)) {
 // FIXME: should we expose the suppression error (invalid use of
 // NOLINT comments)?
 return DiagnosticsEngine::Ignored;



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

[PATCH] D115650: [clangd] Disable support for clang-tidy suppression blocks (NOLINTBEGIN)

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG529833377ccd: [clangd] Disable support for clang-tidy 
suppression blocks (NOLINTBEGIN) (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115650

Files:
  clang-tools-extra/clangd/ParsedAST.cpp


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -395,7 +395,8 @@
   if (IsInsideMainFile &&
   tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
  TidySuppressedErrors,
- /*AllowIO=*/false)) {
+ /*AllowIO=*/false,
+ /*EnableNolintBlocks=*/false)) {
 // FIXME: should we expose the suppression error (invalid use of
 // NOLINT comments)?
 return DiagnosticsEngine::Ignored;


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -395,7 +395,8 @@
   if (IsInsideMainFile &&
   tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
  TidySuppressedErrors,
- /*AllowIO=*/false)) {
+ /*AllowIO=*/false,
+ /*EnableNolintBlocks=*/false)) {
 // FIXME: should we expose the suppression error (invalid use of
 // NOLINT comments)?
 return DiagnosticsEngine::Ignored;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92270: [ConstantFold] Fold more operations to poison

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

I bisected an arm 32 bit issue with WebKit Javascript's infinity value 
(https://github.com/llvm/llvm-project/issues/52669) down to this commit.

Just to make people aware there is another failing case. No reproducer yet I'm 
still working on a way to reduce it, but it'll probably turn out to be covered 
by the points others have made.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92270

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

[clang] 834c8ff - [CodeGen] Avoid some uses of deprecated Address constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T11:13:10+01:00
New Revision: 834c8ff5879839d108af256dfd86bd498ac65efd

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

LOG: [CodeGen] Avoid some uses of deprecated Address constructor

Explicitly pass in the element type instead.

Added: 


Modified: 
clang/lib/CodeGen/CGBuilder.h
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index f3b759743d38..86c108c54878 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -197,6 +197,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 
 return Address(CreateStructGEP(Addr.getElementType(),
Addr.getPointer(), Index, Name),
+   ElTy->getElementType(Index),
Addr.getAlignment().alignmentAtOffset(Offset));
   }
 
@@ -218,6 +219,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 return Address(
 CreateInBoundsGEP(Addr.getElementType(), Addr.getPointer(),
   {getSize(CharUnits::Zero()), getSize(Index)}, Name),
+ElTy->getElementType(),
 Addr.getAlignment().alignmentAtOffset(Index * EltSize));
   }
 
@@ -234,6 +236,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 
 return Address(CreateInBoundsGEP(Addr.getElementType(), Addr.getPointer(),
  getSize(Index), Name),
+   ElTy,
Addr.getAlignment().alignmentAtOffset(Index * EltSize));
   }
 
@@ -250,6 +253,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 
 return Address(CreateGEP(Addr.getElementType(), Addr.getPointer(),
  getSize(Index), Name),
+   Addr.getElementType(),
Addr.getAlignment().alignmentAtOffset(Index * EltSize));
   }
 
@@ -259,6 +263,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 assert(Addr.getElementType() == TypeCache.Int8Ty);
 return Address(CreateInBoundsGEP(Addr.getElementType(), Addr.getPointer(),
  getSize(Offset), Name),
+   Addr.getElementType(),
Addr.getAlignment().alignmentAtOffset(Offset));
   }
   Address CreateConstByteGEP(Address Addr, CharUnits Offset,
@@ -266,6 +271,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 assert(Addr.getElementType() == TypeCache.Int8Ty);
 return Address(CreateGEP(Addr.getElementType(), Addr.getPointer(),
  getSize(Offset), Name),
+   Addr.getElementType(),
Addr.getAlignment().alignmentAtOffset(Offset));
   }
 
@@ -281,8 +287,9 @@ class CGBuilderTy : public CGBuilderBaseTy {
 /*isSigned=*/true);
 if (!GEP->accumulateConstantOffset(DL, Offset))
   llvm_unreachable("offset of GEP with constants is always computable");
-return Address(GEP, Addr.getAlignment().alignmentAtOffset(
-CharUnits::fromQuantity(Offset.getSExtValue(;
+return Address(GEP, GEP->getResultElementType(),
+   Addr.getAlignment().alignmentAtOffset(
+   CharUnits::fromQuantity(Offset.getSExtValue(;
   }
 
   using CGBuilderBaseTy::CreateMemCpy;
@@ -333,6 +340,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
 
 return Address(CreatePreserveStructAccessIndex(ElTy, Addr.getPointer(),
Index, FieldIndex, DbgInfo),
+   ElTy->getElementType(Index),
Addr.getAlignment().alignmentAtOffset(Offset));
   }
 };

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a50f852f9051..c9ae244d5a96 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -5246,9 +5246,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 llvm::Type *BPP = Int8PtrPtrTy;
 
 DestAddr = Address(Builder.CreateBitCast(DestAddr.getPointer(), BPP, "cp"),
-   DestAddr.getAlignment());
+   Int8PtrTy, DestAddr.getAlignment());
 SrcAddr = Address(Builder.CreateBitCast(SrcAddr.getPointer(), BPP, "ap"),
-  SrcAddr.getAlignment());
+  Int8PtrTy, SrcAddr.getAlignment());
 
 Value *ArgPtr = Builder.CreateLoad(SrcAddr, "ap.val");
 return RValue::get(Builder.CreateStore(ArgPtr, DestAddr));
@@ -15405,7 +15405,8 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned 
BuiltinID,
 // If the user wants the entire vector, just load the entire vector.
 if (NumBytes == 16) {
   Value *BC = Builder.CreateBitCast(Ops[0], ResTy->getPointerTo());
-  Value *LD = Builder.CreateLoad(Addres

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Any reason not to handle __builtin_reduce_xor + __builtin_reduce_or as well in 
the same patch since they are very similar?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

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

[clang-tools-extra] b7d2d14 - [clangd] Disable the NOLINTBBEGIN testcase in clangd.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-12-15T11:32:27+01:00
New Revision: b7d2d147477e4fa5e34a3bb74b2a393cdaab9779

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

LOG: [clangd] Disable the NOLINTBBEGIN testcase in clangd.

NOLINTBEGIN is disabled, in 529833377ccdf4381f8bc9961bfa96ec4f5e2eed

Added: 


Modified: 
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index b3872ab0bc592..920e56a21cf78 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -478,8 +478,9 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) {
   #define BAD2 BAD
   double h = BAD2;  // NOLINT
   // NOLINTBEGIN
-  double x = BAD2;
-  double y = BAD2;
+  // FIXME: re-enable when NOLINTBEGIN suppresss block is enabled in 
clangd.
+  // double x = BAD2;
+  // double y = BAD2;
   // NOLINTEND
 
   // verify no crashes on unmatched nolints.



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

[clang] bd9e239 - [analyzer] Expand conversion check to check more expressions for overflow and underflow

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2021-12-15T11:41:34+01:00
New Revision: bd9e23943a2299804172da69b308533241f99b60

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

LOG: [analyzer] Expand conversion check to check more expressions for overflow 
and underflow

This expands checking for more expressions. This will check underflow
and loss of precision when using call expressions like:

  void foo(unsigned);
  int i = -1;
  foo(i);

This also includes other expressions as well, so it can catch negative
indices to std::vector since it uses unsigned integers for [] and .at()
function.

Patch by: @pfultz2

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

Added: 
clang/test/Analysis/conversion.cpp

Modified: 
clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
clang/test/Analysis/conversion.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
index 8da482a2aec95..6ea39fb95e9a8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -56,9 +56,8 @@ class ConversionChecker : public 
Checker> {
 
 void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast,
  CheckerContext &C) const {
-  // TODO: For now we only warn about DeclRefExpr, to avoid noise. Warn for
-  // calculations also.
-  if (!isa(Cast->IgnoreParenImpCasts()))
+  // Don't warn for implicit conversions to bool
+  if (Cast->getType()->isBooleanType())
 return;
 
   // Don't warn for loss of sign/precision in macros.
@@ -70,6 +69,9 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr 
*Cast,
   const Stmt *Parent = PM.getParent(Cast);
   if (!Parent)
 return;
+  // Dont warn if this is part of an explicit cast
+  if (isa(Parent))
+return;
 
   bool LossOfSign = false;
   bool LossOfPrecision = false;
@@ -78,8 +80,10 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr 
*Cast,
   if (const auto *B = dyn_cast(Parent)) {
 BinaryOperator::Opcode Opc = B->getOpcode();
 if (Opc == BO_Assign) {
-  LossOfSign = isLossOfSign(Cast, C);
-  LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+  if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) {
+LossOfSign = isLossOfSign(Cast, C);
+LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+  }
 } else if (Opc == BO_AddAssign || Opc == BO_SubAssign) {
   // No loss of sign.
   LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
@@ -98,7 +102,12 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr 
*Cast,
 } else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
   LossOfSign = isLossOfSign(Cast, C);
 }
-  } else if (isa(Parent)) {
+  } else if (isa(Parent)) {
+if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) {
+  LossOfSign = isLossOfSign(Cast, C);
+  LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+}
+  } else {
 LossOfSign = isLossOfSign(Cast, C);
 LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
   }

diff  --git a/clang/test/Analysis/conversion.c 
b/clang/test/Analysis/conversion.c
index 84eccb7e2f506..f6b0c11a15aea 100644
--- a/clang/test/Analysis/conversion.c
+++ b/clang/test/Analysis/conversion.c
@@ -105,6 +105,23 @@ void division(unsigned U, signed S) {
 S = U / S; // expected-warning {{Loss of sign}}
 }
 
+void f(unsigned x) {}
+void g(unsigned x) {}
+
+void functioncall1() {
+  long x = -1;
+  int y = 0;
+  f(x); // expected-warning {{Loss of sign in implicit conversion}}
+  f(y);
+}
+
+void functioncall2(int x, int y) {
+  if (x < 0)
+f(x); // expected-warning {{Loss of sign in implicit conversion}}
+  f(y);
+  f(x); // expected-warning {{Loss of sign in implicit conversion}}
+}
+
 void dontwarn1(unsigned U, signed S) {
   U8 = S; // It might be known that S is always 0x00-0xff.
   S8 = U; // It might be known that U is always 0x00-0xff.
@@ -132,15 +149,38 @@ void dontwarn4() {
   DOSTUFF;
 }
 
-// don't warn for calculations
-// seen some fp. For instance:  c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' 
: c2;
-// there is a todo in the checker to handle calculations
 void dontwarn5() {
-  signed S = -32;
-  U8 = S + 10;
+  unsigned char c1 = 'A';
+  c1 = (c1 >= 'A' && c1 <= 'Z') ? c1 - 'A' + 'a' : c1;
+  unsigned char c2 = 0;
+  c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2;
+  unsigned char c3 = 'Z';
+  c3 = (c3 >= 'A' && c3 <= 'Z') ? c3 - 'A' + 'a' : c3;
+  unsigned char c4 = 'a';
+  c4 = (c4 >= 'A' && c4 <= 'Z') ? c4 - 'A' + 'a' : c4;
+  unsigned char c5 = '@';
+  c5 = (c5 >= 'A' && c5 <= 'Z') ? c5 - 'A' + 'a' 

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd9e23943a22: [analyzer] Expand conversion check to check 
more expressions for overflow and… (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D46081?vs=144202&id=394502#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46081

Files:
  clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  clang/test/Analysis/conversion.c
  clang/test/Analysis/conversion.cpp

Index: clang/test/Analysis/conversion.cpp
===
--- /dev/null
+++ clang/test/Analysis/conversion.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -Wno-conversion -Wno-tautological-constant-compare -analyzer-checker=core,alpha.core.Conversion -verify %s
+
+// expected-no-diagnostics
+
+void dontwarn1() {
+  unsigned long x = static_cast(-1);
+}
+
+void dontwarn2(unsigned x) {
+  if (x == static_cast(-1)) {
+  }
+}
+
+struct C {
+  C(unsigned x, unsigned long y) {}
+};
+
+void f(C) {}
+
+void functioncall1(long x) {
+  f(C(64, x));
+}
Index: clang/test/Analysis/conversion.c
===
--- clang/test/Analysis/conversion.c
+++ clang/test/Analysis/conversion.c
@@ -105,6 +105,23 @@
 S = U / S; // expected-warning {{Loss of sign}}
 }
 
+void f(unsigned x) {}
+void g(unsigned x) {}
+
+void functioncall1() {
+  long x = -1;
+  int y = 0;
+  f(x); // expected-warning {{Loss of sign in implicit conversion}}
+  f(y);
+}
+
+void functioncall2(int x, int y) {
+  if (x < 0)
+f(x); // expected-warning {{Loss of sign in implicit conversion}}
+  f(y);
+  f(x); // expected-warning {{Loss of sign in implicit conversion}}
+}
+
 void dontwarn1(unsigned U, signed S) {
   U8 = S; // It might be known that S is always 0x00-0xff.
   S8 = U; // It might be known that U is always 0x00-0xff.
@@ -132,15 +149,38 @@
   DOSTUFF;
 }
 
-// don't warn for calculations
-// seen some fp. For instance:  c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2;
-// there is a todo in the checker to handle calculations
 void dontwarn5() {
-  signed S = -32;
-  U8 = S + 10;
+  unsigned char c1 = 'A';
+  c1 = (c1 >= 'A' && c1 <= 'Z') ? c1 - 'A' + 'a' : c1;
+  unsigned char c2 = 0;
+  c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2;
+  unsigned char c3 = 'Z';
+  c3 = (c3 >= 'A' && c3 <= 'Z') ? c3 - 'A' + 'a' : c3;
+  unsigned char c4 = 'a';
+  c4 = (c4 >= 'A' && c4 <= 'Z') ? c4 - 'A' + 'a' : c4;
+  unsigned char c5 = '@';
+  c5 = (c5 >= 'A' && c5 <= 'Z') ? c5 - 'A' + 'a' : c5;
+}
+
+void dontwarn6() {
+  int x = ~0;
+  unsigned y = ~0;
+}
+
+void dontwarn7(unsigned x) {
+  if (x == (unsigned)-1) {
+  }
+}
+
+void dontwarn8() {
+  unsigned x = (unsigned)-1;
+}
+
+unsigned dontwarn9() {
+  return ~0;
 }
 
-char dontwarn6(long long x) {
+char dontwarn10(long long x) {
   long long y = 42;
   y += x;
   return y == 42;
Index: clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -56,9 +56,8 @@
 
 void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast,
  CheckerContext &C) const {
-  // TODO: For now we only warn about DeclRefExpr, to avoid noise. Warn for
-  // calculations also.
-  if (!isa(Cast->IgnoreParenImpCasts()))
+  // Don't warn for implicit conversions to bool
+  if (Cast->getType()->isBooleanType())
 return;
 
   // Don't warn for loss of sign/precision in macros.
@@ -70,6 +69,9 @@
   const Stmt *Parent = PM.getParent(Cast);
   if (!Parent)
 return;
+  // Dont warn if this is part of an explicit cast
+  if (isa(Parent))
+return;
 
   bool LossOfSign = false;
   bool LossOfPrecision = false;
@@ -78,8 +80,10 @@
   if (const auto *B = dyn_cast(Parent)) {
 BinaryOperator::Opcode Opc = B->getOpcode();
 if (Opc == BO_Assign) {
-  LossOfSign = isLossOfSign(Cast, C);
-  LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+  if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) {
+LossOfSign = isLossOfSign(Cast, C);
+LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+  }
 } else if (Opc == BO_AddAssign || Opc == BO_SubAssign) {
   // No loss of sign.
   LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
@@ -98,7 +102,12 @@
 } else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
   LossOfSign = isLossOfSign(Cast, C);
 }
-  } else if (isa(Parent)) {
+  } else if (isa(Parent)) {
+if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) {
+  LossOfSign = isLossOfSign(Cast, C);
+  LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+}
+  } else {
 LossOfSign =

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

When running my test case `ctu-lookup-name-with-space.cpp` on Windows, 
`llvm-lit` reports `'cp': command not found`. And this is the reason why it 
fails on Windows.
And when I remove the `cp`s and replace them with original file names, clang 
reports `YAML:1:293: error: Unrecognized escape code`, it seems that the static 
analyzer only reads compilation database in YAML format on Windows.
Should I disable this test case on Windows? Or is there any other approaches to 
make it work on Windows?


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

https://reviews.llvm.org/D102669

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

[clang] 481de0e - [CodeGen] Prefer CreateElementBitCast() where possible

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T11:48:39+01:00
New Revision: 481de0ed804c8547fbfac5fe88d109c692c8d117

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

LOG: [CodeGen] Prefer CreateElementBitCast() where possible

CreateElementBitCast() can preserve the pointer element type in
the presence of opaque pointers, so use it in place of CreateBitCast()
in some places. This also sometimes simplifies the code a bit.

Added: 


Modified: 
clang/lib/CodeGen/CGAtomic.cpp
clang/lib/CodeGen/CGBuilder.h
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CGNonTrivialStruct.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index a7a9ea2fbbca..e81c5ba5055c 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -1321,15 +1321,14 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
 ResVal = Builder.CreateNot(ResVal);
 
   Builder.CreateStore(
-  ResVal,
-  Builder.CreateBitCast(Dest, ResVal->getType()->getPointerTo()));
+  ResVal, Builder.CreateElementBitCast(Dest, ResVal->getType()));
 }
 
 if (RValTy->isVoidType())
   return RValue::get(nullptr);
 
 return convertTempToRValue(
-Builder.CreateBitCast(Dest, ConvertTypeForMem(RValTy)->getPointerTo()),
+Builder.CreateElementBitCast(Dest, ConvertTypeForMem(RValTy)),
 RValTy, E->getExprLoc());
   }
 
@@ -1382,8 +1381,7 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   return RValue::get(nullptr);
 
 return convertTempToRValue(
-Builder.CreateBitCast(Dest, ConvertTypeForMem(RValTy)->getPointerTo(
-Dest.getAddressSpace())),
+Builder.CreateElementBitCast(Dest, ConvertTypeForMem(RValTy)),
 RValTy, E->getExprLoc());
   }
 
@@ -1455,17 +1453,14 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
 
   assert(Atomics.getValueSizeInBits() <= Atomics.getAtomicSizeInBits());
   return convertTempToRValue(
-  Builder.CreateBitCast(Dest, ConvertTypeForMem(RValTy)->getPointerTo(
-  Dest.getAddressSpace())),
+  Builder.CreateElementBitCast(Dest, ConvertTypeForMem(RValTy)),
   RValTy, E->getExprLoc());
 }
 
 Address AtomicInfo::emitCastToAtomicIntPointer(Address addr) const {
-  unsigned addrspace =
-cast(addr.getPointer()->getType())->getAddressSpace();
   llvm::IntegerType *ty =
 llvm::IntegerType::get(CGF.getLLVMContext(), AtomicSizeInBits);
-  return CGF.Builder.CreateBitCast(addr, ty->getPointerTo(addrspace));
+  return CGF.Builder.CreateElementBitCast(addr, ty);
 }
 
 Address AtomicInfo::convertToAtomicIntPointer(Address Addr) const {

diff  --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index 86c108c54878..53537b044f95 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -168,8 +168,9 @@ class CGBuilderTy : public CGBuilderBaseTy {
   /// preserving information like the alignment and address space.
   Address CreateElementBitCast(Address Addr, llvm::Type *Ty,
const llvm::Twine &Name = "") {
-auto PtrTy = Ty->getPointerTo(Addr.getAddressSpace());
-return CreateBitCast(Addr, PtrTy, Name);
+auto *PtrTy = Ty->getPointerTo(Addr.getAddressSpace());
+return Address(CreateBitCast(Addr.getPointer(), PtrTy, Name),
+   Ty, Addr.getAlignment());
   }
 
   using CGBuilderBaseTy::CreatePointerBitCastOrAddrSpaceCast;

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index c9ae244d5a96..228e656b98d7 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18711,8 +18711,8 @@ Value *CodeGenFunction::EmitHexagonBuiltinExpr(unsigned 
BuiltinID,
   case Hexagon::BI__builtin_HEXAGON_V6_vsubcarry_128B: {
 // Get the type from the 0-th argument.
 llvm::Type *VecType = ConvertType(E->getArg(0)->getType());
-Address PredAddr = Builder.CreateBitCast(
-EmitPointerWithAlignment(E->getArg(2)), VecType->getPointerTo(0));
+Address PredAddr = Builder.CreateElementBitCast(
+EmitPointerWithAlignment(E->getArg(2)), VecType);
 llvm::Value *PredIn = V2Q(Builder.CreateLoad(PredAddr));
 llvm::Value *Result = Builder.CreateCall(CGM.getIntrinsic(ID),
 {EmitScalarExpr(E->getArg(0)), EmitScalarExpr(E->getArg(1)), PredIn});

diff  --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index cc838bf38c6c..62fa0020ea93 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -1135,7 +1135,7 @@ void CodeGenFunction::EmitNewArrayInitializer(

[PATCH] D115790: [Coroutines] Run CoroEarly Pass in front of AlwaysInliner in O0 pass and warn for always_inline coroutine

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rjmccall, lxfind, aeubanks.
ChuanqiXu added a project: clang.
Herald added subscribers: ormris, steven_wu, hiraditya.
ChuanqiXu requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits.
Herald added a project: LLVM.

See https://reviews.llvm.org/D100282 and 
https://github.com/llvm/llvm-project/issues/49264 for the background.
Simply, coroutine shouldn't be inlined before CoroSplit. And the marker for 
pre-splited coroutine is created in  CoroEarly pass, which ran after 
AlwaysInliner Pass in O0 pipeline. So that the AlwaysInliner couldn't detect it 
shouldn't inline a coroutine. So here is the error.

In D100282 , we have discussed how should we 
fix this bug. Should we move the CoroEarly pass in front of AlwaysInliner? Or 
should we make it in the frontend. According to @lxfind , it would be better to 
move the CoroEarly pass.

And I have another reason to not implement it in the frontend. Since I found 
that if we make it in the frontend, we need to edit the `Coroutines.rst` 
document to tell that the frontend should emit a `"coroutine.presplit"` 
attribute (at least for switch-resumed ABI), it is weird. Since it is required 
all the time, it should be done automatically in the middle end. So I think it 
would be better to move the CoroEarly pass.

BTW, according to the discuss in D100282 , it 
is suggested that we should add a warning for marking a coroutine as 
`always_inline`. I implement it in this patch. But I am willing to split it if 
required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115790

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  clang/test/SemaCXX/coroutines.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-O0-defaults.ll

Index: llvm/test/Other/new-pm-O0-defaults.ll
===
--- llvm/test/Other/new-pm-O0-defaults.ll
+++ llvm/test/Other/new-pm-O0-defaults.ll
@@ -31,14 +31,13 @@
 
 ; CHECK-DIS: Running analysis: InnerAnalysisManagerProxy
 ; CHECK-DIS-NEXT: Running pass: AddDiscriminatorsPass
+; CHECK-CORO: Running pass: CoroEarlyPass
 ; CHECK-DIS-NEXT: Running pass: AlwaysInlinerPass
 ; CHECK-DIS-NEXT: Running analysis: ProfileSummaryAnalysis
-; CHECK-DEFAULT: Running pass: AlwaysInlinerPass
-; CHECK-DEFAULT-NEXT: Running analysis: InnerAnalysisManagerProxy
+; CHECK-DEFAULT-NEXT: Running pass: AlwaysInlinerPass
 ; CHECK-DEFAULT-NEXT: Running analysis: ProfileSummaryAnalysis
 ; CHECK-MATRIX: Running pass: LowerMatrixIntrinsicsPass
 ; CHECK-MATRIX-NEXT: Running analysis: TargetIRAnalysis
-; CHECK-CORO-NEXT: Running pass: CoroEarlyPass
 ; CHECK-CORO-NEXT: Running analysis: InnerAnalysisManagerProxy
 ; CHECK-CORO-NEXT: Running analysis: LazyCallGraphAnalysis
 ; CHECK-CORO-NEXT: Running analysis: TargetLibraryAnalysis
Index: llvm/test/Other/new-pass-manager.ll
===
--- llvm/test/Other/new-pass-manager.ll
+++ llvm/test/Other/new-pass-manager.ll
@@ -292,8 +292,9 @@
 ; RUN: -passes='default' %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefix=CHECK-O0 --check-prefix=%llvmcheckext
 ; CHECK-O0: Running pass: AlwaysInlinerPass
-; CHECK-O0-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}>
 ; CHECK-O0-NEXT: Running analysis: ProfileSummaryAnalysis
+; CHECK-O0-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}>
+; CHECK-O0-NEXT: Running analysis: LazyCallGraphAnalysis
 ; CHECK-EXT-NEXT: Running pass: {{.*}}Bye
 ; We don't have checks for CHECK-NOEXT here, but this simplifies the test, while
 ; avoiding FileCheck complaining about the unused prefix.
Index: llvm/lib/Passes/PassBuilderPipelines.cpp
===
--- llvm/lib/Passes/PassBuilderPipelines.cpp
+++ llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1717,6 +1717,8 @@
   for (auto &C : PipelineEarlySimplificationEPCallbacks)
 C(MPM, Level);
 
+  MPM.addPass(createModuleToFunctionPassAdaptor(CoroEarlyPass()));
+
   // Build a minimal pipeline based on the semantics required by LLVM,
   // which is just that always inlining occurs. Further, disable generating
   // lifetime intrinsics to avoid enabling further optimizations during
@@ -1771,7 +1773,6 @@
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
-  MPM.addPass(createModuleToFunctionPassAdaptor(CoroEarlyPass()));
   CGSCCPassManager CGPM;
   CGPM.addPass(CoroSplitPass());
   MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
Index: clang/test/SemaCXX/coroutines.cpp

[PATCH] D115650: [clangd] Disable support for clang-tidy suppression blocks (NOLINTBEGIN)

[Shraiysh via Phabricator via cfe-commits]

shraiysh added a comment.

Hey folks, this commit seems to cause multiple buildbot failures 1 
 2 
 3 
. Can someone please 
revert this change while it gets fixed? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115650

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

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[Peter Stys via Phabricator via cfe-commits]

peterstys updated this revision to Diff 394503.
peterstys added a comment.

Applied fixes suggested by the reviewers.


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

https://reviews.llvm.org/D115738

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -759,6 +759,128 @@
GoogleStyle);
 }
 
+TEST_F(FormatTestCSharp, CSharpLambdasDontBreakFollowingCodeAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Sample {
+  public void Test() {
+while (true) {
+  preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+  CodeThatFollowsLambda();
+  IsWellAligned();
+}
+  }
+})",
+   GoogleStyle);
+
+  verifyFormat(R"(//
+public class Sample
+{
+public void Test()
+{
+while (true)
+{
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+CodeThatFollowsLambda();
+IsWellAligned();
+}
+}
+})",
+   MicrosoftStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasComplexLambdasDontBreakAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Test
+{
+private static void ComplexLambda(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+   MicrosoftStyle);
+
+  verifyFormat(R"(//
+public class Test {
+  private static void ComplexLambda(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0) {
+  return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+  }
+})",
+   GoogleStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasMulipleLambdasDontBreakAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Test
+{
+private static void MultipleLambdas(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+   MicrosoftStyle);
+
+  verifyFormat(R"(//
+public class Test {
+  private static void MultipleLambdas(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0) {
+  return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+  }
+})",
+   GoogleStyle);
+}
+
 TEST_F(FormatTestCSharp, CSharpObjectInitializers) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2001,10 +2001,17 @@
   }
   break;
 case tok::equal:
-  if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
-parseStructuralElement();
-  else
+  if (Style.isCSharp() && FormatTok->is(TT_FatArrow)) {
+nextToken();
+if (FormatTok->is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction) {
+FormatTok->MustBreakBefore = true;
+  }
+  parseChildBlock();
+}
+  } else {
 nextToke

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[Peter Stys via Phabricator via cfe-commits]

peterstys updated this revision to Diff 394505.
peterstys marked 4 inline comments as done.
peterstys added a comment.

Removed braces.


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

https://reviews.llvm.org/D115738

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -759,6 +759,128 @@
GoogleStyle);
 }
 
+TEST_F(FormatTestCSharp, CSharpLambdasDontBreakFollowingCodeAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Sample {
+  public void Test() {
+while (true) {
+  preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+  CodeThatFollowsLambda();
+  IsWellAligned();
+}
+  }
+})",
+   GoogleStyle);
+
+  verifyFormat(R"(//
+public class Sample
+{
+public void Test()
+{
+while (true)
+{
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+CodeThatFollowsLambda();
+IsWellAligned();
+}
+}
+})",
+   MicrosoftStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasComplexLambdasDontBreakAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Test
+{
+private static void ComplexLambda(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+   MicrosoftStyle);
+
+  verifyFormat(R"(//
+public class Test {
+  private static void ComplexLambda(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0) {
+  return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+  }
+})",
+   GoogleStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasMulipleLambdasDontBreakAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Test
+{
+private static void MultipleLambdas(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+   MicrosoftStyle);
+
+  verifyFormat(R"(//
+public class Test {
+  private static void MultipleLambdas(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0) {
+  return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+  }
+})",
+   GoogleStyle);
+}
+
 TEST_F(FormatTestCSharp, CSharpObjectInitializers) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2001,10 +2001,16 @@
   }
   break;
 case tok::equal:
-  if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
-parseStructuralElement();
-  else
+  if (Style.isCSharp() && FormatTok->is(TT_FatArrow)) {
+nextToken();
+if (FormatTok->is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction)
+FormatTok->MustBreakBefore = true;  
+  parseChildBlock();
+}
+  } else {

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[Peter Stys via Phabricator via cfe-commits]

peterstys updated this revision to Diff 394506.
peterstys marked an inline comment as done.

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

https://reviews.llvm.org/D115738

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -759,6 +759,128 @@
GoogleStyle);
 }
 
+TEST_F(FormatTestCSharp, CSharpLambdasDontBreakFollowingCodeAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Sample {
+  public void Test() {
+while (true) {
+  preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+  CodeThatFollowsLambda();
+  IsWellAligned();
+}
+  }
+})",
+   GoogleStyle);
+
+  verifyFormat(R"(//
+public class Sample
+{
+public void Test()
+{
+while (true)
+{
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+CodeThatFollowsLambda();
+IsWellAligned();
+}
+}
+})",
+   MicrosoftStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasComplexLambdasDontBreakAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Test
+{
+private static void ComplexLambda(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+   MicrosoftStyle);
+
+  verifyFormat(R"(//
+public class Test {
+  private static void ComplexLambda(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+if (allSelectedScenes.Count == 0) {
+  return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+  }
+})",
+   GoogleStyle);
+}
+
+TEST_F(FormatTestCSharp, CSharpLambdasMulipleLambdasDontBreakAlignment) {
+  FormatStyle GoogleStyle = getGoogleStyle(FormatStyle::LK_CSharp);
+  FormatStyle MicrosoftStyle = getMicrosoftStyle(FormatStyle::LK_CSharp);
+
+  verifyFormat(R"(//
+public class Test
+{
+private static void MultipleLambdas(BuildReport protoReport)
+{
+allSelectedScenes =
+veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0)
+{
+return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+}
+})",
+   MicrosoftStyle);
+
+  verifyFormat(R"(//
+public class Test {
+  private static void MultipleLambdas(BuildReport protoReport) {
+allSelectedScenes = veryVeryLongCollectionNameThatPutsTheLineLenghtAboveTheThresholds
+.Where(scene => scene.enabled)
+.Select(scene => scene.path)
+.ToArray();
+preBindEnumerators.RemoveAll(enumerator => !enumerator.MoveNext());
+if (allSelectedScenes.Count == 0) {
+  return;
+}
+Functions();
+AreWell();
+Aligned();
+AfterLambdaBlock();
+  }
+})",
+   GoogleStyle);
+}
+
 TEST_F(FormatTestCSharp, CSharpObjectInitializers) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2001,10 +2001,16 @@
   }
   break;
 case tok::equal:
-  if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
-parseStructuralElement();
-  else
+  if (Style.isCSharp() && FormatTok->is(TT_FatArrow)) {
+nextToken();
+if (FormatTok->is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction)
+FormatTok->MustBreakBefore = true;  
+  parseChildBlock();
+}
+  } else {
 nextToken();
+  }
   break;

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

In D115231#3194372 , @RKSimon wrote:

> Any reason not to handle __builtin_reduce_xor + __builtin_reduce_or as well 
> in the same patch since they are very similar?

Hi, thanks for take a look at this.
Yes, you're right. The reason I split them is that I'm a beginner, so I want to 
send a "experimental patch" first, this will make review work a little bit 
easier if I mess something up. I think it's reasonable to add all similar 
builtins if currently nothing wrong about this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

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

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[Peter Stys via Phabricator via cfe-commits]

peterstys added a comment.

In D115738#3193107 , @MyDeveloperDay 
wrote:

> I tested this on the original code that made me make the original change, and 
> I like your fix much better ;-)
>
> Thank you for this patch, interested on working on other C# clang-format 
> issues?

Happy to help. My team is using clang-format to format our C# codebase and we 
have few more issues that we'd like to help fixing.


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

https://reviews.llvm.org/D115738

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

[PATCH] D115791: [CodeGen] Store ElementType in LValue

[Nikita Popov via Phabricator via cfe-commits]

nikic created this revision.
nikic added reviewers: opaque-pointers, rjmccall.
nikic requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Store the pointer element type inside LValue so that we can preserve it when 
converting it back into an Address. Storing the pointer element type might not 
be strictly required here in that we could probably re-derive it from the 
QualType (which would require CGF access though), but storing it seems like the 
simpler solution.

The global register case is special and does not store an element type, as the 
value is not a pointer type in that case and it's not possible to create an 
Address from it.

This is the main remaining part from D103465 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115791

Files:
  clang/lib/CodeGen/CGValue.h

Index: clang/lib/CodeGen/CGValue.h
===
--- clang/lib/CodeGen/CGValue.h
+++ clang/lib/CodeGen/CGValue.h
@@ -175,6 +175,7 @@
   } LVType;
 
   llvm::Value *V;
+  llvm::Type *ElementType;
 
   union {
 // Index into a vector subscript: V[i]
@@ -230,6 +231,13 @@
   LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
 assert((!Alignment.isZero() || Type->isIncompleteType()) &&
"initializing l-value with zero alignment!");
+if (isGlobalReg())
+  assert(ElementType == nullptr && "Global reg does not store elem type");
+else
+  assert(llvm::cast(V->getType())
+ ->isOpaqueOrPointeeTypeMatches(ElementType) &&
+ "Pointer element type mismatch");
+
 this->Type = Type;
 this->Quals = Quals;
 const unsigned MaxAlign = 1U << 31;
@@ -327,17 +335,18 @@
 return V;
   }
   Address getAddress(CodeGenFunction &CGF) const {
-return Address(getPointer(CGF), getAlignment());
+return Address(getPointer(CGF), ElementType, getAlignment());
   }
   void setAddress(Address address) {
 assert(isSimple());
 V = address.getPointer();
+ElementType = address.getElementType();
 Alignment = address.getAlignment().getQuantity();
   }
 
   // vector elt lvalue
   Address getVectorAddress() const {
-return Address(getVectorPointer(), getAlignment());
+return Address(getVectorPointer(), ElementType, getAlignment());
   }
   llvm::Value *getVectorPointer() const {
 assert(isVectorElt());
@@ -349,7 +358,7 @@
   }
 
   Address getMatrixAddress() const {
-return Address(getMatrixPointer(), getAlignment());
+return Address(getMatrixPointer(), ElementType, getAlignment());
   }
   llvm::Value *getMatrixPointer() const {
 assert(isMatrixElt());
@@ -362,7 +371,7 @@
 
   // extended vector elements.
   Address getExtVectorAddress() const {
-return Address(getExtVectorPointer(), getAlignment());
+return Address(getExtVectorPointer(), ElementType, getAlignment());
   }
   llvm::Value *getExtVectorPointer() const {
 assert(isExtVectorElt());
@@ -375,7 +384,7 @@
 
   // bitfield lvalue
   Address getBitFieldAddress() const {
-return Address(getBitFieldPointer(), getAlignment());
+return Address(getBitFieldPointer(), ElementType, getAlignment());
   }
   llvm::Value *getBitFieldPointer() const { assert(isBitField()); return V; }
   const CGBitFieldInfo &getBitFieldInfo() const {
@@ -395,6 +404,7 @@
 R.LVType = Simple;
 assert(address.getPointer()->getType()->isPointerTy());
 R.V = address.getPointer();
+R.ElementType = address.getElementType();
 R.Initialize(type, qs, address.getAlignment(), BaseInfo, TBAAInfo);
 return R;
   }
@@ -405,6 +415,7 @@
 LValue R;
 R.LVType = VectorElt;
 R.V = vecAddress.getPointer();
+R.ElementType = vecAddress.getElementType();
 R.VectorIdx = Idx;
 R.Initialize(type, type.getQualifiers(), vecAddress.getAlignment(),
  BaseInfo, TBAAInfo);
@@ -417,6 +428,7 @@
 LValue R;
 R.LVType = ExtVectorElt;
 R.V = vecAddress.getPointer();
+R.ElementType = vecAddress.getElementType();
 R.VectorElts = Elts;
 R.Initialize(type, type.getQualifiers(), vecAddress.getAlignment(),
  BaseInfo, TBAAInfo);
@@ -435,6 +447,7 @@
 LValue R;
 R.LVType = BitField;
 R.V = Addr.getPointer();
+R.ElementType = Addr.getElementType();
 R.BitFieldInfo = &Info;
 R.Initialize(type, type.getQualifiers(), Addr.getAlignment(), BaseInfo,
  TBAAInfo);
@@ -446,6 +459,7 @@
 LValue R;
 R.LVType = GlobalReg;
 R.V = V;
+R.ElementType = nullptr;
 R.Initialize(type, type.getQualifiers(), alignment,
  LValueBaseInfo(AlignmentSource::Decl), TBAAAccessInfo());
 return R;
@@ -457,6 +471,7 @@
 LValue R;
 R.LVType = MatrixElt;
 R.V = matAddress.getPointer();
+R.ElementType = matAddress.getElementType();
 R.VectorIdx = Idx;
 R.Initialize(type, type.getQualifiers(), matAddress.getAlig

[PATCH] D115792: [Modules] Incorrect ODR detection for unresolved using type

[Chuanqi Xu via Phabricator via cfe-commits]

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

See https://github.com/llvm/llvm-project/issues/52713.
This patch is a attempt to fix bug52713.

The key reason for the bug is that `isSameEntity` couldn't recognize `void 
bar(Ty)` in imported module and current module as the same entity. Since the 
compiler thinks the `Ty` type in the argument in imported module and the 
current are different type.

I found the reason is that the `isSameEntity` would judge if two types are the 
same by comparing the addressed of their canonical type. But the `Ty` type in 
this example is `UnresolvedUsingType` and the canonical type of 
`UnresolvedUsingType ` is always null. So that the compare result would be 
different always.

My solution in this patch is to not create a new `UnresolvedUsingType` when 
unserialization if there is already a canonical type in canonical declaration. 
I am not sure if this is proper but it runs well currently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115792

Files:
  clang/include/clang/AST/TypeProperties.td
  clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
  clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
  clang/test/Modules/odr_using_dependent_name.cppm


Index: clang/test/Modules/odr_using_dependent_name.cppm
===
--- /dev/null
+++ clang/test/Modules/odr_using_dependent_name.cppm
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/odr_using_dependent_name/X.cppm 
--precompile -o %t/X.pcm
+// RUN: %clang -std=c++20 -I%S/Inputs/odr_using_dependent_name 
-fprebuilt-module-path=%t %s --precompile -c
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Y;
+import X;
Index: clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
@@ -0,0 +1,9 @@
+template 
+struct bar {
+  using Ty = int;
+};
+template 
+struct foo : public bar {
+  using typename bar::Ty;
+  void baz(Ty);
+};
Index: clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module X;
Index: clang/include/clang/AST/TypeProperties.td
===
--- clang/include/clang/AST/TypeProperties.td
+++ clang/include/clang/AST/TypeProperties.td
@@ -358,6 +358,9 @@
   }
 
   def : Creator<[{
+auto *Previous = 
cast(declaration)->getCanonicalDecl();
+if (Previous && Previous->getTypeForDecl())
+  return Previous->getTypeForDecl()->getCanonicalTypeInternal();
 return ctx.getTypeDeclType(cast(declaration));
   }]>;
 }


Index: clang/test/Modules/odr_using_dependent_name.cppm
===
--- /dev/null
+++ clang/test/Modules/odr_using_dependent_name.cppm
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/odr_using_dependent_name/X.cppm --precompile -o %t/X.pcm
+// RUN: %clang -std=c++20 -I%S/Inputs/odr_using_dependent_name -fprebuilt-module-path=%t %s --precompile -c
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Y;
+import X;
Index: clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
@@ -0,0 +1,9 @@
+template 
+struct bar {
+  using Ty = int;
+};
+template 
+struct foo : public bar {
+  using typename bar::Ty;
+  void baz(Ty);
+};
Index: clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module X;
Index: clang/include/clang/AST/TypeProperties.td
===
--- clang/include/clang/AST/TypeProperties.td
+++ clang/include/clang/AST/TypeProperties.td
@@ -358,6 +358,9 @@
   }
 
   def : Creator<[{
+auto *Previous = cast(declaration)->getCanonicalDecl();
+if (Previous && Previous->getTypeForDecl())
+  return Previous->getTypeForDecl()->getCanonicalTypeInternal();
 return ctx.getTypeDeclType(cast(declaration));
   }]>;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

[Momchil Velikov via Phabricator via cfe-commits]

chill added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:391-392
 
+  if (!Arch.empty() && !isBranchProtectionSupportedArch(Arch))
+return false;
+

amilendra wrote:
> chill wrote:
> > On empty `Arch` it'd continue down the function, but we'd like to return 
> > failure.
> I am having trouble getting the test `arm-branch-protection-attr-1.c` to work 
> after these changes. `validateBranchProtection()` checks the combination of 
> two parameters, the branch protection attribute and architecture.
> If the architecture is empty, like below, shouldn't the function to continue 
> checking further than simply returning false? 
> ```
> __attribute__((target("branch-protection=bti"))) void btionly() {}
> ```
> Or should I be using something else other than 
> `CGM.getTarget().getTargetOpts().CPU` to get the architecture in 
> `ARMTargetCodeGenInfo::setTargetAttributes`?
> 
We shouldn't be getting an empty `Arch`, or rather we should definitely know 
what we are generating code for.
If that cannot be reliably obtained via wherever the `Arch` parameter comes 
from, maybe we could instead check
target features (`TargetOptions::Features`).  It's conceptually //more 
correct// too, even though in this particular instance
it probably does not matter much.

As a general note too, I think it'd be better to check for when PACBTI-M 
instructions (NOP or not) are definitely *not* available
as architecture names where they are is likely to change with time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115501

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

[clang] 90bbf79 - [CodeGen] Avoid some deprecated Address constructors

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T12:45:23+01:00
New Revision: 90bbf79c7b9330bba91f4f2abd4e0a43cb93e42b

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

LOG: [CodeGen] Avoid some deprecated Address constructors

Some of these are on the critical path towards making something
minimal work with opaque pointers.

Added: 


Modified: 
clang/lib/CodeGen/CGExpr.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 1e36a3defd5b4..3e60838c56ea9 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -71,7 +71,7 @@ Address 
CodeGenFunction::CreateTempAllocaWithoutCast(llvm::Type *Ty,
  llvm::Value *ArraySize) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
   Alloca->setAlignment(Align.getAsAlign());
-  return Address(Alloca, Align);
+  return Address(Alloca, Ty, Align);
 }
 
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
@@ -101,7 +101,7 @@ Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, 
CharUnits Align,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
   }
 
-  return Address(V, Align);
+  return Address(V, Ty, Align);
 }
 
 /// CreateTempAlloca - This creates an alloca and inserts it into the entry
@@ -144,7 +144,7 @@ Address CodeGenFunction::CreateMemTemp(QualType Ty, 
CharUnits Align,
 /*ArraySize=*/nullptr, Alloca);
 
   if (Ty->isConstantMatrixType()) {
-auto *ArrayTy = cast(Result.getType()->getElementType());
+auto *ArrayTy = cast(Result.getElementType());
 auto *VectorTy = llvm::FixedVectorType::get(ArrayTy->getElementType(),
 ArrayTy->getNumElements());
 
@@ -1160,7 +1160,8 @@ Address CodeGenFunction::EmitPointerWithAlignment(const 
Expr *E,
   // Otherwise, use the alignment of the type.
   CharUnits Align =
   CGM.getNaturalPointeeTypeAlignment(E->getType(), BaseInfo, TBAAInfo);
-  return Address(EmitScalarExpr(E), Align);
+  llvm::Type *ElemTy = ConvertTypeForMem(E->getType()->getPointeeType());
+  return Address(EmitScalarExpr(E), ElemTy, Align);
 }
 
 llvm::Value *CodeGenFunction::EmitNonNullRValueCheck(RValue RV, QualType T) {



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

[PATCH] D115709: [RISCV] Remove Vamo Extention

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added a comment.

I think it'd be helpful for the description to note why this is being removed, 
what happened to the extension, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115709

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

[PATCH] D115794: [clang-format] put non-empty catch block on one line with AllowShortBlocksOnASingleLine: Empty

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: Sam0523, HazardyKnusperkeks, owenpan, curdeius.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://github.com/llvm/llvm-project/issues/52715

Fixes #52715

`AllowShortBlocksOnASingleLine` seems to never be checked for "Empty" as such 
if its used it will be considered "Always" as we only ever check 
`AllowShortBlocksOnASingleLine != Never`

This impacts C++ as well as C# hence the slightly duplicated test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115794

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1369,5 +1369,22 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, EmptyShortBlock) {
+  auto Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception e) {\n"
+   "  e.printStackTrace();\n"
+   "}\n",
+   Style);
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception e) {}\n",
+   Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22783,6 +22783,23 @@
   verifyFormat("co_return co_yield foo();");
 }
 
+TEST_F(FormatTest, EmptyShortBlock) {
+  auto Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception &e) {\n"
+   "  e.printStackTrace();\n"
+   "}\n",
+   Style);
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception &e) {}\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -584,6 +584,9 @@
 Keywords.kw___except)) {
   if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
 return 0;
+  if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Empty &&
+  !I[1]->First->is(tok::r_brace))
+return 0;
   // Don't merge when we can't except the case when
   // the control statement block is empty
   if (!Style.AllowShortIfStatementsOnASingleLine &&


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1369,5 +1369,22 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, EmptyShortBlock) {
+  auto Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception e) {\n"
+   "  e.printStackTrace();\n"
+   "}\n",
+   Style);
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception e) {}\n",
+   Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22783,6 +22783,23 @@
   verifyFormat("co_return co_yield foo();");
 }
 
+TEST_F(FormatTest, EmptyShortBlock) {
+  auto Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception &e) {\n"
+   "  e.printStackTrace();\n"
+   "}\n",
+   Style);
+
+  verifyFormat("try {\n"
+   "  doA();\n"
+   "} catch (Exception &e) {}\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -584,6 +584,9 @@
 Keywords.kw___except)) {
   if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
 return 0;
+  if (Style.AllowShor

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D102669#3194405 , @OikawaKirie 
wrote:

> Should I disable this test case on Windows? Or is there any other approaches 
> to make it work on Windows?

I'm fine with disabling this test on Windows.


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

https://reviews.llvm.org/D102669

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

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1870
   // C# may break after => if the next character is a newline.
   if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) {
 // calling `addUnwrappedLine()` here causes odd parsing errors.

Let's fix this too while we are at it. In the future, we may want to factor 
lines 1862-1874 and 2004-2009.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2008
+  if (Style.BraceWrapping.AfterFunction)
+FormatTok->MustBreakBefore = true;  
+  parseChildBlock();

Remove trailing whitespaces.


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

https://reviews.llvm.org/D115738

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

[PATCH] D113622: [analyzer] support ignoring use-after-free checking with reference_counted attribute

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D113622#3193319 , @chrisdangelo 
wrote:

> I've successfully exercised these changes against a large C / C++ project and 
> studied the output with @NoQ.

Could you please share the results to have look? How can I reproduce and 
evaluate the effect of this change?


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

https://reviews.llvm.org/D113622

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

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1872
 // calling `addUnwrappedLine()` here causes odd parsing errors.
 FormatTok->MustBreakBefore = true;
   }

part of me thinks the MustBreakBefore should be handled separately in 
TokenAnnotator::mustBreakBefore() and let it do its stuff. 


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

https://reviews.llvm.org/D115738

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

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:766
+
+  verifyFormat(R"(//
+public class Sample {

Nit: (only my preference) but I don't like the use of RawStrings in these tests 
(I know others have let them creep in) but I find them more unreadable because 
we lose the indentation.. I think I'm just so used to the other style that this 
just crates a little.

Just my personal preference (you can ignore)


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

https://reviews.llvm.org/D115738

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

[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this!




Comment at: clang/www/cxx_status.html:1202
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  Partial (12)
 

Rather than following this approach, it'd be nice to have the information 
provided directly inline, as done in: 
https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_status.html#L916



Comment at: clang/www/cxx_status.html:1267-1270
+(12): The optimizer couldn't handle TLS with
+  `__attribute__((const))` attribute correctly. It implies that there would
+  be problems if the coroutine is possible to resume on a different thread.
+

Also, when moving it to be a detail of the current row, I'd drop the `(12): ` 
from the text.

I think we should also add some words to the effect of "This feature was 
implemented based on the Coroutines TS and requires further analysis to 
determine what support remains to be added." This should hopefully make it 
clear that the list of issues found is not exhaustive and that investigative 
work is still needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115778

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

[PATCH] D113245: [Sema] Mark explicit specialization declaration in a friend invalid

[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! The failing precommit CI tests look to be unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113245

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

[PATCH] D110742: [OpenCL] Add pure and const attributes to vload builtins

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

> Does the langref need to be amended, first, or is it okay to interpret the 
> `readnone` attribute as it was clearly intended, without going through the 
> process of updating the langref first?
>
> I can update this review to use `__attribute__((pure))` for all address 
> spaces, for the time being, but it seems a shame that the poor wording in the 
> langref might (necessarily) prevent us from making the optimal change.

Apologies for the late reply...  I'd prefer to get the langref updated first, 
for the sake of consistency and to ensure other stakeholders agree with the 
interpretation.  You can still go ahead with the `__attribute__((pure))` 
changes of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110742

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

[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Builtins.def:651
+BUILTIN(__builtin_elementwise_floor, "v.", "nct")
+BUILTIN(__builtin_elementwise_roundeven, "v.", "nct")
+BUILTIN(__builtin_elementwise_trunc, "v.", "nct")

The RFC does not mention this builtin. It specifies 
`__builtin_elementwise_rint` and `__builtin_elementwise_round`. Is there a 
reason why this uses `roundeven` instead and should there be other rounding 
options provided at the same time?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3137-3160
   case Builtin::BI__builtin_elementwise_ceil: {
 Value *Op0 = EmitScalarExpr(E->getArg(0));
 Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::ceil, Op0,
  nullptr, "elt.ceil");
 return RValue::get(Result);
   }
+  case Builtin::BI__builtin_elementwise_floor: {

It's starting to seem like we should have a helper function that takes the 
intrinsic to create and the name to give the operation; WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.
Herald added a subscriber: carlosgalvezp.

@aaron.ballman ping for review :)

I do not intend to fix the raised issues with this revision. I think it is more 
valuable to get this check in and get more user-feedback. But the found bugs 
are on the todo-list!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 394527.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D115778

Files:
  clang/www/cxx_status.html


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. Also there are works reamined to
+  check through the sections on coroutines in the C++ standard.
+
 
 
 


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. Also there are works reamined to
+  check through the sections on coroutines in the C++ standard.
+
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115622: [Debugify] Optimize debugify original mode

[Stephen Tozer via Phabricator via cfe-commits]

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

Seems like a simple and sensible change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115622

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

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:2115
+
+const auto *Arg = TheCall->getArg(0);
+const VectorType *TyA = Arg->getType()->getAs();

Please spell out the type here (same below).



Comment at: clang/lib/Sema/SemaChecking.cpp:2116
+const auto *Arg = TheCall->getArg(0);
+const VectorType *TyA = Arg->getType()->getAs();
+if (!TyA) {

No need to repeat the type name here (same below).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

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

[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/www/cxx_status.html:1267-1270
+(12): The optimizer couldn't handle TLS with
+  `__attribute__((const))` attribute correctly. It implies that there would
+  be problems if the coroutine is possible to resume on a different thread.
+

aaron.ballman wrote:
> Also, when moving it to be a detail of the current row, I'd drop the `(12): ` 
> from the text.
> 
> I think we should also add some words to the effect of "This feature was 
> implemented based on the Coroutines TS and requires further analysis to 
> determine what support remains to be added." This should hopefully make it 
> clear that the list of issues found is not exhaustive and that investigative 
> work is still needed.
The current implementation have some minor differences with coroutines TS 
already. So I chose the wording Richard gives in the mailing list. I think it 
is proper.


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

https://reviews.llvm.org/D115778

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

[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial

[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 minor wording fix. Thank you for this!




Comment at: clang/www/cxx_status.html:1206-1207
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. Also there are works reamined to
+  check through the sections on coroutines in the C++ standard.
+

Fixes typo and grammar but hopefully gets the same point across.


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

https://reviews.llvm.org/D115778

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did you 
ever get the chance to talk with @cjdb about the overlap mentioned in 
https://reviews.llvm.org/D54943#2950100? (The other review also seems to have 
stalled out, so I'm not certain where things are at.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D54943#3194643 , @aaron.ballman 
wrote:

> Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did you 
> ever get the chance to talk with @cjdb about the overlap mentioned in 
> https://reviews.llvm.org/D54943#2950100? (The other review also seems to have 
> stalled out, so I'm not certain where things are at.)

We talked only short in the review, but I am not sure if there is such a big 
overlap between the checks. The other check looks at arguments, which is 
something this check currently skips completely.
I think, this check should continue to avoid this analysis, too.

The underlying `ExprMutAnalyzer` is reusable (I think so, if not that should be 
fixed). But my current understanding is, that the argument checker already 
knows the values are `const`, because it matches on `const &` parameters.
IMHO keeping the checks independent would be better. Otherwise a future 
combined check can be aliased with specific configurations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 394529.
ChuanqiXu added a comment.

Address comments. Thanks for reviewing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115778

Files:
  clang/www/cxx_status.html


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. This feature requires 
further
+  analysis of the C++ Standard to determine what work is necessary for 
conformance.
+
 
 
 


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. This feature requires further
+  analysis of the C++ Standard to determine what work is necessary for conformance.
+
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5653d12 - [docs] Give the reason why the support for coroutine is partial

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-15T21:01:46+08:00
New Revision: 5653d127d7c454be2df1024b26f06612b79f4435

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

LOG: [docs] Give the reason why the support for coroutine is partial

This helps user to know what level of support there
is (roughly) for coroutine feature.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index 6894ca1b1ea64..10ba777be648c 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@ C++20 implementation status
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a 
diff erent thread. This feature requires further
+  analysis of the C++ Standard to determine what work is necessary for 
conformance.
+
 
 
 



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

[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial

[Chuanqi Xu 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 rG5653d127d7c4: [docs] Give the reason why the support for 
coroutine is partial (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115778

Files:
  clang/www/cxx_status.html


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. This feature requires 
further
+  analysis of the C++ Standard to determine what work is necessary for 
conformance.
+
 
 
 


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1199,7 +1199,13 @@
 
   Coroutines
   https://wg21.link/p0912r5";>P0912R5
-  Partial
+  
+Partial
+  The optimizer does not yet handle TLS with
+  `__attribute__((const))` attribute correctly. There can be issues where the
+  coroutine may resume on a different thread. This feature requires further
+  analysis of the C++ Standard to determine what work is necessary for conformance.
+
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d930c31 - [CodeGen] Pass element type to EmitCheckedInBoundsGEP()

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T14:03:33+01:00
New Revision: d930c3155c1ba5114ad12fc15635cb0a2480ac9f

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

LOG: [CodeGen] Pass element type to EmitCheckedInBoundsGEP()

Same as for other GEP creation methods.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 228e656b98d7..740f464a1dd7 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18023,7 +18023,7 @@ RValue CodeGenFunction::EmitBuiltinAlignTo(const 
CallExpr *E, bool AlignUp) {
 if (getLangOpts().isSignedOverflowDefined())
   Result = Builder.CreateGEP(Int8Ty, Base, Difference, "aligned_result");
 else
-  Result = EmitCheckedInBoundsGEP(Base, Difference,
+  Result = EmitCheckedInBoundsGEP(Int8Ty, Base, Difference,
   /*SignedIndices=*/true,
   /*isSubtraction=*/!AlignUp,
   E->getExprLoc(), "aligned_result");

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 3e60838c56ea..b9317856715d 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3585,7 +3585,7 @@ static llvm::Value *emitArraySubscriptGEP(CodeGenFunction 
&CGF,
   SourceLocation loc,
 const llvm::Twine &name = "arrayidx") {
   if (inbounds) {
-return CGF.EmitCheckedInBoundsGEP(ptr, indices, signedIndices,
+return CGF.EmitCheckedInBoundsGEP(elemType, ptr, indices, signedIndices,
   CodeGenFunction::NotSubtraction, loc,
   name);
   } else {

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index ae9434f96529..f1ee56531378 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2631,12 +2631,12 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const 
UnaryOperator *E, LValue LV,
   = CGF.getContext().getAsVariableArrayType(type)) {
   llvm::Value *numElts = CGF.getVLASize(vla).NumElts;
   if (!isInc) numElts = Builder.CreateNSWNeg(numElts, "vla.negsize");
+  llvm::Type *elemTy = value->getType()->getPointerElementType();
   if (CGF.getLangOpts().isSignedOverflowDefined())
-value = Builder.CreateGEP(value->getType()->getPointerElementType(),
-  value, numElts, "vla.inc");
+value = Builder.CreateGEP(elemTy, value, numElts, "vla.inc");
   else
 value = CGF.EmitCheckedInBoundsGEP(
-value, numElts, /*SignedIndices=*/false, isSubtraction,
+elemTy, value, numElts, /*SignedIndices=*/false, isSubtraction,
 E->getExprLoc(), "vla.inc");
 
 // Arithmetic on function pointers (!) is just +-1.
@@ -2647,7 +2647,8 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const 
UnaryOperator *E, LValue LV,
   if (CGF.getLangOpts().isSignedOverflowDefined())
 value = Builder.CreateGEP(CGF.Int8Ty, value, amt, "incdec.funcptr");
   else
-value = CGF.EmitCheckedInBoundsGEP(value, amt, /*SignedIndices=*/false,
+value = CGF.EmitCheckedInBoundsGEP(CGF.Int8Ty, value, amt,
+   /*SignedIndices=*/false,
isSubtraction, E->getExprLoc(),
"incdec.funcptr");
   value = Builder.CreateBitCast(value, input->getType());
@@ -2655,13 +2656,13 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const 
UnaryOperator *E, LValue LV,
 // For everything else, we can just do a simple increment.
 } else {
   llvm::Value *amt = Builder.getInt32(amount);
+  llvm::Type *elemTy = value->getType()->getPointerElementType();
   if (CGF.getLangOpts().isSignedOverflowDefined())
-value = Builder.CreateGEP(value->getType()->getPointerElementType(),
-  value, amt, "incdec.ptr");
+value = Builder.CreateGEP(elemTy, value, amt, "incdec.ptr");
   else
-value = CGF.EmitCheckedInBoundsGEP(value, amt, /*SignedIndices=*/false,
-   isSubtraction, E->getExprLoc(),
-   "incdec.ptr");
+value = CGF.EmitCheckedInBoundsGEP(
+elemTy, value, amt, /*SignedIndices=*/false, isSubtraction,
+E->getExprLoc(), "incdec.ptr");
 }
 
   // Vector incr

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: nikic, dblaikie, rnk.
Herald added subscribers: dexonsmith, jdoerfert, mgrang, hiraditya.
serge-sans-paille requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This class can only add / remove Attributes, but it does it relatively
efficiently compared to existing AttrBuilder, providing a consistent 
compile-time
speedup according to https://llvm-compile-time-tracker.com/

Internally it maintains two SmallVector of sorted Attributes, which turns out to
be more efficient than temporary hashmap and bitfields as used by AttrBuilder.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115798

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenABITypes.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/lib/IR/AttributeImpl.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -602,7 +602,8 @@
 Align MemSetAlign =
 CI->getAttributes().getParamAttrs(0).getAlignment().valueOrOne();
 CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, MemSetAlign);
-AttrBuilder ArgAttrs(CI->getAttributes().getParamAttrs(0));
+SmallAttrBuilder ArgAttrs(CI->getContext(),
+  CI->getAttributes().getParamAttrs(0));
 NewCI->setAttributes(NewCI->getAttributes().addParamAttributes(
 CI->getContext(), 0, ArgAttrs));
 copyFlags(*CI, NewCI);
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1183,12 +1183,13 @@
   Begin->getIterator(), End->getIterator(), InlinerAttributeWindow + 1);
 }
 
-static AttrBuilder IdentifyValidAttributes(CallBase &CB) {
+static SmallAttrBuilder IdentifyValidAttributes(CallBase &CB) {
 
   AttrBuilder AB(CB.getAttributes(), AttributeList::ReturnIndex);
-  if (AB.empty())
-return AB;
-  AttrBuilder Valid;
+  SmallAttrBuilder Valid(CB.getContext());
+  if (AB.empty()) {
+return Valid;
+  }
   // Only allow these white listed attributes to be propagated back to the
   // callee. This is because other attributes may only be valid on the call
   // itself, i.e. attributes such as signext and zeroext.
@@ -1207,7 +1208,7 @@
   if (!UpdateReturnAttributes)
 return;
 
-  AttrBuilder Valid = IdentifyValidAttributes(CB);
+  SmallAttrBuilder Valid = IdentifyValidAttributes(CB);
   if (Valid.empty())
 return;
   auto *CalledFunction = CB.getCalledFunction();
@@ -1252,6 +1253,7 @@
 // existing attribute value (i.e. attributes such as dereferenceable,
 // dereferenceable_or_null etc). See AttrBuilder::merge for more details.
 AttributeList AL = NewRetVal->getAttributes();
+
 AttributeList NewAL = AL.addRetAttributes(Context, Valid);
 NewRetVal->setAttributes(NewAL);
   }
Index: llvm/lib/IR/Function.cpp
===
--- llvm/lib/IR/Function.cpp
+++ llvm/lib/IR/Function.cpp
@@ -549,6 +549,10 @@
   AttributeSets = AttributeSets.addFnAttributes(getContext(), Attrs);
 }
 
+void Function::addFnAttrs(const SmallAttrBuilder &Attrs) {
+  AttributeSets = AttributeSets.addFnAttributes(getContext(), Attrs);
+}
+
 void Function::addRetAttr(Attribute::AttrKind Kind) {
   AttributeSets = AttributeSets.addRetAttribute(getContext(), Kind);
 }
@@ -593,6 +597,10 @@
   AttributeSets = AttributeSets.removeFnAttributes(getContext(), Attrs);
 }
 
+void Function::removeFnAttrs(const SmallAttrBuilder &Attrs) {
+  AttributeSets = AttributeSets.removeFnAttributes(getContext(), Attrs);
+}
+
 void Function::removeRetAttr(Attribute::AttrKind Kind) {
   AttributeSets = AttributeSets.removeRetAttribute(getContext(), Kind);
 }
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -600,6 +600,10 @@
   return AttributeSet(AttributeSetNode::get(C, B));
 }
 
+AttributeSet AttributeSet::get(LLVMContext &C, const SmallAttrBuilder &B) {
+  return AttributeSet(AttributeSetNode::get(C, B));
+}
+
 AttributeSet AttributeSet::get(LLVMContext &C, ArrayRef Attrs) {
   return AttributeSet(AttributeSetNode::get(C, Attrs));
 }
@@ -607,14 +611,14 @@
 AttributeSet AttributeSet::addAttribute(LLVMContext &C,
 Attribute::AttrKind Kind) const {
   if (has

[PATCH] D114394: Compile-time computation of string attribute hashes

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Update: the strategy used by ``AttrBuilder`` might not be the best way to build 
new attributes. I've proposed an alternative implementation in 
https://reviews.llvm.org/D115798. This is somehow orthogonal to that patch, but 
it removes the need for temporary copies of string attributes Key/Value in the 
AttrBuilder which is already a big plus.


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

https://reviews.llvm.org/D114394

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

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

My plan mid-term plan would be to rename `AttrBuilder` into `AttrQuery` at some 
point, and use `SmallAttrBuilder` as the actual `AttrBuilder` in most places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115798

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D54943#3194647 , @JonasToth wrote:

> In D54943#3194643 , @aaron.ballman 
> wrote:
>
>> Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did 
>> you ever get the chance to talk with @cjdb about the overlap mentioned in 
>> https://reviews.llvm.org/D54943#2950100? (The other review also seems to 
>> have stalled out, so I'm not certain where things are at.)
>
> We talked only short in the review, but I am not sure if there is such a big 
> overlap between the checks. The other check looks at arguments, which is 
> something this check currently skips completely.
> I think, this check should continue to avoid this analysis, too.

That's what sort of worries me though -- a function parameter is notionally a 
local variable, so the fact that one check tries to solve one half of the 
problem and the other check tries to solve the other half feels unclean to me. 
I'm fine with this check ignoring parameters for now, but I think there should 
only be one check for "make this const if it can be made so".

> The underlying `ExprMutAnalyzer` is reusable (I think so, if not that should 
> be fixed). But my current understanding is, that the argument checker already 
> knows the values are `const`, because it matches on `const &` parameters.
> IMHO keeping the checks independent would be better. Otherwise a future 
> combined check can be aliased with specific configurations.

I think surfacing the checks as separate checks makes a lot of sense (once is 
about C++ Core Guideline conformance and the other is about performance), but 
I'm not sure I understand why the implementations should be different yet.

That said, I think this particular check is more mature and closer to landing 
than the other one. Based on the amount of effort in this patch already, unless 
there are strong objections, I think we should base the "make this const if it 
can be made so" checks on this one.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:49-51
+  // The rules for C and 'const' are different and incompatible for this check.
+  if (!getLangOpts().CPlusPlus)
+return;

The new-fangled way to do this is to implement an override for 
`isLanguageVersionSupported()` (as in 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h#L26)



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:111-116
+  /// If the variable was declared in a template it might be analyzed multiple
+  /// times. Only one of those instantiations shall emit a warning. NOTE: This
+  /// shall only deduplicate warnings for variables that are not instantiation
+  /// dependent. Variables like 'int x = 42;' in a template that can become
+  /// const emit multiple warnings otherwise.
+  const bool IsNormalVariableInTemplate = Function->isTemplateInstantiation();





Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:168
+*Variable, *Result.Context, DeclSpec::TQ_const,
+QualifierTarget::Value, QualifierPolicy::Right)) {
+  Diag << *Fix;

JonasToth wrote:
> tiagoma wrote:
> > The core guidelines suggests the use of west const. Could this be made the 
> > default?
> > https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rl-const
> https://reviews.llvm.org/D69764
> 
> 
> I was hoping for this feature to handle this instead.
> It is extremely complicated to figure out where to insert the `const`, 
> because pointer, pointee, value, templates, macros and so on and so forth.
> I implemented it such that it is always correct (the right side) and hoped 
> that clang-format can fix it up afterwards (`clang-tidy -fix -format`)
That's the correct approach -- we don't try to apply style choices to fixes in 
clang-tidy unless they're uncontentious (removing extra whitespace kinda 
stuff); we expect the user to run clang-format to fix those sort of things up.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31
+WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)),
+TransformValues(Options.get("TransformValues", 1)),
+TransformReferences(Options.get("TransformReferences", 1)),

JonasToth wrote:
> tiagoma wrote:
> > JonasToth wrote:
> > > Deactivating the transformations by default, as they are not ready for 
> > > production yet.
> > Ok. This got me off-guard. I would rather have this on.
> the transformations were buggy and required a lot of fix-up. This might not 
> be the case, but i would rather have it non-destructive by default because i 
> think many people will throw this check at their code-base.
> 
> if the transformations are solid already we can 

[clang] e7007b6 - [Sema] Add FixIt when a C++ out-of-line method has extra/missing const

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-12-15T14:30:54+01:00
New Revision: e7007b69d43b36032975d759ab4ae473fb389851

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

LOG: [Sema] Add FixIt when a C++ out-of-line method has extra/missing const

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

Added: 
clang/test/FixIt/member-mismatch.cpp

Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 421ca95bb5430..b1312a7ccc512 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -8504,7 +8504,14 @@ static NamedDecl *DiagnoseInvalidRedeclaration(
 << NewFD->getParamDecl(Idx - 1)->getType();
 } else if (FDisConst != NewFDisConst) {
   SemaRef.Diag(FD->getLocation(), diag::note_member_def_close_const_match)
-  << NewFDisConst << FD->getSourceRange().getEnd();
+  << NewFDisConst << FD->getSourceRange().getEnd()
+  << (NewFDisConst
+  ? FixItHint::CreateRemoval(ExtraArgs.D.getFunctionTypeInfo()
+ .getConstQualifierLoc())
+  : 
FixItHint::CreateInsertion(ExtraArgs.D.getFunctionTypeInfo()
+   .getRParenLoc()
+   .getLocWithOffset(1),
+   " const"));
 } else
   SemaRef.Diag(FD->getLocation(),
IsMember ? diag::note_member_def_close_match

diff  --git a/clang/test/FixIt/member-mismatch.cpp 
b/clang/test/FixIt/member-mismatch.cpp
new file mode 100644
index 0..2d2aec2e176b2
--- /dev/null
+++ b/clang/test/FixIt/member-mismatch.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+class Foo {
+  int get() const; // expected-note {{because it is const qualified}}
+  void set(int); // expected-note {{because it is not const qualified}}
+};
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:15-[[@LINE+1]]:15}:" const"
+int Foo::get() {} // expected-error {{does not match any declaration}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:20-[[@LINE+1]]:26}:""
+void Foo::set(int) const {} // expected-error {{does not match any 
declaration}}



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

[PATCH] D115567: [Sema] Add FixIt when a C++ out-of-line method has extra/missing const

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rGe7007b69d43b: [Sema] Add FixIt when a C++ out-of-line method 
has extra/missing const (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D115567?vs=393646&id=394536#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115567

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/FixIt/member-mismatch.cpp


Index: clang/test/FixIt/member-mismatch.cpp
===
--- /dev/null
+++ clang/test/FixIt/member-mismatch.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+class Foo {
+  int get() const; // expected-note {{because it is const qualified}}
+  void set(int); // expected-note {{because it is not const qualified}}
+};
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:15-[[@LINE+1]]:15}:" const"
+int Foo::get() {} // expected-error {{does not match any declaration}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:20-[[@LINE+1]]:26}:""
+void Foo::set(int) const {} // expected-error {{does not match any 
declaration}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8504,7 +8504,14 @@
 << NewFD->getParamDecl(Idx - 1)->getType();
 } else if (FDisConst != NewFDisConst) {
   SemaRef.Diag(FD->getLocation(), diag::note_member_def_close_const_match)
-  << NewFDisConst << FD->getSourceRange().getEnd();
+  << NewFDisConst << FD->getSourceRange().getEnd()
+  << (NewFDisConst
+  ? FixItHint::CreateRemoval(ExtraArgs.D.getFunctionTypeInfo()
+ .getConstQualifierLoc())
+  : 
FixItHint::CreateInsertion(ExtraArgs.D.getFunctionTypeInfo()
+   .getRParenLoc()
+   .getLocWithOffset(1),
+   " const"));
 } else
   SemaRef.Diag(FD->getLocation(),
IsMember ? diag::note_member_def_close_match


Index: clang/test/FixIt/member-mismatch.cpp
===
--- /dev/null
+++ clang/test/FixIt/member-mismatch.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+class Foo {
+  int get() const; // expected-note {{because it is const qualified}}
+  void set(int); // expected-note {{because it is not const qualified}}
+};
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:15-[[@LINE+1]]:15}:" const"
+int Foo::get() {} // expected-error {{does not match any declaration}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:20-[[@LINE+1]]:26}:""
+void Foo::set(int) const {} // expected-error {{does not match any declaration}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8504,7 +8504,14 @@
 << NewFD->getParamDecl(Idx - 1)->getType();
 } else if (FDisConst != NewFDisConst) {
   SemaRef.Diag(FD->getLocation(), diag::note_member_def_close_const_match)
-  << NewFDisConst << FD->getSourceRange().getEnd();
+  << NewFDisConst << FD->getSourceRange().getEnd()
+  << (NewFDisConst
+  ? FixItHint::CreateRemoval(ExtraArgs.D.getFunctionTypeInfo()
+ .getConstQualifierLoc())
+  : FixItHint::CreateInsertion(ExtraArgs.D.getFunctionTypeInfo()
+   .getRParenLoc()
+   .getLocWithOffset(1),
+   " const"));
 } else
   SemaRef.Diag(FD->getLocation(),
IsMember ? diag::note_member_def_close_match
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D115243#3185393 , @kadircet wrote:

> Yes and no. As mentioned SymbolSlabs are frozen once created and I totally 
> agree with the reasons there. But AFAICT in none of the indexes (apart from 
> monolithic FileIndex) we ever have a single SymbolSlab.
> We always have multiple of those and periodically build a single slab out of 
> them, by copying/merging each symbol so that we can build a SymbolIndex on 
> top of the final merged SymbolSlab. I was suggesting to perform ultimate 
> Origin tweaking there (we can even respect the Origins of each individual 
> slabs too).

Discussed this offline a bit.

We could pass around pairs of (SymbolSlab, Origin) and make it the index's 
responsibility to set the origin on each symbol. But I don't think this 
communicates so clearly that the origin reflects where the symbol came 
from/passed through, rather than what's serving it. (Given that Symbol::Origin 
exists in the slabs too).

> - Definitely less plumbing, we just let FileSymbol now what it should be 
> called during construction. Background/RemoteIndex already has this knowledge 
> hard coded.

Origin for an index isn't monolithic. Even a slab can contain symbols that are 
the result of merging, but FileIndex will soon have symbols from stdlib index + 
preamble index too.
Given this, I'm not sure it's much less plumbing - it's not just passing a 
constructor parameter to the index.

> - In theory those indexes that operate on top of a multiple SymbolSlabs 
> should be inserting their own "origin" into the final merged slab as it 
> contains some extra logic, just like MergeIndex.

This makes sense to me if it's useful, but I think this is *additional* to the 
origin reflecting the source of the slabs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115243

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

[PATCH] D113917: Add infrastructure to support matcher names.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D113917#3193374 , @ymandel wrote:

> Aaron - agreed on the points about `StringRef` vs `std::string`. But, before 
> I make that change, what did you think of moving to a more general method 
> `getMatcherSpec` that returns an object? That object will provide the name 
> (for now) and other data in the future (like the enum we discussed in the RFC 
> thread)?

I think that's a reasonable approach!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113917

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

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 394538.
sammccall added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115243

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolOrigin.cpp
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/index/remote/Index.proto
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp

Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -49,7 +49,6 @@
   End:
 Line: 1
 Column: 1
-Origin:128
 Flags:129
 Documentation:'Foo doc'
 ReturnType:'int'
@@ -121,6 +120,10 @@
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 
+auto readIndexFile(llvm::StringRef Text) {
+  return readIndexFile(Text, SymbolOrigin::Static);
+}
+
 TEST(SerializationTest, NoCrashOnEmptyYAML) {
   EXPECT_TRUE(bool(readIndexFile("")));
 }
@@ -143,7 +146,7 @@
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), "file:///path/foo.h");
-  EXPECT_EQ(Sym1.Origin, static_cast(1 << 7));
+  EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
   EXPECT_EQ(static_cast(Sym1.Flags), 129);
   EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -392,7 +392,7 @@
   L.Signature = "()";   // present in left only
   R.CompletionSnippetSuffix = "{$1:0}"; // present in right only
   R.Documentation = "--doc--";
-  L.Origin = SymbolOrigin::Dynamic;
+  L.Origin = SymbolOrigin::Preamble;
   R.Origin = SymbolOrigin::Static;
   R.Type = "expectedType";
 
@@ -404,8 +404,8 @@
   EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}");
   EXPECT_EQ(M.Documentation, "--doc--");
   EXPECT_EQ(M.Type, "expectedType");
-  EXPECT_EQ(M.Origin,
-SymbolOrigin::Dynamic | SymbolOrigin::Static | SymbolOrigin::Merge);
+  EXPECT_EQ(M.Origin, SymbolOrigin::Preamble | SymbolOrigin::Static |
+  SymbolOrigin::Merge);
 }
 
 TEST(MergeTest, PreferSymbolWithDefn) {
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -73,7 +73,8 @@
 if (Storage.find(ShardIdentifier) == Storage.end()) {
   return nullptr;
 }
-auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
+auto IndexFile =
+readIndexFile(Storage[ShardIdentifier], SymbolOrigin::Background);
 if (!IndexFile) {
   ADD_FAILURE() << "Error while reading " << ShardIdentifier << ':'
 << IndexFile.takeError();
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -586,7 +586,7 @@
 auto NewIndex = std::make_unique(std::make_unique());
 auto IndexLoadTask = [File = External.Location,
   PlaceHolder = NewIndex.get()] {
-  if (auto Idx = loadIndex(File, /*UseDex=*/true))
+  if (auto Idx = loadIndex(File, SymbolOrigin::Static, /*UseDex=*/true))
 PlaceHolder->reset(std::move(Idx));
 };
 if (Tasks) {
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -395,7 +395,8 @@
"{0}, new index was modified at {1}. Attempting to reload.",
LastStatus.getLastModificationTime(), Status->getLastModificationTime());
   LastStatus = *Status;
-  std::unique_ptr NewIndex = loadIndex

[clang] b9492ec - [CodeGen] Avoid some pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-15T14:46:10+01:00
New Revision: b9492ec649144976ec4748bdeebab9d5b92b67f4

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

LOG: [CodeGen] Avoid some pointer element type accesses

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGExpr.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 76d3cffbe659..79442d7f51c4 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4648,13 +4648,13 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 //
 // In other cases, we assert that the types match up (until pointers stop
 // having pointee types).
-llvm::Type *TypeFromVal;
 if (Callee.isVirtual())
-  TypeFromVal = Callee.getVirtualFunctionType();
-else
-  TypeFromVal =
-  Callee.getFunctionPointer()->getType()->getPointerElementType();
-assert(IRFuncTy == TypeFromVal);
+  assert(IRFuncTy == Callee.getVirtualFunctionType());
+else {
+  llvm::PointerType *PtrTy =
+  
llvm::cast(Callee.getFunctionPointer()->getType());
+  assert(PtrTy->isOpaqueOrPointeeTypeMatches(IRFuncTy));
+}
   }
 #endif
 

diff  --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h
index e3d9fec6d363..c8594068c3fc 100644
--- a/clang/lib/CodeGen/CGCall.h
+++ b/clang/lib/CodeGen/CGCall.h
@@ -115,7 +115,8 @@ class CGCallee {
 AbstractInfo = abstractInfo;
 assert(functionPtr && "configuring callee without function pointer");
 assert(functionPtr->getType()->isPointerTy());
-assert(functionPtr->getType()->getPointerElementType()->isFunctionTy());
+assert(functionPtr->getType()->isOpaquePointerTy() ||
+   functionPtr->getType()->getPointerElementType()->isFunctionTy());
   }
 
   static CGCallee forBuiltin(unsigned builtinID,

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 941671c61482..97ca3a1e1db1 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1146,7 +1146,7 @@ Address CodeGenModule::createUnnamedGlobalFrom(const 
VarDecl &D,
 CacheEntry->setAlignment(Align.getAsAlign());
   }
 
-  return Address(CacheEntry, Align);
+  return Address(CacheEntry, CacheEntry->getValueType(), Align);
 }
 
 static Address createUnnamedGlobalForMemcpyFrom(CodeGenModule &CGM,

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index b9317856715d..f22c09a09e3e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3697,7 +3697,7 @@ static Address emitArraySubscriptGEP(CodeGenFunction 
&CGF, Address addr,
 idx, DbgInfo);
   }
 
-  return Address(eltPtr, eltAlign);
+  return Address(eltPtr, CGF.ConvertTypeForMem(eltType), eltAlign);
 }
 
 LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E,



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

[PATCH] D115799: [clangd] Add ) to signature-help triggers

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

It is important for nested function calls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115799

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/test/initialize-params.test


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -107,7 +107,8 @@
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
-# CHECK-NEXT:  ","
+# CHECK-NEXT:  ",",
+# CHECK-NEXT:  ")"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "textDocumentSync": {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -555,7 +555,7 @@
}},
   {"signatureHelpProvider",
llvm::json::Object{
-   {"triggerCharacters", {"(", ","}},
+   {"triggerCharacters", {"(", ",", ")"}},
}},
   {"declarationProvider", true},
   {"definitionProvider", true},


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -107,7 +107,8 @@
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
-# CHECK-NEXT:  ","
+# CHECK-NEXT:  ",",
+# CHECK-NEXT:  ")"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "textDocumentSync": {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -555,7 +555,7 @@
}},
   {"signatureHelpProvider",
llvm::json::Object{
-   {"triggerCharacters", {"(", ","}},
+   {"triggerCharacters", {"(", ",", ")"}},
}},
   {"declarationProvider", true},
   {"definitionProvider", true},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Our plan is to land this after new year some time, so that remote index 
servers don't get disrupted by format changes while people are on holiday)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115243

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

[clang-tools-extra] 517f1d9 - [clangd] Add ) to signature-help triggers

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-12-15T14:56:27+01:00
New Revision: 517f1d9e5cd0b78b37eda83f58ed531f73b7a15c

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

LOG: [clangd] Add ) to signature-help triggers

It is important for nested function calls.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/test/initialize-params.test

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 3f0bff6e00032..93382466160c8 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -555,7 +555,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
}},
   {"signatureHelpProvider",
llvm::json::Object{
-   {"triggerCharacters", {"(", ","}},
+   {"triggerCharacters", {"(", ",", ")"}},
}},
   {"declarationProvider", true},
   {"definitionProvider", true},

diff  --git a/clang-tools-extra/clangd/test/initialize-params.test 
b/clang-tools-extra/clangd/test/initialize-params.test
index 0e33bcd74903d..a79f1075118ac 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -107,7 +107,8 @@
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
-# CHECK-NEXT:  ","
+# CHECK-NEXT:  ",",
+# CHECK-NEXT:  ")"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "textDocumentSync": {



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

[PATCH] D115799: [clangd] Add ) to signature-help triggers

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG517f1d9e5cd0: [clangd] Add ) to signature-help triggers 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115799

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/test/initialize-params.test


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -107,7 +107,8 @@
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
-# CHECK-NEXT:  ","
+# CHECK-NEXT:  ",",
+# CHECK-NEXT:  ")"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "textDocumentSync": {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -555,7 +555,7 @@
}},
   {"signatureHelpProvider",
llvm::json::Object{
-   {"triggerCharacters", {"(", ","}},
+   {"triggerCharacters", {"(", ",", ")"}},
}},
   {"declarationProvider", true},
   {"definitionProvider", true},


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -107,7 +107,8 @@
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
-# CHECK-NEXT:  ","
+# CHECK-NEXT:  ",",
+# CHECK-NEXT:  ")"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "textDocumentSync": {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -555,7 +555,7 @@
}},
   {"signatureHelpProvider",
llvm::json::Object{
-   {"triggerCharacters", {"(", ","}},
+   {"triggerCharacters", {"(", ",", ")"}},
}},
   {"declarationProvider", true},
   {"definitionProvider", true},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 394542.
serge-sans-paille added a comment.

Leverage the fact that AttributeSet nodes are already sorted


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

https://reviews.llvm.org/D115798

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenABITypes.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/lib/IR/AttributeImpl.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -602,7 +602,8 @@
 Align MemSetAlign =
 CI->getAttributes().getParamAttrs(0).getAlignment().valueOrOne();
 CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, MemSetAlign);
-AttrBuilder ArgAttrs(CI->getAttributes().getParamAttrs(0));
+SmallAttrBuilder ArgAttrs(CI->getContext(),
+  CI->getAttributes().getParamAttrs(0));
 NewCI->setAttributes(NewCI->getAttributes().addParamAttributes(
 CI->getContext(), 0, ArgAttrs));
 copyFlags(*CI, NewCI);
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1183,12 +1183,13 @@
   Begin->getIterator(), End->getIterator(), InlinerAttributeWindow + 1);
 }
 
-static AttrBuilder IdentifyValidAttributes(CallBase &CB) {
+static SmallAttrBuilder IdentifyValidAttributes(CallBase &CB) {
 
   AttrBuilder AB(CB.getAttributes(), AttributeList::ReturnIndex);
-  if (AB.empty())
-return AB;
-  AttrBuilder Valid;
+  SmallAttrBuilder Valid(CB.getContext());
+  if (AB.empty()) {
+return Valid;
+  }
   // Only allow these white listed attributes to be propagated back to the
   // callee. This is because other attributes may only be valid on the call
   // itself, i.e. attributes such as signext and zeroext.
@@ -1207,7 +1208,7 @@
   if (!UpdateReturnAttributes)
 return;
 
-  AttrBuilder Valid = IdentifyValidAttributes(CB);
+  SmallAttrBuilder Valid = IdentifyValidAttributes(CB);
   if (Valid.empty())
 return;
   auto *CalledFunction = CB.getCalledFunction();
@@ -1252,6 +1253,7 @@
 // existing attribute value (i.e. attributes such as dereferenceable,
 // dereferenceable_or_null etc). See AttrBuilder::merge for more details.
 AttributeList AL = NewRetVal->getAttributes();
+
 AttributeList NewAL = AL.addRetAttributes(Context, Valid);
 NewRetVal->setAttributes(NewAL);
   }
Index: llvm/lib/IR/Function.cpp
===
--- llvm/lib/IR/Function.cpp
+++ llvm/lib/IR/Function.cpp
@@ -549,6 +549,10 @@
   AttributeSets = AttributeSets.addFnAttributes(getContext(), Attrs);
 }
 
+void Function::addFnAttrs(const SmallAttrBuilder &Attrs) {
+  AttributeSets = AttributeSets.addFnAttributes(getContext(), Attrs);
+}
+
 void Function::addRetAttr(Attribute::AttrKind Kind) {
   AttributeSets = AttributeSets.addRetAttribute(getContext(), Kind);
 }
@@ -593,6 +597,10 @@
   AttributeSets = AttributeSets.removeFnAttributes(getContext(), Attrs);
 }
 
+void Function::removeFnAttrs(const SmallAttrBuilder &Attrs) {
+  AttributeSets = AttributeSets.removeFnAttributes(getContext(), Attrs);
+}
+
 void Function::removeRetAttr(Attribute::AttrKind Kind) {
   AttributeSets = AttributeSets.removeRetAttribute(getContext(), Kind);
 }
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -600,6 +600,10 @@
   return AttributeSet(AttributeSetNode::get(C, B));
 }
 
+AttributeSet AttributeSet::get(LLVMContext &C, const SmallAttrBuilder &B) {
+  return AttributeSet(AttributeSetNode::get(C, B));
+}
+
 AttributeSet AttributeSet::get(LLVMContext &C, ArrayRef Attrs) {
   return AttributeSet(AttributeSetNode::get(C, Attrs));
 }
@@ -607,14 +611,14 @@
 AttributeSet AttributeSet::addAttribute(LLVMContext &C,
 Attribute::AttrKind Kind) const {
   if (hasAttribute(Kind)) return *this;
-  AttrBuilder B;
+  SmallAttrBuilder B(C);
   B.addAttribute(Kind);
   return addAttributes(C, AttributeSet::get(C, B));
 }
 
 AttributeSet AttributeSet::addAttribute(LLVMContext &C, StringRef Kind,
 StringRef Value) const {
-  AttrBuilder B;
+  SmallAttrBuilder B(C);
   B.addAttribute(Kind, Value);
   return addAttributes(C, AttributeSet::get(C, B));
 }
@@ -627,7 +631,7 @@
   if (!AS.hasAttributes())
 return *this;
 
-  At

[PATCH] D115794: [clang-format] put non-empty catch block on one line with AllowShortBlocksOnASingleLine: Empty

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115794

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

[clang] b41bb6c - [Driver] Default to contemporary FreeBSD profiling behaviour

[Ed Maste via cfe-commits]

Author: Ed Maste
Date: 2021-12-15T09:05:35-05:00
New Revision: b41bb6c1b715da77025962f736740c78128c78e1

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

LOG: [Driver] Default to contemporary FreeBSD profiling behaviour

Prior to FreeBSD 14, FreeBSD provided special _p.a libraries for use
with -pg.  They are no longer used or provided.  If the target does
not specify a major version (e.g. amd64-unknown-freebsd, rather than
amd64-unknown-freebsd12) default to the new behaviour.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/test/Driver/freebsd.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp 
b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index d08ea282f6df..de635f5816cf 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -293,8 +293,8 @@ void freebsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
-  bool Profiling = Args.hasArg(options::OPT_pg) &&
-   ToolChain.getTriple().getOSMajorVersion() < 14;
+  unsigned Major = ToolChain.getTriple().getOSMajorVersion();
+  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
@@ -419,8 +419,8 @@ void FreeBSD::addLibStdCxxIncludePaths(
 void FreeBSD::AddCXXStdlibLibArgs(const ArgList &Args,
   ArgStringList &CmdArgs) const {
   CXXStdlibType Type = GetCXXStdlibType(Args);
-  bool Profiling =
-  Args.hasArg(options::OPT_pg) && getTriple().getOSMajorVersion() < 14;
+  unsigned Major = getTriple().getOSMajorVersion();
+  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
 
   switch (Type) {
   case ToolChain::CST_Libcxx:

diff  --git a/clang/test/Driver/freebsd.cpp b/clang/test/Driver/freebsd.cpp
index 512d29eeb64a..2304a61fa9ea 100644
--- a/clang/test/Driver/freebsd.cpp
+++ b/clang/test/Driver/freebsd.cpp
@@ -16,7 +16,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-PG-TEN %s
 // RUN: %clangxx %s -### -pg -o %t.o -target amd64-unknown-freebsd9.2 
-stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PG-NINE %s
-// CHECK-PG-DEFAULT: "-lc++_p" "-lm_p"
+// CHECK-PG-DEFAULT: "-lc++" "-lm"
 // CHECK-PG-FOURTEEN: "-lc++" "-lm"
 // CHECK-PG-TEN: "-lc++_p" "-lm_p"
 // CHECK-PG-NINE: "-lstdc++_p" "-lm_p"



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

[PATCH] D114396: [Driver] Default to current FreeBSD profiling behaviour

[Ed Maste via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb41bb6c1b715: [Driver] Default to contemporary FreeBSD 
profiling behaviour (authored by emaste).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114396

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/test/Driver/freebsd.cpp


Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -16,7 +16,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-PG-TEN %s
 // RUN: %clangxx %s -### -pg -o %t.o -target amd64-unknown-freebsd9.2 
-stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PG-NINE %s
-// CHECK-PG-DEFAULT: "-lc++_p" "-lm_p"
+// CHECK-PG-DEFAULT: "-lc++" "-lm"
 // CHECK-PG-FOURTEEN: "-lc++" "-lm"
 // CHECK-PG-TEN: "-lc++_p" "-lm_p"
 // CHECK-PG-NINE: "-lstdc++_p" "-lm_p"
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -293,8 +293,8 @@
   addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
-  bool Profiling = Args.hasArg(options::OPT_pg) &&
-   ToolChain.getTriple().getOSMajorVersion() < 14;
+  unsigned Major = ToolChain.getTriple().getOSMajorVersion();
+  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
@@ -419,8 +419,8 @@
 void FreeBSD::AddCXXStdlibLibArgs(const ArgList &Args,
   ArgStringList &CmdArgs) const {
   CXXStdlibType Type = GetCXXStdlibType(Args);
-  bool Profiling =
-  Args.hasArg(options::OPT_pg) && getTriple().getOSMajorVersion() < 14;
+  unsigned Major = getTriple().getOSMajorVersion();
+  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
 
   switch (Type) {
   case ToolChain::CST_Libcxx:


Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -16,7 +16,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-PG-TEN %s
 // RUN: %clangxx %s -### -pg -o %t.o -target amd64-unknown-freebsd9.2 -stdlib=platform 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PG-NINE %s
-// CHECK-PG-DEFAULT: "-lc++_p" "-lm_p"
+// CHECK-PG-DEFAULT: "-lc++" "-lm"
 // CHECK-PG-FOURTEEN: "-lc++" "-lm"
 // CHECK-PG-TEN: "-lc++_p" "-lm_p"
 // CHECK-PG-NINE: "-lstdc++_p" "-lm_p"
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -293,8 +293,8 @@
   addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
-  bool Profiling = Args.hasArg(options::OPT_pg) &&
-   ToolChain.getTriple().getOSMajorVersion() < 14;
+  unsigned Major = ToolChain.getTriple().getOSMajorVersion();
+  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
@@ -419,8 +419,8 @@
 void FreeBSD::AddCXXStdlibLibArgs(const ArgList &Args,
   ArgStringList &CmdArgs) const {
   CXXStdlibType Type = GetCXXStdlibType(Args);
-  bool Profiling =
-  Args.hasArg(options::OPT_pg) && getTriple().getOSMajorVersion() < 14;
+  unsigned Major = getTriple().getOSMajorVersion();
+  bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
 
   switch (Type) {
   case ToolChain::CST_Libcxx:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

> The RFC does not mention this builtin. It specifies 
> `__builtin_elementwise_rint` and `__builtin_elementwise_round`. Is there a 
> reason why this uses `roundeven` instead and should there be other rounding 
> options provided at the same time?

Well, I think there may be a misunderstanding. I referenced: 
https://clang.llvm.org/docs/LanguageExtensions.html#vector-builtins
There is this builtin:

  T __builtin_elementwise_roundeven(T x)

But I just found in the 
RFC(https://lists.llvm.org/pipermail/cfe-dev/2021-September/068999.html), we 
have:

  • __builtin_elementwise_rint
  • __builtin_elementwise_round

It looks confusing, I would like to review the RFC again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429

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

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

I like the general direction. A possible reframing would be SmallAttrBuilder -> 
MutableAttributeSet. I think my main question here would be in which contexts 
we still use / want to use the old AttrBuilder?




Comment at: llvm/include/llvm/IR/Attributes.h:974
+  SmallVector EnumAttrs;
+  SmallVector StringAttrs;
+  using iterator = typename SmallVector::iterator;

Just wondering if storing both in one vector would make sense? Would allow 
reusing the normal comparison on attributes and generally simplify the code. Or 
is that noticeably less efficient?



Comment at: llvm/include/llvm/IR/Attributes.h:1125
+  }
+  std::pair, ArrayRef> uniquify() const {
+return std::make_pair(ArrayRef(EnumAttrs),

What is uniquify() supposed to mean in this context?


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

https://reviews.llvm.org/D115798

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

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Actual benchmarks: 
https://llvm-compile-time-tracker.com/compare.php?from=7d97678df7f514c14b7611447dad02e9cc5168c9&to=f39a39e09e8f4f3b7dc94e4d23d9acfbf36ab2e5&stat=instructions


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

https://reviews.llvm.org/D115798

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

[PATCH] D115740: [clang][dataflow] Add simplistic constant-propagation analysis.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
Herald added a subscriber: mgorny.
ymandel updated this revision to Diff 394325.
ymandel added a comment.
ymandel updated this revision to Diff 394417.
ymandel updated this revision to Diff 394549.
ymandel edited the summary of this revision.
ymandel added reviewers: gribozavr2, sgatev, xazax.hun.
Herald added a subscriber: rnkovacs.
ymandel published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

fix potential null pointer deref.


ymandel added a comment.

Reworks analysis to use bottom, fixes two small bugs in testing framework.


ymandel added a comment.

revised bottom representation and updated tests


Adds a very simple constant-propagation analysis for demo and testing purposes.

Also, fixes two (small) bugs in the testing framework code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115740

Files:
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/ConstantPropagation.cpp
  clang/unittests/Analysis/FlowSensitive/ConstantPropagation.h
  clang/unittests/Analysis/FlowSensitive/ConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -1,4 +1,4 @@
-//===--- DataflowValues.h - Data structure for dataflow values --*- C++ -*-===//
+//===--- TestingSupport.h - Testing utils for dataflow analyses -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,9 +6,7 @@
 //
 //===--===//
 //
-// This file defines a skeleton data structure for encapsulating the dataflow
-// values for a CFG.  Typically this is subclassed to provide methods for
-// computing these values from a CFG.
+// This file defines utilities to simplify testing of dataflow analyses.
 //
 //===--===//
 
@@ -81,8 +79,7 @@
   using StateT = DataflowAnalysisState;
 
   llvm::Annotations AnnotatedCode(Code);
-  auto Unit = tooling::buildASTFromCodeWithArgs(
-  AnnotatedCode.code(), {"-fsyntax-only", "-std=c++17"});
+  auto Unit = tooling::buildASTFromCodeWithArgs(AnnotatedCode.code(), Args);
   auto &Context = Unit->getASTContext();
 
   if (Context.getDiagnostics().getClient()->getNumErrors() != 0) {
@@ -134,8 +131,7 @@
 return;
   if (auto *Lattice = llvm::any_cast(
   &State.Lattice.Value)) {
-Results.emplace_back(
-It->second, StateT{std::move(*Lattice), std::move(State.Env)});
+Results.emplace_back(It->second, StateT{*Lattice, State.Env});
   } else {
 FAIL() << "Could not cast lattice element to expected type.";
   }
Index: clang/unittests/Analysis/FlowSensitive/ConstantPropagationTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/ConstantPropagationTest.cpp
@@ -0,0 +1,231 @@
+#include "ConstantPropagation.h"
+#include "TestingSupport.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace dataflow {
+
+std::ostream &operator<<(std::ostream &OS,
+ const ConstantPropagationLattice &L) {
+  if (L == L.bottom())
+return OS << "None";
+  if (L == L.top())
+return OS << "Any";
+  return OS << L.Data->Var->getName().str() << " = " << L.Data->Value;
+}
+
+namespace {
+using ::testing::Pair;
+using ::testing::UnorderedElementsAre;
+
+MATCHER_P(HasConstantVal, v, "") {
+  return arg.Data.hasValue() && arg.Data->Value == v;
+}
+
+MATCHER(IsUnknown, "") { return arg == arg.bottom(); }
+MATCHER(Varies, "") { return arg == arg.top(); }
+
+MATCHER_P(HoldsCPLattice, m,
+  ((negation ? "doesn't hold" : "holds") +
+   llvm::StringRef(" a lattice element that ") +
+   ::testing::DescribeMatcher(m, negation))
+  .str()) {
+  return ExplainMatchResult(m, arg.Lattice, result_listener);
+}
+
+class ConstantPropagationTest : public ::testing::Test {
+protected:
+  ConstantPropagationTest() = default;
+
+  te

[PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 394553.
sammccall marked an inline comment as done.
sammccall added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

update lldb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
  clang/test/SemaCXX/warn-enum-compare.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/Tooling/StencilTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -559,7 +559,7 @@
 
 TEST_F(StencilTest, DescribeUnqualifiedType) {
   std::string Snippet = "using N::C; C c; c;";
-  std::string Expected = "N::C";
+  std::string Expected = "C";
   auto StmtMatch =
   matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
   ASSERT_TRUE(StmtMatch);
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/SemaCXX/warn-enum-compare.cpp
===
--- clang/test/SemaCXX/warn-enum-compare.cpp
+++ clang/test/SemaCXX/warn-enum-compare.cpp
@@ -78,15 +78,15 @@
 
   while (B1 == B2); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
   while (name1::B2 == name2::B3); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
+  while (z == name2::B2); // expected-warning  {{comparison of different enumeration types ('Baz' and 'name2::Baz')}}
 
   while (B1

[PATCH] D115728: [mlir][linalg] Replace LinalgOps.h and LinalgTypes.h by a single header.

[Tobias Gysi 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 rGb7f2c108eb87: [mlir][linalg] Replace LinalgOps.h and 
LinalgTypes.h by a single header. (authored by gysit).

Changed prior to commit:
  https://reviews.llvm.org/D115728?vs=394234&id=394526#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115728

Files:
  clang/docs/tools/clang-formatted-files.txt
  mlir/include/mlir/Conversion/LinalgToStandard/LinalgToStandard.h
  mlir/include/mlir/Dialect/Linalg/Analysis/DependenceAnalysis.h
  mlir/include/mlir/Dialect/Linalg/IR/Linalg.h
  mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.h
  mlir/include/mlir/Dialect/Linalg/IR/LinalgTypes.h
  mlir/include/mlir/Dialect/Linalg/Utils/Utils.h
  mlir/include/mlir/Dialect/SparseTensor/Utils/Merger.h
  mlir/include/mlir/InitAllDialects.h
  mlir/lib/CAPI/Dialect/Linalg.cpp
  mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp
  mlir/lib/Conversion/LinalgToSPIRV/LinalgToSPIRV.cpp
  mlir/lib/Conversion/LinalgToStandard/LinalgToStandard.cpp
  mlir/lib/Conversion/TosaToLinalg/TosaToLinalg.cpp
  mlir/lib/Conversion/TosaToLinalg/TosaToLinalgPass.cpp
  mlir/lib/Dialect/Linalg/Analysis/DependenceAnalysis.cpp
  mlir/lib/Dialect/Linalg/ComprehensiveBufferize/LinalgInterfaceImpl.cpp
  mlir/lib/Dialect/Linalg/IR/CMakeLists.txt
  mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp
  mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
  mlir/lib/Dialect/Linalg/IR/LinalgTypes.cpp
  mlir/lib/Dialect/Linalg/Transforms/Bufferize.cpp
  mlir/lib/Dialect/Linalg/Transforms/Detensorize.cpp
  mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
  mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
  mlir/lib/Dialect/Linalg/Transforms/ElementwiseToLinalg.cpp
  mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
  mlir/lib/Dialect/Linalg/Transforms/FusionOnTensors.cpp
  mlir/lib/Dialect/Linalg/Transforms/Generalization.cpp
  mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
  mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
  mlir/lib/Dialect/Linalg/Transforms/InlineScalarOperands.cpp
  mlir/lib/Dialect/Linalg/Transforms/Interchange.cpp
  mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp
  mlir/lib/Dialect/Linalg/Transforms/Loops.cpp
  mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp
  mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp
  mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp
  mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
  mlir/lib/Dialect/Linalg/Utils/Utils.cpp
  mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
  mlir/lib/Dialect/Vector/VectorTransforms.cpp
  mlir/test/lib/Dialect/Linalg/TestComprehensiveBufferize.cpp
  mlir/test/lib/Dialect/Linalg/TestLinalgCodegenStrategy.cpp
  mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp
  mlir/test/lib/Dialect/Linalg/TestLinalgHoisting.cpp
  mlir/test/lib/Dialect/Linalg/TestLinalgTransforms.cpp
  mlir/test/lib/Dialect/Test/TestDialect.h
  mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp
  mlir/test/lib/IR/TestSlicing.cpp
  utils/bazel/llvm-project-overlay/mlir/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -6568,11 +6568,10 @@
 name = "LinalgOps",
 srcs = [
 "lib/Dialect/Linalg/IR/LinalgOps.cpp",
-"lib/Dialect/Linalg/IR/LinalgTypes.cpp",
+"lib/Dialect/Linalg/IR/LinalgDialect.cpp",
 ],
 hdrs = [
-"include/mlir/Dialect/Linalg/IR/LinalgOps.h",
-"include/mlir/Dialect/Linalg/IR/LinalgTypes.h",
+"include/mlir/Dialect/Linalg/IR/Linalg.h"
 ],
 includes = ["include"],
 deps = [
Index: mlir/test/lib/IR/TestSlicing.cpp
===
--- mlir/test/lib/IR/TestSlicing.cpp
+++ mlir/test/lib/IR/TestSlicing.cpp
@@ -11,7 +11,7 @@
 //===--===//
 
 #include "mlir/Analysis/SliceAnalysis.h"
-#include "mlir/Dialect/Linalg/IR/LinalgOps.h"
+#include "mlir/Dialect/Linalg/IR/Linalg.h"
 #include "mlir/Dialect/StandardOps/IR/Ops.h"
 #include "mlir/IR/BlockAndValueMapping.h"
 #include "mlir/IR/BuiltinOps.h"
Index: mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp
===
--- mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp
+++ mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp
@@ -11,7 +11,7 @@
 #include "mlir/Analysis/SliceAnalysis.h"
 #include "mlir/Dialect/Affine/IR/AffineOps.h"
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
-#include "mlir/Dialect/Linalg/IR/LinalgOps.h"
+#include "mlir/Dialect/Linalg/IR/Linalg.h"
 #include "mlir/Dialect/Linalg/Passes.h"
 #include "mlir/Dialect/Linalg/Transforms/Transforms.h"
 #include

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added subscribers: estewart08, ronlieb.
ronlieb added a comment.
Herald added a reviewer: bondhugula.
Herald added a subscriber: sdasgup3.

@estewart08 thoughts on a good CMAKE variable to allow users to define 
equivalent of /opt/rocm  ?   and not use environment variable inside the cmake 
file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Diff was created without context unfortunately, scrolling through the cmake 
locally suggests variables have names of the form `LIBOMPTARGET_AMDGPU_FOO` 
though the namespacing isn't as consistent as it might be.

Something like:

  if (not defined (LIBOMPTARGET_AMDGPU_HSA_SEARCH_PATH))
  set(LIBOMPTARGET_AMDGPU_HSA_SEARCH_PATH "/opt/rocm")
  endif()

where I'm not confident of the cmake syntax for 'is this thing defined' or 
'assign', looks like it might be `if (NOT LIBOMPTARGET_AMDGPU_HSA_SEARCH_PATH)` 
and `set`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.h:974
+  SmallVector EnumAttrs;
+  SmallVector StringAttrs;
+  using iterator = typename SmallVector::iterator;

nikic wrote:
> Just wondering if storing both in one vector would make sense? Would allow 
> reusing the normal comparison on attributes and generally simplify the code. 
> Or is that noticeably less efficient?
Yeah, a single container is less efficient: It makes comparison more costly (as 
you need to check which kind of attribute we're dealing with each time), having 
two containers also reduces the cost of insert / remove (for low number of 
elements, we have higher chance to hit the container back). The lower bound 
alos typically takes less steps when the nature of attribute is known at 
compile time, which happens a lot.



Comment at: llvm/include/llvm/IR/Attributes.h:1125
+  }
+  std::pair, ArrayRef> uniquify() const {
+return std::make_pair(ArrayRef(EnumAttrs),

nikic wrote:
> What is uniquify() supposed to mean in this context?
Woops that's a leftover of a previous attempt when I was not trying to maintain 
the sorted invariant, I'll rename this.


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

https://reviews.llvm.org/D115798

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31
+WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)),
+TransformValues(Options.get("TransformValues", 1)),
+TransformReferences(Options.get("TransformReferences", 1)),

aaron.ballman wrote:
> JonasToth wrote:
> > tiagoma wrote:
> > > JonasToth wrote:
> > > > Deactivating the transformations by default, as they are not ready for 
> > > > production yet.
> > > Ok. This got me off-guard. I would rather have this on.
> > the transformations were buggy and required a lot of fix-up. This might not 
> > be the case, but i would rather have it non-destructive by default because 
> > i think many people will throw this check at their code-base.
> > 
> > if the transformations are solid already we can activate it again (or maybe 
> > references only or so).
> Are we still intending to be conservative here? This defaults some of the 
> Transform to 1 instead of 0, but that's different than what the documentation 
> currently reads.
good question. given the current result of it being quiet solid we might have 
it on? But I am fine with it being off.
I am not sure if it would induce too much transformations to handle. I am 
unsure.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:40
+void define_locals(T np_arg0, T &np_arg1, int np_arg2) {
+  T np_local0 = 0;
+  int p_local1 = 42;

aaron.ballman wrote:
> Are we being conservative here about the diagnostic? It seems like the 
> instantiations in the TU would mean that this could be declared `const`.
Yes, the check ignores templates when matching. The reasoning behind this was: 
too much at once. TU-local templates should be transformable, but it must be 
ensured that all instantiations can be `const`. Probably the best solution 
would be that the template uses concepts and the concepts allow `const` at this 
place.
This is hopefully follow-up :)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:120
+
+// taken from http://www.cplusplus.com/reference/type_traits/integral_constant/
+template 

aaron.ballman wrote:
> This is a problem -- I see no license on that site, but I do see "All rights 
> reserved". I don't think we should copy from there.
True. I will try to copy&paste from libc++ instead and remove the reference :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31
+WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)),
+TransformValues(Options.get("TransformValues", 1)),
+TransformReferences(Options.get("TransformReferences", 1)),

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > tiagoma wrote:
> > > > JonasToth wrote:
> > > > > Deactivating the transformations by default, as they are not ready 
> > > > > for production yet.
> > > > Ok. This got me off-guard. I would rather have this on.
> > > the transformations were buggy and required a lot of fix-up. This might 
> > > not be the case, but i would rather have it non-destructive by default 
> > > because i think many people will throw this check at their code-base.
> > > 
> > > if the transformations are solid already we can activate it again (or 
> > > maybe references only or so).
> > Are we still intending to be conservative here? This defaults some of the 
> > Transform to 1 instead of 0, but that's different than what the 
> > documentation currently reads.
> good question. given the current result of it being quiet solid we might have 
> it on? But I am fine with it being off.
> I am not sure if it would induce too much transformations to handle. I am 
> unsure.
Based on the test coverage we have, I think I'd be fine with it being on by 
default, but I do think the most conservative approach would be to be off by 
default. I don't have a strong opinion either way aside from "the documentation 
should match the code."



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:40
+void define_locals(T np_arg0, T &np_arg1, int np_arg2) {
+  T np_local0 = 0;
+  int p_local1 = 42;

JonasToth wrote:
> aaron.ballman wrote:
> > Are we being conservative here about the diagnostic? It seems like the 
> > instantiations in the TU would mean that this could be declared `const`.
> Yes, the check ignores templates when matching. The reasoning behind this 
> was: too much at once. TU-local templates should be transformable, but it 
> must be ensured that all instantiations can be `const`. Probably the best 
> solution would be that the template uses concepts and the concepts allow 
> `const` at this place.
> This is hopefully follow-up :)
Okay, that sounds good to me!



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:120
+
+// taken from http://www.cplusplus.com/reference/type_traits/integral_constant/
+template 

JonasToth wrote:
> aaron.ballman wrote:
> > This is a problem -- I see no license on that site, but I do see "All 
> > rights reserved". I don't think we should copy from there.
> True. I will try to copy&paste from libc++ instead and remove the reference :)
Thank you. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Jun Zhang via Phabricator via cfe-commits]

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

Explicitly speak out types for Arg, but don't repeat the vector type name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-reduction-math.c
  clang/test/Sema/builtins-reduction-math.c

Index: clang/test/Sema/builtins-reduction-math.c
===
--- clang/test/Sema/builtins-reduction-math.c
+++ clang/test/Sema/builtins-reduction-math.c
@@ -35,3 +35,20 @@
   i = __builtin_reduce_min(i);
   // expected-error@-1 {{1st argument must be a vector type (was 'int')}}
 }
+
+void test_builtin_reduce_xor(int i, float4 v, int3 iv) {
+  struct Foo s = __builtin_reduce_xor(iv);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+
+  i = __builtin_reduce_xor();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_reduce_xor(iv, iv);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  i = __builtin_reduce_xor(i);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'int')}}
+
+  i = __builtin_reduce_xor(v);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'float4' (vector of 4 'float' values))}}
+}
Index: clang/test/CodeGen/builtins-reduction-math.c
===
--- clang/test/CodeGen/builtins-reduction-math.c
+++ clang/test/CodeGen/builtins-reduction-math.c
@@ -57,3 +57,14 @@
   const si8 cvi1 = vi1;
   unsigned long long r5 = __builtin_reduce_min(cvi1);
 }
+
+void test_builtin_reduce_xor(si8 vi1, u4 vu1) {
+
+  // CHECK:  [[VI1:%.+]] = load <8 x i16>, <8 x i16>* %vi1.addr, align 16
+  // CHECK-NEXT: call i16 @llvm.vector.reduce.xor.v8i16(<8 x i16> [[VI1]])
+  short r2 = __builtin_reduce_xor(vi1);
+
+  // CHECK:  [[VU1:%.+]] = load <4 x i32>, <4 x i32>* %vu1.addr, align 16
+  // CHECK-NEXT: call i32 @llvm.vector.reduce.xor.v4i32(<4 x i32> [[VU1]])
+  unsigned r3 = __builtin_reduce_xor(vu1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2108,10 +2108,38 @@
   return ExprError();
 break;
   case Builtin::BI__builtin_reduce_max:
-  case Builtin::BI__builtin_reduce_min:
-if (SemaBuiltinReduceMath(TheCall))
+  case Builtin::BI__builtin_reduce_min: {
+if (SemaBuiltinReduceMathPreCheck(TheCall))
   return ExprError();
+
+const Expr *Arg = TheCall->getArg(0);
+const auto *TyA = Arg->getType()->getAs();
+if (!TyA) {
+  Diag(Arg->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+  << 1 << /* vector ty*/ 4 << Arg->getType();
+  return ExprError();
+}
+
+TheCall->setType(TyA->getElementType());
+break;
+  }
+
+  // __builtin_reduce_xor supports vector of integers only.
+  case Builtin::BI__builtin_reduce_xor: {
+if (SemaBuiltinReduceMathPreCheck(TheCall))
+  return ExprError();
+
+const Expr *Arg = TheCall->getArg(0);
+const auto *TyA = Arg->getType()->getAs();
+if (!TyA || !TyA->getElementType()->isIntegerType()) {
+  Diag(Arg->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+  << 1 /* vector of integers */ << 5 << Arg->getType();
+  return ExprError();
+}
+TheCall->setType(TyA->getElementType());
 break;
+  }
+
   case Builtin::BI__builtin_matrix_transpose:
 return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 
@@ -16752,7 +16780,7 @@
   return false;
 }
 
-bool Sema::SemaBuiltinReduceMath(CallExpr *TheCall) {
+bool Sema::SemaBuiltinReduceMathPreCheck(CallExpr *TheCall) {
   if (checkArgCount(*this, TheCall, 1))
 return true;
 
@@ -16761,14 +16789,6 @@
 return true;
 
   TheCall->setArg(0, A.get());
-  const VectorType *TyA = A.get()->getType()->getAs();
-  if (!TyA) {
-SourceLocation ArgLoc = TheCall->getArg(0)->getBeginLoc();
-return Diag(ArgLoc, diag::err_builtin_invalid_arg_type)
-   << 1 << /* vector ty*/ 4 << A.get()->getType();
-  }
-
-  TheCall->setType(TyA->getElementType());
   return false;
 }
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3204,6 +3204,13 @@
 return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_reduce_xor: {
+Value *Op0 = EmitScalarExpr(E->getArg(0));
+Value *Result = Builder.CreateUnaryIntrinsic(
+llvm::Intrinsic::vector_reduce_xor, Op0, nullptr, "rdx.xor");
+return RV

[PATCH] D115670: Correct behavior of Vector boolean-operations, implement vector operator-

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Note, my pre-merge check failure seems completely unrelated.  It seems to have 
some problem with the some 'go' bindings, but I don't believe that has anything 
to do with this change.


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

https://reviews.llvm.org/D115670

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

[PATCH] D115740: [clang][dataflow] Add simplistic constant-propagation analysis.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

updated file header


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115740

Files:
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -1,4 +1,4 @@
-//===--- DataflowValues.h - Data structure for dataflow values --*- C++ -*-===//
+//===--- TestingSupport.h - Testing utils for dataflow analyses -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,9 +6,7 @@
 //
 //===--===//
 //
-// This file defines a skeleton data structure for encapsulating the dataflow
-// values for a CFG.  Typically this is subclassed to provide methods for
-// computing these values from a CFG.
+// This file defines utilities to simplify testing of dataflow analyses.
 //
 //===--===//
 
@@ -81,8 +79,7 @@
   using StateT = DataflowAnalysisState;
 
   llvm::Annotations AnnotatedCode(Code);
-  auto Unit = tooling::buildASTFromCodeWithArgs(
-  AnnotatedCode.code(), {"-fsyntax-only", "-std=c++17"});
+  auto Unit = tooling::buildASTFromCodeWithArgs(AnnotatedCode.code(), Args);
   auto &Context = Unit->getASTContext();
 
   if (Context.getDiagnostics().getClient()->getNumErrors() != 0) {
@@ -134,8 +131,7 @@
 return;
   if (auto *Lattice = llvm::any_cast(
   &State.Lattice.Value)) {
-Results.emplace_back(
-It->second, StateT{std::move(*Lattice), std::move(State.Env)});
+Results.emplace_back(It->second, StateT{*Lattice, State.Env});
   } else {
 FAIL() << "Could not cast lattice element to expected type.";
   }
Index: clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
@@ -0,0 +1,374 @@
+//===- unittests/Analysis/FlowSensitive/SingelVarConstantPropagation.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines a simplistic version of Constant Propagation as an example
+// of a forward, monotonic dataflow analysis. The analysis only tracks one
+// variable at a time -- the one with the most recent declaration encountered.
+//
+//===--===//
+
+#include "TestingSupport.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace dataflow {
+namespace {
+using namespace ast_matchers;
+
+// A semi-lattice for dataflow analysis that tracks the value of a single
+// integer variable. If it can be identified with a single (constant) value,
+// then that value is stored.
+struct ConstantPropagationLattice {
+  // A null `Var` represents "top": either more than one value is possible or
+  // more than one variable was encountered. Otherwise, `Data` indicates that
+  // `Var` has the given `Value` at the program point with which this lattice
+  // element is associated, for all paths through the program.
+  struct VarValue {
+const VarDecl *Var;
+int64_t Value;
+
+friend bool operator==(VarValue Lhs, VarValue Rhs) {
+  return Lhs.Var == Rhs.Var && Lhs.Value == Rhs.Value;
+}
+  };
+  // `None` is "bottom".
+  llvm::Optional Data;
+
+  static constexpr ConstantPropagationLattice bottom() { return {llvm::None}; }
+  stat

[PATCH] D115740: [clang][dataflow] Add simplistic constant-propagation analysis.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Coalesce all CP files into one test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115740

Files:
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -1,4 +1,4 @@
-//===--- DataflowValues.h - Data structure for dataflow values --*- C++ -*-===//
+//===--- TestingSupport.h - Testing utils for dataflow analyses -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,9 +6,7 @@
 //
 //===--===//
 //
-// This file defines a skeleton data structure for encapsulating the dataflow
-// values for a CFG.  Typically this is subclassed to provide methods for
-// computing these values from a CFG.
+// This file defines utilities to simplify testing of dataflow analyses.
 //
 //===--===//
 
@@ -81,8 +79,7 @@
   using StateT = DataflowAnalysisState;
 
   llvm::Annotations AnnotatedCode(Code);
-  auto Unit = tooling::buildASTFromCodeWithArgs(
-  AnnotatedCode.code(), {"-fsyntax-only", "-std=c++17"});
+  auto Unit = tooling::buildASTFromCodeWithArgs(AnnotatedCode.code(), Args);
   auto &Context = Unit->getASTContext();
 
   if (Context.getDiagnostics().getClient()->getNumErrors() != 0) {
@@ -134,8 +131,7 @@
 return;
   if (auto *Lattice = llvm::any_cast(
   &State.Lattice.Value)) {
-Results.emplace_back(
-It->second, StateT{std::move(*Lattice), std::move(State.Env)});
+Results.emplace_back(It->second, StateT{*Lattice, State.Env});
   } else {
 FAIL() << "Could not cast lattice element to expected type.";
   }
Index: clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
@@ -0,0 +1,368 @@
+//===--===//
+//
+// This file defines a simplistic version of Constant Propagation as an example
+// of a forward, monotonic dataflow analysis. The analysis only tracks one
+// variable at a time -- the one with the most recent declaration encountered.
+//
+//===--===//
+
+#include "TestingSupport.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace dataflow {
+namespace {
+using namespace ast_matchers;
+
+// A semi-lattice for dataflow analysis that tracks the value of a single
+// integer variable. If it can be identified with a single (constant) value,
+// then that value is stored.
+struct ConstantPropagationLattice {
+  // A null `Var` represents "top": either more than one value is possible or
+  // more than one variable was encountered. Otherwise, `Data` indicates that
+  // `Var` has the given `Value` at the program point with which this lattice
+  // element is associated, for all paths through the program.
+  struct VarValue {
+const VarDecl *Var;
+int64_t Value;
+
+friend bool operator==(VarValue Lhs, VarValue Rhs) {
+  return Lhs.Var == Rhs.Var && Lhs.Value == Rhs.Value;
+}
+  };
+  // `None` is "bottom".
+  llvm::Optional Data;
+
+  static constexpr ConstantPropagationLattice bottom() { return {llvm::None}; }
+  static constexpr ConstantPropagationLattice top() {
+return {VarValue{nullptr, 0}};
+  }
+
+  friend bool operator==(ConstantPropagationLattice Element1,
+ ConstantPropagationLattice Element2) {
+return Element1.Data == Element2.Data;
+  }
+
+ 

[PATCH] D115803: [clang-format] Fix tabs when using BreakBeforeTernaryOperators=false.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/52724.

This is rather a workaround than a correct fix. To properly fix it, we'd need 
to find a better way to tell when not to decrease the StartOfTokenColumn.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115803

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13452,6 +13452,25 @@
"}",
Tab));
 
+  verifyFormat("void f() {\n"
+   "\treturn true ? aa\n"
+   "\t: bb\n"
+   "}",
+   Tab);
+  FormatStyle TabNoBreak = Tab;
+  TabNoBreak.BreakBeforeTernaryOperators = false;
+  verifyFormat("void f() {\n"
+   "\treturn true ? aa :\n"
+   "\t  bb\n"
+   "}",
+   TabNoBreak);
+  verifyFormat("void f() {\n"
+   "\treturn true ?\n"
+   "\t    :\n"
+   "\t   \n"
+   "}",
+   TabNoBreak);
+
   Tab.UseTab = FormatStyle::UT_Never;
   EXPECT_EQ("/*\n"
 "  a\t\tcomment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1282,9 +1282,12 @@
  C.EscapedNewlineColumn);
   else
 appendNewlineText(ReplacementText, C.NewlinesBefore);
+  // FIXME: This assert should hold if we computed the column correctly.
+  // assert((int)C.StartOfTokenColumn >= C.Spaces);
   appendIndentText(
   ReplacementText, C.Tok->IndentLevel, std::max(0, C.Spaces),
-  C.StartOfTokenColumn - std::max(0, C.Spaces), C.IsAligned);
+  std::max((int)C.StartOfTokenColumn, C.Spaces) - std::max(0, 
C.Spaces),
+  C.IsAligned);
   ReplacementText.append(C.CurrentLinePrefix);
   storeReplacement(C.OriginalWhitespaceRange, ReplacementText);
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13452,6 +13452,25 @@
"}",
Tab));
 
+  verifyFormat("void f() {\n"
+   "\treturn true ? aa\n"
+   "\t: bb\n"
+   "}",
+   Tab);
+  FormatStyle TabNoBreak = Tab;
+  TabNoBreak.BreakBeforeTernaryOperators = false;
+  verifyFormat("void f() {\n"
+   "\treturn true ? aa :\n"
+   "\t  bb\n"
+   "}",
+   TabNoBreak);
+  verifyFormat("void f() {\n"
+   "\treturn true ?\n"
+   "\t    :\n"
+   "\t   \n"
+   "}",
+   TabNoBreak);
+
   Tab.UseTab = FormatStyle::UT_Never;
   EXPECT_EQ("/*\n"
 "  a\t\tcomment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1282,9 +1282,12 @@
  C.EscapedNewlineColumn);
   else
 appendNewlineText(ReplacementText, C.NewlinesBefore);
+  // FIXME: This assert should hold if we computed the column correctly.
+  // assert((int)C.StartOfTokenColumn >= C.Spaces);
   appendIndentText(
   ReplacementText, C.Tok->IndentLevel, std::max(0, C.Spaces),
-  C.StartOfTokenColumn - std::max(0, C.Spaces), C.IsAligned);
+  std::max((int)C.StartOfTokenColumn, C.Spaces) - std::max(0, C.Spaces),
+  C.IsAligned);
   ReplacementText.append(C.CurrentLinePrefix);
   storeReplacement(C.OriginalWhitespaceRange, ReplacementText);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115576: [clang-tidy][#51939] Exempt placement-new expressions from 'bugprone-throw-keyword-missing'

[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! Can you also add a release note about the fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115576

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

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

[Sanjay Patel via Phabricator via cfe-commits]

spatel created this revision.
spatel added reviewers: MatzeB, neildhar, nikic, aqjune, dmgreen.
Herald added a subscriber: mcrosier.
spatel requested review of this revision.

We got an unintended consequence of the optimizer getting smarter when 
compiling in a non-standard mode, and there's no good way to inhibit those 
optimizations at a later stage. The test is based on an example linked from 
D92270 .

We allow the "no-strict-float-cast-overflow" exception to normal C cast rules 
to preserve legacy code that does not expect overflowing casts from FP to int 
to produce UB. See D46236  for details.


https://reviews.llvm.org/D115804

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/no-junk-ftrunc.c


Index: clang/test/CodeGen/no-junk-ftrunc.c
===
--- clang/test/CodeGen/no-junk-ftrunc.c
+++ clang/test/CodeGen/no-junk-ftrunc.c
@@ -1,14 +1,23 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | 
FileCheck %s --check-prefix=NOSTRICT
+
+// When compiling with non-standard semantics, use intrinsics to inhibit the 
optimizer.
+
 // NOSTRICT-LABEL: main
+// NOSTRICT: call i32 @llvm.fptosi.sat.i32.f64
+// NOSTRICT: call i32 @llvm.fptoui.sat.i32.f64
 // NOSTRICT: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=STRICT
 // STRICT-LABEL: main
+// STRICT: = fptosi
+// STRICT: = fptoui
 // STRICT-NOT: strict-float-cast-overflow
 
+
 int main() {
-  return 0;
+  double d = 1e20;
+  return (int)d != 1e20 && (unsigned)d != 1e20;
 }
-
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1240,7 +1240,18 @@
 
   if (isa(DstElementTy)) {
 assert(SrcElementTy->isFloatingPointTy() && "Unknown real conversion");
-if (DstElementType->isSignedIntegerOrEnumerationType())
+bool IsSigned = DstElementType->isSignedIntegerOrEnumerationType();
+
+// If we can't recognize overflow as undefined behavior, assume that
+// overflow saturates. This protects against normal optimizations if we are
+// compiling with non-standard FP semantics.
+if (!CGF.CGM.getCodeGenOpts().StrictFloatCastOverflow) {
+  llvm::Intrinsic::ID IID =
+  IsSigned ? llvm::Intrinsic::fptosi_sat : llvm::Intrinsic::fptoui_sat;
+  return Builder.CreateCall(CGF.CGM.getIntrinsic(IID, {DstTy, SrcTy}), 
Src);
+}
+
+if (IsSigned)
   return Builder.CreateFPToSI(Src, DstTy, "conv");
 return Builder.CreateFPToUI(Src, DstTy, "conv");
   }


Index: clang/test/CodeGen/no-junk-ftrunc.c
===
--- clang/test/CodeGen/no-junk-ftrunc.c
+++ clang/test/CodeGen/no-junk-ftrunc.c
@@ -1,14 +1,23 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | FileCheck %s --check-prefix=NOSTRICT
+
+// When compiling with non-standard semantics, use intrinsics to inhibit the optimizer.
+
 // NOSTRICT-LABEL: main
+// NOSTRICT: call i32 @llvm.fptosi.sat.i32.f64
+// NOSTRICT: call i32 @llvm.fptoui.sat.i32.f64
 // NOSTRICT: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=STRICT
 // STRICT-LABEL: main
+// STRICT: = fptosi
+// STRICT: = fptoui
 // STRICT-NOT: strict-float-cast-overflow
 
+
 int main() {
-  return 0;
+  double d = 1e20;
+  return (int)d != 1e20 && (unsigned)d != 1e20;
 }
-
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1240,7 +1240,18 @@
 
   if (isa(DstElementTy)) {
 assert(SrcElementTy->isFloatingPointTy() && "Unknown real conversion");
-if (DstElementType->isSignedIntegerOrEnumerationType())
+bool IsSigned = DstElementType->isSignedIntegerOrEnumerationType();
+
+// If we can't recognize overflow as undefined behavior, assume that
+// overflow saturates. This protects against normal optimizations if we are
+// compiling with non-standard FP semantics.
+if (!CGF.CGM.getCodeGenOpts().StrictFloatCastOverflow) {
+  llvm::Intrinsic::ID IID =
+  IsSigned ? llvm::Intrinsic::fptosi_sat : llvm::Intrinsic::fptoui_sat;
+  return Builder.CreateCall(CGF.CGM.getIntrinsic(IID, {DstTy, SrcTy}), Src);
+}
+
+if (IsSigned)
   return Builder.CreateFPToSI(Src, DstTy, "conv");
 return Builder.CreateFPToUI(Src, DstTy, "

[PATCH] D115670: Correct behavior of Vector boolean-operations, implement vector operator-

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:10389
+return false;
+  case UO_Minus: {
+for (unsigned EltNum = 0; EltNum < VD->getNumElements(); ++EltNum) {

Would it be worth handling ~ and ! as well given that they seem pretty trivial 
to support? I don't insist, but I'm trying to reason whether the `for` loop 
should be hoisted out of the `switch` because it seems like it'll be needed for 
all of the cases.



Comment at: clang/lib/Sema/SemaExpr.cpp:12262-12287
 if (TypeSize == Context.getTypeSize(Context.CharTy))
   return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
 else if (TypeSize == Context.getTypeSize(Context.ShortTy))
   return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
 else if (TypeSize == Context.getTypeSize(Context.IntTy))
   return Context.getExtVectorType(Context.IntTy, VTy->getNumElements());
+else if (TypeSize == Context.getTypeSize(Context.Int128Ty))

NFC nit: a whole pile of `else after return` that can be removed someday.


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

https://reviews.llvm.org/D115670

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

[PATCH] D92270: [ConstantFold] Fold more operations to poison

[Sanjay Patel via Phabricator via cfe-commits]

spatel added inline comments.



Comment at: llvm/lib/IR/ConstantFold.cpp:633
 // the input constant.
-return UndefValue::get(DestTy);
+return PoisonValue::get(DestTy);
   }

aqjune wrote:
> neildhar wrote:
> > spatel wrote:
> > > MatzeB wrote:
> > > > I believe this is causing some of our clients trouble, especially since 
> > > > we somehow have a `-fno-strict-float-cast-overflow` flag in clang that 
> > > > says float->int overflows are not UB... (CC @spatel )
> > > I can guess at what the example looks like, but it would be great to have 
> > > a reduced test.
> > > There should be a function attribute in IR corresponding to that clang 
> > > flag, so we could alter the behavior here based on checking that? Not 
> > > sure if there's precedence for that kind of transform though.
> > Here's a minimal repro for the issue we ran into: 
> > https://godbolt.org/z/Wdr7q1a9M
> Clang is lowering fp-to-int casts into fptosi/ui 
> (https://godbolt.org/z/Gz3Y7YKKf), but I think in this case clang must emit 
> the fptosi.sat intrinsic: 
> https://llvm.org/docs/LangRef.html#llvm-fptosi-sat-intrinsic
> It guarantees that the result is well-defined.
I agree with this suggestion. Here's a patch proposal:
D115804


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92270

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

[clang-tools-extra] b7d5577 - [clang-tidy][#51939] Exempt placement-new expressions from 'bugprone-throw-keyword-missing'

[Markus Böck via cfe-commits]

Author: Markus Böck
Date: 2021-12-15T16:59:14+01:00
New Revision: b7d55771ce3ecc5c1768bd5877dbdd312f9ab0d7

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

LOG: [clang-tidy][#51939] Exempt placement-new expressions from 
'bugprone-throw-keyword-missing'

The purpose of this checker is to flag a missing throw keyword, and does so by 
checking for the construction of an exception class that is then unused.
This works great except that placement new expressions are also flagged as 
those lead to the construction of an object as well, even though they are not 
temporary (as that is dependent on the storage).
This patch fixes the issue by exempting the match if it is within a 
placement-new.

Fixes https://github.com/llvm/llvm-project/issues/51939

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
index 5327a0c8d4c6b..3f5d875f471dd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
@@ -24,11 +24,13 @@ void ThrowKeywordMissingCheck::registerMatchers(MatchFinder 
*Finder) {
   cxxConstructExpr(
   hasType(cxxRecordDecl(
   isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION",
-  unless(anyOf(hasAncestor(stmt(
-   anyOf(cxxThrowExpr(), callExpr(), returnStmt(,
-   hasAncestor(decl(anyOf(varDecl(), fieldDecl(,
-   allOf(hasAncestor(CtorInitializerList),
- unless(hasAncestor(cxxCatchStmt()))
+  unless(anyOf(
+  hasAncestor(
+  stmt(anyOf(cxxThrowExpr(), callExpr(), returnStmt(,
+  hasAncestor(decl(anyOf(varDecl(), fieldDecl(,
+  hasAncestor(expr(cxxNewExpr(hasAnyPlacementArg(anything(),
+  allOf(hasAncestor(CtorInitializerList),
+unless(hasAncestor(cxxCatchStmt()))
   .bind("temporary-exception-not-thrown"),
   this);
 }

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 341eb82a1e903..bcb9a0f40719c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,10 @@ Changes in existing checks
 
 - Fixed a false positive in :doc:`fuchsia-trailing-return
   ` for C++17 deduction guides.
+  
+- Fixed a false positive in :doc:`bugprone-throw-keyword-missing
+  ` when creating an 
exception object
+  using placement new
 
 Removed checks
 ^^

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
index dff600c947070..49233c0deefdf 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
@@ -175,3 +175,14 @@ struct ExceptionRAII {
 void exceptionRAIITest() {
   ExceptionRAII E;
 }
+
+namespace std {
+typedef decltype(sizeof(void*)) size_t;
+}
+
+void* operator new(std::size_t, void*);
+
+void placeMentNewTest() {
+  alignas(RegularException) unsigned char expr[sizeof(RegularException)];
+  new (expr) RegularException{};
+}



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

[PATCH] D115576: [clang-tidy][#51939] Exempt placement-new expressions from 'bugprone-throw-keyword-missing'

[Markus Böck via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7d55771ce3e: [clang-tidy][#51939] Exempt placement-new 
expressions from 'bugprone-throw… (authored by zero9178).

Changed prior to commit:
  https://reviews.llvm.org/D115576?vs=393671&id=394575#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115576

Files:
  clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
@@ -175,3 +175,14 @@
 void exceptionRAIITest() {
   ExceptionRAII E;
 }
+
+namespace std {
+typedef decltype(sizeof(void*)) size_t;
+}
+
+void* operator new(std::size_t, void*);
+
+void placeMentNewTest() {
+  alignas(RegularException) unsigned char expr[sizeof(RegularException)];
+  new (expr) RegularException{};
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,10 @@
 
 - Fixed a false positive in :doc:`fuchsia-trailing-return
   ` for C++17 deduction guides.
+  
+- Fixed a false positive in :doc:`bugprone-throw-keyword-missing
+  ` when creating an 
exception object
+  using placement new
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
@@ -24,11 +24,13 @@
   cxxConstructExpr(
   hasType(cxxRecordDecl(
   isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION",
-  unless(anyOf(hasAncestor(stmt(
-   anyOf(cxxThrowExpr(), callExpr(), returnStmt(,
-   hasAncestor(decl(anyOf(varDecl(), fieldDecl(,
-   allOf(hasAncestor(CtorInitializerList),
- unless(hasAncestor(cxxCatchStmt()))
+  unless(anyOf(
+  hasAncestor(
+  stmt(anyOf(cxxThrowExpr(), callExpr(), returnStmt(,
+  hasAncestor(decl(anyOf(varDecl(), fieldDecl(,
+  hasAncestor(expr(cxxNewExpr(hasAnyPlacementArg(anything(),
+  allOf(hasAncestor(CtorInitializerList),
+unless(hasAncestor(cxxCatchStmt()))
   .bind("temporary-exception-not-thrown"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-throw-keyword-missing.cpp
@@ -175,3 +175,14 @@
 void exceptionRAIITest() {
   ExceptionRAII E;
 }
+
+namespace std {
+typedef decltype(sizeof(void*)) size_t;
+}
+
+void* operator new(std::size_t, void*);
+
+void placeMentNewTest() {
+  alignas(RegularException) unsigned char expr[sizeof(RegularException)];
+  new (expr) RegularException{};
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,10 @@
 
 - Fixed a false positive in :doc:`fuchsia-trailing-return
   ` for C++17 deduction guides.
+  
+- Fixed a false positive in :doc:`bugprone-throw-keyword-missing
+  ` when creating an exception object
+  using placement new
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
@@ -24,11 +24,13 @@
   cxxConstructExpr(
   hasType(cxxRecordDecl(
   isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION",
-  unless(anyOf(hasAncestor(stmt(
-   anyOf(cxxThrowExpr(), callExpr(), returnStmt(,
-   hasAncestor(decl(anyOf(varDecl(), fieldDecl(,
-   allOf(hasAncestor(CtorInitializerList),
- unless(hasAncestor(cxxCatchStmt()))
+  unless(anyOf(
+  hasAncestor(
+  stmt(anyOf(cxxThrowExpr(), callExpr(), returnStmt()))

[PATCH] D115576: [clang-tidy][#51939] Exempt placement-new expressions from 'bugprone-throw-keyword-missing'

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D115576#3194944 , @aaron.ballman 
wrote:

> LGTM! Can you also add a release note about the fix?

Addressed in the final commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115576

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

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Looks reasonable. Making float to int cast well defined is exactly why these 
intrinsics exist. Should we also drop the "string-float-cast-overflow" 
attribute, as UB is now prevented in a different way?


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

https://reviews.llvm.org/D115804

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

[PATCH] D115740: [clang][dataflow] Add simplistic constant-propagation analysis.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:69
+
+  friend bool operator==(ConstantPropagationLattice Element1,
+ ConstantPropagationLattice Element2) {

Should this be a const ref? Same below.



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:97
+
+} // namespace
+

Why not put the definitions below also in the anonymous namespace?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:119
+
+static llvm::Optional transferInternal(const BoundNodes &Nodes,
+const ASTContext &Context) {

What do you think about inlining this in 
`ConstantPropagationAnalysis::transfer`? We don't expect to call it from 
elsewhere, right?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:160
+
+  ConstantPropagationLattice transfer(const Stmt *Stmt,
+  ConstantPropagationLattice Element,

Maybe call this `S` to disambiguate from the name of the type?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:161
+  ConstantPropagationLattice transfer(const Stmt *Stmt,
+  ConstantPropagationLattice Element,
+  Environment &Env) {

Should this be a const ref?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:180
+
+MATCHER_P(HasConstantVal, v, "") {
+  return arg.Data.hasValue() && arg.Data->Value == v;

Maybe call this `HasConstVal` if you'd like to keep this short?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:197
+protected:
+  ConstantPropagationTest() = default;
+

Is this necessary?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:217
+TEST_F(ConstantPropagationTest, JustInit) {
+  std::string code = R"(
+void fun() {

Capitalize - `Code`? Or maybe inline this in the function call? Same below.



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:245
+
+TEST_F(ConstantPropagationTest, Assignment) {
+  std::string code = R"(

Maybe add tests for assignment from a function call (e.g. `int target = 
foo();`) and assignment from an arithmetic expression (e.g. `int target = 1 + 
2;`)? Both should result in `IsUnknown()` state, right?



Comment at: 
clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp:342
+  if (b) {
+target = 1;
+  } else {

Add internal checks, similar to those in `SameAssignmentInBranches`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115740

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:447
 Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+if (Tok.is(tok::ellipsis)) {

erichkeane wrote:
> steffenlarsen wrote:
> > erichkeane wrote:
> > > steffenlarsen wrote:
> > > > erichkeane wrote:
> > > > > steffenlarsen wrote:
> > > > > > erichkeane wrote:
> > > > > > > So I was thinking about this overnight... I wonder if this logic 
> > > > > > > is inverted here?  Aaron can better answer, but I wonder if we 
> > > > > > > should be instead detecting when we are on the 'last' parameter 
> > > > > > > and it is one of these `VariadicExprArgument` things (that accept 
> > > > > > > a pack), then dropping the parser into a loop of:
> > > > > > > 
> > > > > > > while (Tok.is(tok::comma)){
> > > > > > >   Tok.Consume();
> > > > > > >   ArgExpr = CorrectTypos(ParseAssignmentExpr(...));
> > > > > > > }
> > > > > > > // finally, consume the closing paren.
> > > > > > > 
> > > > > > > WDYT?
> > > > > > The parsing of attribute arguments is already this lenient. It does 
> > > > > > not actually care how many arguments the attribute says it takes 
> > > > > > (unless it takes a type and then it will only currently supply 
> > > > > > _one_ argument). It simply consumes as many expressions it can 
> > > > > > before reaching the end parenthesis, then leaves the attributes to 
> > > > > > consume them in `clang/lib/Sema/SemaDeclAttr.cpp`.
> > > > > > As a result we do not need any additional work here to handle 
> > > > > > variadic arguments, but it also means that we have to introduce the 
> > > > > > restrictions that we can only allow packs in the last argument and 
> > > > > > allow only that argument to be variadic, as otherwise we have no 
> > > > > > way of knowing when and where the packs pass between argument 
> > > > > > boundaries.
> > > > > My point is, I don't think THIS function should be handling the 
> > > > > ellipsis, the expression parsing functions we send the parsing of 
> > > > > each component off to should do that.
> > > > > 
> > > > > We unfortunately cannot move the entirety of this parsing section to 
> > > > > do that, since 'identifier'/'type', and 'Function' argument types end 
> > > > > up needing special handling, but it would be nice if our 'expr' types 
> > > > > would all be able to do this, and I suggested earlier that we could 
> > > > > start with the `VariadicExprArgument` to make this an easier patch 
> > > > > that didn't have to deal with the issues that come with that.
> > > > > 
> > > > > That said, it would be nice if we could get CLOSE to that.
> > > > Sorry. I am still not sure I am following. Do you mean the consumption 
> > > > of pack expressions should be moved into the expression parsing so 
> > > > other places (not just attributes) would accept it? If so, I am not 
> > > > opposed to the idea though I have no idea which expressions these would 
> > > > be nor the full implications of such a change. For these we at least 
> > > > have the isolation of having to explicitly support the packs in the 
> > > > given attributes.
> > > I'm saying the other expression parsers should ALREADY properly handle 
> > > packs, and we should just use those.
> > I think I see what you mean. There is the `Parser::ParseExpressionList` 
> > which may fit the bill, though it seems to also accept initializer lists. 
> > But since it is an expression the attributes can decide if it is a valid 
> > expression for them, so maybe that is not a problem?
> Hmm... I don't have a good idea of that, Aaron has better visibility into 
> that sort of thing.
> 
> Part of the advantage to doing it the way I suggest is that it ends up being 
> a single call (that is, the parser should parse the commas!), so we should 
> only have to consume the closing paren.
I think we want something more like `Parser::ParseSimpleExpressionList()` 
but... that doesn't handle packs, so those appear to be handled special: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseExpr.cpp#L3059.
 But one of these two functions seems to be along the correct lines of what we 
want (with modification).

FWIW, I like the sound of Erich's approach better than what's in the current 
patch. I'd rather not try to specially handle the ellipsis so much as 
recognizing when we expect an arbitrary list of variadic expression arguments 
as the last "parameter" for an attribute.


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

https://reviews.llvm.org/D114439

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

[clang] da920c3 - [clang][deps] NFC: Move entry initialization into member functions

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-12-15T16:39:29+01:00
New Revision: da920c3bcc794ee86c48f0e77eab4db068f17b5d

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

LOG: [clang][deps] NFC: Move entry initialization into member functions

This is a prep-patch for making `CachedFileSystemEntry` initialization more 
lazy.

Added: 


Modified: 

clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 4b286df8cd8b..4e770bb2e789 100644
--- 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -36,20 +36,17 @@ class CachedFileSystemEntry {
   /// Creates an uninitialized entry.
   CachedFileSystemEntry() : MaybeStat(llvm::vfs::Status()) {}
 
-  CachedFileSystemEntry(std::error_code Error) : MaybeStat(std::move(Error)) {}
+  /// Initialize the cached file system entry.
+  void init(llvm::ErrorOr &&MaybeStatus, StringRef Filename,
+llvm::vfs::FileSystem &FS, bool ShouldMinimize = true);
 
-  /// Create an entry that represents an opened source file with minimized or
-  /// original contents.
+  /// Initialize the entry as file with minimized or original contents.
   ///
   /// The filesystem opens the file even for `stat` calls open to avoid the
   /// issues with stat + open of minimized files that might lead to a
   /// mismatching size of the file.
-  static CachedFileSystemEntry createFileEntry(StringRef Filename,
-   llvm::vfs::FileSystem &FS,
-   bool Minimize = true);
-
-  /// Create an entry that represents a directory on the filesystem.
-  static CachedFileSystemEntry createDirectoryEntry(llvm::vfs::Status &&Stat);
+  llvm::ErrorOr
+  initFile(StringRef Filename, llvm::vfs::FileSystem &FS, bool Minimize = 
true);
 
   /// \returns True if the entry is initialized.
   bool isInitialized() const {
@@ -203,11 +200,6 @@ class DependencyScanningWorkerFilesystem : public 
llvm::vfs::ProxyFileSystem {
   llvm::ErrorOr
   getOrCreateFileSystemEntry(const StringRef Filename);
 
-  /// Create a cached file system entry based on the initial status result.
-  CachedFileSystemEntry
-  createFileSystemEntry(llvm::ErrorOr &&MaybeStatus,
-StringRef Filename, bool ShouldMinimize);
-
   /// The global cache shared between worker threads.
   DependencyScanningFilesystemSharedCache &SharedCache;
   /// The local cache is used by the worker thread to cache file system queries

diff  --git 
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 32e8acbcfc3d..08cf05de9e03 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -16,20 +16,21 @@ using namespace clang;
 using namespace tooling;
 using namespace dependencies;
 
-CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
-StringRef Filename, llvm::vfs::FileSystem &FS, bool Minimize) {
+llvm::ErrorOr
+CachedFileSystemEntry::initFile(StringRef Filename, llvm::vfs::FileSystem &FS,
+bool Minimize) {
   // Load the file and its content from the file system.
   llvm::ErrorOr> MaybeFile =
   FS.openFileForRead(Filename);
   if (!MaybeFile)
 return MaybeFile.getError();
+
   llvm::ErrorOr Stat = (*MaybeFile)->status();
   if (!Stat)
 return Stat.getError();
 
-  llvm::vfs::File &F = **MaybeFile;
   llvm::ErrorOr> MaybeBuffer =
-  F.getBuffer(Stat->getName());
+  (*MaybeFile)->getBuffer(Stat->getName());
   if (!MaybeBuffer)
 return MaybeBuffer.getError();
 
@@ -42,22 +43,18 @@ CachedFileSystemEntry 
CachedFileSystemEntry::createFileEntry(
 // Use the original file unless requested otherwise, or
 // if the minimization failed.
 // FIXME: Propage the diagnostic if desired by the client.
-CachedFileSystemEntry Result;
-Result.MaybeStat = std::move(*Stat);
-Result.Contents = std::move(*MaybeBuffer);
-return Result;
+Contents = std::move(*MaybeBuffer);
+return Stat;
   }
 
-  CachedFileSystemEntry Result;
-  size_t Size = MinimizedFileContents.size();
-  Result.MaybeStat = llvm::vfs::Status(Stat->getName(), Stat->getUniqueID(),
-   Stat->getLastModificationTime(),
-   Stat-