[PATCH] D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added reviewers: nhaehnle, t.p.northover, dblaikie.
Herald added a reviewer: bollu.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.

Removes deprecated overloads of LoadInst constructor, CallInst::Create, 
InvokeInst::Create, IRBuilder::CreateCall, IRBuilder::CreateInvoke. (Leaving 
around deprecated IRBuilder::CreateLoad for now.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76269

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Instructions.h
  llvm/tools/llvm-stress/llvm-stress.cpp
  polly/include/polly/CodeGen/LoopGenerators.h
  polly/include/polly/CodeGen/LoopGeneratorsGOMP.h
  polly/include/polly/CodeGen/LoopGeneratorsKMP.h
  polly/lib/CodeGen/LoopGeneratorsGOMP.cpp
  polly/lib/CodeGen/LoopGeneratorsKMP.cpp
  polly/lib/Transform/RewriteByReferenceParameters.cpp

Index: polly/lib/Transform/RewriteByReferenceParameters.cpp
===
--- polly/lib/Transform/RewriteByReferenceParameters.cpp
+++ polly/lib/Transform/RewriteByReferenceParameters.cpp
@@ -66,11 +66,12 @@
 std::string InstName = Alloca->getName().str();
 
 auto NewAlloca =
-new AllocaInst(Alloca->getType()->getElementType(), 0,
+new AllocaInst(Alloca->getAllocatedType(), 0,
"polly_byref_alloca_" + InstName, &*Entry->begin());
 
 auto *LoadedVal =
-new LoadInst(Alloca, "polly_byref_load_" + InstName, );
+new LoadInst(Alloca->getAllocatedType(), Alloca,
+ "polly_byref_load_" + InstName, );
 
 new StoreInst(LoadedVal, NewAlloca, );
 auto *NewBitCast = new BitCastInst(NewAlloca, BitCast->getType(),
Index: polly/lib/CodeGen/LoopGeneratorsKMP.cpp
===
--- polly/lib/CodeGen/LoopGeneratorsKMP.cpp
+++ polly/lib/CodeGen/LoopGeneratorsKMP.cpp
@@ -59,7 +59,7 @@
   Builder.CreateCall(F, Args);
 }
 
-void ParallelLoopGeneratorKMP::deployParallelExecution(Value *SubFn,
+void ParallelLoopGeneratorKMP::deployParallelExecution(Function *SubFn,
Value *SubFnParam,
Value *LB, Value *UB,
Value *Stride) {
Index: polly/lib/CodeGen/LoopGeneratorsGOMP.cpp
===
--- polly/lib/CodeGen/LoopGeneratorsGOMP.cpp
+++ polly/lib/CodeGen/LoopGeneratorsGOMP.cpp
@@ -47,7 +47,7 @@
   Builder.CreateCall(F, Args);
 }
 
-void ParallelLoopGeneratorGOMP::deployParallelExecution(Value *SubFn,
+void ParallelLoopGeneratorGOMP::deployParallelExecution(Function *SubFn,
 Value *SubFnParam,
 Value *LB, Value *UB,
 Value *Stride) {
Index: polly/include/polly/CodeGen/LoopGeneratorsKMP.h
===
--- polly/include/polly/CodeGen/LoopGeneratorsKMP.h
+++ polly/include/polly/CodeGen/LoopGeneratorsKMP.h
@@ -69,7 +69,7 @@
   void createCallSpawnThreads(Value *SubFn, Value *SubFnParam, Value *LB,
   Value *UB, Value *Stride);
 
-  void deployParallelExecution(Value *SubFn, Value *SubFnParam, Value *LB,
+  void deployParallelExecution(Function *SubFn, Value *SubFnParam, Value *LB,
Value *UB, Value *Stride) override;
 
   virtual Function *prepareSubFnDefinition(Function *F) const override;
Index: polly/include/polly/CodeGen/LoopGeneratorsGOMP.h
===
--- polly/include/polly/CodeGen/LoopGeneratorsGOMP.h
+++ polly/include/polly/CodeGen/LoopGeneratorsGOMP.h
@@ -45,7 +45,7 @@
   void createCallSpawnThreads(Value *SubFn, Value *SubFnParam, Value *LB,
   Value *UB, Value *Stride);
 
-  void deployParallelExecution(Value *SubFn, Value *SubFnParam, Value *LB,
+  void deployParallelExecution(Function *SubFn, Value *SubFnParam, Value *LB,
Value *UB, Value *Stride) override;
 
   virtual Function *prepareSubFnDefinition(Function *F) const override;
Index: polly/include/polly/CodeGen/LoopGenerators.h
===
--- polly/include/polly/CodeGen/LoopGenerators.h
+++ polly/include/polly/CodeGen/LoopGenerators.h
@@ -188,7 +188,7 @@
   /// @param LB The lower bound for the loop we parallelize.
   /// @param UB The upper bound for the loop we parallelize.
   /// @param Stride The stride of the loop we parallelize.
-  virtual void deployParallelExecution(Value *SubFn, Value *SubFnParam,
+  virtual void deployParallelExecution(Function *SubFn, Value *SubFnParam,
  

[PATCH] D60748: Fix i386 struct and union parameter alignment

[LiuChen via Phabricator via cfe-commits]

LiuChen3 added a comment.

In D60748#1924794 , @rjmccall wrote:

> Oh, I see you just updated your patch months ago without ever mentioning that 
> it was ready for review.
>
> It sounds to me like GCC retroactively added a switch specifying which 
> version of the ABI to follow on this point, somewhat confusingly called 
> `-malign-data`.  That's probably the right move here for us, too, especially 
> since FreeBSD says they'd like to use it.  That also means the condition of 
> when to use your new logic will have to change; basically, we need a 
> CodeGenOption for this that will default to the old ABI, and the driver will 
> pass down a different default on Linux.


Thanks for review.
 `-malign-data` is another topic. Just like what I said above, at least 
`-malign-data` will not affect the calling convention of struct and union. I 
agree with you that adding an option to control this new logi. I'll working on 
it.


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

https://reviews.llvm.org/D60748



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

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added a comment.

In D75917#1925700 , @JonChesterfield 
wrote:

>




> I think I follow. The syncscope takes a string, therefore the builtin that 
> maps onto fence should also take a string for that parameter? That's fine by 
> me. Will help if a new non-opencl syncscope is introduced later.

Right. To be precise, it is a target-specific string, and should not be 
processed as if it was an OpenCL scope. The builtin should allow anything that 
the IR fence would allow in a .ll file created for the specified target.


Repository:
  rC Clang

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

https://reviews.llvm.org/D75917



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

[PATCH] D76262: [NFC] Add UsedDeclVisitor

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


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

https://reviews.llvm.org/D76262



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

[PATCH] D76262: [NFC] Add UsedDeclVisitor

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Sema/UsedDeclVisitor.h:43
+  asImpl().visitUsedDecl(E->getMemberLoc(), D);
+}
+  }

rjmccall wrote:
> You need to recurse on the base expression here.  (And that's a good test 
> case for your own patch!)
fixed


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

https://reviews.llvm.org/D76262



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

[PATCH] D75169: [ARM] Enforcing calling convention for half-precision FP arguments and returns for big-endian AArch32

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Ugh.  I'd hate to introduce yet another weird little tweak to `ABIArgInfo` 
that's used on exactly one target.  For 16-bit composite types, we seem to 
coerce to a type like `[1 x i32]`; would that be okay here?

You don't have a test that checks that you get the IR you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75169



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

[PATCH] D76262: [NFC] Add UsedDeclVisitor

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 250688.
yaxunl marked 2 inline comments as done.
yaxunl added a comment.

visit base expr of member expr.


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

https://reviews.llvm.org/D76262

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/UsedDeclVisitor.h

Index: clang/lib/Sema/UsedDeclVisitor.h
===
--- /dev/null
+++ clang/lib/Sema/UsedDeclVisitor.h
@@ -0,0 +1,90 @@
+//===- UsedDeclVisitor.h - ODR-used declarations visitor *- C++ -*-===//
+//
+// 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 UsedDeclVisitor, a CRTP class which visits all the
+//  declarations that are ODR-used by an expression or statement.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+#define LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+
+#include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/Sema/SemaInternal.h"
+
+namespace clang {
+template 
+class UsedDeclVisitor : public EvaluatedExprVisitor {
+protected:
+  Sema 
+
+public:
+  typedef EvaluatedExprVisitor Inherited;
+
+  UsedDeclVisitor(Sema ) : Inherited(S.Context), S(S) {}
+
+  Derived () { return *static_cast(this); }
+
+  void VisitDeclRefExpr(DeclRefExpr *E) {
+auto *D = E->getDecl();
+if (isa(D) || isa(D)) {
+  asImpl().visitUsedDecl(E->getLocation(), D);
+}
+  }
+
+  void VisitMemberExpr(MemberExpr *E) {
+auto *D = E->getMemberDecl();
+if (isa(D) || isa(D)) {
+  asImpl().visitUsedDecl(E->getMemberLoc(), D);
+}
+asImpl().Visit(E->getBase());
+  }
+
+  void VisitCapturedStmt(CapturedStmt *Node) {
+asImpl().visitUsedDecl(Node->getBeginLoc(), Node->getCapturedDecl());
+Inherited::VisitCapturedStmt(Node);
+  }
+
+  void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+asImpl().visitUsedDecl(
+E->getBeginLoc(),
+const_cast(E->getTemporary()->getDestructor()));
+asImpl().Visit(E->getSubExpr());
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (E->getOperatorNew())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorNew());
+if (E->getOperatorDelete())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+Inherited::VisitCXXNewExpr(E);
+  }
+
+  void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+if (E->getOperatorDelete())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+QualType Destroyed = S.Context.getBaseElementType(E->getDestroyedType());
+if (const RecordType *DestroyedRec = Destroyed->getAs()) {
+  CXXRecordDecl *Record = cast(DestroyedRec->getDecl());
+  asImpl().visitUsedDecl(E->getBeginLoc(), S.LookupDestructor(Record));
+}
+
+Inherited::VisitCXXDeleteExpr(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+asImpl().visitUsedDecl(E->getBeginLoc(), E->getConstructor());
+Inherited::VisitCXXConstructExpr(E);
+  }
+
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+asImpl().Visit(E->getExpr());
+  }
+};
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "TreeTransform.h"
+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
@@ -17373,71 +17374,33 @@
 }
 
 namespace {
-  /// Helper class that marks all of the declarations referenced by
-  /// potentially-evaluated subexpressions as "referenced".
-  class EvaluatedExprMarker : public EvaluatedExprVisitor {
-Sema 
-bool SkipLocalVariables;
-
-  public:
-typedef EvaluatedExprVisitor Inherited;
-
-EvaluatedExprMarker(Sema , bool SkipLocalVariables)
-  : Inherited(S.Context), S(S), SkipLocalVariables(SkipLocalVariables) { }
-
-void VisitDeclRefExpr(DeclRefExpr *E) {
-  // If we were asked not to visit local variables, don't.
-  if (SkipLocalVariables) {
-if (VarDecl *VD = dyn_cast(E->getDecl()))
-  if (VD->hasLocalStorage())
-return;
-  }
-
-  S.MarkDeclRefReferenced(E);
-}
-
-void VisitMemberExpr(MemberExpr *E) {
-  S.MarkMemberReferenced(E);
-  Inherited::VisitMemberExpr(E);
-}
-
-void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
-  S.MarkFunctionReferenced(
-  E->getBeginLoc(),
-  const_cast(E->getTemporary()->getDestructor()));
-  Visit(E->getSubExpr());
-

[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:677
+  E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+

Why only when the l-value is volatile?



Comment at: clang/lib/Sema/SemaExpr.cpp:5733
+  QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+

You should do this in `MaybeBindToTemporary`, and then you probably don't need 
it in most of these other places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66094



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

[PATCH] D76211: OpenMP Metadirective with user defined condition

[Alok Mishra via Phabricator via cfe-commits]

alokmishra.besu updated this revision to Diff 250684.
alokmishra.besu added a comment.
Herald added a project: OpenMP.
Herald added a subscriber: openmp-commits.

Adding test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76211

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/runtime/test/metadirective/metadirective_user.c

Index: openmp/runtime/test/metadirective/metadirective_user.c
===
--- /dev/null
+++ openmp/runtime/test/metadirective/metadirective_user.c
@@ -0,0 +1,30 @@
+#include 
+
+void test_default(int N) {
+#pragma omp metadirective when(user = {condition(N < 10)}  \
+   : parallel) default(parallel for)
+  for (int i = 0; i < N; i++) {
+printf("Printing from test_default : %d\n", i);
+  }
+}
+
+void test_when(int N) {
+#pragma omp metadirective when(user = {condition(N >= 10)} : parallel for)
+  for (int i = 0; i < N; i++) {
+printf("Printing from test_when: %d\n", i);
+  }
+}
+
+int main() {
+  int N = 15;
+  test_when(N);
+  test_default(N);
+
+  printf("\n");
+
+  N = 5;
+  test_when(N);
+  test_default(N);
+
+  return 0;
+}
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -92,6 +92,7 @@
 __OMP_DIRECTIVE_EXT(parallel_master_taskloop_simd,
 "parallel master taskloop simd")
 __OMP_DIRECTIVE(depobj)
+__OMP_DIRECTIVE(metadirective)
 
 // Has to be the last because Clang implicitly expects it to be.
 __OMP_DIRECTIVE(unknown)
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -578,6 +578,9 @@
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
+  case Stmt::OMPMetaDirectiveClass:
+K = CXCursor_OMPMetaDirective;
+break;
   case Stmt::OMPParallelDirectiveClass:
 K = CXCursor_OMPParallelDirective;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2207,6 +2207,10 @@
   Visitor->AddStmt(C->getNumForLoops());
 }
 
+void OMPClauseEnqueue::VisitOMPWhenClause(const OMPWhenClause *C) {
+  Visitor->AddStmt(C->getExpr());
+}
+
 void OMPClauseEnqueue::VisitOMPDefaultClause(const OMPDefaultClause *C) { }
 
 void OMPClauseEnqueue::VisitOMPProcBindClause(const OMPProcBindClause *C) { }
@@ -5475,6 +5479,8 @@
 return cxstring::createRef("CXXAccessSpecifier");
   case CXCursor_ModuleImportDecl:
 return cxstring::createRef("ModuleImport");
+  case CXCursor_OMPMetaDirective:
+return cxstring::createRef("OMPMetaDirective");
   case CXCursor_OMPParallelDirective:
 return cxstring::createRef("OMPParallelDirective");
   case CXCursor_OMPSimdDirective:
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1291,6 +1291,7 @@
 case Stmt::OMPTargetTeamsDistributeParallelForDirectiveClass:
 case Stmt::OMPTargetTeamsDistributeParallelForSimdDirectiveClass:
 case Stmt::OMPTargetTeamsDistributeSimdDirectiveClass:
+case Stmt::OMPMetaDirectiveClass:
 case Stmt::CapturedStmtClass: {
   const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState());
   Engine.addAbortedBlock(node, currBldrCtx->getBlock());
Index: clang/lib/Serialization/ASTWriterStmt.cpp

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: manmanren.
rjmccall added a comment.

This might also be interesting to @manmanren.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D76264: [ObjC generics] Fix missing protocols on type parameter for unparameterized generics.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: erik.pilkington, ahatanak.
Herald added subscribers: ributzka, jfb, dexonsmith, jkorous.
Herald added a project: clang.

When a type parameter is used with some protocols, we should keep these
protocols both when the generic type is parameterized and not
parameterized.

rdar://problem/58773533


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76264

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaObjC/parameterized_classes_subst.m

Index: clang/test/SemaObjC/parameterized_classes_subst.m
===
--- clang/test/SemaObjC/parameterized_classes_subst.m
+++ clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,34 @@
 - (void)mapUsingBlock2:(id)block { // expected-warning{{conflicting parameter types in implementation}}
 }
 @end
+
+// --
+// Protocols on type parameter in unspecialized context.
+// --
+@protocol ContainerItem
+@property (nonatomic) int protocolProp;
+@end
+
+@interface TestUnbounded : NSObject
+@property (nonatomic, retain) T element;
+@end
+@implementation TestUnbounded
+- (void)testProtocolOnTypeParameter {
+  (void)[self element].protocolProp;
+}
+@end
+
+@interface TestBounded> : NSObject
+@property (nonatomic, retain) T element;
+@end
+@implementation TestBounded
+- (void)testProtocolOnTypeParameter {
+  (void)[self element].protocolProp;
+}
+@end
+
+void accessNonParameterizedGenerics(
+TestUnbounded *protocolOnProperty, TestBounded *protocolOnTypeBound) {
+  (void)[protocolOnProperty element].protocolProp;
+  (void)[protocolOnTypeBound element].protocolProp;
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -1195,51 +1195,53 @@
   QualType VisitObjCTypeParamType(const ObjCTypeParamType *OTPTy) {
 // Replace an Objective-C type parameter reference with the corresponding
 // type argument.
+QualType newType;
 ObjCTypeParamDecl *typeParam = OTPTy->getDecl();
 // If we have type arguments, use them.
 if (!TypeArgs.empty()) {
-  QualType argType = TypeArgs[typeParam->getIndex()];
-  if (OTPTy->qual_empty())
-return argType;
-
-  // Apply protocol lists if exists.
-  bool hasError;
-  SmallVector protocolsVec;
-  protocolsVec.append(OTPTy->qual_begin(), OTPTy->qual_end());
-  ArrayRef protocolsToApply = protocolsVec;
-  return Ctx.applyObjCProtocolQualifiers(
-  argType, protocolsToApply, hasError, true/*allowOnPointerType*/);
+  newType = TypeArgs[typeParam->getIndex()];
+} else {
+  switch (SubstContext) {
+  case ObjCSubstitutionContext::Ordinary:
+  case ObjCSubstitutionContext::Parameter:
+  case ObjCSubstitutionContext::Superclass:
+// Substitute the bound.
+newType = typeParam->getUnderlyingType();
+break;
+
+  case ObjCSubstitutionContext::Result:
+  case ObjCSubstitutionContext::Property: {
+// Substitute the __kindof form of the underlying type.
+const auto *objPtr =
+typeParam->getUnderlyingType()->castAs();
+
+// __kindof types, id, and Class don't need an additional
+// __kindof.
+if (objPtr->isKindOfType() || objPtr->isObjCIdOrClassType()) {
+  newType = typeParam->getUnderlyingType();
+} else {
+  // Add __kindof.
+  const auto *obj = objPtr->getObjectType();
+  QualType resultTy = Ctx.getObjCObjectType(obj->getBaseType(),
+obj->getTypeArgsAsWritten(),
+obj->getProtocols(),
+/*isKindOf=*/true);
+
+  // Rebuild object pointer type.
+  newType = Ctx.getObjCObjectPointerType(resultTy);
+}
+break;
+  }
+  }
 }
 
-switch (SubstContext) {
-case ObjCSubstitutionContext::Ordinary:
-case ObjCSubstitutionContext::Parameter:
-case ObjCSubstitutionContext::Superclass:
-  // Substitute the bound.
-  return typeParam->getUnderlyingType();
-
-case ObjCSubstitutionContext::Result:
-case ObjCSubstitutionContext::Property: {
-  // Substitute the __kindof form of the underlying type.
-  const auto *objPtr =
-  typeParam->getUnderlyingType()->castAs();
-
-  // __kindof types, id, and Class don't need an additional
-  // __kindof.
-  if (objPtr->isKindOfType() || objPtr->isObjCIdOrClassType())
-return typeParam->getUnderlyingType();
-
-  // Add __kindof.
-  const auto *obj = objPtr->getObjectType();
-  QualType resultTy = Ctx.getObjCObjectType(
-  obj->getBaseType(), obj->getTypeArgsAsWritten(), 

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

bader wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > yaxunl wrote:
> > > > rjmccall wrote:
> > > > > erichkeane wrote:
> > > > > > rjmccall wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > Note that when recommitting this (if you choose to), this needs 
> > > > > > > > to also handle NamespaceDecl.  We're a downstream and 
> > > > > > > > discovered that this doesn't properly handle functions or 
> > > > > > > > records handled in a namespace.
> > > > > > > > 
> > > > > > > > It can be implemented identically to TranslationUnitDecl.
> > > > > > > Wait, what?  We shouldn't be doing this for TranslationUnitDecl 
> > > > > > > either.   I don't even know how we're "using" a 
> > > > > > > TranslationUnitDecl, but neither this case not the case for 
> > > > > > > `NamespaceDecl` should be recursively using every declaration 
> > > > > > > declared inside it.  If there's a declaration in a namespace 
> > > > > > > that's being used, it should be getting visited as part of the 
> > > > > > > actual use of it.
> > > > > > > 
> > > > > > > The logic for `RecordDecl` has the same problem.  
> > > > > > Despite the name, this seems to be more of a home-written ast 
> > > > > > walking class.  The entry point is the 'translation unit' which 
> > > > > > seems to walk through everything in an attempt to find all the 
> > > > > > functions (including those that are 'marked' as used by an 
> > > > > > attribute).
> > > > > > 
> > > > > > You'll see the FunctionDecl section makes this assumption as well 
> > > > > > (not necessarily that we got to a function via a call). IMO, this 
> > > > > > approach is strange, and we should register entry points in some 
> > > > > > manner (functions marked as emitted to the device in some fashion), 
> > > > > > then just follow its call-graph (via the clang::CallGraph?) to emit 
> > > > > > all of these functions.
> > > > > > 
> > > > > > It seemed really odd to see this approach here, but it seemed well 
> > > > > > reviewed by the time I noticed it (via a downstream bug) so I 
> > > > > > figured I'd lost my chance to disagree with the approach.
> > > > > > 
> > > > > > 
> > > > > Sure, but `visitUsedDecl` isn't the right place to be entering the 
> > > > > walk.  `visitUsedDecl` is supposed to be the *callback* from the 
> > > > > walk.  If they need to walk all the global declarations to find 
> > > > > kernels instead of tracking the kernels as they're encountered (which 
> > > > > would be a *much* better approach), it should be done as a separate 
> > > > > function.
> > > > > 
> > > > > I just missed this in the review.
> > > > The deferred diagnostics could be initiated by non-kernel functions or 
> > > > even host functions.
> > > > 
> > > > Let's consider a device code library where no kernels are defined. A 
> > > > device function is emitted, which calls a host device function which 
> > > > has a deferred diagnostic. All device functions that are emitted need 
> > > > to be checked.
> > > > 
> > > > Same with host functions that are emitted, which may call a host device 
> > > > function which has deferred diagnostic.
> > > > 
> > > > Also not just function calls need to be checked. A function address may 
> > > > be taken then called through function pointer. Therefore any reference 
> > > > to a function needs to be followed.
> > > > 
> > > > In the case of OpenMP, the initialization of a global function pointer 
> > > > which refers a function may trigger a deferred diangostic. There are 
> > > > tests for that.
> > > Right, I get that emitting deferred diagnostics for a declaration D needs 
> > > to trigger any deferred diagnostics in declarations used by D, 
> > > recursively.  You essentially have a graph of lazily-emitted declarations 
> > > (which may or may not have deferred diagnostics) and a number of 
> > > eagerly-emitted "root" declarations with use-edges leading into that 
> > > graph.  Any declaration that's reachable from a root will need to be 
> > > emitted and so needs to have any deferred diagnostics emitted as well.  
> > > My question is why you're finding these roots with a retroactive walk of 
> > > the entire translation unit instead of either building a list of roots as 
> > > you go or (better yet) building a list of lazily-emitted declarations 
> > > that are used by those roots.  You can unambiguously identify at the 
> > > point of declaration whether an entity will be eagerly or lazily emitted, 
> > > right?  If you just store those initial edges into the lazily-emitted 
> > > declarations graph and then initiate the recursive walk from them at the 
> > > end of the translation unit, you'll only end up walking declarations that 
> > > are actually relevant to your compilation, so you'll have much 

[PATCH] D76262: [NFC] Add UsedDeclVisitor

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, looks good except for one oversight.




Comment at: clang/lib/Sema/UsedDeclVisitor.h:43
+  asImpl().visitUsedDecl(E->getMemberLoc(), D);
+}
+  }

You need to recurse on the base expression here.  (And that's a good test case 
for your own patch!)


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

https://reviews.llvm.org/D76262



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

[PATCH] D75715: Switch TypeSystemClang over to CreateDeserialized() (NFC)

[Adrian Prantl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG90a2fbdb044b: Switch to TypeSystemClang over to 
CreateDeserialized() (NFC) (authored by aprantl).
Herald added projects: clang, LLDB.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D75715?vs=248624=250670#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75715

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclTemplate.h
  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
@@ -1207,10 +1207,11 @@
   // complete definition just in case.
 
   bool has_name = !name.empty();
-
-  CXXRecordDecl *decl = CXXRecordDecl::Create(
-  ast, (TagDecl::TagKind)kind, decl_ctx, SourceLocation(), SourceLocation(),
-  has_name ? (name) : nullptr);
+  CXXRecordDecl *decl = CXXRecordDecl::CreateDeserialized(ast, 0);
+  decl->setTagKind(static_cast(kind));
+  decl->setDeclContext(decl_ctx);
+  if (has_name)
+decl->setDeclName((name));
 
   if (!has_name) {
 // In C++ a lambda is also represented as an unnamed class. This is
@@ -1331,9 +1332,12 @@
 
   TemplateParameterList *template_param_list = CreateTemplateParameterList(
   ast, template_param_infos, template_param_decls);
-  FunctionTemplateDecl *func_tmpl_decl = FunctionTemplateDecl::Create(
-  ast, decl_ctx, func_decl->getLocation(), func_decl->getDeclName(),
-  template_param_list, func_decl);
+  FunctionTemplateDecl *func_tmpl_decl =
+  FunctionTemplateDecl::CreateDeserialized(ast, 0);
+  func_tmpl_decl->setDeclContext(decl_ctx);
+  func_tmpl_decl->setLocation(func_decl->getLocation());
+  func_tmpl_decl->setDeclName(func_decl->getDeclName());
+  func_tmpl_decl->init(func_decl, template_param_list);
 
   for (size_t i = 0, template_param_decl_count = template_param_decls.size();
i < template_param_decl_count; ++i) {
@@ -1384,11 +1388,11 @@
   TemplateParameterList *template_param_list = CreateTemplateParameterList(
   ast, template_param_infos, template_param_decls);
 
-  CXXRecordDecl *template_cxx_decl = CXXRecordDecl::Create(
-  ast, (TagDecl::TagKind)kind,
-  decl_ctx, // What decl context do we use here? TU? The actual decl
-// context?
-  SourceLocation(), SourceLocation(), _info);
+  CXXRecordDecl *template_cxx_decl = CXXRecordDecl::CreateDeserialized(ast, 0);
+  template_cxx_decl->setTagKind(static_cast(kind));
+  // What decl context do we use here? TU? The actual decl context?
+  template_cxx_decl->setDeclContext(decl_ctx);
+  template_cxx_decl->setDeclName(decl_name);
 
   for (size_t i = 0, template_param_decl_count = template_param_decls.size();
i < template_param_decl_count; ++i) {
@@ -1400,11 +1404,11 @@
   // template_cxx_decl->startDefinition();
   // template_cxx_decl->completeDefinition();
 
-  class_template_decl = ClassTemplateDecl::Create(
-  ast,
-  decl_ctx, // What decl context do we use here? TU? The actual decl
-// context?
-  SourceLocation(), decl_name, template_param_list, template_cxx_decl);
+  class_template_decl = ClassTemplateDecl::CreateDeserialized(ast, 0);
+  // What decl context do we use here? TU? The actual decl context?
+  class_template_decl->setDeclContext(decl_ctx);
+  class_template_decl->setDeclName(decl_name);
+  class_template_decl->init(template_cxx_decl, template_param_list);
   template_cxx_decl->setDescribedClassTemplate(class_template_decl);
 
   if (class_template_decl) {
@@ -1458,9 +1462,16 @@
 ast, template_param_infos.packed_args->args);
   }
   ClassTemplateSpecializationDecl *class_template_specialization_decl =
-  ClassTemplateSpecializationDecl::Create(
-  ast, (TagDecl::TagKind)kind, decl_ctx, SourceLocation(),
-  SourceLocation(), class_template_decl, args, nullptr);
+  ClassTemplateSpecializationDecl::CreateDeserialized(ast, 0);
+  class_template_specialization_decl->setTagKind(
+  static_cast(kind));
+  class_template_specialization_decl->setDeclContext(decl_ctx);
+  class_template_specialization_decl->setInstantiationOf(class_template_decl);
+  class_template_specialization_decl->setTemplateArgs(
+  TemplateArgumentList::CreateCopy(ast, args));
+  ast.getTypeDeclType(class_template_specialization_decl, nullptr);
+  class_template_specialization_decl->setDeclName(
+  class_template_decl->getDeclName());
 
   class_template_specialization_decl->setSpecializationKind(
   TSK_ExplicitSpecialization);
@@ -1589,11 +1600,11 @@
   if (decl_ctx == nullptr)
 decl_ctx = ast.getTranslationUnitDecl();
 
-  ObjCInterfaceDecl *decl = ObjCInterfaceDecl::Create(
- 

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 250666.
yaxunl marked 6 inline comments as done.
yaxunl added a comment.

revised by John's comments.


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

https://reviews.llvm.org/D70172

Files:
  clang/include/clang/Sema/ExternalSemaSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
  clang/test/SemaCUDA/bad-calls-on-same-line.cu
  clang/test/SemaCUDA/call-device-fn-from-host.cu
  clang/test/SemaCUDA/call-host-fn-from-device.cu
  clang/test/SemaCUDA/openmp-target.cu
  clang/test/SemaCUDA/trace-through-global.cu

Index: clang/test/SemaCUDA/trace-through-global.cu
===
--- clang/test/SemaCUDA/trace-through-global.cu
+++ clang/test/SemaCUDA/trace-through-global.cu
@@ -38,7 +38,7 @@
   // Notice that these two diagnostics are different: Because the call to hd1
   // is not dependent on T, the call to hd1 comes from 'launch_kernel', while
   // the call to hd3, being dependent, comes from 'launch_kernel'.
-  hd1(); // expected-note {{called by 'launch_kernel'}}
+  hd1(); // expected-note {{called by 'launch_kernel'}}
   hd3(T()); // expected-note {{called by 'launch_kernel'}}
 }
 
Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -16,9 +16,9 @@
 void bazzz() {bazz();}
 #pragma omp declare target to(bazzz) device_type(nohost)
 void any() {bazz();} // expected-error {{function with 'device_type(nohost)' is not available on host}}
-void host1() {bazz();}
+void host1() {bazz();} // expected-error {{function with 'device_type(nohost)' is not available on host}}
 #pragma omp declare target to(host1) device_type(host)
-void host2() {bazz();}
+void host2() {bazz();} // expected-error {{function with 'device_type(nohost)' is not available on host}}
 #pragma omp declare target to(host2)
 void device() {host1();}
 #pragma omp declare target to(device) device_type(nohost)
Index: clang/test/SemaCUDA/call-host-fn-from-device.cu
===
--- clang/test/SemaCUDA/call-host-fn-from-device.cu
+++ clang/test/SemaCUDA/call-host-fn-from-device.cu
@@ -56,14 +56,14 @@
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 __global__ void kernel() { hd2(); }
 
 __host__ __device__ void hd() { host_fn(); }
 // expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 
 template  __host__ __device__ void hd3() { host_fn(); }
-// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 __device__ void device_fn() { hd3(); }
 
 // No error because this is never instantiated.
Index: clang/test/SemaCUDA/call-device-fn-from-host.cu
===
--- clang/test/SemaCUDA/call-device-fn-from-host.cu
+++ clang/test/SemaCUDA/call-device-fn-from-host.cu
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - \
 // RUN:   -verify -verify-ignore-unexpected=note
 // RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - \
-// RUN:   -verify -verify-ignore-unexpected=note -fopenmp
+// RUN:   -verify=expected,omp -verify-ignore-unexpected=note -fopenmp
 
 // Note: This test won't work with -fsyntax-only, because some of these errors
 // are emitted during codegen.
@@ -39,7 +39,7 @@
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 2 {{reference to __device__ function 'device_fn' in __host__ __device__ function}}
+// expected-error@-1 {{reference to __device__ function 'device_fn' in __host__ __device__ function}}
 void host_fn() { hd2(); }
 
 __host__ __device__ void hd() { device_fn(); }
Index: clang/test/SemaCUDA/bad-calls-on-same-line.cu
===
--- clang/test/SemaCUDA/bad-calls-on-same-line.cu
+++ clang/test/SemaCUDA/bad-calls-on-same-line.cu
@@ -33,8 +33,8 @@
 
 void host_fn() {
   hd();
-  hd();  // expected-note {{function 

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D75917#1925617 , @sameerds wrote:

> Is it really? The scope argument of the IR fence is a target-specific string:
>  http://llvm.org/docs/LangRef.html#syncscope
>
> The change that I see here is assuming a numerical argument, and also 
> assuming that the numbers used must conform to the OpenCL enum. That would 
> certainly make the builtin quite different from the IR fence.


I think I follow. The syncscope takes a string, therefore the builtin that maps 
onto fence should also take a string for that parameter? That's fine by me. 
Will help if a new non-opencl syncscope is introduced later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D75917



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

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

rsmith wrote:
> efriedma wrote:
> > rsmith wrote:
> > > nickdesaulniers wrote:
> > > > efriedma wrote:
> > > > > You need to be more careful here; we can call ConstExprEmitter for 
> > > > > arbitrary expressions.
> > > > "Be more careful" how?
> > > Here are some specific cases in which you need to be more careful because 
> > > the code as-is would do the wrong thing:
> > > 
> > >  * when emitting a global's initializer in C++, where the value of the 
> > > object denoted by the DeclRefExpr could have changed between its 
> > > initialization and the expression we're currently emitting 
> > >  * when emitting anything other than a global initializer in C, where the 
> > > value of a global could have changed after its emission
> > >  * when emitting a reference to a non-global declaration in C (local 
> > > variables might change between initialization and use)
> > > 
> > > We would need to restrict this to the cases where the variable's value 
> > > cannot possibly have changed between initialization and use.
> > > 
> > > In C, that's (mostly) the case for a static storage variable referenced 
> > > from the initializer of a static storage variable, for a thread storage 
> > > variable referenced from the initializer of a static storage variable, or 
> > > for a thread storage variable referenced from the initializer of a thread 
> > > storage variable. Even then, this isn't strictly correct in the presence 
> > > of DSOs, but I think it should be correct if the two variables are 
> > > defined in the same translation unit.
> > > 
> > > In C++, that's (mostly) the case when the variable is `const` or 
> > > `constexpr` and has no mutable subobjects. (There's still the case where 
> > > the reference happens outside the lifetime of the object -- for the most 
> > > part we can handwave that away by saying it must be UB, but that's not 
> > > true in general in the period of construction and period of destruction.)
> > > 
> > > In both cases, the optimization is (arguably) still wrong if there are 
> > > any volatile subobjects.
> > And this is why I don't want to duplicate the logic. :)
> > 
> > I'd rather not make different assumptions for C and C++; instead, I'd 
> > prefer to just use the intersection that's safe in both.  I'm concerned 
> > that we could come up with weird results for mixed C and C++ code, 
> > otherwise.  Also, it's easier to define the C++ rules because we can base 
> > them on the constexpr rules in the standard.
> I agree we probably want the same outcome as D76169 provides, if either the 
> performance is acceptable or we can find a way to avoid the performance cost 
> for the cases we already accept. Perhaps we could get that outcome by 
> ensuring that we try the CGExprConstant fast-path (at least in C 
> compilations, maybe in general) before we consider the complete-but-slow 
> evaluation strategy in ExprConstant?
I like the idea of guarding constant evaluation with a "fast path" that doesn't 
actually compute the APValues in simple cases.  We can just implement the 
simple stuff, and fall back to the full logic for complicated stuff.

My one concern is that we could go to a bunch of effort to emit a variable on 
the fast path, then fall off the fast path later because "return a[0];" tries 
to constant-fold a big array "a".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096



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

[clang] 90a2fbd - Switch to TypeSystemClang over to CreateDeserialized() (NFC)

[Adrian Prantl via cfe-commits]

Author: Adrian Prantl
Date: 2020-03-16T18:11:36-07:00
New Revision: 90a2fbdb044bcf285646e79f3b096ccb196f6d02

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

LOG: Switch to TypeSystemClang over to CreateDeserialized() (NFC)

which is the more appropriate API for its use-case.

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

Added: 


Modified: 
clang/include/clang/AST/DeclCXX.h
clang/include/clang/AST/DeclTemplate.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 987738f73676..60d99f1a487f 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -2415,17 +2415,6 @@ class CXXConstructorDecl final
  : ExplicitSpecKind::ResolvedFalse);
   }
 
-  void setExplicitSpecifier(ExplicitSpecifier ES) {
-assert((!ES.getExpr() ||
-CXXConstructorDeclBits.HasTrailingExplicitSpecifier) &&
-   "cannot set this explicit specifier. no trail-allocated space for "
-   "explicit");
-if (ES.getExpr())
-  *getCanonicalDecl()->getTrailingObjects() = ES;
-else
-  CXXConstructorDeclBits.IsSimpleExplicit = ES.isExplicit();
-  }
-
   enum TraillingAllocKind {
 TAKInheritsConstructor = 1,
 TAKHasTailExplicit = 1 << 1,
@@ -2451,6 +2440,17 @@ class CXXConstructorDecl final
  InheritedConstructor Inherited = InheritedConstructor(),
  Expr *TrailingRequiresClause = nullptr);
 
+  void setExplicitSpecifier(ExplicitSpecifier ES) {
+assert((!ES.getExpr() ||
+CXXConstructorDeclBits.HasTrailingExplicitSpecifier) &&
+   "cannot set this explicit specifier. no trail-allocated space for "
+   "explicit");
+if (ES.getExpr())
+  *getCanonicalDecl()->getTrailingObjects() = ES;
+else
+  CXXConstructorDeclBits.IsSimpleExplicit = ES.isExplicit();
+  }
+
   ExplicitSpecifier getExplicitSpecifier() {
 return getCanonicalDecl()->getExplicitSpecifierInternal();
   }

diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index def1ab18706c..e9c4879b41e8 100755
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1927,6 +1927,10 @@ class ClassTemplateSpecializationDecl
 getTemplateSpecializationKind());
   }
 
+  void setSpecializedTemplate(ClassTemplateDecl *Specialized) {
+SpecializedTemplate = Specialized;
+  }
+
   void setSpecializationKind(TemplateSpecializationKind TSK) {
 SpecializationKind = TSK;
   }

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 42655ad6cc24..c278aef8949e 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1207,10 +1207,11 @@ CompilerType 
TypeSystemClang::CreateRecordType(DeclContext *decl_ctx,
   // complete definition just in case.
 
   bool has_name = !name.empty();
-
-  CXXRecordDecl *decl = CXXRecordDecl::Create(
-  ast, (TagDecl::TagKind)kind, decl_ctx, SourceLocation(), 
SourceLocation(),
-  has_name ? (name) : nullptr);
+  CXXRecordDecl *decl = CXXRecordDecl::CreateDeserialized(ast, 0);
+  decl->setTagKind(static_cast(kind));
+  decl->setDeclContext(decl_ctx);
+  if (has_name)
+decl->setDeclName((name));
 
   if (!has_name) {
 // In C++ a lambda is also represented as an unnamed class. This is
@@ -1331,9 +1332,12 @@ clang::FunctionTemplateDecl 
*TypeSystemClang::CreateFunctionTemplateDecl(
 
   TemplateParameterList *template_param_list = CreateTemplateParameterList(
   ast, template_param_infos, template_param_decls);
-  FunctionTemplateDecl *func_tmpl_decl = FunctionTemplateDecl::Create(
-  ast, decl_ctx, func_decl->getLocation(), func_decl->getDeclName(),
-  template_param_list, func_decl);
+  FunctionTemplateDecl *func_tmpl_decl =
+  FunctionTemplateDecl::CreateDeserialized(ast, 0);
+  func_tmpl_decl->setDeclContext(decl_ctx);
+  func_tmpl_decl->setLocation(func_decl->getLocation());
+  func_tmpl_decl->setDeclName(func_decl->getDeclName());
+  func_tmpl_decl->init(func_decl, template_param_list);
 
   for (size_t i = 0, template_param_decl_count = template_param_decls.size();
i < template_param_decl_count; ++i) {
@@ -1384,11 +1388,11 @@ ClassTemplateDecl 
*TypeSystemClang::CreateClassTemplateDecl(
   TemplateParameterList *template_param_list = CreateTemplateParameterList(
   ast, template_param_infos, template_param_decls);
 
-  CXXRecordDecl *template_cxx_decl = CXXRecordDecl::Create(
-  ast, (TagDecl::TagKind)kind,
-  decl_ctx, // What decl context do we use here? 

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 23 inline comments as done.
yaxunl added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:1470
+/// diagnostics should be emitted.
+SmallVector DeclsToCheckForDeferredDiags;
+

rjmccall wrote:
> This needs to be saved and restored in modules / PCH.
done



Comment at: clang/lib/Sema/Sema.cpp:1468
   }
-  S.DeviceDeferredDiags.erase(It);
 

yaxunl wrote:
> Fznamznon wrote:
> > This particular change causes duplication of deferred diagnostics.
> > Consider the following example (please correct me if I'm doing something 
> > wrong, I'm not an expert in OpenMP):
> > 
> > ```
> > int foobar1() { throw 1; } // error is expected here
> > 
> > // let's try to use foobar1 in the code where exceptions aren't allowed
> > #pragma omp declare target
> > int (*B)() = 
> > #pragma omp end declare target
> > 
> > // and in some other place let's use foobar1 in device code again
> > #pragma omp declare target
> > int a = foobar1();
> > #pragma omp end declare target
> > 
> > ```
> > Then diagnostic for `foobar1`  will be duplicated for each use of `foobar1` 
> > under `target` directive.
> > I first experienced this behavior not with OpenMP, so I suppose reproducer 
> > can be done for each programming model which uses deferred diagnostics.
> The change is intentional so that each call chain causing the diagnostic can 
> be identified. The drawback is that it is more verbose.
> 
> I can change this behavior so that the diagnostic will be emitted only for 
> the first call chain that causes the diagnostic, if less verbose diagnostics 
> is preferred.
the change is intentional to report all use chains which result in deferred 
diagnostics, otherwise user may fix one issue then see another issue, instead 
of see all of the issues in one compilation.



Comment at: clang/lib/Sema/Sema.cpp:1472
   if (HasWarningOrError && ShowCallStack)
-emitCallStackNotes(S, FD);
+emitCallStackNotes(*this, FD);
+}

rjmccall wrote:
> Hmm.  I know this is existing code, but I just realized something.  I think 
> it's okay to not emit the notes on every diagnostic, but you might want to 
> emit them on the first diagnostic from a function instead of after the last.  
> If the real bug is that the program is using something it's not supposed to 
> use, and there are enough errors in that function to reach the error limit, 
> then the diagnostics emitter will associate these notes with a diagnostic 
> it's suppressing and so presumably suppress them as well, leaving the user 
> with no way to find this information.
done



Comment at: clang/lib/Sema/Sema.cpp:1494
+  visitUsedDecl(E->getLocation(), FD);
+}
+  }

rjmccall wrote:
> This needs to trigger if you use a variable with delayed diagnostics, too, 
> right?
> 
> When you add these methods to `UsedDeclVisitor`, you'll be able to remove 
> them here.
fixed



Comment at: clang/lib/Sema/Sema.cpp:1512
+Inherited::VisitCapturedStmt(Node);
+  }
+

rjmccall wrote:
> Should this also go in the base `UsedDeclVisitor`?  I'm less sure about that 
> because the captured statement is really always a part of the enclosing 
> function, right?  Should the delay mechanism just be looking through local 
> sub-contexts instead?
yes this one should also go to UsedDeclVisitor since this statement causes a 
RecordDecl generated which includes a FunctionDecl for a kernel, therefore this 
RecordDecl needs to be visited as used decl. I am not sure if other sub-context 
have the same effect. If so, I think they need to be handled case by case.



Comment at: clang/lib/Sema/UsedDeclVisitor.h:21
+template 
+class UsedDeclVisitor : public EvaluatedExprVisitor {
+protected:

rjmccall wrote:
> Could you add this in a separate patch?
extracted to https://reviews.llvm.org/D76262



Comment at: clang/lib/Sema/UsedDeclVisitor.h:30
+
+  Derived () { return *static_cast(this); }
+

rjmccall wrote:
> There should definitely be cases in here for every expression that uses a 
> declaration, including both `DeclRefExpr` and `MemberExpr`.  Those might be 
> overridden in subclasses, but that's their business; the default behavior 
> should be to visit every used decl.
done


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

https://reviews.llvm.org/D70172



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

[PATCH] D76262: [NFC] Add UsedDeclVisitor

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: rjmccall.

This patch is extracted from https://reviews.llvm.org/D70172


https://reviews.llvm.org/D76262

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/UsedDeclVisitor.h

Index: clang/lib/Sema/UsedDeclVisitor.h
===
--- /dev/null
+++ clang/lib/Sema/UsedDeclVisitor.h
@@ -0,0 +1,89 @@
+//===- UsedDeclVisitor.h - ODR-used declarations visitor *- C++ -*-===//
+//
+// 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 UsedDeclVisitor, a CRTP class which visits all the
+//  declarations that are ODR-used by an expression or statement.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+#define LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+
+#include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/Sema/SemaInternal.h"
+
+namespace clang {
+template 
+class UsedDeclVisitor : public EvaluatedExprVisitor {
+protected:
+  Sema 
+
+public:
+  typedef EvaluatedExprVisitor Inherited;
+
+  UsedDeclVisitor(Sema ) : Inherited(S.Context), S(S) {}
+
+  Derived () { return *static_cast(this); }
+
+  void VisitDeclRefExpr(DeclRefExpr *E) {
+auto *D = E->getDecl();
+if (isa(D) || isa(D)) {
+  asImpl().visitUsedDecl(E->getLocation(), D);
+}
+  }
+
+  void VisitMemberExpr(MemberExpr *E) {
+auto *D = E->getMemberDecl();
+if (isa(D) || isa(D)) {
+  asImpl().visitUsedDecl(E->getMemberLoc(), D);
+}
+  }
+
+  void VisitCapturedStmt(CapturedStmt *Node) {
+asImpl().visitUsedDecl(Node->getBeginLoc(), Node->getCapturedDecl());
+Inherited::VisitCapturedStmt(Node);
+  }
+
+  void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+asImpl().visitUsedDecl(
+E->getBeginLoc(),
+const_cast(E->getTemporary()->getDestructor()));
+asImpl().Visit(E->getSubExpr());
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (E->getOperatorNew())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorNew());
+if (E->getOperatorDelete())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+Inherited::VisitCXXNewExpr(E);
+  }
+
+  void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+if (E->getOperatorDelete())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+QualType Destroyed = S.Context.getBaseElementType(E->getDestroyedType());
+if (const RecordType *DestroyedRec = Destroyed->getAs()) {
+  CXXRecordDecl *Record = cast(DestroyedRec->getDecl());
+  asImpl().visitUsedDecl(E->getBeginLoc(), S.LookupDestructor(Record));
+}
+
+Inherited::VisitCXXDeleteExpr(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+asImpl().visitUsedDecl(E->getBeginLoc(), E->getConstructor());
+Inherited::VisitCXXConstructExpr(E);
+  }
+
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+asImpl().Visit(E->getExpr());
+  }
+};
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "TreeTransform.h"
+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
@@ -17373,71 +17374,33 @@
 }
 
 namespace {
-  /// Helper class that marks all of the declarations referenced by
-  /// potentially-evaluated subexpressions as "referenced".
-  class EvaluatedExprMarker : public EvaluatedExprVisitor {
-Sema 
-bool SkipLocalVariables;
-
-  public:
-typedef EvaluatedExprVisitor Inherited;
-
-EvaluatedExprMarker(Sema , bool SkipLocalVariables)
-  : Inherited(S.Context), S(S), SkipLocalVariables(SkipLocalVariables) { }
-
-void VisitDeclRefExpr(DeclRefExpr *E) {
-  // If we were asked not to visit local variables, don't.
-  if (SkipLocalVariables) {
-if (VarDecl *VD = dyn_cast(E->getDecl()))
-  if (VD->hasLocalStorage())
-return;
-  }
-
-  S.MarkDeclRefReferenced(E);
-}
-
-void VisitMemberExpr(MemberExpr *E) {
-  S.MarkMemberReferenced(E);
-  Inherited::VisitMemberExpr(E);
-}
-
-void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
-  S.MarkFunctionReferenced(
-  E->getBeginLoc(),
-  const_cast(E->getTemporary()->getDestructor()));
-  Visit(E->getSubExpr());
-}
+/// Helper class that marks all of the declarations referenced by
+/// potentially-evaluated subexpressions as 

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

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

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

efriedma wrote:
> rsmith wrote:
> > nickdesaulniers wrote:
> > > efriedma wrote:
> > > > You need to be more careful here; we can call ConstExprEmitter for 
> > > > arbitrary expressions.
> > > "Be more careful" how?
> > Here are some specific cases in which you need to be more careful because 
> > the code as-is would do the wrong thing:
> > 
> >  * when emitting a global's initializer in C++, where the value of the 
> > object denoted by the DeclRefExpr could have changed between its 
> > initialization and the expression we're currently emitting 
> >  * when emitting anything other than a global initializer in C, where the 
> > value of a global could have changed after its emission
> >  * when emitting a reference to a non-global declaration in C (local 
> > variables might change between initialization and use)
> > 
> > We would need to restrict this to the cases where the variable's value 
> > cannot possibly have changed between initialization and use.
> > 
> > In C, that's (mostly) the case for a static storage variable referenced 
> > from the initializer of a static storage variable, for a thread storage 
> > variable referenced from the initializer of a static storage variable, or 
> > for a thread storage variable referenced from the initializer of a thread 
> > storage variable. Even then, this isn't strictly correct in the presence of 
> > DSOs, but I think it should be correct if the two variables are defined in 
> > the same translation unit.
> > 
> > In C++, that's (mostly) the case when the variable is `const` or 
> > `constexpr` and has no mutable subobjects. (There's still the case where 
> > the reference happens outside the lifetime of the object -- for the most 
> > part we can handwave that away by saying it must be UB, but that's not true 
> > in general in the period of construction and period of destruction.)
> > 
> > In both cases, the optimization is (arguably) still wrong if there are any 
> > volatile subobjects.
> And this is why I don't want to duplicate the logic. :)
> 
> I'd rather not make different assumptions for C and C++; instead, I'd prefer 
> to just use the intersection that's safe in both.  I'm concerned that we 
> could come up with weird results for mixed C and C++ code, otherwise.  Also, 
> it's easier to define the C++ rules because we can base them on the constexpr 
> rules in the standard.
I agree we probably want the same outcome as D76169 provides, if either the 
performance is acceptable or we can find a way to avoid the performance cost 
for the cases we already accept. Perhaps we could get that outcome by ensuring 
that we try the CGExprConstant fast-path (at least in C compilations, maybe in 
general) before we consider the complete-but-slow evaluation strategy in 
ExprConstant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096



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

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added a comment.

In D75917#1917296 , @JonChesterfield 
wrote:

> In D75917#1916972 , @sameerds wrote:
>
> > Well, there is a problem: The LangRef says that scopes are target-defined. 
> > This change says that scopes are defined by the high-level language and 
> > further assumes that OpenCL scopes make sense in all languages. Besides 
> > conflicting with the LangRef, this not seem to work with C++, which has no 
> > scopes and nor with CUDA or HIP, whose scopes are not represented in any 
> > AtomicScopeModel.
>
>
> I don't follow. IR has a fence instruction. This builtin maps directly to it, 
> passing whatever integer arguments were given to the intrinsic along 
> unchanged. It's exactly as valid, or invalid, as said fence instruction.


Is it really? The scope argument of the IR fence is a target-specific string:
http://llvm.org/docs/LangRef.html#syncscope

The change that I see here is assuming a numerical argument, and also assuming 
that the numbers used must conform to the OpenCL enum. That would certainly 
make the builtin quite different from the IR fence.


Repository:
  rC Clang

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

https://reviews.llvm.org/D75917



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

[PATCH] D75274: Fix profiling option on PS4 target

[Dmitry Mikulin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfbb23c9714f2: Fix profiling options on PS4 target: - 
libclang_rt.profile should be added when… (authored by dmikulin).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D75274?vs=247032=250660#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75274

Files:
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/test/Driver/ps4-runtime-flags.c


Index: clang/test/Driver/ps4-runtime-flags.c
===
--- clang/test/Driver/ps4-runtime-flags.c
+++ clang/test/Driver/ps4-runtime-flags.c
@@ -10,10 +10,15 @@
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs -fno-profile-arcs %s 
-### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fno-profile-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate 
-fno-profile-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-NO-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-generate=dir %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fno-profile-instr-generate %s -### 
2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate 
-fno-profile-instr-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate 
-fno-profile-instr-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 
-fprofile-instr-generate=somefile.profraw %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fcs-profile-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fcs-profile-generate 
-fno-profile-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-NO-PROFILE %s
 //
 // CHECK-PS4-PROFILE: "--dependent-lib=libclang_rt.profile-x86_64.a"
 // CHECK-PS4-NO-PROFILE-NOT: "--dependent-lib=libclang_rt.profile-x86_64.a"
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -30,13 +30,17 @@
   if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
-options::OPT_fno_profile_instr_generate, false) ||
+options::OPT_fno_profile_generate, false) ||
Args.hasFlag(options::OPT_fprofile_generate_EQ,
-options::OPT_fno_profile_instr_generate, false) ||
+options::OPT_fno_profile_generate, false) ||
Args.hasFlag(options::OPT_fprofile_instr_generate,
 options::OPT_fno_profile_instr_generate, false) ||
Args.hasFlag(options::OPT_fprofile_instr_generate_EQ,
 options::OPT_fno_profile_instr_generate, false) ||
+   Args.hasFlag(options::OPT_fcs_profile_generate,
+options::OPT_fno_profile_generate, false) ||
+   Args.hasFlag(options::OPT_fcs_profile_generate_EQ,
+options::OPT_fno_profile_generate, false) ||
Args.hasArg(options::OPT_fcreate_profile) ||
Args.hasArg(options::OPT_coverage)))
 CmdArgs.push_back("--dependent-lib=libclang_rt.profile-x86_64.a");


Index: clang/test/Driver/ps4-runtime-flags.c
===
--- clang/test/Driver/ps4-runtime-flags.c
+++ clang/test/Driver/ps4-runtime-flags.c
@@ -10,10 +10,15 @@
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs -fno-profile-arcs %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fno-profile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate -fno-profile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-generate=dir %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate %s -### 2>&1 | FileCheck 

[PATCH] D74238: [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user-defined conversion were allowed.

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

rsmith added a comment.

How do you defend against attempting an infinite sequence of user-defined 
conversion? For example:

  template struct X {
  operator X();
  };
  X<0> x0 = X<1>();




Comment at: clang/lib/Sema/SemaOverload.cpp:7333
+// Save the bad conversion in the candidate, for later use by diagnostics.
+Candidate.Conversions[0].setBad(BadConversionSequence::no_conversion,
+From, CallResultType);

`Conversions[0]` is for the conversion of the object argument to the implicit 
object parameter. This should be stored in `Candidate.FinalConversion` instead.



Comment at: clang/lib/Sema/SemaOverload.cpp:11349
+  if (Cand->FailureKind == ovl_fail_bad_final_conversion
+  && Cand->Conversions[0].isBad()) {
+SetAsMultipleUserDefined(Cand->Conversions[0]);

`&&` goes on the previous line (here and below).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74238



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

[clang] fbb23c9 - Fix profiling options on PS4 target:

[Dmitry Mikulin via cfe-commits]

Author: Dmitry Mikulin
Date: 2020-03-16T16:52:47-07:00
New Revision: fbb23c9714f21c5f46ced5ccaed0cb90e05f61e7

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

LOG: Fix profiling options on PS4 target:
- libclang_rt.profile should be added when -fcs-profile-generate is on 
thecommand line.
- OPT_fno_profile_instr_generate was used as a negative for 
OPT_fprofile_generate. Fix it to use OPT_fno_profile_generate.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/PS4CPU.cpp
clang/test/Driver/ps4-runtime-flags.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 93d86e6d04b1..9ecbb7241d45 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -30,13 +30,17 @@ void tools::PS4cpu::addProfileRTArgs(const ToolChain , 
const ArgList ,
   if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
-options::OPT_fno_profile_instr_generate, false) ||
+options::OPT_fno_profile_generate, false) ||
Args.hasFlag(options::OPT_fprofile_generate_EQ,
-options::OPT_fno_profile_instr_generate, false) ||
+options::OPT_fno_profile_generate, false) ||
Args.hasFlag(options::OPT_fprofile_instr_generate,
 options::OPT_fno_profile_instr_generate, false) ||
Args.hasFlag(options::OPT_fprofile_instr_generate_EQ,
 options::OPT_fno_profile_instr_generate, false) ||
+   Args.hasFlag(options::OPT_fcs_profile_generate,
+options::OPT_fno_profile_generate, false) ||
+   Args.hasFlag(options::OPT_fcs_profile_generate_EQ,
+options::OPT_fno_profile_generate, false) ||
Args.hasArg(options::OPT_fcreate_profile) ||
Args.hasArg(options::OPT_coverage)))
 CmdArgs.push_back("--dependent-lib=libclang_rt.profile-x86_64.a");

diff  --git a/clang/test/Driver/ps4-runtime-flags.c 
b/clang/test/Driver/ps4-runtime-flags.c
index 315976d4e228..3131690304db 100644
--- a/clang/test/Driver/ps4-runtime-flags.c
+++ b/clang/test/Driver/ps4-runtime-flags.c
@@ -10,10 +10,15 @@
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs -fno-profile-arcs %s 
-### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fno-profile-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate 
-fno-profile-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-NO-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-generate=dir %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 -fno-profile-instr-generate %s -### 
2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate 
-fno-profile-instr-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-NO-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate 
-fno-profile-instr-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-PROFILE %s
 // RUN: %clang -target x86_64-scei-ps4 
-fprofile-instr-generate=somefile.profraw %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fcs-profile-generate %s -### 2>&1 | 
FileCheck --check-prefix=CHECK-PS4-PROFILE %s
+// RUN: %clang -target x86_64-scei-ps4 -fcs-profile-generate 
-fno-profile-generate %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-PS4-NO-PROFILE %s
 //
 // CHECK-PS4-PROFILE: "--dependent-lib=libclang_rt.profile-x86_64.a"
 // CHECK-PS4-NO-PROFILE-NOT: "--dependent-lib=libclang_rt.profile-x86_64.a"



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

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

rsmith wrote:
> nickdesaulniers wrote:
> > efriedma wrote:
> > > You need to be more careful here; we can call ConstExprEmitter for 
> > > arbitrary expressions.
> > "Be more careful" how?
> Here are some specific cases in which you need to be more careful because the 
> code as-is would do the wrong thing:
> 
>  * when emitting a global's initializer in C++, where the value of the object 
> denoted by the DeclRefExpr could have changed between its initialization and 
> the expression we're currently emitting 
>  * when emitting anything other than a global initializer in C, where the 
> value of a global could have changed after its emission
>  * when emitting a reference to a non-global declaration in C (local 
> variables might change between initialization and use)
> 
> We would need to restrict this to the cases where the variable's value cannot 
> possibly have changed between initialization and use.
> 
> In C, that's (mostly) the case for a static storage variable referenced from 
> the initializer of a static storage variable, for a thread storage variable 
> referenced from the initializer of a static storage variable, or for a thread 
> storage variable referenced from the initializer of a thread storage 
> variable. Even then, this isn't strictly correct in the presence of DSOs, but 
> I think it should be correct if the two variables are defined in the same 
> translation unit.
> 
> In C++, that's (mostly) the case when the variable is `const` or `constexpr` 
> and has no mutable subobjects. (There's still the case where the reference 
> happens outside the lifetime of the object -- for the most part we can 
> handwave that away by saying it must be UB, but that's not true in general in 
> the period of construction and period of destruction.)
> 
> In both cases, the optimization is (arguably) still wrong if there are any 
> volatile subobjects.
And this is why I don't want to duplicate the logic. :)

I'd rather not make different assumptions for C and C++; instead, I'd prefer to 
just use the intersection that's safe in both.  I'm concerned that we could 
come up with weird results for mixed C and C++ code, otherwise.  Also, it's 
easier to define the C++ rules because we can base them on the constexpr rules 
in the standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096



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

[PATCH] D73513: [memtag] Plug in stack safety analysis.

[Evgenii Stepanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a3723ef114d: [memtag] Plug in stack safety analysis. 
(authored by eugenis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73513

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Driver/memtag.c
  llvm/include/llvm/Analysis/StackSafetyAnalysis.h
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
  llvm/test/CodeGen/AArch64/stack-tagging.ll

Index: llvm/test/CodeGen/AArch64/stack-tagging.ll
===
--- llvm/test/CodeGen/AArch64/stack-tagging.ll
+++ llvm/test/CodeGen/AArch64/stack-tagging.ll
@@ -33,6 +33,7 @@
   %x1 = alloca i32, align 4
   %x2 = alloca i8, align 4
   %x3 = alloca i32, i32 11, align 4
+  %x4 = alloca i32, align 4, !stack-safe !0
   call void @use32(i32* %x1)
   call void @use8(i8* %x2)
   call void @use32(i32* %x3)
@@ -49,6 +50,9 @@
 ; CHECK:  alloca { [11 x i32], [4 x i8] }, align 16
 ; CHECK:  call { [11 x i32], [4 x i8] }* @llvm.aarch64.tagp.{{.*}}({ [11 x i32], [4 x i8] }* {{.*}}, i64 2)
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
+; CHECK:  alloca i32, align 4
+; CHECK-NOT: @llvm.aarch64.tagp
+; CHECK-NOT: @llvm.aarch64.settag
 
 ; CHECK:  call void @use32(
 ; CHECK:  call void @use8(
@@ -185,3 +189,5 @@
 ; CHECK: call void @llvm.aarch64.settag(
 ; CHECK: call void @llvm.aarch64.settag(
 ; CHECK: ret void
+
+!0 = !{}
\ No newline at end of file
Index: llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
===
--- /dev/null
+++ llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
@@ -0,0 +1,34 @@
+; RUN: llvm-as %s -o %t0.bc
+; RUN: llvm-as %S/Inputs/ipa.ll -o %t1.bc
+; RUN: llvm-link -disable-lazy-loading %t0.bc %t1.bc -o %t.combined.bc
+; RUN: opt -S -passes="stack-safety-annotator" %t.combined.bc -o - 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @Write1(i8* %p)
+declare void @Write8(i8* %p)
+
+; Basic out-of-bounds.
+define void @f1() {
+; CHECK-LABEL: define void @f1() {
+; CHECK: alloca i32, align 4{{$}}
+entry:
+  %x = alloca i32, align 4
+  %x1 = bitcast i32* %x to i8*
+  call void @Write8(i8* %x1)
+  ret void
+}
+
+; Basic in-bounds.
+define void @f2() {
+; CHECK-LABEL: define void @f2() {
+; CHECK: alloca i32, align 4, !stack-safe ![[A:[0-9]+]]{{$}}
+entry:
+  %x = alloca i32, align 4
+  %x1 = bitcast i32* %x to i8*
+  call void @Write1(i8* %x1)
+  ret void
+}
+
+; CHECK: ![[A]] = !{}
Index: llvm/lib/Target/AArch64/AArch64StackTagging.cpp
===
--- llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -400,7 +400,9 @@
   // dynamic alloca instrumentation for them as well.
   !AI.isUsedWithInAlloca() &&
   // swifterror allocas are register promoted by ISel
-  !AI.isSwiftError();
+  !AI.isSwiftError() &&
+  // safe allocas are not interesting
+  !AI.getMetadata("stack-safe");
   return IsInteresting;
 }
 
Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -92,6 +92,7 @@
 MODULE_PASS("kasan-module", ModuleAddressSanitizerPass(/*CompileKernel=*/true, false, true, false))
 MODULE_PASS("sancov-module", ModuleSanitizerCoveragePass())
 MODULE_PASS("poison-checking", PoisonCheckingPass())
+MODULE_PASS("stack-safety-annotator", StackSafetyGlobalAnnotatorPass())
 #undef MODULE_PASS
 
 #ifndef CGSCC_ANALYSIS
Index: llvm/lib/Analysis/StackSafetyAnalysis.cpp
===
--- llvm/lib/Analysis/StackSafetyAnalysis.cpp
+++ llvm/lib/Analysis/StackSafetyAnalysis.cpp
@@ -99,11 +99,11 @@
 }
 
 struct AllocaInfo {
-  const AllocaInst *AI = nullptr;
+  AllocaInst *AI = nullptr;
   uint64_t Size = 0;
   UseInfo Use;
 
-  AllocaInfo(unsigned PointerSize, const AllocaInst *AI, uint64_t Size)
+  AllocaInfo(unsigned PointerSize, AllocaInst *AI, uint64_t Size)
   : AI(AI), Size(Size), Use(PointerSize) {}
 
   StringRef getName() const { return AI->getName(); }
@@ -205,7 +205,7 @@
 namespace {
 
 class StackSafetyLocalAnalysis {
-  const Function 
+  Function 
   const DataLayout 
   ScalarEvolution 
   unsigned PointerSize = 0;
@@ -227,7 +227,7 @@
   }
 
 public:
-  StackSafetyLocalAnalysis(const Function , ScalarEvolution )
+  StackSafetyLocalAnalysis(Function , ScalarEvolution )
   : F(F), DL(F.getParent()->getDataLayout()), SE(SE),
 PointerSize(DL.getPointerSizeInBits()),
 UnknownRange(PointerSize, true) {}
@@ 

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted after 7PM PST today.

Thanks

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

[clang] 19fccc5 - [Concepts] Fix incorrect control flow when TryAnnotateTypeConstraint annotates an invalid template-id

[Saar Raz via cfe-commits]

Author: Saar Raz
Date: 2020-03-17T01:49:42+02:00
New Revision: 19fccc52ff2c1da1f93d9317c34769bd9bab8ac8

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

LOG: [Concepts] Fix incorrect control flow when TryAnnotateTypeConstraint 
annotates an invalid template-id

TryAnnotateTypeConstraint could annotate a template-id which doesn't end up 
being a type-constraint,
in which case control flow would incorrectly flow into ParseImplicitInt.

Reenter the loop in this case.
Enable relevant tests for C++20. This required disabling typo-correction during 
TryAnnotateTypeConstraint
and changing a test case which is broken due to a separate bug (will be 
reported and handled separately).

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseTemplate.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/invalid-member-expr.cpp
clang/test/SemaCXX/typo-correction.cpp
clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fd58c00640b0..121ceb7e5d36 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6986,7 +6986,8 @@ class Sema final {
   QualType ObjectType, bool EnteringContext,
   bool ,
   SourceLocation TemplateKWLoc = SourceLocation(),
-  AssumedTemplateKind *ATK = nullptr);
+  AssumedTemplateKind *ATK = nullptr,
+  bool Disambiguation = false);
 
   TemplateNameKind isTemplateName(Scope *S,
   CXXScopeSpec ,
@@ -6995,7 +6996,8 @@ class Sema final {
   ParsedType ObjectType,
   bool EnteringContext,
   TemplateTy ,
-  bool );
+  bool ,
+  bool Disambiguation = false);
 
   /// Try to resolve an undeclared template name as a type template.
   ///

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 6356d82f2d46..09d1732739b8 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3253,6 +3253,9 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec ,
   goto DoneWithDeclSpec;
 if (isTypeConstraintAnnotation())
   continue;
+if (NextToken().is(tok::annot_template_id))
+  // Might have been annotated by TryAnnotateTypeConstraint.
+  continue;
 // Eat the scope spec so the identifier is current.
 ConsumeAnnotationToken();
 ParsedAttributesWithRange Attrs(AttrFactory);
@@ -3406,6 +3409,9 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec ,
   goto DoneWithDeclSpec;
 if (isTypeConstraintAnnotation())
   continue;
+if (Tok.is(tok::annot_template_id))
+  // Might have been annotated by TryAnnotateTypeConstraint.
+  continue;
 ParsedAttributesWithRange Attrs(AttrFactory);
 if (ParseImplicitInt(DS, nullptr, TemplateInfo, AS, DSContext, Attrs)) 
{
   if (!Attrs.empty()) {

diff  --git a/clang/lib/Parse/ParseTemplate.cpp 
b/clang/lib/Parse/ParseTemplate.cpp
index 53c4829f43f4..0406820f74a3 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -710,7 +710,8 @@ bool Parser::TryAnnotateTypeConstraint() {
   /*ObjectType=*/ParsedType(),
   /*EnteringContext=*/false,
   PossibleConcept,
-  MemberOfUnknownSpecialization);
+  MemberOfUnknownSpecialization,
+  /*Disambiguation=*/true);
 if (MemberOfUnknownSpecialization || !PossibleConcept ||
 TNK != TNK_Concept_template) {
   if (SS.isNotEmpty())

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 4923d0a6dad2..1ba66d4a976a 100755
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -174,7 +174,8 @@ TemplateNameKind Sema::isTemplateName(Scope *S,
   ParsedType ObjectTypePtr,
   bool EnteringContext,
   TemplateTy ,
-  bool ) {
+  bool ,
+  bool Disambiguation) {
   assert(getLangOpts().CPlusPlus && "No template names in C!");
 
   DeclarationName TName;

[clang] 2a3723e - [memtag] Plug in stack safety analysis.

[Evgenii Stepanov via cfe-commits]

Author: Evgenii Stepanov
Date: 2020-03-16T16:35:25-07:00
New Revision: 2a3723ef114d467179d463539dd73974b87ccf85

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

LOG: [memtag] Plug in stack safety analysis.

Summary:
Run StackSafetyAnalysis at the end of the IR pipeline and annotate
proven safe allocas with !stack-safe metadata. Do not instrument such
allocas in the AArch64StackTagging pass.

Reviewers: pcc, vitalybuka, ostannard

Reviewed By: vitalybuka

Subscribers: merge_guards_bot, kristof.beyls, hiraditya, cfe-commits, gilang, 
llvm-commits

Tags: #clang, #llvm

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

Added: 
clang/test/Driver/memtag.c
llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/test/CodeGen/AArch64/stack-tagging.ll

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index b6ca46e7e835..28e4ecc7b4bf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeReader.h"
@@ -345,6 +346,11 @@ static void addDataFlowSanitizerPass(const 
PassManagerBuilder ,
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
+static void addMemTagOptimizationPasses(const PassManagerBuilder ,
+legacy::PassManagerBase ) {
+  PM.add(createStackSafetyGlobalInfoWrapperPass(/*SetMetadata=*/true));
+}
+
 static TargetLibraryInfoImpl *createTLII(llvm::Triple ,
  const CodeGenOptions ) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
@@ -696,6 +702,11 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
,
addDataFlowSanitizerPass);
   }
 
+  if (LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
+PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
+   addMemTagOptimizationPasses);
+  }
+
   // Set up the per-function pass manager.
   FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
   if (CodeGenOpts.VerifyModule)
@@ -1300,6 +1311,11 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   /*CompileKernel=*/true, /*Recover=*/true));
 }
 
+if (CodeGenOpts.OptimizationLevel > 0 &&
+LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
+  MPM.addPass(StackSafetyGlobalAnnotatorPass());
+}
+
 if (CodeGenOpts.OptimizationLevel == 0) {
   addCoroutinePassesAtO0(MPM, LangOpts, CodeGenOpts);
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);

diff  --git a/clang/test/Driver/memtag.c b/clang/test/Driver/memtag.c
new file mode 100644
index ..9c548910048e
--- /dev/null
+++ b/clang/test/Driver/memtag.c
@@ -0,0 +1,23 @@
+// REQUIRES: aarch64-registered-target
+
+// Old pass manager.
+// RUN: %clang -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-NO-SAFETY
+// RUN: %clang -O1 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O2 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O3 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+
+// New pass manager.
+// RUN: %clang -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-NO-SAFETY
+// RUN: %clang -O1 -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O2 -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O3 -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag 

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

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

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

nickdesaulniers wrote:
> efriedma wrote:
> > You need to be more careful here; we can call ConstExprEmitter for 
> > arbitrary expressions.
> "Be more careful" how?
Here are some specific cases in which you need to be more careful because the 
code as-is would do the wrong thing:

 * when emitting a global's initializer in C++, where the value of the object 
denoted by the DeclRefExpr could have changed between its initialization and 
the expression we're currently emitting 
 * when emitting anything other than a global initializer in C, where the value 
of a global could have changed after its emission
 * when emitting a reference to a non-global declaration in C (local variables 
might change between initialization and use)

We would need to restrict this to the cases where the variable's value cannot 
possibly have changed between initialization and use.

In C, that's (mostly) the case for a static storage variable referenced from 
the initializer of a static storage variable, for a thread storage variable 
referenced from the initializer of a static storage variable, or for a thread 
storage variable referenced from the initializer of a thread storage variable. 
Even then, this isn't strictly correct in the presence of DSOs, but I think it 
should be correct if the two variables are defined in the same translation unit.

In C++, that's (mostly) the case when the variable is `const` or `constexpr` 
and has no mutable subobjects. (There's still the case where the reference 
happens outside the lifetime of the object -- for the most part we can handwave 
that away by saying it must be UB, but that's not true in general in the period 
of construction and period of destruction.)

In both cases, the optimization is (arguably) still wrong if there are any 
volatile subobjects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096



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

[PATCH] D76030: [CUDA] Warn about unsupported CUDA SDK version only if it's used.

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: hans.
tra added a comment.

@hans  -- this should be cherry-picked into 10 if it's not too late yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76030



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

In D76229#1925360 , @lebedev.ri wrote:

> This seems to be already handled by clang static analyzer? 
> (`clang-analyzer-cplusplus.PlacementNew`)
>
>   :19:5: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
>   ::new () long;
>   ^
>   :19:5: note: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes
>   :64:3: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
> ::new (buffer3) long;
> ^
>
>
> https://godbolt.org/z/9VX5WW


Oh no..It is sad :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo marked an inline comment as done and an inline comment as not done.
oontvoo added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1266
+  if (PP.isIncludeVisibleInLocalModule(File, M)) return false;
+  else  PP.setIncludeVisibleForHeader(File, M);
+} else {

oontvoo wrote:
> jyknight wrote:
> > I wonder if this should be just using the CurSubmoduleState. Actually, is 
> > "M" even needed in this function at all -- why isn't everything just using 
> > CurSubmoduleState? (It's very likely I'm just confused about what the 
> > semantics of this are...).
> > "Is M needed ...  ?"
> 
> Yes - I think so because if we're looking at Module header, then I *think* 
> the header's content will/should be visible to the submods in it.
> 
> On the other hand (ie., the else branch), if we're looking a "regular" 
> header, then it should not be visible to the submods  
[not done - re-opening for question]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951



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

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

[serge via Phabricator via cfe-commits]

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

Renamed registration constructor to use a more llvm-ish name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579

Files:
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  lld/Common/TargetOptionsCommandFlags.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
  llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc
  llvm/include/llvm/module.modulemap
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/MC/CMakeLists.txt
  llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
  llvm/tools/dsymutil/DwarfStreamer.cpp
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llc/CMakeLists.txt
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/CMakeLists.txt
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-isel-fuzzer/llvm-isel-fuzzer.cpp
  llvm/tools/llvm-lto/CMakeLists.txt
  llvm/tools/llvm-lto/llvm-lto.cpp
  llvm/tools/llvm-lto2/CMakeLists.txt
  llvm/tools/llvm-lto2/llvm-lto2.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/CMakeLists.txt
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/CMakeLists.txt
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/lto/lto.cpp
  llvm/tools/opt/opt.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp

Index: llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
+++ llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
@@ -25,7 +25,7 @@
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
-#include "llvm/MC/MCTargetOptionsCommandFlags.inc"
+#include "llvm/MC/MCTargetOptionsCommandFlags.h"
 #include "llvm/PassAnalysisSupport.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/TargetRegistry.h"
@@ -37,6 +37,8 @@
 using namespace llvm;
 using namespace dwarf;
 
+mc::RegisterMCTargetOptionsFlags MOF;
+
 namespace {} // end anonymous namespace
 
 //===--===//
@@ -433,7 +435,7 @@
TripleName,
inconvertibleErrorCode());
 
-  MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
+  MCTargetOptions MCOptions = mc::InitMCTargetOptionsFromFlags();
   MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
   if (!MAI)
 return make_error("no asm info for target " + TripleName,
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -22,7 +22,7 @@
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
-#include "llvm/CodeGen/CommandFlags.inc"
+#include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/DataLayout.h"
@@ -62,6 +62,8 @@
 using namespace llvm;
 using namespace opt_tool;
 
+static codegen::RegisterCodeGenFlags CFG;
+
 // The OptimizationList is automatically populated with registered Passes by the
 // PassNameParser.
 //
@@ -471,16 +473,17 @@
StringRef FeaturesStr,
const TargetOptions ) {
   std::string Error;
-  const Target *TheTarget = TargetRegistry::lookupTarget(MArch, TheTriple,
- Error);
+  const Target *TheTarget =
+  TargetRegistry::lookupTarget(codegen::getMArch(), TheTriple, Error);
   // Some modules don't specify a triple, and this is okay.
   if (!TheTarget) {
 return nullptr;
   }
 
-  return TheTarget->createTargetMachine(TheTriple.getTriple(), CPUStr,
-FeaturesStr, Options, getRelocModel(),
-getCodeModel(), GetCodeGenOptLevel());
+  return TheTarget->createTargetMachine(
+  TheTriple.getTriple(), codegen::getCPUStr(), codegen::getFeaturesStr(),
+  Options, codegen::getExplicitRelocModel(),
+  codegen::getExplicitCodeModel(), GetCodeGenOptLevel());
 }
 
 #ifdef BUILD_EXAMPLES
@@ -667,11 +670,11 @@
   Triple ModuleTriple(M->getTargetTriple());
   std::string CPUStr, FeaturesStr;
   TargetMachine *Machine = nullptr;
-  const TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
+  const TargetOptions Options = codegen::InitTargetOptionsFromCodeGenFlags();
 
   if (ModuleTriple.getArch()) {
-CPUStr = getCPUStr();
-FeaturesStr = getFeaturesStr();
+ 

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@MaskRay no longer a WIP ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

In D76229#1925360 , @lebedev.ri wrote:

> This seems to be already handled by clang static analyzer? 
> (`clang-analyzer-cplusplus.PlacementNew`)
>
>   :19:5: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
>   ::new () long;
>   ^
>   :19:5: note: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes
>   :64:3: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
> ::new (buffer3) long;
> ^
>
>
> https://godbolt.org/z/9VX5WW


But it seems like not all of my tests pass on static analyzer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

In D76229#1925268 , @aaron.ballman 
wrote:

> Have you considered making this a static analyzer check as opposed to a 
> clang-tidy check? I think using dataflow analysis will really reduce the 
> false positives because it will be able to track the allocation and alignment 
> data across control flow.


I have never try to write static analyzer before. Okay, I will look at examples 
of how to work with dataflow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 250630.
stuij edited the summary of this revision.
stuij added a comment.

Updating D76062 : [PATCH] [ARM] ARMv8.6-a 
command-line + BFloat16 Asm Support

follow changes in patch: [TableGen] Support combining AssemblerPredicates with 
ORs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMInstrNEON.td
  llvm/lib/Target/ARM/ARMInstrVFP.td
  llvm/lib/Target/ARM/ARMPredicates.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/test/MC/AArch64/SVE/bfcvt-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfcvt.s
  llvm/test/MC/AArch64/SVE/bfcvtnt-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfcvtnt.s
  llvm/test/MC/AArch64/SVE/bfdot-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfdot.s
  llvm/test/MC/AArch64/SVE/bfmlal-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfmlal.s
  llvm/test/MC/AArch64/SVE/bfmmla-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfmmla.s
  llvm/test/MC/AArch64/armv8.6a-bf16.s
  llvm/test/MC/ARM/bfloat16-a32-errors.s
  llvm/test/MC/ARM/bfloat16-a32-errors2.s
  llvm/test/MC/ARM/bfloat16-a32.s
  llvm/test/MC/ARM/bfloat16-t32-errors.s
  llvm/test/MC/ARM/bfloat16-t32.s
  llvm/test/MC/Disassembler/AArch64/armv8.6a-bf16.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-a32_1.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-a32_2.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-t32.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-t32_errors.txt
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -26,9 +26,9 @@
 "armv7e-m","armv7em",  "armv8-a", "armv8","armv8a",
 "armv8l",  "armv8.1-a","armv8.1a","armv8.2-a","armv8.2a",
 "armv8.3-a",   "armv8.3a", "armv8.4-a",   "armv8.4a", "armv8.5-a",
-"armv8.5a", "armv8-r", "armv8r",  "armv8-m.base", "armv8m.base",
-"armv8-m.main", "armv8m.main", "iwmmxt",  "iwmmxt2",  "xscale",
-"armv8.1-m.main",
+"armv8.5a", "armv8.6-a",   "armv8.6a", "armv8-r", "armv8r",
+"armv8-m.base", "armv8m.base", "armv8-m.main", "armv8m.main", "iwmmxt",
+"iwmmxt2",  "xscale",  "armv8.1-m.main",
 };
 
 bool testARMCPU(StringRef CPUName, StringRef ExpectedArch,
@@ -411,6 +411,9 @@
   testARMArch("armv8.5-a", "generic", "v8.5a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
+  testARMArch("armv8.6-a", "generic", "v8.6a",
+  ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(
   testARMArch("armv8-r", "cortex-r52", "v8r",
   ARMBuildAttrs::CPUArch::v8_R));
   EXPECT_TRUE(
@@ -678,7 +681,7 @@
   "v7",   "v7a","v7ve",  "v7hl",   "v7l",   "v7-r",   "v7r",   "v7-m",
   "v7m",  "v7k","v7s",   "v7e-m",  "v7em",  "v8-a",   "v8","v8a",
   "v8l",  "v8.1-a", "v8.1a", "v8.2-a", "v8.2a", "v8.3-a", "v8.3a", "v8.4-a",
-  "v8.4a", "v8.5-a","v8.5a", "v8-r",   "v8m.base", "v8m.main", "v8.1m.main"
+  "v8.4a", "v8.5-a","v8.5a", "v8.6-a", "v8.6a", "v8-r",   "v8m.base", "v8m.main", "v8.1m.main"
   };
 
   for (unsigned i = 0; i < array_lengthof(Arch); i++) {
@@ -743,6 +746,7 @@
 case ARM::ArchKind::ARMV8_3A:
 case ARM::ArchKind::ARMV8_4A:
 case ARM::ArchKind::ARMV8_5A:
+case ARM::ArchKind::ARMV8_6A:
   EXPECT_EQ(ARM::ProfileKind::A, ARM::parseArchProfile(ARMArch[i]));
   break;
 default:
@@ -1008,6 +1012,8 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv8.5-a", "generic", "v8.5a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv8.6-a", "generic", "v8.6a",
+  

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:1266
+  if (PP.isIncludeVisibleInLocalModule(File, M)) return false;
+  else  PP.setIncludeVisibleForHeader(File, M);
+} else {

jyknight wrote:
> I wonder if this should be just using the CurSubmoduleState. Actually, is "M" 
> even needed in this function at all -- why isn't everything just using 
> CurSubmoduleState? (It's very likely I'm just confused about what the 
> semantics of this are...).
> "Is M needed ...  ?"

Yes - I think so because if we're looking at Module header, then I *think* the 
header's content will/should be visible to the submods in it.

On the other hand (ie., the else branch), if we're looking a "regular" header, 
then it should not be visible to the submods  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

This seems to be already handled by clang static analyzer? 
(`clang-analyzer-cplusplus.PlacementNew`)

  :19:5: warning: Storage provided to placement new is only 2 bytes, 
whereas the allocated type requires 8 bytes 
[clang-analyzer-cplusplus.PlacementNew]
  ::new () long;
  ^
  :19:5: note: Storage provided to placement new is only 2 bytes, 
whereas the allocated type requires 8 bytes
  :64:3: warning: Storage provided to placement new is only 2 bytes, 
whereas the allocated type requires 8 bytes 
[clang-analyzer-cplusplus.PlacementNew]
::new (buffer3) long;
^

https://godbolt.org/z/9VX5WW


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75579: [WIP] Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by libraries

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

The title still says this is a WIP..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D75687: [clangd] Only minimally escape text when rendering to markdown.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry for the slow turnaround, bit off more than I could chew on a few fronts 
:-(




Comment at: clang-tools-extra/clangd/FormattedString.cpp:69
+return false;
+  if (Contents.front() == '!' || Contents.front() == '?' ||
+  Contents.front() == '/')

kadircet wrote:
> what is the `?` for ?
` nit: After.ltrim('#')
I'm trying to avoid repeating the known values of C as there's various 
fallthrough/copied cases and it seems more fragile.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:164
+  case '<': // HTML tag (or autolink, which we choose not to escape)
+return looksLikeTag(After);
+  case '>': // Quote marker. Needs escaping at start of line.

kadircet wrote:
> is this really worth all the trouble ?
Simplified it a bit. I think it's worth not escaping `<` when not matched with 
`>`. Sadly the rule can't be quite that simple because multiple chunks can form 
a tag.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:166
+  case '>': // Quote marker. Needs escaping at start of line.
+return StartsLine && Before.empty();
+  case '&': { // HTML entity reference

kadircet wrote:
> I believe it is also allowed to have whitespaces(less than 4?) before the `>` 
> to be interpreted as a quote marker.
Yes, but a chunk never begins with whitespace (see assert at top of function)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75687



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

[PATCH] D75687: [clangd] Only minimally escape text when rendering to markdown.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 250624.
sammccall marked 10 inline comments as done.
sammccall added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75687

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1905,7 +1905,7 @@
   llvm::StringRef ExpectedMarkdown = R"md(### variable `foo`  
 
 ---
-Value \= `val`  
+Value = `val`  
 
 ---
 ```cpp
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -17,25 +17,94 @@
 namespace markup {
 namespace {
 
-TEST(Render, Escaping) {
-  // Check some ASCII punctuation
-  Paragraph P;
-  P.appendText("*!`");
-  EXPECT_EQ(P.asMarkdown(), "\\*\\!\\`");
+std::string escape(llvm::StringRef Text) {
+  return Paragraph().appendText(Text.str()).asMarkdown();
+}
+
+MATCHER_P(escaped, C, "") {
+  return testing::ExplainMatchResult(::testing::HasSubstr(std::string{'\\', C}),
+ arg, result_listener);
+}
 
+MATCHER(escapedNone, "") {
+  return testing::ExplainMatchResult(::testing::Not(::testing::HasSubstr("\\")),
+ arg, result_listener);
+}
+
+TEST(Render, Escaping) {
   // Check all ASCII punctuation.
-  P = Paragraph();
   std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
-  // Same text, with each character escaped.
-  std::string EscapedPunctuation;
-  EscapedPunctuation.reserve(2 * Punctuation.size());
-  for (char C : Punctuation)
-EscapedPunctuation += std::string("\\") + C;
-  P.appendText(Punctuation);
-  EXPECT_EQ(P.asMarkdown(), EscapedPunctuation);
+  std::string EscapedPunc = R"txt(!"#$%&'()\*+,-./:;<=>?@[\\]^\_\`{|}~)txt";
+  EXPECT_EQ(escape(Punctuation), EscapedPunc);
+
+  // Inline code
+  EXPECT_EQ(escape("`foo`"), R"(\`foo\`)");
+  EXPECT_EQ(escape("`foo"), R"(\`foo)");
+  EXPECT_EQ(escape("foo`"), R"(foo\`)");
+  EXPECT_EQ(escape("``foo``"), R"(\`\`foo\`\`)");
+  // Code blocks
+  EXPECT_EQ(escape("```"), R"(\`\`\`)"); // This could also be inline code!
+  EXPECT_EQ(escape("~~~"), R"(\~~~)");
+
+  // Rulers and headings
+  EXPECT_THAT(escape("## Heading"), escaped('#'));
+  EXPECT_THAT(escape("Foo # bar"), escapedNone());
+  EXPECT_EQ(escape("---"), R"(\---)");
+  EXPECT_EQ(escape("-"), R"(\-)");
+  EXPECT_EQ(escape("==="), R"(\===)");
+  EXPECT_EQ(escape("="), R"(\=)");
+  EXPECT_EQ(escape("***"), R"(\*\*\*)"); // \** could start emphasis!
+
+  // HTML tags.
+  EXPECT_THAT(escape(""), escaped('<'));
+  EXPECT_THAT(escape("std::vector"), escaped('<'));
+  EXPECT_THAT(escape("std::map"), escapedNone());
+  // Autolinks
+  EXPECT_THAT(escape("Email "), escapedNone());
+  EXPECT_THAT(escape("Website "), escapedNone());
+
+  // Bullet lists.
+  EXPECT_THAT(escape("- foo"), escaped('-'));
+  EXPECT_THAT(escape("* foo"), escaped('*'));
+  EXPECT_THAT(escape("+ foo"), escaped('+'));
+  EXPECT_THAT(escape("+"), escaped('+'));
+  EXPECT_THAT(escape("a + foo"), escapedNone());
+  EXPECT_THAT(escape("a+ foo"), escapedNone());
+  EXPECT_THAT(escape("1. foo"), escaped('.'));
+  EXPECT_THAT(escape("a. foo"), escapedNone());
+
+  // Emphasis.
+  EXPECT_EQ(escape("*foo*"), R"(\*foo\*)");
+  EXPECT_EQ(escape("**foo**"), R"(\*\*foo\*\*)");
+  EXPECT_THAT(escape("*foo"), escaped('*'));
+  EXPECT_THAT(escape("foo *"), escapedNone());
+  EXPECT_THAT(escape("foo * bar"), escapedNone());
+  EXPECT_THAT(escape("foo_bar"), escaped('_'));
+  EXPECT_THAT(escape("foo _ bar"), escapedNone());
+
+  // HTML entities.
+  EXPECT_THAT(escape("fish "), escaped('&'));
+  EXPECT_THAT(escape("fish & chips;"), escapedNone());
+  EXPECT_THAT(escape("fish "), escapedNone());
+  EXPECT_THAT(escape("foo  bar"), escaped('&'));
+  EXPECT_THAT(escape("foo  bar"), escaped('&'));
+  EXPECT_THAT(escape("foo &?; bar"), escapedNone());
+
+  // Links.
+  EXPECT_THAT(escape("[foo](bar)"), escaped(']'));
+  EXPECT_THAT(escape("[foo]: bar"), escaped(']'));
+  // No need to escape these, as the target never exists.
+  EXPECT_THAT(escape("[foo][]"), escapedNone());
+  EXPECT_THAT(escape("[foo][bar]"), escapedNone());
+  EXPECT_THAT(escape("[foo]"), escapedNone());
 
   // In code blocks we don't need to escape ASCII punctuation.
-  P = Paragraph();
+  Paragraph P = Paragraph();
   P.appendCode("* foo !+ bar * baz");
   EXPECT_EQ(P.asMarkdown(), "`* foo !+ bar * baz`");
 

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Have you considered making this a static analyzer check as opposed to a 
clang-tidy check? I think using dataflow analysis will really reduce the false 
positives because it will be able to track the allocation and alignment data 
across control flow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

[Balogh, Ádám via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp:32
 
   template 
   void analyzerContainerDataField(const CallExpr *CE, CheckerContext ,

Szelethus wrote:
> NoQ wrote:
> > `llvm::function_ref`?
> Is that a thing? Nice.
You mean a callback instead of a template?


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

https://reviews.llvm.org/D75514



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

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

[Balogh, Ádám via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) {
+Name = DRE->getDecl()->getName();

baloghadamsoftware wrote:
> NoQ wrote:
> > Hmm, i think you should instead look at `ContReg`, i.e. whether it's a 
> > non-anonymous `VarRegion` or a `FieldRegion` or something like that (in 
> > other patches as well). It would work more often and it'll transparently 
> > handle references.
> Unfortunately it is a `SymRegion` so it does not work :-( (Even using 
> `getMostDerivedRegion()` does not help.)
You mean the first checking form `SymbolicRegion`, then get its symbol, check 
for `SymbolRegionValue`, then get its `TypedValueRegion`, check for 
`DeclRegion` and use its `Decl`? This sound waaay more complicated and less 
readable. I am not sure which are the side cases: is it always 
`SymbolicRegion`? Is the `Symbol` of `SymbolicRegion` always a 
`SymbolRegionValue`? Is ithe `TypedValueRegion` (the return value of its 
`getRegion()`) always a `DeclRegion`?


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

https://reviews.llvm.org/D73720



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

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

[Balogh, Ádám via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) {
+Name = DRE->getDecl()->getName();

NoQ wrote:
> Hmm, i think you should instead look at `ContReg`, i.e. whether it's a 
> non-anonymous `VarRegion` or a `FieldRegion` or something like that (in other 
> patches as well). It would work more often and it'll transparently handle 
> references.
Unfortunately it is a `SymRegion` so it does not work :-( (Even using 
`getMostDerivedRegion()` does not help.)


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

https://reviews.llvm.org/D73720



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

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added a comment.

Cheers for working on this @kmclaughlin ! Have you considered adding an 
interface for the new PM? You could check this for reference: 
https://reviews.llvm.org/rGd6de5f12d485a85504bc99d384a85634574a27e2 (also 
implements a FunctionPass).




Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:30
+
+#define DEBUG_TYPE "sve-intrinsicopts"
+

In AArch64TargetMachine.cpp it's:
```
static cl::opt EnableSVEIntrinsicOpts(
"aarch64-sve-intrinsic-opts", cl::Hidden,
cl::desc("Enable SVE intrinsic opts"),
cl::init(true));
```
so it probably make sense to use `see-intrinsic-opts` here as well?



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:110
+  auto *PN = dyn_cast(X->getOperand(0));
+  if (!PN)
+return false;

Given where this method is called, this is guaranteed to be always non-null. 
Perhaps `assert` instead?



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:120
+if (!Reinterpret ||
+RequiredType != Reinterpret->getArgOperand(0)->getType())
+  return false;

Isn't it guaranteed that `RequiredType == 
Reinterpret->getArgOperand(0)->getType()` is always true? I.e., `PN` and the 
incoming values have identical type.



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:125
+  // Create the new Phi
+  LLVMContext  = PN->getContext();
+  IRBuilder<> Builder(C1);

[nit] Perhaps `Ctx` instead of `C1`?



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:131
+
+  for (unsigned i = 0; i < PN->getNumIncomingValues(); i++) {
+auto *Reinterpret = cast(PN->getIncomingValue(i));

`i` -> `I`



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:187
+  // elide away both reinterprets if there are no other users of Y.
+  if (auto *Y = isReinterpretToSVBool(I->getArgOperand(0))) {
+Value *SourceVal = Y->getArgOperand(0);

I'd be tempted to rewrite this to use early exit 
(https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code):

```
auto *Y = isReinterpretToSVBool(I->getArgOperand(0));
if (nullptr == Y)
  return false;

// The rest of the function
```



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:224
+  bool Changed = false;
+  for (auto II = BB->begin(), IE = BB->end(); II != IE;) {
+Instruction *I = &(*II);

1. Could this be a for-range loop instead?

2. This loop seems to be a perfect candidate for `make_early_inc_range` 
(https://github.com/llvm/llvm-project/blob/172f1460ae05ab5c33c757142c8bdb10acfbdbe1/llvm/include/llvm/ADT/STLExtras.h#L499),
 e.g.
```
 for (Instruction  : make_early_inc_range(BB))
  Changed |= optimizeIntrinsic();
```



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:240
+  ReversePostOrderTraversal RPOT(Root);
+  for (auto I = RPOT.begin(), E = RPOT.end(); I != E; ++I) {
+Changed |= optimizeBlock(*I);

AFAIK, this iterates over BBs so `I` should be replaced with `BB`. 

Also, perhaps `for (auto *BB : RPOT) {` instead? 



Comment at: llvm/test/CodeGen/AArch64/sve-intrinsic-opts-ptest.ll:18
+; OPT-LABEL: ptest_any2
+; OPT: %out = call i1 @llvm.aarch64.sve.ptest.any.nxv16i1( 
%1,  %2)
+  %mask = tail call  @llvm.aarch64.sve.ptrue.nxv2i1(i32 31)

What's `%1` and `%2`? Is it worth adding the calls that generated them in the 
expected output?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76078



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

[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping!


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

https://reviews.llvm.org/D73967



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

[PATCH] D76219: [Sema][SVE] Reject "delete" with sizeless types

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76219



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

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D75811#1923281 , @tambre wrote:

> Your help here and over on CMake's side has been very helpful. Thank you!
>  I'll @ you on CMake's side if I need any help while working on CUDA support. 
> Hopefully you won't mind. :)


No problem. I'll be happy to help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75811



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

[PATCH] D75579: [WIP] Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by libraries

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@MaskRay gentle up :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

We already support sizeof expressions where the result can't be computed at 
compile-time.  For example: `int f(int n, int (*p)[n]) { return sizeof(*p); }`. 
 (For constant contexts specifically, see HandleSizeof() in ExprConstant.cpp.)  
I think any argument that it would be hard to extend that to cover sizeless 
types is pretty weak.

Of course, just because we can support it doesn't mean we should.  As a matter 
of language design, we might want to discourage users from writing that sort of 
thing.  It could be argued either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76084



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

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

[Eli Friedman via Phabricator via cfe-commits]

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

This patch LGTM, at least for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76084



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d382dcd46a1: [Syntax] Build declarator nodes (authored by 
hlopko, committed by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-*
-| | 

[PATCH] D76086: [Sema][SVE] Reject arithmetic on pointers to sizeless types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I don't think there's really any reason to force the user into this sort of 
thing... but again, we can relax this later, so I'm not that concerned about it 
at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76086



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

[PATCH] D72089: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72089



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added a comment.

BTW, would be also nice to have tests for trailing return types not at the top 
level -- in a follow-up:

`auto x(char a, auto (*b)(int) -> short) -> void;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220



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

[clang] 7d382dc - [Syntax] Build declarator nodes

[Dmitri Gribenko via cfe-commits]

Author: Marcel Hlopko
Date: 2020-03-16T19:13:59+01:00
New Revision: 7d382dcd46a18c23a01e3754807f577598a9bc84

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

LOG: [Syntax] Build declarator nodes

Summary:
Copy of https://reviews.llvm.org/D72089 with Ilya's permission. See
https://reviews.llvm.org/D72089 for the first batch of comments.

Reviewers: gribozavr2

Reviewed By: gribozavr2

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Nodes.h
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/lib/Tooling/Syntax/Nodes.cpp
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h 
b/clang/include/clang/Tooling/Syntax/Nodes.h
index 25acc1757428..82fcac33f99b 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -38,10 +38,10 @@ enum class NodeKind : uint16_t {
   Leaf,
   TranslationUnit,
 
-  // Expressions
+  // Expressions.
   UnknownExpression,
 
-  // Statements
+  // Statements.
   UnknownStatement,
   DeclarationStatement,
   EmptyStatement,
@@ -58,7 +58,7 @@ enum class NodeKind : uint16_t {
   ExpressionStatement,
   CompoundStatement,
 
-  // Declarations
+  // Declarations.
   UnknownDeclaration,
   EmptyDeclaration,
   StaticAssertDeclaration,
@@ -68,7 +68,16 @@ enum class NodeKind : uint16_t {
   NamespaceAliasDefinition,
   UsingNamespaceDirective,
   UsingDeclaration,
-  TypeAliasDeclaration
+  TypeAliasDeclaration,
+
+  // Declarators.
+  SimpleDeclarator,
+  ParenDeclarator,
+
+  ArraySubscript,
+  TrailingReturnType,
+  ParametersAndQualifiers,
+  MemberPointer
 };
 /// For debugging purposes.
 llvm::raw_ostream <<(llvm::raw_ostream , NodeKind K);
@@ -101,11 +110,19 @@ enum class NodeRole : uint8_t {
   ExpressionStatement_expression,
   CompoundStatement_statement,
   StaticAssertDeclaration_condition,
-  StaticAssertDeclaration_message
+  StaticAssertDeclaration_message,
+  SimpleDeclaration_declarator,
+  ArraySubscript_sizeExpression,
+  TrailingReturnType_arrow,
+  TrailingReturnType_declarator,
+  ParametersAndQualifiers_parameter,
+  ParametersAndQualifiers_trailingReturn
 };
 /// For debugging purposes.
 llvm::raw_ostream <<(llvm::raw_ostream , NodeRole R);
 
+class SimpleDeclarator;
+
 /// A root node for a translation unit. Parent is always null.
 class TranslationUnit final : public Tree {
 public:
@@ -375,6 +392,8 @@ class SimpleDeclaration final : public Declaration {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::SimpleDeclaration;
   }
+  /// FIXME: use custom iterator instead of 'vector'.
+  std::vector declarators();
 };
 
 /// namespace  {  }
@@ -424,6 +443,113 @@ class TypeAliasDeclaration final : public Declaration {
   }
 };
 
+/// Covers a name, an initializer and a part of the type outside declaration
+/// specifiers. Examples are:
+/// `*a` in `int *a`
+/// `a[10]` in `int a[10]`
+/// `*a = nullptr` in `int *a = nullptr`
+/// Declarators can be unnamed too:
+/// `**` in `new int**`
+/// `* = nullptr` in `void foo(int* = nullptr)`
+/// Most declarators you encounter are instances of SimpleDeclarator. They may
+/// contain an inner declarator inside parentheses, we represent it as
+/// ParenDeclarator. E.g.
+/// `(*a)` in `int (*a) = 10`
+class Declarator : public Tree {
+public:
+  Declarator(NodeKind K) : Tree(K) {}
+  static bool classof(const Node *N) {
+return NodeKind::SimpleDeclarator <= N->kind() &&
+   N->kind() <= NodeKind::ParenDeclarator;
+  }
+};
+
+/// A top-level declarator without parentheses. See comment of Declarator for
+/// more details.
+class SimpleDeclarator final : public Declarator {
+public:
+  SimpleDeclarator() : Declarator(NodeKind::SimpleDeclarator) {}
+  static bool classof(const Node *N) {
+return N->kind() == NodeKind::SimpleDeclarator;
+  }
+};
+
+/// Declarator inside parentheses.
+/// E.g. `(***a)` from `int (***a) = nullptr;`
+/// See comment of Declarator for more details.
+class ParenDeclarator final : public Declarator {
+public:
+  ParenDeclarator() : Declarator(NodeKind::ParenDeclarator) {}
+  static bool classof(const Node *N) {
+return N->kind() == NodeKind::ParenDeclarator;
+  }
+  syntax::Leaf *lparen();
+  syntax::Leaf *rparen();
+};
+
+/// Array size specified inside a declarator.
+/// E.g:
+///   `[10]` in `int a[10];`
+///   `[static 10]` in `void f(int xs[static 10]);`
+class ArraySubscript final : public Tree {
+public:
+  ArraySubscript() : Tree(NodeKind::ArraySubscript) {}
+  static bool classof(const Node *N) {
+return N->kind() == NodeKind::ArraySubscript;
+  }
+  // TODO: add an accessor for the 

[PATCH] D75010: [OpenMP] Adding InaccessibleMemOnly and InaccessibleMemOrArgMemOnly for runtime calls.

[Stefan Stipanovic via Phabricator via cfe-commits]

sstefan1 updated this revision to Diff 250599.
sstefan1 added a comment.

more tests

couldn't get `update_test_checks.py` to update function signatures, even with 
the flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75010

Files:
  clang/test/OpenMP/barrier_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll
  llvm/test/Transforms/OpenMP/parallel_deletion.ll

Index: llvm/test/Transforms/OpenMP/parallel_deletion.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion.ll
@@ -1,4 +1,4 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
 ; RUN: opt -S -attributor -openmpopt -attributor-disable=false < %s | FileCheck %s
 ; RUN: opt -S -passes='attributor,cgscc(openmpopt)' -attributor-disable=false < %s | FileCheck %s
 ;
@@ -142,7 +142,7 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:[[TMP:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull align 4 dereferenceable(4) [[TMP]])
+; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull align 4 dereferenceable(4) [[TMP]]) #0
 ; CHECK-NEXT:store i32 0, i32* [[A]], align 4
 ; CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* nonnull align 8 dereferenceable(24) @0, i32 1, void (i32*, i32*, ...)* nonnull bitcast (void (i32*, i32*, i32*)* @.omp_outlined..3 to void (i32*, i32*, ...)*), i32* nocapture nofree nonnull align 4 dereferenceable(4) [[A]])
 ; CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* nonnull align 8 dereferenceable(24) @0, i32 1, void (i32*, i32*, ...)* nonnull bitcast (void (i32*, i32*, i32*)* @.omp_outlined..4 to void (i32*, i32*, ...)*), i32* nocapture nonnull align 4 dereferenceable(4) [[A]])
Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -9,6 +9,7 @@
 
 %struct.omp_lock_t = type { i8* }
 %struct.omp_nest_lock_t = type { i8* }
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
 
 define void @call_all(i32 %schedule, %struct.omp_lock_t* %lock, i32 %lock_hint, %struct.omp_nest_lock_t* %nest_lock, i32 %i, i8* %s, i64 %st, i8* %vp, double %d, i32 %proc_bind, i64 %allocator_handle, i8* %cp, i64 %event_handle, i32 %pause_resource) {
 entry:
@@ -460,6 +461,40 @@
 
 declare dso_local i32 @omp_get_supported_active_levels()
 
+declare void @__kmpc_barrier(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
+
+declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
+
+declare void @__kmpc_flush(%struct.ident_t*)
+
+declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
+
+declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
+
+declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32)
+
+declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32)
+
+declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32)
+
+declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32)
+
+declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_master(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_end_master(%struct.ident_t*, i32)
+
+declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32])
+
+declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32], i32)
+
+declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32])
+
 ; CHECK: ; Function Attrs: nounwind
 ; CHECK-NEXT: declare dso_local void @omp_set_num_threads(i32)
 
@@ -685,67 +720,118 @@
 ; CHECK: ; Function Attrs: nounwind
 ; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels() #0
 
-; OPTIMISTIC: ; Function Attrs: nofree nosync nounwind writeonly
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32)
+
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
+
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
+
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*)
+
+; CHECK: Function Attrs: nounwind
+; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
+
+; CHECK: Function Attrs: nounwind
+; CHECK-NEXT: 

[PATCH] D76218: [Sema][SVE] Reject "new" with sizeless types

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76218



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

[PATCH] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX.

[Sean Fertile via Phabricator via cfe-commits]

sfertile added a comment.

As a first step, I suggest we break the clang changes and the LLVM changes into 
2 separate patches.


Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D76130



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

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

[Ties Stuij via Phabricator via cfe-commits]

stuij commandeered this revision.
stuij added a reviewer: LukeGeeson.
stuij added a comment.

Commandeered because Luke is on vacation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062



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

[PATCH] D76122: [ARM,MVE] Add intrinsics and isel for MVE integer VMLA.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14289-14292
   case Intrinsic::arm_neon_vqrshifts:
   case Intrinsic::arm_neon_vqrshiftu:
 // No immediate versions of these to check for.
 break;

dmgreen wrote:
> I know it's not part of this patch, but what is this code for?
I have no idea! They date from rG2e076c4e02fb99 in 2009 which first introduced 
NEON support, and even then, the switch they appear in had a `default: break` 
clause.

My best guess is that they're intended as a kind of documentation. They signal 
to the reader: "I didn't //forget// about these intrinsics, I carefully 
considered them, decided they didn't need handling here, and included a comment 
saying why not."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76122



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko updated this revision to Diff 250591.
hlopko added a comment.

Fix clang tidy warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-*
-| | |-a
+| | |-SimpleDeclarator
+| | | |-*
+| | | `-a
 | | |-,
-| | `-b
+   

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked 3 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:88
 struct FnDescription;
 using FnCheck = std::function;

NoQ wrote:
> balazske wrote:
> > NoQ wrote:
> > > `llvm::function_ref`?
> > `function_ref`'s documentation says:
> > > This class does not own the callable, so it is not in general safe to 
> > > store a function_ref.
> > The `FnDescription` stores these functions so it is not safe to use 
> > `llvm::function_ref`?
> > 
> > 
> > 
> I think you're using it only for global function pointers, no?
Probably can work but I tried it and got compile errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 250588.
balazske marked 2 inline comments as done.
balazske added a comment.

Addressed some comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,17 +19,21 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include 
 
 using namespace clang;
 using namespace ento;
 using namespace std::placeholders;
 
+#define ASSERT_STREAMSTATE_OPENED  \
+  assert(SS->isOpened() && \
+ "Create of error node failed (only way to get this state)?");
+
 namespace {
 
 /// Full state information about a stream pointer.
 struct StreamState {
   /// State of a stream symbol.
+  /// FIXME: We need maybe an "escaped" state later.
   enum KindTy {
 Opened, /// Stream is opened.
 Closed, /// Closed stream (an invalid stream pointer after it was closed).
@@ -37,37 +41,44 @@
   } State;
 
   /// The error state of a stream.
+  /// Valid only if the stream is opened.
   enum ErrorKindTy {
-NoError,/// No error flag is set or stream is not open.
-EofError,   /// EOF condition (`feof` is true).
-OtherError, /// Other (non-EOF) error (`ferror` is true).
-AnyError/// EofError or OtherError, used if exact error is unknown.
+/// No error flag is set (or stream is not open).
+NoError,
+/// EOF condition (`feof` is true).
+FEof,
+/// Other generic (non-EOF) error (`ferror` is true).
+FError,
   } ErrorState = NoError;
 
   bool isOpened() const { return State == Opened; }
   bool isClosed() const { return State == Closed; }
   bool isOpenFailed() const { return State == OpenFailed; }
 
-  bool isNoError() const { return ErrorState == NoError; }
-  bool isEofError() const { return ErrorState == EofError; }
-  bool isOtherError() const { return ErrorState == OtherError; }
-  bool isAnyError() const { return ErrorState == AnyError; }
+  bool isNoError() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == NoError;
+  }
+  bool isFEof() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == FEof;
+  }
+  bool isFError() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == FError;
+  }
 
   bool operator==(const StreamState ) const {
+// In not opened state error should always NoError.
 return State == X.State && ErrorState == X.ErrorState;
   }
 
   static StreamState getOpened() { return StreamState{Opened}; }
   static StreamState getClosed() { return StreamState{Closed}; }
   static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
-  static StreamState getOpenedWithAnyError() {
-return StreamState{Opened, AnyError};
-  }
-  static StreamState getOpenedWithEofError() {
-return StreamState{Opened, EofError};
-  }
-  static StreamState getOpenedWithOtherError() {
-return StreamState{Opened, OtherError};
+  static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
+  static StreamState getOpenedWithFError() {
+return StreamState{Opened, FError};
   }
 
   void Profile(llvm::FoldingSetNodeID ) const {
@@ -102,7 +113,7 @@
 DefinedSVal makeRetVal(CheckerContext , const CallExpr *CE) {
   assert(CE && "Expecting a call expression.");
 
-  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  const LocationContext *LCtx = C.getLocationContext();
   return C.getSValBuilder()
   .conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
   .castAs();
@@ -135,9 +146,12 @@
   {{"fsetpos", 2}, {::preDefault, nullptr, 0}},
   {{"clearerr", 1},
{::preDefault, ::evalClearerr, 0}},
-  {{"feof", 1}, {::preDefault, ::evalFeof, 0}},
+  {{"feof", 1},
+   {::preDefault,
+::evalFeofFerror<::isFEof>, 0}},
   {{"ferror", 1},
-   {::preDefault, ::evalFerror, 0}},
+   {::preDefault,
+::evalFeofFerror<::isFError>, 0}},
   {{"fileno", 1}, {::preDefault, nullptr, 0}},
   };
 
@@ -161,11 +175,9 @@
   void evalClearerr(const FnDescription *Desc, const CallEvent ,
 CheckerContext ) const;
 
-  void evalFeof(const FnDescription *Desc, const CallEvent ,
-CheckerContext ) const;
-
-  void evalFerror(const FnDescription *Desc, const CallEvent ,
-  CheckerContext ) const;
+  template 
+  void 

[PATCH] D75063: [analyzer] StdLibraryFunctionsChecker: Add NotNull Arg Constraint

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:189
+ValueConstraintPtr negate() const override {
+  NotNullConstraint tmp(*this);
+  tmp.CannotBeNull = !this->CannotBeNull;

Is `Tmp` better name?



Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:59
+fread(buf, sizeof(int), 10, fp); // expected-warning{{Function argument 
constraint is not satisfied}}
+}

One difficulty is that `fread` can work if the `buf` is 0 and `count` is 0 too. 
There is for sure code that uses this feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75063



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

[PATCH] D60748: Fix i386 struct and union parameter alignment

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Oh, I see you just updated your patch months ago without ever mentioning that 
it was ready for review.

It sounds to me like GCC retroactively added a switch specifying which version 
of the ABI to follow on this point, somewhat confusingly called `-malign-data`. 
 That's probably the right move here for us, too, especially since FreeBSD says 
they'd like to use it.  That also means the condition of when to use your new 
logic will have to change; basically, we need a CodeGenOption for this that 
will default to the old ABI, and the driver will pass down a different default 
on Linux.


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

https://reviews.llvm.org/D60748



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

[PATCH] D76234: clang: Simplify implementation of Type::isXXXType()

[Yannic Bonenberger via Phabricator via cfe-commits]

yannic created this revision.
yannic added reviewers: klimek, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76234

Files:
  clang/include/clang/AST/Type.h


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6777,9 +6777,9 @@
 }
 
 inline bool Type::isSpecificBuiltinType(unsigned K) const {
-  if (const BuiltinType *BT = getAs())
-if (BT->getKind() == (BuiltinType::Kind) K)
-  return true;
+  if (const BuiltinType *BT = getAs()) {
+return BT->getKind() == static_cast(K);
+  }
   return false;
 }
 
@@ -6798,9 +6798,7 @@
 
 inline bool Type::isSpecificPlaceholderType(unsigned K) const {
   assert(BuiltinType::isPlaceholderTypeKind((BuiltinType::Kind) K));
-  if (const auto *BT = dyn_cast(this))
-return (BT->getKind() == (BuiltinType::Kind) K);
-  return false;
+  return isSpecificBuiltinType(K);
 }
 
 inline bool Type::isNonOverloadPlaceholderType() const {
@@ -6810,34 +6808,24 @@
 }
 
 inline bool Type::isVoidType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Void;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Void);
 }
 
 inline bool Type::isHalfType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Half;
   // FIXME: Should we allow complex __fp16? Probably not.
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Half);
 }
 
 inline bool Type::isFloat16Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float16;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float16);
 }
 
 inline bool Type::isFloat128Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float128;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float128);
 }
 
 inline bool Type::isNullPtrType() const {
-  if (const auto *BT = getAs())
-return BT->getKind() == BuiltinType::NullPtr;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::NullPtr);
 }
 
 bool IsEnumDeclComplete(EnumDecl *);


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6777,9 +6777,9 @@
 }
 
 inline bool Type::isSpecificBuiltinType(unsigned K) const {
-  if (const BuiltinType *BT = getAs())
-if (BT->getKind() == (BuiltinType::Kind) K)
-  return true;
+  if (const BuiltinType *BT = getAs()) {
+return BT->getKind() == static_cast(K);
+  }
   return false;
 }
 
@@ -6798,9 +6798,7 @@
 
 inline bool Type::isSpecificPlaceholderType(unsigned K) const {
   assert(BuiltinType::isPlaceholderTypeKind((BuiltinType::Kind) K));
-  if (const auto *BT = dyn_cast(this))
-return (BT->getKind() == (BuiltinType::Kind) K);
-  return false;
+  return isSpecificBuiltinType(K);
 }
 
 inline bool Type::isNonOverloadPlaceholderType() const {
@@ -6810,34 +6808,24 @@
 }
 
 inline bool Type::isVoidType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Void;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Void);
 }
 
 inline bool Type::isHalfType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Half;
   // FIXME: Should we allow complex __fp16? Probably not.
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Half);
 }
 
 inline bool Type::isFloat16Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float16;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float16);
 }
 
 inline bool Type::isFloat128Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float128;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float128);
 }
 
 inline bool Type::isNullPtrType() const {
-  if (const auto *BT = getAs())
-return BT->getKind() == BuiltinType::NullPtr;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::NullPtr);
 }
 
 bool IsEnumDeclComplete(EnumDecl *);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73638: [AST] Move dependence computations into a separate file

[Sam McCall via Phabricator via cfe-commits]

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

(I do think there are ways we could write the dependency computations more 
clearly, but don't want to diverge further from the existing code)




Comment at: clang/include/clang/AST/TemplateBase.h:672
   TemplateArgumentLoc *OutArgArray);
+  // FIXME: cleanup the unsued Deps.
   void initializeFrom(SourceLocation TemplateKWLoc,

hokein wrote:
> sammccall wrote:
> > I don't understand this comment, can you expand it?
> The parameter `Deps` is the result populated by this method, the caller 
> doesn't need it since we have the `computeDependencies`.
Thanks, but I meant can you expand the comment :-)
(The explanation you just gave is fine)



Comment at: clang/lib/AST/ComputeDependence.cpp:171
+  E->getSubExpr()->getDependence();
+  if (E->getType()->isDependentType())
+D |= ExprDependence::Type;

the old code for type-dependence is `E->getType()->isDependentType()`.
The new one is the typedependence of `E->getType() | E->getSubExpr() | 
E->getWrittenTypeInfo()->getType()`.

E->getType() is always the getNonLValueExprType of the 
E->getWrittenTypeInfo()->getType(), and that function doesn't change the 
dependence of a type, so I think you can drop the E->getType() here.

You still have E->getSubExpr() as a driver for type-dependence, whith I think 
is incorrect. va_arg(list, t) returns type t regardless of the type of list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73638



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

[PATCH] D76128: [AST] Correct the CXXOperatorCallExpr source range.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a593e29ab9b: [AST] Correct the CXXOperatorCallExpr source 
range. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76128

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExprCXX.cpp


Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -637,7 +637,7 @@
   // Postfix operator
   return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Arrow) {
-return getArg(0)->getSourceRange();
+return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Call) {
 return SourceRange(getArg(0)->getBeginLoc(), getRParenLoc());
   } else if (Kind == OO_Subscript) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -452,6 +452,14 @@
 }
   )cpp",
 
+  R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * $decl[[operator]]->(); };
+void test(S2 s2) {
+  s2-^>f();
+}
+  )cpp",
+
   R"cpp(// Declaration of explicit template specialization
 template 
 struct $decl[[Foo]] {};
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -380,6 +380,14 @@
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
   // FIXME: The following to should be class itself instead.
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
+
+  {R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * operator->(); };
+void test(S2 s2) {
+  s2[[-^>]]f();
+}
+  )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -649,9 +649,14 @@
const ForStmt *Loop,
const Expr *ContainerExpr) {
   StringRef ContainerString;
-  if (isa(ContainerExpr->IgnoreParenImpCasts())) {
+  ContainerExpr = ContainerExpr->IgnoreParenImpCasts();
+  if (isa(ContainerExpr)) {
 ContainerString = "this";
   } else {
+// For CXXOperatorCallExpr (e.g. vector_ptr->size()), its first argument is
+// the class object (vector_ptr) we are targeting.
+if (const auto* E = dyn_cast(ContainerExpr))
+  ContainerExpr = E->getArg(0);
 ContainerString =
 getStringFromRange(Context->getSourceManager(), Context->getLangOpts(),
ContainerExpr->getSourceRange());


Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -637,7 +637,7 @@
   // Postfix operator
   return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Arrow) {
-return getArg(0)->getSourceRange();
+return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Call) {
 return SourceRange(getArg(0)->getBeginLoc(), getRParenLoc());
   } else if (Kind == OO_Subscript) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -452,6 +452,14 @@
 }
   )cpp",
 
+  R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * $decl[[operator]]->(); };
+void test(S2 s2) {
+  s2-^>f();
+}
+  )cpp",
+
   R"cpp(// Declaration of explicit template specialization
 template 
 struct $decl[[Foo]] {};
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -380,6 +380,14 @@
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
   // FIXME: The following to should be class itself instead.
   {"struct foo 

[PATCH] D76125: [clangd] Decouple preambleworker from astworker, NFCI

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:155
 namespace {
+/// Responsible for building and providing access to the preamble of a TU. This
+/// worker doesn't guarantee that each preamble request will be built, in case

This doc comment doesn't mention "thread" once :-)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:156
+/// Responsible for building and providing access to the preamble of a TU. This
+/// worker doesn't guarantee that each preamble request will be built, in case
+/// of multiple preamble requests, it will only build the last one. Once stop 
is

move documentation of requestBuild() to that function?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:157
+/// worker doesn't guarantee that each preamble request will be built, in case
+/// of multiple preamble requests, it will only build the last one. Once stop 
is
+/// called it will build any remaining request and will exit the run loop.

Move documentation of stop() semantics to stop()?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:159
+/// called it will build any remaining request and will exit the run loop.
+class PreambleWorker {
+public:

we may want to call this PreambleThread? Current name emphasizes peer 
relationship with ASTWorker, but current code has parent/child relationship.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:171
+
+  void requestBuild(CompilerInvocation *CI, ParseInputs PI) {
+// If compiler invocation was broken, just fail out early.

or just update()?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:201
+  /// will unblock even after an unsuccessful build.
+  void waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
+

I think all the members in this class with "preamble" in the name can drop that 
word without any loss, e.g. `PreambleWorker::waitForFirst()`



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:205
+  /// built or latest attempt resulted in a failure.
+  std::shared_ptr getLatestBuiltPreamble() const {
+std::lock_guard Lock(Mutex);

latest()?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:218
+PreambleRequested.wait(Lock, [this] { return PreambleReq || Done; });
+// No request means we are done.
+if (!PreambleReq)

Why do we rebuild the preamble if stop is requested?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:265
+  /// Updates the TUStatus and emits it. Only called in the worker thread.
+  void emitTUStatus(TUAction Action,
+const TUStatus::BuildDetails *Details = nullptr) {

as discussed a bit in chat, I think this part needs some more thought. 
Currently the ASTWorker and PreambleWorker emit data for the same file with 
some interleaving that's hard to reason about. The state needs an owner.

(e.g. consider a class that holds a TUStatus and exposes an Event, 
with threadsafe setPreambleAction and setWorkerAction methods, or so)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:291
+  /// valid for Req.
+  std::shared_ptr buildPreamble(Request Req) {
+assert(Req.CI && "Got preamble request with null compiler invocation");

just build(), to avoid confusion clangd::buildPreamble?

Also consider merging with updateLatestBuiltPreamble, both are small and 
private and they always go together.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:295
+std::shared_ptr OldPreamble =
+Inputs.ForceRebuild ? std::shared_ptr()
+: getLatestBuiltPreamble();

(nullptr should work here?)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:312
+
+  mutable std::mutex Mutex;
+  bool Done = false;

Yikes, lots of state :-)
I'd consider merging the two CVs - I find keeping the events separate and 
reasoning when you need notify_one vs _all doesn't seem to pay off, may just be 
the way I think about queues. Up to you.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:322
+  mutable std::condition_variable PreambleBuilt;
+  std::shared_ptr LastBuiltPreamble;
+

nit: last vs latest, be consistent



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:325
+  Notification BuiltFirstPreamble;
+  const PathRef FileName;
+  ParsingCallbacks 

please make this owning for simplicity, this isn't a lightweight object anyway



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:600
 Inputs, CompilerInvocationDiagConsumer, );
+auto OldPreamble = PW.getLatestBuiltPreamble();
+PW.requestBuild(Invocation.get(), Inputs);

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:32
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 

sdesmalen wrote:
> thakis wrote:
> > Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually 
> > a layering violation: clang-tblgen is used to generate headers included in 
> > clang/Basic. In this case it happens to work, but it's because you're 
> > lucky, and it could break for subtle reasons if the TypeFlags header starts 
> > including some other header in Basic that happens to include something 
> > generated.
> > 
> > Please restructure this so that the TableGen code doesn't need an include 
> > from Basic.
> Thanks for pointing out! The only directory that seems to have common 
> includes between Clang TableGen/CodeGen is the llvm/Support directory, any 
> objection to me moving the header there?
That seems like a strange place for this header.

Maybe you can rework things so that you don't have to share a header between 
clang's tablegen and clang's Basic? No other tablegen output so far has needed 
that. (see e.g. the `  /// These must be kept in sync with the flags in 
utils/TableGen/NeonEmitter.h.` line in TargetBuiltins.h).

If that isn't possible at all, I suppose you could put the .h file in 
clang/utils/TableGen and also make clang-tblgen write the .h file and use the 
written .h file in Basic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko updated this revision to Diff 250574.
hlopko marked 8 inline comments as done.
hlopko added a comment.

Resolving comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-*
-| | |-a
+| | |-SimpleDeclarator
+| | | |-*
+| 

[PATCH] D76220: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:522
+///  `(volatile int a)` in `int foo(volatile int a);`
+///  `(int&& a)` in `int foo(int&& a);`
+///  `() -> int` in `auto foo() -> int;`

gribozavr2 wrote:
> I meant:
> 
> `int foo() volatile;`
> `int foo() &&;`
> 
> Of course parameters can be cv and ref-qualified, but that's not the 
> interesting part here. What's interesting is function qualifiers.
Rewrote the comment, added tests.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:526
+///  `() noexcept` in `int foo() noexcept;`
+///  `() throw()` in `int foo() throw();`
+///

gribozavr2 wrote:
> Would you mind adding these examples to tests?
We already have them (see "Exception specification in parameter lists" and 
"Trailing return type in parameter lists").



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:544
+/// E.g. `X::*` in `int X::* a = 0;`
+class MemberPointer final : public Tree {
+public:

gribozavr2 wrote:
> Seems a bit weird that we have a separate node for member pointers, array 
> subscripts, but not for regular pointers.
> 
> I think the rationale behind it is that since we are not building a tree 
> structure out of declarators, the only token under a regular pointer 
> declarator would be a star, so creating that node would not be very helpful.
Yeah I agree, and even though not very helpful, it looks more polished to me, 
so I'll add upload a separate patch with that.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:61
+///
+///   (!) TypeLocs are stored inside out (in the example above `*volatile` is
+///   the TypeLoc returned by `Decl.getTypeSourceInfo()`, and `*const` is

gribozavr2 wrote:
> I don't think LLVM does `(!)` type of ASCII art.
> 
> """
> It is non-trivial to get the start location because TypeLocs are stored 
> inside out. In the example above, ...
> """
Ack, I saw some usages of (!) so was using it too :) Removed in the whole file.

Updated the comment.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:106
+if (T.getTypePtr()->hasTrailingReturn())
+  return SourceLocation(); // avoid recursing into the suffix of 
declarator.
+return VisitTypeLoc(T);

gribozavr2 wrote:
> I suspect there might be an issue here with nested function declarators 
> because we will avoid recursing not only into the suffix (which I think means 
> the trailing return type), but also into parameters.
> 
> If it was not necessary to recurse into parameters, why would the return 
> statement below do that?
Noted, will address in a separate patch.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1282
+struct X {};
+int X::* a;
+)cpp",

gribozavr2 wrote:
> Please also add tests with qualifiers.
> 
> `const int X::* b;`
> `const int X::* const c;`
> 
> Also for member functions:
> 
> `int (X::*d)(int param);`
These 2 tests break asserts, will fix in a separate patch.

const int X::* const c;
int (X::*d)(int param);


Added a test for `const int X::* b;` into this one.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220



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

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76184



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

[clang] 8a593e2 - [AST] Correct the CXXOperatorCallExpr source range.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-03-16T16:51:10+01:00
New Revision: 8a593e29ab9b2799ded3d85a8110d51d4283f128

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

LOG: [AST] Correct the CXXOperatorCallExpr source range.

Summary:
Previously, the range for "->" CXXOperatorCallExpr is the range of the
class object (not including the operator!), e.g. "[[vector_ptr]]->size()".

This patch includes the range of the operator, which fixes the issue
where clangd doesn't go to the overloaded operator "->" definition.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: ilya-biryukov, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
clang/lib/AST/ExprCXX.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index cbec56fbf7a5..fefa56ae5ef5 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -649,9 +649,14 @@ StringRef LoopConvertCheck::getContainerString(ASTContext 
*Context,
const ForStmt *Loop,
const Expr *ContainerExpr) {
   StringRef ContainerString;
-  if (isa(ContainerExpr->IgnoreParenImpCasts())) {
+  ContainerExpr = ContainerExpr->IgnoreParenImpCasts();
+  if (isa(ContainerExpr)) {
 ContainerString = "this";
   } else {
+// For CXXOperatorCallExpr (e.g. vector_ptr->size()), its first argument is
+// the class object (vector_ptr) we are targeting.
+if (const auto* E = dyn_cast(ContainerExpr))
+  ContainerExpr = E->getArg(0);
 ContainerString =
 getStringFromRange(Context->getSourceManager(), Context->getLangOpts(),
ContainerExpr->getSourceRange());

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 7316dea42bd2..e1c873a22b13 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -380,6 +380,14 @@ TEST(SelectionTest, CommonAncestor) {
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
   // FIXME: The following to should be class itself instead.
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
+
+  {R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * operator->(); };
+void test(S2 s2) {
+  s2[[-^>]]f();
+}
+  )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index d387fd219a08..1e687fd109e3 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -452,6 +452,14 @@ TEST(LocateSymbol, All) {
 }
   )cpp",
 
+  R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * $decl[[operator]]->(); };
+void test(S2 s2) {
+  s2-^>f();
+}
+  )cpp",
+
   R"cpp(// Declaration of explicit template specialization
 template 
 struct $decl[[Foo]] {};

diff  --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 69db80f452aa..b507afd6551d 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -637,7 +637,7 @@ SourceRange CXXOperatorCallExpr::getSourceRangeImpl() const 
{
   // Postfix operator
   return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Arrow) {
-return getArg(0)->getSourceRange();
+return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Call) {
 return SourceRange(getArg(0)->getBeginLoc(), getRParenLoc());
   } else if (Kind == OO_Subscript) {



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

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D71018#1924604 , @martong wrote:

> - Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc


I realized that the mentioned functions are redundant, because we have a while 
loop which iterates over all related typelocs (`getNextTypeLoc`).
Also, it is better to remove the `llvm_unreachable` because there are many 
`TypeLoc`s that we just don't handle, e.g. `TypeOfExprTypeLoc`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018



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

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 250569.
njames93 added a comment.

- Rename matcher to hasFinalExpr
- Don't review it yet


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76196

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3006,6 +3006,13 @@
   EXPECT_FALSE(matches("void F() { return; }", RetVal));
 }
 
+TEST(StatementMatcher, hasFinalExpr) {
+  StatementMatcher RetVal = stmtExpr(hasFinalExpr(binaryOperator()));
+  EXPECT_TRUE(matches("void F() { ({int a, b; a + b;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a; a;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a;}); }", RetVal));
+}
+
 TEST(StatementMatcher, ForFunction) {
   const auto CppString1 =
 "struct PosVec {"
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6729,6 +6729,24 @@
   return false;
 }
 
+/// Matches the result value expression of a GNU statement expression.
+///
+/// Given
+/// \code
+///   ({int *Ptr; *Ptr;});
+/// \endcode
+/// hasFinalExpr(unaryOperator())
+///   matches '({int *Ptr; *Ptr;})'
+/// with unaryOperator()
+///   matching '*Ptr'
+AST_MATCHER_P(StmtExpr, hasFinalExpr, internal::Matcher, InnerMatcher) {
+  if (const CompoundStmt *CS = Node.getSubStmt()) {
+if (const auto *E = dyn_cast_or_null(CS->body_back()))
+  return InnerMatcher.matches(*E, Finder, Builder);
+  }
+  return false;
+}
+
 /// Matches CUDA kernel call expression.
 ///
 /// Example matches,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7188,6 +7188,18 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1StmtExpr.html;>StmtExprhasFinalExprMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
+Matches the result 
value expression of a GNU statement expression.
+
+Given
+  ({int *Ptr; *Ptr;});
+hasFinalExpr(unaryOperator())
+  matches '({int *Ptr; *Ptr;})'
+with unaryOperator()
+  matching '*Ptr'
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtalignOfExprMatcherhttps://clang.llvm.org/doxygen/classclang_1_1UnaryExprOrTypeTraitExpr.html;>UnaryExprOrTypeTraitExpr
  InnerMatcher
 Same as 
unaryExprOrTypeTraitExpr, but only matching
 alignof.


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3006,6 +3006,13 @@
   EXPECT_FALSE(matches("void F() { return; }", RetVal));
 }
 
+TEST(StatementMatcher, hasFinalExpr) {
+  StatementMatcher RetVal = stmtExpr(hasFinalExpr(binaryOperator()));
+  EXPECT_TRUE(matches("void F() { ({int a, b; a + b;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a; a;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a;}); }", RetVal));
+}
+
 TEST(StatementMatcher, ForFunction) {
   const auto CppString1 =
 "struct PosVec {"
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6729,6 +6729,24 @@
   return false;
 }
 
+/// Matches the result value expression of a GNU statement expression.
+///
+/// Given
+/// \code
+///   ({int *Ptr; *Ptr;});
+/// \endcode
+/// hasFinalExpr(unaryOperator())
+///   matches '({int *Ptr; *Ptr;})'
+/// with unaryOperator()
+///   matching '*Ptr'
+AST_MATCHER_P(StmtExpr, hasFinalExpr, internal::Matcher, InnerMatcher) {
+  if (const CompoundStmt *CS = Node.getSubStmt()) {
+if (const auto *E = dyn_cast_or_null(CS->body_back()))
+  return InnerMatcher.matches(*E, Finder, Builder);
+  }
+  return false;
+}
+
 /// Matches CUDA kernel call expression.
 ///
 /// Example matches,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7188,6 +7188,18 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1StmtExpr.html;>StmtExprhasFinalExprMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr InnerMatcher
+Matches the result value expression of a GNU statement 

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Also fix the test case, premerge found a failure




Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:23
+
+  for (const Stmt *Child : TheStmt->children()) {
+if (const auto *TheDeclRefExpr = dyn_cast(Child))

For better or worse children can sometimes contain nullptr, best to check for 
that first



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:52
+
+void PlacementNewStorageCheck::check(const MatchFinder::MatchResult ) {
+  const auto *NewExpr = Result.Nodes.getNodeAs("new");

Not gonna say this is blocking anything, but this is quite a large function 
that could be made smaller (more readable) by moving some of those lambdas out 
of the function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 250570.
martong added a comment.

- Fix the base commit of this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5845,6 +5845,53 @@
   EXPECT_TRUE(isa(To->getReturnType()));
 }
 
+struct ImportTypeLoc : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportTypeLoc, Function) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  int f(int p) {
+return 1;
+  }
+  )",
+  Lang_CXX11, "input0.cc");
+  FunctionDecl *FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  FunctionDecl *ToF = Import(FromF, Lang_CXX11);
+  ASSERT_TRUE(ToF);
+  FunctionProtoTypeLoc TL =
+  ToF->getTypeSourceInfo()->getTypeLoc().castAs();
+  EXPECT_EQ(TL.getNumParams(), 1u);
+  EXPECT_TRUE(TL.getParam(0));
+}
+
+TEST_P(ImportTypeLoc, Lambda) {
+  // In this test case, first the CXXRecordDecl of the lambda is imported, then
+  // the operator().
+  Decl *FromTU = getTuDecl(
+  R"(
+  auto l1 = [](unsigned lp) { return 1; };
+  int f(int p) {
+return l1(p);
+  }
+  )",
+  Lang_CXX11, "input0.cc");
+  FunctionDecl *FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  FunctionDecl *ToF = Import(FromF, Lang_CXX11);
+  ASSERT_TRUE(ToF);
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  CXXMethodDecl *ToLambdaMD = FirstDeclMatcher()
+  .match(ToTU, lambdaExpr())
+  ->getCallOperator();
+  FunctionProtoTypeLoc TL = ToLambdaMD->getTypeSourceInfo()
+->getTypeLoc()
+.castAs();
+  EXPECT_EQ(TL.getNumParams(), 1u);
+  EXPECT_TRUE(TL.getParam(0));
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
@@ -5903,5 +5950,8 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportTypeLoc,
+DefaultTestValuesForRunOptions, );
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -42,6 +42,7 @@
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/AST/TypeLocVisitor.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/Builtins.h"
@@ -695,6 +696,8 @@
 // that type is declared inside the body of the function.
 // E.g. auto f() { struct X{}; return X(); }
 bool hasAutoReturnTypeDeclaredInside(FunctionDecl *D);
+
+friend class TypeLocImporter;
   };
 
 template 
@@ -3275,6 +3278,15 @@
 FromReturnTy, FromFPT->getParamTypes(), FromEPI);
   }
 
+  // Import the function parameters.
+  SmallVector Parameters;
+  for (auto P : D->parameters()) {
+if (Expected ToPOrErr = import(P))
+  Parameters.push_back(*ToPOrErr);
+else
+  return ToPOrErr.takeError();
+  }
+
   QualType T;
   TypeSourceInfo *TInfo;
   SourceLocation ToInnerLocStart, ToEndLoc;
@@ -3288,15 +3300,6 @@
   else
 return Imp.takeError();
 
-  // Import the function parameters.
-  SmallVector Parameters;
-  for (auto P : D->parameters()) {
-if (Expected ToPOrErr = import(P))
-  Parameters.push_back(*ToPOrErr);
-else
-  return ToPOrErr.takeError();
-  }
-
   // Create the imported function.
   FunctionDecl *ToFunction = nullptr;
   if (auto *FromConstructor = dyn_cast(D)) {
@@ -3402,16 +3405,6 @@
   }
   ToFunction->setParams(Parameters);
 
-  // We need to complete creation of FunctionProtoTypeLoc manually with setting
-  // params it refers to.
-  if (TInfo) {
-if (auto ProtoLoc =
-TInfo->getTypeLoc().IgnoreParens().getAs()) {
-  for (unsigned I = 0, N = Parameters.size(); I != N; ++I)
-ProtoLoc.setParam(I, Parameters[I]);
-}
-  }
-
   // Import the describing template function, if any.
   if (FromFT) {
 auto ToFTOrErr = import(FromFT);
@@ -8111,20 +8104,305 @@
   return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
 }
 
+namespace clang {
+
+#define IMPORT_TYPE_LOC_OR_RETURN_ERROR(NAME)  \
+  if (auto ImpOrErr = Importer.Import(From.get##NAME()))   \
+To.set##NAME(*ImpOrErr);   \
+  else 

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 250568.
martong added a comment.

- Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018

Files:
  clang/lib/AST/ASTImporter.cpp

Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8106,7 +8106,7 @@
 
 namespace clang {
 
-#define IMPORT_TYPE_LOC(NAME)  \
+#define IMPORT_TYPE_LOC_OR_RETURN_ERROR(NAME)  \
   if (auto ImpOrErr = Importer.Import(From.get##NAME()))   \
 To.set##NAME(*ImpOrErr);   \
   else \
@@ -8123,126 +8123,80 @@
   TypeLocImporter(ASTImporter , TypeLoc ToL)
   : Importer(Importer), ToL(ToL) {}
 
-  Error VisitTypeSpecTypeLoc(TypeSpecTypeLoc From) {
-auto To = ToL.castAs();
-IMPORT_TYPE_LOC(NameLoc);
+  Error VisitTypeLoc(TypeLoc From) {
+// The visitor does not call directly VisitTypeSpecTypeLoc, because that is
+// not a leaf node in the hierarchy (i.e. there is no such enum value in
+// TypeNodes.inc).
+if (auto FTS = From.getAs())
+  return VisitTypeSpecTypeLoc(FTS);
 return Error::success();
   }
 
-  Error VisitTypedefTypeLoc(TypedefTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitInjectedClassNameTypeLoc(InjectedClassNameTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitUnresolvedUsingTypeLoc(UnresolvedUsingTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitRecordTypeLoc(RecordTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitEnumTypeLoc(EnumTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitSubstTemplateTypeParmTypeLoc(SubstTemplateTypeParmTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error
-  VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitVectorTypeLoc(VectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitDependentVectorTypeLoc(DependentVectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitExtVectorTypeLoc(ExtVectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error
-  VisitDependentSizedExtVectorTypeLoc(DependentSizedExtVectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitComplexTypeLoc(ComplexTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitDecltypeTypeLoc(DecltypeTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitDeducedTypeLoc(DeducedTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitAutoTypeLoc(AutoTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
+  Error VisitTypeSpecTypeLoc(TypeSpecTypeLoc From) {
+auto To = ToL.castAs();
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(NameLoc);
+return Error::success();
   }
 
   Error VisitBuiltinTypeLoc(BuiltinTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(BuiltinLoc);
-// FIXME: Import other attributes?
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(BuiltinLoc);
 return Error::success();
   }
 
   Error VisitParenTypeLoc(ParenTypeLoc From) {
 auto To = ToL.castAs();
 
-IMPORT_TYPE_LOC(LParenLoc);
-IMPORT_TYPE_LOC(RParenLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(LParenLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(RParenLoc);
+
+return Error::success();
+  }
 
+  Error VisitPointerTypeLoc(PointerTypeLoc From) {
+auto To = ToL.castAs();
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
 return Error::success();
   }
 
   Error VisitBlockPointerTypeLoc(BlockPointerTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(CaretLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(CaretLoc);
 return Error::success();
   }
 
   Error VisitMemberPointerTypeLoc(MemberPointerTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(StarLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
+return Error::success();
+  }
+
+  Error VisitObjCObjectPointerTypeLoc(ObjCObjectPointerTypeLoc From) {
+auto To = ToL.castAs();
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
 return Error::success();
   }
 
   Error VisitLValueReferenceTypeLoc(LValueReferenceTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(AmpLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(AmpLoc);
 return Error::success();
   }
 
   Error VisitRValueReferenceTypeLoc(RValueReferenceTypeLoc From) {
 auto To = 

[clang] 6ce537c - Revert "[SVE] Auto-generate builtins and header for svld1."

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-03-16T15:22:15Z
New Revision: 6ce537ccfcfc9262ecb8472f7f3c86285b7198fb

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

LOG: Revert "[SVE] Auto-generate builtins and header for svld1."

This reverts commit 8b409eabaf755c88a7d652fe99d3ad858a4fe82a.

Reverting this patch for now because it breaks some buildbots.

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsAArch64.def
clang/include/clang/Basic/CMakeLists.txt
clang/include/clang/Basic/TargetBuiltins.h
clang/include/clang/Basic/arm_sve.td
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/utils/TableGen/SveEmitter.cpp
clang/utils/TableGen/TableGen.cpp
clang/utils/TableGen/TableGenBackends.h

Removed: 
clang/include/clang/Basic/AArch64SVETypeFlags.h
clang/include/clang/Basic/BuiltinsSVE.def



diff  --git a/clang/include/clang/Basic/AArch64SVETypeFlags.h 
b/clang/include/clang/Basic/AArch64SVETypeFlags.h
deleted file mode 100644
index 2b11fe6f9b2b..
--- a/clang/include/clang/Basic/AArch64SVETypeFlags.h
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- AArch64SVETypeFlags.h - Flags used to generate ACLE builtins- C++ 
-*-===//
-//
-//  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
-//
-//===--===//
-
-#ifndef LLVM_CLANG_BASIC_AARCH64SVETYPEFLAGS_H
-#define LLVM_CLANG_BASIC_AARCH64SVETYPEFLAGS_H
-
-#include 
-
-namespace clang {
-
-/// Flags to identify the types for overloaded SVE builtins.
-class SVETypeFlags {
-  uint64_t Flags;
-
-public:
-  /// These must be kept in sync with the flags in
-  /// include/clang/Basic/arm_sve.td.
-  static const uint64_t MemEltTypeOffset = 4; // Bit offset of MemEltTypeMask
-  static const uint64_t EltTypeMask  = 0x000f;
-  static const uint64_t MemEltTypeMask   = 0x0070;
-  static const uint64_t IsLoad   = 0x0080;
-
-  enum EltType {
-Invalid,
-Int8,
-Int16,
-Int32,
-Int64,
-Float16,
-Float32,
-Float64,
-Bool8,
-Bool16,
-Bool32,
-Bool64
-  };
-
-  enum MemEltTy {
-MemEltTyDefault,
-MemEltTyInt8,
-MemEltTyInt16,
-MemEltTyInt32,
-MemEltTyInt64
-  };
-
-  SVETypeFlags(uint64_t F) : Flags(F) { }
-  SVETypeFlags(EltType ET, bool IsUnsigned) : Flags(ET) { }
-
-  EltType getEltType() const { return (EltType)(Flags & EltTypeMask); }
-  MemEltTy getMemEltType() const {
-return (MemEltTy)((Flags & MemEltTypeMask) >> MemEltTypeOffset);
-  }
-
-  bool isLoad() const { return Flags & IsLoad; }
-
-  uint64_t getBits() const { return Flags; }
-  bool isFlagSet(uint64_t Flag) const { return Flags & Flag; }
-};
-
-} // end namespace clang
-
-#endif

diff  --git a/clang/include/clang/Basic/BuiltinsAArch64.def 
b/clang/include/clang/Basic/BuiltinsAArch64.def
index f07c567053de..8f3a24c2e1f6 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -99,6 +99,19 @@ BUILTIN(__builtin_arm_tcommit, "v", "n")
 BUILTIN(__builtin_arm_tcancel, "vWUIi", "n")
 BUILTIN(__builtin_arm_ttest, "WUi", "nc")
 
+// SVE
+BUILTIN(__builtin_sve_svld1_s16, "q8sq16bSsC*", "n")
+BUILTIN(__builtin_sve_svld1_s32, "q4iq16bSiC*", "n")
+BUILTIN(__builtin_sve_svld1_s64, "q2Wiq16bSWiC*", "n")
+BUILTIN(__builtin_sve_svld1_s8, "q16Scq16bScC*", "n")
+BUILTIN(__builtin_sve_svld1_u16, "q8Usq16bUsC*", "n")
+BUILTIN(__builtin_sve_svld1_u32, "q4Uiq16bUiC*", "n")
+BUILTIN(__builtin_sve_svld1_u64, "q2UWiq16bUWiC*", "n")
+BUILTIN(__builtin_sve_svld1_u8, "q16Ucq16bUcC*", "n")
+BUILTIN(__builtin_sve_svld1_f64, "q2dq16bdC*", "n")
+BUILTIN(__builtin_sve_svld1_f32, "q4fq16bfC*", "n")
+BUILTIN(__builtin_sve_svld1_f16, "q8hq16bhC*", "n")
+
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

diff  --git a/clang/include/clang/Basic/BuiltinsSVE.def 
b/clang/include/clang/Basic/BuiltinsSVE.def
deleted file mode 100644
index 2839ca992d98..
--- a/clang/include/clang/Basic/BuiltinsSVE.def
+++ /dev/null
@@ -1,20 +0,0 @@
-//===--- BuiltinsSVE.def - SVE Builtin function database *- C++ 
-*-===//
-//
-//  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 

[PATCH] D76220: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:519
+
+/// Parameter list for a function type.
+/// E.g.:

... and a trailing return type, if the function has one.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:522
+///  `(volatile int a)` in `int foo(volatile int a);`
+///  `(int&& a)` in `int foo(int&& a);`
+///  `() -> int` in `auto foo() -> int;`

I meant:

`int foo() volatile;`
`int foo() &&;`

Of course parameters can be cv and ref-qualified, but that's not the 
interesting part here. What's interesting is function qualifiers.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:526
+///  `() noexcept` in `int foo() noexcept;`
+///  `() throw()` in `int foo() throw();`
+///

Would you mind adding these examples to tests?



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:544
+/// E.g. `X::*` in `int X::* a = 0;`
+class MemberPointer final : public Tree {
+public:

Seems a bit weird that we have a separate node for member pointers, array 
subscripts, but not for regular pointers.

I think the rationale behind it is that since we are not building a tree 
structure out of declarators, the only token under a regular pointer declarator 
would be a star, so creating that node would not be very helpful.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:53
+namespace {
+/// Get start location of the Decl from the TypeLoc.
+/// E.g.:

s/of the Decl/of the declarator/



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:61
+///
+///   (!) TypeLocs are stored inside out (in the example above `*volatile` is
+///   the TypeLoc returned by `Decl.getTypeSourceInfo()`, and `*const` is

I don't think LLVM does `(!)` type of ASCII art.

"""
It is non-trivial to get the start location because TypeLocs are stored inside 
out. In the example above, ...
"""



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:106
+if (T.getTypePtr()->hasTrailingReturn())
+  return SourceLocation(); // avoid recursing into the suffix of 
declarator.
+return VisitTypeLoc(T);

I suspect there might be an issue here with nested function declarators because 
we will avoid recursing not only into the suffix (which I think means the 
trailing return type), but also into parameters.

If it was not necessary to recurse into parameters, why would the return 
statement below do that?



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1282
+struct X {};
+int X::* a;
+)cpp",

Please also add tests with qualifiers.

`const int X::* b;`
`const int X::* const c;`

Also for member functions:

`int (X::*d)(int param);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

This patch broke the Clang build with enabled modules (which is used by the 
LLDB bot, but every other bot is also dead: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen marked 2 inline comments as done.
sdesmalen added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:32
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 

thakis wrote:
> Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually a 
> layering violation: clang-tblgen is used to generate headers included in 
> clang/Basic. In this case it happens to work, but it's because you're lucky, 
> and it could break for subtle reasons if the TypeFlags header starts 
> including some other header in Basic that happens to include something 
> generated.
> 
> Please restructure this so that the TableGen code doesn't need an include 
> from Basic.
Thanks for pointing out! The only directory that seems to have common includes 
between Clang TableGen/CodeGen is the llvm/Support directory, any objection to 
me moving the header there?



Comment at: clang/utils/TableGen/TableGen.cpp:196
+clEnumValN(GenArmSveCodeGenMap, "gen-arm-sve-codegenmap",
+   "Generate arm_sve_codegenmap.inc for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",

thakis wrote:
> Any reason these aren't called `-gen-arm-sve-builtin-def` and 
> `-gen-arm-sve-builtin-codegen` for consistency with CDE and MVE?
Not really, I can change that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:43
+}
+
+

Unnecessary empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D76122: [ARM,MVE] Add intrinsics and isel for MVE integer VMLA.

[Dave Green via Phabricator via cfe-commits]

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

LGTM




Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14289-14292
   case Intrinsic::arm_neon_vqrshifts:
   case Intrinsic::arm_neon_vqrshiftu:
 // No immediate versions of these to check for.
 break;

I know it's not part of this patch, but what is this code for?



Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:5172
+if scalar_addend then {
+  def : Pat<(VTI.Vec (add (mul v1, v2), vs)), (VTI.Vec (Inst v1, v2, s))>;
+} else {

Can you make it so that patterns are:
  def : Pat<(VTI.Vec (add (mul v1, v2), vs)),
(VTI.Vec (Inst v1, v2, s))>;
So they are easier to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76122



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

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

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

steakhal added a comment.

In D71524#1917251 , @Szelethus wrote:

> Are we sure this is what we want? If this is a heuristic, we should document 
> it well, and even then I'm not sure whether we want it. I'm also pretty sure 
> this would make the eventual change to `CallDescriptionMap` more difficult, 
> because the way taintedness is propagated around `std::basic_istream` not 
> really the property of the global `>>` operator and what its parameters are, 
> but rather the property of `std::basic_istream::operator>>`, 
> right? What do you think?


I think `CallDescription` can only identify objects/functions which has 
`IdefntifyerInfo` in them. AFAIK operators don't have such. Though somehow AST 
matchers of Clang Tidy were triggered with this: 
`functionDecl(hasName("operator>>"))`
I'm afraid it needs to be a different patch to replace with 
`CallDescriptionMap`, even though I agree with you.


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

https://reviews.llvm.org/D71524



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

[PATCH] D76094: [clangd] Change line break behaviour for hoverinfo

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I understand the main goal here is preserving intended line breaks in 
documentation comments? Thanks for tackling this problem!

A design goal here is that the markup objects are the source of truth about the 
document structure. So if we've decided we want a comment to turn into multiple 
lines, we should create multiple Paragraph objects, this makes it easy to 
adjust the rendering strategy and isolate differences between text/markdown.

I think this means this splitting needs to happen on the way in instead of on 
the way out. In Hover.cpp:

  if (!Documentation.empty())
Output.addParagraph().appendText(Documentation);

would be come something like

  parseDocumentationComment(Output, Documentation);

which would ultimately add a paragraph to `Output` for each line detected in 
Documentation.

> Even though markdown doc comments in cpp are still rare, I removed the 
> markdown escaping

Please let's leave this out, at least from this patch, if it's not the primary 
goal. This is a complicated tradeoff, there's plenty of reasonably common 
patterns where we should be escaping, such as `vector`. Moreover if 
we're treating punctuation in the comments as formatting, we should likely also 
be doing so in plain-text mode.

FYI D75687  (less unneccesary escaping in 
markdown) is also in flight and likely to touch some of the same code.




Comment at: clang-tools-extra/clangd/FormattedString.cpp:103
+// converted to markdown linebreaks if \p Markdown is set.
+std::string convertLineBreaks(llvm::StringRef Input, bool Markdown = false) {
+  Input = Input.trim();

We can separate concerns more clearly by not having this function care about 
markdown.
Can it have a signature like:
 - `vector splitLines(StringRef Text)` (which wouldn't normalize 
whitespace within the lines)
 - `vector splitLines(StringRef Text)` which would
and then continue to have line rendering handled elsewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76094



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

[PATCH] D75446: [clang][Syntax] Handle macro arguments in spelledForExpanded

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I think per offline discussion, this was going to have some additional docs 
clarifying the contracts of this function, and maybe extra tests?

The conclusion IIRC was that this function is designed to find the text that 
you could replace in order to replace an AST node, so it's fairly conservative 
(but arg expansions are fair game assuming expanded once).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75446



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat updated this revision to Diff 250556.
f00kat added a comment.

Fix 'const auto*' in function getDescendantValueDecl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
@@ -0,0 +1,185 @@
+// RUN: %check_clang_tidy %s cert-mem54-cpp %t
+
+inline void *operator new(size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+inline void *operator new[](size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+void test1()
+{
+// bad (short is aligned to 2).
+short s1;
+::new () long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (same alignment because types are the same).
+long s2;
+::new () long;
+
+// ok (long long is greater then long).
+long long s3;
+::new () long;
+
+// bad (alias for type 'short' which is aligned to 2).
+using s4t = short;
+s4t s4;
+::new () long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's4' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (skip void pointer).
+void* s5 = nullptr;
+::new (s5) long;
+
+// bad (short is aligned to 2).
+short* s6 = nullptr;
+::new (s6) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// bad (just check for pointer to pointer).
+short **s7;
+::new (*s7) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test2()
+{
+  // ok.
+  alignas(long) unsigned char buffer1[sizeof(long)];
+  ::new (buffer1) long;
+
+  // bad (buffer is aligned to 'short' instead of 'long').
+  alignas(short) unsigned char buffer2[sizeof(long)];
+  ::new (buffer2) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  // bad (not enough space).
+  alignas(long) unsigned char buffer3[sizeof(short)];
+  ::new (buffer3) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 2 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+
+  // bad (still not enough space).
+  alignas(long) unsigned char buffer4[sizeof(short) + 1];
+  ::new (buffer4) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 3 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+}
+
+void test3()
+{
+// ok (100 % 4 == 0).
+::new ((size_t*)100) long;
+
+// bad (100 % 4 == 2).
+::new ((size_t*)102) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new (reinterpret_cast(100)) long;
+
+::new (reinterpret_cast(102)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new ((size_t*)(100 + 0)) long;
+
+::new ((size_t*)(100 + 2)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+}
+
+short* test4_f1()
+{
+return nullptr;
+}
+
+short& test4_f2()
+{
+static short v;
+return v;
+}
+
+void test4()
+{
+::new (test4_f1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+::new (_f2()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+short* (*p1)();
+p1 = test4_f1;
+::new (p1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test5()
+{
+  struct S
+  {
+  short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N];
+  ::new (buffer1) S[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 64 bytes are not enough for array allocation which requires 64 bytes [cert-mem54-cpp]
+
+  // 

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat updated this revision to Diff 250555.
f00kat added a comment.

1. Static functions instead of anonymous namespaces.
2. const auto*.
3. Added double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
@@ -0,0 +1,185 @@
+// RUN: %check_clang_tidy %s cert-mem54-cpp %t
+
+inline void *operator new(size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+inline void *operator new[](size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+void test1()
+{
+// bad (short is aligned to 2).
+short s1;
+::new () long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (same alignment because types are the same).
+long s2;
+::new () long;
+
+// ok (long long is greater then long).
+long long s3;
+::new () long;
+
+// bad (alias for type 'short' which is aligned to 2).
+using s4t = short;
+s4t s4;
+::new () long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's4' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (skip void pointer).
+void* s5 = nullptr;
+::new (s5) long;
+
+// bad (short is aligned to 2).
+short* s6 = nullptr;
+::new (s6) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// bad (just check for pointer to pointer).
+short **s7;
+::new (*s7) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test2()
+{
+  // ok.
+  alignas(long) unsigned char buffer1[sizeof(long)];
+  ::new (buffer1) long;
+
+  // bad (buffer is aligned to 'short' instead of 'long').
+  alignas(short) unsigned char buffer2[sizeof(long)];
+  ::new (buffer2) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  // bad (not enough space).
+  alignas(long) unsigned char buffer3[sizeof(short)];
+  ::new (buffer3) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 2 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+
+  // bad (still not enough space).
+  alignas(long) unsigned char buffer4[sizeof(short) + 1];
+  ::new (buffer4) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 3 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+}
+
+void test3()
+{
+// ok (100 % 4 == 0).
+::new ((size_t*)100) long;
+
+// bad (100 % 4 == 2).
+::new ((size_t*)102) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new (reinterpret_cast(100)) long;
+
+::new (reinterpret_cast(102)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new ((size_t*)(100 + 0)) long;
+
+::new ((size_t*)(100 + 2)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+}
+
+short* test4_f1()
+{
+return nullptr;
+}
+
+short& test4_f2()
+{
+static short v;
+return v;
+}
+
+void test4()
+{
+::new (test4_f1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+::new (_f2()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+short* (*p1)();
+p1 = test4_f1;
+::new (p1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test5()
+{
+  struct S
+  {
+  short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N];
+  ::new (buffer1) S[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 64 bytes are not enough for array allocation which 

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:88
 struct FnDescription;
 using FnCheck = std::function;

balazske wrote:
> NoQ wrote:
> > `llvm::function_ref`?
> `function_ref`'s documentation says:
> > This class does not own the callable, so it is not in general safe to store 
> > a function_ref.
> The `FnDescription` stores these functions so it is not safe to use 
> `llvm::function_ref`?
> 
> 
> 
I think you're using it only for global function pointers, no?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:366
+
+  State = State->set(StreamSym, StreamState::getOpened());
+  C.addTransition(State);

balazske wrote:
> NoQ wrote:
> > So what exactly happens when you're calling `clearerr` on a closed stream? 
> > Does it actually become opened? Also what happens when you're calling it on 
> > an open-failed stream?
> The stream gets always in opened and error-free state. The `preDefault` 
> checks that the stream is opened (not closed and not open-failed) and creates 
> error node if not. If this create fails that is a problem case, the stream 
> will be opened after `clearerr`. Should check in the eval functions for 
> closed stream even if the pre-functions (in `preCall` callback) have normally 
> checked this?
Aha, ok, got it.
Let's assert it then?



Comment at: clang/test/Analysis/stream-error.c:33-34
+  int ch = fputc('a', F);
+  if (ch == EOF) {
+// FIXME: fputc not handled by checker yet, should expect TRUE
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}

balazske wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Sure, we don't, but the bigger issue here is that we wouldn't mark `F` to 
> > > be in EOF even if we did handle it, we would also need to understand that 
> > > `ch == EOF` is the telling sign. But that also ins't in the scope of this 
> > > patch :)
> > Yeah, that's gonna be fun.
> If there is a `fputc` call (for example) that is recognized (has recognized 
> stream and no functions "fail") there is already an error and non-error 
> branch created for it. On the error branch the return value of `fputc` would 
> be **EOF** and the stream state is set to error. (The `fputc` should be later 
> modeled by this checker, not done yet.)
> 
> If the `ch == EOF` is found then to figure out if this `ch` comes from an 
> `fputc` that is called on a stream that is not recognized (it may be function 
> parameter) and then make a "tracked" stream with error state for it, this is 
> another thing.
Aha, fair enough, that's probably a well-justified state split.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:19
+
+namespace {
+const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) {

Please use static instead of anonymous namespaces for functions. See LLVM 
Coding Guidelines.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:21
+const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) {
+  if (auto TheDeclRefExpr = dyn_cast(TheStmt))
+return TheDeclRefExpr->getDecl();

const auto *. Same in other places.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:47
+void PlacementNewStorageCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;

Belongs to isLanguageVersionSupported() now.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:69
+
+if (auto TheFunctionProtoType = StorageT->getAs())
+  StorageT = TheFunctionProtoType->getReturnType();

const auto *. Same below.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:95
+
+  Checks that placement new provided with properly aligned pointer to
+  sufficient storage capacity.

Please highlight new with double back-ticks.



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst:6
+
+Checks that placement new provided with properly aligned pointer to sufficient 
storage capacity.
+

Please highlight new with double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:32
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 

Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually a 
layering violation: clang-tblgen is used to generate headers included in 
clang/Basic. In this case it happens to work, but it's because you're lucky, 
and it could break for subtle reasons if the TypeFlags header starts including 
some other header in Basic that happens to include something generated.

Please restructure this so that the TableGen code doesn't need an include from 
Basic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D76196#1924393 , @aaron.ballman 
wrote:

> I'm still not understanding your use case, so it's a bit hard for me to tell 
> whether this approach is good or not. Do you have a situation where you 
> cannot match on the expression itself, you also have to know its position? 
> I'm asking because we don't currently have support for querying positional 
> statements (for instance, you cannot ask what the final statement in a 
> `CompoundStmt`) and statement expressions are a bit strange. For instance, 
> what would the final "expression" be in this legal-but-bizarre statement 
> expression: `({int j = 12; j; int x;});`?


It would be the `int x;` but not matchable as its not an expression. The way 
`StmtExpr` works is it evaluates each `Stmt` in order and returns the result of 
the last `Stmt`, if it is an expression. otherwise it basically returns `void`. 
Possible use cases are if you want to match on say an if condition, but the 
condition is inside a `StmtExpr`. I feel like there could be a better way to 
get this behaviour but I'm not exactly sure what it is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76196



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/include/clang/Basic/CMakeLists.txt:44
 
 # ARM NEON and MVE
 clang_tablegen(arm_neon.inc -gen-arm-neon-sema

Update comment to also say "SVE" and "CDE" (or just say "# ARM builtin headers")



Comment at: clang/utils/TableGen/TableGen.cpp:196
+clEnumValN(GenArmSveCodeGenMap, "gen-arm-sve-codegenmap",
+   "Generate arm_sve_codegenmap.inc for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",

Any reason these aren't called `-gen-arm-sve-builtin-def` and 
`-gen-arm-sve-builtin-codegen` for consistency with CDE and MVE?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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