[PATCH] D63956: [analyzer] NonnullGlobalConstants: Don't be confused with a _Nonnull attribute.

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

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LG!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63956



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

Actually, I'm not yet ready to commit this.  I want to enforce the 8548 -> e500 
processor model before I call this ready.  How do I do that with the mcpu?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

Re: r363429 - PR23833, DR2140: an lvalue-to-rvalue conversion on a glvalue of type

[Hubert Tong via cfe-commits]

I don't see how the resolution of Core Issue 2140 changes the status of
nullptr_t lvalue-to-rvalue conversion for constexpr evaluation. PR42440 has
been opened concerning the change to constexpr evaluation.

-- HT

On Fri, Jun 14, 2019 at 1:43 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Fri Jun 14 10:46:38 2019
> New Revision: 363429
>
> URL: http://llvm.org/viewvc/llvm-project?rev=363429&view=rev
> Log:
> PR23833, DR2140: an lvalue-to-rvalue conversion on a glvalue of type
> nullptr_t does not access memory.
>
> We now reuse CK_NullToPointer to represent a conversion from a glvalue
> of type nullptr_t to a prvalue of nullptr_t where necessary.
>
> This reinstates r363337, reverted in r363352.
>
> Modified:
> cfe/trunk/lib/AST/Expr.cpp
> cfe/trunk/lib/CodeGen/CGExprAgg.cpp
> cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
> cfe/trunk/test/Analysis/nullptr.cpp
> cfe/trunk/test/CXX/drs/dr21xx.cpp
> cfe/trunk/test/CodeGenCXX/nullptr.cpp
> cfe/trunk/www/cxx_dr_status.html
>
> Modified: cfe/trunk/lib/AST/Expr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=363429&r1=363428&r2=363429&view=diff
>
> ==
> --- cfe/trunk/lib/AST/Expr.cpp (original)
> +++ cfe/trunk/lib/AST/Expr.cpp Fri Jun 14 10:46:38 2019
> @@ -1885,6 +1885,11 @@ ImplicitCastExpr *ImplicitCastExpr::Crea
> ExprValueKind VK) {
>unsigned PathSize = (BasePath ? BasePath->size() : 0);
>void *Buffer = C.Allocate(totalSizeToAlloc *>(PathSize));
> +  // Per C++ [conv.lval]p3, lvalue-to-rvalue conversions on class and
> +  // std::nullptr_t have special semantics not captured by
> CK_LValueToRValue.
> +  assert((Kind != CK_LValueToRValue ||
> +  !(T->isNullPtrType() || T->getAsCXXRecordDecl())) &&
> + "invalid type for lvalue-to-rvalue conversion");
>ImplicitCastExpr *E =
>  new (Buffer) ImplicitCastExpr(T, Kind, Operand, PathSize, VK);
>if (PathSize)
>
> Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=363429&r1=363428&r2=363429&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Fri Jun 14 10:46:38 2019
> @@ -1352,7 +1352,8 @@ static bool isSimpleZero(const Expr *E,
>// (int*)0 - Null pointer expressions.
>if (const CastExpr *ICE = dyn_cast(E))
>  return ICE->getCastKind() == CK_NullToPointer &&
> -CGF.getTypes().isPointerZeroInitializable(E->getType());
> +   CGF.getTypes().isPointerZeroInitializable(E->getType()) &&
> +   !E->HasSideEffects(CGF.getContext());
>// '\0'
>if (const CharacterLiteral *CL = dyn_cast(E))
>  return CL->getValue() == 0;
>
> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=363429&r1=363428&r2=363429&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Jun 14 10:46:38 2019
> @@ -2148,14 +2148,14 @@ Value *ScalarExprEmitter::VisitCastExpr(
>
>case CK_NullToPointer:
>  if (MustVisitNullValue(E))
> -  (void) Visit(E);
> +  CGF.EmitIgnoredExpr(E);
>
>  return
> CGF.CGM.getNullPointer(cast(ConvertType(DestTy)),
>DestTy);
>
>case CK_NullToMemberPointer: {
>  if (MustVisitNullValue(E))
> -  (void) Visit(E);
> +  CGF.EmitIgnoredExpr(E);
>
>  const MemberPointerType *MPT =
> CE->getType()->getAs();
>  return CGF.CGM.getCXXABI().EmitNullMemberPointer(MPT);
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=363429&r1=363428&r2=363429&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Jun 14 10:46:38 2019
> @@ -635,8 +635,10 @@ ExprResult Sema::DefaultLvalueConversion
>if (E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
>  Cleanup.setExprNeedsCleanups(true);
>
> -  Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
> nullptr,
> - VK_RValue);
> +  // C++ [conv.lval]p3:
> +  //   If T is cv std::nullptr_t, the result is a null pointer constant.
> +  CastKind CK = T->isNullPtrType() ? CK_NullToPointer : CK_LValueToRValue;
> +  Res = ImplicitCastExpr::Create(Context, T, CK, E, nullptr, VK_RValue);
>
>// C11 6.3.2.1p2:
>//   ...

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > It looks like you're generally warning about this based on the specific 
> > > > context that forced an lvalue-to-rvalue conversion.  I'm not sure 
> > > > `volatile` is special except that we actually perform the load even in 
> > > > unused-value contexts.  Is the assumption that you've exhaustively 
> > > > covered all the other contexts of lvalue-to-rvalue conversions whose 
> > > > values will actually be used?  That seems questionable to me.
> > > Yes, that was my assumption. All the other contexts where 
> > > lvalue-to-rvalue conversion is performed and the result is used are 
> > > already covered by other calls sites of `checkNonTrivialCUnion`, which 
> > > informs the users that the struct/union is being used in an invalid 
> > > context.
> > > 
> > > Do you have a case in mind that I didn't think of where a 
> > > lvalue-to-rvalue conversion requires a non-trivial 
> > > initialization/destruction/copying of a union but clang fails to emit any 
> > > diagnostics?
> > > 
> > > Also I realized that lvalue-to-rvalue conversion of volatile types 
> > > doesn't always require non-trivial destruction, so I think 
> > > `CheckDestruct` shouldn't be set in this case.
> > > 
> > > ```
> > > void foo(U0 *a, volatile U0 *b) {
> > >   // this doesn't require destruction.
> > >   // this is perfectly valid if U0 is non-trivial to destruct but trivial 
> > > to copy.
> > >   *a = *b;  
> > > }
> > > ```
> > > 
> > > For the same reason, I think `CheckDestruct` shouldn't be set for 
> > > function returns (but it should be set for function parameters since they 
> > > are destructed by the callee).
> > There are a *lot* of places that trigger lvalue-to-rvalue conversion.  Many 
> > of them aren't legal with structs (in C), but I'm worried about approving a 
> > pattern with the potential to be wrong by default just because we didn't 
> > think about some weird case.  As an example, casts can trigger 
> > lvalue-to-rvalue conversion; I think the only casts allowed with structs 
> > are the identity cast and the cast to `void`, but those are indeed allowed. 
> >  Now, a cast to `void` means the value is ignored, so we can elide a 
> > non-volatile load in the operand, and an identity cast isn't terminal; if 
> > the idea is that we're checking all the *terminal* uses of a struct 
> > r-value, then we're in much more restricted territory (and don't need to 
> > worry about things like commas and conditional operators that can propagate 
> > values out).  But this still worries me.
> > 
> > I'm not sure we need to be super-pedantic about destructibility vs. 
> > copyability in some of this checking.  It's certain possible in C++, but I 
> > can't imagine what sort of *primitive* type would be trivially copyable but 
> > not trivially destructible.  (The reverse isn't true: something like a 
> > relative pointer would be non-trivially copyable but still trivially 
> > destructible.)
> > 
> > Is there any way to make this check cheaper so that we can immediately 
> > avoid any further work for types that are obviously copyable/destructible?  
> > All the restricted types are (possibly arrays of) record types, right?
> I'm not sure I fully understand what you are saying, but by "cheaper", do you 
> mean fewer and simpler rules for allowing or disallowing non-trivial unions 
> even when that might result in rejecting unions used in contexts in which 
> non-trivial initialization/destruction/copying is not required? If so, we can 
> probably diagnose any lvalue-to-rvalue conversions regardless of whether the 
> source is volatile if the type is either non-trivial to copy or destruct.
Sorry, that point was separate from the discussion of `volatile` and 
lvalue-to-rvalue conversions.  I mean that you're changing a lot of core paths 
in Sema, and it would be nice if we could very quickly decide based on the type 
that no restrictions apply instead of having to make a function call, a switch, 
and a bunch of other calls in order to realize that e.g. `void*` never needs 
additional checking.  Currently you have a `!CPlusPlus` check in front of all 
the `checkNonTrivialCUnion` calls; I would like something that reliably avoids 
doing this work for the vast majority of types that are not restricted, even in 
C.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753



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

[PATCH] D58164: Block+lambda: allow reference capture

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  In that case, yeah, it'd be good to get Richard's opinion about the 
right way to get that information here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58164



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

[PATCH] D62645: [Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, looks much better.




Comment at: include/clang/Sema/Sema.h:2066
   bool DeduceVariableDeclarationType(VarDecl *VDecl, bool DirectInit,
- Expr *Init);
+ Expr *Init, bool DefaultedAnyToId);
   void CheckCompleteVariableDeclaration(VarDecl *VD);

A shame this needs to be propagated down like this.  Is there a reasonable way 
to just mark on the declaration that we defaulted the type, like an unparseable 
attribute?  This might be interesting to other clients ultimately.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62645



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

[PATCH] D63697: clang-format: Fix error return when using inplace with stdin

[William Woodruff via Phabricator via cfe-commits]

woodruffw added a comment.

Yep, I'll add one in a bit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63697



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

[PATCH] D63822: [Driver] Fix style issues of --print-supported-cpus

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364704: [Driver] Fix style issues of --print-supported-cpus 
after D63105 (authored by MaskRay, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63822

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Driver/print-supported-cpus.c
  cfe/trunk/tools/driver/cc1_main.cpp

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2644,12 +2644,12 @@
 def s : Flag<["-"], "s">, Group;
 def target : Joined<["--"], "target=">, Flags<[DriverOption, CoreOption]>,
   HelpText<"Generate code for the given target">;
-def _print_supported_cpus : Flag<["-", "--"], "print-supported-cpus">,
+def print_supported_cpus : Flag<["-", "--"], "print-supported-cpus">,
   Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Print supported cpu models for the given target (if target is not specified,"
" it will print the supported cpus for the default target)">;
-def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias<_print_supported_cpus>;
-def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias<_print_supported_cpus>;
+def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
+def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
 def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[DriverOption]>,
   HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,
Index: cfe/trunk/test/Driver/print-supported-cpus.c
===
--- cfe/trunk/test/Driver/print-supported-cpus.c
+++ cfe/trunk/test/Driver/print-supported-cpus.c
@@ -1,35 +1,27 @@
-// Test that the --print-supported-cpus flag works
-// Also test its aliases: -mcpu=? and -mtune=?
+// Test that --print-supported-cpus lists supported CPU models.
 
 // REQUIRES: x86-registered-target
 // REQUIRES: arm-registered-target
 
-// RUN: %clang --target=x86_64-unknown-linux-gnu \
-// RUN:   --print-supported-cpus 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-X86
+// RUN: %clang --target=x86_64-unknown-linux-gnu --print-supported-cpus 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-X86
+
+// Test -mcpu=? and -mtune=? alises.
+// RUN: %clang --target=x86_64-unknown-linux-gnu -mcpu=? 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-X86
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu -mtune=? -fuse-ld=dummy 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-X86
+
+// CHECK-NOT: warning: argument unused during compilation
 // CHECK-X86: Target: x86_64-unknown-linux-gnu
 // CHECK-X86: corei7
 // CHECK-X86: Use -mcpu or -mtune to specify the target's processor.
 
-// RUN: %clang --target=x86_64-unknown-linux-gnu \
-// RUN:   -mcpu=? 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-X86-MCPU
-// CHECK-X86-MCPU: Target: x86_64-unknown-linux-gnu
-// CHECK-X86-MCPU: corei7
-// CHECK-X86-MCPU: Use -mcpu or -mtune to specify the target's processor.
-
-// RUN: %clang --target=arm-unknown-linux-android \
-// RUN:   --print-supported-cpus 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ARM
+// RUN: %clang --target=arm-unknown-linux-android --print-supported-cpus 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ARM
+
 // CHECK-ARM: Target: arm-unknown-linux-android
 // CHECK-ARM: cortex-a73
 // CHECK-ARM: cortex-a75
 // CHECK-ARM: Use -mcpu or -mtune to specify the target's processor.
-
-// RUN: %clang --target=arm-unknown-linux-android \
-// RUN:   -mtune=? 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ARM-MTUNE
-// CHECK-ARM-MTUNE: Target: arm-unknown-linux-android
-// CHECK-ARM-MTUNE: cortex-a73
-// CHECK-ARM-MTUNE: cortex-a75
-// CHECK-ARM-MTUNE: Use -mcpu or -mtune to specify the target's processor.
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -280,6 +280,7 @@
 
 // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_fsyntax_only)) ||
+ (PhaseArg = DAL.getLastArg(options::OPT_print_supported_cpus)) ||
  (PhaseArg = DAL.getLastArg(options::OPT_module_file_info)) ||
  (PhaseArg = DAL.getLastArg(options::OPT_verify_pch)) ||
  (PhaseArg = DAL.getLastArg(options::OPT_rewrite_objc)) ||
@@ -1673,7 +1674,7 @@
 
   if (C.getArgs().hasArg(options::OPT_v) ||
   C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
-  C.getArgs().hasArg(options::OPT__print_supported_cpus)) {
+  C.getArgs().hasArg(options::OPT_print

r364704 - [Driver] Fix style issues of --print-supported-cpus after D63105

[Fangrui Song via cfe-commits]

Author: maskray
Date: Fri Jun 28 18:24:36 2019
New Revision: 364704

URL: http://llvm.org/viewvc/llvm-project?rev=364704&view=rev
Log:
[Driver] Fix style issues of --print-supported-cpus after D63105

Reviewed By: ziangwan

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/print-supported-cpus.c
cfe/trunk/tools/driver/cc1_main.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=364704&r1=364703&r2=364704&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Jun 28 18:24:36 2019
@@ -2644,12 +2644,12 @@ def : Separate<["--"], "no-system-header
 def s : Flag<["-"], "s">, Group;
 def target : Joined<["--"], "target=">, Flags<[DriverOption, CoreOption]>,
   HelpText<"Generate code for the given target">;
-def _print_supported_cpus : Flag<["-", "--"], "print-supported-cpus">,
+def print_supported_cpus : Flag<["-", "--"], "print-supported-cpus">,
   Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Print supported cpu models for the given target (if target is not 
specified,"
" it will print the supported cpus for the default target)">;
-def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias<_print_supported_cpus>;
-def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias<_print_supported_cpus>;
+def mcpu_EQ_QUESTION : Flag<["-"], "mcpu=?">, Alias;
+def mtune_EQ_QUESTION : Flag<["-"], "mtune=?">, Alias;
 def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[DriverOption]>,
   HelpText<"Use the gcc toolchain at the given directory">;
 def time : Flag<["-"], "time">,

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=364704&r1=364703&r2=364704&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Fri Jun 28 18:24:36 2019
@@ -280,6 +280,7 @@ phases::ID Driver::getFinalPhase(const D
 
 // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_fsyntax_only)) ||
+ (PhaseArg = DAL.getLastArg(options::OPT_print_supported_cpus)) ||
  (PhaseArg = DAL.getLastArg(options::OPT_module_file_info)) ||
  (PhaseArg = DAL.getLastArg(options::OPT_verify_pch)) ||
  (PhaseArg = DAL.getLastArg(options::OPT_rewrite_objc)) ||
@@ -1673,7 +1674,7 @@ bool Driver::HandleImmediateArgs(const C
 
   if (C.getArgs().hasArg(options::OPT_v) ||
   C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
-  C.getArgs().hasArg(options::OPT__print_supported_cpus)) {
+  C.getArgs().hasArg(options::OPT_print_supported_cpus)) {
 PrintVersion(C, llvm::errs());
 SuppressMissingInputWarning = true;
   }
@@ -3377,21 +3378,16 @@ void Driver::BuildActions(Compilation &C
 Args.ClaimAllArgs(options::OPT_cl_compile_Group);
   }
 
-  // if the user specify --print-supported-cpus, or use -mcpu=?, or use
-  // -mtune=? (aliases), clang will only print out supported cpu names
-  // without doing compilation.
-  if (Arg *A = Args.getLastArg(options::OPT__print_supported_cpus)) {
-// the compilation now has only two phases: Input and Compile
-// use the --prints-supported-cpus flag as the dummy input to cc1
+  // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a 
custom
+  // Compile phase that prints out supported cpu models and quits.
+  if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {
+// Use the -mcpu=? flag as the dummy input to cc1.
 Actions.clear();
 Action *InputAc = C.MakeAction(*A, types::TY_C);
 Actions.push_back(
 C.MakeAction(InputAc, types::TY_Nothing));
-// claim all the input files to prevent argument unused warnings
-for (auto &I : Inputs) {
-  const Arg *InputArg = I.second;
-  InputArg->claim();
-}
+for (auto &I : Inputs)
+  I.second->claim();
   }
 
   // Claim ignored clang-cl options.

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=364704&r1=364703&r2=364704&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Jun 28 18:24:36 2019
@@ -1767,7 +1767,7 @@ static InputKind ParseFrontendArgs(Front
   Opts.ShowHelp = Args.hasArg(OPT_help);
   Opts.ShowStats = Args.hasArg(OPT_print_stats);
   Opts.ShowTimers = Args.hasArg(OPT_ftime_report);
-  Opts.PrintSupportedCPUs = Args.has

[PATCH] D63968: [analyzer] Fix target region invalidation when returning into a ctor initializer.

[Csaba Dabis via Phabricator via cfe-commits]

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

You have made a great use of that mistake by me. So that is why everything is 
so perfect, because it fires, just it should fire later. Thanks you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63968



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

[PATCH] D63968: [analyzer] Fix target region invalidation when returning into a ctor initializer.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.

Enabling pointer escape notification for the return value invalidation when 
conservatively evaluating calls that return C++ objects by value was supposed 
to be pointless. Indeed, we're looking at a freshly conjured value; why would 
any checker already track it at this point? Yet it does cause a change in 
results, so i decided to reduce and investigate a reproducer @Charusso sent me.

When i was thinking about it previously (as of D44131 
), i was imagining a function that constructs 
a temporary that would later be copied to its target destination. In this case 
there's indeed no need to notify checkers of pointer escape.

However, once RVO was implemented (D47671 ), 
the target region is no longer necessarily a temporary; it may be an arbitrary 
memory region. In particular, it may be a non-base region, such as 
`FieldRegion` if the construction context is a 
`ConstructorInitializerConstructionContext`. In this case the invalidation 
covers not only the target region but its whole base region that may already 
contain some values that might as well be tracked by the checkers.

The right solution, therefore, is to restrict invalidation so that it didn't 
propagate to the base region, but only wipe the target region itself. It 
probably doesn't work perfectly because RegionStore doesn't enjoy this sort of 
stuff, but it should be something.

In the newly added test case the previous behavior was to immediately forget 
about `b1.p` and `b2.p`, therefore evals on lines 21 and 23 yielded both `TRUE` 
and `FALSE` each (due to eagerly-assume) and the leak was diagnosed on line 22 
(even though the pointer is used later).


Repository:
  rC Clang

https://reviews.llvm.org/D63968

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/rvo.cpp


Index: clang/test/Analysis/rvo.cpp
===
--- /dev/null
+++ clang/test/Analysis/rvo.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker core,cplusplus \
+// RUN:-analyzer-checker debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+};
+
+A getA();
+
+struct B {
+  int *p;
+  A a;
+
+  B(int *p) : p(p), a(getA()) {}
+};
+
+void foo() {
+  B b1(nullptr);
+  clang_analyzer_eval(b1.p == nullptr); // expected-warning{{TRUE}}
+  B b2(new int); // No leak yet!
+  clang_analyzer_eval(b2.p == nullptr); // expected-warning{{FALSE}}
+  // expected-warning@-1{{Potential leak of memory pointed to by 'b2.p'}}
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -634,12 +634,19 @@
 std::tie(State, Target) =
 prepareForObjectConstruction(Call.getOriginExpr(), State, LCtx,
  RTC->getConstructionContext(), CallOpts);
-assert(Target.getAsRegion());
-// Invalidate the region so that it didn't look uninitialized. Don't notify
-// the checkers.
-State = State->invalidateRegions(Target.getAsRegion(), E, Count, LCtx,
+const MemRegion *TargetR = Target.getAsRegion();
+assert(TargetR);
+// Invalidate the region so that it didn't look uninitialized. If this is
+// a field or element constructor, we do not want to invalidate
+// the whole structure. Pointer escape is meaningless because
+// the structure is a product of conservative evaluation
+// and therefore contains nothing interesting at this point.
+RegionAndSymbolInvalidationTraits ITraits;
+ITraits.setTrait(TargetR,
+RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion);
+State = State->invalidateRegions(TargetR, E, Count, LCtx,
  /* CausedByPointerEscape=*/false, nullptr,
- &Call, nullptr);
+ &Call, &ITraits);
 
 R = State->getSVal(Target.castAs(), E->getType());
   } else {


Index: clang/test/Analysis/rvo.cpp
===
--- /dev/null
+++ clang/test/Analysis/rvo.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker core,cplusplus \
+// RUN:-analyzer-checker debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+};
+
+A getA();
+
+struct B {
+  int *p;
+  A a;
+
+  B(int *p) : p(p), a(getA()) {}
+};
+
+void foo() {
+  B b1(nullptr);
+  clang_analyzer_eval(b1.p == nullptr); // expected-warni

Re: [PATCH] D63155: [clang][NewPM] Fix broken profile test

[Xinliang David Li via cfe-commits]

I agree that the  test coverage needs to be improved eg better check etc.

David


On Fri, Jun 28, 2019 at 5:21 PM Chandler Carruth via Phabricator via
llvm-commits  wrote:

> chandlerc added a comment.
>
> In D63155#1563275 , @xur wrote:
>
> > In D63155#1563266 , @chandlerc
> wrote:
> >
> > > I just think we also need to understand why *no other test failed*,
> and why the only test we have for doing PGO at O0 is this one and it could
> be "fixed" but such a deeply unrelated change
> >
> >
> > We have special code to do PGO at O0 in old pass manager. This is not
> done in the new pass manager.
>
>
> I'm not sure how this addresses my question about test coverage.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D63155/new/
>
> https://reviews.llvm.org/D63155
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

In D63155#1563275 , @xur wrote:

> In D63155#1563266 , @chandlerc wrote:
>
> > I just think we also need to understand why *no other test failed*, and why 
> > the only test we have for doing PGO at O0 is this one and it could be 
> > "fixed" but such a deeply unrelated change
>
>
> We have special code to do PGO at O0 in old pass manager. This is not done in 
> the new pass manager.


I'm not sure how this addresses my question about test coverage.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63967: Handle IntToPtr in isBytewiseValue

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

This helps with more efficient use of memset for pattern initialization


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63967

Files:
  clang/test/CodeGenCXX/auto-var-init.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/unittests/Analysis/ValueTrackingTest.cpp


Index: llvm/unittests/Analysis/ValueTrackingTest.cpp
===
--- llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -782,11 +782,11 @@
 "i16* inttoptr (i64 0 to i16*)",
 },
 {
-"",
+"i8 -1",
 "i16* inttoptr (i64 -1 to i16*)",
 },
 {
-"",
+"i8 -86",
 "i16* inttoptr (i64 -6148914691236517206 to i16*)",
 },
 {
@@ -794,7 +794,7 @@
 "i16* inttoptr (i48 -1 to i16*)",
 },
 {
-"",
+"i8 -1",
 "i16* inttoptr (i96 -1 to i16*)",
 },
 {
Index: llvm/lib/Analysis/ValueTracking.cpp
===
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -3219,6 +3219,17 @@
 }
   }
 
+  if (auto *CE = dyn_cast(C)) {
+if (CE->getOpcode() == Instruction::IntToPtr) {
+  auto PS = DL.getPointerSizeInBits(
+  cast(CE->getType())->getAddressSpace());
+  return isBytewiseValue(
+  ConstantExpr::getIntegerCast(CE->getOperand(0),
+   Type::getIntNTy(Ctx, PS), false),
+  DL);
+}
+  }
+
   auto Merge = [&](Value *LHS, Value *RHS) -> Value * {
 if (LHS == RHS)
   return LHS;
Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1032,14 +1032,8 @@
 // CHECK:%uninit = alloca [4 x i32*], align
 // CHECK-NEXT:   call void @{{.*}}used{{.*}}%uninit)
 // PATTERN-O1-LABEL: @test_intptr4_uninit()
-// PATTERN-O1:   %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* 
%uninit, i64 0, i64 0
-// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %1, align 16
-// PATTERN-O1-NEXT:  %2 = getelementptr inbounds [4 x i32*], [4 x i32*]* 
%uninit, i64 0, i64 1
-// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %2, align 8
-// PATTERN-O1-NEXT:  %3 = getelementptr inbounds [4 x i32*], [4 x i32*]* 
%uninit, i64 0, i64 2
-// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %3, align 16
-// PATTERN-O1-NEXT:  %4 = getelementptr inbounds [4 x i32*], [4 x i32*]* 
%uninit, i64 0, i64 3
-// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %4, align 8
+// PATTERN-O1:   %1 = bitcast [4 x i32*]* %uninit to i8*
+// PATTERN-O1-NEXT:  call void @llvm.memset.p0i8.i64(i8* nonnull align 16 %1, 
i8 -86, i64 32, i1 false)
 // ZERO-LABEL:   @test_intptr4_uninit()
 // ZERO: call void @llvm.memset{{.*}}, i8 0,
 


Index: llvm/unittests/Analysis/ValueTrackingTest.cpp
===
--- llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -782,11 +782,11 @@
 "i16* inttoptr (i64 0 to i16*)",
 },
 {
-"",
+"i8 -1",
 "i16* inttoptr (i64 -1 to i16*)",
 },
 {
-"",
+"i8 -86",
 "i16* inttoptr (i64 -6148914691236517206 to i16*)",
 },
 {
@@ -794,7 +794,7 @@
 "i16* inttoptr (i48 -1 to i16*)",
 },
 {
-"",
+"i8 -1",
 "i16* inttoptr (i96 -1 to i16*)",
 },
 {
Index: llvm/lib/Analysis/ValueTracking.cpp
===
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -3219,6 +3219,17 @@
 }
   }
 
+  if (auto *CE = dyn_cast(C)) {
+if (CE->getOpcode() == Instruction::IntToPtr) {
+  auto PS = DL.getPointerSizeInBits(
+  cast(CE->getType())->getAddressSpace());
+  return isBytewiseValue(
+  ConstantExpr::getIntegerCast(CE->getOperand(0),
+   Type::getIntNTy(Ctx, PS), false),
+  DL);
+}
+  }
+
   auto Merge = [&](Value *LHS, Value *RHS) -> Value * {
 if (LHS == RHS)
   return LHS;
Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1032,14 +1032,8 @@
 // CHECK:%uninit = alloca [4 x i32*], align
 // CHECK-NEXT:   call void @{{.*}}used{{.*}}%uninit)
 // PATTERN-O1-LABEL: @test_intptr4_uninit()
-// PATTERN-O1:   %1 = get

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Reverted in r364692


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Rong Xu via Phabricator via cfe-commits]

xur added a comment.

In D63155#1563266 , @chandlerc wrote:

> In D63155#1563240 , @leonardchan 
> wrote:
>
> > In D63155#1563229 , @xur wrote:
> >
> > > This patch does not make sense to me.
> > >
> > > The reason of failing with -fexperimental-new-pass-manager is because we 
> > > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > > instrumentation/use passes are under 
> > > PassBuilder::buildPerModuleDefaultPipeline.
> > >
> > > This patch add a pass
> > >
> > >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> > >
> > > which only gives you the wrong signal  of instrumentation is done.
> > >
> > > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > > interactions for thinlto under ldd for CSPGO.
> > >  Regular FDO should not use it.
> > >
> > > The right fix is to enable PGO instrumentation and use in pass builder 
> > > for O0.
> > >
> > > I would like to request to revert this patch.
> >
> >
> > As an alternative, could I instead request that we remove the BackendUtil 
> > changes and just mark the runs in gcc-flag-compatibility.c with 
> > `-fno-experimental-new-pass-manager`. This way we could clarify that under 
> > the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as 
> > is.
>
>
> No, I think we should be doing PGO at O0 in the new PM if we do so in the old 
> PM.
>
> I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct 
> to fix this test case. That seems plausible to me at least, and I think 
> reverting and figuring out what the right way to do it is a fine approach.


Just check the IR, we still not doing instrumentation at O0. This patch just 
mask the error.

> I just think we also need to understand why *no other test failed*, and why 
> the only test we have for doing PGO at O0 is this one and it could be "fixed" 
> but such a deeply unrelated change

We have special code to do PGO at O0 in old pass manager. This is not done in 
the new pass manager.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

r364692 - Revert "[clang][NewPM] Fix broken profile test"

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Fri Jun 28 17:10:22 2019
New Revision: 364692

URL: http://llvm.org/viewvc/llvm-project?rev=364692&view=rev
Log:
Revert "[clang][NewPM] Fix broken profile test"

This reverts commit ab2c0ed01edfec9a9402d03bdf8633b34b73f3a7.

See https://reviews.llvm.org/D63155

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/Profile/gcc-flag-compatibility.c

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=364692&r1=364691&r2=364692&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Fri Jun 28 17:10:22 2019
@@ -60,7 +60,6 @@
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
-#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
@@ -1222,11 +1221,6 @@ void EmitAssemblyHelper::EmitAssemblyWit
 
 if (CodeGenOpts.OptimizationLevel == 0)
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
-
-if (CodeGenOpts.hasProfileIRInstr()) {
-  // This file is stored as the ProfileFile.
-  MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
-}
   }
 
   // FIXME: We still use the legacy pass manager to do code generation. We

Modified: cfe/trunk/test/Profile/gcc-flag-compatibility.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/gcc-flag-compatibility.c?rev=364692&r1=364691&r2=364692&view=diff
==
--- cfe/trunk/test/Profile/gcc-flag-compatibility.c (original)
+++ cfe/trunk/test/Profile/gcc-flag-compatibility.c Fri Jun 28 17:10:22 2019
@@ -7,29 +7,25 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate | FileCheck 
-check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: __llvm_profile_filename
 
 // Check that -fprofile-generate=/path/to generates /path/to/default.profraw
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN-EQ %s
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN-EQ %s
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to | 
FileCheck -check-prefix=PROFILE-GEN-EQ %s
 // PROFILE-GEN-EQ: constant [{{.*}} x i8] c"/path/to{{/|\\5C}}{{.*}}\00"
 
 // Check that -fprofile-use=some/path reads some/path/default.profdata
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir/some/path
 // RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/default.profdata
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE-2 %s
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fexperimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE-2 %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path | FileCheck -check-prefix=PROFILE-USE-2 %s
 // PROFILE-USE-2: = !{!"branch_weights", i32 101, i32 2}
 
 // Check that -fprofile-use=some/path/file.prof reads some/path/file.prof
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir/some/path
 // RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/file.prof
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof -fno-experimental-new-pass-manager | 
FileCheck -check-prefix=PROFILE-USE-3 %s
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof -fexperimental-new-pass-manager | 
FileCheck -check-prefix=PROFILE-USE-3 %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof | FileCheck 
-check-prefix=PROFILE-USE-3 %s
 // PROFILE-USE-3: = !{!"branch_weights", i32 101, i32 2}
 
 int X = 0;


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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Rong Xu via Phabricator via cfe-commits]

xur added a comment.

they are not doing the exacly the same thing for old pass manager and new pass 
manger: old pass manger is doing instrumentation, while the new pass manager 
with this change is NOT.
the test is not check instrumentation, (it only check the variables that 
generates by InstroProfiling pass). 
In this sense, the test is not well written.

I can draft a patch for this.

In D63155#1563239 , @chandlerc wrote:

> In D63155#1563229 , @xur wrote:
>
> > This patch does not make sense to me.
> >
> > The reason of failing with -fexperimental-new-pass-manager is because we 
> > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > instrumentation/use passes are under 
> > PassBuilder::buildPerModuleDefaultPipeline.
> >
> > This patch add a pass
> >
> >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> >
> > which only gives you the wrong signal  of instrumentation is done.
> >
> > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > interactions for thinlto under ldd for CSPGO.
> >  Regular FDO should not use it.
> >
> > The right fix is to enable PGO instrumentation and use in pass builder for 
> > O0.
> >
> > I would like to request to revert this patch.
>
>
> I mean, I'm happy for the patch to be reverted, but I still really don't 
> understand why this fixes the test to work *exactly* the same as w/ the old 
> pass manager and doesn't break any other tests if it is completely wrong? It 
> seems like there must be something strange with the test coverage if this is 
> so far off of correct without any failures...
>
> I also don't understand what fix you are suggesting instead, but maybe you 
> can show a patch?




In D63155#1563266 , @chandlerc wrote:

> In D63155#1563240 , @leonardchan 
> wrote:
>
> > In D63155#1563229 , @xur wrote:
> >
> > > This patch does not make sense to me.
> > >
> > > The reason of failing with -fexperimental-new-pass-manager is because we 
> > > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > > instrumentation/use passes are under 
> > > PassBuilder::buildPerModuleDefaultPipeline.
> > >
> > > This patch add a pass
> > >
> > >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> > >
> > > which only gives you the wrong signal  of instrumentation is done.
> > >
> > > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > > interactions for thinlto under ldd for CSPGO.
> > >  Regular FDO should not use it.
> > >
> > > The right fix is to enable PGO instrumentation and use in pass builder 
> > > for O0.
> > >
> > > I would like to request to revert this patch.
> >
> >
> > As an alternative, could I instead request that we remove the BackendUtil 
> > changes and just mark the runs in gcc-flag-compatibility.c with 
> > `-fno-experimental-new-pass-manager`. This way we could clarify that under 
> > the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as 
> > is.
>
>
> No, I think we should be doing PGO at O0 in the new PM if we do so in the old 
> PM.
>
> I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct 
> to fix this test case. That seems plausible to me at least, and I think 
> reverting and figuring out what the right way to do it is a fine approach.
>
> I just think we also need to understand why *no other test failed*, and why 
> the only test we have for doing PGO at O0 is this one and it could be "fixed" 
> but such a deeply unrelated change





Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Understood. I'll revert this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

In D63155#1563240 , @leonardchan wrote:

> In D63155#1563229 , @xur wrote:
>
> > This patch does not make sense to me.
> >
> > The reason of failing with -fexperimental-new-pass-manager is because we 
> > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > instrumentation/use passes are under 
> > PassBuilder::buildPerModuleDefaultPipeline.
> >
> > This patch add a pass
> >
> >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> >
> > which only gives you the wrong signal  of instrumentation is done.
> >
> > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > interactions for thinlto under ldd for CSPGO.
> >  Regular FDO should not use it.
> >
> > The right fix is to enable PGO instrumentation and use in pass builder for 
> > O0.
> >
> > I would like to request to revert this patch.
>
>
> As an alternative, could I instead request that we remove the BackendUtil 
> changes and just mark the runs in gcc-flag-compatibility.c with 
> `-fno-experimental-new-pass-manager`. This way we could clarify that under 
> the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as is.


No, I think we should be doing PGO at O0 in the new PM if we do so in the old 
PM.

I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct 
to fix this test case. That seems plausible to me at least, and I think 
reverting and figuring out what the right way to do it is a fine approach.

I just think we also need to understand why *no other test failed*, and why the 
only test we have for doing PGO at O0 is this one and it could be "fixed" but 
such a deeply unrelated change


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D62441: [analyzer] NFC: Introduce a convenient CallDescriptionMap class.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso accepted this revision.
Charusso added a comment.

Great patch, thanks you! I wanted to make my own `IdentifierInfo` array 
previously.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1060
+public:
+  const static unsigned NoArgRequirement = 
std::numeric_limits::max();
+

What about `Optional<>`? When I first met that function I have fallen in love.



Comment at: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp:72
+  {{"foo"}, true},
+  }), "void foo(); void bar() { foo(); }"));
+

May it worth to mention the second true/false business stands for the call 
being called.



Comment at: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp:103
+  {{"foo"}, true},
+  }), "void foo(); struct bar { void foo(); }; void test() { foo(); }"));
+

So `{{"bar", "foo"}, true}`? I like puzzles, but it would be cool to state out 
why it is negative as other tests all have negative sub-tests.


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

https://reviews.llvm.org/D62441



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

Re: [PATCH] D63155: [clang][NewPM] Fix broken profile test

[Rong Xu via cfe-commits]

they are not doing the exacly the same thing for old pass manager and new
pass manger: old pass manger is doing instrumentation, while the new pass
manager with this change is NOT.
the test is not check instrumentation, (it only check the variables that
generates by InstroProfiling pass).
In this sense, the test is not well written.

I can draft a patch for this.

On Fri, Jun 28, 2019 at 4:53 PM Chandler Carruth via Phabricator <
revi...@reviews.llvm.org> wrote:

> chandlerc added a comment.
>
> In D63155#1563229 , @xur wrote:
>
> > This patch does not make sense to me.
> >
> > The reason of failing with -fexperimental-new-pass-manager is because we
> don't do PGO instrumentation at -O0. This is due to the fact that PGO
> instrumentation/use passes are under
> PassBuilder::buildPerModuleDefaultPipeline.
> >
> > This patch add a pass
> >
> >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> >
> > which only gives you the wrong signal  of instrumentation is done.
> >
> > I wrote pass PGOInstrumentationGenCreateVar only for some trick
> interactions for thinlto under ldd for CSPGO.
> >  Regular FDO should not use it.
> >
> > The right fix is to enable PGO instrumentation and use in pass builder
> for O0.
> >
> > I would like to request to revert this patch.
>
>
> I mean, I'm happy for the patch to be reverted, but I still really don't
> understand why this fixes the test to work *exactly* the same as w/ the old
> pass manager and doesn't break any other tests if it is completely wrong?
> It seems like there must be something strange with the test coverage if
> this is so far off of correct without any failures...
>
> I also don't understand what fix you are suggesting instead, but maybe you
> can show a patch?
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D63155/new/
>
> https://reviews.llvm.org/D63155
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> I mean, I'm happy for the patch to be reverted, but I still really don't 
> understand why this fixes the test to work *exactly* the same as w/ the old 
> pass manager and doesn't break any other tests if it is completely wrong? It 
> seems like there must be something strange with the test coverage if this is 
> so far off of correct without any failures...
> 
> I also don't understand what fix you are suggesting instead, but maybe you 
> can show a patch?

This is also the fix I'm suggesting.

  diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
  index 5d66473e7b9..f924ecbd8c6 100644
  --- a/clang/lib/CodeGen/BackendUtil.cpp
  +++ b/clang/lib/CodeGen/BackendUtil.cpp
  @@ -1220,12 +1220,13 @@ void 
EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
 }
   }
   
  -if (CodeGenOpts.OptimizationLevel == 0)
  +if (CodeGenOpts.OptimizationLevel == 0) {
 addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
   
  -if (CodeGenOpts.hasProfileIRInstr()) {
  -  // This file is stored as the ProfileFile.
  -  MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
  +  if (CodeGenOpts.hasProfileIRInstr()) {
  +// This file is stored as the ProfileFile.
  +MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
  +  }
   }
 }


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D63155#1563229 , @xur wrote:

> This patch does not make sense to me.
>
> The reason of failing with -fexperimental-new-pass-manager is because we 
> don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> instrumentation/use passes are under 
> PassBuilder::buildPerModuleDefaultPipeline.
>
> This patch add a pass
>
>   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
>
> which only gives you the wrong signal  of instrumentation is done.
>
> I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions 
> for thinlto under ldd for CSPGO.
>  Regular FDO should not use it.
>
> The right fix is to enable PGO instrumentation and use in pass builder for O0.
>
> I would like to request to revert this patch.


As an alternative, could I instead request that we remove the BackendUtil 
changes and just mark the runs in gcc-flag-compatibility.c with 
`-fno-experimental-new-pass-manager`. This way we could clarify that under the 
new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as is.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

In D63155#1563229 , @xur wrote:

> This patch does not make sense to me.
>
> The reason of failing with -fexperimental-new-pass-manager is because we 
> don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> instrumentation/use passes are under 
> PassBuilder::buildPerModuleDefaultPipeline.
>
> This patch add a pass
>
>   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
>
> which only gives you the wrong signal  of instrumentation is done.
>
> I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions 
> for thinlto under ldd for CSPGO.
>  Regular FDO should not use it.
>
> The right fix is to enable PGO instrumentation and use in pass builder for O0.
>
> I would like to request to revert this patch.


I mean, I'm happy for the patch to be reverted, but I still really don't 
understand why this fixes the test to work *exactly* the same as w/ the old 
pass manager and doesn't break any other tests if it is completely wrong? It 
seems like there must be something strange with the test coverage if this is so 
far off of correct without any failures...

I also don't understand what fix you are suggesting instead, but maybe you can 
show a patch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[Rong Xu via Phabricator via cfe-commits]

xur added a comment.

This patch does not make sense to me.

The reason of failing with -fexperimental-new-pass-manager is because we don't 
do PGO instrumentation at -O0. This is due to the fact that PGO 
instrumentation/use passes are under PassBuilder::buildPerModuleDefaultPipeline.

This patch add a pass
 MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
which only gives you the wrong signal  of instrumentation is done.

I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions 
for thinlto under ldd for CSPGO.
Regular FDO should not use it.

The right fix is to enable PGO instrumentation and use in pass builder for O0.

I would like to request to revert this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63965: [analyzer] exploded-graph-rewriter: Add support for program point tags.

[Csaba Dabis via Phabricator via cfe-commits]

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

Good idea! Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63965



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

[PATCH] D63915: [analyzer][WIP] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/return-value-guaranteed.cpp:20
+struct Foo { int Field; };
+bool error();
+bool problem();

NoQ wrote:
> `core.builtin.NoReturnFunctions` reacts on this function. You can easily see 
> it in the explodedgraph dump, as the last sink node in `test_calls::parseFoo` 
> is tagged with that checker. You'll have to pick a different name for testing 
> purposes.
(yes, this is why your tests aren't working)
(see also D63965)


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

https://reviews.llvm.org/D63915



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

[PATCH] D63965: [analyzer] exploded-graph-rewriter: Add support for program point tags.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

Keep it on a separate line for more visibility.

F9428700: Screen Shot 2019-06-28 at 4.26.13 PM.png 



Repository:
  rC Clang

https://reviews.llvm.org/D63965

Files:
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/utils/analyzer/exploded-graph-rewriter.py


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -387,6 +387,12 @@
'%s'
% (color, p.kind))
 
+if p.tag is not None:
+self._dump(''
+   ''
+   'Tag:  '
+   '%s' % p.tag)
+
 def visit_environment(self, e, prev_e=None):
 self._dump('')
 
Index: clang/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ clang/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -41,6 +41,14 @@
 // CHECK-SAME: 
 // CHECK-SAME: x
 // CHECK-SAME:   
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   Tag: 
+// CHECK-SAME:   ExprEngine : Clean Node
+// CHECK-SAME: 
+// CHECK-SAME:   
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{
@@ -56,7 +64,7 @@
   "line": 4,
   "column": 5
 },
-"tag": null
+"tag": "ExprEngine : Clean Node"
   }
 ]}
 \l}"];


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -387,6 +387,12 @@
'%s'
% (color, p.kind))
 
+if p.tag is not None:
+self._dump(''
+   ''
+   'Tag:  '
+   '%s' % p.tag)
+
 def visit_environment(self, e, prev_e=None):
 self._dump('')
 
Index: clang/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ clang/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -41,6 +41,14 @@
 // CHECK-SAME: 
 // CHECK-SAME: x
 // CHECK-SAME:   
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   Tag: 
+// CHECK-SAME:   ExprEngine : Clean Node
+// CHECK-SAME: 
+// CHECK-SAME:   
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{
@@ -56,7 +64,7 @@
   "line": 4,
   "column": 5
 },
-"tag": null
+"tag": "ExprEngine : Clean Node"
   }
 ]}
 \l}"];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63857: [clang-doc] Add a structured HTML generator

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207174.
DiegoAstiazaran marked 2 inline comments as done.
DiegoAstiazaran added a comment.

Add a variable to use as a reference to the last item in a vector instead of 
doing vector.back().


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

https://reviews.llvm.org/D63857

Files:
  clang-tools-extra/clang-doc/CMakeLists.txt
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -0,0 +1,276 @@
+//===-- clang-doc/HTMLGeneratorTest.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 "ClangDocTest.h"
+#include "Generators.h"
+#include "Representation.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace doc {
+
+std::unique_ptr getHTMLGenerator() {
+  auto G = doc::findGeneratorByName("html");
+  if (!G)
+return nullptr;
+  return std::move(G.get());
+}
+
+TEST(HTMLGeneratorTest, emitNamespaceHTML) {
+  NamespaceInfo I;
+  I.Name = "Namespace";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace);
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildEnums.emplace_back();
+  I.ChildEnums.back().Name = "OneEnum";
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+namespace Namespace
+
+  namespace Namespace
+  Namespaces
+  
+ChildNamespace
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitRecordHTML) {
+  RecordInfo I;
+  I.Name = "r";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
+
+  I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
+  I.TagType = TagTypeKind::TTK_Class;
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
+  I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
+
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildEnums.emplace_back();
+  I.ChildEnums.back().Name = "OneEnum";
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+class r
+
+  class r
+  
+Defined at line 10 of test.cpp
+  
+  
+Inherits from F, G
+  
+  Members
+  
+private int X
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitFunctionHTML) {
+  FunctionInfo I;
+  I.Name = "f";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
+
+  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  I.Params.emplace_back("int", "P");
+  I.IsMethod = true;
+  I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+
+
+  f
+  
+void f(int P)
+  
+  
+Defined at line 10 of test.cpp
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitEnumHTML) {
+  EnumInfo I;
+  I.Name = "e";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});

[PATCH] D63793: Treat the range of representable values of floating-point types as [-inf, +inf] not as [-max, +max].

[Sanjay Patel via Phabricator via cfe-commits]

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

I don't have much experience with the front-end and have no experience with the 
sanitizers, but the changes match my understanding for this type of 
cast/conversion, so LGTM.




Comment at: docs/UndefinedBehaviorSanitizer.rst:181
   -  ``-fsanitize=implicit-conversion``: Checks for suspicious
  behaviour of implicit conversions. Enables
  ``implicit-unsigned-integer-truncation``,

Independent of this patch but: 1 British behaviour in an otherwise American doc.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63793



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

[PATCH] D63962: [clang-doc] Fix segfault in comment sorting

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a reviewer: jakehehrlich.
juliehockett added a project: clang-tools-extra.

https://reviews.llvm.org/D63962

Files:
  clang-tools-extra/clang-doc/Representation.h


Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>


Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62645: [Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 207166.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Add function `Expr::hasNonOverloadPlaceholderType`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62645

Files:
  include/clang/AST/Expr.h
  include/clang/Sema/Sema.h
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/SemaObjC/arc-repeated-weak.mm

Index: test/SemaObjC/arc-repeated-weak.mm
===
--- test/SemaObjC/arc-repeated-weak.mm
+++ test/SemaObjC/arc-repeated-weak.mm
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++14 -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++14 -Warc-repeated-use-of-weak -verify %s
 
 @interface Test {
 @public
@@ -467,6 +467,18 @@
   __typeof__(NSBundle2.foo2.weakProp) t5;
 }
 
+void testAuto() {
+  auto __weak wp = NSBundle2.foo2.weakProp;
+}
+
+void testLambdaCaptureInit() {
+  [capture(NSBundle2.foo2.weakProp)] {} ();
+}
+
+void testAutoNew() {
+  auto p = new auto(NSBundle2.foo2.weakProp);
+}
+
 // This used to crash in the constructor of WeakObjectProfileTy when a
 // DeclRefExpr was passed that didn't reference a VarDecl.
 
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -783,7 +783,7 @@
   // Deduce the type of the init capture.
   QualType DeducedType = deduceVarTypeFromInitializer(
   /*VarDecl*/nullptr, DeclarationName(Id), DeductType, TSI,
-  SourceRange(Loc, Loc), IsDirectInit, Init);
+  SourceRange(Loc, Loc), IsDirectInit, Init, false);
   if (DeducedType.isNull())
 return QualType();
 
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1793,6 +1793,14 @@
 NumInits = List->getNumExprs();
   }
 
+  for (unsigned I = 0, E = NumInits; I != E; ++I)
+if (Inits[I]->hasNonOverloadPlaceholderType()) {
+  ExprResult Result = CheckPlaceholderExpr(Inits[I]);
+  if (!Result.isUsable())
+return ExprError();
+  Inits[I] = Result.get();
+}
+
   // C++11 [expr.new]p15:
   //   A new-expression that creates an object of type T initializes that
   //   object as follows:
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -6678,6 +6678,14 @@
 ExprResult Sema::ActOnParenListExpr(SourceLocation L,
 SourceLocation R,
 MultiExprArg Val) {
+  for (size_t I = 0, E = Val.size(); I != E; ++I)
+if (Val[I]->hasNonOverloadPlaceholderType()) {
+  ExprResult Result = CheckPlaceholderExpr(Val[I]);
+  if (!Result.isUsable())
+return ExprError();
+  Val[I] = Result.get();
+}
+
   return ParenListExpr::Create(Context, L, Val, R);
 }
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10907,7 +10907,7 @@
 DeclarationName Name, QualType Type,
 TypeSourceInfo *TSI,
 SourceRange Range, bool DirectInit,
-Expr *Init) {
+Expr *Init, bool DefaultedAnyToId) {
   bool IsInitCapture = !VDecl;
   assert((!VDecl || !VDecl->isInitCapture()) &&
  "init captures are expected to be deduced prior to initialization");
@@ -10985,18 +10985,6 @@
 return QualType();
   }
 
-  // Expressions default to 'id' when we're in a debugger.
-  bool DefaultedAnyToId = false;
-  if (getLangOpts().DebuggerCastResultToId &&
-  Init->getType() == Context.UnknownAnyTy && !IsInitCapture) {
-ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
-if (Result.isInvalid()) {
-  return QualType();
-}
-Init = Result.get();
-DefaultedAnyToId = true;
-  }
-
   // C++ [dcl.decomp]p1:
   //   If the assignment-expression [...] has array type A and no ref-qualifier
   //   is present, e has type cv A
@@ -11040,10 +11028,10 @@
 }
 
 bool Sema::DeduceVariableDeclarationType(VarDecl *VDecl, bool DirectInit,
- Expr 

[PATCH] D63155: [clang][NewPM] Fix broken profile test

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

Rong, can you take a look at this patch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63961: [clangd][xpc] pass the LSP value using data instead of string

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 207164.
arphaman added a comment.

Added missing `xpc_release` in the test.


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

https://reviews.llvm.org/D63961

Files:
  clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
  clang-tools-extra/clangd/xpc/Conversion.cpp


Index: clang-tools-extra/clangd/xpc/Conversion.cpp
===
--- clang-tools-extra/clangd/xpc/Conversion.cpp
+++ clang-tools-extra/clangd/xpc/Conversion.cpp
@@ -19,14 +19,21 @@
 
 xpc_object_t jsonToXpc(const json::Value &JSON) {
   const char *const Key = "LSP";
-  xpc_object_t PayloadObj = xpc_string_create(llvm::to_string(JSON).c_str());
+  std::string Str = llvm::to_string(JSON);
+  xpc_object_t PayloadObj = xpc_data_create(Str.data(), Str.size());
   return xpc_dictionary_create(&Key, &PayloadObj, 1);
 }
 
 json::Value xpcToJson(const xpc_object_t &XPCObject) {
   if (xpc_get_type(XPCObject) == XPC_TYPE_DICTIONARY) {
-const char *const LSP = xpc_dictionary_get_string(XPCObject, "LSP");
-auto Json = json::parse(llvm::StringRef(LSP));
+size_t Length;
+const char *LSP =
+(const char *)xpc_dictionary_get_data(XPCObject, "LSP", &Length);
+if (!LSP) {
+  elog("ignoring non-LSP XPC message");
+  return json::Value(nullptr);
+}
+auto Json = json::parse(llvm::StringRef(LSP, Length));
 if (Json)
   return *Json;
 else
Index: clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
===
--- clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
+++ clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
@@ -30,6 +30,13 @@
   }
 }
 
+TEST(JsonXpcConversionTest, IgnoreNonLSPDictionary) {
+  xpc_object_t EmptyDict = xpc_dictionary_create(nullptr, nullptr, 0);
+  json::Value Val = xpcToJson(EmptyDict);
+  ASSERT_EQ(Val.kind(), json::Value::Null);
+  xpc_release(EmptyDict);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/xpc/Conversion.cpp
===
--- clang-tools-extra/clangd/xpc/Conversion.cpp
+++ clang-tools-extra/clangd/xpc/Conversion.cpp
@@ -19,14 +19,21 @@
 
 xpc_object_t jsonToXpc(const json::Value &JSON) {
   const char *const Key = "LSP";
-  xpc_object_t PayloadObj = xpc_string_create(llvm::to_string(JSON).c_str());
+  std::string Str = llvm::to_string(JSON);
+  xpc_object_t PayloadObj = xpc_data_create(Str.data(), Str.size());
   return xpc_dictionary_create(&Key, &PayloadObj, 1);
 }
 
 json::Value xpcToJson(const xpc_object_t &XPCObject) {
   if (xpc_get_type(XPCObject) == XPC_TYPE_DICTIONARY) {
-const char *const LSP = xpc_dictionary_get_string(XPCObject, "LSP");
-auto Json = json::parse(llvm::StringRef(LSP));
+size_t Length;
+const char *LSP =
+(const char *)xpc_dictionary_get_data(XPCObject, "LSP", &Length);
+if (!LSP) {
+  elog("ignoring non-LSP XPC message");
+  return json::Value(nullptr);
+}
+auto Json = json::parse(llvm::StringRef(LSP, Length));
 if (Json)
   return *Json;
 else
Index: clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
===
--- clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
+++ clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
@@ -30,6 +30,13 @@
   }
 }
 
+TEST(JsonXpcConversionTest, IgnoreNonLSPDictionary) {
+  xpc_object_t EmptyDict = xpc_dictionary_create(nullptr, nullptr, 0);
+  json::Value Val = xpcToJson(EmptyDict);
+  ASSERT_EQ(Val.kind(), json::Value::Null);
+  xpc_release(EmptyDict);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63961: [clangd][xpc] pass it LSP value using data instead of string

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added a reviewer: jkorous.
Herald added subscribers: kadircet, dexonsmith, MaskRay, ilya-biryukov.
Herald added a project: clang.

Use XPC's data type instead of string to pass the LSP value from Clangd. This 
will make it easier to use from sourcekit-lsp 
(https://github.com/apple/sourcekit-lsp/pull/112), where I will be able to drop 
a conversion between `Data` and `String`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D63961

Files:
  clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
  clang-tools-extra/clangd/xpc/Conversion.cpp


Index: clang-tools-extra/clangd/xpc/Conversion.cpp
===
--- clang-tools-extra/clangd/xpc/Conversion.cpp
+++ clang-tools-extra/clangd/xpc/Conversion.cpp
@@ -19,14 +19,21 @@
 
 xpc_object_t jsonToXpc(const json::Value &JSON) {
   const char *const Key = "LSP";
-  xpc_object_t PayloadObj = xpc_string_create(llvm::to_string(JSON).c_str());
+  std::string Str = llvm::to_string(JSON);
+  xpc_object_t PayloadObj = xpc_data_create(Str.data(), Str.size());
   return xpc_dictionary_create(&Key, &PayloadObj, 1);
 }
 
 json::Value xpcToJson(const xpc_object_t &XPCObject) {
   if (xpc_get_type(XPCObject) == XPC_TYPE_DICTIONARY) {
-const char *const LSP = xpc_dictionary_get_string(XPCObject, "LSP");
-auto Json = json::parse(llvm::StringRef(LSP));
+size_t Length;
+const char *LSP =
+(const char *)xpc_dictionary_get_data(XPCObject, "LSP", &Length);
+if (!LSP) {
+  elog("ignoring non-LSP XPC message");
+  return json::Value(nullptr);
+}
+auto Json = json::parse(llvm::StringRef(LSP, Length));
 if (Json)
   return *Json;
 else
Index: clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
===
--- clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
+++ clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
@@ -30,6 +30,12 @@
   }
 }
 
+TEST(JsonXpcConversionTest, IgnoreNonLSPDictionary) {
+  xpc_object_t EmptyDict = xpc_dictionary_create(nullptr, nullptr, 0);
+  json::Value Val = xpcToJson(EmptyDict);
+  ASSERT_EQ(Val.kind(), json::Value::Null);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/xpc/Conversion.cpp
===
--- clang-tools-extra/clangd/xpc/Conversion.cpp
+++ clang-tools-extra/clangd/xpc/Conversion.cpp
@@ -19,14 +19,21 @@
 
 xpc_object_t jsonToXpc(const json::Value &JSON) {
   const char *const Key = "LSP";
-  xpc_object_t PayloadObj = xpc_string_create(llvm::to_string(JSON).c_str());
+  std::string Str = llvm::to_string(JSON);
+  xpc_object_t PayloadObj = xpc_data_create(Str.data(), Str.size());
   return xpc_dictionary_create(&Key, &PayloadObj, 1);
 }
 
 json::Value xpcToJson(const xpc_object_t &XPCObject) {
   if (xpc_get_type(XPCObject) == XPC_TYPE_DICTIONARY) {
-const char *const LSP = xpc_dictionary_get_string(XPCObject, "LSP");
-auto Json = json::parse(llvm::StringRef(LSP));
+size_t Length;
+const char *LSP =
+(const char *)xpc_dictionary_get_data(XPCObject, "LSP", &Length);
+if (!LSP) {
+  elog("ignoring non-LSP XPC message");
+  return json::Value(nullptr);
+}
+auto Json = json::parse(llvm::StringRef(LSP, Length));
 if (Json)
   return *Json;
 else
Index: clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
===
--- clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
+++ clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp
@@ -30,6 +30,12 @@
   }
 }
 
+TEST(JsonXpcConversionTest, IgnoreNonLSPDictionary) {
+  xpc_object_t EmptyDict = xpc_dictionary_create(nullptr, nullptr, 0);
+  json::Value Val = xpcToJson(EmptyDict);
+  ASSERT_EQ(Val.kind(), json::Value::Null);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63857: [clang-doc] Add a structured HTML generator

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:228
+  Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Functions"));
+  Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
+  for (const auto &F : Functions) {

Refering to this as `Out.back()` is confusing, can you give it a variable name? 
It took me longer than I care to admit to figure out that you weren't appending 
to `Out` in the loop. A pretty common pattern I use is to just assign `back()` 
to a reference that I'm going to use a lot.

```
Out.emplace_back(...);
auto& DivBody = Out.back();
```

That way you get the inplace construction and you can refer to it wherever 
you'd like by name.


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

https://reviews.llvm.org/D63857



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

[PATCH] D58164: Block+lambda: allow reference capture

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Currently a block captures a variable (POD or non-POD) by reference if the 
enclosing lambda captures it by reference and captures by copy if the enclosing 
lambda captures by copy.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58164



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

[PATCH] D63857: [clang-doc] Add a structured HTML generator

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207160.
DiegoAstiazaran retitled this revision from "[clang-doc] Structured HTML 
generator" to "[clang-doc] Add a structured HTML generator".
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.
Herald added subscribers: kadircet, arphaman, mgorny.

This patch depended on D63180 , that revision 
has been abandoned and combined with this one.


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

https://reviews.llvm.org/D63857

Files:
  clang-tools-extra/clang-doc/CMakeLists.txt
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -0,0 +1,276 @@
+//===-- clang-doc/HTMLGeneratorTest.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 "ClangDocTest.h"
+#include "Generators.h"
+#include "Representation.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace doc {
+
+std::unique_ptr getHTMLGenerator() {
+  auto G = doc::findGeneratorByName("html");
+  if (!G)
+return nullptr;
+  return std::move(G.get());
+}
+
+TEST(HTMLGeneratorTest, emitNamespaceHTML) {
+  NamespaceInfo I;
+  I.Name = "Namespace";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace);
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildEnums.emplace_back();
+  I.ChildEnums.back().Name = "OneEnum";
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+namespace Namespace
+
+  namespace Namespace
+  Namespaces
+  
+ChildNamespace
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitRecordHTML) {
+  RecordInfo I;
+  I.Name = "r";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
+
+  I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
+  I.TagType = TagTypeKind::TTK_Class;
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
+  I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
+
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildEnums.emplace_back();
+  I.ChildEnums.back().Name = "OneEnum";
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+class r
+
+  class r
+  
+Defined at line 10 of test.cpp
+  
+  
+Inherits from F, G
+  
+  Members
+  
+private int X
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+TEST(HTMLGeneratorTest, emitFunctionHTML) {
+  FunctionInfo I;
+  I.Name = "f";
+  I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
+
+  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  I.Params.emplace_back("int", "P");
+  I.IsMethod = true;
+  I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
+
+  auto G = getHTMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual);
+  assert(!Err);
+  std::string Expected = R"raw(
+
+
+
+  f
+  
+void f(int P)
+  
+  
+Defined at line 10 of test.cpp
+  
+
+)raw";
+
+  EXPECT_EQ(Expected, Actua

r364690 - [clang][test][NFC] Explicitly specify clang ABI in AST Dumper test

[Jan Korous via cfe-commits]

Author: jkorous
Date: Fri Jun 28 15:37:31 2019
New Revision: 364690

URL: http://llvm.org/viewvc/llvm-project?rev=364690&view=rev
Log:
[clang][test][NFC] Explicitly specify clang ABI in AST Dumper test

Clang <= 4 used the pre-C++11 rule about which structures can be passed in 
registers.

Modified:
cfe/trunk/test/AST/ast-dump-record-definition-data-json.cpp

Modified: cfe/trunk/test/AST/ast-dump-record-definition-data-json.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-record-definition-data-json.cpp?rev=364690&r1=364689&r2=364690&view=diff
==
--- cfe/trunk/test/AST/ast-dump-record-definition-data-json.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-record-definition-data-json.cpp Fri Jun 28 
15:37:31 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++17 -ast-dump=json %s 
| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fclang-abi-compat=7.0 
-std=c++17 -ast-dump=json %s | FileCheck %s
 
 void f() {
   auto IsNotGenericLambda = [](){};


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

[PATCH] D63180: [clang-doc] Adds HTML generator

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran abandoned this revision.
DiegoAstiazaran added a comment.

In D63180#1562982 , @jakehehrlich 
wrote:

> I'm getting confused in all the different patches. I reviewed the HTML 
> generator one like yesterday and was awaiting changes. Doesn't this do the 
> same thing?


This revision was supposed to be a basic generator and D63857 
 added a more structured functionality to it. 
This revision will be abandoned and everything will be combined in D63857 
.


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

https://reviews.llvm.org/D63180



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

[PATCH] D63857: [clang-doc] Structured HTML generator

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Can you upload `git diff -U10 ` functions so that I 
can see the code without it being fragmented? It's hard to parse some of the 
more fragmented stuff here




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:191-194
+static std::vector> genHTML(const EnumInfo &I);
+static std::vector> genHTML(const FunctionInfo &I);
+
+static std::vector>

Never use 'static' for functions in C++. For functions anon-namespaces and 
'static' mean the same thing but C++ generates a lot of stuff behind the scenes 
and to make those 'static' you need to use an non-namespace.

In general the rule is to put as much as you can in anon-namespaces and always 
prefer that for functions/globals/constants over 'static'. This is important 
for `constexpr` variables specifically because depending on the C++ version 
they don't play nice with the `static` keyword.


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

https://reviews.llvm.org/D63857



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

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > It looks like you're generally warning about this based on the specific 
> > > context that forced an lvalue-to-rvalue conversion.  I'm not sure 
> > > `volatile` is special except that we actually perform the load even in 
> > > unused-value contexts.  Is the assumption that you've exhaustively 
> > > covered all the other contexts of lvalue-to-rvalue conversions whose 
> > > values will actually be used?  That seems questionable to me.
> > Yes, that was my assumption. All the other contexts where lvalue-to-rvalue 
> > conversion is performed and the result is used are already covered by other 
> > calls sites of `checkNonTrivialCUnion`, which informs the users that the 
> > struct/union is being used in an invalid context.
> > 
> > Do you have a case in mind that I didn't think of where a lvalue-to-rvalue 
> > conversion requires a non-trivial initialization/destruction/copying of a 
> > union but clang fails to emit any diagnostics?
> > 
> > Also I realized that lvalue-to-rvalue conversion of volatile types doesn't 
> > always require non-trivial destruction, so I think `CheckDestruct` 
> > shouldn't be set in this case.
> > 
> > ```
> > void foo(U0 *a, volatile U0 *b) {
> >   // this doesn't require destruction.
> >   // this is perfectly valid if U0 is non-trivial to destruct but trivial 
> > to copy.
> >   *a = *b;  
> > }
> > ```
> > 
> > For the same reason, I think `CheckDestruct` shouldn't be set for function 
> > returns (but it should be set for function parameters since they are 
> > destructed by the callee).
> There are a *lot* of places that trigger lvalue-to-rvalue conversion.  Many 
> of them aren't legal with structs (in C), but I'm worried about approving a 
> pattern with the potential to be wrong by default just because we didn't 
> think about some weird case.  As an example, casts can trigger 
> lvalue-to-rvalue conversion; I think the only casts allowed with structs are 
> the identity cast and the cast to `void`, but those are indeed allowed.  Now, 
> a cast to `void` means the value is ignored, so we can elide a non-volatile 
> load in the operand, and an identity cast isn't terminal; if the idea is that 
> we're checking all the *terminal* uses of a struct r-value, then we're in 
> much more restricted territory (and don't need to worry about things like 
> commas and conditional operators that can propagate values out).  But this 
> still worries me.
> 
> I'm not sure we need to be super-pedantic about destructibility vs. 
> copyability in some of this checking.  It's certain possible in C++, but I 
> can't imagine what sort of *primitive* type would be trivially copyable but 
> not trivially destructible.  (The reverse isn't true: something like a 
> relative pointer would be non-trivially copyable but still trivially 
> destructible.)
> 
> Is there any way to make this check cheaper so that we can immediately avoid 
> any further work for types that are obviously copyable/destructible?  All the 
> restricted types are (possibly arrays of) record types, right?
I'm not sure I fully understand what you are saying, but by "cheaper", do you 
mean fewer and simpler rules for allowing or disallowing non-trivial unions 
even when that might result in rejecting unions used in contexts in which 
non-trivial initialization/destruction/copying is not required? If so, we can 
probably diagnose any lvalue-to-rvalue conversions regardless of whether the 
source is volatile if the type is either non-trivial to copy or destruct.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753



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

[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

[Mitch Phillips via Phabricator via cfe-commits]

hctim abandoned this revision.
hctim added a comment.

Abandoned as this was merged in small chunks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60593



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

[PATCH] D63960: [C++20] Add consteval-specifique semantic

[Tyker via Phabricator via cfe-commits]

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

Changes:

- Calls to consteval function and constructors are not evaluated as soon as 
they are reached.
- Add diagnostic for taking address of a consteval function in non-constexpr 
context.
- Add diagnostic for address of consteval function accessible at runtime.
- Add tests

Serialization and importing depends on https://reviews.llvm.org/D63640


Repository:
  rC Clang

https://reviews.llvm.org/D63960

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -56,3 +56,276 @@
 consteval int main() { // expected-error {{'main' is not allowed to be declared consteval}}
   return 0;
 }
+
+int i_runtime; // expected-note+ {{declared here}}
+constexpr int i_constexpr = 0;
+
+consteval int f_eval(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+constexpr auto l_eval = [](int i) consteval {
+// expected-note@-1+ {{declared here}}
+  return i;
+};
+
+struct A {
+  int I = 0;
+  consteval int f_eval(int i) const {
+// expected-note@-1+ {{declared here}}
+return I + i;
+// expected-note@-1 {{is not allowed in a constant expression}}
+  }
+};
+
+constexpr A a;
+
+namespace invalid_call {
+
+int d2 = f_eval(i_runtime);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+int l2 = l_eval(i_runtime);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+int m2 = a.f_eval(i_runtime);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+int d4 = f_eval(i_constexpr);
+int l4 = l_eval(i_constexpr);
+int m4 = a.f_eval(i_constexpr);
+
+constexpr int f1(int i) { // expected-note+ {{declared here}}
+  int d0 = f_eval(0);
+  int l0 = l_eval(0);
+  int m0 = a.f_eval(0);
+  int d2 = f_eval(i);
+  // expected-error@-1 {{could not be evaluated}}
+  // expected-error@-2 {{must be initialized}}
+  // FIXME: the error above should not appear when the initializer present but is invalid.
+  // expected-note@-4 {{is not allowed in a constant expression}}
+
+  int l2 = l_eval(i);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+  int m2 = a.f_eval(i);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+  int d6 = f_eval(i_constexpr);
+  int l6 = l_eval(i_constexpr);
+  int m6 = a.f_eval(i_constexpr);
+  int d8 = f_eval(i_runtime);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+  int l8 = l_eval(i_runtime);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+  int m8 = a.f_eval(i_runtime);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{is not allowed in a constant expression}}
+  return 0;
+}
+
+consteval int f2(int i) {
+// expected-error@-1 {{never produces a constant expression}}
+  int d0 = f_eval(i);
+  int l0 = l_eval(i);
+  int m0 = a.f_eval(i);
+  int d1 = f_eval(i_runtime);
+// expected-note@-1 {{is not allowed in a constant expression}}
+  int l1 = l_eval(i_runtime);
+  int m1 = a.f_eval(i_runtime);
+  return 0;
+}
+
+void test() {
+  A a_dependent;
+  // expected-note@-1+ {{declared here}}
+
+  int Int = 0;
+  auto l_dependent = [Int](int i) consteval {
+// expected-note@-1+ {{declared here}}
+return Int + i;
+// expected-note@-1 {{is not allowed in a constant expression}}
+  };
+
+  int i_l = l_dependent(0);
+  // expected-error@-1 {{could not be evaluated}}
+  // expected-note@-2 {{in call}}
+
+  int i_m = a_dependent.f_eval(0);
+  // expected-error@-1 {{could not be evaluated}}
+  // expected-note@-2 {{in call}}
+}
+
+}
+
+namespace taking_address {
+
+using func_type = int(int);
+using mem_ptr_type = int(A::*)(int);
+
+func_type* p1 = (&f_eval);
+// expected-error@-1 {{take address}}
+func_type* p2= &(((f_eval)));
+// expected-error@-1 {{take address}}
+func_type* p3 = (func_type*)f_eval;
+// expected-error@-1 {{take address}}
+func_type* p4 = static_cast(f_eval);
+// expected-error@-1 {{take address}}
+func_type* p5 = reinterpret_cast(f_eval);
+// expected-error@-1 {{take address}}
+func_type* p6 = reinterpret_cast(&reinterpret_cast(f_eval));
+// expected-error@-1 {{take address}}
+func_type* p7 = __builtin_addressof(f_eval);
+// expected-error@-1 {{take address}}
+

[PATCH] D63915: [analyzer][WIP] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Aha, nice, thanks for adding a description, it is a very good thing to do. 
Like, every commit should be obvious. Even though i know what's going on, 
nobody else does, so it's good to explain that this is for modeling certain 
weird LLVM-specific APIs that intentionally always return true.

I guess this checker should eventually be merged with 
`StdCLibraryFunctionsChecker` and such. I wonder if it's trivial to convert it 
to use `CallDescription`s and then extend it trivially to C++ functions. See 
also D54149 . This is one more reason why i 
don't want to reinvent APINotes: i'm responsible for one more re-inventing. 
I'll see if i can generalize upon all of these.




Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:31-37
+namespace {
+struct CallTy {
+  bool TruthValue;
+  StringRef Name;
+  Optional Class;
+};
+}

Let's use `CallDescriptionMap` (it hasn't landed yet but it's almost 
there, D62441).

It should save quite some code; feel free to introduce a convenient constructor 
if it turns out to be hard to produce an initializer list.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:81
+  // Create a note.
+  const NoteTag *CallTag = C.getNoteTag([Call](BugReport &BR) -> std::string {
+SmallString<128> Msg;

Please make the note prunable, so that it didn't bring in the nested stack 
frame if it appears in a nested stack frame (cf. D61817) - you might need to 
add a new flag to `CheckerContext::getNoteTag()` to make this happen.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:89
+
+Out << Call->Name << "' always return "
+<< (Call->TruthValue ? "true" : "false");

"returns"



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:98
+  SVal RetV = CE.getReturnValue();
+  Optional RetDV = RetV.getAs();
+

`castAs` if you're sure.

Generally it's either evaluated conservatively (so the return value is a 
conjured symbol) or inlined (and in this case returning an undefined value 
would result in a fatal warning), so return values shouldn't ever be undefined.

The only way to obtain an undefined return value is by binding it in 
`evalCall()`, but even then, i'd rather issue a warning immediately.



Comment at: clang/test/Analysis/return-value-guaranteed.cpp:20
+struct Foo { int Field; };
+bool error();
+bool problem();

`core.builtin.NoReturnFunctions` reacts on this function. You can easily see it 
in the explodedgraph dump, as the last sink node in `test_calls::parseFoo` is 
tagged with that checker. You'll have to pick a different name for testing 
purposes.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207153.
DiegoAstiazaran marked 3 inline comments as done.
DiegoAstiazaran added a comment.

Change an if statement for a switch statement in `emitInfo(const RecordDecl *D, 
...)`, it now handles correctly unexpected info types.
Fix spelling in comments.


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

https://reviews.llvm.org/D63911

Files:
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -35,48 +35,30 @@
   ClangDocSerializeTestVisitor(EmittedInfoList &EmittedInfos, bool Public)
   : EmittedInfos(EmittedInfos), Public(Public) {}
 
-  bool VisitNamespaceDecl(const NamespaceDecl *D) {
+  template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
  /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
+if (I.first)
+  EmittedInfos.emplace_back(std::move(I.first));
+if (I.second)
+  EmittedInfos.emplace_back(std::move(I.second));
 return true;
   }
 
+  bool VisitNamespaceDecl(const NamespaceDecl *D) { return mapDecl(D); }
+
   bool VisitFunctionDecl(const FunctionDecl *D) {
 // Don't visit CXXMethodDecls twice
 if (dyn_cast(D))
   return true;
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
+return mapDecl(D);
   }
 
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) { return mapDecl(D); }
 
-  bool VisitRecordDecl(const RecordDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
 
-  bool VisitEnumDecl(const EnumDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
 };
 
 void ExtractInfosFromCode(StringRef Code, size_t NumExpectedInfos, bool Public,
@@ -101,19 +83,19 @@
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 3,
+  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 5,
/*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID, "A");
   CheckNamespaceInfo(&ExpectedA, A);
 
-  NamespaceInfo *B = InfoAsNamespace(Infos[1].get());
+  NamespaceInfo *B = InfoAsNamespace(Infos[2].get());
   NamespaceInfo ExpectedB(EmptySID, "B");
   ExpectedB.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
   CheckNamespaceInfo(&ExpectedB, B);
 
-  NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[2].get());
+  NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[4].get());
   NamespaceInfo ExpectedBWithFunction(EmptySID);
   FunctionInfo F;
   F.Name = "f";
@@ -127,7 +109,7 @@
 
 TEST(SerializeTest, emitAnonymousNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace { }", 1, /*Public=*/false, Infos);
+  ExtractInfosFromCode("namespace { }", 2, /*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
@@ -142,7 +124,8 @@
   E() {}
 protected:
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+   4, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
@@ -150,7 +133,7 @@
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   CheckRecordInfo(&ExpectedE, E);
 
-  RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[1].get());
+  RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
   RecordInfo ExpectedRecordWithEConstructor(EmptySID);
   FunctionInfo EConstructor;
   EConstructor.Name = "E";
@@ -164,7 +147,7 @@
   st

[PATCH] D63954: Add lifetime categories attributes

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

xazax.hun added a comment.

I actually think we should hard code STL (and maybe some other popular) types 
into the compiler. I.e.: if an STL type is unannotated and the warnings are 
turned on, we could look up the default annotations from a table and append 
them during parsing. This could be done in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954



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

[PATCH] D63857: [clang-doc] Structured HTML generator

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207148.
DiegoAstiazaran marked 6 inline comments as done.
DiegoAstiazaran added a comment.

Make DoctypeDecl a constexpr.
Early exit in TagNode::Render.
Functions of HTMLGenerator now return HTMLNodes instead of adding children to a 
Node passed by reference.


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

https://reviews.llvm.org/D63857

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -40,16 +40,31 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(&I, Actual);
   assert(!Err);
-  std::string Expected = R"raw(namespace Namespace
-Namespaces
-ChildNamespace
-Records
-ChildStruct
-Functions
-OneFunction
- OneFunction()
-Enums
-enum OneEnum
+  std::string Expected = R"raw(
+
+namespace Namespace
+
+  namespace Namespace
+  Namespaces
+  
+ChildNamespace
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
 )raw";
 
   EXPECT_EQ(Expected, Actual.str());
@@ -80,18 +95,37 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(&I, Actual);
   assert(!Err);
-  std::string Expected = R"raw(class r
-Defined at line 10 of test.cpp
-Inherits from F, G
-Members
-private int X
-Records
-ChildStruct
-Functions
-OneFunction
- OneFunction()
-Enums
-enum OneEnum
+  std::string Expected = R"raw(
+
+class r
+
+  class r
+  
+Defined at line 10 of test.cpp
+  
+  
+Inherits from F, G
+  
+  Members
+  
+private int X
+  
+  Records
+  
+ChildStruct
+  
+  Functions
+  
+OneFunction
+
+   OneFunction()
+
+  
+  Enums
+  
+enum OneEnum
+  
+
 )raw";
 
   EXPECT_EQ(Expected, Actual.str());
@@ -116,9 +150,18 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(&I, Actual);
   assert(!Err);
-  std::string Expected = R"raw(f
-void f(int P)
-Defined at line 10 of test.cpp
+  std::string Expected = R"raw(
+
+
+
+  f
+  
+void f(int P)
+  
+  
+Defined at line 10 of test.cpp
+  
+
 )raw";
 
   EXPECT_EQ(Expected, Actual.str());
@@ -141,11 +184,18 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(&I, Actual);
   assert(!Err);
-  std::string Expected = R"raw(enum class e
-
-X
-
-Defined at line 10 of test.cpp
+  std::string Expected = R"raw(
+
+
+
+  enum class e
+  
+X
+  
+  
+Defined at line 10 of test.cpp
+  
+
 )raw";
 
   EXPECT_EQ(Expected, Actual.str());
@@ -194,12 +244,29 @@
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(&I, Actual);
   assert(!Err);
-  std::string Expected = R"raw(f
-void f(int I, int J)
-Defined at line 10 of test.cpp
-
- Brief description.
- Extended description that continues onto the next line.
+  std::string Expected = R"raw(
+
+
+
+  f
+  
+void f(int I, int J)
+  
+  
+Defined at line 10 of test.cpp
+  
+  
+
+  
+ Brief description.
+  
+  
+ Extended description that
+ continues onto the next line.
+  
+
+  
+
 )raw";
 
   EXPECT_EQ(Expected, Actual.str());
Index: clang-tools-extra/clang-doc/MDGenerator.cpp
===
--- clang-tools-extra/clang-doc/MDGenerator.cpp
+++ clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -20,11 +20,11 @@
 
 // Markdown generation
 
-std::string genItalic(const Twine &Text) { return "*" + Text.str() + "*"; }
+static std::string genItalic(const Twine &Text) { return "*" + Text.str() + "*"; }
 
-std::string genEmphasis(const Twine &Text) { return "**" + Text.str() + "**"; }
+static std::string genEmphasis(const Twine &Text) { return "**" + Text.str() + "**"; }
 
-std::string genLink(const Twine &Text, const Twine &Link) {
+static std::string genLink(const Twine &Text, const Twine &Link) {
   return "[" + Text.str() + "](" + Link.str() + ")";
 }
 
@@ -92,7 +92,7 @@
   }
 }
 
-void genMarkdown(const EnumInfo &I, llvm::raw_ostream &OS) {
+static void genMarkdown(const EnumInfo &I, llvm::raw_ostream &OS) {
   if (I.Scoped)
 writeLine("| enum class " + I.Name + " |", OS);
   else
@@ -112,7 +112,7 @@
 writeDescription(C, OS);
 }
 
-void genMarkdown(const FunctionInfo &I, llvm::raw_ostream &OS) {
+static void genMarkdown(const FunctionInfo &I, llvm::raw_ostream &OS) {
   std::string Buffer;
   llvm::raw_string_ostream Stream(Buffer);
   bool First = true;
@@ -139,7 +139,7 @@
 writeDescription(C, OS);
 }
 
-void genMarkdown(const NamespaceInfo &I, llvm::raw_ostream &OS) {
+static void genMarkdown(const NamespaceInfo &I, 

[PATCH] D63857: [clang-doc] Structured HTML generator

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:91
+struct HTMLFile {
+  llvm::SmallString<16> DoctypeDecl{""};
+  std::vector> Children; // List of child nodes

jakehehrlich wrote:
> Does this ever change? If not this should be a global constexpr constant 
> char* inside of an anon-namespace.
> 
> e.g.
> 
> namespace {
>   constexpr const char* kDoctypeDecl = "...";
> }
In HTML5 it's always `` so I've already changed it in the latest 
patch.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:170
+OS << " " << A.getKey() << "=\"" << A.getValue() << "\"";
+  OS << (SelfClosing ? "/>" : ">");
+  if (!InlineChildren)

jakehehrlich wrote:
> You can exit here if its self closing right?
Yes, fixed in the latest patch update.


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

https://reviews.llvm.org/D63857



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

[PATCH] D63180: [clang-doc] Adds HTML generator

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

In D63180#1562982 , @jakehehrlich 
wrote:

> I'm getting confused in all the different patches. I reviewed the HTML 
> generator one like yesterday and was awaiting changes. Doesn't this do the 
> same thing?


Diego, why don't you just combine this patch with the structured HTML patch? 
Would lead to less confusion and churn.


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

https://reviews.llvm.org/D63180



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

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

We can try to add the annotations to libcxx (which will still take time),
but there are also other standard libraries (libstdc++, MSVC) that
can be used with clang.
Eventually everybody will have lifetime annotations (of course ;-) ),
but in the meantime there seems to be no other way to enable
the analysis on unmodified STL.

My idea would be to maintain a `lifetime-annotations` headers (outside of clang 
repo?)
with #ifdef magic to add lifetime annotations for major STL variants/versions 
as a drop-in
to enable lifetime analysis. It's not nice, but only temporary and at least 
better than requiring
to change your STL to get lifetime analysis.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954



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

[PATCH] D63180: [clang-doc] Adds HTML generator

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

I'm getting confused in all the different patches. I reviewed the HTML 
generator one like yesterday and was awaiting changes. Doesn't this do the same 
thing?


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

https://reviews.llvm.org/D63180



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

[PATCH] D63954: Add lifetime categories attributes

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

> We explicitly allow to add an annotation after
>  the definition of the class to allow adding annotations
>  to external source from by the user, e.g.
> 
>   #include 
>   
>   namespace std {
>   template
>   class [[gsl::Owner(T)]] vector;
>   }

Wait, does that even work? What if the vector was declared in an inline 
namespace in the header? Seems like a strange recommendation for users. Is 
there some reason you can't just add these annotations to standard libraries? I 
doubt libcxx would have a problem with getting better warnings on their types.




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4560-4561
+  if(AL.getKind() ==  ParsedAttr::AT_Owner) {
+if (checkAttrMutualExclusion(S, D, AL))
+  return;
+if (const auto *Attr = D->getAttr()) {

This is duplicated with the first line in the function.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4564
+  if (Attr->getDerefType().getTypePtr() != ParmType.getTypePtr()) {
+S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) << AL << 
Attr;
+S.Diag(Attr->getLocation(), diag::note_conflicting_attribute);

`diag::warn_duplicate_attr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954



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

[PATCH] D62962: Clang implementation of sizeless types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Since this is an extension, it wouldb be great to have a (on-by-default? at 
> least in -Wall) diagnostic for every instance of usage of this extension.

We generally only add warnings for extensions which are not allowed by the 
standard.  It's impossible to define a sizeless type without using a reserved 
keyword, like __SVInt8_t, so we don't need a warning.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D63956: [analyzer] NonnullGlobalConstants: Don't be confused with a _Nonnull attribute.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

The NonnullGlobalConstants checker models the rule "it doesn't make sense to 
make a constant global pointer and initialize it to null"; it makes sure that 
whatever it's initialized with is known to be non-null.

Ironically, annotating the type of the pointer as `_Nonnull` breaks the checker.

Fix handling of the `_Nonnull` annotation so that it was instead one more 
reason to believe that the value is non-null.


Repository:
  rC Clang

https://reviews.llvm.org/D63956

Files:
  clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  clang/test/Analysis/nonnull-global-constants.mm


Index: clang/test/Analysis/nonnull-global-constants.mm
===
--- clang/test/Analysis/nonnull-global-constants.mm
+++ clang/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@
 void testNonnullNonconstBool() {
   clang_analyzer_eval(kBoolMutable); // expected-warning{{UNKNOWN}}
 }
+
+// If it's annotated as nonnull, it doesn't even need to be const.
+extern CFStringRef _Nonnull str3;
+void testNonnullNonconstCFString() {
+  clang_analyzer_eval(str3); // expected-warning{{TRUE}}
+}
+
+// This one's nonnull for two reasons.
+extern const CFStringRef _Nonnull str4;
+void testNonnullNonnullCFString() {
+  clang_analyzer_eval(str4); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@
 return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast(Ty)) {
-Ty = T->getDecl()->getUnderlyingType();
-
-// It is sufficient for any intermediate typedef
-// to be classified const.
-HasConst = HasConst || Ty.isConstQualified();
-if (isNonnullType(Ty) && HasConst)
-  return true;
+  while (const Type *T = Ty.getTypePtr()) {
+if (const auto *TT = dyn_cast(T)) {
+  Ty = TT->getDecl()->getUnderlyingType();
+  // It is sufficient for any intermediate typedef
+  // to be classified const.
+  HasConst = HasConst || Ty.isConstQualified();
+  if (isNonnullType(Ty) && HasConst)
+return true;
+} else if (const auto *AT = dyn_cast(T)) {
+  if (AT->getAttrKind() == attr::TypeNonNull)
+return true;
+  Ty = AT->getModifiedType();
+} else {
+  return false;
+}
   }
   return false;
 }


Index: clang/test/Analysis/nonnull-global-constants.mm
===
--- clang/test/Analysis/nonnull-global-constants.mm
+++ clang/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@
 void testNonnullNonconstBool() {
   clang_analyzer_eval(kBoolMutable); // expected-warning{{UNKNOWN}}
 }
+
+// If it's annotated as nonnull, it doesn't even need to be const.
+extern CFStringRef _Nonnull str3;
+void testNonnullNonconstCFString() {
+  clang_analyzer_eval(str3); // expected-warning{{TRUE}}
+}
+
+// This one's nonnull for two reasons.
+extern const CFStringRef _Nonnull str4;
+void testNonnullNonnullCFString() {
+  clang_analyzer_eval(str4); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@
 return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast(Ty)) {
-Ty = T->getDecl()->getUnderlyingType();
-
-// It is sufficient for any intermediate typedef
-// to be classified const.
-HasConst = HasConst || Ty.isConstQualified();
-if (isNonnullType(Ty) && HasConst)
-  return true;
+  while (const Type *T = Ty.getTypePtr()) {
+if (const auto *TT = dyn_cast(T)) {
+  Ty = TT->getDecl()->getUnderlyingType();
+  // It is sufficient for any intermediate typedef
+  // to be classified const.
+  HasConst = HasConst || Ty.isConstQualified();
+  if (isNonnullType(Ty) && HasConst)
+return true;
+} else if (const auto *AT = dyn_cast(T)) {
+  if (AT->getAttrKind() == attr::TypeNonNull)
+return true;
+  Ty = AT->getModifiedType();
+} else {
+  return false;
+}
   }
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r364683 - [OPENMP]Improve analysis of implicit captures.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Jun 28 13:45:14 2019
New Revision: 364683

URL: http://llvm.org/viewvc/llvm-project?rev=364683&view=rev
Log:
[OPENMP]Improve analysis of implicit captures.

If the variable is used in the OpenMP region implicitly, we need to
check the data-sharing attributes for such variables and generate
implicit clauses for them. Patch improves analysis of such variables for
better handling of data-sharing rules.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_default_messages.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_default_messages.cpp
cfe/trunk/test/OpenMP/nvptx_lambda_capturing.cpp
cfe/trunk/test/OpenMP/target_parallel_for_default_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_default_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_lastprivate_messages.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_reduction_messages.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_aligned_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/teams_reduction_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=364683&r1=364682&r2=364683&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Jun 28 13:45:14 2019
@@ -2636,24 +2636,7 @@ class DSAAttrChecker final : public Stmt
 // Check implicitly captured variables.
 if (!S->hasAssociatedStmt() || !S->getAssociatedStmt())
   return;
-for (const CapturedStmt::Capture &Cap :
- S->getInnermostCapturedStmt()->captures()) {
-  if (!Cap.capturesVariable())
-continue;
-  VarDecl *VD = Cap.getCapturedVar();
-  // Do not try to map the variable if it or its sub-component was mapped
-  // already.
-  if (isOpenMPTargetExecutionDirective(Stack->getCurrentDirective()) &&
-  Stack->checkMappableExprComponentListsForDecl(
-  VD, /*CurrentRegionOnly=*/true,
-  [](OMPClauseMappableExprCommon::MappableExprComponentListRef,
- OpenMPClauseKind) { return true; }))
-continue;
-  DeclRefExpr *DRE = buildDeclRefExpr(
-  SemaRef, VD, VD->getType().getNonLValueExprType(SemaRef.Context),
-  Cap.getLocation(), /*RefersToCapture=*/true);
-  Visit(DRE);
-}
+visitSubCaptures(S->getInnermostCapturedStmt());
   }
 
 public:
@@ -2669,7 +2652,10 @@ public:
 Visit(CED->getInit());
 return;
   }
-  }
+  } else if (VD->isImplicit() || isa(VD))
+// Do not analyze internal variables and do not enclose them into
+// implicit clauses.
+return;
   VD = VD->getCanonicalDecl();
   // Skip internally declared variables.
   if (VD->hasLocalStorage() && CS && !CS->capturesVariable(VD))
@@ -2921,6 +2907,25 @@ public:
 }
   }
 
+  void visitSubCaptures(CapturedStmt *S) {
+for (const CapturedStmt::Capture &Cap : S->captures()) {
+  if (!Cap.capturesVariable() && !Cap.capturesVariableByCopy())
+continue;
+  VarDecl *VD = Cap.getCapturedVar();
+  // Do not try to map the variable if it or its sub-component was mapped
+  // already.
+  if (isOpenMPTargetExecutionDirective(Stack->getCurrentDirective()) &&
+  Stack->checkMappableExprComponentListsForDecl(
+  VD, /*CurrentRegionOnly=*/true,
+  [](OMPClauseMappableExprCommon::MappableExprComponentListRef,
+ OpenMPClauseKind) { return true; }))
+continue;
+  DeclRefExpr *DRE = buildDeclRefExpr(
+  SemaRef, VD, VD->getType().getNonLValueExprType(SemaRef.Context),
+  Cap.getLocation(), /*RefersToCapture=*/true);
+ 

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 207138.
mgehre added a comment.

Fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp

Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+void [[gsl::Owner]] f();
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+[[gsl::Owner]] void f();
+ // expected-warning@-1 {{'Owner' attribute only applies to classes}}
+
+struct S {
+};
+
+S [[gsl::Owner]] Instance;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+class [[gsl::Owner]] OwnerMissingParameter {};
+// expected-error@-1 {{'Owner' attribute takes one argument}}
+class [[gsl::Pointer]] PointerMissingParameter {};
+// expected-error@-1 {{'Pointer' attribute takes one argument}}
+
+class [[gsl::Owner(7)]] OwnerDerefNoType {};
+// expected-error@-1 {{expected a type}} expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+
+class [[gsl::Pointer("int")]] PointerDerefNoType {};
+// expected-error@-1 {{expected a type}} expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+
+class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer {};
+// expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] [[gsl::Owner(int)]] DuplicateOwner {};
+// expected-error@-1 {{'Owner' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Pointer(int)]] [[gsl::Pointer(int)]] DuplicatePointer {};
+// expected-error@-1 {{'Pointer' and 'Pointer' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(void)]] OwnerVoidDerefType {};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType {};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+
+class [[gsl::Owner(int)]] AnOwner {};
+class [[gsl::Pointer(S)]] APointer {};
+
+class AddOwnerLater {};
+class [[gsl::Owner(int)]] AddOwnerLater;
+
+class [[gsl::Pointer(int)]] AddConflictLater {};
+class [[gsl::Owner(int)]] AddConflictLater;
+// expected-error@-1 {{'Owner' and 'Pointer' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] AddConflictLater2 {};
+class [[gsl::Owner(float)]] AddConflictLater2;
+// expected-error@-1 {{'Owner' and 'Owner' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] AddTheSameLater {};
+class [[gsl::Owner(int)]] AddTheSameLater;
\ No newline at end of file
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -116,8 +116,10 @@
 // CHECK-NEXT: OpenCLNoSVM (SubjectMatchRule_variable)
 // CHECK-NEXT: OptimizeNone (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Overloadable (SubjectMatchRule_function)
+// CHECK-NEXT: Owner (SubjectMatchRule_record)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: Pointer (SubjectMatchRule_record)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ReqdWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: RequireConstantInit (SubjectMatchRule_variable_is_global)
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -211,6 +211,15 @@
 }
 }
 
+namespace TestLifetimeCategories {
+  class [[gsl::Owner(int)]] AOwner {};
+  // CHECK: CXXRecordDecl{{.*}} class AOwner
+  // CHECK: OwnerAttr {{.*}} int
+  class [[gsl::Pointer(int)]] APointer {};
+  // CHECK: CXXRecordDecl{{.*}} class APointer
+  // CHECK: PointerAttr {{.*}} int
+}
+
 // Verify the order of attributes in the Ast. It must reflect the order
 // in the parsed source.
 int mergeAttrTest()

[PATCH] D63915: [analyzer][WIP] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207135.
Charusso added a comment.

- Fix the note as the class name is optional.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/return-value-guaranteed.cpp

Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -analyzer-checker=apiModeling.ReturnValue \
+// RUN:  -analyzer-config apiModeling.ReturnValue:Calls="true:error" \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -analyzer-checker=apiModeling.ReturnValue \
+// RUN:  -analyzer-config \
+// RUN:   apiModeling.ReturnValue:Calls="true:error;false:error:ErrorHandler" \
+// RUN:  -verify %s
+
+// FIXME: expected-no-diagnostics
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+
+struct Foo { int Field; };
+bool error();
+bool problem();
+void doSomething();
+
+// We predefine the return value of 'error()' to 'true' and we cannot take the
+// false-branches where error would occur.
+namespace test_calls {
+bool parseFoo(Foo &F) {
+  if (problem())
+return error();
+
+  F.Field = 0;
+  return false;
+}
+
+bool parseFile() {
+  clang_analyzer_eval(error() == true); // FIXME: xpected-warning{{TRUE}}
+
+  Foo F;
+  if (parseFoo(F))
+return true;
+
+  if (F.Field == 0) {
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  return false;
+}
+} // namespace test_calls
+
+namespace test_classes {
+struct ErrorHandler {
+  static bool error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem())
+return error();
+
+  F.Field = 0;
+  return ErrorHandler::error();
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F))
+return true;
+
+  if (F.Field == 0) {
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  return false;
+}
+} // namespace test_classes
+
+void test_note() {
+  clang_analyzer_eval(error() == true); // FIXME: xpected-warning{{TRUE}}
+  if (error())
+(void)(1 / !error());
+}
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -9,6 +9,7 @@
 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
+// CHECK-NEXT: apiModeling.ReturnValue:Calls = ""
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
@@ -88,4 +89,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 85
+// CHECK-NEXT: num-entries = 86
Index: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
@@ -0,0 +1,141 @@
+//===- ReturnValueChecker - Applies guaranteed return values *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This defines ReturnValueChecker, which checks for calls with guaranteed
+// boolean return value. It ensures the return value of each function call.
+//
+//===--===//
+
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
+
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+struct CallTy {
+  bool TruthValue;
+  StringRef Name;
+  Optional Class;
+};
+}
+
+namespace {
+class ReturnValueChecker : public Checker {
+pub

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added a reviewer: gribozavr.
Herald added a project: clang.

This is the first part of work announced in 
"[RFC] Adding lifetime analysis to clang" [0],
i.e. the addition of the [[gsl::Owner(T)]] and
[[gsl::Pointer(T)]] attributes, which
will enable user-defined types to participate in
the lifetime analysis (which will be part of the
next PR).
The type `T` here is called "DerefType" in the paper,
and denotes the type that an Owner owns and a Pointer
points to. E.g. `std::vector` should be annotated
with `[[gsl::Owner(int)]]` and 
a `std::vector::iterator` with `[[gsl::Pointer(int)]]`.

We explicitly allow to add an annotation after
the definition of the class to allow adding annotations
to external source from by the user, e.g.

  #include 
  
  namespace std {
  template
  class [[gsl::Owner(T)]] vector;
  }

until we can ship a standard library with annotations.

[0] http://lists.llvm.org/pipermail/cfe-dev/2018-November/060355.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63954

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp

Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+void [[gsl::Owner]] f();
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+[[gsl::Owner]] void f();
+ // expected-warning@-1 {{'Owner' attribute only applies to classes}}
+
+struct S {
+};
+
+S [[gsl::Owner]] Instance;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+class [[gsl::Owner]] OwnerMissingParameter {};
+// expected-error@-1 {{'Owner' attribute takes one argument}}
+class [[gsl::Pointer]] PointerMissingParameter {};
+// expected-error@-1 {{'Pointer' attribute takes one argument}}
+
+class [[gsl::Owner(7)]] OwnerDerefNoType {};
+// expected-error@-1 {{expected a type}} expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+
+class [[gsl::Pointer("int")]] PointerDerefNoType {};
+// expected-error@-1 {{expected a type}} expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+
+class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer {};
+// expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] [[gsl::Owner(int)]] DuplicateOwner {};
+// expected-error@-1 {{'Owner' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Pointer(int)]] [[gsl::Pointer(int)]] DuplicatePointer {};
+// expected-error@-1 {{'Pointer' and 'Pointer' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(void)]] OwnerVoidDerefType {};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType {};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+
+class [[gsl::Owner(int)]] AnOwner {};
+class [[gsl::Pointer(S)]] APointer {};
+
+class AddOwnerLater {};
+class [[gsl::Owner(int)]] AddOwnerLater;
+
+class [[gsl::Pointer(int)]] AddConflictLater {};
+class [[gsl::Owner(int)]] AddConflictLater;
+// expected-error@-1 {{'Owner' and 'Pointer' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] AddConflictLater2 {};
+class [[gsl::Owner(float)]] AddConflictLater2;
+// expected-error@-1 {{'Owner' and 'Owner' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] AddTheSameLater {};
+class [[gsl::Owner(int)]] AddTheSameLater;
\ No newline at end of file
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -116,8 +116,10 @@
 // CHECK-NEXT: OpenCLNoSVM (SubjectMatchRule_variable)
 // CHECK-NEXT: OptimizeNone (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Overloadable (SubjectMatchRule_function)
+// CHECK-NEXT: Owner (SubjectMatchRule_record)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: Pointer (Subject

[PATCH] D63915: [analyzer][WIP] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207133.
Charusso edited the summary of this revision.
Charusso added a comment.

- Make it match the class name. (Whoops!)
- Reorder the tuple (swap the return value with the call name).
- Remove the Projects option.
- Remove unnecessary input validation.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/return-value-guaranteed.cpp

Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -analyzer-checker=apiModeling.ReturnValue \
+// RUN:  -analyzer-config apiModeling.ReturnValue:Calls="true:error" \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -analyzer-checker=apiModeling.ReturnValue \
+// RUN:  -analyzer-config \
+// RUN:   apiModeling.ReturnValue:Calls="true:error;false:error:ErrorHandler" \
+// RUN:  -verify %s
+
+// FIXME: expected-no-diagnostics
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+
+struct Foo { int Field; };
+bool error();
+bool problem();
+void doSomething();
+
+// We predefine the return value of 'error()' to 'true' and we cannot take the
+// false-branches where error would occur.
+namespace test_calls {
+bool parseFoo(Foo &F) {
+  if (problem())
+return error();
+
+  F.Field = 0;
+  return false;
+}
+
+bool parseFile() {
+  clang_analyzer_eval(error() == true); // FIXME: xpected-warning{{TRUE}}
+
+  Foo F;
+  if (parseFoo(F))
+return true;
+
+  if (F.Field == 0) {
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  return false;
+}
+} // namespace test_calls
+
+namespace test_classes {
+struct ErrorHandler {
+  static bool error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem())
+return error();
+
+  F.Field = 0;
+  return ErrorHandler::error();
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F))
+return true;
+
+  if (F.Field == 0) {
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  return false;
+}
+} // namespace test_classes
+
+void test_note() {
+  clang_analyzer_eval(error() == true); // FIXME: xpected-warning{{TRUE}}
+  if (error())
+(void)(1 / !error());
+}
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -9,6 +9,7 @@
 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
+// CHECK-NEXT: apiModeling.ReturnValue:Calls = ""
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
@@ -88,4 +89,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 85
+// CHECK-NEXT: num-entries = 86
Index: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
@@ -0,0 +1,137 @@
+//===- ReturnValueChecker - Applies guaranteed return values *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This defines ReturnValueChecker, which checks for calls with guaranteed
+// boolean return value. It ensures the return value of each function call.
+//
+//===--===//
+
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
+
+
+using namespace clang;
+using na

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Eric Fiselier via Phabricator via cfe-commits]

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

I don't have any other comments.

LGTM.


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

https://reviews.llvm.org/D62977



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

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:366
+
+  if (I->Namespace[0].RefType == InfoType::IT_namespace) {
+auto Parent = llvm::make_unique();

Can you make this a switch statement, doing the appropriate things for 
`IT_record` and `IT_namespace` and using `llvm_unreachable("Invalid reference 
type")` for all other possible values? This will help prevent silent 
regressions down the line.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:374
+
+  // At this point any parent will be a RecordInfo
+  auto Parent = llvm::make_unique();

nit: generally try to make comments full sentences



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:398
   I->ChildFunctions.emplace_back(std::move(Func));
-  return std::unique_ptr{std::move(I)};
+  // Info es wrapped in its parent scope so it's returned in the second 
position
+  return {nullptr, std::unique_ptr{std::move(I)}};

nit: spelling, here and below


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

https://reviews.llvm.org/D63911



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

r364679 - Revert enabling frame pointer elimination on OpenBSD for now.

[Brad Smith via cfe-commits]

Author: brad
Date: Fri Jun 28 12:57:51 2019
New Revision: 364679

URL: http://llvm.org/viewvc/llvm-project?rev=364679&view=rev
Log:
Revert enabling frame pointer elimination on OpenBSD for now.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/frame-pointer-elim.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=364679&r1=364678&r2=364679&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Jun 28 12:57:51 2019
@@ -534,19 +534,6 @@ static bool useFramePointerForTargetByDe
 return !areOptimizationsEnabled(Args);
   }
 
-  if (Triple.isOSOpenBSD()) {
-switch (Triple.getArch()) {
-case llvm::Triple::mips64:
-case llvm::Triple::mips64el:
-case llvm::Triple::ppc:
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
-  return !areOptimizationsEnabled(Args);
-default:
-  return true;
-}
-  }
-
   if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI ||
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {

Modified: cfe/trunk/test/Driver/frame-pointer-elim.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/frame-pointer-elim.c?rev=364679&r1=364678&r2=364679&view=diff
==
--- cfe/trunk/test/Driver/frame-pointer-elim.c (original)
+++ cfe/trunk/test/Driver/frame-pointer-elim.c Fri Jun 28 12:57:51 2019
@@ -26,19 +26,6 @@
 // RUN:   FileCheck --check-prefix=NETBSD %s
 // NETBSD-NOT: "-momit-leaf-frame-pointer"
 
-// OpenBSD follows the same rules as Linux.
-// RUN: %clang -### -target x86_64-unknown-openbsd -S -O1 %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OPENBSD-OPT %s
-// RUN: %clang -### -target powerpc-unknown-openbsd -S -O1 %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OPENBSD-OPT %s
-// OPENBSD-OPT: "-momit-leaf-frame-pointer"
-
-// RUN: %clang -### -target x86_64-unknown-openbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OPENBSD %s
-// RUN: %clang -### -target powerpc-unknown-openbsd -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=OPENBSD %s
-// OPENBSD-NOT: "-momit-leaf-frame-pointer"
-
 // Darwin disables omitting the leaf frame pointer even under optimization
 // unless the command lines are given.
 // RUN: %clang -### -target i386-apple-darwin -S %s 2>&1 | \


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

[PATCH] D63616: Implement `-fsanitize-coverage-whitelist` and `-fsanitize-coverage-blacklist` for clang

[Yannis Juglaret via Phabricator via cfe-commits]

tuktuk added a comment.

Thanks for the reviews.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63616



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

[PATCH] D63666: [clang-doc] Add templates to HTML generator

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran abandoned this revision.
DiegoAstiazaran added a comment.

D63857  replaces this revision.
Nodes that represent each HTML tag are used instead of templates, it's easier 
to maintain.


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

https://reviews.llvm.org/D63666



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

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207126.
DiegoAstiazaran marked 6 inline comments as done.
DiegoAstiazaran added a comment.
Herald added subscribers: kadircet, arphaman.

Add comments.
Extract repeated logic into mapDecl function in SerializeTest.cpp
Use EmptySID instead of specific USR in tests.
Use {} instead of {nullptr, nullptr}.


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

https://reviews.llvm.org/D63911

Files:
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -35,48 +35,30 @@
   ClangDocSerializeTestVisitor(EmittedInfoList &EmittedInfos, bool Public)
   : EmittedInfos(EmittedInfos), Public(Public) {}
 
-  bool VisitNamespaceDecl(const NamespaceDecl *D) {
+  template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
  /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
+if (I.first)
+  EmittedInfos.emplace_back(std::move(I.first));
+if (I.second)
+  EmittedInfos.emplace_back(std::move(I.second));
 return true;
   }
 
+  bool VisitNamespaceDecl(const NamespaceDecl *D) { return mapDecl(D); }
+
   bool VisitFunctionDecl(const FunctionDecl *D) {
 // Don't visit CXXMethodDecls twice
 if (dyn_cast(D))
   return true;
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
+return mapDecl(D);
   }
 
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) { return mapDecl(D); }
 
-  bool VisitRecordDecl(const RecordDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
 
-  bool VisitEnumDecl(const EnumDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
 };
 
 void ExtractInfosFromCode(StringRef Code, size_t NumExpectedInfos, bool Public,
@@ -101,19 +83,19 @@
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 3,
+  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 5,
/*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID, "A");
   CheckNamespaceInfo(&ExpectedA, A);
 
-  NamespaceInfo *B = InfoAsNamespace(Infos[1].get());
+  NamespaceInfo *B = InfoAsNamespace(Infos[2].get());
   NamespaceInfo ExpectedB(EmptySID, "B");
   ExpectedB.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
   CheckNamespaceInfo(&ExpectedB, B);
 
-  NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[2].get());
+  NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[4].get());
   NamespaceInfo ExpectedBWithFunction(EmptySID);
   FunctionInfo F;
   F.Name = "f";
@@ -127,7 +109,7 @@
 
 TEST(SerializeTest, emitAnonymousNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace { }", 1, /*Public=*/false, Infos);
+  ExtractInfosFromCode("namespace { }", 2, /*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
@@ -142,7 +124,8 @@
   E() {}
 protected:
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+   4, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
@@ -150,7 +133,7 @@
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   CheckRecordInfo(&ExpectedE, E);
 
-  RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[1].get());
+  RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
   RecordInfo ExpectedRecordWithEConstructor(EmptySID);
   FunctionInfo EConstructor;
   ECons

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:341
+  Parent->USR = ParentUSR;
+  Parent->ChildNamespaces.emplace_back(I->USR, I->Name, 
InfoType::IT_namespace);
+  return {std::unique_ptr{std::move(I)},

juliehockett wrote:
> You're probably going to want the path in this too, here and below
This will be added when D63663 is pushed.


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

https://reviews.llvm.org/D63911



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

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

ahatanak wrote:
> rjmccall wrote:
> > It looks like you're generally warning about this based on the specific 
> > context that forced an lvalue-to-rvalue conversion.  I'm not sure 
> > `volatile` is special except that we actually perform the load even in 
> > unused-value contexts.  Is the assumption that you've exhaustively covered 
> > all the other contexts of lvalue-to-rvalue conversions whose values will 
> > actually be used?  That seems questionable to me.
> Yes, that was my assumption. All the other contexts where lvalue-to-rvalue 
> conversion is performed and the result is used are already covered by other 
> calls sites of `checkNonTrivialCUnion`, which informs the users that the 
> struct/union is being used in an invalid context.
> 
> Do you have a case in mind that I didn't think of where a lvalue-to-rvalue 
> conversion requires a non-trivial initialization/destruction/copying of a 
> union but clang fails to emit any diagnostics?
> 
> Also I realized that lvalue-to-rvalue conversion of volatile types doesn't 
> always require non-trivial destruction, so I think `CheckDestruct` shouldn't 
> be set in this case.
> 
> ```
> void foo(U0 *a, volatile U0 *b) {
>   // this doesn't require destruction.
>   // this is perfectly valid if U0 is non-trivial to destruct but trivial to 
> copy.
>   *a = *b;  
> }
> ```
> 
> For the same reason, I think `CheckDestruct` shouldn't be set for function 
> returns (but it should be set for function parameters since they are 
> destructed by the callee).
There are a *lot* of places that trigger lvalue-to-rvalue conversion.  Many of 
them aren't legal with structs (in C), but I'm worried about approving a 
pattern with the potential to be wrong by default just because we didn't think 
about some weird case.  As an example, casts can trigger lvalue-to-rvalue 
conversion; I think the only casts allowed with structs are the identity cast 
and the cast to `void`, but those are indeed allowed.  Now, a cast to `void` 
means the value is ignored, so we can elide a non-volatile load in the operand, 
and an identity cast isn't terminal; if the idea is that we're checking all the 
*terminal* uses of a struct r-value, then we're in much more restricted 
territory (and don't need to worry about things like commas and conditional 
operators that can propagate values out).  But this still worries me.

I'm not sure we need to be super-pedantic about destructibility vs. copyability 
in some of this checking.  It's certain possible in C++, but I can't imagine 
what sort of *primitive* type would be trivially copyable but not trivially 
destructible.  (The reverse isn't true: something like a relative pointer would 
be non-trivially copyable but still trivially destructible.)

Is there any way to make this check cheaper so that we can immediately avoid 
any further work for types that are obviously copyable/destructible?  All the 
restricted types are (possibly arrays of) record types, right?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753



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

[PATCH] D52847: [clang-doc] Handle anonymous namespaces

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364674: [clang-doc] Handle anonymous namespaces (authored by 
juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52847?vs=206979&id=207127#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52847

Files:
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
@@ -131,6 +131,7 @@
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
+  ExpectedA.Name = "@nonymous_namespace";
   CheckNamespaceInfo(&ExpectedA, A);
 }
 
Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -223,6 +223,8 @@
   void mergeBase(Info &&I);
   bool mergeable(const Info &Other);
 
+  llvm::SmallString<16> extractName();
+
   // Returns a reference to the parent scope (that is, the immediate parent
   // namespace or class in which this decl resides).
   llvm::Expected getEnclosingScope();
Index: clang-tools-extra/trunk/clang-doc/Representation.cpp
===
--- clang-tools-extra/trunk/clang-doc/Representation.cpp
+++ clang-tools-extra/trunk/clang-doc/Representation.cpp
@@ -21,6 +21,7 @@
 //===--===//
 #include "Representation.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace doc {
@@ -194,5 +195,37 @@
   SymbolInfo::merge(std::move(Other));
 }
 
+llvm::SmallString<16> Info::extractName() {
+  if (!Name.empty())
+return Name;
+
+  switch (IT) {
+  case InfoType::IT_namespace:
+// Cover the case where the project contains a base namespace called
+// 'GlobalNamespace' (i.e. a namespace at the same level as the global
+// namespace, which would conflict with the hard-coded global namespace name
+// below.)
+if (Name == "GlobalNamespace" && Namespace.empty())
+  return llvm::SmallString<16>("@GlobalNamespace");
+// The case of anonymous namespaces is taken care of in serialization,
+// so here we can safely assume an unnamed namespace is the global
+// one.
+return llvm::SmallString<16>("GlobalNamespace");
+  case InfoType::IT_record:
+return llvm::SmallString<16>("@nonymous_record_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_enum:
+return llvm::SmallString<16>("@nonymous_enum_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_function:
+return llvm::SmallString<16>("@nonymous_function_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_default:
+return llvm::SmallString<16>("@nonymous_" + toHex(llvm::toStringRef(USR)));
+  }
+  llvm_unreachable("Invalid InfoType.");
+  return llvm::SmallString<16>("");
+}
+
 } // namespace doc
 } // namespace clang
Index: clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
@@ -132,9 +132,6 @@
   if (CreateDirectory(Path))
 return llvm::make_error("Unable to create directory.\n",
llvm::inconvertibleErrorCode());
-
-  if (Name.empty())
-Name = "GlobalNamespace";
   llvm::sys::path::append(Path, Name + Ext);
   return Path;
 }
@@ -222,8 +219,8 @@
 
 doc::Info *I = Reduced.get().get();
 
-auto InfoPath =
-getInfoOutputFile(OutDirectory, I->Namespace, I->Name, "." + Format);
+auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
+  I->extractName(), "." + Format);
 if (!InfoPath) {
   llvm::errs() << toString(InfoPath.takeError()) << "\n";
   continue;
Index: clang-tools-extra/trunk/clang-doc/Serialize.cpp
===
--- clang-tools-extra/trunk/clang-doc/Serialize.cpp
+++ clang-tools-extra/trunk/clang-doc/Serialize.cpp
@@ -270,13 +270,19 @@
 template 
 static void
 populateParentNamespaces(llvm::SmallVector &Namesp

[clang-tools-extra] r364674 - [clang-doc] Handle anonymous namespaces

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Fri Jun 28 12:07:56 2019
New Revision: 364674

URL: http://llvm.org/viewvc/llvm-project?rev=364674&view=rev
Log:
[clang-doc] Handle anonymous namespaces

Improves output for anonymous decls, and updates the '--public' flag to exclude 
everything under an anonymous namespace.

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

Modified:
clang-tools-extra/trunk/clang-doc/Representation.cpp
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/clang-doc/Serialize.cpp
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/Representation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Representation.cpp?rev=364674&r1=364673&r2=364674&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Representation.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Representation.cpp Fri Jun 28 12:07:56 
2019
@@ -21,6 +21,7 @@
 
//===--===//
 #include "Representation.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace doc {
@@ -194,5 +195,37 @@ void FunctionInfo::merge(FunctionInfo &&
   SymbolInfo::merge(std::move(Other));
 }
 
+llvm::SmallString<16> Info::extractName() {
+  if (!Name.empty())
+return Name;
+
+  switch (IT) {
+  case InfoType::IT_namespace:
+// Cover the case where the project contains a base namespace called
+// 'GlobalNamespace' (i.e. a namespace at the same level as the global
+// namespace, which would conflict with the hard-coded global namespace 
name
+// below.)
+if (Name == "GlobalNamespace" && Namespace.empty())
+  return llvm::SmallString<16>("@GlobalNamespace");
+// The case of anonymous namespaces is taken care of in serialization,
+// so here we can safely assume an unnamed namespace is the global
+// one.
+return llvm::SmallString<16>("GlobalNamespace");
+  case InfoType::IT_record:
+return llvm::SmallString<16>("@nonymous_record_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_enum:
+return llvm::SmallString<16>("@nonymous_enum_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_function:
+return llvm::SmallString<16>("@nonymous_function_" +
+ toHex(llvm::toStringRef(USR)));
+  case InfoType::IT_default:
+return llvm::SmallString<16>("@nonymous_" + toHex(llvm::toStringRef(USR)));
+  }
+  llvm_unreachable("Invalid InfoType.");
+  return llvm::SmallString<16>("");
+}
+
 } // namespace doc
 } // namespace clang

Modified: clang-tools-extra/trunk/clang-doc/Representation.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Representation.h?rev=364674&r1=364673&r2=364674&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Representation.h (original)
+++ clang-tools-extra/trunk/clang-doc/Representation.h Fri Jun 28 12:07:56 2019
@@ -223,6 +223,8 @@ struct Info {
   void mergeBase(Info &&I);
   bool mergeable(const Info &Other);
 
+  llvm::SmallString<16> extractName();
+
   // Returns a reference to the parent scope (that is, the immediate parent
   // namespace or class in which this decl resides).
   llvm::Expected getEnclosingScope();

Modified: clang-tools-extra/trunk/clang-doc/Serialize.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Serialize.cpp?rev=364674&r1=364673&r2=364674&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Serialize.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Serialize.cpp Fri Jun 28 12:07:56 2019
@@ -270,13 +270,19 @@ static void parseBases(RecordInfo &I, co
 template 
 static void
 populateParentNamespaces(llvm::SmallVector &Namespaces,
- const T *D) {
+ const T *D, bool &IsAnonymousNamespace) {
   const auto *DC = dyn_cast(D);
   while ((DC = DC->getParent())) {
-if (const auto *N = dyn_cast(DC))
-  Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
+if (const auto *N = dyn_cast(DC)) {
+  std::string Namespace;
+  if (N->isAnonymousNamespace()) {
+Namespace = "@nonymous_namespace";
+IsAnonymousNamespace = true;
+  } else
+Namespace = N->getNameAsString();
+  Namespaces.emplace_back(getUSRForDecl(N), Namespace,
   InfoType::IT_namespace);
-else if (const auto *N = dyn_cast(DC))
+} else if (const auto *N = dyn_cast(DC))
   Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
   InfoType::IT_record);

[PATCH] D56366: New warning call-to-virtual-from-ctor-dtor when calling a virtual function from a constructor or a destructor

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Oh, sorry, I had completely forgotten about that.

My contributions to Clang are primarily code review these days; I am not 
working on that idea.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56366



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

[PATCH] D63616: Implement `-fsanitize-coverage-whitelist` and `-fsanitize-coverage-blacklist` for clang

[Matt Morehouse via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63616



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

[PATCH] D62970: [clang-doc] De-duplicate comments and locations

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364670: [clang-doc] De-duplicate comments and locations 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62970?vs=206570&id=207111#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62970

Files:
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
@@ -159,15 +159,37 @@
   One.IsMethod = true;
   One.Parent = Reference(EmptySID, "Parent", InfoType::IT_namespace);
 
+  One.Description.emplace_back();
+  auto OneFullComment = &One.Description.back();
+  OneFullComment->Kind = "FullComment";
+  auto OneParagraphComment = llvm::make_unique();
+  OneParagraphComment->Kind = "ParagraphComment";
+  auto OneTextComment = llvm::make_unique();
+  OneTextComment->Kind = "TextComment";
+  OneTextComment->Text = "This is a text comment.";
+  OneParagraphComment->Children.push_back(std::move(OneTextComment));
+  OneFullComment->Children.push_back(std::move(OneParagraphComment));
+
   FunctionInfo Two;
   Two.Name = "f";
   Two.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  Two.Loc.emplace_back(20, llvm::SmallString<16>{"test.cpp"});
+  Two.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   Two.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   Two.Params.emplace_back("int", "P");
 
+  Two.Description.emplace_back();
+  auto TwoFullComment = &Two.Description.back();
+  TwoFullComment->Kind = "FullComment";
+  auto TwoParagraphComment = llvm::make_unique();
+  TwoParagraphComment->Kind = "ParagraphComment";
+  auto TwoTextComment = llvm::make_unique();
+  TwoTextComment->Kind = "TextComment";
+  TwoTextComment->Text = "This is a text comment.";
+  TwoParagraphComment->Children.push_back(std::move(TwoTextComment));
+  TwoFullComment->Children.push_back(std::move(TwoParagraphComment));
+
   std::vector> Infos;
   Infos.emplace_back(llvm::make_unique(std::move(One)));
   Infos.emplace_back(llvm::make_unique(std::move(Two)));
@@ -178,13 +200,23 @@
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   Expected->Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
-  Expected->Loc.emplace_back(20, llvm::SmallString<16>{"test.cpp"});
 
   Expected->ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   Expected->Params.emplace_back("int", "P");
   Expected->IsMethod = true;
   Expected->Parent = Reference(EmptySID, "Parent", InfoType::IT_namespace);
 
+  Expected->Description.emplace_back();
+  auto ExpectedFullComment = &Expected->Description.back();
+  ExpectedFullComment->Kind = "FullComment";
+  auto ExpectedParagraphComment = llvm::make_unique();
+  ExpectedParagraphComment->Kind = "ParagraphComment";
+  auto ExpectedTextComment = llvm::make_unique();
+  ExpectedTextComment->Kind = "TextComment";
+  ExpectedTextComment->Text = "This is a text comment.";
+  ExpectedParagraphComment->Children.push_back(std::move(ExpectedTextComment));
+  ExpectedFullComment->Children.push_back(std::move(ExpectedParagraphComment));
+
   auto Actual = mergeInfos(Infos);
   assert(Actual);
   CheckFunctionInfo(InfoAsFunction(Expected.get()),
Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -46,6 +46,45 @@
   CommentInfo() = default;
   CommentInfo(CommentInfo &Other) = delete;
   CommentInfo(CommentInfo &&Other) = default;
+  CommentInfo &operator=(CommentInfo &&Other) = default;
+
+  bool operator==(const CommentInfo &Other) const {
+auto FirstCI = std::tie(Kind, Text, Name, Direction, ParamName, CloseName,
+SelfClosing, Explicit, AttrKeys, AttrValues, Args);
+auto SecondCI =
+std::tie(Other.Kind, Other.Text, Other.Name, Other.Direction,
+ Other.ParamName, Other.CloseName, Other.SelfClosing,
+ Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
+
+if (FirstCI != SecondCI || Children.size() != Other.Children.size())
+  return false;
+
+return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
+  llvm::deref{});
+  }
+
+  // This operator is used to sort a vector of CommentInfos.
+  // No specific order (attributes more important than others) is required. Any
+  // sort is enough, the order is only ne

[clang-tools-extra] r364670 - [clang-doc] De-duplicate comments and locations

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Fri Jun 28 11:17:58 2019
New Revision: 364670

URL: http://llvm.org/viewvc/llvm-project?rev=364670&view=rev
Log:
[clang-doc] De-duplicate comments and locations

De-duplicate comments and declaration locations in reduce function.
When two files include the same header file, this file's content is mapped
twice causing comments and locations to be duplicated after the reduce stage.

Committed on behalf of Diego Astiazarán (diegoaa...@gmail.com).

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

Modified:
clang-tools-extra/trunk/clang-doc/Representation.cpp
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/Representation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Representation.cpp?rev=364670&r1=364669&r2=364670&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Representation.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Representation.cpp Fri Jun 28 11:17:58 
2019
@@ -122,6 +122,9 @@ void Info::mergeBase(Info &&Other) {
   // Unconditionally extend the description, since each decl may have a 
comment.
   std::move(Other.Description.begin(), Other.Description.end(),
 std::back_inserter(Description));
+  std::sort(Description.begin(), Description.end());
+  auto Last = std::unique(Description.begin(), Description.end());
+  Description.erase(Last, Description.end());
 }
 
 bool Info::mergeable(const Info &Other) {
@@ -134,6 +137,9 @@ void SymbolInfo::merge(SymbolInfo &&Othe
 DefLoc = std::move(Other.DefLoc);
   // Unconditionally extend the list of locations, since we want all of them.
   std::move(Other.Loc.begin(), Other.Loc.end(), std::back_inserter(Loc));
+  std::sort(Loc.begin(), Loc.end());
+  auto Last = std::unique(Loc.begin(), Loc.end());
+  Loc.erase(Last, Loc.end());
   mergeBase(std::move(Other));
 }
 

Modified: clang-tools-extra/trunk/clang-doc/Representation.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Representation.h?rev=364670&r1=364669&r2=364670&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Representation.h (original)
+++ clang-tools-extra/trunk/clang-doc/Representation.h Fri Jun 28 11:17:58 2019
@@ -46,6 +46,45 @@ struct CommentInfo {
   CommentInfo() = default;
   CommentInfo(CommentInfo &Other) = delete;
   CommentInfo(CommentInfo &&Other) = default;
+  CommentInfo &operator=(CommentInfo &&Other) = default;
+
+  bool operator==(const CommentInfo &Other) const {
+auto FirstCI = std::tie(Kind, Text, Name, Direction, ParamName, CloseName,
+SelfClosing, Explicit, AttrKeys, AttrValues, Args);
+auto SecondCI =
+std::tie(Other.Kind, Other.Text, Other.Name, Other.Direction,
+ Other.ParamName, Other.CloseName, Other.SelfClosing,
+ Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
+
+if (FirstCI != SecondCI || Children.size() != Other.Children.size())
+  return false;
+
+return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
+  llvm::deref{});
+  }
+
+  // This operator is used to sort a vector of CommentInfos.
+  // No specific order (attributes more important than others) is required. Any
+  // sort is enough, the order is only needed to call std::unique after sorting
+  // the vector.
+  bool operator<(const CommentInfo &Other) const {
+auto FirstCI = std::tie(Kind, Text, Name, Direction, ParamName, CloseName,
+SelfClosing, Explicit, AttrKeys, AttrValues, Args);
+auto SecondCI =
+std::tie(Other.Kind, Other.Text, Other.Name, Other.Direction,
+ Other.ParamName, Other.CloseName, Other.SelfClosing,
+ Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
+
+if (FirstCI < SecondCI ||
+(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+  return true;
+
+if (FirstCI > SecondCI || Children.size() > Other.Children.size())
+  return false;
+
+return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
+  llvm::deref{});
+  }
 
   SmallString<16>
   Kind; // Kind of comment (FullComment, ParagraphComment, TextComment,
@@ -148,6 +187,15 @@ struct Location {
std::tie(Other.LineNumber, Other.Filename);
   }
 
+  // This operator is used to sort a vector of Locations.
+  // No specific order (attributes more important than others) is required. Any
+  // sort is enough, the order is only needed to call std::unique after sorting
+  // the vector.
+  bool operator<(const Location &Other) const {
+return std::tie(LineNumber, Filename) <
+   std::tie(Other.LineNumber, Other.Filename);
+  }
+
  

[PATCH] D63943: ObjC: Squeeze in one more bit for the count of protocols in 'id' types

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose created this revision.
jordan_rose added reviewers: jfb, doug.gregor, dexonsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Someone actually had an Objective-C object pointer type with more than 64 
protocols, probably collected through typedefs. Raise the ceiling by one order 
of magnitude...even though this is just kicking the can down the road, and we 
need a proper diagnostic for this.


Repository:
  rC Clang

https://reviews.llvm.org/D63943

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

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -621,7 +621,7 @@
Base->isVariablyModifiedType(),
Base->containsUnexpandedParameterPack()),
   BaseType(Base) {
-  ObjCObjectTypeBits.IsKindOf = isKindOf;
+  CachedSuperClassAndIsKindOf.setInt(isKindOf);
 
   ObjCObjectTypeBits.NumTypeArgs = typeArgs.size();
   assert(getTypeArgsAsWritten().size() == typeArgs.size() &&
@@ -1454,20 +1454,20 @@
   // superclass type.
   ObjCInterfaceDecl *classDecl = getInterface();
   if (!classDecl) {
-CachedSuperClassType.setInt(true);
+CachedSuperClassAndIsKindOf.setPointer({nullptr, true});
 return;
   }
 
   // Extract the superclass type.
   const ObjCObjectType *superClassObjTy = classDecl->getSuperClassType();
   if (!superClassObjTy) {
-CachedSuperClassType.setInt(true);
+CachedSuperClassAndIsKindOf.setPointer({nullptr, true});
 return;
   }
 
   ObjCInterfaceDecl *superClassDecl = superClassObjTy->getInterface();
   if (!superClassDecl) {
-CachedSuperClassType.setInt(true);
+CachedSuperClassAndIsKindOf.setPointer({nullptr, true});
 return;
   }
 
@@ -1476,14 +1476,14 @@
   QualType superClassType(superClassObjTy, 0);
   ObjCTypeParamList *superClassTypeParams = superClassDecl->getTypeParamList();
   if (!superClassTypeParams) {
-CachedSuperClassType.setPointerAndInt(
-  superClassType->castAs(), true);
+CachedSuperClassAndIsKindOf.setPointer({
+  superClassType->castAs(), true});
 return;
   }
 
   // If the superclass reference is unspecialized, return it.
   if (superClassObjTy->isUnspecialized()) {
-CachedSuperClassType.setPointerAndInt(superClassObjTy, true);
+CachedSuperClassAndIsKindOf.setPointer({superClassObjTy, true});
 return;
   }
 
@@ -1491,8 +1491,8 @@
   // parameters in the superclass reference to substitute.
   ObjCTypeParamList *typeParams = classDecl->getTypeParamList();
   if (!typeParams) {
-CachedSuperClassType.setPointerAndInt(
-  superClassType->castAs(), true);
+CachedSuperClassAndIsKindOf.setPointer({
+  superClassType->castAs(), true});
 return;
   }
 
@@ -1502,20 +1502,19 @@
 QualType unspecializedSuper
   = classDecl->getASTContext().getObjCInterfaceType(
   superClassObjTy->getInterface());
-CachedSuperClassType.setPointerAndInt(
-  unspecializedSuper->castAs(),
-  true);
+CachedSuperClassAndIsKindOf.setPointer({
+  unspecializedSuper->castAs(), true});
 return;
   }
 
   // Substitute the provided type arguments into the superclass type.
   ArrayRef typeArgs = getTypeArgs();
   assert(typeArgs.size() == typeParams->size());
-  CachedSuperClassType.setPointerAndInt(
+  CachedSuperClassAndIsKindOf.setPointer({
 superClassType.substObjCTypeArgs(classDecl->getASTContext(), typeArgs,
  ObjCSubstitutionContext::Superclass)
   ->castAs(),
-true);
+true});
 }
 
 const ObjCInterfaceType *ObjCObjectPointerType::getInterfaceType() const {
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1556,10 +1556,7 @@
 unsigned NumTypeArgs : 7;
 
 /// The number of protocols stored directly on this object type.
-unsigned NumProtocols : 6;
-
-/// Whether this is a "kindof" type.
-unsigned IsKindOf : 1;
+unsigned NumProtocols : 7;
   };
 
   class ReferenceTypeBitfields {
@@ -5560,9 +5557,12 @@
   /// Either a BuiltinType or an InterfaceType or sugar for either.
   QualType BaseType;
 
-  /// Cached superclass type.
-  mutable llvm::PointerIntPair
-CachedSuperClassType;
+  using CachedSuperClassStorage =
+  llvm::PointerIntPair;
+
+  /// Cached superclass type plus whether this type was written with '__kindof'.
+  mutable llvm::PointerIntPair
+  CachedSuperClassAndIsKindOf;
 
   QualType *getTypeArgStorage();
   const QualType *getTypeArgStorage() const {
@@ -5592,7 +5592,6 @@
   BaseType(QualType(this_(), 0)) {
 ObjCObjectTypeBits.NumProtocols = 0;
 ObjCObjectTypeBits.NumTypeArgs = 0;
-ObjCObjectTypeBits.IsKindOf = 0;
   }
 
   void computeSuperClassTypeSlow() const;
@@ -5658,7 +5657,9 @@
   }
 
   /// Whether this is a "__kindof" type as written.
-  bool isKindOfTypeAsWritten() const { return ObjCObjectTypeBits.IsKind

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 207102.
jcai19 added a comment.

Update warning messages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63623

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-posix-return.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s bugprone-posix-return %t
+
+#define NULL nullptr
+#define ZERO 0
+#define NEGATIVE_ONE -1
+
+typedef long off_t;
+typedef decltype(sizeof(int)) size_t;
+typedef int pid_t;
+typedef struct __posix_spawn_file_actions* posix_spawn_file_actions_t;
+typedef struct __posix_spawnattr* posix_spawnattr_t;
+
+extern "C" int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+extern "C" int posix_fallocate(int fd, off_t offset, off_t len);
+extern "C" int posix_madvise(void *addr, size_t len, int advice);
+extern "C" int posix_memalign(void **memptr, size_t alignment, size_t size);
+extern "C" int posix_openpt(int flags);
+extern "C" int posix_spawn(pid_t *pid, const char *path,
+const posix_spawn_file_actions_t *file_actions,
+const posix_spawnattr_t *attrp,
+char *const argv[], char *const envp[]);
+extern "C" int posix_spawnp(pid_t *pid, const char *file,
+ const posix_spawn_file_actions_t *file_actions,
+ const posix_spawnattr_t *attrp,
+ char *const argv[], char *const envp[]);
+
+void warningLessThanZero() {
+  if (posix_fadvise(0, 0, 0, 0) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: posix_fadvise
+  // CHECK-FIXES: posix_fadvise(0, 0, 0, 0) > 0
+  if (posix_fallocate(0, 0, 0) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning:
+  // CHECK-FIXES: posix_fallocate(0, 0, 0) > 0
+  if (posix_madvise(NULL, 0, 0) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  // CHECK-FIXES: posix_madvise(NULL, 0, 0) > 0
+  if (posix_memalign(NULL, 0, 0) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning:
+  // CHECK-FIXES: posix_memalign(NULL, 0, 0) > 0
+  if (posix_spawn(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
+  // CHECK-FIXES: posix_spawn(NULL, NULL, NULL, NULL, {NULL}, {NULL}) > 0
+  if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
+  // CHECK-FIXES: posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) > 0
+}
+
+void warningAlwaysTrue() {
+  if (posix_fadvise(0, 0, 0, 0) >= 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: redundant check as posix_fadvise
+}
+
+void warningEqualsNegative() {
+  if (posix_fadvise(0, 0, 0, 0) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: posix_fadvise
+  if (posix_fadvise(0, 0, 0, 0) != -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fadvise(0, 0, 0, 0) <= -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fadvise(0, 0, 0, 0) < -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fallocate(0, 0, 0) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning:
+  if (posix_madvise(NULL, 0, 0) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_memalign(NULL, 0, 0) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning:
+  if (posix_spawn(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
+  if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
+}
+
+void WarningWithMacro() {
+  if (posix_fadvise(0, 0, 0, 0) < ZERO) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  // CHECK-FIXES: posix_fadvise(0, 0, 0, 0) > ZERO
+  if (posix_fadvise(0, 0, 0, 0) >= ZERO) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fadvise(0, 0, 0, 0) == NEGATIVE_ONE) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fadvise(0, 0, 0, 0) != NEGATIVE_ONE) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fadvise(0, 0, 0, 0) <= NEGATIVE_ONE) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (posix_fadvise(0, 0, 0, 0) < NEGATIVE_ONE) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+}
+
+void noWarning() {
+  if (posix_openpt(0) < 0) {}
+  if (posix_openpt(0) <= 0) {}
+  if (posix_openpt(0) == -1) {}
+  if (posix_openpt(0) != -1) {}
+  if (posix_openpt(0) <= -1) {}
+  if (posix_openpt(0) 

[PATCH] D63789: [ODRHash] Fix null pointer dereference for ObjC selectors with empty slots.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364664: [ODRHash] Fix null pointer dereference for ObjC 
selectors with empty slots. (authored by vsapsai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63789?vs=206992&id=207099#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63789

Files:
  cfe/trunk/lib/AST/ODRHash.cpp
  cfe/trunk/test/Modules/odr_hash.mm


Index: cfe/trunk/test/Modules/odr_hash.mm
===
--- cfe/trunk/test/Modules/odr_hash.mm
+++ cfe/trunk/test/Modules/odr_hash.mm
@@ -57,6 +57,14 @@
 @interface Interface3 
 @end
 
+@interface EmptySelectorSlot
+- (void)method:(int)arg;
+- (void)method:(int)arg :(int)empty;
+
+- (void)multiple:(int)arg1 args:(int)arg2 :(int)arg3;
+- (void)multiple:(int)arg1 :(int)arg2 args:(int)arg3;
+@end
+
 #endif
 
 #if defined(FIRST)
@@ -289,6 +297,29 @@
 }  // namespace ObjCTypeParam
 }  // namespace Types
 
+namespace CallMethods {
+#if defined(FIRST)
+void invalid1(EmptySelectorSlot *obj) {
+  [obj method:0];
+}
+void invalid2(EmptySelectorSlot *obj) {
+  [obj multiple:0 args:0 :0];
+}
+#elif defined(SECOND)
+void invalid1(EmptySelectorSlot *obj) {
+  [obj method:0 :0];
+}
+void invalid2(EmptySelectorSlot *obj) {
+  [obj multiple:0 :0 args:0];
+}
+#endif
+// expected-error@second.h:* {{'CallMethods::invalid1' has different 
definitions in different modules; definition in module 'SecondModule' first 
difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+
+// expected-error@second.h:* {{'CallMethods::invalid2' has different 
definitions in different modules; definition in module 'SecondModule' first 
difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+}  // namespace CallMethods
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST
Index: cfe/trunk/lib/AST/ODRHash.cpp
===
--- cfe/trunk/lib/AST/ODRHash.cpp
+++ cfe/trunk/lib/AST/ODRHash.cpp
@@ -71,8 +71,13 @@
 AddBoolean(S.isKeywordSelector());
 AddBoolean(S.isUnarySelector());
 unsigned NumArgs = S.getNumArgs();
+ID.AddInteger(NumArgs);
 for (unsigned i = 0; i < NumArgs; ++i) {
-  AddIdentifierInfo(S.getIdentifierInfoForSlot(i));
+  const IdentifierInfo *II = S.getIdentifierInfoForSlot(i);
+  AddBoolean(II);
+  if (II) {
+AddIdentifierInfo(II);
+  }
 }
 break;
   }


Index: cfe/trunk/test/Modules/odr_hash.mm
===
--- cfe/trunk/test/Modules/odr_hash.mm
+++ cfe/trunk/test/Modules/odr_hash.mm
@@ -57,6 +57,14 @@
 @interface Interface3 
 @end
 
+@interface EmptySelectorSlot
+- (void)method:(int)arg;
+- (void)method:(int)arg :(int)empty;
+
+- (void)multiple:(int)arg1 args:(int)arg2 :(int)arg3;
+- (void)multiple:(int)arg1 :(int)arg2 args:(int)arg3;
+@end
+
 #endif
 
 #if defined(FIRST)
@@ -289,6 +297,29 @@
 }  // namespace ObjCTypeParam
 }  // namespace Types
 
+namespace CallMethods {
+#if defined(FIRST)
+void invalid1(EmptySelectorSlot *obj) {
+  [obj method:0];
+}
+void invalid2(EmptySelectorSlot *obj) {
+  [obj multiple:0 args:0 :0];
+}
+#elif defined(SECOND)
+void invalid1(EmptySelectorSlot *obj) {
+  [obj method:0 :0];
+}
+void invalid2(EmptySelectorSlot *obj) {
+  [obj multiple:0 :0 args:0];
+}
+#endif
+// expected-error@second.h:* {{'CallMethods::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+
+// expected-error@second.h:* {{'CallMethods::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+}  // namespace CallMethods
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST
Index: cfe/trunk/lib/AST/ODRHash.cpp
===
--- cfe/trunk/lib/AST/ODRHash.cpp
+++ cfe/trunk/lib/AST/ODRHash.cpp
@@ -71,8 +71,13 @@
 AddBoolean(S.isKeywordSelector());
 AddBoolean(S.isUnarySelector());
 unsigned NumArgs = S.getNumArgs();
+ID.AddInteger(NumArgs);
 for (unsigned i = 0; i < NumArgs; ++i) {
-  AddIdentifierInfo(S.getIdentifierInfoForSlot(i));
+  const IdentifierInfo *II = S.getIdentifierInfoForSlot(i);
+  AddBoolean(II);
+  if (II) {
+AddIdentifierInfo(II);
+  }
 }
 break;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r364664 - [ODRHash] Fix null pointer dereference for ObjC selectors with empty slots.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Fri Jun 28 10:42:17 2019
New Revision: 364664

URL: http://llvm.org/viewvc/llvm-project?rev=364664&view=rev
Log:
[ODRHash] Fix null pointer dereference for ObjC selectors with empty slots.

`Selector::getIdentifierInfoForSlot` returns NULL if a slot has no
corresponding identifier. Add a boolean to the hash and a NULL check.

rdar://problem/51615164

Reviewers: rtrieu

Reviewed By: rtrieu

Subscribers: dexonsmith, cfe-commits, jkorous

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


Modified:
cfe/trunk/lib/AST/ODRHash.cpp
cfe/trunk/test/Modules/odr_hash.mm

Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=364664&r1=364663&r2=364664&view=diff
==
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Fri Jun 28 10:42:17 2019
@@ -71,8 +71,13 @@ void ODRHash::AddDeclarationNameImpl(Dec
 AddBoolean(S.isKeywordSelector());
 AddBoolean(S.isUnarySelector());
 unsigned NumArgs = S.getNumArgs();
+ID.AddInteger(NumArgs);
 for (unsigned i = 0; i < NumArgs; ++i) {
-  AddIdentifierInfo(S.getIdentifierInfoForSlot(i));
+  const IdentifierInfo *II = S.getIdentifierInfoForSlot(i);
+  AddBoolean(II);
+  if (II) {
+AddIdentifierInfo(II);
+  }
 }
 break;
   }

Modified: cfe/trunk/test/Modules/odr_hash.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.mm?rev=364664&r1=364663&r2=364664&view=diff
==
--- cfe/trunk/test/Modules/odr_hash.mm (original)
+++ cfe/trunk/test/Modules/odr_hash.mm Fri Jun 28 10:42:17 2019
@@ -57,6 +57,14 @@
 @interface Interface3 
 @end
 
+@interface EmptySelectorSlot
+- (void)method:(int)arg;
+- (void)method:(int)arg :(int)empty;
+
+- (void)multiple:(int)arg1 args:(int)arg2 :(int)arg3;
+- (void)multiple:(int)arg1 :(int)arg2 args:(int)arg3;
+@end
+
 #endif
 
 #if defined(FIRST)
@@ -289,6 +297,29 @@ Invalid3 i3;
 }  // namespace ObjCTypeParam
 }  // namespace Types
 
+namespace CallMethods {
+#if defined(FIRST)
+void invalid1(EmptySelectorSlot *obj) {
+  [obj method:0];
+}
+void invalid2(EmptySelectorSlot *obj) {
+  [obj multiple:0 args:0 :0];
+}
+#elif defined(SECOND)
+void invalid1(EmptySelectorSlot *obj) {
+  [obj method:0 :0];
+}
+void invalid2(EmptySelectorSlot *obj) {
+  [obj multiple:0 :0 args:0];
+}
+#endif
+// expected-error@second.h:* {{'CallMethods::invalid1' has different 
definitions in different modules; definition in module 'SecondModule' first 
difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+
+// expected-error@second.h:* {{'CallMethods::invalid2' has different 
definitions in different modules; definition in module 'SecondModule' first 
difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+}  // namespace CallMethods
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST


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

[PATCH] D56366: New warning call-to-virtual-from-ctor-dtor when calling a virtual function from a constructor or a destructor

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a subscriber: xbolva00.
ArnaudBienner added a comment.

Thanks @xbolva00  for adding reviewers and @rjmccall for reviewing!

@rjmccall as you might remember, we had a discussion on the mailing list 
("Warning when calling virtual functions from constructor/desctructor?") and 
from what I understand the overall feeling was that this patch/warning won't be 
accepted until we move forward with your proposal of having a "-Wversion=" flag 
to allow deactivate new warnings when upgrading clang, but enable them by 
default otherwise.
Have you made any progress on that? Or do you think the warning can be 
implemented anyway? (maybe off by default?)

Just want to avoid spending our time reviewing/doing changes on a patch that 
won't be accepted in the end :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D56366



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

[PATCH] D63734: Update CODE_OWNERS.txt for clang-doc

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364663: Update CODE_OWNERS.txt for clang-doc (authored by 
juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63734?vs=206278&id=207095#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63734

Files:
  clang-tools-extra/trunk/CODE_OWNERS.TXT


Index: clang-tools-extra/trunk/CODE_OWNERS.TXT
===
--- clang-tools-extra/trunk/CODE_OWNERS.TXT
+++ clang-tools-extra/trunk/CODE_OWNERS.TXT
@@ -19,3 +19,7 @@
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc


Index: clang-tools-extra/trunk/CODE_OWNERS.TXT
===
--- clang-tools-extra/trunk/CODE_OWNERS.TXT
+++ clang-tools-extra/trunk/CODE_OWNERS.TXT
@@ -19,3 +19,7 @@
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r364663 - Update CODE_OWNERS.txt for clang-doc

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Fri Jun 28 10:32:26 2019
New Revision: 364663

URL: http://llvm.org/viewvc/llvm-project?rev=364663&view=rev
Log:
Update CODE_OWNERS.txt for clang-doc

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

Modified:
clang-tools-extra/trunk/CODE_OWNERS.TXT

Modified: clang-tools-extra/trunk/CODE_OWNERS.TXT
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/CODE_OWNERS.TXT?rev=364663&r1=364662&r2=364663&view=diff
==
--- clang-tools-extra/trunk/CODE_OWNERS.TXT (original)
+++ clang-tools-extra/trunk/CODE_OWNERS.TXT Fri Jun 28 10:32:26 2019
@@ -19,3 +19,7 @@ D: clang-rename, all parts of clang-tool
 N: Alexander Kornienko
 E: ale...@google.com
 D: clang-tidy
+
+N: Julie Hockett
+E: juliehock...@google.com
+D: clang-doc


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

[PATCH] D63773: [clangd] dummy variable extraction on a function scope

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 207084.
SureYeaah added a comment.

[Clangd] Refactored code

- Created new class Extract to store information about the expression being 
extracted.
- Doesn't fix all of previous comments

Looking for comments on the new class stucture


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63773

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -239,13 +239,13 @@
   checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }");
 
   const char *Input = "int x = 2 ^+ 2;";
-  auto result = getMessage(ID, Input);
-  EXPECT_THAT(result, ::testing::HasSubstr("BinaryOperator"));
-  EXPECT_THAT(result, ::testing::HasSubstr("'+'"));
-  EXPECT_THAT(result, ::testing::HasSubstr("|-IntegerLiteral"));
-  EXPECT_THAT(result,
+  auto Result = getMessage(ID, Input);
+  EXPECT_THAT(Result, ::testing::HasSubstr("BinaryOperator"));
+  EXPECT_THAT(Result, ::testing::HasSubstr("'+'"));
+  EXPECT_THAT(Result, ::testing::HasSubstr("|-IntegerLiteral"));
+  EXPECT_THAT(Result,
   ::testing::HasSubstr(" 'int' 2\n`-IntegerLiteral"));
-  EXPECT_THAT(result, ::testing::HasSubstr(" 'int' 2"));
+  EXPECT_THAT(Result, ::testing::HasSubstr(" 'int' 2"));
 }
 
 TEST(TweakTest, ShowSelectionTree) {
@@ -277,6 +277,209 @@
   const char *Input = "struct ^X { int x; int y; }";
   EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 |   int x"));
 }
+TEST(TweakTest, ExtractVariable) {
+  llvm::StringLiteral ID = "ExtractVariable";
+  checkAvailable(ID, R"cpp(
+int xyz() {
+  return 1;
+}
+void f() {
+  int a = 5 + [[4 * [[^xyz();
+  int x = ^1, y = x + 1, z = ^1;
+  switch(a) {
+case 1: {
+  a = ^1;
+  break;
+}
+default: {
+  a = ^3; 
+}
+  }
+  // if testing
+  if(^1) {}
+  if(a < ^3)
+if(a == 4)
+  a = 5;
+else
+  a = 6;
+  else if (a < 4) {
+a = ^4;
+  }
+  else {
+a = ^5;
+  }
+  // for loop testing
+  for(a = ^1; a > ^3+^4; a++) 
+a = 2;
+  // while testing
+  while(a < ^1) {
+^a++;
+  }
+  // do while testing
+  do
+a = 1;
+  while(a < ^3);
+}
+  )cpp");
+  checkNotAvailable(ID, R"cpp(
+void f(int b = ^1) {
+  int a = 5 + 4 * 3;
+  // check whether extraction breaks scope
+  int x = 1, y = ^x + 1;
+  // switch testing
+  switch(a) {
+case 1: 
+  a = ^1;
+  break;
+default:
+  a = ^3; 
+  }
+  // if testing
+  if(a < 3)
+if(a == ^4)
+  a = ^5;
+else
+  a = ^6;
+  else if (a < ^4) {
+a = 4;
+  }
+  else {
+a = 5;
+  }
+  // for loop testing
+  for(int a = 1, b = 2, c = 3; ^a > ^b ^+ ^c; ^a++) 
+a = ^2;
+  // while testing
+  while(a < 1) {
+a++;
+  }
+  // do while testing
+  do
+a = ^1;
+  while(a < 3);
+  // testing in cases where braces are required
+  if (true)
+do
+  a = 1;
+while(a < ^1);
+}
+  )cpp");
+  // vector of pairs of input and output strings
+  const std::vector>
+  InputOutputs = {
+  // extraction from variable declaration/assignment
+  {R"cpp(void varDecl() {
+   int a = 5 * (4 + (3 [[- 1)]]);
+ })cpp",
+   R"cpp(void varDecl() {
+   auto dummy = (3 - 1); int a = 5 * (4 + dummy);
+ })cpp"},
+  // extraction from for loop init/cond/incr
+  {R"cpp(void forLoop() {
+   for(int a = 1; a < ^3; a++) {
+ a = 5 + 4 * 3;
+   }
+ })cpp",
+   R"cpp(void forLoop() {
+   auto dummy = 3; for(int a = 1; a < dummy; a++) {
+ a = 5 + 4 * 3;
+   }
+ })cpp"},
+  // extraction inside for loop body
+  {R"cpp(void forBody() {
+   for(int a = 1; a < 3; a++) {
+ a = 5 + [[4 * 3]];
+   }
+ })cpp",
+   R"cpp(void forBody() {
+   for(int a = 1; a < 3; a++) {
+ auto dummy = 4 * 3; a = 5 + dummy;
+   }
+ })cpp"},
+  // extraction inside while loop condition
+  {R"cpp(void whileLoop(int a) {
+   while(a < 5 + [[4 * 3]]) 
+ a += 1;
+   

[PATCH] D63940: [NFC] Pass DataLayout into isBytewiseValue

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
vitalybuka added a parent revision: D63854: [NFC] Convert large lambda into 
method.

We will need to handle IntToPtr which I will submit in a separate patch as it's
not going to be NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63940

Files:
  clang/lib/CodeGen/CGDecl.cpp
  llvm/include/llvm/Analysis/ValueTracking.h
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -412,7 +412,7 @@
   if (!NextStore->isSimple()) break;
 
   // Check to see if this stored value is of the same byte-splattable value.
-  Value *StoredByte = isBytewiseValue(NextStore->getOperand(0));
+  Value *StoredByte = isBytewiseValue(NextStore->getOperand(0), DL);
   if (isa(ByteVal) && StoredByte)
 ByteVal = StoredByte;
   if (ByteVal != StoredByte)
@@ -749,7 +749,7 @@
   // byte at a time like "0" or "-1" or any width, as well as things like
   // 0xA0A0A0A0 and 0.0.
   auto *V = SI->getOperand(0);
-  if (Value *ByteVal = isBytewiseValue(V)) {
+  if (Value *ByteVal = isBytewiseValue(V, DL)) {
 if (Instruction *I = tryMergingIntoMemset(SI, SI->getPointerOperand(),
   ByteVal)) {
   BBI = I->getIterator(); // Don't invalidate iterator.
@@ -1229,7 +1229,8 @@
   // If copying from a constant, try to turn the memcpy into a memset.
   if (GlobalVariable *GV = dyn_cast(M->getSource()))
 if (GV->isConstant() && GV->hasDefinitiveInitializer())
-  if (Value *ByteVal = isBytewiseValue(GV->getInitializer())) {
+  if (Value *ByteVal = isBytewiseValue(GV->getInitializer(),
+   M->getModule()->getDataLayout())) {
 IRBuilder<> Builder(M);
 Builder.CreateMemSet(M->getRawDest(), ByteVal, M->getLength(),
  M->getDestAlignment(), false);
Index: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
===
--- llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
+++ llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
@@ -450,7 +450,7 @@
   // turned into a memset of i8 -1, assuming that all the consecutive bytes
   // are stored.  A store of i32 0x01020304 can never be turned into a memset,
   // but it can be turned into memset_pattern if the target supports it.
-  Value *SplatValue = isBytewiseValue(StoredVal);
+  Value *SplatValue = isBytewiseValue(StoredVal, *DL);
   Constant *PatternValue = nullptr;
 
   // Note: memset and memset_pattern on unordered-atomic is yet not supported
@@ -627,7 +627,7 @@
 Constant *FirstPatternValue = nullptr;
 
 if (For == ForMemset::Yes)
-  FirstSplatValue = isBytewiseValue(FirstStoredVal);
+  FirstSplatValue = isBytewiseValue(FirstStoredVal, *DL);
 else
   FirstPatternValue = getMemSetPatternValue(FirstStoredVal, DL);
 
@@ -660,7 +660,7 @@
   Constant *SecondPatternValue = nullptr;
 
   if (For == ForMemset::Yes)
-SecondSplatValue = isBytewiseValue(SecondStoredVal);
+SecondSplatValue = isBytewiseValue(SecondStoredVal, *DL);
   else
 SecondPatternValue = getMemSetPatternValue(SecondStoredVal, DL);
 
@@ -880,7 +880,7 @@
 Value *StoredVal, Instruction *TheStore,
 SmallPtrSetImpl &Stores, const SCEVAddRecExpr *Ev,
 const SCEV *BECount, bool NegStride, bool IsLoopMemset) {
-  Value *SplatValue = isBytewiseValue(StoredVal);
+  Value *SplatValue = isBytewiseValue(StoredVal, *DL);
   Constant *PatternValue = nullptr;
 
   if (!SplatValue)
Index: llvm/lib/Analysis/ValueTracking.cpp
===
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -3166,7 +3166,7 @@
   return true;
 }
 
-Value *llvm::isBytewiseValue(Value *V) {
+Value *llvm::isBytewiseValue(Value *V, const DataLayout &DL) {
 
   // All byte-wide stores are splatable, even of arbitrary variables.
   if (V->getType()->isIntegerTy(8))
@@ -3205,7 +3205,8 @@
 else if (CFP->getType()->isDoubleTy())
   Ty = Type::getInt64Ty(Ctx);
 // Don't handle long double formats, which have strange constraints.
-return Ty ? isBytewiseValue(ConstantExpr::getBitCast(CFP, Ty)) : nullptr;
+return Ty ? isBytewiseValue(ConstantExpr::getBitCast(CFP, Ty), DL)
+  : nullptr;
   }
 
   // We can handle constant integers that are multiple of 8 bits.
@@ -3233,20 +3234,20 @@
   if (ConstantDataSequential *CA = dyn_cast(C)) {
 Value *Val = UndefInt8;
 for (unsigned I = 0, E = CA->getNumElements(); I != E; ++I)
- 

[PATCH] D63922: [clangd] Show better message when we rename macros.

[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/SourceCode.cpp:661
+   Preprocessor &PP) {
+  // Also handle possible macro at the searched location.
+  Token Result;

"also" no longer makes sense here



Comment at: clang-tools-extra/clangd/SourceCode.h:204
 
+struct MacroSym {
+  llvm::StringRef Name;

not sure about "sym" - it's an abbreviation and not very descriptive.
`MacroDefinition` is really the best name, but taken :-(

Maybe `DefinedMacro` or `ResolvedMacro`?



Comment at: clang-tools-extra/clangd/SourceCode.h:204
 
+struct MacroSym {
+  llvm::StringRef Name;

sammccall wrote:
> not sure about "sym" - it's an abbreviation and not very descriptive.
> `MacroDefinition` is really the best name, but taken :-(
> 
> Maybe `DefinedMacro` or `ResolvedMacro`?
(thanks for moving this here, definitely makes sense!)



Comment at: clang-tools-extra/clangd/SourceCode.h:209
+// Gets the macro at a specified \p Loc.
+llvm::Optional locateMacroAt(SourceLocation Loc,
+   const SourceManager &SM,

Can we add tests for this now that it's a public API?



Comment at: clang-tools-extra/clangd/SourceCode.h:210
+llvm::Optional locateMacroAt(SourceLocation Loc,
+   const SourceManager &SM,
+   const LangOptions &LangOpts,

you can get the SM and LangOpts from the PP



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:167
   AST, Pos, AST.getSourceManager().getMainFileID());
+  // FIXME: this should be moved to rename tooling library?
+  if (locateMacroAt(SourceLocationBeg, ASTCtx.getSourceManager(),

is the fixme to detect the error, or to support renaming macros?
(And if we're not going to support them, why not?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63922



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

[PATCH] D63616: Implement `-fsanitize-coverage-whitelist` and `-fsanitize-coverage-blacklist` for clang

[Yannis Juglaret via Phabricator via cfe-commits]

tuktuk updated this revision to Diff 207086.
tuktuk edited the summary of this revision.
tuktuk added a comment.

I followed Matt Morehouse's advice: mainly, I adapted the test so that it uses 
libFuzzer's default SanitizerCoverage options instead of `trace-pc`, and I 
rewrote some parts of the code to make it less redundant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63616

Files:
  clang/docs/SanitizerCoverage.rst
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cc
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
@@ -179,8 +180,16 @@
 class SanitizerCoverageModule : public ModulePass {
 public:
   SanitizerCoverageModule(
-  const SanitizerCoverageOptions &Options = SanitizerCoverageOptions())
+  const SanitizerCoverageOptions &Options = SanitizerCoverageOptions(),
+  const std::vector &WhitelistFiles =
+  std::vector(),
+  const std::vector &BlacklistFiles =
+  std::vector())
   : ModulePass(ID), Options(OverrideFromCL(Options)) {
+if (WhitelistFiles.size() > 0)
+  Whitelist = SpecialCaseList::createOrDie(WhitelistFiles);
+if (BlacklistFiles.size() > 0)
+  Blacklist = SpecialCaseList::createOrDie(BlacklistFiles);
 initializeSanitizerCoverageModulePass(*PassRegistry::getPassRegistry());
   }
   bool runOnModule(Module &M) override;
@@ -250,6 +259,9 @@
   SmallVector GlobalsToAppendToCompilerUsed;
 
   SanitizerCoverageOptions Options;
+
+  std::unique_ptr Whitelist;
+  std::unique_ptr Blacklist;
 };
 
 } // namespace
@@ -313,6 +325,12 @@
 bool SanitizerCoverageModule::runOnModule(Module &M) {
   if (Options.CoverageType == SanitizerCoverageOptions::SCK_None)
 return false;
+  if (Whitelist &&
+  !Whitelist->inSection("coverage", "src", M.getSourceFileName()))
+return false;
+  if (Blacklist &&
+  Blacklist->inSection("coverage", "src", M.getSourceFileName()))
+return false;
   C = &(M.getContext());
   DL = &M.getDataLayout();
   CurModule = &M;
@@ -541,6 +559,10 @@
   if (F.hasPersonalityFn() &&
   isAsynchronousEHPersonality(classifyEHPersonality(F.getPersonalityFn(
 return false;
+  if (Whitelist && !Whitelist->inSection("coverage", "fun", F.getName()))
+return false;
+  if (Blacklist && Blacklist->inSection("coverage", "fun", F.getName()))
+return false;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge)
 SplitAllCriticalEdges(F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
   SmallVector IndirCalls;
@@ -898,6 +920,8 @@
 "ModulePass",
 false, false)
 ModulePass *llvm::createSanitizerCoverageModulePass(
-const SanitizerCoverageOptions &Options) {
-  return new SanitizerCoverageModule(Options);
+const SanitizerCoverageOptions &Options,
+const std::vector &WhitelistFiles,
+const std::vector &BlacklistFiles) {
+  return new SanitizerCoverageModule(Options, WhitelistFiles, BlacklistFiles);
 }
Index: llvm/include/llvm/Transforms/Instrumentation.h
===
--- llvm/include/llvm/Transforms/Instrumentation.h
+++ llvm/include/llvm/Transforms/Instrumentation.h
@@ -183,7 +183,9 @@
 
 // Insert SanitizerCoverage instrumentation.
 ModulePass *createSanitizerCoverageModulePass(
-const SanitizerCoverageOptions &Options = SanitizerCoverageOptions());
+const SanitizerCoverageOptions &Options = SanitizerCoverageOptions(),
+const std::vector& WhitelistFiles = std::vector(),
+const std::vector& BlacklistFiles = std::vector());
 
 /// Calculate what to divide by to scale counts.
 ///
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cc
===
--- /dev/null
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cc
@@ -0,0 +1,118 @@
+// Tests -fsanitize-coverage-whitelist=whitelist.txt and
+//

[PATCH] D63822: [Driver] Fix style issues of --print-supported-cpus

[Ziang Wan via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D63822



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

[PATCH] D63892: [LibTooling] Extend `RewriteRule` with support for adding includes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Responded to comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63892

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -198,6 +198,42 @@
   testRule(std::move(Rule), Input, Expected);
 }
 
+TEST_F(TransformerTest, AddIncludeQuoted) {
+  RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
+  change(text("other()")));
+  addInclude(Rule, "clang/OtherLib.h");
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include "clang/OtherLib.h"
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
+TEST_F(TransformerTest, AddIncludeAngled) {
+  RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
+  change(text("other()")));
+  addInclude(Rule, "clang/OtherLib.h", IncludeFormat::Angled);
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include 
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
 TEST_F(TransformerTest, NodePartNameNamedDecl) {
   StringRef Fun = "fun";
   RewriteRule Rule = makeRule(functionDecl(hasName("bad")).bind(Fun),
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -98,8 +98,14 @@
 
 RewriteRule tooling::makeRule(DynTypedMatcher M, SmallVector Edits,
   TextGenerator Explanation) {
-  return RewriteRule{{RewriteRule::Case{std::move(M), std::move(Edits),
-std::move(Explanation)}}};
+  return RewriteRule{{RewriteRule::Case{
+  std::move(M), std::move(Edits), std::move(Explanation), {;
+}
+
+void tooling::addInclude(RewriteRule &Rule, StringRef Header,
+ IncludeFormat Format) {
+  for (auto &Case : Rule.Cases)
+Case.AddedIncludes.emplace_back(Header.str(), Format);
 }
 
 // Determines whether A is a base type of B in the class hierarchy, including
@@ -217,8 +223,8 @@
   Root->second.getSourceRange().getBegin());
   assert(RootLoc.isValid() && "Invalid location for Root node of match.");
 
-  auto Transformations = tooling::detail::translateEdits(
-  Result, tooling::detail::findSelectedCase(Result, Rule).Edits);
+  RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, Rule);
+  auto Transformations = tooling::detail::translateEdits(Result, Case.Edits);
   if (!Transformations) {
 Consumer(Transformations.takeError());
 return;
@@ -241,5 +247,17 @@
 }
   }
 
+  for (const auto &I : Case.AddedIncludes) {
+auto &Header = I.first;
+switch (I.second) {
+  case IncludeFormat::Quoted:
+AC.addHeader(Header);
+break;
+  case IncludeFormat::Angled:
+AC.addHeader((llvm::Twine("<") + Header + ">").str());
+break;
+}
+  }
+
   Consumer(std::move(AC));
 }
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -86,6 +86,12 @@
   TextGenerator Note;
 };
 
+/// Format of the path in an include directive -- angle brackets or quotes.
+enum class IncludeFormat {
+  Quoted,
+  Angled,
+};
+
 /// Description of a source-code transformation.
 //
 // A *rewrite rule* describes a transformation of source code. A simple rule
@@ -114,6 +120,10 @@
 ast_matchers::internal::DynTypedMatcher Matcher;
 SmallVector Edits;
 TextGenerator Explanation;
+// Include paths to add to the file affected by this case.  These are
+// bundled with the `Case`, rather than the `RewriteRule`, because each case
+// might have different associated changes to the includes.
+std::vector> AddedIncludes;
   };
   // We expect RewriteRules will most commonly include only one case.
   SmallVector Cases;
@@ -137,6 +147,19 @@
   return makeRule(std::move(M), std::move(Edits), std::move(Explanation));
 }
 
+/// For every case in Rule, adds an include directive for the given header. The
+/// common use is assumed to be a rule with only one case. For example, to
+/// replace a function call and add headers corresponding to the new code, one
+/

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

rjmccall wrote:
> It looks like you're generally warning about this based on the specific 
> context that forced an lvalue-to-rvalue conversion.  I'm not sure `volatile` 
> is special except that we actually perform the load even in unused-value 
> contexts.  Is the assumption that you've exhaustively covered all the other 
> contexts of lvalue-to-rvalue conversions whose values will actually be used?  
> That seems questionable to me.
Yes, that was my assumption. All the other contexts where lvalue-to-rvalue 
conversion is performed and the result is used are already covered by other 
calls sites of `checkNonTrivialCUnion`, which informs the users that the 
struct/union is being used in an invalid context.

Do you have a case in mind that I didn't think of where a lvalue-to-rvalue 
conversion requires a non-trivial initialization/destruction/copying of a union 
but clang fails to emit any diagnostics?

Also I realized that lvalue-to-rvalue conversion of volatile types doesn't 
always require non-trivial destruction, so I think `CheckDestruct` shouldn't be 
set in this case.

```
void foo(U0 *a, volatile U0 *b) {
  // this doesn't require destruction.
  // this is perfectly valid if U0 is non-trivial to destruct but trivial to 
copy.
  *a = *b;  
}
```

For the same reason, I think `CheckDestruct` shouldn't be set for function 
returns (but it should be set for function parameters since they are destructed 
by the callee).


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753



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

[PATCH] D63893: [clang-tidy] Extend TransformerClangTidyCheck to support adding includes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Added check for use of AddedIncludes before allocating/registering 
IncludeInserter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63893

Files:
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -19,6 +19,7 @@
 namespace utils {
 namespace {
 using tooling::change;
+using tooling::IncludeFormat;
 using tooling::RewriteRule;
 using tooling::text;
 using tooling::stencil::cat;
@@ -121,6 +122,54 @@
   Input, nullptr, "input.cc", None, Options));
 }
 
+RewriteRule replaceCall(IncludeFormat Format) {
+  using namespace ::clang::ast_matchers;
+  RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
+  change(text("other()")), text("no message"));
+  addInclude(Rule, "clang/OtherLib.h", Format);
+  return Rule;
+}
+
+template 
+class IncludeCheck : public TransformerClangTidyCheck {
+public:
+  IncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : TransformerClangTidyCheck(replaceCall(Format), Name, Context) {}
+};
+
+TEST(TransformerClangTidyCheckTest, AddIncludeQuoted) {
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include "clang/OtherLib.h"
+
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  EXPECT_EQ(Expected,
+test::runCheckOnCode>(Input));
+}
+
+TEST(TransformerClangTidyCheckTest, AddIncludeAngled) {
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include 
+
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  EXPECT_EQ(Expected,
+test::runCheckOnCode>(Input));
+}
+
 } // namespace
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -10,7 +10,9 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_TRANSFORMER_CLANG_TIDY_CHECK_H
 
 #include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Tooling/Refactoring/Transformer.h"
 #include 
 #include 
@@ -52,11 +54,14 @@
   TransformerClangTidyCheck(tooling::RewriteRule R, StringRef Name,
 ClangTidyContext *Context);
 
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) final;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
 
 private:
   Optional Rule;
+  std::unique_ptr Inserter;
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -44,6 +44,20 @@
  " explicitly provide an empty explanation if none is desired");
 }
 
+void TransformerClangTidyCheck::registerPPCallbacks(
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  // Only allocate and register the IncludeInsert when some `Case` will add
+  // includes.
+  if (Rule && std::any_of(Rule->Cases.begin(), Rule->Cases.end(),
+  [](const RewriteRule::Case &C) {
+return !C.AddedIncludes.empty();
+  })) {
+Inserter = llvm::make_unique(
+SM, getLangOpts(), utils::IncludeSorter::IS_LLVM);
+PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  }
+}
+
 void TransformerClangTidyCheck::registerMatchers(
 ast_matchers::MatchFinder *Finder) {
   if (Rule)
@@ -87,6 +101,15 @@
   for (const auto &T : *Transformations) {
 Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
   }
+
+  for (const auto &I : Case.AddedIncludes) {
+auto &Header = I.first;
+if (Optional Fix = Inserter->CreateIncludeInsertion(
+Result.SourceManager->getMainFileID(), Header,
+/*IsAngled=*/I.second == tooling::IncludeFormat::Angled)) {
+  D

[PATCH] D63892: [LibTooling] Extend `RewriteRule` with support for adding includes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 207083.
ymandel marked 2 inline comments as done.
ymandel added a comment.

comments tweak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63892

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -198,6 +198,42 @@
   testRule(std::move(Rule), Input, Expected);
 }
 
+TEST_F(TransformerTest, AddIncludeQuoted) {
+  RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
+  change(text("other()")));
+  addInclude(Rule, "clang/OtherLib.h");
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include "clang/OtherLib.h"
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
+TEST_F(TransformerTest, AddIncludeAngled) {
+  RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
+  change(text("other()")));
+  addInclude(Rule, "clang/OtherLib.h", IncludeFormat::Angled);
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include 
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
 TEST_F(TransformerTest, NodePartNameNamedDecl) {
   StringRef Fun = "fun";
   RewriteRule Rule = makeRule(functionDecl(hasName("bad")).bind(Fun),
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -98,8 +98,14 @@
 
 RewriteRule tooling::makeRule(DynTypedMatcher M, SmallVector Edits,
   TextGenerator Explanation) {
-  return RewriteRule{{RewriteRule::Case{std::move(M), std::move(Edits),
-std::move(Explanation)}}};
+  return RewriteRule{{RewriteRule::Case{
+  std::move(M), std::move(Edits), std::move(Explanation), {;
+}
+
+void tooling::addInclude(RewriteRule &Rule, StringRef Header,
+ IncludeFormat Format) {
+  for (auto &Case : Rule.Cases)
+Case.AddedIncludes.emplace_back(Header.str(), Format);
 }
 
 // Determines whether A is a base type of B in the class hierarchy, including
@@ -217,8 +223,8 @@
   Root->second.getSourceRange().getBegin());
   assert(RootLoc.isValid() && "Invalid location for Root node of match.");
 
-  auto Transformations = tooling::detail::translateEdits(
-  Result, tooling::detail::findSelectedCase(Result, Rule).Edits);
+  RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, Rule);
+  auto Transformations = tooling::detail::translateEdits(Result, Case.Edits);
   if (!Transformations) {
 Consumer(Transformations.takeError());
 return;
@@ -241,5 +247,17 @@
 }
   }
 
+  for (const auto &I : Case.AddedIncludes) {
+auto &Header = I.first;
+switch (I.second) {
+  case IncludeFormat::Quoted:
+AC.addHeader(Header);
+break;
+  case IncludeFormat::Angled:
+AC.addHeader((llvm::Twine("<") + Header + ">").str());
+break;
+}
+  }
+
   Consumer(std::move(AC));
 }
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -86,6 +86,12 @@
   TextGenerator Note;
 };
 
+/// Format of the path in an include directive -- angle brackets or quotes.
+enum class IncludeFormat {
+  Quoted,
+  Angled,
+};
+
 /// Description of a source-code transformation.
 //
 // A *rewrite rule* describes a transformation of source code. A simple rule
@@ -114,6 +120,10 @@
 ast_matchers::internal::DynTypedMatcher Matcher;
 SmallVector Edits;
 TextGenerator Explanation;
+// Include paths to add to the file affected by this case.  These are
+// bundled with the `Case`, rather than the `RewriteRule`, because each case
+// might have different associated changes to the includes.
+std::vector> AddedIncludes;
   };
   // We expect RewriteRules will most commonly include only one case.
   SmallVector Cases;
@@ -137,6 +147,19 @@
   return makeRule(std::move(M), std::move(Edits), std::move(Explanation));
 }
 
+/// For every case in Rule, adds an include directive for the given header. The
+/// common use is assumed to be a rule with only one case. For example, to
+/// replace a function call and add headers co

[PATCH] D63936: [ARM] Minor fixes in command line option parsing

[Alexandros Lamprineas via Phabricator via cfe-commits]

labrinea created this revision.
labrinea added reviewers: simon_tatham, SjoerdMeijer, ostannard.
Herald added subscribers: dmgreen, hiraditya, kristof.beyls, javed.absar.
Herald added projects: clang, LLVM.

When processing the command line options `march`, `mcpu` and `mfpu`,  we store 
the implied target features on a vector.  The change D62998 
 introduced a temporary vector, where the 
processed features get accumulated. When calling `DecodeARMFeaturesFromCPU`, 
which sets the default features for the specified CPU, we certainly don't want 
to override the features that have been explicitly specified on the command 
line. Therefore, the default features should appear first in the final vector. 
This problem became evident once I added the missing (unhandled) target 
features in `ARM::getExtensionFeatures` and I am fixing it with this patch.

The second change this patch makes is that it teaches 
`ARM::appendArchExtFeatures` to account dependencies when processing target 
features: i.e. when you say `-march=armv8.1-m.main+mve.fp+nofp` it means 
`mve.fp` should get discarded too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63936

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/CodeGen/arm-target-features.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -571,17 +571,18 @@
 TEST(TargetParserTest, ARMExtensionFeatures) {
   std::map> Extensions;
 
-  Extensions[ARM::AEK_CRC]= { "+crc",   "-crc" };
-  Extensions[ARM::AEK_DSP]= { "+dsp",   "-dsp" };
+  for (auto &Ext : ARM::ARCHExtNames) {
+if (Ext.Feature && Ext.NegFeature)
+  Extensions[Ext.ID] = { StringRef(Ext.Feature),
+ StringRef(Ext.NegFeature) };
+  }
+
   Extensions[ARM::AEK_HWDIVARM]   = { "+hwdiv-arm", "-hwdiv-arm" };
   Extensions[ARM::AEK_HWDIVTHUMB] = { "+hwdiv", "-hwdiv" };
-  Extensions[ARM::AEK_RAS]= { "+ras",   "-ras" };
-  Extensions[ARM::AEK_FP16FML]= { "+fp16fml",   "-fp16fml" };
-  Extensions[ARM::AEK_DOTPROD]= { "+dotprod",   "-dotprod" };
 
   std::vector Features;
 
-  EXPECT_FALSE(AArch64::getExtensionFeatures(ARM::AEK_INVALID, Features));
+  EXPECT_FALSE(ARM::getExtensionFeatures(ARM::AEK_INVALID, Features));
 
   for (auto &E : Extensions) {
 // test +extension
@@ -598,7 +599,7 @@
 Found = std::find(std::begin(Features), std::end(Features), E.second.at(1));
 EXPECT_TRUE(Found != std::end(Features));
 EXPECT_TRUE(Extensions.size() == Features.size());
-   }
+  }
 }
 
 TEST(TargetParserTest, ARMFPUFeatures) {
Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -409,30 +409,12 @@
   if (Extensions == AEK_INVALID)
 return false;
 
-  if (Extensions & AEK_CRC)
-Features.push_back("+crc");
-  else
-Features.push_back("-crc");
-
-  if (Extensions & AEK_DSP)
-Features.push_back("+dsp");
-  else
-Features.push_back("-dsp");
-
-  if (Extensions & AEK_FP16FML)
-Features.push_back("+fp16fml");
-  else
-Features.push_back("-fp16fml");
-
-  if (Extensions & AEK_RAS)
-Features.push_back("+ras");
-  else
-Features.push_back("-ras");
-
-  if (Extensions & AEK_DOTPROD)
-Features.push_back("+dotprod");
-  else
-Features.push_back("-dotprod");
+  for (const auto AE : ARCHExtNames) {
+if ((Extensions & AE.ID) == AE.ID && AE.Feature)
+  Features.push_back(AE.Feature);
+else if (AE.NegFeature)
+  Features.push_back(AE.NegFeature);
+  }
 
   return getHWDivFeatures(Extensions, Features);
 }
@@ -508,16 +490,30 @@
   return ARM::FK_INVALID;
 }
 
+static unsigned getAEKID(StringRef ArchExtName) {
+  for (const auto AE : ARM::ARCHExtNames)
+if (AE.getName() == ArchExtName)
+  return AE.ID;
+  return ARM::AEK_INVALID;
+}
+
 bool ARM::appendArchExtFeatures(
   StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
   std::vector &Features) {
-  StringRef StandardFeature = getArchExtFeature(ArchExt);
-  if (!StandardFeature.empty()) {
-Features.push_back(StandardFeature);
-return true;
-  }
 
+  size_t StartingNumFeatures = Features.size();
   const bool Negated = stripNegationPrefix(ArchExt);
+  unsigned ID = getAEKID(ArchExt);
+
+  if (ID == AEK_INVALID)
+return false;
+
+  for (const auto AE : ARCHExtNames) {
+if (Negated && (AE.ID & ID) == ID && AE.NegFeature)
+  Features.push_back(AE.NegFeature);
+else if (AE.ID == ID && AE.Feature)
+  Features.push_back(AE.Feature);
+  }
 
   if (CPU == "")
 CPU = "generic

r364655 - [OPENMP]Fix top DSA for static members.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Jun 28 09:16:00 2019
New Revision: 364655

URL: http://llvm.org/viewvc/llvm-project?rev=364655&view=rev
Log:
[OPENMP]Fix top DSA for static members.

Fixed handling of the data-sharing attributes for static members when
requesting top most attribute. Previously, it might return the incorrect
attributes for static members if they were overriden in the outer
constructs.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/simd_loop_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=364655&r1=364654&r2=364655&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Jun 28 09:16:00 2019
@@ -1354,16 +1354,28 @@ const DSAStackTy::DSAVarData DSAStackTy:
   // in a Construct, C/C++, predetermined, p.7]
   //  Variables with static storage duration that are declared in a scope
   //  inside the construct are shared.
-  auto &&MatchesAlways = [](OpenMPDirectiveKind) { return true; };
   if (VD && VD->isStaticDataMember()) {
-DSAVarData DVarTemp = hasDSA(D, isOpenMPPrivate, MatchesAlways, 
FromParent);
-if (DVarTemp.CKind != OMPC_unknown && DVarTemp.RefExpr)
+// Check for explicitly specified attributes.
+const_iterator I = begin();
+const_iterator EndI = end();
+if (FromParent && I != EndI)
+  ++I;
+auto It = I->SharingMap.find(D);
+if (It != I->SharingMap.end()) {
+  const DSAInfo &Data = It->getSecond();
+  DVar.RefExpr = Data.RefExpr.getPointer();
+  DVar.PrivateCopy = Data.PrivateCopy;
+  DVar.CKind = Data.Attributes;
+  DVar.ImplicitDSALoc = I->DefaultAttrLoc;
+  DVar.DKind = I->Directive;
   return DVar;
+}
 
 DVar.CKind = OMPC_shared;
 return DVar;
   }
 
+  auto &&MatchesAlways = [](OpenMPDirectiveKind) { return true; };
   // The predetermined shared attribute for const-qualified types having no
   // mutable members was removed after OpenMP 3.1.
   if (SemaRef.LangOpts.OpenMP <= 31) {

Modified: cfe/trunk/test/OpenMP/simd_loop_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/simd_loop_messages.cpp?rev=364655&r1=364654&r2=364655&view=diff
==
--- cfe/trunk/test/OpenMP/simd_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/simd_loop_messages.cpp Fri Jun 28 09:16:00 2019
@@ -9,6 +9,7 @@ static int sii;
 static int globalii;
 
 struct S {
+  // expected-note@+1 {{static data member is predetermined as shared}}
   static int ssi;
 };
 
@@ -21,6 +22,10 @@ int test_iteration_spaces() {
 #pragma omp simd linear(S::ssi)
   for (S::ssi = 0; S::ssi < 10; ++S::ssi)
 ;
+// expected-error@+1 {{shared variable cannot be private}}
+#pragma omp simd private(S::ssi)
+  for (S::ssi = 0; S::ssi < 10; ++S::ssi)
+;
 #pragma omp simd // no messages expected
   for (S::ssi = 0; S::ssi < 10; ++S::ssi)
 ;


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

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

@ruiu, we have some hardware, which bootloader does not support PT_GNU_STACK 
segments, and would therefore benefit quite a bit from this option. As this 
does not really depend on NetBSD support, could you please merge this patch in 
time for 9.0? Thanks!


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

https://reviews.llvm.org/D56554



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

r364650 - [OPENMP]Fix DSA for loop iteration variables in simd loops.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Jun 28 08:16:37 2019
New Revision: 364650

URL: http://llvm.org/viewvc/llvm-project?rev=364650&view=rev
Log:
[OPENMP]Fix DSA for loop iteration variables in simd loops.

According to the OpenMP 5.0 standard, the loop iteration variable in the 
associated
for-loop of a simd construct with just one associated for-loop may be
listed in a private, lastprivate, or linear clause with a linear-step
that is the increment of the associated for-loop. Also, the loop
teration variables in the associated for-loops of a simd construct with
multiple associated for-loops may be listed in a private or lastprivate
clause.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/simd_loop_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=364650&r1=364649&r2=364650&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Jun 28 08:16:37 2019
@@ -5697,7 +5697,9 @@ static bool checkOpenMPIterationSpace(
 ? ((NestedLoopCount == 1) ? OMPC_linear : OMPC_lastprivate)
 : OMPC_private;
 if (((isOpenMPSimdDirective(DKind) && DVar.CKind != OMPC_unknown &&
-  DVar.CKind != PredeterminedCKind && DVar.RefExpr) ||
+  DVar.CKind != PredeterminedCKind && DVar.RefExpr &&
+  (SemaRef.getLangOpts().OpenMP <= 45 ||
+   (DVar.CKind != OMPC_lastprivate && DVar.CKind != OMPC_private))) ||
  ((isOpenMPWorksharingDirective(DKind) || DKind == OMPD_taskloop ||
isOpenMPDistributeDirective(DKind)) &&
   !isOpenMPSimdDirective(DKind) && DVar.CKind != OMPC_unknown &&

Modified: cfe/trunk/test/OpenMP/simd_loop_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/simd_loop_messages.cpp?rev=364650&r1=364649&r2=364650&view=diff
==
--- cfe/trunk/test/OpenMP/simd_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/simd_loop_messages.cpp Fri Jun 28 08:16:37 2019
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -fopenmp -x c++ -std=c++11 -fexceptions 
-fcxx-exceptions -verify %s
 // RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -x c++ -std=c++11 -fexceptions 
-fcxx-exceptions -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fopenmp -x c++ -std=c++11 -fexceptions 
-fcxx-exceptions -verify %s -fopenmp-version=50 -DOMP50
+// RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -x c++ -std=c++11 -fexceptions 
-fcxx-exceptions -verify %s -fopenmp-version=50 -DOMP50
 
 static int sii;
 // expected-note@+1 {{defined as threadprivate or thread local}}
@@ -239,12 +241,22 @@ int test_iteration_spaces() {
   for (ii = 0; (ii < 10); (ii-=0))
 c[ii] = a[ii];
 
-  // expected-note@+2  {{defined as private}}
-  // expected-error@+2 {{loop iteration variable in the associated loop of 
'omp simd' directive may not be private, predetermined as linear}}
+#ifndef OMP50
+  // expected-note@+3  {{defined as private}}
+  // expected-error@+3 {{loop iteration variable in the associated loop of 
'omp simd' directive may not be private, predetermined as linear}}
+#endif // OMP50
   #pragma omp simd private(ii)
   for (ii = 0; ii < 10; ii++)
 c[ii] = a[ii];
 
+#ifndef OMP50
+  // expected-note@+3  {{defined as lastprivate}}
+  // expected-error@+3 {{loop iteration variable in the associated loop of 
'omp simd' directive may not be lastprivate, predetermined as linear}}
+#endif // OMP50
+  #pragma omp simd lastprivate(ii)
+  for (ii = 0; ii < 10; ii++)
+c[ii] = a[ii];
+
   // expected-error@+1 {{unexpected OpenMP clause 'shared' in directive 
'#pragma omp simd'}}
   #pragma omp simd shared(ii)
   for (ii = 0; ii < 10; ii++)


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

[PATCH] D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter

[Gabor Marton via Phabricator via cfe-commits]

martong marked 2 inline comments as done.
martong added a comment.

In D62131#1517634 , @a_sidorin wrote:

> Hi Gabor,
>  Could you provide an example of an import sequence leading to this behavior? 
> It's hard for me to imagine such a situation.


Alexei, thanks for the review again. I don't think I could create an import 
sequence for this. However, I have experienced this kind of poisonous cache 
behavior during the debugging of a mysterious false positive structural 
inequivalency (during the analysis of protobuf).
The following happened: During the analysis we compared two Decls which turned 
out to be inequivalent, so we cached them. Later during the analysis, however, 
we added a new node to the redecl chain of one of these Decls which we 
previously compared. Then another structural equivalent check followed for the 
two Decls. And this time they should have been considered structurally 
equivalent, but the cache already contained them as nonequivalent. This 
resulted in a false positive NameConflict error.

By this change the error had gone, and we did not recognize any analysis 
slowdown. Remember, we still have a cache, but not a global cache in the 
ASTImporter object.




Comment at: clang/test/ASTMerge/struct/test.c:37
 // CHECK: struct2.c:53:37: note: field 'f' has type 'float' here
-// CHECK: struct1.c:54:8: warning: type 'struct DeepError' has incompatible 
definitions in different translation units
-// CHECK: struct1.c:56:41: note: field 'Deeper' has type 'struct DeeperError 
*' here

a_sidorin wrote:
> It looks strange to me that these warnings are gone.
For me, these warnings seems just like noise, by themselves line 37-40 does not 
show where is the exact mismatch.
The exact mismatch is shown in line 34-36, those warnings indicate that 
`DeeperError` has incompatible fields in the different TUs with different types 
(int vs float). That is the lowest level where the mismatch happens, I don't 
think we should indicate a warning for all other types which contain the 
mismatched types.

The same is true below (line 50-52) with the struct and union.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62131



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

[PATCH] D62127: [Hexagon] driver uses out-of-date option name and binary name

[Krzysztof Parzyszek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364648: [Hexagon] driver uses out-of-date option name and 
binary name (authored by kparzysz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62127?vs=200221&id=207067#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62127

Files:
  cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
@@ -130,11 +130,11 @@
   const Driver &D = HTC.getDriver();
   ArgStringList CmdArgs;
 
-  CmdArgs.push_back("-march=hexagon");
+  CmdArgs.push_back("--arch=hexagon");
 
   RenderExtraToolArgs(JA, CmdArgs);
 
-  const char *AsName = "hexagon-llvm-mc";
+  const char *AsName = "llvm-mc";
   CmdArgs.push_back("-filetype=obj");
   CmdArgs.push_back(Args.MakeArgString(
   "-mcpu=hexagon" +


Index: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
@@ -130,11 +130,11 @@
   const Driver &D = HTC.getDriver();
   ArgStringList CmdArgs;
 
-  CmdArgs.push_back("-march=hexagon");
+  CmdArgs.push_back("--arch=hexagon");
 
   RenderExtraToolArgs(JA, CmdArgs);
 
-  const char *AsName = "hexagon-llvm-mc";
+  const char *AsName = "llvm-mc";
   CmdArgs.push_back("-filetype=obj");
   CmdArgs.push_back(Args.MakeArgString(
   "-mcpu=hexagon" +
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r364648 - [Hexagon] driver uses out-of-date option name and binary name

[Krzysztof Parzyszek via cfe-commits]

Author: kparzysz
Date: Fri Jun 28 08:08:03 2019
New Revision: 364648

URL: http://llvm.org/viewvc/llvm-project?rev=364648&view=rev
Log:
[Hexagon] driver uses out-of-date option name and binary name

Patch by A. Skrobov (t.yomitch).

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=364648&r1=364647&r2=364648&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Fri Jun 28 08:08:03 2019
@@ -130,11 +130,11 @@ void hexagon::Assembler::ConstructJob(Co
   const Driver &D = HTC.getDriver();
   ArgStringList CmdArgs;
 
-  CmdArgs.push_back("-march=hexagon");
+  CmdArgs.push_back("--arch=hexagon");
 
   RenderExtraToolArgs(JA, CmdArgs);
 
-  const char *AsName = "hexagon-llvm-mc";
+  const char *AsName = "llvm-mc";
   CmdArgs.push_back("-filetype=obj");
   CmdArgs.push_back(Args.MakeArgString(
   "-mcpu=hexagon" +


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

r364647 - [OPENMP]Fix checks for DSA in simd constructs.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Jun 28 07:59:25 2019
New Revision: 364647

URL: http://llvm.org/viewvc/llvm-project?rev=364647&view=rev
Log:
[OPENMP]Fix checks for DSA in simd constructs.

The errors for incorrectly specified data-sharing attributes for simd
constructs must be emitted only for the explicitly provided clauses, not
the predetermined ones.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/simd_loop_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=364647&r1=364646&r2=364647&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Jun 28 07:59:25 2019
@@ -5697,12 +5697,12 @@ static bool checkOpenMPIterationSpace(
 ? ((NestedLoopCount == 1) ? OMPC_linear : OMPC_lastprivate)
 : OMPC_private;
 if (((isOpenMPSimdDirective(DKind) && DVar.CKind != OMPC_unknown &&
-  DVar.CKind != PredeterminedCKind) ||
+  DVar.CKind != PredeterminedCKind && DVar.RefExpr) ||
  ((isOpenMPWorksharingDirective(DKind) || DKind == OMPD_taskloop ||
isOpenMPDistributeDirective(DKind)) &&
   !isOpenMPSimdDirective(DKind) && DVar.CKind != OMPC_unknown &&
   DVar.CKind != OMPC_private && DVar.CKind != OMPC_lastprivate)) &&
-(DVar.CKind != OMPC_private || DVar.RefExpr != nullptr)) {
+(DVar.CKind != OMPC_private || DVar.RefExpr)) {
   SemaRef.Diag(Init->getBeginLoc(), diag::err_omp_loop_var_dsa)
   << getOpenMPClauseName(DVar.CKind) << getOpenMPDirectiveName(DKind)
   << getOpenMPClauseName(PredeterminedCKind);

Modified: cfe/trunk/test/OpenMP/simd_loop_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/simd_loop_messages.cpp?rev=364647&r1=364646&r2=364647&view=diff
==
--- cfe/trunk/test/OpenMP/simd_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/simd_loop_messages.cpp Fri Jun 28 07:59:25 2019
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -fopenmp -x c++ -std=c++11 -fexceptions 
-fcxx-exceptions -verify %s
-
 // RUN: %clang_cc1 -fsyntax-only -fopenmp-simd -x c++ -std=c++11 -fexceptions 
-fcxx-exceptions -verify %s
 
 static int sii;
@@ -7,12 +6,22 @@ static int sii;
 #pragma omp threadprivate(sii)
 static int globalii;
 
+struct S {
+  static int ssi;
+};
+
 int test_iteration_spaces() {
   const int N = 100;
   float a[N], b[N], c[N];
   int ii, jj, kk;
   float fii;
   double dii;
+#pragma omp simd linear(S::ssi)
+  for (S::ssi = 0; S::ssi < 10; ++S::ssi)
+;
+#pragma omp simd // no messages expected
+  for (S::ssi = 0; S::ssi < 10; ++S::ssi)
+;
   #pragma omp simd
   for (int i = 0; i < 10; i+=1) {
 c[i] = a[i] + b[i];


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