[PATCH] D80980: [AST] Fix a null initializer crash for InitListExpr

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaInit.cpp:1640
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
+  if (!VerifyOnly && expr)

I have the same question.

but if you looked at the other 2 places (same FIXME) of this file, they reset 
the `Init` only when the `Result` is valid. I think this place maybe an 
overlook.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80980



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

[PATCH] D80980: [AST] Fix a null initializer crash for InitListExpr

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
hokein marked an inline comment as done.
hokein added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:1640
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
+  if (!VerifyOnly && expr)

I have the same question.

but if you looked at the other 2 places (same FIXME) of this file, they reset 
the `Init` only when the `Result` is valid. I think this place maybe an 
overlook.


The Initializer of a InitListExpr can be reset to null, which leads to
nullptr-acces crashes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80980

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/AST/ast-dump-recovery.cpp


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -181,3 +181,14 @@
 // Verified that the generated call operator is invalid.
 // CHECK: |-CXXMethodDecl {{.*}} invalid operator() 'auto () const -> auto'
 using Escape = decltype([] { return undef(); }());
+
+// CHECK:  VarDecl {{.*}} NoCrashOnInvalidInitList
+// CHECK-NEXT: `-RecoveryExpr {{.*}} '' contains-errors lvalue
+// CHECK-NEXT:   `-InitListExpr
+// CHECK-NEXT: `-DesignatedInitExpr {{.*}} 'void'
+// CHECK-NEXT:   `-CXXNullPtrLiteralExpr {{.*}} 'nullptr_t'
+struct {
+  int& abc;
+} NoCrashOnInvalidInitList = {
+  .abc = nullptr,
+};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1638,7 +1638,7 @@
 
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
-  if (!VerifyOnly)
+  if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
   if (hadError)


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -181,3 +181,14 @@
 // Verified that the generated call operator is invalid.
 // CHECK: |-CXXMethodDecl {{.*}} invalid operator() 'auto () const -> auto'
 using Escape = decltype([] { return undef(); }());
+
+// CHECK:  VarDecl {{.*}} NoCrashOnInvalidInitList
+// CHECK-NEXT: `-RecoveryExpr {{.*}} '' contains-errors lvalue
+// CHECK-NEXT:   `-InitListExpr
+// CHECK-NEXT: `-DesignatedInitExpr {{.*}} 'void'
+// CHECK-NEXT:   `-CXXNullPtrLiteralExpr {{.*}} 'nullptr_t'
+struct {
+  int& abc;
+} NoCrashOnInvalidInitList = {
+  .abc = nullptr,
+};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1638,7 +1638,7 @@
 
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
-  if (!VerifyOnly)
+  if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
   if (hadError)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e0bca46 - Options for Basic Block Sections, enabled in D68063 and D73674.

[Sriraman Tallam via cfe-commits]

Author: Sriraman Tallam
Date: 2020-06-02T00:23:32-07:00
New Revision: e0bca46b0854143b2f93b60aac99e669c9776979

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

LOG: Options for Basic Block Sections, enabled in D68063 and D73674.

This patch adds clang options:
-fbasic-block-sections={all,,labels,none} and
-funique-basic-block-section-names.
LLVM Support for basic block sections is already enabled.

+ -fbasic-block-sections={all, , labels, none} : Enables/Disables basic
block sections for all or a subset of basic blocks. "labels" only enables
basic block symbols.
+ -funique-basic-block-section-names: Enables unique section names for
basic block sections, disabled by default.

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

Added: 
clang/test/CodeGen/Inputs/basic-block-sections.funcnames
clang/test/CodeGen/basic-block-sections.c
clang/test/Driver/fbasic-block-sections.c
clang/test/Driver/funique-basic-block-section-names.c

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/docs/UsersManual.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
lld/ELF/Config.h
lld/ELF/Driver.cpp
lld/ELF/LTO.cpp
llvm/include/llvm/CodeGen/CommandFlags.h
llvm/include/llvm/CodeGen/Passes.h
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/BBSectionsPrepare.cpp
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index a10e747153e2..4fde6034975e 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -1331,6 +1331,10 @@ Restrict code to those available for App Extensions
 
 .. option:: -fautolink, -fno-autolink
 
+.. option:: -fbasic-block-sections=labels, -fbasic-block-sections=all, 
-fbasic-block-sections=list=, -fbasic-block-sections=none
+
+Generate labels for each basic block or place each basic block or a subset of 
basic blocks in its own section.
+
 .. option:: -fblocks, -fno-blocks
 
 Enable the 'blocks' language feature

diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index ae479e0a58ae..d13d4180a112 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1698,6 +1698,44 @@ are listed below.
  $ cd $P/bar && clang -c -funique-internal-linkage-names name_conflict.c
  $ cd $P && clang foo/name_conflict.o && bar/name_conflict.o
 
+**-fbasic-block-sections=[labels, all, list=, none]**
+
+  Controls whether Clang emits a label for each basic block.  Further, with
+  values "all" and "list=arg", each basic block or a subset of basic blocks
+  can be placed in its own unique section.
+
+  With the ``list=`` option, a file containing the subset of basic blocks
+  that need to placed in unique sections can be specified.  The format of the
+  file is as follows.  For example, ``list=spec.txt`` where ``spec.txt`` is the
+  following:
+
+  ::
+
+!foo
+!!2
+!_Z3barv
+
+  will place the machine basic block with ``id 2`` in function ``foo`` in a
+  unique section.  It will also place all basic blocks of functions ``bar``
+  in unique sections.
+
+  Further, section clusters can also be specified using the ``list=``
+  option.  For example, ``list=spec.txt`` where ``spec.txt`` contains:
+
+  ::
+
+!foo
+!!1 !!3 !!5
+!!2 !!4 !!6
+
+  will create two unique sections for function ``foo`` with the first
+  containing the odd numbered basic blocks and the second containing the
+  even numbered basic blocks.
+
+  Basic block sections allow the linker to reorder basic blocks and enables
+  link-time optimizations like whole program inter-procedural basic block
+  reordering.
+
 Profile Guided Optimization
 ---
 

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index d20282316b64..89a67235b966 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -45,6 +45,9 @@ CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete 
ctors/dtors as linker
  ///< aliases to base ctors when possible.
 CODEGENOPT(DataSections  , 1, 0) ///< Set when -fdata-sections is enabled.
 CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-nam

Re: Bug in QualTypeNames.cpp and adding an option to prepend "::" to fully qualified names.

[Jean-Baptiste Lespiau via cfe-commits]

Yes it was: https://reviews.llvm.org/D80800#2065643

Thanks to Sam, I understand the situation better, but I have been slowed
down quite a lot by the fact that
(a) the logic to print types is duplicated both in TypePrinter.cpp and
NamedDecl.{h, cpp}. There are 2 different code-paths that are printing the
fully qualified namespaces. Thus, it probably means that to implement this
feature, we need to do it for both, and test it for both.
(a) there are lacking tests, so I have to add tests that should have been
added a long time ago (thus, I have to understand the Matcher API, to be
able to get back a specific qualified, to be able to retrieve it and print
it, to check the output).

I did spend some time during the weekend (as I am doing this in addition to
my normal work, as this is not necessary for my project). So given the
amount of work required, I am also looking if I cannot fix the bug in CLIF
without such change in Clang, that, I must admit, will be difficult for me
to get. Initially, I thought it would be at most a 10h fix, and now it's
eating me up, so maybe I should stop for my own sake :P

On Tue, Jun 2, 2020 at 2:47 AM David Blaikie  wrote:

> Did this end up moving to a review thread? Could someone include a
> link to that phab review, if it did.
>
> Otherwise - I'd still be curious about the answer to Sam's question
> about SuppressUnwrittenScope & see if the inline namespaces could be
> avoided & then going for the "modifying getFullyQualifiedName() to do
> something roughly equivalent but less buggy using the pretty-printer"
>
> On Fri, May 29, 2020 at 7:06 AM Sam McCall  wrote:
> >
> > Sorry about ignoring this, I didn't really have an opinion - clang is
> fairly full of helper functions that don't quite do what you expect.
> > This does look like a bug though.
> >
> > Generally for printing types, PrettyPrinter is the way to go. So I'm
> supportive of any of:
> >  - adding the option to explicitly root names at the global namespace
> >  - modifying CLIF to use a PrintingPolicy directly
> >  - modifying getFullyQualifiedName() to do something roughly equivalent
> but less buggy using the pretty-printer
> >  - deprecating getFullyQualifiedName()
> >
> > > The presence of inline namespaces is about the only bit I'd find a
> touch questionable
> > Does SuppressUnwrittenScope do what you want?
> >
> > I did miss the patch attached to this, we do apparently have docs that
> say mailing patch files to the *-commits@ mailing lists is a good way to
> get them reviewed, but IMO those docs are out of date - Phabricator is the
> way to go. Feel free to send it to me.
> > It will need a test, I think
> llvm-project/clang/unittests/AST/DeclPrinterTest.cpp is probably the best
> place to add one.
> >
> > On Sat, May 23, 2020 at 10:22 PM Jean-Baptiste Lespiau <
> jblesp...@google.com> wrote:
> >>
> >> Hi,
> >>
> >> should we submit this CL, adding the option to prepend classes and
> struct fully qualified names with "::"?
> >>
> >> This can then allow people to slowly switch to the new function, and I
> can close my fix/close my bug in CLIF. I can do more on this CL if it is
> needed, but I would like to be sure this will get submitted and I will not
> waste my time, and know exactly what you want me to do.
> >>
> >> Possible options include:
> >> - adding the full context, with the bug, in the CL description
> >> - adding documentation on `getFullyQualifiedName` expliciting in which
> context the bug can occur
> >> - adding a FullyQualifiedTypePrinter() that will take an AST or a
> TypePolicy, and set the correct fields, so one can do
> QualType.getAsString(FullyQualifiedTypePrinter(ast)) to replace the
> hereabove one.
> >> - add some tests, but I do not know where and how.
> >>
> >> When this few lines are submitted, I can update google3 for clang, and
> I can start changing some internal occurrences, and finally fix the CLIF
> bug.
> >>
> >> Thanks!
> >>
> >> On Tue, May 12, 2020 at 11:19 PM Jean-Baptiste Lespiau <
> jblesp...@google.com> wrote:
> >>>
> >>>
> >>>
> >>> On Tue, May 12, 2020 at 11:11 PM David Blaikie 
> wrote:
> 
>  On Tue, May 12, 2020 at 12:40 PM Jean-Baptiste Lespiau <
> jblesp...@google.com> wrote:
> >
> > Hi,
> >
> > thanks for your answer.
> >
> > Just a few remarks:
> >
> > 1. I imagine that we cannot know if people are using
> getFullyQualifiedName. In particular, people could have developed their own
> internal tools, thus we cannot be aware of all callers. I do not know
> Clang's policy, but can we delete or completely change a function without
> deprecating it first?
> 
> 
>  The LLVM project offers little/no API guarantees -
> potentially/especially for a function with no use in-tree. But, yes, as
> Googlers we'd be encouraged not to commit an API change knowing this might
> cause widespread internal breakage without a plan/pre-emptively fixing
> things.
> 
> >
> > I was imagining that the process was to deprecate

[PATCH] D80771: [MTE] Convert StackSafety into analysis

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 267802.
vitalybuka added a comment.

remove unneeded pass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80771

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Driver/memtag.c
  clang/test/Driver/memtag_lto.c
  llvm/include/llvm/Analysis/StackSafetyAnalysis.h
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Target/AArch64/AArch64.h
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.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
@@ -1,4 +1,5 @@
-; RUN: opt < %s -stack-tagging -S -o - | FileCheck %s
+; RUN: opt < %s -stack-tagging -S -o - | FileCheck %s --check-prefixes=CHECK,SSI
+; RUN: opt < %s -stack-tagging -stack-tagging-use-stack-safety=0 -S -o - | FileCheck %s --check-prefixes=CHECK,NOSSI
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android"
@@ -8,6 +9,11 @@
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
 
+define dso_local void @noUse32(i32*) sanitize_memtag {
+entry:
+  ret void
+}
+
 define void @OneVar() sanitize_memtag {
 entry:
   %x = alloca i32, align 4
@@ -33,7 +39,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
+  %x4 = alloca i32, align 4
   call void @use32(i32* %x1)
   call void @use8(i8* %x2)
   call void @use32(i32* %x3)
@@ -50,9 +56,12 @@
 ; 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
+; SSI:alloca i32, align 4
+; NOSSI:  alloca { i32, [12 x i8] }, align 16
+; NOSSI: @llvm.aarch64.tagp.
+; NOSSI: call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
+; SSI-NOT: @llvm.aarch64.tagp
+; SSI-NOT: @llvm.aarch64.settag
 
 ; CHECK:  call void @use32(
 ; CHECK:  call void @use8(
@@ -61,6 +70,7 @@
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
+; NOSSI:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
 ; CHECK-NEXT:  ret void
 
 
@@ -161,12 +171,15 @@
 another_bb:
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cz)
   store i32 7, i32* %z
+  call void @noUse32(i32* %z)
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cz)
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cz)
   store i32 7, i32* %z
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cz)
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cxcy)
   store i32 8, i32* %xy
+  call void @noUse32(i32* %x)
+  call void @noUse32(i32* %y)
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cxcy)
   ret void
 }
@@ -179,15 +192,18 @@
 ; CHECK: alloca { i32, [12 x i8] }, align 16
 ; CHECK: call { i32, [12 x i8] }* @llvm.aarch64.tagp
 ; CHECK: call void @llvm.aarch64.settag(
-; CHECK: alloca { i32, [12 x i8] }, align 16
-; CHECK: call { i32, [12 x i8] }* @llvm.aarch64.tagp
-; CHECK: call void @llvm.aarch64.settag(
+; SSI: alloca i32, align 4
+; NOSSI: alloca { i32, [12 x i8] }, align 16
+; NOSSI: call { i32, [12 x i8] }* @llvm.aarch64.tagp
+; NOSSI: call void @llvm.aarch64.settag(
 ; CHECK: store i32
+; CHECK: call void @noUse32(i32*
 ; CHECK: store i32
 ; CHECK: store i32
+; CHECK: call void @noUse32(i32*
 ; CHECK: call void @llvm.aarch64.settag(
 ; CHECK: call void @llvm.aarch64.settag(
-; CHECK: call void @llvm.aarch64.settag(
+; NOSSI: call void @llvm.aarch64.settag(
 ; CHECK: ret void
 
 !0 = !{}
\ No newline at end of file
Index: llvm/test/CodeGen/AArch64/O3-pipeline.ll
===
--- llvm/test/CodeGen/AArch64/O3-pipeline.ll
+++ llvm/test/CodeGen/AArch64/O3-pipeline.ll
@@ -66,6 +66,14 @@
 ; CHECK-NEXT:   Interleaved Load Combine Pass
 ; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Interleaved Access Pass
+; CHECK-NEXT: Stack Safety Analysis
+; CHECK-NEXT:   FunctionPass Manager
+; CHECK-NEXT: Dominator Tree Construction
+; CHECK-NEXT: Natural Loop Information
+; CHECK-NEXT: Scalar Evolution Analysis
+; CHECK-NEXT: Stack Safety Local Analysis
+; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator 

[PATCH] D80981: [AST] Fix a crash on accessing a class without definition in constexpr function context.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80981

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1401,3 +1401,15 @@
   //   decreasing address
   static_assert(f(6) == 543210);
 }
+
+namespace NoCrash {
+struct ForwardDecl; // expected-note {{forward declaration of}}
+struct Foo {// expected-note 2{{candidate constructor}}
+  ForwardDecl f;// expected-error {{field has incomplete type}}
+};
+
+constexpr Foo getFoo() {
+  Foo e = 123; // expected-error {{no viable conversion from 'int' to 
'NoCrash::Foo'}}
+  return e;
+}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4316,7 +4316,8 @@
   if (auto *RD = T->getAsCXXRecordDecl()) {
 if (RD->isUnion())
   return APValue((const FieldDecl*)nullptr);
-
+if (!RD->hasDefinition())
+  return APValue();
 APValue Struct(APValue::UninitStruct(), RD->getNumBases(),
std::distance(RD->field_begin(), RD->field_end()));
 


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1401,3 +1401,15 @@
   //   decreasing address
   static_assert(f(6) == 543210);
 }
+
+namespace NoCrash {
+struct ForwardDecl; // expected-note {{forward declaration of}}
+struct Foo {// expected-note 2{{candidate constructor}}
+  ForwardDecl f;// expected-error {{field has incomplete type}}
+};
+
+constexpr Foo getFoo() {
+  Foo e = 123; // expected-error {{no viable conversion from 'int' to 'NoCrash::Foo'}}
+  return e;
+}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4316,7 +4316,8 @@
   if (auto *RD = T->getAsCXXRecordDecl()) {
 if (RD->isUnion())
   return APValue((const FieldDecl*)nullptr);
-
+if (!RD->hasDefinition())
+  return APValue();
 APValue Struct(APValue::UninitStruct(), RD->getNumBases(),
std::distance(RD->field_begin(), RD->field_end()));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Looks similar to https://bugs.llvm.org/show_bug.cgi?id=46142. It sounds pretty 
important to address quickly because it crashes on a fairly large portion of 
C++ code with default settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77178



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

[PATCH] D80822: Run syntax tree tests in many language modes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko accepted this revision.
hlopko 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/D80822/new/

https://reviews.llvm.org/D80822



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

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
tmsriram marked 2 inline comments as done.
Closed by commit rGe0bca46b0854: Options for Basic Block Sections, enabled in 
D68063 and D73674. (authored by tmsriram).

Changed prior to commit:
  https://reviews.llvm.org/D68049?vs=264733&id=267811#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68049

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/Inputs/basic-block-sections.funcnames
  clang/test/CodeGen/basic-block-sections.c
  clang/test/Driver/fbasic-block-sections.c
  clang/test/Driver/funique-basic-block-section-names.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/BBSectionsPrepare.cpp
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -879,7 +879,7 @@
 Name += MBB.getParent()->getName();
   } else {
 Name += MBB.getParent()->getSection()->getName();
-if (TM.getUniqueBBSectionNames()) {
+if (TM.getUniqueBasicBlockSectionNames()) {
   Name += ".";
   Name += MBB.getSymbol()->getName();
 } else {
Index: llvm/lib/CodeGen/MachineFunction.cpp
===
--- llvm/lib/CodeGen/MachineFunction.cpp
+++ llvm/lib/CodeGen/MachineFunction.cpp
@@ -340,7 +340,7 @@
   MBBNumbering.resize(BlockNo);
 }
 
-/// This is used with -fbasicblock-sections or -fbasicblock-labels option.
+/// This is used with -fbasic-block-sections or -fbasicblock-labels option.
 /// A unary encoding of basic block labels is done to keep ".strtab" sizes
 /// small.
 void MachineFunction::createBBLabels() {
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -78,7 +78,7 @@
 CGOPT(unsigned, TLSSize)
 CGOPT(bool, EmulatedTLS)
 CGOPT(bool, UniqueSectionNames)
-CGOPT(bool, UniqueBBSectionNames)
+CGOPT(bool, UniqueBasicBlockSectionNames)
 CGOPT(EABI, EABIVersion)
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
@@ -350,11 +350,11 @@
   cl::init(true));
   CGBINDOPT(UniqueSectionNames);
 
-  static cl::opt UniqueBBSectionNames(
+  static cl::opt UniqueBasicBlockSectionNames(
   "unique-bb-section-names",
   cl::desc("Give unique names to every basic block section"),
   cl::init(false));
-  CGBINDOPT(UniqueBBSectionNames);
+  CGBINDOPT(UniqueBasicBlockSectionNames);
 
   static cl::opt EABIVersion(
   "meabi", cl::desc("Set EABI type (default depends on triple):"),
@@ -460,7 +460,7 @@
   Options.FunctionSections = getFunctionSections();
   Options.BBSections = getBBSectionsMode(Options);
   Options.UniqueSectionNames = getUniqueSectionNames();
-  Options.UniqueBBSectionNames = getUniqueBBSectionNames();
+  Options.UniqueBasicBlockSectionNames = getUniqueBasicBlockSectionNames();
   Options.TLSSize = getTLSSize();
   Options.EmulatedTLS = getEmulatedTLS();
   Options.ExplicitEmulatedTLS = EmulatedTLSView->getNumOccurrences() > 0;
Index: llvm/lib/CodeGen/BBSectionsPrepare.cpp
===
--- llvm/lib/CodeGen/BBSectionsPrepare.cpp
+++ llvm/lib/CodeGen/BBSectionsPrepare.cpp
@@ -9,15 +9,15 @@
 // BBSectionsPrepare implementation.
 //
 // The purpose of this pass is to assign sections to basic blocks when
-// -fbasicblock-sections= option is used. Further, with profile information only
-// the subset of basic blocks with profiles are placed in separate sections and
-// the rest are grouped in a cold section. The exception handling blocks are
+// -fbasic-block-sections= option is used. Further, with profile information
+// only the subset of basic blocks with profiles are placed in separate sections
+// and the rest are grouped in a cold section. The exception handling blocks are
 // treated specially to ensure they are all in one seciton.
 //
 // Basic Block Sections
 // 
 //
-// With option, -fbasicblock-sections=list, every function may be split into
+// With option, -fbasic-block-sections=list, every funct

Re: [PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

[Kadir Çetinkaya via cfe-commits]

Hi Jan,

I don't think there's much point in running ReplayPreamble with an empty
preamble, but this should already be a no-op as there can't be any includes
inside the preamble region if size is 0.

I can't seem to reproduce a failure with the root causes you've provided.
Even when ReplayPreamble::create doesn't take the early exit, for an empty
preamble we should not have any includes, hence ReplayPreamble::replay
would be a no-op. That's what I am getting while running the tests (you can
check this by printing the MainFileIncludes in ReplayPreamble::create
before early exit. Note that PatchesAdditionalIncludes builds two ASTs, the
first one will have additional includes as it builds a full AST with a
non-empty preamble, but the second AST should be built with an empty
preamble and empty preamble-includes)

It seems like something else is going on here, any chance you are inserting
an implicit include inside your custom PP logic? If that's the case we
should look for a fix in include extraction logic as it shouldn't pick up
includes that are coming from implicit(more over non-main-file) sources.

On Tue, Jun 2, 2020 at 8:09 AM Jan Korous via Phabricator <
revi...@reviews.llvm.org> wrote:

> jkorous added a comment.
>
> Hi @kadircet!
> I am investigating a failure of `PatchesAdditionalIncludes` test that we
> got internally. It's a failed assert in `ReplayPreamble::replay`.
> Our clangd source code is for all practical purposes identical to upstream
> version but not so with clang source. Specifically what we do is that in
> `CompilerInstance::createPreprocessor` we always add one particular
> callback.
> This means that when in the test we are calling `buildPreamble` for
> `TestTU TU` with empty buffer we never hit the early return in
> `ReplayPreamble::attach()` (ParsedAST.cpp:124) like upstream version does
> and proceed to create a new `ReplayPreamble` object with `PreambleBounds`
> of `size() == 0` which leads to `ReplayPreamble::MainFileTokens` to be
> empty and later we hit failed assert in `ReplayPreamble::replay` about
> `assert(HashTok != MainFileTokens.end() && ...)`.
>
> Now, while I can just tweak either `ReplayPreamble::attach()` or remove
> the PP callback in the test internally I am wondering whether you consider
> empty preamble & PP with callbacks valid use-case of `ReplayPreamble` and
> worth a fix.
>
> This is where we are creating the empty `MainFileTokens`:
>
>   * frame #0: 0x000103337649 ClangdTests` clang::clangd::(anonymous
> namespace)::ReplayPreamble::ReplayPreamble(this=0x000114f45da0,
> Includes=0x7ffeefbfc678, Delegate=0x000114f3bd80,
> SM=0x000114f3dd80, PP=0x000115850218, LangOpts=0x000114f38eb0,
> PB=0x7ffeefbfc658)  + 169 at ParsedAST.cpp:142
> frame #1: 0x00010333756d ClangdTests` clang::clangd::(anonymous
> namespace)::ReplayPreamble::ReplayPreamble(this=0x000114f45da0,
> Includes=0x7ffeefbfc678, Delegate=0x000114f3bd80,
> SM=0x000114f3dd80, PP=0x000115850218, LangOpts=0x000114f38eb0,
> PB=0x7ffeefbfc658)  + 77 at ParsedAST.cpp:139
> frame #2: 0x000103334fa7 ClangdTests` clang::clangd::(anonymous
> namespace)::ReplayPreamble::attach(Includes=0x7ffeefbfc678,
> Clang=0x000114f33ea0, PB=0x7ffeefbfc658)  + 183 at ParsedAST.cpp:126
> frame #3: 0x000103334189 ClangdTests`
> clang::clangd::ParsedAST::build(Filename=(Data = "/clangd-test/foo.cpp",
> Length = 20), Inputs=0x7ffeefbfdf98, CI=nullptr,
> CompilerInvocationDiags=ArrayRef @ 0x7ffeefbfd830,
> Preamble=std::__1::shared_ptr clang::clangd::PreambleData>::element_type @ 0x000114f3e728 strong=2
> weak=1)  + 3897 at ParsedAST.cpp:385
> frame #4: 0x0001006f1032 ClangdTests` clang::clangd::(anonymous
> namespace)::ParsedASTTest_PatchesAdditionalIncludes_Test::TestBody(this=0x000114f04090)
> + 1778 at ParsedASTTests.cpp:477
>
> This is the failed assert:
>
>   * frame #4: 0x000103337a0c ClangdTests` clang::clangd::(anonymous
> namespace)::ReplayPreamble::replay(this=0x000114f45da0)  + 556 at
> ParsedAST.cpp:182
> frame #5: 0x00010333777c ClangdTests` clang::clangd::(anonymous
> namespace)::ReplayPreamble::FileChanged(this=0x000114f45da0, Loc=(ID =
> 74), Reason=ExitFile, Kind=C_User, PrevFID=(ID = 2))  + 156 at
> ParsedAST.cpp:166
> frame #6: 0x000101b7900a ClangdTests`
> clang::PPChainedCallbacks::FileChanged(this=0x000116204080, Loc=(ID =
> 74), Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 90 at
> PPCallbacks.h:390
> frame #7: 0x000101b79046 ClangdTests`
> clang::PPChainedCallbacks::FileChanged(this=0x0001162040c0, Loc=(ID =
> 74), Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 150 at
> PPCallbacks.h:391
> frame #8: 0x000101b79046 ClangdTests`
> clang::PPChainedCallbacks::FileChanged(this=0x000116204100, Loc=(ID =
> 74), Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 150 at
> PPCallbacks.h:391
> frame #9: 0x000101b7904

[PATCH] D80822: Run syntax tree tests in many language modes

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:54-63
+  bool isCXX() const {
+return Language == Lang_CXX || Language == Lang_CXX11 ||
+   Language == Lang_CXX14 || Language == Lang_CXX17 ||
+   Language == Lang_CXX2a;
+  }
+
+  bool isCXX11OrLater() const {

These could be on the clang/Testing



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:181
+
+Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
 assert(Invocation);

Note: This already adds -fsyntax-only as a flag



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:206
   void expectTreeDumpEqual(StringRef Code, StringRef Tree,
bool RunWithDelayedTemplateParsing = true) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));

is RunWithDelayedTemplateParsing being used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80822



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

[PATCH] D80822: Run syntax tree tests in many language modes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 267814.
gribozavr added a comment.

Updated according to the code review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80822

Files:
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  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
@@ -17,6 +17,7 @@
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Testing/CommandLineArgs.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Syntax/BuildTree.h"
 #include "clang/Tooling/Syntax/Mutations.h"
@@ -25,6 +26,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
@@ -45,12 +47,71 @@
 T->lastLeaf()->token() + 1);
 }
 
-class SyntaxTreeTest : public ::testing::Test {
+struct TestClangConfig {
+  TestLanguage Language;
+  std::string Target;
+
+  bool isCXX() const {
+return Language == Lang_CXX || Language == Lang_CXX11 ||
+   Language == Lang_CXX14 || Language == Lang_CXX17 ||
+   Language == Lang_CXX2a;
+  }
+
+  bool isCXX11OrLater() const {
+return Language == Lang_CXX11 || Language == Lang_CXX14 ||
+   Language == Lang_CXX17 || Language == Lang_CXX2a;
+  }
+
+  bool hasDelayedTemplateParsing() const {
+return Target == "x86_64-pc-win32-msvc";
+  }
+
+  std::vector getCommandLineArgs() const {
+std::vector Result = getCommandLineArgsForTesting(Language);
+Result.push_back("-target");
+Result.push_back(Target);
+return Result;
+  }
+
+  std::string toString() const {
+std::string Result;
+llvm::raw_string_ostream OS(Result);
+OS << "{ Language=" << Language << ", Target=" << Target << " }";
+return OS.str();
+  }
+
+  friend std::ostream &operator<<(std::ostream &OS,
+  const TestClangConfig &ClangConfig) {
+return OS << ClangConfig.toString();
+  }
+
+  static std::vector &allConfigs() {
+static std::vector all_configs = []() {
+  std::vector all_configs;
+  for (TestLanguage lang : {Lang_C, Lang_C89, Lang_CXX, Lang_CXX11,
+Lang_CXX14, Lang_CXX17, Lang_CXX2a}) {
+TestClangConfig config;
+config.Language = lang;
+config.Target = "x86_64-pc-linux-gnu";
+all_configs.push_back(config);
+
+// Windows target is interesting to test because it enables
+// `-fdelayed-template-parsing`.
+config.Target = "x86_64-pc-win32-msvc";
+all_configs.push_back(config);
+  }
+  return all_configs;
+}();
+return all_configs;
+  }
+};
+
+class SyntaxTreeTest : public ::testing::Test,
+   public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *
-  buildTree(llvm::StringRef Code,
-const std::string &Target = "x86_64-pc-linux-gnu") {
+  syntax::TranslationUnit *buildTree(llvm::StringRef Code,
+ const TestClangConfig &ClangConfig) {
 // FIXME: this code is almost the identical to the one in TokensTest. Share
 //it.
 class BuildSyntaxTree : public ASTConsumer {
@@ -105,11 +166,19 @@
diag::Severity::Ignored, SourceLocation());
 
 // Prepare to run a compiler.
-std::vector Args = {
-"syntax-test", "-target",   Target.c_str(),
-FileName,  "-fsyntax-only", "-std=c++17",
+std::vector Args = {
+"syntax-test",
+"-fsyntax-only",
 };
-Invocation = createInvocationFromCommandLine(Args, Diags, FS);
+llvm::copy(ClangConfig.getCommandLineArgs(), std::back_inserter(Args));
+Args.push_back(FileName);
+
+std::vector ArgsCStr;
+for (const std::string &arg : Args) {
+  ArgsCStr.push_back(arg.c_str());
+}
+
+Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
 assert(Invocation);
 Invocation->getFrontendOpts().DisableFree = false;
 Invocation->getPreprocessorOpts().addRemappedFile(
@@ -133,32 +202,15 @@
 return Root;
   }
 
-  void expectTreeDumpEqual(StringRef Code, StringRef Tree,
-   bool RunWithDelayedTemplateParsing = true) {
+  void expectTreeDumpEqual(StringRef Code, StringRef Tree) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
 SCOPED_TRACE(Code);
 
-std::string Expected = Tree.trim().str();
-
-// We want to run the test with -fdelayed-template-parsing en

[PATCH] D80822: Run syntax tree tests in many language modes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:54-63
+  bool isCXX() const {
+return Language == Lang_CXX || Language == Lang_CXX11 ||
+   Language == Lang_CXX14 || Language == Lang_CXX17 ||
+   Language == Lang_CXX2a;
+  }
+
+  bool isCXX11OrLater() const {

eduucaldas wrote:
> These could be on the clang/Testing
I agree, but I'll move this type in a future change, when I will be unifying 
the infrastructure with the one that we have in other tests. I'm afraid of 
moving it right now because I'm not sure it is the right abstraction.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:181
+
+Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
 assert(Invocation);

eduucaldas wrote:
> Note: This already adds -fsyntax-only as a flag
It is not clear we can rely on that -- it seems like an implementation detail 
of `createInvocationFromCommandLine` that the TODO suggests should be changed.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:206
   void expectTreeDumpEqual(StringRef Code, StringRef Tree,
bool RunWithDelayedTemplateParsing = true) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));

eduucaldas wrote:
> is RunWithDelayedTemplateParsing being used?
Indeed not -- removed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80822



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

[clang] 44f989e - Run syntax tree tests in many language modes

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2020-06-02T10:30:01+02:00
New Revision: 44f989e7809633f13bd0420cc1d79660ad982173

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

LOG: Run syntax tree tests in many language modes

Reviewers: hlopko, eduucaldas

Reviewed By: hlopko, eduucaldas

Subscribers: gribozavr2, mgorny, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/unittests/Tooling/Syntax/CMakeLists.txt
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/unittests/Tooling/Syntax/CMakeLists.txt 
b/clang/unittests/Tooling/Syntax/CMakeLists.txt
index 5de7d24e2675..62d1ecc81cad 100644
--- a/clang/unittests/Tooling/Syntax/CMakeLists.txt
+++ b/clang/unittests/Tooling/Syntax/CMakeLists.txt
@@ -14,6 +14,7 @@ clang_target_link_libraries(SyntaxTests
   clangFrontend
   clangLex
   clangSerialization
+  clangTesting
   clangTooling
   clangToolingCore
   clangToolingSyntax

diff  --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp 
b/clang/unittests/Tooling/Syntax/TreeTest.cpp
index 04786257c434..705cb872c6a4 100644
--- a/clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Testing/CommandLineArgs.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Syntax/BuildTree.h"
 #include "clang/Tooling/Syntax/Mutations.h"
@@ -25,6 +26,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
@@ -45,12 +47,71 @@ static llvm::ArrayRef tokens(syntax::Node 
*N) {
 T->lastLeaf()->token() + 1);
 }
 
-class SyntaxTreeTest : public ::testing::Test {
+struct TestClangConfig {
+  TestLanguage Language;
+  std::string Target;
+
+  bool isCXX() const {
+return Language == Lang_CXX || Language == Lang_CXX11 ||
+   Language == Lang_CXX14 || Language == Lang_CXX17 ||
+   Language == Lang_CXX2a;
+  }
+
+  bool isCXX11OrLater() const {
+return Language == Lang_CXX11 || Language == Lang_CXX14 ||
+   Language == Lang_CXX17 || Language == Lang_CXX2a;
+  }
+
+  bool hasDelayedTemplateParsing() const {
+return Target == "x86_64-pc-win32-msvc";
+  }
+
+  std::vector getCommandLineArgs() const {
+std::vector Result = getCommandLineArgsForTesting(Language);
+Result.push_back("-target");
+Result.push_back(Target);
+return Result;
+  }
+
+  std::string toString() const {
+std::string Result;
+llvm::raw_string_ostream OS(Result);
+OS << "{ Language=" << Language << ", Target=" << Target << " }";
+return OS.str();
+  }
+
+  friend std::ostream &operator<<(std::ostream &OS,
+  const TestClangConfig &ClangConfig) {
+return OS << ClangConfig.toString();
+  }
+
+  static std::vector &allConfigs() {
+static std::vector all_configs = []() {
+  std::vector all_configs;
+  for (TestLanguage lang : {Lang_C, Lang_C89, Lang_CXX, Lang_CXX11,
+Lang_CXX14, Lang_CXX17, Lang_CXX2a}) {
+TestClangConfig config;
+config.Language = lang;
+config.Target = "x86_64-pc-linux-gnu";
+all_configs.push_back(config);
+
+// Windows target is interesting to test because it enables
+// `-fdelayed-template-parsing`.
+config.Target = "x86_64-pc-win32-msvc";
+all_configs.push_back(config);
+  }
+  return all_configs;
+}();
+return all_configs;
+  }
+};
+
+class SyntaxTreeTest : public ::testing::Test,
+   public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *
-  buildTree(llvm::StringRef Code,
-const std::string &Target = "x86_64-pc-linux-gnu") {
+  syntax::TranslationUnit *buildTree(llvm::StringRef Code,
+ const TestClangConfig &ClangConfig) {
 // FIXME: this code is almost the identical to the one in TokensTest. Share
 //it.
 class BuildSyntaxTree : public ASTConsumer {
@@ -105,11 +166,19 @@ class SyntaxTreeTest : public ::testing::Test {
diag::Severity::Ignored, SourceLocation());
 
 // Prepare to run a compiler.
-std::vector Args = {
-"syntax-test", "-target",   Target.c_str(),
-FileName,  "-fsyntax-only", "-std=c++17",
+std::vector Args = {
+"syntax-test",
+"-fsyntax-only",

[clang] 97b8dab - [AST] Fix a null initializer crash for InitListExpr

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-06-02T10:48:48+02:00
New Revision: 97b8dabba5c5c1d799bd8b6856d4a81360361769

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

LOG: [AST] Fix a null initializer crash for InitListExpr

Summary:
The Initializer of a InitListExpr can be reset to null, which leads to
nullptr-acces crashes.

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/SemaInit.cpp
clang/test/AST/ast-dump-recovery.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 56d7ac8dde50..0a98cb2a5ce0 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -1638,7 +1638,7 @@ void InitListChecker::CheckReferenceType(const 
InitializedEntity &Entity,
 
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
-  if (!VerifyOnly)
+  if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
   if (hadError)

diff  --git a/clang/test/AST/ast-dump-recovery.cpp 
b/clang/test/AST/ast-dump-recovery.cpp
index 9b13f4d3e06b..a212ff41c0c9 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -181,3 +181,14 @@ void InitializerForAuto() {
 // Verified that the generated call operator is invalid.
 // CHECK: |-CXXMethodDecl {{.*}} invalid operator() 'auto () const -> auto'
 using Escape = decltype([] { return undef(); }());
+
+// CHECK:  VarDecl {{.*}} NoCrashOnInvalidInitList
+// CHECK-NEXT: `-RecoveryExpr {{.*}} '' contains-errors lvalue
+// CHECK-NEXT:   `-InitListExpr
+// CHECK-NEXT: `-DesignatedInitExpr {{.*}} 'void'
+// CHECK-NEXT:   `-CXXNullPtrLiteralExpr {{.*}} 'nullptr_t'
+struct {
+  int& abc;
+} NoCrashOnInvalidInitList = {
+  .abc = nullptr,
+};



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

Re: [PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

[Sam McCall via cfe-commits]

On Tue, Jun 2, 2020 at 10:38 AM Kadir Çetinkaya  wrote:

> Hi Jan,
>
> I don't think there's much point in running ReplayPreamble with an empty
> preamble, but this should already be a no-op as there can't be any includes
> inside the preamble region if size is 0.
>
> I can't seem to reproduce a failure with the root causes you've provided.
> Even when ReplayPreamble::create doesn't take the early exit, for an empty
> preamble we should not have any includes, hence ReplayPreamble::replay
> would be a no-op. That's what I am getting while running the tests (you can
> check this by printing the MainFileIncludes in ReplayPreamble::create
> before early exit. Note that PatchesAdditionalIncludes builds two ASTs, the
> first one will have additional includes as it builds a full AST with a
> non-empty preamble, but the second AST should be built with an empty
> preamble and empty preamble-includes)
>
Yeah, this sounds suspicious to me.
In that loop just before the failed assert, can you dump the details of
`Inc`?
Specifically `Written`, `Resolved`, and HashOffset, and ideally work out
what buffer HashOffset really indexes into.


>
> It seems like something else is going on here, any chance you are
> inserting an implicit include inside your custom PP logic? If that's the
> case we should look for a fix in include extraction logic as it shouldn't
> pick up includes that are coming from implicit(more over non-main-file)
> sources.
>
> On Tue, Jun 2, 2020 at 8:09 AM Jan Korous via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> jkorous added a comment.
>>
>> Hi @kadircet!
>> I am investigating a failure of `PatchesAdditionalIncludes` test that we
>> got internally. It's a failed assert in `ReplayPreamble::replay`.
>> Our clangd source code is for all practical purposes identical to
>> upstream version but not so with clang source. Specifically what we do is
>> that in `CompilerInstance::createPreprocessor` we always add one particular
>> callback.
>> This means that when in the test we are calling `buildPreamble` for
>> `TestTU TU` with empty buffer we never hit the early return in
>> `ReplayPreamble::attach()` (ParsedAST.cpp:124) like upstream version does
>> and proceed to create a new `ReplayPreamble` object with `PreambleBounds`
>> of `size() == 0` which leads to `ReplayPreamble::MainFileTokens` to be
>> empty and later we hit failed assert in `ReplayPreamble::replay` about
>> `assert(HashTok != MainFileTokens.end() && ...)`.
>>
>> Now, while I can just tweak either `ReplayPreamble::attach()` or remove
>> the PP callback in the test internally I am wondering whether you consider
>> empty preamble & PP with callbacks valid use-case of `ReplayPreamble` and
>> worth a fix.
>>
>> This is where we are creating the empty `MainFileTokens`:
>>
>>   * frame #0: 0x000103337649 ClangdTests` clang::clangd::(anonymous
>> namespace)::ReplayPreamble::ReplayPreamble(this=0x000114f45da0,
>> Includes=0x7ffeefbfc678, Delegate=0x000114f3bd80,
>> SM=0x000114f3dd80, PP=0x000115850218, LangOpts=0x000114f38eb0,
>> PB=0x7ffeefbfc658)  + 169 at ParsedAST.cpp:142
>> frame #1: 0x00010333756d ClangdTests` clang::clangd::(anonymous
>> namespace)::ReplayPreamble::ReplayPreamble(this=0x000114f45da0,
>> Includes=0x7ffeefbfc678, Delegate=0x000114f3bd80,
>> SM=0x000114f3dd80, PP=0x000115850218, LangOpts=0x000114f38eb0,
>> PB=0x7ffeefbfc658)  + 77 at ParsedAST.cpp:139
>> frame #2: 0x000103334fa7 ClangdTests` clang::clangd::(anonymous
>> namespace)::ReplayPreamble::attach(Includes=0x7ffeefbfc678,
>> Clang=0x000114f33ea0, PB=0x7ffeefbfc658)  + 183 at ParsedAST.cpp:126
>> frame #3: 0x000103334189 ClangdTests`
>> clang::clangd::ParsedAST::build(Filename=(Data = "/clangd-test/foo.cpp",
>> Length = 20), Inputs=0x7ffeefbfdf98, CI=nullptr,
>> CompilerInvocationDiags=ArrayRef @ 0x7ffeefbfd830,
>> Preamble=std::__1::shared_ptr> clang::clangd::PreambleData>::element_type @ 0x000114f3e728 strong=2
>> weak=1)  + 3897 at ParsedAST.cpp:385
>> frame #4: 0x0001006f1032 ClangdTests` clang::clangd::(anonymous
>> namespace)::ParsedASTTest_PatchesAdditionalIncludes_Test::TestBody(this=0x000114f04090)
>> + 1778 at ParsedASTTests.cpp:477
>>
>> This is the failed assert:
>>
>>   * frame #4: 0x000103337a0c ClangdTests` clang::clangd::(anonymous
>> namespace)::ReplayPreamble::replay(this=0x000114f45da0)  + 556 at
>> ParsedAST.cpp:182
>> frame #5: 0x00010333777c ClangdTests` clang::clangd::(anonymous
>> namespace)::ReplayPreamble::FileChanged(this=0x000114f45da0, Loc=(ID =
>> 74), Reason=ExitFile, Kind=C_User, PrevFID=(ID = 2))  + 156 at
>> ParsedAST.cpp:166
>> frame #6: 0x000101b7900a ClangdTests`
>> clang::PPChainedCallbacks::FileChanged(this=0x000116204080, Loc=(ID =
>> 74), Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 90 at
>> PPCallbacks.h:390
>> frame #7: 0x000101b79046 ClangdTests`
>> clang::P

[PATCH] D80981: [AST] Fix a crash on accessing a class without definition in constexpr function context.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4320
+if (!RD->hasDefinition())
+  return APValue();
 APValue Struct(APValue::UninitStruct(), RD->getNumBases(),

This doesn't look all that safe - you're using a `None` value to indicate 
failure, but no current code path does that and none of the callers seem to 
check for failure.
(e.g. `evaluateVarDecl` returns true instead of false).
Presumably we're going to get a diagnostic somewhere (though it's not 
completely obvious to me) but can we be sure we won't assume this value has the 
right type somewhere down the line?

I get the feeling this is correct and I don't have enough context to understand 
why... how about you :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80981



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

[PATCH] D80986: [clangd] Prototype for postfix completion.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
hokein edited the summary of this revision.

This is a quick proof-of-concept demo for postfix completion.

demo (ignore the wrong highlightings in VSCode):

F12057429: zII8TJLJQ5.gif 

The big question: Where should we implement? in clangd, or in Sema?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80986

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/Sema/SemaCodeComplete.cpp

Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5145,6 +5145,7 @@
   }
 
   CodeCompletionContext CCContext(contextKind, ConvertedBaseType);
+  CCContext.setBaseExpr(Base);
   CCContext.setPreferredType(PreferredType);
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(), CCContext,
Index: clang/include/clang/Sema/CodeCompleteConsumer.h
===
--- clang/include/clang/Sema/CodeCompleteConsumer.h
+++ clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -351,6 +351,8 @@
   /// The type of the base object in a member access expression.
   QualType BaseType;
 
+  Expr *BaseExpr = nullptr;
+
   /// The identifiers for Objective-C selector parts.
   ArrayRef SelIdents;
 
@@ -394,6 +396,8 @@
   /// Retrieve the type of the base object in a member-access
   /// expression.
   QualType getBaseType() const { return BaseType; }
+  void setBaseExpr(Expr *E) { BaseExpr = E; }
+  Expr *getBaseExpr() const { return BaseExpr; }
 
   /// Retrieve the Objective-C selector identifiers.
   ArrayRef getSelIdents() const { return SelIdents; }
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -158,6 +158,19 @@
   unsigned References; // # of usages in file.
 };
 
+struct PostfixCompletionResult {
+  // Display string, not inserted.
+  std::string Signature;
+  // The range being replaced by ReplacedText.
+  // e.g.
+  //  is_success.if
+  //  ~
+  Range ReplacedRange;
+  // The snippet being inserted when doing the completion.
+  // e.g. if (is_success) {}
+  std::string SnippetText;
+};
+
 /// A code completion result, in clang-native form.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
 struct CompletionCandidate {
@@ -166,6 +179,7 @@
   const CodeCompletionResult *SemaResult = nullptr;
   const Symbol *IndexResult = nullptr;
   const RawIdentifier *IdentifierResult = nullptr;
+  const PostfixCompletionResult *PostfixResult = nullptr;
   llvm::SmallVector RankedIncludeHeaders;
 
   // Returns a token identifying the overload set this is part of.
@@ -206,7 +220,7 @@
   return llvm::hash_combine(Scratch,
 headerToInsertIfAllowed(Opts).getValueOr(""));
 }
-assert(IdentifierResult);
+assert(IdentifierResult || PostfixResult);
 return 0;
   }
 
@@ -316,6 +330,11 @@
   Completion.Kind = CompletionItemKind::Text;
   Completion.Name = std::string(C.IdentifierResult->Name);
 }
+if (C.PostfixResult) {
+  Completion.Signature = C.PostfixResult->Signature;
+  Completion.Kind = CompletionItemKind::Snippet;
+  Completion.SnippetSuffix = C.PostfixResult->SnippetText;
+}
 
 // Turn absolute path into a literal string that can be #included.
 auto Inserted = [&](llvm::StringRef Header)
@@ -371,6 +390,9 @@
   S.Signature = std::string(C.IndexResult->Signature);
   S.SnippetSuffix = std::string(C.IndexResult->CompletionSnippetSuffix);
   S.ReturnType = std::string(C.IndexResult->ReturnType);
+} else if (C.PostfixResult) {
+  S.SnippetSuffix = C.PostfixResult->SnippetText;
+  S.Signature = C.PostfixResult->Signature;
 }
 if (ExtractDocumentation && !Completion.Documentation) {
   auto SetDoc = [&](llvm::StringRef Doc) {
@@ -609,6 +631,15 @@
   return {Scopes.scopesForIndexQuery(), false};
 }
 
+bool contextAllowsPostfix(enum CodeCompletionContext::Kind K) {
+  switch (K) {
+  case CodeCompletionContext::CCC_DotMemberAccess:
+return true;
+  default:
+return false;
+  }
+}
+
 // Should we perform index-based completion in a context of the specified kind?
 // FIXME: consider allowing completion, but restricting the result types.
 bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
@@ -721,8 +752,10 @@
 // If a callback is called without any sema result and the context does not
 // support index-based completion, we simply skip it to give way to
 // potential

[PATCH] D80822: Run syntax tree tests in many language modes

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG44f989e78096: Run syntax tree tests in many language modes 
(authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80822

Files:
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  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
@@ -17,6 +17,7 @@
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Testing/CommandLineArgs.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Syntax/BuildTree.h"
 #include "clang/Tooling/Syntax/Mutations.h"
@@ -25,6 +26,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
@@ -45,12 +47,71 @@
 T->lastLeaf()->token() + 1);
 }
 
-class SyntaxTreeTest : public ::testing::Test {
+struct TestClangConfig {
+  TestLanguage Language;
+  std::string Target;
+
+  bool isCXX() const {
+return Language == Lang_CXX || Language == Lang_CXX11 ||
+   Language == Lang_CXX14 || Language == Lang_CXX17 ||
+   Language == Lang_CXX2a;
+  }
+
+  bool isCXX11OrLater() const {
+return Language == Lang_CXX11 || Language == Lang_CXX14 ||
+   Language == Lang_CXX17 || Language == Lang_CXX2a;
+  }
+
+  bool hasDelayedTemplateParsing() const {
+return Target == "x86_64-pc-win32-msvc";
+  }
+
+  std::vector getCommandLineArgs() const {
+std::vector Result = getCommandLineArgsForTesting(Language);
+Result.push_back("-target");
+Result.push_back(Target);
+return Result;
+  }
+
+  std::string toString() const {
+std::string Result;
+llvm::raw_string_ostream OS(Result);
+OS << "{ Language=" << Language << ", Target=" << Target << " }";
+return OS.str();
+  }
+
+  friend std::ostream &operator<<(std::ostream &OS,
+  const TestClangConfig &ClangConfig) {
+return OS << ClangConfig.toString();
+  }
+
+  static std::vector &allConfigs() {
+static std::vector all_configs = []() {
+  std::vector all_configs;
+  for (TestLanguage lang : {Lang_C, Lang_C89, Lang_CXX, Lang_CXX11,
+Lang_CXX14, Lang_CXX17, Lang_CXX2a}) {
+TestClangConfig config;
+config.Language = lang;
+config.Target = "x86_64-pc-linux-gnu";
+all_configs.push_back(config);
+
+// Windows target is interesting to test because it enables
+// `-fdelayed-template-parsing`.
+config.Target = "x86_64-pc-win32-msvc";
+all_configs.push_back(config);
+  }
+  return all_configs;
+}();
+return all_configs;
+  }
+};
+
+class SyntaxTreeTest : public ::testing::Test,
+   public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *
-  buildTree(llvm::StringRef Code,
-const std::string &Target = "x86_64-pc-linux-gnu") {
+  syntax::TranslationUnit *buildTree(llvm::StringRef Code,
+ const TestClangConfig &ClangConfig) {
 // FIXME: this code is almost the identical to the one in TokensTest. Share
 //it.
 class BuildSyntaxTree : public ASTConsumer {
@@ -105,11 +166,19 @@
diag::Severity::Ignored, SourceLocation());
 
 // Prepare to run a compiler.
-std::vector Args = {
-"syntax-test", "-target",   Target.c_str(),
-FileName,  "-fsyntax-only", "-std=c++17",
+std::vector Args = {
+"syntax-test",
+"-fsyntax-only",
 };
-Invocation = createInvocationFromCommandLine(Args, Diags, FS);
+llvm::copy(ClangConfig.getCommandLineArgs(), std::back_inserter(Args));
+Args.push_back(FileName);
+
+std::vector ArgsCStr;
+for (const std::string &arg : Args) {
+  ArgsCStr.push_back(arg.c_str());
+}
+
+Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
 assert(Invocation);
 Invocation->getFrontendOpts().DisableFree = false;
 Invocation->getPreprocessorOpts().addRemappedFile(
@@ -133,32 +202,15 @@
 return Root;
   }
 
-  void expectTreeDumpEqual(StringRef Code, StringRef Tree,
-   bool RunWithDelayedTemplateParsing = true) {
+  void expectTreeDumpEqual(StringRef Code, StringRef Tree) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
 SCOPED_TRACE(Code);
 
-std::string Expected = Tree.trim().str();
-
-  

[PATCH] D80980: [AST] Fix a null initializer crash for InitListExpr

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG97b8dabba5c5: [AST] Fix a null initializer crash for 
InitListExpr (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80980

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/AST/ast-dump-recovery.cpp


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -181,3 +181,14 @@
 // Verified that the generated call operator is invalid.
 // CHECK: |-CXXMethodDecl {{.*}} invalid operator() 'auto () const -> auto'
 using Escape = decltype([] { return undef(); }());
+
+// CHECK:  VarDecl {{.*}} NoCrashOnInvalidInitList
+// CHECK-NEXT: `-RecoveryExpr {{.*}} '' contains-errors lvalue
+// CHECK-NEXT:   `-InitListExpr
+// CHECK-NEXT: `-DesignatedInitExpr {{.*}} 'void'
+// CHECK-NEXT:   `-CXXNullPtrLiteralExpr {{.*}} 'nullptr_t'
+struct {
+  int& abc;
+} NoCrashOnInvalidInitList = {
+  .abc = nullptr,
+};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1638,7 +1638,7 @@
 
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
-  if (!VerifyOnly)
+  if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
   if (hadError)


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -181,3 +181,14 @@
 // Verified that the generated call operator is invalid.
 // CHECK: |-CXXMethodDecl {{.*}} invalid operator() 'auto () const -> auto'
 using Escape = decltype([] { return undef(); }());
+
+// CHECK:  VarDecl {{.*}} NoCrashOnInvalidInitList
+// CHECK-NEXT: `-RecoveryExpr {{.*}} '' contains-errors lvalue
+// CHECK-NEXT:   `-InitListExpr
+// CHECK-NEXT: `-DesignatedInitExpr {{.*}} 'void'
+// CHECK-NEXT:   `-CXXNullPtrLiteralExpr {{.*}} 'nullptr_t'
+struct {
+  int& abc;
+} NoCrashOnInvalidInitList = {
+  .abc = nullptr,
+};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1638,7 +1638,7 @@
 
   expr = Result.getAs();
   // FIXME: Why are we updating the syntactic init list?
-  if (!VerifyOnly)
+  if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
   if (hadError)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74299: [clang-tidy] extend tests of run-clang-tidy

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin added a comment.

*ping*


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

https://reviews.llvm.org/D74299



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

[PATCH] D80508: [AST] Fix the source range for TagDecl if there is no matched } brace.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Decl.cpp:4129
 SourceRange TagDecl::getSourceRange() const {
-  SourceLocation RBraceLoc = BraceRange.getEnd();
-  SourceLocation E = RBraceLoc.isValid() ? RBraceLoc : getLocation();
+  SourceLocation E = BraceRange.getBegin().isValid() ?
+ BraceRange.getEnd() : getLocation();

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > I'm confused about this change.
> > > 
> > > We were using BR.end if it was valid, now we're using it if BR.start is 
> > > valid. So this changes behavior in two cases:
> > >  - BR.start is valid, BR.end is invalid: old was {getOuterLocStart(), 
> > > getLocation()}. new is {getOuterLocStart(), invalid}
> > >  - BR.end is valid, BR.start is invalid: old was {getOuterLocStart(), 
> > > BraceRange.getEnd()}. new is {getOuterLocStart(), getLocation()}
> > > 
> > > These both seem worse to me, what am I missing?
> > > 
> > > Patch description refers to the first case, I wonder if this is just a 
> > > bug in clangd cancelling out a bad AST (does it just mark tokens until 
> > > EOF if the end location is invalid?)
> > > We were using BR.end if it was valid, now we're using it if BR.start is 
> > > valid. So this changes behavior in two cases:
> > 
> > > BR.start is valid, BR.end is invalid: old was {getOuterLocStart(), 
> > > getLocation()}. new is {getOuterLocStart(), invalid}
> > > BR.end is valid, BR.start is invalid: old was {getOuterLocStart(), 
> > > BraceRange.getEnd()}. new is {getOuterLocStart(), getLocation()}
> > 
> > case 1), I think this is intended, though it looks like worse at first 
> > glance :( Invalid source location has subtle semantic in clang -- "BR.end() 
> > is invalid" implies that there is no `}` in the source code.
> > 
> > And  `Missing-}`  namespaceDecl in today's clang is using invalid loc (not 
> > the NameLoc) as the end loc:
> > 
> > ```
> > // `-NamespaceDecl 0x8d27728 > col:11 abc
> > namespace abc {
> > // missing }
> > ```
> > 
> > case 2) is less interesting, I wonder whether we will encounter this case 
> > in practice, tried and failed to come up with an example to it happen. I 
> > think we can preserve the old behavior in this patch.
> > 
> > 
> > > I wonder if this is just a bug in clangd cancelling out a bad AST (does 
> > > it just mark tokens until EOF if the end location is invalid?)
> > 
> > I assume "clangd" you mean "clang"? I think clang's behavior is fine, the 
> > AST is recovered well, it is an issue that the TagDecl::getSourceRange 
> > doesn't correctly claim the range of TagDecl.
> > 
> > e.g. 
> > ```
> > class Test {
> >   int field;
> > // missing }
> > // `-CXXRecordDecl 0x96dd738  col:7 class Test 
> > definition
> > //   |-CXXRecordDecl 0x96dd878  col:7 implicit class Test
> > //   `-FieldDecl 0x96dd940  col:7 field 'int'
> > ```
> > 
> > And we may not set the BraceRange.end to the EOF position -- as it changes 
> > the semantic, making it valid implies we have a `}` in the source code.
> > 
> No, I meant clangd. Invalid source ranges hit codepaths in Selection.cpp that 
> isn't deliberately handling them, and we're getting "random" results that may 
> happen to do what you want in some cases, but...
> 
> e.g. mayHit returns true if the range isn't valid, which is probably good, 
> but the reason is bizarre: it thinks the range is from a macro expansion.
> 
> However claimRange seems to claim no tokens. So I'd guess hover, find 
> references etc on ABC wouldn't work after this patch, so there's regressions 
> there too.
> 
> Still even if it breaks clangd, the most important question here is how 
> *should* the source range be represented. Is a half-broken range actually 
> useful? Maybe it's namespacedecl that should be "fixed".
ok, that's fair enough, so the current situation is 

- braceRange has its semantics, we should keep the EndLoc as Invalid;
- getSourceRange() returns a wrong range ({getOuterLocStart(), getLocation()}, 
{getOuterLocStart(), invalid} are not ideal), which doesn't reflect the AST;

Idea: we can add a new member `EndOfLoc` in TagDecl, and getSourceRange returns 
{getOuterLocStart(), EndOfLoc}, but this will increase the TagDecl size by 8 
bytes, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80508



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

[clang] a6a42df - [Sema] Fix -Wunused-variable in CreateBuiltinMatrixSubscriptExpr (NFC).

[Florian Hahn via cfe-commits]

Author: Florian Hahn
Date: 2020-06-02T10:45:30+01:00
New Revision: a6a42df506ca93df69725f732c396050060f026f

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

LOG: [Sema] Fix -Wunused-variable in CreateBuiltinMatrixSubscriptExpr (NFC).

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e177c9d4adec..2221f98b943a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4737,6 +4737,7 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr 
*Base, Expr *RowIdx,
 bool ConversionOk = tryConvertToTy(*this, Context.getSizeType(), 
&ConvExpr);
 assert(ConversionOk &&
"should be able to convert any integer type to size type");
+(void)ConversionOk;
 return ConvExpr.get();
   };
 



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

Re: [PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

[Kadir Çetinkaya via cfe-commits]

managed to reproduce the issue. sent out https://reviews.llvm.org/D80988
for a fix.

On Tue, Jun 2, 2020 at 12:08 PM Sam McCall  wrote:

> On Tue, Jun 2, 2020 at 10:38 AM Kadir Çetinkaya 
> wrote:
>
>> Hi Jan,
>>
>> I don't think there's much point in running ReplayPreamble with an empty
>> preamble, but this should already be a no-op as there can't be any includes
>> inside the preamble region if size is 0.
>>
>> I can't seem to reproduce a failure with the root causes you've provided.
>> Even when ReplayPreamble::create doesn't take the early exit, for an empty
>> preamble we should not have any includes, hence ReplayPreamble::replay
>> would be a no-op. That's what I am getting while running the tests (you can
>> check this by printing the MainFileIncludes in ReplayPreamble::create
>> before early exit. Note that PatchesAdditionalIncludes builds two ASTs, the
>> first one will have additional includes as it builds a full AST with a
>> non-empty preamble, but the second AST should be built with an empty
>> preamble and empty preamble-includes)
>>
> Yeah, this sounds suspicious to me.
> In that loop just before the failed assert, can you dump the details of
> `Inc`?
> Specifically `Written`, `Resolved`, and HashOffset, and ideally work out
> what buffer HashOffset really indexes into.
>
>
>>
>> It seems like something else is going on here, any chance you are
>> inserting an implicit include inside your custom PP logic? If that's the
>> case we should look for a fix in include extraction logic as it shouldn't
>> pick up includes that are coming from implicit(more over non-main-file)
>> sources.
>>
>> On Tue, Jun 2, 2020 at 8:09 AM Jan Korous via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> jkorous added a comment.
>>>
>>> Hi @kadircet!
>>> I am investigating a failure of `PatchesAdditionalIncludes` test that we
>>> got internally. It's a failed assert in `ReplayPreamble::replay`.
>>> Our clangd source code is for all practical purposes identical to
>>> upstream version but not so with clang source. Specifically what we do is
>>> that in `CompilerInstance::createPreprocessor` we always add one particular
>>> callback.
>>> This means that when in the test we are calling `buildPreamble` for
>>> `TestTU TU` with empty buffer we never hit the early return in
>>> `ReplayPreamble::attach()` (ParsedAST.cpp:124) like upstream version does
>>> and proceed to create a new `ReplayPreamble` object with `PreambleBounds`
>>> of `size() == 0` which leads to `ReplayPreamble::MainFileTokens` to be
>>> empty and later we hit failed assert in `ReplayPreamble::replay` about
>>> `assert(HashTok != MainFileTokens.end() && ...)`.
>>>
>>> Now, while I can just tweak either `ReplayPreamble::attach()` or remove
>>> the PP callback in the test internally I am wondering whether you consider
>>> empty preamble & PP with callbacks valid use-case of `ReplayPreamble` and
>>> worth a fix.
>>>
>>> This is where we are creating the empty `MainFileTokens`:
>>>
>>>   * frame #0: 0x000103337649 ClangdTests` clang::clangd::(anonymous
>>> namespace)::ReplayPreamble::ReplayPreamble(this=0x000114f45da0,
>>> Includes=0x7ffeefbfc678, Delegate=0x000114f3bd80,
>>> SM=0x000114f3dd80, PP=0x000115850218, LangOpts=0x000114f38eb0,
>>> PB=0x7ffeefbfc658)  + 169 at ParsedAST.cpp:142
>>> frame #1: 0x00010333756d ClangdTests` clang::clangd::(anonymous
>>> namespace)::ReplayPreamble::ReplayPreamble(this=0x000114f45da0,
>>> Includes=0x7ffeefbfc678, Delegate=0x000114f3bd80,
>>> SM=0x000114f3dd80, PP=0x000115850218, LangOpts=0x000114f38eb0,
>>> PB=0x7ffeefbfc658)  + 77 at ParsedAST.cpp:139
>>> frame #2: 0x000103334fa7 ClangdTests` clang::clangd::(anonymous
>>> namespace)::ReplayPreamble::attach(Includes=0x7ffeefbfc678,
>>> Clang=0x000114f33ea0, PB=0x7ffeefbfc658)  + 183 at ParsedAST.cpp:126
>>> frame #3: 0x000103334189 ClangdTests`
>>> clang::clangd::ParsedAST::build(Filename=(Data = "/clangd-test/foo.cpp",
>>> Length = 20), Inputs=0x7ffeefbfdf98, CI=nullptr,
>>> CompilerInvocationDiags=ArrayRef @ 0x7ffeefbfd830,
>>> Preamble=std::__1::shared_ptr>> clang::clangd::PreambleData>::element_type @ 0x000114f3e728 strong=2
>>> weak=1)  + 3897 at ParsedAST.cpp:385
>>> frame #4: 0x0001006f1032 ClangdTests` clang::clangd::(anonymous
>>> namespace)::ParsedASTTest_PatchesAdditionalIncludes_Test::TestBody(this=0x000114f04090)
>>> + 1778 at ParsedASTTests.cpp:477
>>>
>>> This is the failed assert:
>>>
>>>   * frame #4: 0x000103337a0c ClangdTests` clang::clangd::(anonymous
>>> namespace)::ReplayPreamble::replay(this=0x000114f45da0)  + 556 at
>>> ParsedAST.cpp:182
>>> frame #5: 0x00010333777c ClangdTests` clang::clangd::(anonymous
>>> namespace)::ReplayPreamble::FileChanged(this=0x000114f45da0, Loc=(ID =
>>> 74), Reason=ExitFile, Kind=C_User, PrevFID=(ID = 2))  + 156 at
>>> ParsedAST.cpp:166
>>> frame #6: 0x000101b7900a Cl

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:114-115
 // Convert the array length to size_t.
 NonLoc IndexLength =
 SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs();
 // Multiply the array length by the element size.

balazske wrote:
> vabridgers wrote:
> > NoQ wrote:
> > > Do i understand correctly that this cast is the only difference between 
> > > the value that has been checked and the value on which the assertion is 
> > > asserted?
> > Yes, looks that way to me. Let's see if Balasz, Gabor, Adam or Kristof 
> > responds in the next day or two? Thanks Artem!
> Yes the cast is the difference. Even if this problem is fixed (cast included 
> in `checkVLAIndexSize`) the same problem happens.
> The following code (in `checkVLAIndexSize`) prints `(reg_$0) + 1 0` 
> before the crash. So there is some difference between `assume` and 
> `getKnownValue`. It can be better to include check of `getKnownValue` in 
> `checkVLAIndexSize` (at least for zero value).
> ```lang=c++
>   // Convert the array length to size_t.
>   NonLoc SizeNL =
>   SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs();
> 
>   // Check if the size is zero.
>   ProgramStateRef StateNotZero, StateZero;
>   std::tie(StateNotZero, StateZero) = State->assume(SizeNL);
> 
>   if (StateZero && !StateNotZero) {
> reportBug(VLA_Zero, SizeE, StateZero, C);
> return nullptr;
>   }
> 
>   // From this point on, assume that the size is not zero.
>   State = StateNotZero;
> 
>   if (const llvm::APSInt *IndexLVal = SVB.getKnownValue(State, SizeNL)) {
> uint64_t L = IndexLVal->getZExtValue();
> llvm::errs() << SizeNL << " " << L << "\n";
> assert(L != 0);
>   }
> 
> ```
It might be that `simplifySVal` isn't applied consistently. Like, `assume()` 
fails to apply it so it fails to see that the value is in fact concrete zero.

Let's remove the assertion for now and add a FIXME to investigate why can't we 
add it back. Like, there's nothing preventing us from investigating it now but 
it will take some time whereas crashes are already there and we should fix them 
to unblock other people.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903



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

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

[Florian Hahn via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:4738-4739
+bool ConversionOk = tryConvertToTy(*this, Context.getSizeType(), 
&ConvExpr);
+assert(ConversionOk &&
+   "should be able to convert any integer type to size type");
+return ConvExpr.get();

nickdesaulniers wrote:
> In a release build:
> ```
> llvm-project/clang/lib/Sema/SemaExpr.cpp:4737:10: warning: unused variable 
> 'ConversionOk' [-Wunused-variable]
> bool ConversionOk = tryConvertToTy(*this, Context.getSizeType(), 
> &ConvExpr);
>  ^
> ```
Oh, of course, thanks for letting me know! Should be fixed in 
a6a42df506ca93df69725f732c396050060f026f


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76791



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

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a reviewer: vrnithinkumar.
NoQ added a comment.

+Nithin because it may be relevant to the smart pointer checker if 
inter-checker communication turns out to be necessary (eg., with the move 
checker).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80905



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

[PATCH] D80517: [analyzer] CmpRuns.py: Refactor and add type annotations

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I guess this one's stuck on the discussion about whether we need to retain 
python2 support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80517



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

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added reviewers: vrnithinkumar, vsavchenko, xazax.hun.
NoQ added a comment.

+GSoC gang because it'll ultimately help Nithin's future checker track smart 
pointers returned from functions.


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

https://reviews.llvm.org/D80366



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

[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added reviewers: Eugene.Zelenko, thegameg.
DmitryPolukhin marked an inline comment as done.
DmitryPolukhin added subscribers: thegameg, Eugene.Zelenko.
DmitryPolukhin added a comment.

+ @gribozavr, @Eugene.Zelenko, @thegameg who touched/reviewed this code, please 
take a look.




Comment at: llvm/lib/Support/YAMLTraits.cpp:904
+   std::string &Val) {
+  Val.clear();
+  size_t CurrentPos = 0;

mgehre wrote:
> I wonder whether using StringRef::split() would lead to an easier 
> implementation 
> (https://llvm.org/doxygen/classllvm_1_1StringRef.html#af0284e4c41c0e09c0bc4767bc77a899d)
I'm not sure that it will be easier to read or more efficient 
(`StringRef::split` will require additional vector).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80301



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

[PATCH] D80988: [clangd] Copy existing includes in ReplayPreamble

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: sammccall, jkorous.
Herald added subscribers: cfe-commits, usaxena95, arphaman, dexonsmith, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

ReplayPreamble was just grabbing the reference of IncludeStructure
passed to it and then replayed any includes seen so while exiting
built-in file.

This implies any include seen in built-in files being replayed as part
of preamble, even though they are not. This wasn't an issue until we've
started patching preambles, as includes from built-in files were not
mapped back to main-file.

This patch copies over existing includes at the time of
ReplayPreamble::attach and only replies those to prevent any includes
from the preamble patch getting mixed-in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80988

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


Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
+MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
+
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   struct Inclusion {
 Inclusion(const SourceManager &SM, SourceLocation HashLoc,
@@ -439,6 +441,24 @@
 Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
 EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
+
+  // Make sure replay logic works with patched preambles.
+  TU.Code = "";
+  StoreDiags Diags;
+  auto Inputs = TU.inputs();
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  auto EmptyPreamble =
+  buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr);
+  ASSERT_TRUE(EmptyPreamble);
+  TU.Code = "#include ";
+  Includes.clear();
+  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
+ std::move(CI), {}, EmptyPreamble);
+  ASSERT_TRUE(PatchedAST);
+  // Make sure includes were seen only once.
+  EXPECT_THAT(Includes,
+  ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
+  WithFileName("a.h")));
 }
 
 TEST(ParsedASTTest, PatchesAdditionalIncludes) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -136,7 +136,8 @@
   ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
  const SourceManager &SM, Preprocessor &PP,
  const LangOptions &LangOpts, const PreambleBounds &PB)
-  : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+  : IncludesToReplay(Includes.MainFileIncludes), Delegate(Delegate), 
SM(SM),
+PP(PP) {
 // Only tokenize the preamble section of the main file, as we are not
 // interested in the rest of the tokens.
 MainFileTokens = syntax::tokenize(
@@ -167,7 +168,7 @@
   }
 
   void replay() {
-for (const auto &Inc : Includes.MainFileIncludes) {
+for (const auto &Inc : IncludesToReplay) {
   const FileEntry *File = nullptr;
   if (Inc.Resolved != "")
 if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +228,7 @@
 }
   }
 
-  const IncludeStructure &Includes;
+  std::vector IncludesToReplay;
   PPCallbacks *Delegate;
   const SourceManager &SM;
   Preprocessor &PP;


Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
+MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
+
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   struct Inclusion {
 Inclusion(const SourceManager &SM, SourceLocation HashLoc,
@@ -439,6 +441,24 @@
 Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
 EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
+
+  // Make sure replay logic works with patched preambles.
+  TU.Code = "";
+  StoreDiags Diags;
+  auto Inputs = TU.inputs();
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  auto EmptyPreamble =
+  buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr);
+  ASSERT_TRUE(EmptyPreamble);
+  TU.Code = "#include ";
+  Includes.clear();
+  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
+ std::move(CI), {}, EmptyPreamble);
+  ASSERT_TRUE(PatchedAST);
+  // Make sure includes were seen only once.
+  EXPECT_THAT(Includes,
+  E

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

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

baloghadamsoftware marked 4 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

aaron.ballman wrote:
> By my reading of the core guideline 
> (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
>  it looks like `n` should also be diagnosed because all of the constructors 
> in the class initialize the member to the same constant value. Is there a 
> reason to deviate from the rule (or have I missed something)?
> 
> Also, I'd like to see a test case like:
> ```
> class C {
>   int n;
> public:
>   C() { n = 0; }
>   explicit C(int) { n = 12; }
> };
> ```
This check only cares for initializations inside the body (rule `C.49`, but if 
the proper fix is to convert them to default member initializer according to 
rule `C.48` then we follow that rule in the fix). For initializations 
implemented as constructor member initializers but according to `C.48` they 
should have been implemented as default member initializers we already have 
check `modernize-use-default-member-init`.


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

https://reviews.llvm.org/D71199



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

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

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

balazske added a comment.

It is no problem to return instead of the assert. I could fix the problem by 
using

  SVal LessThanZeroVal = SVB.evalBinOp(State, BO_LE, SizeD, Zero, SizeTy);

in `checkVLAIndexSize` (`BO_LT` is used before). Still the proposed return 
makes the code more safe.




Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:130
+  // constraints are not solved for expressions with multiple
+  // symbols, so just bail on invalid indices at this point.
+  if (IndexL <= 0)

I do not know if this is accurate reason for the problem, a more general text 
is better? And should start with uppercase.



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:131
+  // symbols, so just bail on invalid indices at this point.
+  if (IndexL <= 0)
+return nullptr;

Can be `==`.



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:134
+
   assert(IndexL > 0 && "Index length should have been checked for zero.");
   if (KnownSize <= SizeMax / IndexL) {

The assert does not look good if there is already the check before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

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

baloghadamsoftware updated this revision to Diff 267833.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,455 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+// CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple2() : n (0) {
+x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+// CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+x = xx;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+// CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+n = nn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+// CHECK-FIXES: Simple4() : n(something_int()) {
+n = something_int();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+// CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+if (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+if (!dice())
+  return;
+m = 1;
+// NO-MESSAGES: initialization of 'm' follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+while (dice())
+  m = 1;
+// NO-MESSAGES: initialization of 'm' is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() : n(0) {
+while (!dice())
+  return;
+m = 1;
+// NO-MESSAGES:

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

We (me, Valeriy, Devin)'ve just been talking about this and mostly of agreed 
that `core.CallAndMessage` should ideally be removed (at least as a 
`Checkers.td` entity) and individual checks moved to the relevant package (null 
dereference, use of uninitialized value, etc.). It won't hurt backwards 
compatibility too much because so far there was no reasonable outcome that one 
might expect from disabling `core.CallAndMessage`, given that it's a weird mix 
of completely unrelated checks. The benefit is that disabling, say, null 
dereference checker will actually disable all null dereference warnings (as 
opposed to having to additionally disable the ones that come from 
`CallAndMessage`).

Also a regular reminder that we wouldn't have had this conversation if we had 
hashtags instead of packages (#CallAndMessage #NullDereference #C++ 
#AllPlatforms #UndefinedBehavior #SecurityCritical #PathSensitive #Stable 
#2012).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77866



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

[PATCH] D80988: [clangd] Copy existing includes in ReplayPreamble

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:386
 // Replay the preamble includes so that clang-tidy checks can see them.
 ReplayPreamble::attach(Includes, *Clang, Preamble->Preamble.getBounds());
   }

it seems like it would be clearer to pass Preamble->preambleIncludes() here, at 
this point?
(And move within the constructor, so still just one copy)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80988



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

[PATCH] D80986: [clangd] Prototype for postfix completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Neat!

> The big question: Where should we implement? in clangd, or in Sema?

Advantages of Sema:

- better access to AST/context so more flexible
- available to tools other than clangd

Advantages of clangd:

- fewer layer of abstraction to traverse
- access to the index, config etc
- more comfortable making calls about how much to assume about stdlib etc
- maybe a bit easier to understand/test (depends on what you're used to)

We could get access to the index or other things from clangd by adding a hook 
to provide these "extended" member completions, and having clangd implement 
it... Not sure if that's best or worst of both worlds.

One question with Sema is how to make it produce the right text edit. Can we 
model it as a RK_Pattern completion with a typo fix to delete the `foo.` before 
the `if`?

This choice would be clearer if we had a set of examples we wanted to 
implement. If it's just if/for, then Sema seems like the right call...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80986



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

[PATCH] D80931: AMDGPU: Fix clang side null pointer value for private

[Christudasan Devadasan via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D80931



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

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:552
+
+  Index = StackFrame->getIndex();
+

Szelethus wrote:
> baloghadamsoftware wrote:
> > Szelethus wrote:
> > > This mustn't be serious. `StackFrameContext::getIndex()` has **no 
> > > comments** at all! So it is an index to the element within a `CFGBlock` 
> > > to which it also stores a field for??? Ugh. Shouldn't we just have a 
> > > `getCallSiteCFGElement` or something? Especially since we have 
> > > `CFGElementRef`s now.
> > > 
> > > Would you be so nice to hunt this down please? :)
> > Do you mean a new `StackFrameContext::getCFGElement()` member function? I 
> > agree. I can do it, however this technology where we get the block and the 
> > index separately is used at many places in the code, then it would be 
> > appropriate to refactor all these places. Even wrose is the backward 
> > direction, where we look for the CFG element of a statement: we find the 
> > block from `CFGStmtMap` and then we search for the index **liearly**, 
> > instrad of storing it in the map as well!!!
> Nasty. Yeah, I mean not to burden you work refactoring any more then you feel 
> like doing it, but simply adding a `StackFrameContext::getCFGElement()` 
> method and using it in this patch would be nice enough, and some comments to 
> `getIndex()`. But this obviously isn't a high prio issue.
+1
I do believe that clearer interface functions and better segregation of 
different functionalities will make it much easier for us in the future



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:553
+
+  if(const auto Ctor = (*Block)[Index].getAs()) {
+return Ctor->getConstructionContext();

nit: space after `if`

And I would also prefer putting `const auto *Ctor`



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:247-249
+  Optional ExistingVal = getObjectUnderConstruction(State, CE, LCtx);
+  if (ExistingVal.hasValue())
+return std::make_pair(State, *ExistingVal);

Maybe here (and further) we can use a widespread pattern of `bool`-castable 
things declared in the `if` condition (like you do with `dyn_casts`):

```
if (Optional ExistingVal = getObjectUnderConstruction(State, CE, LCtx))
  return std::make_pair(State, *ExistingVal);
```



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:345-360
+  if (const auto *CE = dyn_cast(E)) {
+Optional ExistingVal =
+  getObjectUnderConstruction(State, {CE, Idx}, LCtx);
+if (ExistingVal.hasValue())
+  return std::make_pair(State, *ExistingVal);
+  } else if (const auto *CCE = dyn_cast(E)) {
+Optional ExistingVal =

It seems like a code duplication to me.

First of all we can first compose a `ConstructionContextItem` from either `CE`, 
`CCE`, or `ME` and then call for `getObjectUnderConstruction`.
Additionally, I think we can hide even that by introducing an overload for 
`getObjectUnderConstruction` that takes an expression and an index. 



Comment at: 
clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:24
+public:
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+if (!Call.getOriginExpr())

martong wrote:
> I assume this tests this call expression: `returnC(1)` . But this is 
> absolutely not trivial from the test code, could you please make this cleaner?
I think that this function, the meat and bones of the test, should be properly 
commented and state explicitly what are the assumption and what is the expected 
outcome.



Comment at: 
clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:29
+Optional RetVal = Call.getReturnValueUnderConstruction(0);
+assert(RetVal);
+assert(RetVal->getAsRegion());

martong wrote:
> I think it would make the tests cleaner if we had some member variables set 
> here and then in the test function 
> (`addTestReturnValueUnderConstructionChecker`) we had the expectations on 
> those variables.
> Right now, if we face an assertion it kills the whole unit test suite and it 
> is not obvious why that happened.
Why not `gtest`s assertions? Those will also interrupt the execution, but will 
be much clearer in the output.


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

https://reviews.llvm.org/D80366



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

[PATCH] D80876: [clang] Default to windows response files when running on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D80876#2067532 , @rnk wrote:

> This seems wrong for mingw, so check with @mstorsjo.


Thanks for the headsup!

I agree that this change would make sense and would make things more consistent 
in general, but @rnk is also right that, within a mingw environment, the 
expectation would probably be for the old behaviour.

I did one build test with a large link command (done via libtool), where 
libtool produed a response file, containing unix-style-quoted windows paths, 
like `some\\path\\to\\a\\file` - and if I added `--rsp-quoting=windows` to it, 
it still built fine, so while it would be expanded to actually contain double 
backslashes in that case (I presume?) it seemed harmless. So as long as the 
response files contain only relative pathnames, misinterpreting a unix-quoted 
file as windows quoted seems harmless.

But as for keeping the old default in mingw environments, would it make sense 
to, if on windows, check the default target triple, and if that's a mingw 
target (`Triple.isWindowsGNUEnvironment()`), keep the unix style quoting as 
default?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80876



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

[PATCH] D77062: [analyzer] Added check for unacceptable equality operation between Loc and NonLoc types

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:273
   Optional val = V.getAs();
-  if (!val)
-return std::pair(state, state);
+  if (val && !V.getAs()) {
+ // return pair shall be {null, non-null} so reorder states

Basically `SVal::getAs<>` should not be used for discovering the type of the 
value; only the exact representation that's used for the value of the type 
that's already in your possession. Say, it's ok to use it to discriminate 
between, say, compound value and lazy compound value. It's ok to use it to 
discriminate between a concrete integer and a symbol. It's ok to use it do 
discriminate between a known value and an unknown value. But if it's used for 
discriminating between a compound value and a numeric symbol, i'm 99% sure it's 
incorrect. You should already know the type from the AST before you even obtain 
the value. It doesn't make sense to run the checker at all if the function 
receives a structure. And if it doesn't receive the structure but the run-time 
value is of structure type, then either the checker isn't obtaining the value 
correctly or there's bug in path-sensitive analysis. That's why i still believe 
you're only treating the symptoms. There's nothing normal in the situation 
where "strcpy suddenly accepts a structure (or an array) by value".


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

https://reviews.llvm.org/D77062



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

[clang-tools-extra] bff0c56 - [clangd] Copy existing includes in ReplayPreamble

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-06-02T13:34:40+02:00
New Revision: bff0c56ff92106afad22b54a90e1c612da4d6f99

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

LOG: [clangd] Copy existing includes in ReplayPreamble

ReplayPreamble was just grabbing the reference of IncludeStructure
passed to it and then replayed any includes seen so while exiting
built-in file.

This implies any include seen in built-in files being replayed as part
of preamble, even though they are not. This wasn't an issue until we've
started patching preambles, as includes from built-in files were not
mapped back to main-file.

This patch copies over existing includes at the time of
ReplayPreamble::attach and only replies those to prevent any includes
from the preamble patch getting mixed-in.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 082c7cae0021..9eed9bc3199f 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -136,7 +136,8 @@ class ReplayPreamble : private PPCallbacks {
   ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
  const SourceManager &SM, Preprocessor &PP,
  const LangOptions &LangOpts, const PreambleBounds &PB)
-  : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+  : IncludesToReplay(Includes.MainFileIncludes), Delegate(Delegate), 
SM(SM),
+PP(PP) {
 // Only tokenize the preamble section of the main file, as we are not
 // interested in the rest of the tokens.
 MainFileTokens = syntax::tokenize(
@@ -167,7 +168,7 @@ class ReplayPreamble : private PPCallbacks {
   }
 
   void replay() {
-for (const auto &Inc : Includes.MainFileIncludes) {
+for (const auto &Inc : IncludesToReplay) {
   const FileEntry *File = nullptr;
   if (Inc.Resolved != "")
 if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +228,7 @@ class ReplayPreamble : private PPCallbacks {
 }
   }
 
-  const IncludeStructure &Includes;
+  std::vector IncludesToReplay;
   PPCallbacks *Delegate;
   const SourceManager &SM;
   Preprocessor &PP;

diff  --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp 
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index d86f741d9e72..0ad4a9f20ccb 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
+MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
+
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   struct Inclusion {
 Inclusion(const SourceManager &SM, SourceLocation HashLoc,
@@ -439,6 +441,24 @@ TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
 Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
 EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
+
+  // Make sure replay logic works with patched preambles.
+  TU.Code = "";
+  StoreDiags Diags;
+  auto Inputs = TU.inputs();
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  auto EmptyPreamble =
+  buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr);
+  ASSERT_TRUE(EmptyPreamble);
+  TU.Code = "#include ";
+  Includes.clear();
+  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
+ std::move(CI), {}, EmptyPreamble);
+  ASSERT_TRUE(PatchedAST);
+  // Make sure includes were seen only once.
+  EXPECT_THAT(Includes,
+  ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
+  WithFileName("a.h")));
 }
 
 TEST(ParsedASTTest, PatchesAdditionalIncludes) {



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

[PATCH] D80988: [clangd] Copy existing includes in ReplayPreamble

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 267848.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80988

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

Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
+MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
+
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   struct Inclusion {
 Inclusion(const SourceManager &SM, SourceLocation HashLoc,
@@ -439,6 +441,24 @@
 Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
 EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
+
+  // Make sure replay logic works with patched preambles.
+  TU.Code = "";
+  StoreDiags Diags;
+  auto Inputs = TU.inputs();
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  auto EmptyPreamble =
+  buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr);
+  ASSERT_TRUE(EmptyPreamble);
+  TU.Code = "#include ";
+  Includes.clear();
+  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
+ std::move(CI), {}, EmptyPreamble);
+  ASSERT_TRUE(PatchedAST);
+  // Make sure includes were seen only once.
+  EXPECT_THAT(Includes,
+  ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
+  WithFileName("a.h")));
 }
 
 TEST(ParsedASTTest, PatchesAdditionalIncludes) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -116,7 +116,7 @@
   // Attach preprocessor hooks such that preamble events will be injected at
   // the appropriate time.
   // Events will be delivered to the *currently registered* PP callbacks.
-  static void attach(const IncludeStructure &Includes, CompilerInstance &Clang,
+  static void attach(std::vector Includes, CompilerInstance &Clang,
  const PreambleBounds &PB) {
 auto &PP = Clang.getPreprocessor();
 auto *ExistingCallbacks = PP.getPPCallbacks();
@@ -124,7 +124,7 @@
 if (!ExistingCallbacks)
   return;
 PP.addPPCallbacks(std::unique_ptr(new ReplayPreamble(
-Includes, ExistingCallbacks, Clang.getSourceManager(), PP,
+std::move(Includes), ExistingCallbacks, Clang.getSourceManager(), PP,
 Clang.getLangOpts(), PB)));
 // We're relying on the fact that addPPCallbacks keeps the old PPCallbacks
 // around, creating a chaining wrapper. Guard against other implementations.
@@ -133,10 +133,10 @@
   }
 
 private:
-  ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
+  ReplayPreamble(std::vector Includes, PPCallbacks *Delegate,
  const SourceManager &SM, Preprocessor &PP,
  const LangOptions &LangOpts, const PreambleBounds &PB)
-  : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+  : Includes(std::move(Includes)), Delegate(Delegate), SM(SM), PP(PP) {
 // Only tokenize the preamble section of the main file, as we are not
 // interested in the rest of the tokens.
 MainFileTokens = syntax::tokenize(
@@ -167,7 +167,7 @@
   }
 
   void replay() {
-for (const auto &Inc : Includes.MainFileIncludes) {
+for (const auto &Inc : Includes) {
   const FileEntry *File = nullptr;
   if (Inc.Resolved != "")
 if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +227,7 @@
 }
   }
 
-  const IncludeStructure &Includes;
+  const std::vector Includes;
   PPCallbacks *Delegate;
   const SourceManager &SM;
   Preprocessor &PP;
@@ -382,7 +382,8 @@
 Includes = Preamble->Includes;
 Includes.MainFileIncludes = Patch->preambleIncludes();
 // Replay the preamble includes so that clang-tidy checks can see them.
-ReplayPreamble::attach(Includes, *Clang, Preamble->Preamble.getBounds());
+ReplayPreamble::attach(Patch->preambleIncludes(), *Clang,
+   Preamble->Preamble.getBounds());
   }
   // Important: collectIncludeStructure is registered *after* ReplayPreamble!
   // Otherwise we would collect the replayed includes again...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D80905#2068320 , @NoQ wrote:

> +Nithin because it may be relevant to the smart pointer checker if 
> inter-checker communication turns out to be necessary (eg., with the move 
> checker).


If checkers rely on one another, weak dependencies aren't the answer, but 
rather the guarantee strong dependencies grant. Weak dependencies could be 
useful however if reports from the checker would be hidden by, say, 
`NullDereferenceChecker`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80905



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

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D77866#2068394 , @NoQ wrote:

> We (me, Valeriy, Devin)'ve just been talking about this and mostly of agreed 
> that `core.CallAndMessage` should ideally be removed (at least as a 
> `Checkers.td` entity) and individual checks moved to the relevant package 
> (null dereference, use of uninitialized value, etc.). It won't hurt backwards 
> compatibility too much because so far there was no reasonable outcome that 
> one might expect from disabling `core.CallAndMessage`, given that it's a 
> weird mix of completely unrelated checks. The benefit is that disabling, say, 
> null dereference checker will actually disable all null dereference warnings 
> (as opposed to having to additionally disable the ones that come from 
> `CallAndMessage`).


Luckily this should be super easy to implement after this patch, and it is 
totally what I, and most folks on Ericsson's end want this checker to look 
like. I trust that you've glanced over the comments that justify the tiptoeing 
creating smaller checkers, and it is a result of a conversation we had that the 
price on the user would have to pay the way we implement our database is too 
great.

I'll initialize a new discussion on this on our end, but speaking from **my** 
perspective, we should just deal the cost of this and be done with it. However, 
since so many reports originate from this checker, I wonder whether we 
should go all the way and make sense out of the hash generation we currently 
have.

> Also a regular reminder that we wouldn't have had this conversation if we had 
> hashtags instead of packages (#CallAndMessage #NullDereference #C++ 
> #AllPlatforms #UndefinedBehavior #SecurityCritical #PathSensitive #Stable 
> #2012).

Could you detail this a bit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77866



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

[PATCH] D80925: Fix compiler crash when an expression parsed in the tentative parsing and must be claimed in the another evaluation context.

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseExpr.cpp:1009
+  Actions, Sema::ExpressionEvaluationContext::Unevaluated,
+  Sema::ReuseLambdaContextDecl);
+  Res = Actions.TransformToPotentiallyEvaluated(Res.get());

rjmccall wrote:
> Pushing an unevaluated context here isn't correct.  Here we're parsing the 
> expression for real and should already be in the right context for 
> `TransformToPotentiallyEvaluated`.
`TransformToPotentiallyEvaluated` expects that the innermost context is 
reevaluated. By the time we approach this branch, we're already out of the 
original unevaluated context. So, we need to create fake unevaluated context to 
mimic the original one. Function `TransformToPotentiallyEvaluated` copies the 
context state from the previous context to this new one and rebuilds the 
expression correctly. Instead, I can add a new function that transforms the 
expression in the current context and use it in 
`TransformToPotentiallyEvaluated`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80925



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

[PATCH] D80876: [clang] Default to windows response files when running on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D80876#2068428 , @mstorsjo wrote:

> But as for keeping the old default in mingw environments, would it make sense 
> to, if on windows, check the default target triple, and if that's a mingw 
> target (`Triple.isWindowsGNUEnvironment()`), keep the unix style quoting as 
> default?


Then again... Would this defeat the purpose of this whole patch, for cases when 
building wasm e.g. in MSYS2, where the tools are built targeting mingw by 
default?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80876



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

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal created this revision.
pdhaliwal added reviewers: yaxunl, msearles, sameerds.
Herald added subscribers: cfe-commits, sstefan1, guansong, t-tye, tpr, 
dstuttard, wdng, kzhuravl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

When offloading kind is OFK_OpenMP, the host toolchain (Generic_GCC)
returns DerivedArgList which copies input arguments along with few
other arguments. HIPToolchain again copies input arguments to the
same DerivedArgList.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80996

Files:
  clang/lib/Driver/ToolChains/HIP.cpp


Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -386,8 +386,10 @@
 
   const OptTable &Opts = getDriver().getOpts();
 
-  for (Arg *A : Args) {
-DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+for (Arg *A : Args) {
+  DAL->append(A);
+}
   }
 
   if (!BoundArch.empty()) {


Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -386,8 +386,10 @@
 
   const OptTable &Opts = getDriver().getOpts();
 
-  for (Arg *A : Args) {
-DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+for (Arg *A : Args) {
+  DAL->append(A);
+}
   }
 
   if (!BoundArch.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] a0f13b3 - Revert "[clangd] Copy existing includes in ReplayPreamble"

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-06-02T14:31:45+02:00
New Revision: a0f13b33742372bf00640eb18a2f3229176fc77d

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

LOG: Revert "[clangd] Copy existing includes in ReplayPreamble"

This reverts commit bff0c56ff92106afad22b54a90e1c612da4d6f99. I've
pushed the earlier version of the patch by mistake.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9eed9bc3199f..082c7cae0021 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -136,8 +136,7 @@ class ReplayPreamble : private PPCallbacks {
   ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
  const SourceManager &SM, Preprocessor &PP,
  const LangOptions &LangOpts, const PreambleBounds &PB)
-  : IncludesToReplay(Includes.MainFileIncludes), Delegate(Delegate), 
SM(SM),
-PP(PP) {
+  : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
 // Only tokenize the preamble section of the main file, as we are not
 // interested in the rest of the tokens.
 MainFileTokens = syntax::tokenize(
@@ -168,7 +167,7 @@ class ReplayPreamble : private PPCallbacks {
   }
 
   void replay() {
-for (const auto &Inc : IncludesToReplay) {
+for (const auto &Inc : Includes.MainFileIncludes) {
   const FileEntry *File = nullptr;
   if (Inc.Resolved != "")
 if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -228,7 +227,7 @@ class ReplayPreamble : private PPCallbacks {
 }
   }
 
-  std::vector IncludesToReplay;
+  const IncludeStructure &Includes;
   PPCallbacks *Delegate;
   const SourceManager &SM;
   Preprocessor &PP;

diff  --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp 
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 0ad4a9f20ccb..d86f741d9e72 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,8 +328,6 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
-MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
-
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   struct Inclusion {
 Inclusion(const SourceManager &SM, SourceLocation HashLoc,
@@ -441,24 +439,6 @@ TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
 Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
 EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
-
-  // Make sure replay logic works with patched preambles.
-  TU.Code = "";
-  StoreDiags Diags;
-  auto Inputs = TU.inputs();
-  auto CI = buildCompilerInvocation(Inputs, Diags);
-  auto EmptyPreamble =
-  buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr);
-  ASSERT_TRUE(EmptyPreamble);
-  TU.Code = "#include ";
-  Includes.clear();
-  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
- std::move(CI), {}, EmptyPreamble);
-  ASSERT_TRUE(PatchedAST);
-  // Make sure includes were seen only once.
-  EXPECT_THAT(Includes,
-  ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
-  WithFileName("a.h")));
 }
 
 TEST(ParsedASTTest, PatchesAdditionalIncludes) {



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

[clang-tools-extra] 8506877 - [clangd] Copy existing includes in ReplayPreamble

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-06-02T14:31:45+02:00
New Revision: 8506877c87aa602736aee1fffbd80b886fa40b79

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

LOG: [clangd] Copy existing includes in ReplayPreamble

Summary:
ReplayPreamble was just grabbing the reference of IncludeStructure
passed to it and then replayed any includes seen so while exiting
built-in file.

This implies any include seen in built-in files being replayed as part
of preamble, even though they are not. This wasn't an issue until we've
started patching preambles, as includes from built-in files were not
mapped back to main-file.

This patch copies over existing includes at the time of
ReplayPreamble::attach and only replies those to prevent any includes
from the preamble patch getting mixed-in.

Reviewers: sammccall, jkorous

Subscribers: ilya-biryukov, MaskRay, dexonsmith, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 082c7cae0021..f3568c34f99a 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -116,7 +116,7 @@ class ReplayPreamble : private PPCallbacks {
   // Attach preprocessor hooks such that preamble events will be injected at
   // the appropriate time.
   // Events will be delivered to the *currently registered* PP callbacks.
-  static void attach(const IncludeStructure &Includes, CompilerInstance &Clang,
+  static void attach(std::vector Includes, CompilerInstance &Clang,
  const PreambleBounds &PB) {
 auto &PP = Clang.getPreprocessor();
 auto *ExistingCallbacks = PP.getPPCallbacks();
@@ -124,7 +124,7 @@ class ReplayPreamble : private PPCallbacks {
 if (!ExistingCallbacks)
   return;
 PP.addPPCallbacks(std::unique_ptr(new ReplayPreamble(
-Includes, ExistingCallbacks, Clang.getSourceManager(), PP,
+std::move(Includes), ExistingCallbacks, Clang.getSourceManager(), PP,
 Clang.getLangOpts(), PB)));
 // We're relying on the fact that addPPCallbacks keeps the old PPCallbacks
 // around, creating a chaining wrapper. Guard against other 
implementations.
@@ -133,10 +133,10 @@ class ReplayPreamble : private PPCallbacks {
   }
 
 private:
-  ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
+  ReplayPreamble(std::vector Includes, PPCallbacks *Delegate,
  const SourceManager &SM, Preprocessor &PP,
  const LangOptions &LangOpts, const PreambleBounds &PB)
-  : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+  : Includes(std::move(Includes)), Delegate(Delegate), SM(SM), PP(PP) {
 // Only tokenize the preamble section of the main file, as we are not
 // interested in the rest of the tokens.
 MainFileTokens = syntax::tokenize(
@@ -167,7 +167,7 @@ class ReplayPreamble : private PPCallbacks {
   }
 
   void replay() {
-for (const auto &Inc : Includes.MainFileIncludes) {
+for (const auto &Inc : Includes) {
   const FileEntry *File = nullptr;
   if (Inc.Resolved != "")
 if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +227,7 @@ class ReplayPreamble : private PPCallbacks {
 }
   }
 
-  const IncludeStructure &Includes;
+  const std::vector Includes;
   PPCallbacks *Delegate;
   const SourceManager &SM;
   Preprocessor &PP;
@@ -382,7 +382,8 @@ ParsedAST::build(llvm::StringRef Filename, const 
ParseInputs &Inputs,
 Includes = Preamble->Includes;
 Includes.MainFileIncludes = Patch->preambleIncludes();
 // Replay the preamble includes so that clang-tidy checks can see them.
-ReplayPreamble::attach(Includes, *Clang, Preamble->Preamble.getBounds());
+ReplayPreamble::attach(Patch->preambleIncludes(), *Clang,
+   Preamble->Preamble.getBounds());
   }
   // Important: collectIncludeStructure is registered *after* ReplayPreamble!
   // Otherwise we would collect the replayed includes again...

diff  --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp 
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index d86f741d9e72..0ad4a9f20ccb 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
+MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
+
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   st

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 267856.

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

https://reviews.llvm.org/D80947

Files:
  llvm/docs/CodingStandards.rst

Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -669,15 +669,15 @@
 .. code-block:: c++
 
   // Typically there's no reason to copy.
-  for (const auto &Val : Container) { observe(Val); }
-  for (auto &Val : Container) { Val.change(); }
+  for (const auto &Val : Container) observe(Val);
+  for (auto &Val : Container) Val.change();
 
   // Remove the reference if you really want a new copy.
   for (auto Val : Container) { Val.change(); saveSomewhere(Val); }
 
   // Copy pointers, but make it clear that they're pointers.
-  for (const auto *Ptr : Container) { observe(*Ptr); }
-  for (auto *Ptr : Container) { Ptr->change(); }
+  for (const auto *Ptr : Container) observe(*Ptr);
+  for (auto *Ptr : Container) Ptr->change();
 
 Beware of non-determinism due to ordering of pointers
 ^
@@ -884,7 +884,7 @@
 .. code-block:: c++
 
   Value *doSomething(Instruction *I) {
-// Terminators never need 'something' done to them because ... 
+// Terminators never need 'something' done to them because ...
 if (I->isTerminator())
   return 0;
 
@@ -896,7 +896,7 @@
 // This is really just here for example.
 if (!doOtherThing(I))
   return 0;
-
+
 ... some long code 
   }
 
@@ -1000,7 +1000,7 @@
   Type = Context.getsigjmp_bufType();
 else
   Type = Context.getjmp_bufType();
-
+
 if (Type.isNull()) {
   Error = Signed ? ASTContext::GE_Missing_sigjmp_buf :
ASTContext::GE_Missing_jmp_buf;
@@ -1010,7 +1010,7 @@
 
 The idea is to reduce indentation and the amount of code you have to keep track
 of when reading the code.
-  
+
 Turn Predicate Loops into Predicate Functions
 ^
 
@@ -1081,7 +1081,7 @@
 * **Variable names** should be nouns (as they represent state).  The name should
   be camel case, and start with an upper case letter (e.g. ``Leader`` or
   ``Boats``).
-  
+
 * **Function names** should be verb phrases (as they represent actions), and
   command-like function should be imperative.  The name should be camel case,
   and start with a lower case letter (e.g. ``openFile()`` or ``isFoo()``).
@@ -1091,7 +1091,7 @@
   discriminator for a union, or an indicator of a subclass.  When an enum is
   used for something like this, it should have a ``Kind`` suffix
   (e.g. ``ValueKind``).
-  
+
 * **Enumerators** (e.g. ``enum { Foo, Bar }``) and **public member variables**
   should start with an upper-case letter, just like types.  Unless the
   enumerators are defined in their own small namespace or inside a class,
@@ -1107,7 +1107,7 @@
 MaxSize = 42,
 Density = 12
   };
-  
+
 As an exception, classes that mimic STL classes can have member names in STL's
 style of lower-case words separated by underscores (e.g. ``begin()``,
 ``push_back()``, and ``empty()``). Classes that provide multiple
@@ -1360,7 +1360,7 @@
 The use of ``#include `` in library files is hereby **forbidden**,
 because many common implementations transparently inject a `static constructor`_
 into every translation unit that includes it.
-  
+
 Note that using the other stream headers ( for example) is not
 problematic in this regard --- just . However, ``raw_ostream``
 provides various APIs that are better performing for almost every use than
@@ -1492,7 +1492,7 @@
   public:
 explicit Grokable() { ... }
 virtual ~Grokable() = 0;
-  
+
 ...
 
   };
@@ -1541,8 +1541,8 @@
   };
   } // end anonymous namespace
 
-  static void runHelper() { 
-... 
+  static void runHelper() {
+...
   }
 
   bool StringSort::operator<(const char *RHS) const {
@@ -1570,6 +1570,44 @@
 contrast, when the function is marked static, you don't need to cross-reference
 faraway places in the file to tell that the function is local.
 
+Don't Use Braces on Simple Single-Statement Bodies of if/else/loop Statements
+^
+
+When writing the body of an `if`, `else`, or loop statement, omit the braces to avoid
+unnecessary and otherwise meaningless code. However, braces should be used
+in cases where it significantly improves readability, such as when the single
+statement is accompanied by a comment that loses its meaning if hoisted above the `if`
+or loop statement, or where the single statement is complex enough that it stops being
+clear where the block containing the following statement begins. If said statement is
+an additional `if` or loop, it should be considered a single statement for this rule,
+and follow the rule recursively.
+
+Note that comments should only be ho

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 6 inline comments as done.
erichkeane added inline comments.



Comment at: llvm/docs/CodingStandards.rst:1578
+unnecessary and otherwise meaningless code. Braces should be used however
+in cases where it significantly improves readability, such as when the single
+statement is accompanied by a comment that loses its meaning if hoisted above 
the `if`

hubert.reinterpretcast wrote:
> I think "navigability" is also negatively affected by omission of braces. 
> This seems to be an aspect of readability this is not always considered. It 
> tends to be easier to consume code in an editor when placing a cursor on a 
> brace highlights the matching brace. If a reviewer in a web interface needed 
> to scroll or "draw a line" to where a loop or if/else chain starts when 
> reaching the end of a block, then the lack of braces is harmful. This would 
> especially be the case if the code was such that having comments after the 
> brace would be helpful.
> 
> The use of braces to proactively avoid running into the dangling-else problem 
> should also be permitted or even encouraged.
> 
> Replacing the list of cases where braces help readability with a list of 
> cases where omitting braces are harmful may help. We can then enforce braces 
> for some classes of harmful brace-omission and permit braces for other 
> classes.
> 
> Examples of "mild" harmful cases can then include mixing of braced and 
> non-braced blocks in an if/else chain.
Can you suggest an alternate wording here?  I'm not sure how to capture what 
you're saying.


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

https://reviews.llvm.org/D80947



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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

Very nice! I only have minor comments.

Also I'm curious to see what this will find in Chromium. I guess we'll find out 
:-)




Comment at: clang/include/clang/Analysis/Analyses/UninitializedValues.h:118
+
+  virtual void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+ const UninitUse &use) {}

nit: May put this after handleUseOfUninitVariable() instead, since it's very 
similar.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2110
+def warn_uninit_const_reference : Warning<
+  "variable %0 is uninitialized when passed as a const reference parameter "
+  "here">, InGroup, DefaultIgnore;

Minor detail, but I think it should say "argument" instead of "parameter".

My understanding is that parameters are names used when declaring/defining 
functions, e, g.
int foo(int x) { ... }
here x is a parameter.

And arguments are used when calling functions:
foo(42);
here 42 is an argument.

(https://en.wikipedia.org/wiki/Parameter_(computer_programming))



Comment at: clang/lib/Analysis/UninitializedValues.cpp:891
+
+  void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+ const UninitUse &use) override {

I'd probably move this to right after handleUseOfUninitVariable() since they're 
similar.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1590
+
+// flush all const reference uses diags
+for (const auto &P : constRefUses) {

nit: capital f and period at the end.



Comment at: clang/test/SemaCXX/warn-uninitialized-const-reference.cpp:5
+public:
+int i;
+A() {};

nit: I think 2 spaces for indentation is more common in this code (applies also 
below).


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

https://reviews.llvm.org/D79895



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

[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

I made the decision to make all relative offsets relative to the start of the 
AST block rather than their own sub-block or a neighboring block, in order to 
prevent adding complexity in `ASTReader`. My reasoning that it would be best if 
the reader didn't have to keep track of a bunch of offsets inside the bitcode 
at all times, but that it could just remember the offset of the AST block once 
and always use that for relative offsets. I agree that conceptually making the 
sub-blocks relocatable too would have been nice, but I figured it wasn't worth 
the extra complexity in the reader.




Comment at: clang/lib/Serialization/ASTWriter.cpp:1084
   PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) {
-Signature = createSignature(StringRef(Buffer.begin(), 
StartOfUnhashedControl));
+Record.append(ASTSignature.begin(), ASTSignature.end());
+Stream.EmitRecord(AST_SIGNATURE, Record);

dexonsmith wrote:
> The default abbreviation (7-VBR) isn't great for hashes. Given that we're 
> going to have two of these records, I suggest specifying a custom 
> abbreviation.
> 
> If you were to split out a prep commit as suggested above to change 
> `SIGNATURE` to be an array of 8-bit values (instead of the current array of 
> 32-bit values), that would be a good opportunity to add an abbreviation.
I think encoding the signature as a blob makes the most sense if we make it an 
array of 20 8-bit values, which is what the hasher gives us. However it is not 
quite so simple, as it would need a big change in the way direct imports are 
serialized. The way it is currently done embeds multiple strings and signatures 
within a single unabbreviated record. I can replace the unabbreviated record 
with a sub-block that keeps track of a string table and a record for each 
import containing offsets inside the string table and blobs for the signature.



Comment at: clang/lib/Serialization/ASTWriter.cpp:4729-4733
+StringRef Name = M.Kind == MK_PrebuiltModule ||
+ M.Kind == MK_ExplicitModule ||
+ M.Kind == MK_ImplicitModule
+ ? M.ModuleName
+ : M.FileName;

dexonsmith wrote:
> Is there ever a reason to use `M.FileName` here, or is that always redundant 
> with what's in the control block? I wonder if we can just remove this 
> complexity.
> 
The control block only tracks this information for direct imports although it 
can maybe be extended to do keep track of this for all dependencies. This bit 
of the table could then become and index inside the `IMPORTS` record in the 
control block.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2437
   if (DeclOffsets.size() == Index)
-DeclOffsets.emplace_back(Loc, Offset);
+DeclOffsets.emplace_back(Loc, Offset, ASTBlockRange.first);
   else if (DeclOffsets.size() < Index) {

dexonsmith wrote:
> I suggest making `WriteDecls` relative to the start of the 
> `DECLTYPES_BLOCK_ID`. That will make the block itself (more) self-contained, 
> and also make the offsets smaller (and therefore cheaper to store in bitcode).
> 
> I also don't think you need to actually emit that integer, it's just an 
> implicit contract between the reader and writer.
Not sure what you mean by I don't actually need to emit that integer? If you 
mean `ASTBlockRange.first` then I don't emit it but rather pass it to the 
constructor of DeclOffset so that it performs the subtraction there. Since it 
is a self contained type, it forces the Reader to provide a value for it, 
making it harder to read the raw relative offset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80383



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

[PATCH] D80941: [PowerPC][Power10] Implement Count Leading/Trailing Zeroes Builtins in LLVM/Clang

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

In D80941#2066931 , @lebedev.ri wrote:

> Why not lower it to `@llvm.cttz(and(a, b))`?


That's a great idea. Particularly in the back end where this pattern can appear 
irrespective of the use of the builtins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80941



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

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Can we have a lit test? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80996



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

[PATCH] D80988: [clangd] Copy existing includes in ReplayPreamble

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8506877c87aa: [clangd] Copy existing includes in 
ReplayPreamble (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80988

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

Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@
   testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
+MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; }
+
 TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
   struct Inclusion {
 Inclusion(const SourceManager &SM, SourceLocation HashLoc,
@@ -439,6 +441,24 @@
 Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
 EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
+
+  // Make sure replay logic works with patched preambles.
+  TU.Code = "";
+  StoreDiags Diags;
+  auto Inputs = TU.inputs();
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  auto EmptyPreamble =
+  buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr);
+  ASSERT_TRUE(EmptyPreamble);
+  TU.Code = "#include ";
+  Includes.clear();
+  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
+ std::move(CI), {}, EmptyPreamble);
+  ASSERT_TRUE(PatchedAST);
+  // Make sure includes were seen only once.
+  EXPECT_THAT(Includes,
+  ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
+  WithFileName("a.h")));
 }
 
 TEST(ParsedASTTest, PatchesAdditionalIncludes) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -116,7 +116,7 @@
   // Attach preprocessor hooks such that preamble events will be injected at
   // the appropriate time.
   // Events will be delivered to the *currently registered* PP callbacks.
-  static void attach(const IncludeStructure &Includes, CompilerInstance &Clang,
+  static void attach(std::vector Includes, CompilerInstance &Clang,
  const PreambleBounds &PB) {
 auto &PP = Clang.getPreprocessor();
 auto *ExistingCallbacks = PP.getPPCallbacks();
@@ -124,7 +124,7 @@
 if (!ExistingCallbacks)
   return;
 PP.addPPCallbacks(std::unique_ptr(new ReplayPreamble(
-Includes, ExistingCallbacks, Clang.getSourceManager(), PP,
+std::move(Includes), ExistingCallbacks, Clang.getSourceManager(), PP,
 Clang.getLangOpts(), PB)));
 // We're relying on the fact that addPPCallbacks keeps the old PPCallbacks
 // around, creating a chaining wrapper. Guard against other implementations.
@@ -133,10 +133,10 @@
   }
 
 private:
-  ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
+  ReplayPreamble(std::vector Includes, PPCallbacks *Delegate,
  const SourceManager &SM, Preprocessor &PP,
  const LangOptions &LangOpts, const PreambleBounds &PB)
-  : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+  : Includes(std::move(Includes)), Delegate(Delegate), SM(SM), PP(PP) {
 // Only tokenize the preamble section of the main file, as we are not
 // interested in the rest of the tokens.
 MainFileTokens = syntax::tokenize(
@@ -167,7 +167,7 @@
   }
 
   void replay() {
-for (const auto &Inc : Includes.MainFileIncludes) {
+for (const auto &Inc : Includes) {
   const FileEntry *File = nullptr;
   if (Inc.Resolved != "")
 if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +227,7 @@
 }
   }
 
-  const IncludeStructure &Includes;
+  const std::vector Includes;
   PPCallbacks *Delegate;
   const SourceManager &SM;
   Preprocessor &PP;
@@ -382,7 +382,8 @@
 Includes = Preamble->Includes;
 Includes.MainFileIncludes = Patch->preambleIncludes();
 // Replay the preamble includes so that clang-tidy checks can see them.
-ReplayPreamble::attach(Includes, *Clang, Preamble->Preamble.getBounds());
+ReplayPreamble::attach(Patch->preambleIncludes(), *Clang,
+   Preamble->Preamble.getBounds());
   }
   // Important: collectIncludeStructure is registered *after* ReplayPreamble!
   // Otherwise we would collect the replayed includes again...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80981: [AST] Fix a crash on accessing a class without definition in constexpr function context.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added a subscriber: rsmith.
hokein added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4320
+if (!RD->hasDefinition())
+  return APValue();
 APValue Struct(APValue::UninitStruct(), RD->getNumBases(),

sammccall wrote:
> This doesn't look all that safe - you're using a `None` value to indicate 
> failure, but no current code path does that and none of the callers seem to 
> check for failure.
> (e.g. `evaluateVarDecl` returns true instead of false).
> Presumably we're going to get a diagnostic somewhere (though it's not 
> completely obvious to me) but can we be sure we won't assume this value has 
> the right type somewhere down the line?
> 
> I get the feeling this is correct and I don't have enough context to 
> understand why... how about you :-)
I don't have a promising explanation neither. 

I didn't find a better way to model failures in `getDefaultInitValue`. This 
function is used in multiple places of this file (and I'm not sure whether we 
should change it).

@rsmith any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80981



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

cfe-commits@lists.llvm.org

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki added inline comments.



Comment at: clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c:8
+// CHECK-LABEL: test_vbfdot_f32
+// CHECK:  %0 = bitcast <4 x bfloat> %a to <8 x i8>
+// CHECK:  %1 = bitcast <4 x bfloat> %b to <8 x i8>

Why not `CHECK-NEXT`?



Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-dotprod-intrinsics.ll:4
+; CHECK-LABEL: test_vbfdot_f32
+; CHECK: bfdot   v0.2s, v1.4h, v2.4h
+define <2 x float> @test_vbfdot_f32(<2 x float> %r, <4 x bfloat> %a, <4 x 
bfloat> %b) {

Would it make sense to check the whole body of the compiled function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80752



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

[PATCH] D80952: [FPEnv][Clang][Driver][WIP] Disable constrained floating point on targets lacking support.

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

In D80952#2067643 , @arsenm wrote:

> In D80952#2067563 , @efriedma wrote:
>
> > The problem for the command-line arguments in particular is that they 
> > aren't really new; clang has been eating them for a long time, without any 
> > warning.  So if -frounding-math crashes the compiler, that's a regression.
>
>
> Could we get an -fexperimental-something flag to override this behavior then?


The vast majority of the work needed is in llvm and not clang. Most if not all 
testing is done with the llc executable. There is some work in clang's 
CGBuiltins.cpp, but that's pretty small compared to llvm. And with this patch 
the enabling of the clang support is only one line. That seems cheaper than yet 
another command line flag. Someone doing development of the clang parts is 
going to be building clang anyway so I don't see how a one line change is much 
of a burden.

The flags for strict fp and rounding are already pretty complicated already. 
I'd hate to make it even worse with yet another flag just to avoid having a 
developer have to carry one line in one source file. And when clang+llvm is 
ready for a target that one line would be needed _anyway_. So what would a new 
-fexperimental-something flag gain us?

Also, I'm not sure that the constrained intrinsics will ever be implemented on 
every last one of our targets. So a new flag would complicate matters basically 
forever.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80952



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

[PATCH] D80758: [PowerPC] Add -m[no-]power10-vector clang and llvm option

[Ahsan Saghir via Phabricator via cfe-commits]

saghir updated this revision to Diff 267859.
saghir added a comment.

Removed `P10Altivec`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80758

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/Driver/ppc-dependent-options.cpp
  clang/test/Driver/ppc-features.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h

Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -105,6 +105,7 @@
   bool HasP8Crypto;
   bool HasP9Vector;
   bool HasP9Altivec;
+  bool HasP10Vector;
   bool HasPrefixInstrs;
   bool HasPCRelativeMemops;
   bool HasFCPSGN;
@@ -262,6 +263,7 @@
   bool hasP8Crypto() const { return HasP8Crypto; }
   bool hasP9Vector() const { return HasP9Vector; }
   bool hasP9Altivec() const { return HasP9Altivec; }
+  bool hasP10Vector() const { return HasP10Vector; }
   bool hasPrefixInstrs() const { return HasPrefixInstrs; }
   bool hasPCRelativeMemops() const { return HasPCRelativeMemops; }
   bool hasMFOCRF() const { return HasMFOCRF; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -78,6 +78,7 @@
   HasP8Crypto = false;
   HasP9Vector = false;
   HasP9Altivec = false;
+  HasP10Vector = false;
   HasPrefixInstrs = false;
   HasPCRelativeMemops = false;
   HasFCPSGN = false;
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -216,6 +216,10 @@
 "Enable POWER9 vector instructions",
 [FeatureISA3_0, FeatureP8Vector,
  FeatureP9Altivec]>;
+def FeatureP10Vector  : SubtargetFeature<"power10-vector", "HasP10Vector",
+ "true",
+ "Enable POWER10 vector instructions",
+ [FeatureISA3_1, FeatureP9Vector]>;
 // A separate feature for this even though it is equivalent to P9Vector
 // because this is a feature of the implementation rather than the architecture
 // and may go away with future CPU's.
@@ -337,7 +341,7 @@
   // still exist with the exception of those we know are Power9 specific.
   list P10AdditionalFeatures =
 [DirectivePwr10, FeatureISA3_1, FeaturePrefixInstrs,
- FeaturePCRelativeMemops];
+ FeaturePCRelativeMemops, FeatureP10Vector];
   list P10SpecificFeatures = [];
   list P10InheritableFeatures =
 !listconcat(P9InheritableFeatures, P10AdditionalFeatures);
Index: clang/test/Driver/ppc-features.cpp
===
--- clang/test/Driver/ppc-features.cpp
+++ clang/test/Driver/ppc-features.cpp
@@ -150,6 +150,12 @@
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-power8-vector -mpower8-vector -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-P8VECTOR %s
 // CHECK-P8VECTOR: "-target-feature" "+power8-vector"
 
+// RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-power10-vector -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOP10VECTOR %s
+// CHECK-NOP10VECTOR: "-target-feature" "-power10-vector"
+
+// RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-power10-vector -mpower10-vector -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-P10VECTOR %s
+// CHECK-P10VECTOR: "-target-feature" "+power10-vector"
+
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-crbits -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOCRBITS %s
 // CHECK-NOCRBITS: "-target-feature" "-crbits"
 
Index: clang/test/Driver/ppc-dependent-options.cpp
===
--- clang/test/Driver/ppc-dependent-options.cpp
+++ clang/test/Driver/ppc-dependent-options.cpp
@@ -58,6 +58,14 @@
 // RUN: -mcpu=power9 -std=c++11 -mno-vsx -mfloat128 -mpower9-vector %s 2>&1 | \
 // RUN: FileCheck %s -check-prefix=CHECK-NVSX-MULTI
 
+// RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
+// RUN: -mcpu=power10 -std=c++11 %s 2>&1 | FileCheck %s \
+// RUN: -check-prefix=CHECK-DEFAULT-P10
+
+// RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
+// RUN: -mcpu=power10 -std=c++11 -mno-vsx -mpower10-vector %s 2>&1 | \
+// RUN: FileCheck %s -check-prefix=CHECK-NVSX-P10V
+
 #ifdef __VSX__
 static_assert(false, "VSX enabled");
 #endif
@@ -70,6 +78,10 @@
 static_assert(false, "P9V enabled");
 #endif
 
+#ifdef __POWER10_VECTOR__
+static_assert(false, "P10V enabled");
+#endif
+
 #if !defined(__VS

[PATCH] D80758: [PowerPC] Add -m[no-]power10-vector clang and llvm option

[Ahsan Saghir via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/PPC.cpp:414
+if (Name == "power9-vector")
+  Features["power10-vector"] = false;
 Features[Name] = false;

lei wrote:
> I think it would be good to change this into a switch-stmt as there are 
> enough conditional changed based on `Name` and probably more to come.
Adding string switch here would complicate it more, so sticking with the 
current approach.



Comment at: llvm/lib/Target/PowerPC/PPC.td:219
  FeatureP9Altivec]>;
+def FeatureP10Altivec : SubtargetFeature<"power10-altivec", "HasP10Altivec",
+ "true",

lei wrote:
> what's the relationship between this and p10 vector support?
Removed P10Altivec since this is not really needed for P10.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80758



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

[PATCH] D58579: [Sema] SequenceChecker: C++17 sequencing rule for call expressions.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 267865.
riccibruno added a comment.

Updated with the test case from PR20819.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58579

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp

Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -15,7 +15,6 @@
   int n;
 };
 
-// TODO: Implement the C++17 sequencing rules.
 void test() {
   int a;
   int xs[10];
@@ -256,6 +255,27 @@
   p[i++] = (i = 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
   p++[i++] = (i = p ? i++ : i++); // cxx11-warning {{unsequenced modification and access to 'p'}}
   // cxx11-warning@-1 {{multiple unsequenced modifications to 'i'}}
+
+  (i++, f)(i++, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+  (i++ + i++, f)(42, 42); // cxx11-warning {{multiple unsequenced modifications to 'i'}}
+  // cxx17-warning@-1 {{multiple unsequenced modifications to 'i'}}
+  int (*pf)(int, int);
+  (pf = f)(pf != nullptr, pf != nullptr); // cxx11-warning {{unsequenced modification and access to 'pf'}}
+  pf((pf = f) != nullptr, 42); // cxx11-warning {{unsequenced modification and access to 'pf'}}
+  f((pf = f, 42), (pf = f, 42)); // cxx11-warning {{multiple unsequenced modifications to 'pf'}}
+ // cxx17-warning@-1 {{multiple unsequenced modifications to 'pf'}}
+  pf((pf = f) != nullptr, pf == nullptr); // cxx11-warning {{unsequenced modification and access to 'pf'}}
+  // cxx17-warning@-1 {{unsequenced modification and access to 'pf'}}
+}
+
+namespace PR20819 {
+  struct foo { void bar(int); };
+  foo get_foo(int);
+
+  void g() {
+int a = 0;
+get_foo(a).bar(a++);  // cxx11-warning {{unsequenced modification and access to 'a'}}
+  }
 }
 
 namespace members {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12928,6 +12928,11 @@
   }
 
   void VisitCallExpr(const CallExpr *CE) {
+// FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+
+if (CE->isUnevaluatedBuiltinCall(Context))
+  return;
+
 // C++11 [intro.execution]p15:
 //   When calling a function [...], every value computation and side effect
 //   associated with any argument expression, or with the postfix expression
@@ -12935,10 +12940,41 @@
 //   expression or statement in the body of the function [and thus before
 //   the value computation of its result].
 SequencedSubexpression Sequenced(*this);
-SemaRef.runWithSufficientStackSpace(CE->getExprLoc(),
-[&] { Base::VisitCallExpr(CE); });
+SemaRef.runWithSufficientStackSpace(CE->getExprLoc(), [&] {
+  // C++17 [expr.call]p5
+  //   The postfix-expression is sequenced before each expression in the
+  //   expression-list and any default argument. [...]
+  SequenceTree::Seq CalleeRegion;
+  SequenceTree::Seq OtherRegion;
+  if (SemaRef.getLangOpts().CPlusPlus17) {
+CalleeRegion = Tree.allocate(Region);
+OtherRegion = Tree.allocate(Region);
+  } else {
+CalleeRegion = Region;
+OtherRegion = Region;
+  }
+  SequenceTree::Seq OldRegion = Region;
 
-// FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
+  // Visit the callee expression first.
+  Region = CalleeRegion;
+  if (SemaRef.getLangOpts().CPlusPlus17) {
+SequencedSubexpression Sequenced(*this);
+Visit(CE->getCallee());
+  } else {
+Visit(CE->getCallee());
+  }
+
+  // Then visit the argument expressions.
+  Region = OtherRegion;
+  for (const Expr *Argument : CE->arguments())
+Visit(Argument);
+
+  Region = OldRegion;
+  if (SemaRef.getLangOpts().CPlusPlus17) {
+Tree.merge(CalleeRegion);
+Tree.merge(OtherRegion);
+  }
+});
   }
 
   void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 301a6da - AMDGPU: Fix clang side null pointer value for private

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2020-06-02T09:23:46-04:00
New Revision: 301a6da8c24a09052e3bda10e90b450b7b39ffea

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

LOG: AMDGPU: Fix clang side null pointer value for private

The change to fold_priv_arith looks strange to me, but this was
already the untested behavior for local.

Added: 


Modified: 
clang/lib/Basic/Targets/AMDGPU.h
clang/test/CodeGenOpenCL/amdgpu-nullptr.cl

Removed: 




diff  --git a/clang/lib/Basic/Targets/AMDGPU.h 
b/clang/lib/Basic/Targets/AMDGPU.h
index 6c9060aa3f7b..e4194a881e3f 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -355,7 +355,9 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : 
public TargetInfo {
   // address space has value 0 but in private and local address space has
   // value ~0.
   uint64_t getNullPointerValue(LangAS AS) const override {
-return AS == LangAS::opencl_local ? ~0 : 0;
+// FIXME: Also should handle region.
+return (AS == LangAS::opencl_local || AS == LangAS::opencl_private)
+  ? ~0 : 0;
   }
 
   void setAuxTarget(const TargetInfo *Aux) override;

diff  --git a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl 
b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
index ba34d168bf79..753f7f6f4406 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -19,7 +19,7 @@ typedef struct {
 
 // Test 0 as initializer.
 
-// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8 addrspace(5)* 
null, align 4
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8 addrspace(5)* 
addrspacecast (i8* null to i8 addrspace(5)*), align 4
 private char *private_p = 0;
 
 // CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* 
addrspacecast (i8* null to i8 addrspace(3)*), align 4
@@ -36,7 +36,7 @@ generic char *generic_p = 0;
 
 // Test NULL as initializer.
 
-// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8 
addrspace(5)* null, align 4
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8 
addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), align 4
 private char *private_p_NULL = NULL;
 
 // CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 
addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), align 4
@@ -57,25 +57,28 @@ generic char *generic_p_NULL = NULL;
 // CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32* null, 
align 8
 generic int *fold_generic = (global int*)(generic float*)(private char*)0;
 
-// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16 
addrspace(5)* null, align 4
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16 
addrspace(5)* addrspacecast (i16* null to i16 addrspace(5)*), align 4
 private short *fold_priv = (private short*)(generic int*)(global void*)0;
 
-// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8 
addrspace(5)* inttoptr (i32 10 to i8 addrspace(5)*), align 4
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8 
addrspace(5)* inttoptr (i32 9 to i8 addrspace(5)*), align 4
 private char *fold_priv_arith = (private char*)0 + 10;
 
-// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 14, align 4
+// CHECK: @fold_local_arith = local_unnamed_addr addrspace(1) global i8 
addrspace(3)* inttoptr (i32 9 to i8 addrspace(3)*), align 4
+local char *fold_local_arith = (local char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
 int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
 
-// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 13, align 4
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
 int fold_int2 = (int) ((private void*)0 + 13);
 
-// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 0, align 4
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
 int fold_int3 = (int) ((private int*)0);
 
-// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 8, align 4
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
 int fold_int4 = (int) &((private int*)0)[2];
 
-// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 4, align 4
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
 int fold_int5 = (int) &((private StructTy1*)0)->p2;
 
 
@@ -97,12 +100,12 @@ int fold_int5_local = (int) &((local StructTy1*)0)->p2;
 
 // Test static variable initialization.
 
-// NOOPT: @test_static_var_private.sp1 = internal addrspace(1) global i8 
addrspace(5)* null, align 4
-// NOOPT: @test_static_var_private.sp2 = internal addrspace(1) global i8 
addrspace(5)* null, align 4
-/

[clang] e16a4ef - Remove a comment-out llvm::errs debugging code, NFC.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-06-02T15:42:46+02:00
New Revision: e16a4efdc6f429eabfb47ef90d40fcb6b6e362ad

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

LOG: Remove a comment-out llvm::errs debugging code, NFC.

Added: 


Modified: 
clang/lib/AST/Stmt.cpp

Removed: 




diff  --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index 550f5aa338f1..ce76d4941b32 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/lib/AST/Stmt.cpp
@@ -273,7 +273,6 @@ SourceRange Stmt::getSourceRange() const {
 }
 
 SourceLocation Stmt::getBeginLoc() const {
-  //  llvm::errs() << "getBeginLoc() for " << getStmtClassName() << "\n";
   switch (getStmtClass()) {
   case Stmt::NoStmtClass: llvm_unreachable("statement without class");
 #define ABSTRACT_STMT(type)



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

[clang] 21ccc68 - [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-06-02T15:58:56+02:00
New Revision: 21ccc684ff4c8563e7b20bed4ae7dc7d18fe03f3

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

LOG: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

Summary:
For a none-function-like unresolved expression, clang builds a TypoExpr
for it, and tries to correct it afterwards. If the typo-correction
fails, clang just drops the whole expr.

This patch improves the recovery strategy -- if the typo-correction
fails, we preserve the AST by degrading the typo exprs to recovery
exprs.

This would improve toolings for "undef_var" broken cases:
```
void foo();
void test() {
  fo^o(undef_var); // go-to-def, hover still works.
}
```

TESTED=ran tests with this patch + turn-on-recovery-ast patch, it breaks
one declare_variant_messages testcase (the diagnostics are slightly
changed), I think it is acceptable.

```
Error: 'error' diagnostics seen but not expected:
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 16: 
expected 'match' clause on 'omp declare variant' directive
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 57: 
expected 'match' clause on 'omp declare variant' directive
error: 'warning' diagnostics expected but not seen:
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 47: the 
context selector 'kind' in the context set 'device' cannot have a score 
(''); score ignored
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 87: the 
context selector 'kind' in the context set 'device' cannot have a score 
(''); score ignored
error: 'warning' diagnostics seen but not expected:
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 47: the 
context selector 'kind' in the context set 'device' cannot have a score 
('()'); score ignored
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 87: the 
context selector 'kind' in the context set 'device' cannot have a score 
('()'); score ignored
6 errors generated.
```

Reviewers: sammccall, jdoerfert

Subscribers: sstefan1, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/SemaExprCXX.cpp
clang/test/AST/ast-dump-recovery.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index b655c8281689..9c4e5b4a7848 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@ ExprResult Sema::ActOnFinishFullExpr(Expr *FE, 
SourceLocation CC,
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
-  if (FullExpr.isInvalid())
-return ExprError();
+  if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+  TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+  ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+  }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 

diff  --git a/clang/test/AST/ast-dump-recovery.cpp 
b/clang/test/AST/ast-dump-recovery.cpp
index a212ff41c0c9..6d271cebc7f9 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@ int some_func(int *);
 // CHECK-NEXT:`-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+  // CHECK:  CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}}  'int' 1
+  some_func(undef1, undef2+1);
+
+  // CHECK:  BinaryOperator {{.*}} '' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+  s = some_func(undef1);
+  // CHECK: `-VarDecl {{.*}} invalid var 'int'
+  // FIXME: preserve the broken call.
+  int var = some_func(undef1);
+}
 
 int ambig_func(double);
 int ambig_func(float);



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

[PATCH] D80931: AMDGPU: Fix clang side null pointer value for private

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

301a6da8c24a09052e3bda10e90b450b7b39ffea 



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

https://reviews.llvm.org/D80931



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

[PATCH] D80758: [PowerPC] Add -m[no-]power10-vector clang and llvm option

[Lei Huang via Phabricator via cfe-commits]

lei accepted this revision.
lei added a comment.

LGTM. 
Minor nit can be addressed during commit.




Comment at: clang/lib/Basic/Targets/PPC.cpp:413
+  Features["power9-vector"] = Features["power10-vector"] = false;
+if (Name == "power9-vector")
+  Features["power10-vector"] = false;

nit: this should be an `else if`, same as what you did on line 401.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80758



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

[PATCH] D81000: Renamed Lang_C to Lang_C99, Lang_CXX to Lang_CXX03, and 2a to 20

[Marcel Hlopko via Phabricator via cfe-commits]

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

Looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81000



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

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal updated this revision to Diff 267870.
pdhaliwal added a comment.

Added lit test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80996

Files:
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-openmp-arguments.c


Index: clang/test/Driver/hip-openmp-arguments.c
===
--- /dev/null
+++ clang/test/Driver/hip-openmp-arguments.c
@@ -0,0 +1,13 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -nogpulib \
+// RUN:   -mllvm -amdgpu-dump-hsa-metadata \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-pc-linux-gnu" "-emit-obj"
+// CHECK-SAME: {{.*}} "-fopenmp"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-dump-hsa-metadata" 
"-fopenmp-is-device" {{.*}}
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -386,8 +386,10 @@
 
   const OptTable &Opts = getDriver().getOpts();
 
-  for (Arg *A : Args) {
-DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+for (Arg *A : Args) {
+  DAL->append(A);
+}
   }
 
   if (!BoundArch.empty()) {


Index: clang/test/Driver/hip-openmp-arguments.c
===
--- /dev/null
+++ clang/test/Driver/hip-openmp-arguments.c
@@ -0,0 +1,13 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp \
+// RUN:   -fopenmp-targets=amdgcn-amd-amdhsa -nogpulib \
+// RUN:   -mllvm -amdgpu-dump-hsa-metadata \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-pc-linux-gnu" "-emit-obj"
+// CHECK-SAME: {{.*}} "-fopenmp"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-dump-hsa-metadata" "-fopenmp-is-device" {{.*}}
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -386,8 +386,10 @@
 
   const OptTable &Opts = getDriver().getOpts();
 
-  for (Arg *A : Args) {
-DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+for (Arg *A : Args) {
+  DAL->append(A);
+}
   }
 
   if (!BoundArch.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 267873.
hokein marked 3 inline comments as done.
hokein added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80733

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-recovery.cpp


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
 // CHECK-NEXT:`-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+  // CHECK:  CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}}  'int' 1
+  some_func(undef1, undef2+1);
+
+  // CHECK:  BinaryOperator {{.*}} '' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+  s = some_func(undef1);
+  // CHECK: `-VarDecl {{.*}} invalid var 'int'
+  // FIXME: preserve the broken call.
+  int var = some_func(undef1);
+}
 
 int ambig_func(double);
 int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
-  if (FullExpr.isInvalid())
-return ExprError();
+  if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+  TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+  ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+  }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
 // CHECK-NEXT:`-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+  // CHECK:  CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}}  'int' 1
+  some_func(undef1, undef2+1);
+
+  // CHECK:  BinaryOperator {{.*}} '' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+  s = some_func(undef1);
+  // CHECK: `-VarDecl {{.*}} invalid var 'int'
+  // FIXME: preserve the broken call.
+  int var = some_func(undef1);
+}
 
 int ambig_func(double);
 int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
-  if (FullExpr.isInvalid())
-return ExprError();
+  if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+  TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+  ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+  }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267876.
pestctrl added a comment.

Updated warning message to be more descriptive


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete <= &complete) && // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete > &complete) &&  // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete >= &complete) && // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int 

[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:8310
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.

sammccall wrote:
> This seems reasonably straightforward to fix - there shouldn't be a lot of 
> TypoExprs so we can just add a member. Worth doing I think.
yeah, agree. Will address it in a followup.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:8314
+  using Base = TreeTransform;
+  TyposReplace(Sema &SemaRef) : Base(SemaRef) {}
+  ExprResult TransformTypoExpr(TypoExpr *E) {

sammccall wrote:
> Can't you just write this as `: TreeTransform(SemaRef) {}`, and drop the 
> using?
yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80733



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

[PATCH] D81003: [clang][Sema] SequenceChecker: Also visit default arguments.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added a reviewer: rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

`SequenceChecker` is currently not visiting default arguments and therefore 
missing cases like:

  int a;
  int foo(int x = a++, int y = a);
  void test() {
foo(); // should warn
foo(a++); // idem
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81003

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/warn-unsequenced.cpp


Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -268,6 +268,36 @@
   // cxx17-warning@-1 {{unsequenced 
modification and access to 'pf'}}
 }
 
+namespace default_arg {
+  int a;
+  void f1(int = a, int = a++); // cxx11-warning 2{{unsequenced modification 
and access to 'a'}}
+   // cxx17-warning@-1 2{{unsequenced modification 
and access to 'a'}}
+
+  void f2(int = a++, int = a); // cxx11-warning {{unsequenced modification and 
access to 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification 
and access to 'a'}}
+
+  void f3(int = a++, int = sizeof(a));
+
+  void test() {
+// TODO: Consider adding a remark indicating the function call where
+// the default argument was used.
+int b;
+f1();
+f1(a);
+f1(a,a); // ok
+f1(b);   // ok
+f1(b,a++);   // ok
+
+f2();
+f2(a);   // ok
+f2(a++); // cxx11-warning {{unsequenced modification and access to 
'a'}}
+ // cxx17-warning@-1 {{unsequenced modification and access to 
'a'}}
+
+f3();   // ok
+f3(a++);// ok
+  }
+}
+
 namespace PR20819 {
   struct foo { void bar(int); };
   foo get_foo(int);
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12977,6 +12977,10 @@
 });
   }
 
+  void VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *DAE) {
+Visit(DAE->getExpr());
+  }
+
   void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
 // This is a call, so all subexpressions are sequenced before the result.
 SequencedSubexpression Sequenced(*this);


Index: clang/test/SemaCXX/warn-unsequenced.cpp
===
--- clang/test/SemaCXX/warn-unsequenced.cpp
+++ clang/test/SemaCXX/warn-unsequenced.cpp
@@ -268,6 +268,36 @@
   // cxx17-warning@-1 {{unsequenced modification and access to 'pf'}}
 }
 
+namespace default_arg {
+  int a;
+  void f1(int = a, int = a++); // cxx11-warning 2{{unsequenced modification and access to 'a'}}
+   // cxx17-warning@-1 2{{unsequenced modification and access to 'a'}}
+
+  void f2(int = a++, int = a); // cxx11-warning {{unsequenced modification and access to 'a'}}
+   // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+
+  void f3(int = a++, int = sizeof(a));
+
+  void test() {
+// TODO: Consider adding a remark indicating the function call where
+// the default argument was used.
+int b;
+f1();
+f1(a);
+f1(a,a); // ok
+f1(b);   // ok
+f1(b,a++);   // ok
+
+f2();
+f2(a);   // ok
+f2(a++); // cxx11-warning {{unsequenced modification and access to 'a'}}
+ // cxx17-warning@-1 {{unsequenced modification and access to 'a'}}
+
+f3();   // ok
+f3(a++);// ok
+  }
+}
+
 namespace PR20819 {
   struct foo { void bar(int); };
   foo get_foo(int);
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12977,6 +12977,10 @@
 });
   }
 
+  void VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *DAE) {
+Visit(DAE->getExpr());
+  }
+
   void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
 // This is a call, so all subexpressions are sequenced before the result.
 SequencedSubexpression Sequenced(*this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG21ccc684ff4c: [AST][RecoveryExpr] Build RecoveryExpr for 
"undef_var" cases. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80733

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-recovery.cpp


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
 // CHECK-NEXT:`-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+  // CHECK:  CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}}  'int' 1
+  some_func(undef1, undef2+1);
+
+  // CHECK:  BinaryOperator {{.*}} '' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+  s = some_func(undef1);
+  // CHECK: `-VarDecl {{.*}} invalid var 'int'
+  // FIXME: preserve the broken call.
+  int var = some_func(undef1);
+}
 
 int ambig_func(double);
 int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
-  if (FullExpr.isInvalid())
-return ExprError();
+  if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+  TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+  ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+  }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 


Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
 // CHECK-NEXT:`-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+  // CHECK:  CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}}  'int' 1
+  some_func(undef1, undef2+1);
+
+  // CHECK:  BinaryOperator {{.*}} '' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+  s = some_func(undef1);
+  // CHECK: `-VarDecl {{.*}} invalid var 'int'
+  // FIXME: preserve the broken call.
+  int var = some_func(undef1);
+}
 
 int ambig_func(double);
 int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
-  if (FullExpr.isInvalid())
-return ExprError();
+  if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+  TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+  ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+  }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

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

baloghadamsoftware updated this revision to Diff 267879.
baloghadamsoftware added a comment.

Incorporated @NoQ's solution.


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

https://reviews.llvm.org/D80366

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

Index: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
===
--- /dev/null
+++ clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
@@ -0,0 +1,55 @@
+//===- unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+class TestReturnValueUnderConstructionChecker
+  : public Checker {
+public:
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+if (!Call.getOriginExpr())
+  return;
+
+Optional RetVal = Call.getReturnValueUnderConstruction(0);
+assert(RetVal);
+assert(RetVal->getAsRegion());
+  }
+};
+
+void addTestReturnValueUnderConstructionChecker(
+AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages =
+{{"test.TestReturnValueUnderConstruction", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+  Registry.addChecker(
+  "test.TestReturnValueUnderConstruction", "", "");
+});
+}
+
+TEST(TestReturnValueUnderConstructionChecker,
+ ReturnValueUnderConstructionChecker) {
+  EXPECT_TRUE(runCheckerOnCode(
+  "class C { public: C(int nn): n(nn) {} virtual ~C() {} "
+  "  private: int n; };"
+  "C returnC(int m) { C c(m); return c; }"
+  "void foo() { C c = returnC(1); }"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -7,10 +7,11 @@
   AnalyzerOptionsTest.cpp
   CallDescriptionTest.cpp
   CallEventTest.cpp
-  StoreTest.cpp
+  RangeSetTest.cpp
   RegisterCustomCheckersTest.cpp
+  StoreTest.cpp 
   SymbolReaperTest.cpp
-  RangeSetTest.cpp
+  TestReturnValueUnderConstruction.cpp
   )
 
 clang_target_link_libraries(StaticAnalysisTests
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -109,15 +109,14 @@
   return LValue;
 }
 
-std::pair ExprEngine::handleConstructionContext(
+SVal ExprEngine::computeObjectUnderConstruction(
 const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
 const ConstructionContext *CC, EvalCallOptions &CallOpts) {
   SValBuilder &SVB = getSValBuilder();
   MemRegionManager &MRMgr = SVB.getRegionManager();
   ASTContext &ACtx = SVB.getContext();
 
-  // See if we're constructing an existing region by looking at the
-  // current construction context.
+  // Compute the target region by exploring the construction context.
   if (CC) {
 switch (CC->getKind()) {
 case ConstructionContext::CXX17ElidedCopyVariableKind:
@@ -125,13 +124,9 @@
   const auto *DSCC = cast(CC);
   const auto *DS = DSCC->getDeclStmt();
   const auto *Var = cast(DS->getSingleDecl());
-  SVal LValue = State->getLValue(Var, LCtx);
   QualType Ty = Var->getType();
-  LValue =
-  makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
-  State =
-  addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue);
-  return std::make_pair(State, LValue);
+  return makeZeroElementRegion(State, State->getLValue(Var, LCtx), Ty,
+   CallOpts.IsArrayCtorOrDtor);
 }
 case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
 case ConstructionContext::SimpleConstructorInitializerKind: {
@@ -139,8 +134,7 @@
  

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 267880.
aganea marked an inline comment as done.
aganea added a comment.

As requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/test/CodeGen/debug-info-codeview-buildinfo.c
  clang/tools/driver/cc1_main.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/MC/MCTargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -831,6 +831,31 @@
   return TypeTable.writeLeafType(SIR);
 }
 
+static std::string renderCommandLine(ArrayRef CommandLineArgs,
+ StringRef MainFile) {
+  std::string FlatCmdLine;
+  SmallString<128> TempArg;
+  auto maybeQuote = [&TempArg](const char *Arg) -> StringRef {
+const bool Escape =
+StringRef(Arg).find_first_of(" \"\\$") != StringRef::npos;
+if (!Escape)
+  return Arg;
+TempArg.clear();
+(Twine("\"") + Arg + "\"").toVector(TempArg);
+return TempArg;
+  };
+  for (auto &Arg : CommandLineArgs) {
+if (Arg == nullptr)
+  break;
+// The command-line shall not contain the file to compile.
+if (Arg == MainFile)
+  continue;
+FlatCmdLine += maybeQuote(Arg);
+FlatCmdLine += " ";
+  }
+  return FlatCmdLine;
+}
+
 void CodeViewDebug::emitBuildInfo() {
   // First, make LF_BUILDINFO. It's a sequence of strings with various bits of
   // build info. The known prefix is:
@@ -851,8 +876,14 @@
   getStringIdTypeIdx(TypeTable, MainSourceFile->getDirectory());
   BuildInfoArgs[BuildInfoRecord::SourceFile] =
   getStringIdTypeIdx(TypeTable, MainSourceFile->getFilename());
-  // FIXME: Path to compiler and command line. PDB is intentionally blank unless
-  // we implement /Zi type servers.
+  if (!StringRef(Asm->TM.Options.MCOptions.BuildTool).empty()) {
+BuildInfoArgs[BuildInfoRecord::BuildTool] =
+getStringIdTypeIdx(TypeTable, Asm->TM.Options.MCOptions.BuildTool);
+BuildInfoArgs[BuildInfoRecord::CommandLine] = getStringIdTypeIdx(
+TypeTable, renderCommandLine(Asm->TM.Options.MCOptions.CommandLineArgs,
+ MainSourceFile->getFilename()));
+  }
+  // FIXME: PDB is intentionally blank unless we implement /Zi type servers.
   BuildInfoRecord BIR(BuildInfoArgs);
   TypeIndex BuildInfoIndex = TypeTable.writeLeafType(BIR);
 
Index: llvm/include/llvm/MC/MCTargetOptions.h
===
--- llvm/include/llvm/MC/MCTargetOptions.h
+++ llvm/include/llvm/MC/MCTargetOptions.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_MC_MCTARGETOPTIONS_H
 #define LLVM_MC_MCTARGETOPTIONS_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include 
 #include 
 
@@ -59,6 +60,9 @@
   std::string AssemblyLanguage;
   std::string SplitDwarfFile;
 
+  const char *BuildTool = nullptr;
+  ArrayRef CommandLineArgs;
+
   /// Additional paths to search for `.include` directives when using the
   /// integrated assembler.
   std::vector IASSearchPaths;
Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -203,8 +203,7 @@
   }
 }
 
-extern int cc1_main(ArrayRef Argv, const char *Argv0,
-void *MainAddr);
+extern int cc1_main(ArrayRef Argv, void *MainAddr);
 extern int cc1as_main(ArrayRef Argv, const char *Argv0,
   void *MainAddr);
 extern int cc1gen_reproducer_main(ArrayRef Argv,
@@ -327,7 +326,7 @@
   StringRef Tool = ArgV[1];
   void *GetExecutablePathVP = (void *)(intptr_t)GetExecutablePath;
   if (Tool == "-cc1")
-return cc1_main(makeArrayRef(ArgV).slice(2), ArgV[0], GetExecutablePathVP);
+return cc1_main(makeArrayRef(ArgV), GetExecutablePathVP);
   if (Tool == "-cc1as")
 return cc1as_main(makeArrayRef(ArgV).slice(2), ArgV[0],
   GetExecutablePathVP);
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -181,7 +181,7 @@
   return 0;
 }
 
-int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
+int cc1_main(ArrayRef Argv, void *MainAddr) {
   ensureSufficientStack();
 
   std::unique_ptr Clang(new CompilerInstance());
@@ -203,12 +203,11 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
-

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 267882.
vabridgers added a comment.

Address comments from Artem and Balazs. This change just avoids the crash for 
now until a proper fix can be made.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903

Files:
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/test/Analysis/vla.c


Index: clang/test/Analysis/vla.c
===
--- clang/test/Analysis/vla.c
+++ clang/test/Analysis/vla.c
@@ -137,3 +137,17 @@
   clang_analyzer_eval(clang_analyzer_getExtent(&vla3m) == 2 * x * 4 * 
sizeof(int));
   // expected-warning@-1{{TRUE}}
 }
+
+// https://bugs.llvm.org/show_bug.cgi?id=46128
+// analyzer doesn't handle more than simple symbolic expressions.
+// Just don't crash.
+extern void foo(void);
+int a;
+void b() {
+  int c = a + 1;
+  for (;;) {
+int d[c];
+for (; 0 < c;)
+  foo();
+  }
+} // no-crash
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -126,7 +126,12 @@
   // Size overflow check does not work with symbolic expressions because a
   // overflow situation can not be detected easily.
   uint64_t IndexL = IndexLVal->getZExtValue();
-  assert(IndexL > 0 && "Index length should have been checked for zero.");
+  // FIXME: See https://reviews.llvm.org/D80903 for discussion of
+  // some difference in assume and getKnownValue that leads to
+  // unexpected behavior. Just bail on IndexL == 0 at this point.
+  if (IndexL == 0)
+return nullptr;
+
   if (KnownSize <= SizeMax / IndexL) {
 KnownSize *= IndexL;
   } else {


Index: clang/test/Analysis/vla.c
===
--- clang/test/Analysis/vla.c
+++ clang/test/Analysis/vla.c
@@ -137,3 +137,17 @@
   clang_analyzer_eval(clang_analyzer_getExtent(&vla3m) == 2 * x * 4 * sizeof(int));
   // expected-warning@-1{{TRUE}}
 }
+
+// https://bugs.llvm.org/show_bug.cgi?id=46128
+// analyzer doesn't handle more than simple symbolic expressions.
+// Just don't crash.
+extern void foo(void);
+int a;
+void b() {
+  int c = a + 1;
+  for (;;) {
+int d[c];
+for (; 0 < c;)
+  foo();
+  }
+} // no-crash
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -126,7 +126,12 @@
   // Size overflow check does not work with symbolic expressions because a
   // overflow situation can not be detected easily.
   uint64_t IndexL = IndexLVal->getZExtValue();
-  assert(IndexL > 0 && "Index length should have been checked for zero.");
+  // FIXME: See https://reviews.llvm.org/D80903 for discussion of
+  // some difference in assume and getKnownValue that leads to
+  // unexpected behavior. Just bail on IndexL == 0 at this point.
+  if (IndexL == 0)
+return nullptr;
+
   if (KnownSize <= SizeMax / IndexL) {
 KnownSize *= IndexL;
   } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:3
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
+
+int main() { return 42; }

Is there a way to launch an arbitrary program from lit? I started extracting 
the tool and the cmd-line from the .OBJ, to run it again. But I couldn't figure 
out how to start the extracted tool pathname? Something along the lines of
```
// RUN: eval $CMD
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:389
 
-  for (Arg *A : Args) {
-DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+for (Arg *A : Args) {

Needs a comment? I don't understand why openmp is any different here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80996



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

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:320
+  /// Executable and command-line used to create a given CompilerInvocation.
+  const char *BuildTool = nullptr;
+  ArrayRef CommandLineArgs;

The name BuildTool makes me think of things like Make or MSBuild rather than 
the compiler. And in this case we know the compiler is Clang :-) Also since 
this is for capturing the cc1 arguments, it shouldn't really matter if it's 
clang-cl, clang++ or just clang. So it seems unfortunate that it has to be 
piped through all the way like this..

Is there some way we could just grab the path to the current clang binary 
during the pdb writing?



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:835
+static std::string renderCommandLine(ArrayRef CommandLineArgs,
+ StringRef MainFile) {
+  std::string FlatCmdLine;

Don't we already have code somewhere that can do this quoting? E.g. the code 
that prints the cc1 args for "clang -v"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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

[PATCH] D81008: [AST] Record SourceLocation for TypoExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81008

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-recovery.cpp

Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -13,9 +13,9 @@
 void test_invalid_call(int s) {
   // CHECK:  CallExpr {{.*}} '' contains-errors
   // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
-  // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
-  // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
-  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <>
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} 
+  // CHECK-NEXT: `-BinaryOperator {{.*}}
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}}
   // CHECK-NEXT:   `-IntegerLiteral {{.*}}  'int' 1
   some_func(undef1, undef2+1);
 
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -5167,9 +5167,9 @@
   IdentifierInfo *Typo = TypoName.getName().getAsIdentifierInfo();
   if (!ExternalTypo && ED > 0 && Typo->getName().size() / ED < 3)
 return nullptr;
-
   ExprEvalContexts.back().NumTypos++;
-  return createDelayedTypo(std::move(Consumer), std::move(TDG), std::move(TRC));
+  return createDelayedTypo(std::move(Consumer), std::move(TDG), std::move(TRC),
+   TypoName.getBeginLoc(), TypoName.getEndLoc());
 }
 
 void TypoCorrection::addCorrectionDecl(NamedDecl *CDecl) {
@@ -5481,9 +5481,11 @@
 
 TypoExpr *Sema::createDelayedTypo(std::unique_ptr TCC,
   TypoDiagnosticGenerator TDG,
-  TypoRecoveryCallback TRC) {
+  TypoRecoveryCallback TRC,
+  SourceLocation StartLoc,
+  SourceLocation EndLoc) {
   assert(TCC && "createDelayedTypo requires a valid TypoCorrectionConsumer");
-  auto TE = new (Context) TypoExpr(Context.DependentTy);
+  auto TE = new (Context) TypoExpr(Context.DependentTy, StartLoc, EndLoc);
   auto &State = DelayedTypos[TE];
   State.Consumer = std::move(TCC);
   State.DiagHandler = std::move(TDG);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8307,8 +8307,6 @@
   if (FullExpr.isInvalid()) {
 // Typo-correction fails, we rebuild the broken AST with the typos degraded
 // to RecoveryExpr.
-// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
-// track source locations.
 struct TyposReplace : TreeTransform {
   TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
   ExprResult TransformTypoExpr(TypoExpr *E) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3695,7 +3695,8 @@
   /// Creates a new TypoExpr AST node.
   TypoExpr *createDelayedTypo(std::unique_ptr TCC,
   TypoDiagnosticGenerator TDG,
-  TypoRecoveryCallback TRC);
+  TypoRecoveryCallback TRC, SourceLocation StartLoc,
+  SourceLocation EndLoc);
 
   // The set of known/encountered (unique, canonicalized) NamespaceDecls.
   //
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -6152,8 +6152,11 @@
 /// TypoExpr - Internal placeholder for expressions where typo correction
 /// still needs to be performed and/or an error diagnostic emitted.
 class TypoExpr : public Expr {
+  SourceLocation Start, End;
+
 public:
-  TypoExpr(QualType T) : Expr(TypoExprClass, T, VK_LValue, OK_Ordinary) {
+  TypoExpr(QualType T, SourceLocation Start, SourceLocation End)
+  : Expr(TypoExprClass, T, VK_LValue, OK_Ordinary), Start(Start), End(End) {
 assert(T->isDependentType() && "TypoExpr given a non-dependent type");
 setDependence(ExprDependence::TypeValueInstantiation |
   ExprDependence::Error);
@@ -6166,8 +6169,8 @@
 return const_child_range(const_child_iterator(), const_child_iterator());
   }
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return SourceLocation(); }
-  SourceLocation getEndLoc() const LLVM_READONLY { return SourceLocation(); }
+  SourceLocation getBeginLoc() const LLVM_READONLY { return Start; }
+  SourceLocation getEndLoc() const LLVM_READONLY { return End; }
 
   static bool classof(const Stmt *T) {
 return T->getStmtClass() == Ty

[PATCH] D81009: Reinstate the syntax tree test for 'static' in an array subscript

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gribozavr2 added a reviewer: eduucaldas.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81009

Files:
  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
@@ -51,6 +51,8 @@
   TestLanguage Language;
   std::string Target;
 
+  bool isC99OrLater() const { return Language == Lang_C99; }
+
   bool isCXX() const {
 return Language == Lang_CXX03 || Language == Lang_CXX11 ||
Language == Lang_CXX14 || Language == Lang_CXX17 ||
@@ -1903,7 +1905,6 @@
 int a[10];
 int b[1][2][3];
 int c[] = {1,2,3};
-// void f(int xs[static 10]);
 )cpp",
   R"txt(
 *: TranslationUnit
@@ -1960,6 +1961,36 @@
   `-;   )txt");
 }
 
+TEST_P(SyntaxTreeTest, StaticArraySubscriptsInDeclarators) {
+  if (!GetParam().isC99OrLater()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+void f(int xs[static 10]);
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-f
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-xs
+  |   |   `-ArraySubscript
+  |   | |-[
+  |   | |-static
+  |   | |-UnknownExpression
+  |   | | `-10
+  |   | `-]
+  |   `-)
+  `-;   )txt");
+}
+
 TEST_P(SyntaxTreeTest, ParameterListsInDeclarators) {
   if (!GetParam().isCXX()) {
 // TODO: Split parts that depend on C++ into a separate test.


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -51,6 +51,8 @@
   TestLanguage Language;
   std::string Target;
 
+  bool isC99OrLater() const { return Language == Lang_C99; }
+
   bool isCXX() const {
 return Language == Lang_CXX03 || Language == Lang_CXX11 ||
Language == Lang_CXX14 || Language == Lang_CXX17 ||
@@ -1903,7 +1905,6 @@
 int a[10];
 int b[1][2][3];
 int c[] = {1,2,3};
-// void f(int xs[static 10]);
 )cpp",
   R"txt(
 *: TranslationUnit
@@ -1960,6 +1961,36 @@
   `-;   )txt");
 }
 
+TEST_P(SyntaxTreeTest, StaticArraySubscriptsInDeclarators) {
+  if (!GetParam().isC99OrLater()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+void f(int xs[static 10]);
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-f
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-xs
+  |   |   `-ArraySubscript
+  |   | |-[
+  |   | |-static
+  |   | |-UnknownExpression
+  |   | | `-10
+  |   | `-]
+  |   `-)
+  `-;   )txt");
+}
+
 TEST_P(SyntaxTreeTest, ParameterListsInDeclarators) {
   if (!GetParam().isCXX()) {
 // TODO: Split parts that depend on C++ into a separate test.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 89d9dba - [OPENMP50]Initial codegen for 'affinity' clauses.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-06-02T10:50:08-04:00
New Revision: 89d9dba2c6885949887edf4b80e1aabf8d8f3f88

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

LOG: [OPENMP50]Initial codegen for 'affinity' clauses.

Summary:
Added initial codegen for 'affinity' clauses on task directives.
Emits next code:
```
kmp_task_affinity_info_t affs[];

void *td = __kmpc_task_alloc(..);

affs[].base = &data_i;
affs[].size = sizeof(data_i);
__kmpc_omp_reg_task_with_affinity(&loc, , td, , affs);
```

The result returned by the call of `__kmpc_omp_reg_task_with_affinity`
function is ignored currently sincethe  runtime currently ignores args
and returns 0 uncoditionally.

Reviewers: jdoerfert

Subscribers: yaxunl, guansong, sstefan1, llvm-commits, cfe-commits, caomhin

Tags: #clang, #llvm

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

Added: 
clang/test/OpenMP/task_affinity_codegen.cpp

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index e9b1dffda0c4..231e4485aaeb 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4024,6 +4024,135 @@ checkDestructorsRequired(const RecordDecl 
*KmpTaskTWithPrivatesQTyRD) {
   return NeedsCleanup;
 }
 
+namespace {
+/// Loop generator for OpenMP iterator expression.
+class OMPIteratorGeneratorScope final
+: public CodeGenFunction::OMPPrivateScope {
+  CodeGenFunction &CGF;
+  const OMPIteratorExpr *E = nullptr;
+  SmallVector ContDests;
+  SmallVector ExitDests;
+  OMPIteratorGeneratorScope() = delete;
+  OMPIteratorGeneratorScope(OMPIteratorGeneratorScope &) = delete;
+
+public:
+  OMPIteratorGeneratorScope(CodeGenFunction &CGF, const OMPIteratorExpr *E)
+  : CodeGenFunction::OMPPrivateScope(CGF), CGF(CGF), E(E) {
+if (!E)
+  return;
+SmallVector Uppers;
+for (unsigned I = 0, End = E->numOfIterators(); I < End; ++I) {
+  Uppers.push_back(CGF.EmitScalarExpr(E->getHelper(I).Upper));
+  const auto *VD = cast(E->getIteratorDecl(I));
+  addPrivate(VD, [&CGF, VD]() {
+return CGF.CreateMemTemp(VD->getType(), VD->getName());
+  });
+  const OMPIteratorHelperData &HelperData = E->getHelper(I);
+  addPrivate(HelperData.CounterVD, [&CGF, &HelperData]() {
+return CGF.CreateMemTemp(HelperData.CounterVD->getType(),
+ "counter.addr");
+  });
+}
+Privatize();
+
+for (unsigned I = 0, End = E->numOfIterators(); I < End; ++I) {
+  const OMPIteratorHelperData &HelperData = E->getHelper(I);
+  LValue CLVal =
+  CGF.MakeAddrLValue(CGF.GetAddrOfLocalVar(HelperData.CounterVD),
+ HelperData.CounterVD->getType());
+  // Counter = 0;
+  CGF.EmitStoreOfScalar(
+  llvm::ConstantInt::get(CLVal.getAddress(CGF).getElementType(), 0),
+  CLVal);
+  CodeGenFunction::JumpDest &ContDest =
+  ContDests.emplace_back(CGF.getJumpDestInCurrentScope("iter.cont"));
+  CodeGenFunction::JumpDest &ExitDest =
+  ExitDests.emplace_back(CGF.getJumpDestInCurrentScope("iter.exit"));
+  // N = ;
+  llvm::Value *N = Uppers[I];
+  // cont:
+  // if (Counter < N) goto body; else goto exit;
+  CGF.EmitBlock(ContDest.getBlock());
+  auto *CVal =
+  CGF.EmitLoadOfScalar(CLVal, HelperData.CounterVD->getLocation());
+  llvm::Value *Cmp =
+  HelperData.CounterVD->getType()->isSignedIntegerOrEnumerationType()
+  ? CGF.Builder.CreateICmpSLT(CVal, N)
+  : CGF.Builder.CreateICmpULT(CVal, N);
+  llvm::BasicBlock *BodyBB = CGF.createBasicBlock("iter.body");
+  CGF.Builder.CreateCondBr(Cmp, BodyBB, ExitDest.getBlock());
+  // body:
+  CGF.EmitBlock(BodyBB);
+  // Iteri = Begini + Counter * Stepi;
+  CGF.EmitIgnoredExpr(HelperData.Update);
+}
+  }
+  ~OMPIteratorGeneratorScope() {
+if (!E)
+  return;
+for (unsigned I = E->numOfIterators(); I > 0; --I) {
+  // Counter = Counter + 1;
+  const OMPIteratorHelperData &HelperData = E->getHelper(I - 1);
+  CGF.EmitIgnoredExpr(HelperData.CounterUpdate);
+  // goto cont;
+  CGF.EmitBranchThroughCleanup(ContDests[I - 1]);
+  // exit:
+  CGF.EmitBlock(ExitDests[I - 1].getBlock(), /*IsFinished=*/I == 1);
+}
+  }
+};
+} // namespace
+
+static std::pair
+getPointerAndSize(CodeGenFunction &CGF, const Expr *E) {
+  const auto *OASE = dyn_cast(E);
+  llvm::Value *Addr;
+  if (OASE) {
+const Expr *Base = OASE->getBase();
+Addr = CGF.EmitScalarExpr(Base);
+  } else {
+Addr = CGF.EmitLValue(E).g

[clang] 53c29a4 - Reinstate the syntax tree test for 'static' in an array subscript

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2020-06-02T17:01:51+02:00
New Revision: 53c29a42d044b167f6b5f28e096c8d9e50d6edc7

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

LOG: Reinstate the syntax tree test for 'static' in an array subscript

Reviewers: eduucaldas

Reviewed By: eduucaldas

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp 
b/clang/unittests/Tooling/Syntax/TreeTest.cpp
index bcfb2f7614e8..a7de4b909cb4 100644
--- a/clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -51,6 +51,8 @@ struct TestClangConfig {
   TestLanguage Language;
   std::string Target;
 
+  bool isC99OrLater() const { return Language == Lang_C99; }
+
   bool isCXX() const {
 return Language == Lang_CXX03 || Language == Lang_CXX11 ||
Language == Lang_CXX14 || Language == Lang_CXX17 ||
@@ -1903,7 +1905,6 @@ TEST_P(SyntaxTreeTest, ArraySubscriptsInDeclarators) {
 int a[10];
 int b[1][2][3];
 int c[] = {1,2,3};
-// void f(int xs[static 10]);
 )cpp",
   R"txt(
 *: TranslationUnit
@@ -1960,6 +1961,36 @@ int c[] = {1,2,3};
   `-;   )txt");
 }
 
+TEST_P(SyntaxTreeTest, StaticArraySubscriptsInDeclarators) {
+  if (!GetParam().isC99OrLater()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+void f(int xs[static 10]);
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-f
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-xs
+  |   |   `-ArraySubscript
+  |   | |-[
+  |   | |-static
+  |   | |-UnknownExpression
+  |   | | `-10
+  |   | `-]
+  |   `-)
+  `-;   )txt");
+}
+
 TEST_P(SyntaxTreeTest, ParameterListsInDeclarators) {
   if (!GetParam().isCXX()) {
 // TODO: Split parts that depend on C++ into a separate test.



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

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Thank you for bringing up this issue. I think it highlights an underlying 
problem -- editing templates is quite difficult -- that neither setting will 
address, as Dmitri expanded on above. Given the parallel to macros, I'd say 
your change is better than the status quo. Most clang tidies and other 
rewriting tools that I've encountered simply skip code in macro expansions, 
rather than reason about how to update the macro definition or whatnot. So, by 
that reasoning, we should skip template instantations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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

[PATCH] D80932: [SYCL] Make default address space a superset of OpenCL address spaces.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Why?

Default address space is normally an address space of automatic storage i.e. 
`private` in OpenCL. If you look at the address space map of targets most of 
them map default and private address spaces to the same value. Converting 
between private and other named address space is not allowed in OpenCL see 
section 6.5. I don't believe this change is doing anything legitimate.

Can you explain what you are trying to achieve with this?




Comment at: clang/test/SemaOpenCLCXX/address-space-lambda.cl:34
   priv2();
-  auto priv3 = []() __global {}; //expected-note{{candidate function not 
viable: 'this' object is in address space '__private', but method expects 
object in address space '__global'}} //expected-note{{conversion candidate of 
type 'void (*)()'}}
-  priv3(); //expected-error{{no matching function for call to object of type}}

Here we are testing expected behavior. I believe we already had a discussion 
about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80932



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

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/CodeGenOptions.h:320
+  /// Executable and command-line used to create a given CompilerInvocation.
+  const char *BuildTool = nullptr;
+  ArrayRef CommandLineArgs;

hans wrote:
> The name BuildTool makes me think of things like Make or MSBuild rather than 
> the compiler. And in this case we know the compiler is Clang :-) Also since 
> this is for capturing the cc1 arguments, it shouldn't really matter if it's 
> clang-cl, clang++ or just clang. So it seems unfortunate that it has to be 
> piped through all the way like this..
> 
> Is there some way we could just grab the path to the current clang binary 
> during the pdb writing?
There's `sys::fs::getMainExecutable(const char *argv0, void *MainExecAddr)` but 
that's not guaranteed to work on all platforms, you still need to provide 
argv[0] to fallback, see 
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Unix/Path.inc#L232
I'm not well versed in Unix distros, can this only rarely occur?
Would you prefer that I used that function instead? (it is ok on Windows though)




Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:835
+static std::string renderCommandLine(ArrayRef CommandLineArgs,
+ StringRef MainFile) {
+  std::string FlatCmdLine;

hans wrote:
> Don't we already have code somewhere that can do this quoting? E.g. the code 
> that prints the cc1 args for "clang -v"?
Yes, the code below was copy-pasted from `Command::printArg`. But that's in 
Clang. Should I make that code common somewhere in `llvm/lib/Support`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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

[PATCH] D81009: Reinstate the syntax tree test for 'static' in an array subscript

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG53c29a42d044: Reinstate the syntax tree test for 
'static' in an array subscript (authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81009

Files:
  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
@@ -51,6 +51,8 @@
   TestLanguage Language;
   std::string Target;
 
+  bool isC99OrLater() const { return Language == Lang_C99; }
+
   bool isCXX() const {
 return Language == Lang_CXX03 || Language == Lang_CXX11 ||
Language == Lang_CXX14 || Language == Lang_CXX17 ||
@@ -1903,7 +1905,6 @@
 int a[10];
 int b[1][2][3];
 int c[] = {1,2,3};
-// void f(int xs[static 10]);
 )cpp",
   R"txt(
 *: TranslationUnit
@@ -1960,6 +1961,36 @@
   `-;   )txt");
 }
 
+TEST_P(SyntaxTreeTest, StaticArraySubscriptsInDeclarators) {
+  if (!GetParam().isC99OrLater()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+void f(int xs[static 10]);
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-f
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-xs
+  |   |   `-ArraySubscript
+  |   | |-[
+  |   | |-static
+  |   | |-UnknownExpression
+  |   | | `-10
+  |   | `-]
+  |   `-)
+  `-;   )txt");
+}
+
 TEST_P(SyntaxTreeTest, ParameterListsInDeclarators) {
   if (!GetParam().isCXX()) {
 // TODO: Split parts that depend on C++ into a separate test.


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -51,6 +51,8 @@
   TestLanguage Language;
   std::string Target;
 
+  bool isC99OrLater() const { return Language == Lang_C99; }
+
   bool isCXX() const {
 return Language == Lang_CXX03 || Language == Lang_CXX11 ||
Language == Lang_CXX14 || Language == Lang_CXX17 ||
@@ -1903,7 +1905,6 @@
 int a[10];
 int b[1][2][3];
 int c[] = {1,2,3};
-// void f(int xs[static 10]);
 )cpp",
   R"txt(
 *: TranslationUnit
@@ -1960,6 +1961,36 @@
   `-;   )txt");
 }
 
+TEST_P(SyntaxTreeTest, StaticArraySubscriptsInDeclarators) {
+  if (!GetParam().isC99OrLater()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+void f(int xs[static 10]);
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-f
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-xs
+  |   |   `-ArraySubscript
+  |   | |-[
+  |   | |-static
+  |   | |-UnknownExpression
+  |   | | `-10
+  |   | `-]
+  |   `-)
+  `-;   )txt");
+}
+
 TEST_P(SyntaxTreeTest, ParameterListsInDeclarators) {
   if (!GetParam().isCXX()) {
 // TODO: Split parts that depend on C++ into a separate test.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80240: [OPENMP50]Initial codegen for 'affinity' clauses.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG89d9dba2c688: [OPENMP50]Initial codegen for 
'affinity' clauses. (authored by ABataev).

Changed prior to commit:
  https://reviews.llvm.org/D80240?vs=267254&id=267895#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80240

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/test/OpenMP/task_affinity_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -336,6 +336,8 @@
 __OMP_RTL(__kmpc_push_proc_bind, false, Void, IdentPtr, Int32, /* Int */ Int32)
 __OMP_RTL(__kmpc_serialized_parallel, false, Void, IdentPtr, Int32)
 __OMP_RTL(__kmpc_end_serialized_parallel, false, Void, IdentPtr, Int32)
+__OMP_RTL(__kmpc_omp_reg_task_with_affinity, false, Int32, IdentPtr, Int32,
+  Int8Ptr, Int32, Int8Ptr)
 
 __OMP_RTL(omp_get_thread_num, false, Int32, )
 __OMP_RTL(omp_get_num_threads, false, Int32, )
Index: clang/test/OpenMP/task_affinity_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/task_affinity_codegen.cpp
@@ -0,0 +1,132 @@
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-version=50 -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -fopenmp-version=50 -x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+// CHECK-LABEL: @main
+int main() {
+  float *p;
+  int a = 10;
+  // kmp_task_affinity_info_t affs[1];
+  // CHECK: [[AFFS_ADDR:%.+]] = alloca [1 x %struct.kmp_task_affinity_info_t],
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @{{.+}}, i32 [[GTID:%.+]], i32 1, i64 40, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: [[AFFINE_LST_ADDR:%.+]] = getelementptr inbounds [1 x %struct.kmp_task_affinity_info_t], [1 x %struct.kmp_task_affinity_info_t]* [[AFFS_ADDR]], i64 0, i64 0
+  // CHECK: [[P:%.+]] = load float*, float** [[P_ADDR:%.+]],
+  // CHECK: [[A_VAL:%.+]] = load i32, i32* [[A_ADDR:%.+]],
+  // CHECK: [[A_SZ:%.+]] = sext i32 [[A_VAL]] to i64
+  // CHECK: [[BYTES:%.+]] = mul nuw i64 4, [[A_SZ]]
+  // CHECK: [[SZ:%.+]] = mul nuw i64 [[BYTES]], 10
+  // CHECK: [[A_VAL:%.+]] = load i32, i32* [[A_ADDR]],
+  // CHECK: [[A_SZ1:%.+]] = sext i32 [[A_VAL]] to i64
+  // CHECK: [[SIZE:%.+]] = mul nuw i64 [[SZ]], [[A_SZ]]
+  // CHECK: [[AFFS_0_ADDR:%.+]] = getelementptr %struct.kmp_task_affinity_info_t, %struct.kmp_task_affinity_info_t* [[AFFINE_LST_ADDR]], i64 0
+
+  // affs[0].base = p;
+  // CHECK: [[AFFS_0_BASE_ADDR:%.+]] = getelementptr inbounds %struct.kmp_task_affinity_info_t, %struct.kmp_task_affinity_info_t* [[AFFS_0_ADDR]], i32 0, i32 0
+  // CHECK: [[P_INTPTR:%.+]] = ptrtoint float* [[P]] to i64
+  // CHECK: store i64 [[P_INTPTR]], i64* [[AFFS_0_BASE_ADDR]],
+
+  // affs[0].size = sizeof(*p) * a * 10 * a;
+  // CHECK: [[AFFS_0_SIZE_ADDR:%.+]] = getelementptr inbounds %struct.kmp_task_affinity_info_t, %struct.kmp_task_affinity_info_t* [[AFFS_0_ADDR]], i32 0, i32 1
+  // CHECK: store i64 [[SIZE]], i64* [[AFFS_0_SIZE_ADDR]],
+  // CHECK: [[BC:%.+]] = bitcast %struct.kmp_task_affinity_info_t* [[AFFINE_LST_ADDR]] to i8*
+  // CHECK: call i32 @__kmpc_omp_reg_task_with_affinity(%struct.ident_t* @{{.+}}, i32 [[GTID]], i8* [[TD]], i32 1, i8* [[BC]])
+#pragma omp task affinity(([a][10][a])p)
+  ;
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @{{.+}}, i32 [[GTID]], i32 1, i64 40, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: [[A_VAL:%.+]] = load i32, i32* [[A_ADDR]],
+  // CHECK: [[SUB:%.+]] = sub nsw i32 [[A_VAL]], 0
+  // CHECK: [[CONV:%.+]] = zext i32 [[SUB]] to i64
+
+  //  =  + 1 constant affinity for affinity(a)
+  // CHECK: [[NUM_ELEMS:%.+]] = add nuw i64 1, [[CONV]]
+  // CHECK: [[SV:%.+]] = call i8* @llvm.stacksave()
+  // CHECK: store i8* [[SV]], i8** [[SV_ADDR:%.+]],
+
+  // kmp_task_affinity_info_t affs[];
+  // C

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:389
 
-  for (Arg *A : Args) {
-DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+for (Arg *A : Args) {

arsenm wrote:
> Needs a comment? I don't understand why openmp is any different here
`HostTC.TranslateArgs` (HostTC = Generic_GCC, Line#383) returns `DAL` which  
contains `Args` when offloadkind is OFK_OpenMP (for all other cases, it returns 
nullptr). Thus, Line#{390,391} is just duplicating the arguments which are 
propagating to the opt, llc, etc. commands.
Ref: https://clang.llvm.org/doxygen/Gnu_8cpp_source.html#l02966


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80996



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

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

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

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

At least as fast bugfix it is acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903



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

cfe-commits@lists.llvm.org

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson added a subscriber: pratlucas.
LukeGeeson added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10366
+auto Alignment = CGM.getNaturalPointeeTypeAlignment(
+E->getArg(0)->IgnoreParenCasts()->getType());
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));

simon_tatham wrote:
> What effect is this change of strategy having on the alignment computation, 
> for the already-supported instances of this builtin?
> 
> It //looks// to me as if `__builtin_neon_vld1_v` with (say) a `uint8_t *` 
> pointer argument will now compute `Alignment=1` (the natural alignment of the 
> pointee type), whereas it would previously have computed `Alignment=8` (the 
> size of the whole vector being loaded or stored).
> 
> Is that intentional? Or accidental? Or have I completely misunderstood what's 
> going on?
> 
> (Whichever of the three, some discussion in the commit message would be a 
> good idea, explaining why this change does or does not make a difference, as 
> appropriate.)
Clang was incorrectly assuming that all the pointers from which loads were 
being generated for vld1 intrinsics were aligned according to according to the 
intrinsics result type. This causes alignment faults on the code generated by 
the backend.
This fixes the issue so that alignment is based on the type of the pointer 
provided to as input to the intrinsic.
 
@pratlucas has done some work on this in parallel 
https://reviews.llvm.org/D79721 which has been approved and may overrule this 
particular line of code. 

I shall add a note to the commit message, and tentatively mark this as fixed, 
given it's liable to adopt the work of Lucas.




Comment at: clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c:36
+// CHECK32: ret <4 x bfloat> %vld1_lane
+
+bfloat16x8_t test_vld1q_lane_bf16(bfloat16_t const *ptr, bfloat16x8_t src) {

labrinea wrote:
> CHECK-NEXT or CHECK-DAG are preferable for sequences.
Added to be consistent with the rest of the file (ie no CHECK-NEXT, but 
CHECK32/64)



Comment at: clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c:180
+// %.fca.0.2.insert = insertvalue %struct.bfloat16x4x3_t %.fca.0.1.insert, <4 
x bfloat> %vld3_lane.fca.2.extract, 0, 2
+
+bfloat16x8x3_t test_vld3q_lane_bf16(bfloat16_t const *ptr, bfloat16x8x3_t src) 
{

labrinea wrote:
> where are the check lines?
Added to be consistent with the rest of the file


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

https://reviews.llvm.org/D80716



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

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

My experience with clang-tidy has been that template instantiations are a 
double-edged sword. The instantiation is the only place at which you have 
sufficient information to perform many kinds of analyses, but it's also often 
not plausible to modify the templated code because it's not clear from the 
instantiation context how changes may impact other instantiations. The result 
is that most of the clang-tidy checks wind up punting on template 
instantiations similar to what they do for macros. Based on that experience, I 
think it makes sense to continue in the proposed direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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

cfe-commits@lists.llvm.org

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson updated this revision to Diff 267896.
LukeGeeson marked 4 inline comments as done.

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

https://reviews.llvm.org/D80716

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
  clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/aarch64-bf16-dotprod-intrinsics.ll
  llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll

Index: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll
@@ -0,0 +1,826 @@
+; RUN: llc -mtriple aarch64-arm-none-eabi  -mattr=+bf16 %s -o - | FileCheck %s
+
+%struct.bfloat16x4x2_t = type { [2 x <4 x bfloat>] }
+%struct.bfloat16x8x2_t = type { [2 x <8 x bfloat>] }
+%struct.bfloat16x4x3_t = type { [3 x <4 x bfloat>] }
+%struct.bfloat16x8x3_t = type { [3 x <8 x bfloat>] }
+%struct.bfloat16x4x4_t = type { [4 x <4 x bfloat>] }
+%struct.bfloat16x8x4_t = type { [4 x <8 x bfloat>] }
+
+; CHECK-LABEL: test_vld1_bf16
+; CHECK: ldr	d0, [x0]
+define <4 x bfloat> @test_vld1_bf16(bfloat* nocapture readonly %ptr) local_unnamed_addr #0 {
+entry:
+  %0 = bitcast bfloat* %ptr to <4 x bfloat>*
+  %1 = load <4 x bfloat>, <4 x bfloat>* %0, align 2
+  ret <4 x bfloat> %1
+}
+
+; CHECK-LABEL: test_vld1q_bf16
+; CHECK: ldr	q0, [x0]
+define <8 x bfloat> @test_vld1q_bf16(bfloat* nocapture readonly %ptr) local_unnamed_addr #1 {
+entry:
+  %0 = bitcast bfloat* %ptr to <8 x bfloat>*
+  %1 = load <8 x bfloat>, <8 x bfloat>* %0, align 2
+  ret <8 x bfloat> %1
+}
+
+; CHECK-LABEL: test_vld1_lane_bf16
+; CHECK: ld1	{ v0.h }[0], [x0]
+define <4 x bfloat> @test_vld1_lane_bf16(bfloat* nocapture readonly %ptr, <4 x bfloat> %src) local_unnamed_addr #0 {
+entry:
+  %0 = load bfloat, bfloat* %ptr, align 2
+  %vld1_lane = insertelement <4 x bfloat> %src, bfloat %0, i32 0
+  ret <4 x bfloat> %vld1_lane
+}
+
+; CHECK-LABEL: test_vld1q_lane_bf16
+; CHECK: ld1	{ v0.h }[7], [x0]
+define <8 x bfloat> @test_vld1q_lane_bf16(bfloat* nocapture readonly %ptr, <8 x bfloat> %src) local_unnamed_addr #1 {
+entry:
+  %0 = load bfloat, bfloat* %ptr, align 2
+  %vld1_lane = insertelement <8 x bfloat> %src, bfloat %0, i32 7
+  ret <8 x bfloat> %vld1_lane
+}
+
+; CHECK-LABEL: test_vld1_dup_bf16
+; CHECK: ld1r	{ v0.4h }, [x0]
+define <4 x bfloat> @test_vld1_dup_bf16(bfloat* nocapture readonly %ptr) local_unnamed_addr #0 {
+entry:
+  %0 = load bfloat, bfloat* %ptr, align 2
+  %1 = insertelement <4 x bfloat> undef, bfloat %0, i32 0
+  %lane = shufflevector <4 x bfloat> %1, <4 x bfloat> undef, <4 x i32> zeroinitializer
+  ret <4 x bfloat> %lane
+}
+
+; CHECK-LABEL: test_vld1_bf16_x2
+; CHECK: ld1	{ v0.4h, v1.4h }, [x0]
+define %struct.bfloat16x4x2_t @test_vld1_bf16_x2(bfloat* %ptr) local_unnamed_addr #2 {
+entry:
+  %vld1xN = tail call { <4 x bfloat>, <4 x bfloat> } @llvm.aarch64.neon.ld1x2.v4bf16.p0bf16(bfloat* %ptr)
+  %vld1xN.fca.0.extract = extractvalue { <4 x bfloat>, <4 x bfloat> } %vld1xN, 0
+  %vld1xN.fca.1.extract = extractvalue { <4 x bfloat>, <4 x bfloat> } %vld1xN, 1
+  %.fca.0.0.insert = insertvalue %struct.bfloat16x4x2_t undef, <4 x bfloat> %vld1xN.fca.0.extract, 0, 0
+  %.fca.0.1.insert = insertvalue %struct.bfloat16x4x2_t %.fca.0.0.insert, <4 x bfloat> %vld1xN.fca.1.extract, 0, 1
+  ret %struct.bfloat16x4x2_t %.fca.0.1.insert
+}
+
+declare { <4 x bfloat>, <4 x bfloat> } @llvm.aarch64.neon.ld1x2.v4bf16.p0bf16(bfloat*) #3
+
+; CHECK-LABEL: test_vld1q_bf16_x2
+; CHECK: ld1	{ v0.8h, v1.8h }, [x0]
+define %struct.bfloat16x8x2_t @test_vld1q_bf16_x2(bfloat* %ptr) local_unnamed_addr #2 {
+entry:
+  %vld1xN = tail call { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld1x2.v8bf16.p0bf16(bfloat* %ptr)
+  %vld1xN.fca.0.extract = extractvalue { <8 x bfloat>, <8 x bfloat> } %vld1xN, 0
+  %vld1xN.fca.1.extract = extractvalue { <8 x bfloat>, <8 x bfloat> } %vld1xN, 1
+  %.fca.0.0.insert = insertvalue %struct.bfloat16x8x2_t undef, <8 x bfloat> %vld1xN.fca.0.extract, 0, 0
+  %.fca.0.1.insert = insertvalue %struct.bfloat16x8x2_t %.fca.0.0.insert, <8 x bfloat> %vld1xN.fca.1.extract, 0, 1
+  ret %struct.bfloat16x8x2_t %.fca.0.1.insert
+}
+
+; Function Attrs: argmemonly nounwind readonly
+declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld1x2.v8bf16.p0bf16(bfloat*) #3
+
+; CHECK-LABEL: test_vld1_bf16_x3
+; CHECK: ld1	{ v0.4h, v1.4h, v2.4h }, [x0]
+define %struct.bfloat16x4x3_t @test_vld1_bf16_x3(bfloat* %ptr) local_unnamed_addr #2 {
+entry:
+  %vld1xN = tail call { <4 x bfloat>, <4 x bfloat>, <4 x bfloat> } @llvm.aarch64.neon.ld1x3.v4bf16.p0bf16(bfloat* %ptr)
+  %vld1xN.fca.0.extract = extractvalue { <4 x bfloat>, <4 x bfloat>, <4 x bfloat> } %vld1xN, 0
+  %vld1xN.fca.1.extract = ex

cfe-commits@lists.llvm.org

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson added a comment.

In D80716#2059977 , @stuij wrote:

> We need testing for the backend code.


@stuij  I have added `aarch64-bf16-ldst-intrinsics.ll` to test the backend. 
Please let me know if this is ok :)


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

https://reviews.llvm.org/D80716



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

[PATCH] D80439: Replace separator in OpenMP variant name mangling.

[Lukas Sommer via Phabricator via cfe-commits]

LukasSommerTu added a comment.

@jdoerfert: I've added a test. I kept it focused on the variants for NVPTX 
architecture (mangling is currently the same for all archs), let me know if 
that was what you had in mind.


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

https://reviews.llvm.org/D80439



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

[PATCH] D80439: Replace separator in OpenMP variant name mangling.

[Lukas Sommer via Phabricator via cfe-commits]

LukasSommerTu updated this revision to Diff 267902.
LukasSommerTu added a comment.

Added a small test checking for functions with the correctly mangled name.


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

https://reviews.llvm.org/D80439

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/OpenMPClause.cpp
  clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp

Index: clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
===
--- /dev/null
+++ clang/test/OpenMP/nvptx_declare_variant_name_mangling.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -fopenmp-version=50
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -fopenmp-version=50 | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv}'
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -emit-pch -o %t -fopenmp-version=50
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - -fopenmp-version=50 | FileCheck %s --implicit-check-not='call i32 {@_Z3bazv|@_Z3barv}'
+// expected-no-diagnostics
+
+// CHECK-DAG: @_Z3barv
+// CHECK-DAG: @_Z3bazv
+// CHECK-DAG: @"_Z54bar$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"
+// CHECK-DAG: @"_Z54baz$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"
+// CHECK-DAG: call i32 @"_Z54bar$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"()
+// CHECK-DAG: call i32 @"_Z54baz$ompvariant$S2$s8$Pnvptx$Pnvptx64$S3$s10$Pmatch_anyv"()
+
+#ifndef HEADER
+#define HEADER
+
+#pragma omp declare target
+
+int bar() { return 1; }
+
+int baz() { return 5; }
+
+#pragma omp begin declare variant match(device = {arch(nvptx, nvptx64)}, implementation = {extension(match_any)})
+
+int bar() { return 2; }
+
+int baz() { return 6; }
+
+#pragma omp end declare variant
+
+#pragma omp end declare target
+
+int main() {
+  int res;
+#pragma omp target map(from \
+   : res)
+  res = bar() + baz();
+  return res;
+}
+
+#endif
\ No newline at end of file
Index: clang/lib/AST/OpenMPClause.cpp
===
--- clang/lib/AST/OpenMPClause.cpp
+++ clang/lib/AST/OpenMPClause.cpp
@@ -2109,22 +2109,21 @@
   std::string MangledName;
   llvm::raw_string_ostream OS(MangledName);
   for (const OMPTraitSet &Set : Sets) {
-OS << '.' << 'S' << unsigned(Set.Kind);
+OS << '$' << 'S' << unsigned(Set.Kind);
 for (const OMPTraitSelector &Selector : Set.Selectors) {
 
   bool AllowsTraitScore = false;
   bool RequiresProperty = false;
   isValidTraitSelectorForTraitSet(
   Selector.Kind, Set.Kind, AllowsTraitScore, RequiresProperty);
-  OS << '.' << 's' << unsigned(Selector.Kind);
+  OS << '$' << 's' << unsigned(Selector.Kind);
 
   if (!RequiresProperty ||
   Selector.Kind == TraitSelector::user_condition)
 continue;
 
   for (const OMPTraitProperty &Property : Selector.Properties)
-OS << '.' << 'P'
-   << getOpenMPContextTraitPropertyName(Property.Kind);
+OS << '$' << 'P' << getOpenMPContextTraitPropertyName(Property.Kind);
 }
   }
   return OS.str();
@@ -2133,7 +2132,7 @@
 OMPTraitInfo::OMPTraitInfo(StringRef MangledName) {
   unsigned long U;
   do {
-if (!MangledName.consume_front(".S"))
+if (!MangledName.consume_front("$S"))
   break;
 if (MangledName.consumeInteger(10, U))
   break;
@@ -2141,7 +2140,7 @@
 OMPTraitSet &Set = Sets.back();
 Set.Kind = TraitSet(U);
 do {
-  if (!MangledName.consume_front(".s"))
+  if (!MangledName.consume_front("$s"))
 break;
   if (MangledName.consumeInteger(10, U))
 break;
@@ -2149,11 +2148,11 @@
   OMPTraitSelector &Selector = Set.Selectors.back();
   Selector.Kind = TraitSelector(U);
   do {
-if (!MangledName.consume_front(".P"))
+if (!MangledName.consume_front("$P"))
   break;
 Selector.Properties.push_back(OMPTraitProperty());
 OMPTraitProperty &Property = Selector.Properties.back();
-std::pair PropRestPair = MangledName.split('.');
+std::pair PropRestPair = MangledName.split('$');
 Property.Kind =
 getOpenMPContextTraitPropertyKind(Set.Kind, PropRestPair.first);
 MangledName = PropRestPair.second;
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267901.
pestctrl added a comment.

Updated test with new error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+i

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

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

Marked more comments as "done" after going through the code again.




Comment at: 
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:49
+R"cpp({0}(const {0} &) = delete;
+const {0} &operator=(const {0} &) = delete{1})cpp",
+classIdent->getNameStart(), shouldAppendSemi(Range) ? ";" : "");

kwk wrote:
> njames93 wrote:
> > if you wanted to, you could format this quite easily to avoid the need of 
> > specifying a formatter in the checks
> > ```{2}const {0} &operator=(const {0} &) = delete{1})cpp",...
> > Lexer::getIndentationForLine(Range.getBegin(), SM))```
> Well, that does fix the indentation but for east/west-side alignment of `&` 
> this doesn't solve the problem, does it?
Well, yes but it will only work for the indentation not for anything else, like 
east/west alignment of `&` for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80531



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