[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/AST/ExprCXX.cpp:979
 
+SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const {
+  TemplateParameterList *List = getTemplateParameterList();

hamzasood wrote:
> faisalv wrote:
> > I think this should return an invalid range if getExplicitCount is 0.
> > might assert that when not 0, langleloc does not equal rangleloc.
> > 
> I think it does return an invalid range in that case. Without explicit 
> template parameters, getLAngleLoc and getRAngleLoc return invalid locations. 
> And a range of invalid locations is an invalid range. But I suppose that 
> could be asserted to make sure.
fyi - Since getGenericLambdaTPL sets the LAngleLoc and RAngleLoc to the 
introducer range - unless one handles that case spearately- those funcs won't 
return an invalid loc?  


https://reviews.llvm.org/D36527



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

Re: r311823 - Add flag to request Clang is ABI-compatible with older versions of itself

[Richard Smith via cfe-commits]

Hi Hans,

We should get this into Clang 5 so that people can opt out of the ABI
bugfix for passing C++ class types by value.

On 25 August 2017 at 18:04, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Fri Aug 25 18:04:35 2017
> New Revision: 311823
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311823=rev
> Log:
> Add flag to request Clang is ABI-compatible with older versions of itself
>
> This patch adds a flag -fclang-abi-compat that can be used to request that
> Clang attempts to be ABI-compatible with some older version of itself.
>
> This is provided on a best-effort basis; right now, this can be used to
> undo
> the ABI change in r310401, reverting Clang to its prior C++ ABI for
> pass/return
> by value of class types affected by that change, and to undo the ABI
> change in
> r262688, reverting Clang to using integer registers rather than SSE
> registers
> for passing <1 x long long> vectors. The intent is that we will maintain
> this
> backwards compatibility path as we make ABI-breaking fixes in future.
>
> The reversion to the old behavior for r310401 is also applied to the PS4
> target
> since that change is not part of its platform ABI (which is essentially to
> do
> whatever Clang 3.2 did).
>
> Added:
> cfe/trunk/test/CodeGenCXX/clang-abi-compat.cpp
> cfe/trunk/test/Frontend/clang-abi-compat.cpp
> Modified:
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> cfe/trunk/include/clang/Frontend/CodeGenOptions.h
> cfe/trunk/lib/CodeGen/ABIInfo.h
> cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
> cfe/trunk/lib/CodeGen/CodeGenTypes.h
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
> cfe/trunk/test/Driver/flags.c
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Driver/Options.td?rev=311823=311822=311823=diff
> 
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Fri Aug 25 18:04:35 2017
> @@ -711,6 +711,9 @@ def fbuiltin : Flag<["-"], "fbuiltin">,
>  def fbuiltin_module_map : Flag <["-"], "fbuiltin-module-map">,
> Group,
>Flags<[DriverOption]>, HelpText<"Load the clang builtins module map
> file.">;
>  def fcaret_diagnostics : Flag<["-"], "fcaret-diagnostics">,
> Group;
> +def fclang_abi_compat_EQ : Joined<["-"], "fclang-abi-compat=">,
> Group,
> +  Flags<[CC1Option]>, MetaVarName<"">, Values<".,
> latest">,
> +  HelpText<"Attempt to match the ABI of Clang ">;
>  def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group;
>  def fcolor_diagnostics : Flag<["-"], "fcolor-diagnostics">,
> Group,
>Flags<[CoreOption, CC1Option]>, HelpText<"Use colors in diagnostics">;
>
> Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Frontend/CodeGenOptions.def?rev=311823=311822=311823=diff
> 
> ==
> --- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
> +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Fri Aug 25
> 18:04:35 2017
> @@ -120,6 +120,10 @@ CODEGENOPT(NoZeroInitializedInBSS , 1, 0
>  ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind, 2, Legacy)
>  CODEGENOPT(OmitLeafFramePointer , 1, 0) ///< Set when
> -momit-leaf-frame-pointer is
>  ///< enabled.
> +
> +/// A version of Clang that we should attempt to be ABI-compatible with.
> +ENUM_CODEGENOPT(ClangABICompat, ClangABI, 4, ClangABI::Latest)
> +
>  VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option
> specified.
>  VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is
> specified.
>
>
> Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Frontend/CodeGenOptions.h?rev=311823=311822=311823=diff
> 
> ==
> --- cfe/trunk/include/clang/Frontend/CodeGenOptions.h (original)
> +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h Fri Aug 25 18:04:35
> 2017
> @@ -69,6 +69,23 @@ public:
>  LocalExecTLSModel
>};
>
> +  /// Clang versions with different platform ABI conformance.
> +  enum class ClangABI {
> +/// Attempt to be ABI-compatible with code generated by Clang 3.8.x
> +/// (SVN r257626). This causes <1 x long long> to be passed in an
> +/// integer register instead of an SSE register on x64_64.
> +Ver3_8,
> +
> +/// Attempt to be ABI-compatible with code generated by Clang 4.0.x
> +/// (SVN r291814). 

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

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

rsmith closed this revision.
rsmith added a comment.

Committed as 311823.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

r311823 - Add flag to request Clang is ABI-compatible with older versions of itself

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Aug 25 18:04:35 2017
New Revision: 311823

URL: http://llvm.org/viewvc/llvm-project?rev=311823=rev
Log:
Add flag to request Clang is ABI-compatible with older versions of itself

This patch adds a flag -fclang-abi-compat that can be used to request that
Clang attempts to be ABI-compatible with some older version of itself.

This is provided on a best-effort basis; right now, this can be used to undo
the ABI change in r310401, reverting Clang to its prior C++ ABI for pass/return
by value of class types affected by that change, and to undo the ABI change in
r262688, reverting Clang to using integer registers rather than SSE registers
for passing <1 x long long> vectors. The intent is that we will maintain this
backwards compatibility path as we make ABI-breaking fixes in future.

The reversion to the old behavior for r310401 is also applied to the PS4 target
since that change is not part of its platform ABI (which is essentially to do
whatever Clang 3.2 did).

Added:
cfe/trunk/test/CodeGenCXX/clang-abi-compat.cpp
cfe/trunk/test/Frontend/clang-abi-compat.cpp
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/ABIInfo.h
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
cfe/trunk/test/Driver/flags.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=311823=311822=311823=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Aug 25 18:04:35 2017
@@ -711,6 +711,9 @@ def fbuiltin : Flag<["-"], "fbuiltin">,
 def fbuiltin_module_map : Flag <["-"], "fbuiltin-module-map">, Group,
   Flags<[DriverOption]>, HelpText<"Load the clang builtins module map file.">;
 def fcaret_diagnostics : Flag<["-"], "fcaret-diagnostics">, Group;
+def fclang_abi_compat_EQ : Joined<["-"], "fclang-abi-compat=">, 
Group,
+  Flags<[CC1Option]>, MetaVarName<"">, 
Values<".,latest">,
+  HelpText<"Attempt to match the ABI of Clang ">;
 def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group;
 def fcolor_diagnostics : Flag<["-"], "fcolor-diagnostics">, Group,
   Flags<[CoreOption, CC1Option]>, HelpText<"Use colors in diagnostics">;

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=311823=311822=311823=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Fri Aug 25 18:04:35 2017
@@ -120,6 +120,10 @@ CODEGENOPT(NoZeroInitializedInBSS , 1, 0
 ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind, 2, Legacy)
 CODEGENOPT(OmitLeafFramePointer , 1, 0) ///< Set when 
-momit-leaf-frame-pointer is
 ///< enabled.
+
+/// A version of Clang that we should attempt to be ABI-compatible with.
+ENUM_CODEGENOPT(ClangABICompat, ClangABI, 4, ClangABI::Latest)
+
 VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
 VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is 
specified.
 

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.h?rev=311823=311822=311823=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h Fri Aug 25 18:04:35 2017
@@ -69,6 +69,23 @@ public:
 LocalExecTLSModel
   };
 
+  /// Clang versions with different platform ABI conformance.
+  enum class ClangABI {
+/// Attempt to be ABI-compatible with code generated by Clang 3.8.x
+/// (SVN r257626). This causes <1 x long long> to be passed in an
+/// integer register instead of an SSE register on x64_64.
+Ver3_8,
+
+/// Attempt to be ABI-compatible with code generated by Clang 4.0.x
+/// (SVN r291814). This causes move operations to be ignored when
+/// determining whether a class type can be passed or returned directly.
+Ver4,
+
+/// Conform to the underlying platform's C and C++ ABIs as closely
+/// as we can.
+Latest
+  };
+
   enum StructReturnConventionKind {
 SRCK_Default,  // No special option was passed.
 SRCK_OnStack,  // Small structs on the stack (-fpcc-struct-return).

Modified: 

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Sorry it took so long to wrap my head around this, but it has been about 20 
years since I've had to accommodate an intentional ABI breakage, and that's 
actually what you want to do.  Remembering that case, I have a model for this 
one, and I understand what you are doing now.

Be that as it may, `-fclang-abi-compat` is meaningless on PS4, because every 
time there's a case for checking ClangABICompat, PS4 will want to do it the 3.2 
way regardless.  And we'll just make those checks in all those places.  At 
least it will be relatively easy to audit, and I'm sure our licensees will be 
happy to ignore the new flag.

So, I am convinced, and the patch LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:3861
< Align.getQuantity()) ||
 (ArgInfo.getIndirectByVal() && (RVAddrSpace != ArgAddrSpace))) {
   // Create an aligned temporary, and copy to it.

rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > This should be comparing AST address spaces.
> > The AST address space of RV cannot be obtained through 
> > `CGFunctionInfo::const_arg_iterator it` and `it->type` since `it->type` 
> > takes type of 
> > 
> > 
> > ```
> > ImplicitCastExpr 0x60a9ff0  'struct S':'struct S' 
> > `-DeclRefExpr 0x60a9f28  '__global struct S':'__global struct S' 
> > lvalue Var 0x607efb0
> > ```
> > 
> > and the original addr space is lost due to LValueToRValue cast.
> > 
> > To get the AST addr space of RV, it seems I need to save the argument Expr 
> > in CallArgList and get it from Expr.
> > 
> I think your last two comments are related.  I'm not sure why we haven't 
> copied into a temporary here, and if we had, the assumption of 
> LangAS::Default would be fine.  Would you mind doing the investigation there?
It seems the backend will insert a temp copy for byval arguments, therefore 
normally a byval argument does not need caller to create a temp copy in LLVM 
IR. An explicit temp copy is only needed for special cases, e.g. alignment 
mismatch with ABI.

For example, the following C program,


```
struct S {
  long x[100];
};

struct S g_s;

void f(struct S s);

void g() {
  f(g_s);
}

```

will generate the following IR on x86_64:


```
target triple = "x86_64-unknown-linux-gnu"

%struct.S = type { [100 x i64] }

@g_s = common global %struct.S zeroinitializer, align 8

; Function Attrs: noinline nounwind optnone
define void @g() #0 {
entry:
  call void @f(%struct.S* byval align 8 @g_s)
  ret void
}

declare void @f(%struct.S* byval align 8) #1

```

However, the following C program


```
struct S {
  int x[100];
};

struct S g_s;

void f(struct S s);

void g() {
  f(g_s);
}

```

will generate the following IR


```
target triple = "x86_64-unknown-linux-gnu"

%struct.S = type { [100 x i32] }

@g_s = common global %struct.S zeroinitializer, align 4

; Function Attrs: noinline nounwind optnone
define void @g() #0 {
entry:
  %byval-temp = alloca %struct.S, align 8
  %0 = bitcast %struct.S* %byval-temp to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %0, i8* bitcast (%struct.S* @g_s to 
i8*), i64 400, i32 4, i1 false)
  call void @f(%struct.S* byval align 8 %byval-temp)
  ret void
}

declare void @f(%struct.S* byval align 8) #1

```

The temp var is generated by line 3863. The control flow reaches line 3863 
because the alignment of the argument is 4 but the ABI requires it to be 8, so 
a temp is created to match the ABI align requirement.

That means, in the OpenCL example, it is normal that a temp var is not 
generated at line 3848. The temp is supposed to be generated at line 3863 too, 
like the C example.


https://reviews.llvm.org/D34367



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

[PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

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

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

Only typographical nits and post-commit suggestions, so please go ahead. Thanks!




Comment at: include/clang/Serialization/ModuleManager.h:63
+  /// \brief Preprocessor's HeaderSearchInfo containing the module map.
+  const HeaderSearch& HeaderSearchInfo;
+

` &`, not `& `, please.



Comment at: lib/Frontend/CompilerInvocation.cpp:986
+if (Val.find('=') == StringRef::npos)
+  Opts.ExtraDeps.push_back(Val);
+  }

Does a module file specified via `-fmodule-file=foo=foo.pcm` get included in 
the dep file if it is used? (If not, I'm happy for that to be fixed in a 
separate change, but it does need to be fixed for depfile-based build systems.)



Comment at: lib/Frontend/CompilerInvocation.cpp:982
+StringRef Val = A->getValue();
+if (Val.find('=') == StringRef::npos)
+  Opts.ExtraDeps.push_back(Val);

boris wrote:
> rsmith wrote:
> > There should be some way to specify a module file that contains a `=` in 
> > its file name. Ideally, that way would be compatible with our 
> > currently-supported form of `-fmodule-name=filename`. I don't see a way to 
> > support that other than `stat`ing every value we're given and checking to 
> > see whether it exists before attempting to split on a `=`... thoughts?
> > 
> > One somewhat hackish alternative would be to only recognize an `=` if there 
> > is no preceding `/`. (So `-fmodule-file=foo=bar.pcm` specifies a mapping, 
> > and `-fmodule-file=./foo=bar.pcm` specifies the file `./foo=bar.pcm`.)
> > 
> > (FWIW, we also support module names containing `=` characters.)
> A couple of thoughts:
> 
> 1. Using stat() is also not without issues: I may have an unrelated file with 
> a name that matches the whole value -- this will be fun to debug.
> 
> 2. GCC seems to have given up on trying to support paths with '=' since 
> AFAIK, none of their key=value options (e.g., -fdebug-prefix-map) support any 
> kind of escaping. I think the implicit assumption is that if you are using 
> paths with '=' in them, then you've asked for it.
> 
> 3. The '/' idea will work but will get a bit hairier if we also want to 
> support Windows (will need to check for both slashes).
> 
> 4. Another option is to reserve empty module name as a way to escape '=': 
> -fmodule-file==foo=bar.pcm. Not backwards compatible (but neither is ./) and 
> looks a bit weird but is the simplest to implement.
> 
> My preference order is (2), (4), (3), (1). Let me know which way you want to 
> go.
> 
> 
We've had a while to think about this, and haven't found a completely 
satisfactory answer, so let's go with (2) for now, without prejudice to future 
alternatives. It's fully GCC-compatible, and happens to be what this patch 
already does :)



Comment at: lib/Serialization/ASTReader.cpp:10180
+  ModuleMgr(PP.getFileManager(), PP.getPCMCache(), PCHContainerRdr,
+PP.getHeaderSearchInfo ()),
   PCMCache(PP.getPCMCache()), DummyIdResolver(PP),

No space before `()`.



Comment at: lib/Serialization/ASTReader.cpp:4145-4146
+// by-name lookup.
+if (Type == MK_PrebuiltModule || Type == MK_ExplicitModule)
+  ModuleMgr.registerPrebuilt(F);
 break;

boris wrote:
> rsmith wrote:
> > I'm worried by this: the `Type` can be different for different imports of 
> > the same .pcm file, and you'll only get here for the *first* import edge. 
> > So you can fail to add the module to the "prebuilt modules" map here, and 
> > then later attempt to look it up there (when processing the module offset 
> > map) and fail to find it.
> > 
> > Instead of keeping a separate lookup structure on the module manager, could 
> > you look up the `Module`, and then use its `ASTFile` to find the 
> > corresponding `ModuleFile`?
> > 
> > (If you go that way, the changes to module lookup when reading the module 
> > offset map could be generalized to apply to all `MK_*Module` types.)
> Yes, I was not entirely happy with this register prebuilt business so thanks 
> for the hint. I've implemented this and all tests pass (both Clang's and my 
> own). Though I cannot say I fully understand all the possible scenarios (like 
> when can ASTFile be NULL).
> 
> 
> 
> > (If you go that way, the changes to module lookup when reading the module 
> > offset map could be generalized to apply to all MK_*Module types.)
> 
> Are you suggesting that we switch to storing module names instead of module 
> files for all the module types in the offset map? If so, I think I've tried 
> that at some point but it didn't go well (existing tests were failing if I 
> remember correctly). I can try again if you think this should work. 
> 
> 
Yes, that's what I was suggesting, but don't block this commit on it. 
Ultimately, we should probably 

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/CodeGen/CGVTables.cpp:157
+  if (DebugInfo)
+DebugInfo->replaceTemporaryNodes();
+

aprantl wrote:
> Have you looked at what it would take to only finalize the nodes reachable 
> via the function?
> Otherwise — have you audited that this is always safe?
I do not know how to find nodes reachable from a particular function, either 
before or after they are finalized.

GenerateVarArgsThunk is called after we do EmitGlobalFunctionDefinition on the 
function we are about to clone.  The ReplaceMap holds replaceable forward type 
declarations, I think?  So I can imagine that codegen for a subsequent function 
might need to create a complete type, even one that is reachable from the 
function we are about to clone.

Of course I find the whole metadata memory management scheme pretty opaque, but 
this is my best guess about the situation.


https://reviews.llvm.org/D37038



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

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

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

rsmith added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:2934
+ABICompatArg->render(Args, CmdArgs);
+
   // Add runtime flag for PS4 when PGO or Coverage are enabled.

probinson wrote:
> rsmith wrote:
> > probinson wrote:
> > > ```
> > > else if (getToolChain().getTriple().isPS4())
> > >   CmdArgs.push_back("-fclang-abi-compat=3.2");
> > > ```
> > > 
> > > Which lets us avoid piles of PS4 special cases all over everywhere.
> > > Sony is historically very conservative about compatibility, and we'll be 
> > > happier defaulting it this way.  Setting platform-specific defaults in 
> > > the driver seems to be pretty common already, this is just one more.
> > I initially thought that made sense too, but I'm now fairly convinced that 
> > it doesn't.
> > 
> > 
> > Let's take Darwin as an example. There are some facets of Darwin's platform 
> > ABI that are determined by what old versions of Clang did, and other facets 
> > of its platform ABI that have changed to match changes to the x86_64 psABI 
> > and Itanium C++ ABI. But it's irrelevant where the platform ABI rules come 
> > from; the point is that there is some set of platform ABI rules that you 
> > could write down, and Clang attempts to implement those rules correctly by 
> > default.
> > 
> > The flag being added in this patch provides the ability to request that 
> > Clang does something else: that it produces code that is ABI-compatible 
> > with what a prior version of itself did when targeting that platform ABI. 
> > In particular, we fixed the C++ calling convention for certain rare class 
> > types in Clang 5 to conform to (an update to) the Itanium C++ ABI rules, 
> > and this patch allows that to be undone.
> > 
> > It seems to me that the situation for PS4 is exactly the same. There is 
> > some platform ABI that you could write down, and Clang attempts to be 
> > compatible with that by default. And it's irrelevant whether that's the ABI 
> > that Clang 3.2 used or the ABI of GCC 2.95; it's just the platform ABI. 
> > This is not a "be compatible with clang 3.2" mode, it is (as far as I can 
> > tell) the actual platform ABI.
> > 
> > 
> > Let me repeat an example I gave before: suppose Clang 5 has some 
> > (accidental) ABI change in it for PS4, and we fix that in Clang 6 to once 
> > again follow the platform ABI by doing what Clang 3.2 did. In that case, 
> > building with `-fclang-abi-compat=5` should theoretically reinstate that 
> > accidental ABI change.
> > 
> > Hopefully that should clarify that this does *not* actually let us avoid 
> > PS4 special cases anywhere. ABI choices depend on both the platform ABI 
> > *and* on the version of Clang that we're providing compatibility with (if 
> > any).
> > 
> > 
> > That said, it's clearly not up to me what the PS4 platform ABI is. If you 
> > want to say that the PS4 platform ABI is actually something other than what 
> > Clang 3.2 does, but all object code on your system is compiled in a 
> > compatibility mode that diverges from the platform ABI and matches Clang 
> > 3.2, then I would concede that we can remove the PS4 platform special cases 
> > elsewhere and set a default here. But that sounds like a very strange 
> > decision to make, and it creates a horrible problem for the meaning of the 
> > `-fclang-abi-compat` flag: if someone in the future specifies 
> > `-fclang-abi-compat=5` when targeting PS4, and Clang 5 by default set 
> > `-fclang-abi-compat=3.2`, then are we targeting what Clang 5 would have 
> > done by default or what Clang 5 would have done when told to be compatible 
> > with itself? As you can see, this default would create a lot of confusion.
> On the other hand, if all the places that check ClangABICompat also check for 
> PS4 and Darwin, then specifying `-fclang-abi-compat` while targeting PS4 or 
> Darwin has no effect, and also no diagnostic.  Which seems to make 
> `-fclang-abi-compat` totally pointless.  Is there a non-PS4/Darwin use-case 
> for this flag?
`-fclang-abi-compat` requests that we generate code that is ABI-compatible with 
a prior Clang version. The use case (for *any* platform) is that you have some 
code built with an earlier version of Clang that had a bug in its 
implementation of the platform ABI, and you want to generate more code that is 
ABI-compatible with it.

`-fclang-abi-compat=4`, for instance, has an effect on Darwin: it turns off the 
recent bug fix to the calling convention for passing 
trivially-copyable-but-not-trivially-moveable C++ class types. (But it does 
nothing on PS4, because -- with this patch -- Clang 5 does not change that 
aspect of the ABI for PS4.)


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
   if (symLHS && symRHS &&
-  (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
+  (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
 return makeNonLoc(symLHS, Op, symRHS, ResultTy);

zaks.anna wrote:
> As a follow up to the previous version of this patch, I do not think we 
> should set the default complexity limit to 1.
> 
> What is the relation between this limit and the limit in 
> VisitNonLocSymbolVal? If they are related, would it be worthwhile turning 
> these into an analyzer option?
To clarify, the current version of this patch does not modify the `MaxComp` 
parameter.

My understanding is that `MaxComp` is the upper bound for building a new 
`NonLoc` symbol from two children, based on the sum of the number of child 
symbols (complexity) across both children.

In contrast, the limit in `VisitNonLocSymbolVal` (@NoQ, correct me if I'm 
wrong), is the upper bound for recursively evaluating and inlining a `NonLoc` 
symbol, triggered from `simplifySVal` by `evalBinOpNN`. Note that these two 
latter functions indirectly call each other recursively (through `evalBinOp`), 
causing the previous runtime blowup. Furthermore, each call to 
`computeComplexity`will re-iterate through all child symbols of the current 
symbol, but only the first complexity check at the root symbol is actually 
necessary, because by definition the complexity of a child symbol at each 
recursive call is monotonically decreasing.

I think it'd be useful to allow both to be configurable, but I don't see a 
direct relationship between the two.


https://reviews.llvm.org/D35450



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

[PATCH] D36802: AMDGPU: Cleanup most of the macros

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl updated this revision to Diff 112761.
kzhuravl marked an inline comment as done.
kzhuravl retitled this revision from "AMDGPU: Insert __devicename__ macros" to 
"AMDGPU: Cleanup most of the macros".
kzhuravl edited the summary of this revision.
kzhuravl added a comment.

fast fmaf macro will be in the follow up patch.


https://reviews.llvm.org/D36802

Files:
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  test/Driver/amdgpu-macros.cl

Index: test/Driver/amdgpu-macros.cl
===
--- /dev/null
+++ test/Driver/amdgpu-macros.cl
@@ -0,0 +1,182 @@
+// Check that appropriate macros are defined for every supported AMDGPU
+// "-target" and "-mcpu".
+
+//
+// R600.
+//
+
+// RUN: %clang -target r600 -mcpu=r600 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=R600-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv630 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=R600-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv635 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=R600-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv610 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RS880-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv620 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RS880-CHECK %s
+// RUN: %clang -target r600 -mcpu=rs780 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RS880-CHECK %s
+// RUN: %clang -target r600 -mcpu=rs880 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RS880-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv670 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RV670-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv710 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RV710-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv730 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RV730-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv740 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RV770-CHECK %s
+// RUN: %clang -target r600 -mcpu=rv770 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=RV770-CHECK %s
+// RUN: %clang -target r600 -mcpu=palm -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CEDAR-CHECK %s
+// RUN: %clang -target r600 -mcpu=cedar -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CEDAR-CHECK %s
+// RUN: %clang -target r600 -mcpu=sumo -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=SUMO-CHECK %s
+// RUN: %clang -target r600 -mcpu=sumo2 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=SUMO-CHECK %s
+// RUN: %clang -target r600 -mcpu=redwood -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=REDWOOD-CHECK %s
+// RUN: %clang -target r600 -mcpu=juniper -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=JUNIPER-CHECK %s
+// RUN: %clang -target r600 -mcpu=juniper -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=JUNIPER-CHECK %s
+// RUN: %clang -target r600 -mcpu=hemlock -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CYPRESS-CHECK %s
+// RUN: %clang -target r600 -mcpu=cypress -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CYPRESS-CHECK %s
+// RUN: %clang -target r600 -mcpu=barts -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=BARTS-CHECK %s
+// RUN: %clang -target r600 -mcpu=turks -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=TURKS-CHECK %s
+// RUN: %clang -target r600 -mcpu=caicos -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CAICOS-CHECK %s
+// RUN: %clang -target r600 -mcpu=cayman -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CAYMAN-CHECK %s
+// RUN: %clang -target r600 -mcpu=aruba -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-R600-CHECK --check-prefix=CAYMAN-CHECK %s
+
+//
+// GCN.
+//
+
+// RUN: %clang -target amdgcn -mcpu=gfx600 -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK --check-prefix=ARCH-GCN-CHECK --check-prefix=GFX600-CHECK %s
+// RUN: %clang -target amdgcn -mcpu=tahiti -E -dM %s | FileCheck --check-prefix=AMDGPU-CHECK 

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:2934
+ABICompatArg->render(Args, CmdArgs);
+
   // Add runtime flag for PS4 when PGO or Coverage are enabled.

rsmith wrote:
> probinson wrote:
> > ```
> > else if (getToolChain().getTriple().isPS4())
> >   CmdArgs.push_back("-fclang-abi-compat=3.2");
> > ```
> > 
> > Which lets us avoid piles of PS4 special cases all over everywhere.
> > Sony is historically very conservative about compatibility, and we'll be 
> > happier defaulting it this way.  Setting platform-specific defaults in the 
> > driver seems to be pretty common already, this is just one more.
> I initially thought that made sense too, but I'm now fairly convinced that it 
> doesn't.
> 
> 
> Let's take Darwin as an example. There are some facets of Darwin's platform 
> ABI that are determined by what old versions of Clang did, and other facets 
> of its platform ABI that have changed to match changes to the x86_64 psABI 
> and Itanium C++ ABI. But it's irrelevant where the platform ABI rules come 
> from; the point is that there is some set of platform ABI rules that you 
> could write down, and Clang attempts to implement those rules correctly by 
> default.
> 
> The flag being added in this patch provides the ability to request that Clang 
> does something else: that it produces code that is ABI-compatible with what a 
> prior version of itself did when targeting that platform ABI. In particular, 
> we fixed the C++ calling convention for certain rare class types in Clang 5 
> to conform to (an update to) the Itanium C++ ABI rules, and this patch allows 
> that to be undone.
> 
> It seems to me that the situation for PS4 is exactly the same. There is some 
> platform ABI that you could write down, and Clang attempts to be compatible 
> with that by default. And it's irrelevant whether that's the ABI that Clang 
> 3.2 used or the ABI of GCC 2.95; it's just the platform ABI. This is not a 
> "be compatible with clang 3.2" mode, it is (as far as I can tell) the actual 
> platform ABI.
> 
> 
> Let me repeat an example I gave before: suppose Clang 5 has some (accidental) 
> ABI change in it for PS4, and we fix that in Clang 6 to once again follow the 
> platform ABI by doing what Clang 3.2 did. In that case, building with 
> `-fclang-abi-compat=5` should theoretically reinstate that accidental ABI 
> change.
> 
> Hopefully that should clarify that this does *not* actually let us avoid PS4 
> special cases anywhere. ABI choices depend on both the platform ABI *and* on 
> the version of Clang that we're providing compatibility with (if any).
> 
> 
> That said, it's clearly not up to me what the PS4 platform ABI is. If you 
> want to say that the PS4 platform ABI is actually something other than what 
> Clang 3.2 does, but all object code on your system is compiled in a 
> compatibility mode that diverges from the platform ABI and matches Clang 3.2, 
> then I would concede that we can remove the PS4 platform special cases 
> elsewhere and set a default here. But that sounds like a very strange 
> decision to make, and it creates a horrible problem for the meaning of the 
> `-fclang-abi-compat` flag: if someone in the future specifies 
> `-fclang-abi-compat=5` when targeting PS4, and Clang 5 by default set 
> `-fclang-abi-compat=3.2`, then are we targeting what Clang 5 would have done 
> by default or what Clang 5 would have done when told to be compatible with 
> itself? As you can see, this default would create a lot of confusion.
On the other hand, if all the places that check ClangABICompat also check for 
PS4 and Darwin, then specifying `-fclang-abi-compat` while targeting PS4 or 
Darwin has no effect, and also no diagnostic.  Which seems to make 
`-fclang-abi-compat` totally pointless.  Is there a non-PS4/Darwin use-case for 
this flag?


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112759.
morehouse added a comment.

Full diff.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -78,6 +79,9 @@
 
 static const char *const SanCovLowestStackName = "__sancov_lowest_stack";
 
+__attribute__((tls_model("initial-exec")))
+thread_local uintptr_t __sancov_lowest_stack;
+
 static cl::opt ClCoverageLevel(
 "sanitizer-coverage-level",
 cl::desc("Sanitizer Coverage. 0: none, 1: entry block, 2: all blocks, "
@@ -200,13 +204,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function ,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function , ArrayRef AllBlocks);
+  bool InjectCoverage(Function , ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function , Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function , ArrayRef AllBlocks);
   void CreatePCArray(Function , ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function , BasicBlock , size_t Idx);
+  void InjectCoverageAtBlock(Function , BasicBlock , size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module , const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +497,7 @@
   (F).getDomTree();
   const PostDominatorTree *PDT =
   (F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto  : F) {
 if (shouldInstrumentBlock(F, , DT, PDT, Options))
@@ -515,10 +522,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast())
   

[libunwind] r311818 - Creating release candidate rc3 from release_500 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug 25 15:51:25 2017
New Revision: 311818

URL: http://llvm.org/viewvc/llvm-project?rev=311818=rev
Log:
Creating release candidate rc3 from release_500 branch

Added:
libunwind/tags/RELEASE_500/rc3/   (props changed)
  - copied from r311817, libunwind/branches/release_50/

Propchange: libunwind/tags/RELEASE_500/rc3/
--
svn:mergeinfo = /libunwind/trunk:308871,309147


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

[libcxx] r311811 - Creating release candidate rc3 from release_500 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug 25 15:51:17 2017
New Revision: 311811

URL: http://llvm.org/viewvc/llvm-project?rev=311811=rev
Log:
Creating release candidate rc3 from release_500 branch

Added:
libcxx/tags/RELEASE_500/rc3/   (props changed)
  - copied from r311810, libcxx/branches/release_50/

Propchange: libcxx/tags/RELEASE_500/rc3/
--
--- svn:mergeinfo (added)
+++ svn:mergeinfo Fri Aug 25 15:51:17 2017
@@ -0,0 +1,2 @@
+/libcxx/branches/apple:136569-137939
+/libcxx/trunk:309296,309307,309474,309838,309851,309917,309920


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

[libcxxabi] r311812 - Creating release candidate rc3 from release_500 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug 25 15:51:17 2017
New Revision: 311812

URL: http://llvm.org/viewvc/llvm-project?rev=311812=rev
Log:
Creating release candidate rc3 from release_500 branch

Added:
libcxxabi/tags/RELEASE_500/rc3/
  - copied from r311811, libcxxabi/branches/release_50/

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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112756.
morehouse added a comment.

- Add weak reference in SanitizerCoverage.cpp


https://reviews.llvm.org/D37156

Files:
  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
@@ -78,6 +78,9 @@
 
 static const char *const SanCovLowestStackName = "__sancov_lowest_stack";
 
+__attribute__((tls_model("initial-exec")))
+thread_local uintptr_t __sancov_lowest_stack;
+
 static cl::opt ClCoverageLevel(
 "sanitizer-coverage-level",
 cl::desc("Sanitizer Coverage. 0: none, 1: entry block, 2: all blocks, "


Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -78,6 +78,9 @@
 
 static const char *const SanCovLowestStackName = "__sancov_lowest_stack";
 
+__attribute__((tls_model("initial-exec")))
+thread_local uintptr_t __sancov_lowest_stack;
+
 static cl::opt ClCoverageLevel(
 "sanitizer-coverage-level",
 cl::desc("Sanitizer Coverage. 0: none, 1: entry block, 2: all blocks, "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

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

Turns out I should have been testing the benchmarks with 
`FUZZING_ENGINE=fsanitize_fuzzer`.  My mistake.

After adding the weak reference to SanitizerCoverage.cpp, both lcms and proj4 
build with `fsanitize_fuzzer`.


Repository:
  rL LLVM

https://reviews.llvm.org/D37156



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

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Jason Newton via Phabricator via cfe-commits]

nevion added a comment.

Can anyone inform me why these don't work on clang-format-diff?


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

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

rsmith added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:2934
+ABICompatArg->render(Args, CmdArgs);
+
   // Add runtime flag for PS4 when PGO or Coverage are enabled.

probinson wrote:
> ```
> else if (getToolChain().getTriple().isPS4())
>   CmdArgs.push_back("-fclang-abi-compat=3.2");
> ```
> 
> Which lets us avoid piles of PS4 special cases all over everywhere.
> Sony is historically very conservative about compatibility, and we'll be 
> happier defaulting it this way.  Setting platform-specific defaults in the 
> driver seems to be pretty common already, this is just one more.
I initially thought that made sense too, but I'm now fairly convinced that it 
doesn't.


Let's take Darwin as an example. There are some facets of Darwin's platform ABI 
that are determined by what old versions of Clang did, and other facets of its 
platform ABI that have changed to match changes to the x86_64 psABI and Itanium 
C++ ABI. But it's irrelevant where the platform ABI rules come from; the point 
is that there is some set of platform ABI rules that you could write down, and 
Clang attempts to implement those rules correctly by default.

The flag being added in this patch provides the ability to request that Clang 
does something else: that it produces code that is ABI-compatible with what a 
prior version of itself did when targeting that platform ABI. In particular, we 
fixed the C++ calling convention for certain rare class types in Clang 5 to 
conform to (an update to) the Itanium C++ ABI rules, and this patch allows that 
to be undone.

It seems to me that the situation for PS4 is exactly the same. There is some 
platform ABI that you could write down, and Clang attempts to be compatible 
with that by default. And it's irrelevant whether that's the ABI that Clang 3.2 
used or the ABI of GCC 2.95; it's just the platform ABI. This is not a "be 
compatible with clang 3.2" mode, it is (as far as I can tell) the actual 
platform ABI.


Let me repeat an example I gave before: suppose Clang 5 has some (accidental) 
ABI change in it for PS4, and we fix that in Clang 6 to once again follow the 
platform ABI by doing what Clang 3.2 did. In that case, building with 
`-fclang-abi-compat=5` should theoretically reinstate that accidental ABI 
change.

Hopefully that should clarify that this does *not* actually let us avoid PS4 
special cases anywhere. ABI choices depend on both the platform ABI *and* on 
the version of Clang that we're providing compatibility with (if any).


That said, it's clearly not up to me what the PS4 platform ABI is. If you want 
to say that the PS4 platform ABI is actually something other than what Clang 
3.2 does, but all object code on your system is compiled in a compatibility 
mode that diverges from the platform ABI and matches Clang 3.2, then I would 
concede that we can remove the PS4 platform special cases elsewhere and set a 
default here. But that sounds like a very strange decision to make, and it 
creates a horrible problem for the meaning of the `-fclang-abi-compat` flag: if 
someone in the future specifies `-fclang-abi-compat=5` when targeting PS4, and 
Clang 5 by default set `-fclang-abi-compat=3.2`, then are we targeting what 
Clang 5 would have done by default or what Clang 5 would have done when told to 
be compatible with itself? As you can see, this default would create a lot of 
confusion.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

r311803 - Revert "[SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer"

[Matt Morehouse via cfe-commits]

Author: morehouse
Date: Fri Aug 25 15:01:21 2017
New Revision: 311803

URL: http://llvm.org/viewvc/llvm-project?rev=311803=rev
Log:
Revert "[SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer"

This reverts r311801 due to a bot failure.

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=311803=311802=311803=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Fri Aug 25 15:01:21 2017
@@ -290,11 +290,10 @@ SanitizerArgs::SanitizerArgs(const ToolC
   if (Add & Fuzzer)
 Add |= FuzzerNoLink;
 
-  // Enable coverage and stack depth tracking if the fuzzing flag is set.
+  // Enable coverage if the fuzzing flag is set.
   if (Add & FuzzerNoLink)
 CoverageFeatures |= CoverageTracePCGuard | CoverageIndirCall |
-CoverageTraceCmp | CoveragePCTable |
-CoverageStackDepth;
+CoverageTraceCmp | CoveragePCTable;
 
   Kinds |= Add;
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {


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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311801: [SanitizeCoverage] Enable stack-depth coverage for 
-fsanitize=fuzzer (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D37156?vs=112739=112746#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37156

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  compiler-rt/trunk/test/fuzzer/deep-recursion.test
  llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function ,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function , ArrayRef AllBlocks);
+  bool InjectCoverage(Function , ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function , Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function , ArrayRef AllBlocks);
   void CreatePCArray(Function , ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function , BasicBlock , size_t Idx);
+  void InjectCoverageAtBlock(Function , BasicBlock , size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module , const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   (F).getDomTree();
   const PostDominatorTree *PDT =
   (F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto  : F) {
 if (shouldInstrumentBlock(F, , DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast())
   GepTraceTargets.push_back(GEP);
-   }
+  if (Options.StackDepth)

r311801 - [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via cfe-commits]

Author: morehouse
Date: Fri Aug 25 14:18:29 2017
New Revision: 311801

URL: http://llvm.org/viewvc/llvm-project?rev=311801=rev
Log:
[SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

Summary:
- Don't sanitize __sancov_lowest_stack.
- Don't instrument leaf functions.
- Add CoverageStackDepth to Fuzzer and FuzzerNoLink.

Reviewers: vitalybuka, kcc

Reviewed By: kcc

Subscribers: cfe-commits, llvm-commits, hiraditya

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

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=311801=311800=311801=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Fri Aug 25 14:18:29 2017
@@ -290,10 +290,11 @@ SanitizerArgs::SanitizerArgs(const ToolC
   if (Add & Fuzzer)
 Add |= FuzzerNoLink;
 
-  // Enable coverage if the fuzzing flag is set.
+  // Enable coverage and stack depth tracking if the fuzzing flag is set.
   if (Add & FuzzerNoLink)
 CoverageFeatures |= CoverageTracePCGuard | CoverageIndirCall |
-CoverageTraceCmp | CoveragePCTable;
+CoverageTraceCmp | CoveragePCTable |
+CoverageStackDepth;
 
   Kinds |= Add;
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {


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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

OK - I'll commit this on sunday if no one blocks it by then.  (I'll add the 
fixme's).

Nice Work!


https://reviews.llvm.org/D36527



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

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:2934
+ABICompatArg->render(Args, CmdArgs);
+
   // Add runtime flag for PS4 when PGO or Coverage are enabled.

```
else if (getToolChain().getTriple().isPS4())
  CmdArgs.push_back("-fclang-abi-compat=3.2");
```

Which lets us avoid piles of PS4 special cases all over everywhere.
Sony is historically very conservative about compatibility, and we'll be 
happier defaulting it this way.  Setting platform-specific defaults in the 
driver seems to be pretty common already, this is just one more.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Kostya Serebryany via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D37156



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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112739.
morehouse added a comment.

- Use existing linear scan, and check for InvokeInst.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function ,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function , ArrayRef AllBlocks);
+  bool InjectCoverage(Function , ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function , Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function , ArrayRef AllBlocks);
   void CreatePCArray(Function , ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function , BasicBlock , size_t Idx);
+  void InjectCoverageAtBlock(Function , BasicBlock , size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module , const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   (F).getDomTree();
   const PostDominatorTree *PDT =
   (F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto  : F) {
 if (shouldInstrumentBlock(F, , DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast())
   GepTraceTargets.push_back(GEP);
-   }
+  if (Options.StackDepth)
+if (isa(Inst) ||
+(isa(Inst) && !isa(Inst)))
+  IsLeafFunc = false;
+}
   }
 
-  InjectCoverage(F, BlocksToInstrument);
+  InjectCoverage(F, BlocksToInstrument, IsLeafFunc);
   

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D37156#852780, @kcc wrote:

> Did you check this on something other than the unit tests? 
>  E.g. a couple of benchmarks from fuzzer-test-suite?


Just tested on the proj4 and lcms benchmarks and no issues came up.


https://reviews.llvm.org/D37156



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

Re: r311792 - [Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for

[Hans Wennborg via cfe-commits]

Merged to 5.0 in r311800.

On Fri, Aug 25, 2017 at 12:14 PM, Daniel Jasper via cfe-commits
 wrote:
> Author: djasper
> Date: Fri Aug 25 12:14:53 2017
> New Revision: 311792
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311792=rev
> Log:
> [Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for
> alignments
>
> Indent should be compared before nesting level to determine if a token
> is on the same scope as the one we align with. Because it was inverted,
> clang-format sometimes tried to align tokens with tokens from outer
> scopes, causing the assert(Shift >= 0) to fire.
>
> This fixes bug #33507. Patch by Beren Minor, thank you!
>
> Modified:
> cfe/trunk/lib/Format/WhitespaceManager.cpp
> cfe/trunk/lib/Format/WhitespaceManager.h
> cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=311792=311791=311792=diff
> ==
> --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
> +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Fri Aug 25 12:14:53 2017
> @@ -246,12 +246,12 @@ AlignTokenSequence(unsigned Start, unsig
>
>for (unsigned i = Start; i != End; ++i) {
>  if (ScopeStack.size() != 0 &&
> -Changes[i].nestingAndIndentLevel() <
> -Changes[ScopeStack.back()].nestingAndIndentLevel())
> +Changes[i].indentAndNestingLevel() <
> +Changes[ScopeStack.back()].indentAndNestingLevel())
>ScopeStack.pop_back();
>
> -if (i != Start && Changes[i].nestingAndIndentLevel() >
> -  Changes[i - 1].nestingAndIndentLevel())
> +if (i != Start && Changes[i].indentAndNestingLevel() >
> +  Changes[i - 1].indentAndNestingLevel())
>ScopeStack.push_back(i);
>
>  bool InsideNestedScope = ScopeStack.size() != 0;
> @@ -327,8 +327,8 @@ static unsigned AlignTokens(const Format
>
>// Measure the scope level (i.e. depth of (), [], {}) of the first token, 
> and
>// abort when we hit any token in a higher scope than the starting one.
> -  auto NestingAndIndentLevel = StartAt < Changes.size()
> -   ? Changes[StartAt].nestingAndIndentLevel()
> +  auto IndentAndNestingLevel = StartAt < Changes.size()
> +   ? Changes[StartAt].indentAndNestingLevel()
> : std::pair(0, 0);
>
>// Keep track of the number of commas before the matching tokens, we will 
> only
> @@ -359,7 +359,7 @@ static unsigned AlignTokens(const Format
>
>unsigned i = StartAt;
>for (unsigned e = Changes.size(); i != e; ++i) {
> -if (Changes[i].nestingAndIndentLevel() < NestingAndIndentLevel)
> +if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
>break;
>
>  if (Changes[i].NewlinesBefore != 0) {
> @@ -375,7 +375,7 @@ static unsigned AlignTokens(const Format
>
>  if (Changes[i].Tok->is(tok::comma)) {
>++CommasBeforeMatch;
> -} else if (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel) {
> +} else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
>// Call AlignTokens recursively, skipping over this scope block.
>unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i);
>i = StoppedAt - 1;
>
> Modified: cfe/trunk/lib/Format/WhitespaceManager.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=311792=311791=311792=diff
> ==
> --- cfe/trunk/lib/Format/WhitespaceManager.h (original)
> +++ cfe/trunk/lib/Format/WhitespaceManager.h Fri Aug 25 12:14:53 2017
> @@ -154,12 +154,11 @@ public:
>  const Change *StartOfBlockComment;
>  int IndentationOffset;
>
> -// A combination of nesting level and indent level, which are used in
> +// A combination of indent level and nesting level, which are used in
>  // tandem to compute lexical scope, for the purposes of deciding
>  // when to stop consecutive alignment runs.
> -std::pair
> -nestingAndIndentLevel() const {
> -  return std::make_pair(Tok->NestingLevel, Tok->IndentLevel);
> +std::pair indentAndNestingLevel() const {
> +  return std::make_pair(Tok->IndentLevel, Tok->NestingLevel);
>  }
>};
>
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=311792=311791=311792=diff
> ==
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Aug 25 12:14:53 2017
> @@ -8958,6 +8958,16 @@ 

Re: r311695 - [ubsan] PR34266: When sanitizing the 'this' value for a member function that happens to be a lambda call operator, use the lambda's 'this' pointer, not the captured enclosing 'this' poin

[Hans Wennborg via cfe-commits]

r311799. Thanks!

On Thu, Aug 24, 2017 at 7:14 PM, Richard Smith  wrote:
> Hi Hans,
>
> This fixes a regression in ubsan's handling of lambda expressions. Seems
> like a reasonable candidate for Clang 5, but it is rather late in the day...
>
>
> On 24 August 2017 at 13:10, Richard Smith via cfe-commits
>  wrote:
>>
>> Author: rsmith
>> Date: Thu Aug 24 13:10:33 2017
>> New Revision: 311695
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=311695=rev
>> Log:
>> [ubsan] PR34266: When sanitizing the 'this' value for a member function
>> that happens to be a lambda call operator, use the lambda's 'this' pointer,
>> not the captured enclosing 'this' pointer (if any).
>>
>> Do not sanitize the 'this' pointer of a member call operator for a lambda
>> with
>> no capture-default, since that call operator can legitimately be called
>> with a
>> null this pointer from the static invoker function. Any actual call with a
>> null
>> this pointer should still be caught in the caller (if it is being
>> sanitized).
>>
>> This reinstates r311589 (reverted in r311680) with the above fix.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclCXX.h
>> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>> cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=311695=311694=311695=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Aug 24 13:10:33 2017
>> @@ -2027,7 +2027,10 @@ public:
>>
>>/// \brief Returns the type of the \c this pointer.
>>///
>> -  /// Should only be called for instance (i.e., non-static) methods.
>> +  /// Should only be called for instance (i.e., non-static) methods. Note
>> +  /// that for the call operator of a lambda closure type, this returns
>> the
>> +  /// desugared 'this' type (a pointer to the closure type), not the
>> captured
>> +  /// 'this' type.
>>QualType getThisType(ASTContext ) const;
>>
>>unsigned getTypeQualifiers() const {
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=311695=311694=311695=diff
>>
>> ==
>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Aug 24 13:10:33 2017
>> @@ -22,6 +22,7 @@
>>  #include "CodeGenPGO.h"
>>  #include "TargetInfo.h"
>>  #include "clang/AST/ASTContext.h"
>> +#include "clang/AST/ASTLambda.h"
>>  #include "clang/AST/Decl.h"
>>  #include "clang/AST/DeclCXX.h"
>>  #include "clang/AST/StmtCXX.h"
>> @@ -1014,11 +1015,22 @@ void CodeGenFunction::StartFunction(Glob
>>  }
>>
>>  // Check the 'this' pointer once per function, if it's available.
>> -if (CXXThisValue) {
>> +if (CXXABIThisValue) {
>>SanitizerSet SkippedChecks;
>>SkippedChecks.set(SanitizerKind::ObjectSize, true);
>>QualType ThisTy = MD->getThisType(getContext());
>> -  EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
>> +
>> +  // If this is the call operator of a lambda with no
>> capture-default, it
>> +  // may have a static invoker function, which may call this operator
>> with
>> +  // a null 'this' pointer.
>> +  if (isLambdaCallOperator(MD) &&
>> +  cast(MD->getParent())->getLambdaCaptureDefault()
>> ==
>> +  LCD_None)
>> +SkippedChecks.set(SanitizerKind::Null, true);
>> +
>> +  EmitTypeCheck(isa(MD) ? TCK_ConstructorCall
>> +: TCK_MemberCall,
>> +Loc, CXXABIThisValue, ThisTy,
>>
>> getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
>>  SkippedChecks);
>>  }
>>
>> Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=311695=311694=311695=diff
>>
>> ==
>> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Thu Aug 24 13:10:33
>> 2017
>> @@ -449,6 +449,28 @@ void upcast_to_vbase() {
>>  }
>>  }
>>
>> +struct ThisAlign {
>> +  void this_align_lambda();
>> +  void this_align_lambda_2();
>> +};
>> +void ThisAlign::this_align_lambda() {
>> +  // CHECK-LABEL: define
>> {{.*}}@"_ZZN9ThisAlign17this_align_lambdaEvENK3$_0clEv"
>> +  // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]])
>> +  // CHECK: %[[this_addr:.*]] = alloca
>> +  // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]],
>> +  // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]],
>> +  // CHECK: 

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

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

rsmith added a comment.

In https://reviews.llvm.org/D36501#852864, @rjmccall wrote:

> The only thing I would say here is that, as a platform vendor, I would like 
> the ability to opt in to ABI fixes that aren't in my base-line version.  In 
> particular, we're generally more aggressive about taking C++ ABI fixes on 
> Darwin when there's a strong argument that the impact is likely to be low, 
> like for some of these indirect-ABI cases that only make a difference for 
> classes with e.g. only deleted copy/move constructors but a trivial 
> destructor.


I don't think the idea of a base-line version matches with the direction of the 
patch, and I've removed the part of the patch that has a base-line Clang 
version for PS4. I think that if your platform ABI dictates that you do thing X 
differently from, say, the x86_64 psABI or the Itanium C++ ABI, then that's 
simply part of your platform ABI, and not a clang compatibility feature, 
regardless of whether the historical reason your platform ABI made that choice 
was due to clang compatibility.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

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

rsmith updated this revision to Diff 112733.

Repository:
  rL LLVM

https://reviews.llvm.org/D36501

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/ABIInfo.h
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/CodeGenTypes.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/clang-abi-compat.cpp
  test/CodeGenCXX/uncopyable-args.cpp
  test/Driver/flags.c
  test/Frontend/clang-abi-compat.cpp

Index: test/Frontend/clang-abi-compat.cpp
===
--- test/Frontend/clang-abi-compat.cpp
+++ test/Frontend/clang-abi-compat.cpp
@@ -0,0 +1,15 @@
+// RUN: not %clang_cc1 -fclang-abi-compat=banana %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=2.9 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=8 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=3.10 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4.1 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=04 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4. %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4.00 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// INVALID: error: invalid value '{{.*}}' in '-fclang-abi-compat={{.*}}'
+//
+// RUN: %clang_cc1 -fclang-abi-compat=3.0 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=3.9 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=4 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=4.0 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=latest %s -fsyntax-only
Index: test/Driver/flags.c
===
--- test/Driver/flags.c
+++ test/Driver/flags.c
@@ -24,3 +24,6 @@
 
 // RUN: %clang -target armv7-apple-darwin10 -### -S -mno-implicit-float -mimplicit-float %s 2>&1 | FileCheck -check-prefix=TEST8 %s
 // TEST8-NOT: "-no-implicit-float"
+
+// RUN: %clang -target x86_64-linux-gnu -### -c -fclang-abi-compat=3.2 %s 2>&1 | FileCheck -check-prefix=TEST9 %s
+// TEST9: "-fclang-abi-compat=3.2"
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=NEWABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-scei-ps4 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=18 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-18
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=19 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-19
 
@@ -56,8 +58,10 @@
 // CHECK-LABEL: define void @_ZN9move_ctor3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// NEWABI: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// OLDABI: call void @_ZN9move_ctor3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// OLDABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
 }
@@ -76,8 +80,10 @@
 // CHECK-LABEL: define void @_ZN11all_deleted3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// NEWABI: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// OLDABI: call void @_ZN11all_deleted3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// OLDABI-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
 }
@@ -95,8 

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:3861
< Align.getQuantity()) ||
 (ArgInfo.getIndirectByVal() && (RVAddrSpace != ArgAddrSpace))) {
   // Create an aligned temporary, and copy to it.

yaxunl wrote:
> rjmccall wrote:
> > This should be comparing AST address spaces.
> The AST address space of RV cannot be obtained through 
> `CGFunctionInfo::const_arg_iterator it` and `it->type` since `it->type` takes 
> type of 
> 
> 
> ```
> ImplicitCastExpr 0x60a9ff0  'struct S':'struct S' 
> `-DeclRefExpr 0x60a9f28  '__global struct S':'__global struct S' 
> lvalue Var 0x607efb0
> ```
> 
> and the original addr space is lost due to LValueToRValue cast.
> 
> To get the AST addr space of RV, it seems I need to save the argument Expr in 
> CallArgList and get it from Expr.
> 
I think your last two comments are related.  I'm not sure why we haven't copied 
into a temporary here, and if we had, the assumption of LangAS::Default would 
be fine.  Would you mind doing the investigation there?


https://reviews.llvm.org/D34367



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

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The only thing I would say here is that, as a platform vendor, I would like the 
ability to opt in to ABI fixes that aren't in my base-line version.  In 
particular, we're generally more aggressive about taking C++ ABI fixes on 
Darwin when there's a strong argument that the impact is likely to be low, like 
for some of these indirect-ABI cases that only make a difference for classes 
with e.g. only deleted copy/move constructors but a trivial destructor.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

r311794 - [sanitizer-coverage] extend fsanitize-coverage=pc-table with flags for every PC

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Fri Aug 25 12:29:47 2017
New Revision: 311794

URL: http://llvm.org/viewvc/llvm-project?rev=311794=rev
Log:
[sanitizer-coverage] extend fsanitize-coverage=pc-table with flags for every PC

Modified:
cfe/trunk/docs/SanitizerCoverage.rst

Modified: cfe/trunk/docs/SanitizerCoverage.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/SanitizerCoverage.rst?rev=311794=311793=311794=diff
==
--- cfe/trunk/docs/SanitizerCoverage.rst (original)
+++ cfe/trunk/docs/SanitizerCoverage.rst Fri Aug 25 12:29:47 2017
@@ -148,19 +148,21 @@ With ``-fsanitize-coverage=pc-table`` th
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` 
or
 ``-fsanitize-coverage=trace-pc-guard``.
 
-Users need to implement a single function to capture the counters at startup:
+Users need to implement a single function to capture the PC table at startup:
 
 .. code-block:: c++
 
   extern "C"
-  void __sanitizer_cov_pcs_init(const uint8_t *pcs_beg,
-const uint8_t *pcs_end) {
+  void __sanitizer_cov_pcs_init(const uintptr_t *pcs_beg,
+const uintptr_t *pcs_end) {
 // [pcs_beg,pcs_end) is the array of ptr-sized integers representing
-// PCs of the instrumented blocks in the current DSO.
-// Capture this array in order to read the PCs.
-// The number of PCs for a given DSO is the same as the number of
-// 8-bit counters (-fsanitize-coverage=inline-8bit-counters) or
+// pairs [PC,PCFlags] for every instrumented block in the current DSO.
+// Capture this array in order to read the PCs and their Flags.
+// The number of PCs and PCFlags for a given DSO is the same as the number
+// of 8-bit counters (-fsanitize-coverage=inline-8bit-counters) or
 // trace_pc_guard callbacks (-fsanitize-coverage=trace-pc-guard)
+// A PCFlags describes the basic block:
+//  * bit0: 1 if the block is the function entry block, 0 otherwise.
   }
 
 


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

r311792 - [Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Fri Aug 25 12:14:53 2017
New Revision: 311792

URL: http://llvm.org/viewvc/llvm-project?rev=311792=rev
Log:
[Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for
alignments

Indent should be compared before nesting level to determine if a token
is on the same scope as the one we align with. Because it was inverted,
clang-format sometimes tried to align tokens with tokens from outer
scopes, causing the assert(Shift >= 0) to fire.

This fixes bug #33507. Patch by Beren Minor, thank you!

Modified:
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=311792=311791=311792=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Fri Aug 25 12:14:53 2017
@@ -246,12 +246,12 @@ AlignTokenSequence(unsigned Start, unsig
 
   for (unsigned i = Start; i != End; ++i) {
 if (ScopeStack.size() != 0 &&
-Changes[i].nestingAndIndentLevel() <
-Changes[ScopeStack.back()].nestingAndIndentLevel())
+Changes[i].indentAndNestingLevel() <
+Changes[ScopeStack.back()].indentAndNestingLevel())
   ScopeStack.pop_back();
 
-if (i != Start && Changes[i].nestingAndIndentLevel() >
-  Changes[i - 1].nestingAndIndentLevel())
+if (i != Start && Changes[i].indentAndNestingLevel() >
+  Changes[i - 1].indentAndNestingLevel())
   ScopeStack.push_back(i);
 
 bool InsideNestedScope = ScopeStack.size() != 0;
@@ -327,8 +327,8 @@ static unsigned AlignTokens(const Format
 
   // Measure the scope level (i.e. depth of (), [], {}) of the first token, and
   // abort when we hit any token in a higher scope than the starting one.
-  auto NestingAndIndentLevel = StartAt < Changes.size()
-   ? Changes[StartAt].nestingAndIndentLevel()
+  auto IndentAndNestingLevel = StartAt < Changes.size()
+   ? Changes[StartAt].indentAndNestingLevel()
: std::pair(0, 0);
 
   // Keep track of the number of commas before the matching tokens, we will 
only
@@ -359,7 +359,7 @@ static unsigned AlignTokens(const Format
 
   unsigned i = StartAt;
   for (unsigned e = Changes.size(); i != e; ++i) {
-if (Changes[i].nestingAndIndentLevel() < NestingAndIndentLevel)
+if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
   break;
 
 if (Changes[i].NewlinesBefore != 0) {
@@ -375,7 +375,7 @@ static unsigned AlignTokens(const Format
 
 if (Changes[i].Tok->is(tok::comma)) {
   ++CommasBeforeMatch;
-} else if (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel) {
+} else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
   unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i);
   i = StoppedAt - 1;

Modified: cfe/trunk/lib/Format/WhitespaceManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=311792=311791=311792=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.h (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.h Fri Aug 25 12:14:53 2017
@@ -154,12 +154,11 @@ public:
 const Change *StartOfBlockComment;
 int IndentationOffset;
 
-// A combination of nesting level and indent level, which are used in
+// A combination of indent level and nesting level, which are used in
 // tandem to compute lexical scope, for the purposes of deciding
 // when to stop consecutive alignment runs.
-std::pair
-nestingAndIndentLevel() const {
-  return std::make_pair(Tok->NestingLevel, Tok->IndentLevel);
+std::pair indentAndNestingLevel() const {
+  return std::make_pair(Tok->IndentLevel, Tok->NestingLevel);
 }
   };
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=311792=311791=311792=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Aug 25 12:14:53 2017
@@ -8958,6 +8958,16 @@ TEST_F(FormatTest, AlignConsecutiveDecla
Alignment);
   Alignment.BinPackParameters = true;
   Alignment.ColumnLimit = 80;
+
+  // Bug 33507
+  Alignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat(
+  "auto found = range::find_if(vsProducts, [&](auto * aProduct) {\n"
+  "  static const 

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

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

rsmith added a comment.

In https://reviews.llvm.org/D36501#852765, @probinson wrote:

> I would prefer a diagnostic if PS4 && >3.2.  Whether you map "latest" to 3.2 
> or diagnose that also, up to you, I'm okay either way.


On reflection, I don't think doing anything special for PS4 here is really 
correct. (Suppose Clang 5 has some (accidental) ABI change in it for PS4, and 
we fix that in Clang 6 to once again follow the psABI and do what Clang 3.2 
did. In that case, building with `-fclang-abi-compat=5` should theoretically 
reinstate that accidental ABI change.) The old-Clang compatibility mode and the 
platform ABI are separate, and we should not conflate them. New patch on the 
way.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D37120: [analyzer] Fix modeling arithmetic

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:363
 case nonloc::LocAsIntegerKind:
   return evalBinOpLL(state, op, lhsL,
  rhs.castAs().getLoc(),

@NoQ , @dcoughlin 
while we are looking at this code - just to double check - is this line (363) 
actually correct ?
Let's take a look at the following example:
   bool f(long x, double *p1, double *p2) {
  long y = (long)p1 - (long) p2; 
  // or,alternatively (long)p1 * (long)p2  or (long)p1 / (long)p2
  return y == x;
}
it looks like again the analyzer will try to use evalBinOpLL and evaluate this 
as an operation over pointers, while (if my understanding is correct) we should 
be working with integers here (and yes, in most cases it should return 
UnknownVal)
  


Repository:
  rL LLVM

https://reviews.llvm.org/D37120



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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

Did you check this on something other than the unit tests? 
E.g. a couple of benchmarks from fuzzer-test-suite?




Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:177
 
+bool IsLeafFunc(const Function ) {
+  for (const BasicBlock  : F.getBasicBlockList())

we already have a linear scan in SanitizerCoverageModule::runOnFunction -- 
don't introduce a second one. 
Also, there is InvokeInst in addition to CallInst, see the loop in 
runOnFunction.

You can simply extend the loop in runOnFunction to set a flag if the function 
has non-intrin calls/ invokes 


https://reviews.llvm.org/D37156



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

r311790 - Fix typo in comment, no behavior change.

[Nico Weber via cfe-commits]

Author: nico
Date: Fri Aug 25 11:41:41 2017
New Revision: 311790

URL: http://llvm.org/viewvc/llvm-project?rev=311790=rev
Log:
Fix typo in comment, no behavior change.

Modified:
cfe/trunk/lib/CodeGen/CGCleanup.cpp

Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=311790=311789=311790=diff
==
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Fri Aug 25 11:41:41 2017
@@ -1096,7 +1096,7 @@ void CodeGenFunction::EmitBranchThroughC
 break;
   }
 
-  // Otherwise, tell the scope that there's a jump propoagating
+  // Otherwise, tell the scope that there's a jump propagating
   // through it.  If this isn't new information, all the rest of
   // the work has been done before.
   if (!Scope.addBranchThrough(Dest.getBlock()))


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

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I would prefer a diagnostic if PS4 && >3.2.  Whether you map "latest" to 3.2 or 
diagnose that also, up to you, I'm okay either way.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: hiraditya.

- Don't sanitize __sancov_lowest_stack.
- Don't instrument leaf functions.
- Add CoverageStackDepth to Fuzzer and FuzzerNoLink.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -173,6 +174,13 @@
   return Options;
 }
 
+bool IsLeafFunc(const Function ) {
+  for (const BasicBlock  : F.getBasicBlockList())
+for (const Instruction  : BB.getInstList())
+  if (isa(Insn) && !isa(Insn)) return false;
+  return true;
+}
+
 class SanitizerCoverageModule : public ModulePass {
 public:
   SanitizerCoverageModule(
@@ -731,6 +739,22 @@
 
   IRBuilder<> IRB(&*IP);
   IRB.SetCurrentDebugLocation(EntryLoc);
+  if (Options.StackDepth && IsEntryBB && !IsLeafFunc(F)) {
+// Check stack depth.  If it's the deepest so far, record it.
+Function *GetFrameAddr =
+Intrinsic::getDeclaration(F.getParent(), Intrinsic::frameaddress);
+auto FrameAddrPtr =
+IRB.CreateCall(GetFrameAddr, {Constant::getNullValue(Int32Ty)});
+auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
+auto LowestStack = IRB.CreateLoad(SanCovLowestStack);
+auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
+auto ThenTerm = SplitBlockAndInsertIfThen(IsStackLower, &*IP, false);
+IRBuilder<> ThenIRB(ThenTerm);
+auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
+SetNoSanitizeMetadata(LowestStack);
+SetNoSanitizeMetadata(Store);
+IRB.SetInsertPoint(&*IP);
+  }
   if (Options.TracePC) {
 IRB.CreateCall(SanCovTracePC); // gets the PC using GET_CALLER_PC.
 IRB.CreateCall(EmptyAsm, {}); // Avoids callback merge.
@@ -753,19 +777,6 @@
 SetNoSanitizeMetadata(Load);
 SetNoSanitizeMetadata(Store);
   }
-  if (Options.StackDepth && IsEntryBB) {
-// Check stack 

r311788 - [NFC] Remove a cstyle cast and replace some uses of Decl with NamedDecl during the processing of TemplateParameterLists.

[Faisal Vali via cfe-commits]

Author: faisalv
Date: Fri Aug 25 11:24:20 2017
New Revision: 311788

URL: http://llvm.org/viewvc/llvm-project?rev=311788=rev
Log:
[NFC] Remove a cstyle cast and replace some uses of Decl with NamedDecl during 
the processing of TemplateParameterLists.

Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=311788=311787=311788=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Fri Aug 25 11:24:20 2017
@@ -2724,11 +2724,11 @@ private:
AccessSpecifier AS=AS_none,
AttributeList *AccessAttrs = nullptr);
   bool ParseTemplateParameters(unsigned Depth,
-   SmallVectorImpl ,
+   SmallVectorImpl ,
SourceLocation ,
SourceLocation );
   bool ParseTemplateParameterList(unsigned Depth,
-  SmallVectorImpl );
+  SmallVectorImpl );
   bool isStartOfTemplateTypeParameter();
   Decl *ParseTemplateParameter(unsigned Depth, unsigned Position);
   Decl *ParseTypeParameter(unsigned Depth, unsigned Position);

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=311788=311787=311788=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Aug 25 11:24:20 2017
@@ -6052,7 +6052,7 @@ public:
  SourceLocation ExportLoc,
  SourceLocation TemplateLoc,
  SourceLocation LAngleLoc,
- ArrayRef Params,
+ ArrayRef Params,
  SourceLocation RAngleLoc,
  Expr *RequiresClause);
 

Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=311788=311787=311788=diff
==
--- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTemplate.cpp Fri Aug 25 11:24:20 2017
@@ -112,7 +112,7 @@ Parser::ParseTemplateDeclarationOrSpecia
 
 // Parse the '<' template-parameter-list '>'
 SourceLocation LAngleLoc, RAngleLoc;
-SmallVector TemplateParams;
+SmallVector TemplateParams;
 if (ParseTemplateParameters(CurTemplateDepthTracker.getDepth(),
 TemplateParams, LAngleLoc, RAngleLoc)) {
   // Skip until the semi-colon or a '}'.
@@ -329,10 +329,9 @@ Parser::ParseSingleDeclarationAfterTempl
 /// that enclose this template parameter list.
 ///
 /// \returns true if an error occurred, false otherwise.
-bool Parser::ParseTemplateParameters(unsigned Depth,
-   SmallVectorImpl ,
- SourceLocation ,
- SourceLocation ) {
+bool Parser::ParseTemplateParameters(
+unsigned Depth, SmallVectorImpl ,
+SourceLocation , SourceLocation ) {
   // Get the template parameter list.
   if (!TryConsumeToken(tok::less, LAngleLoc)) {
 Diag(Tok.getLocation(), diag::err_expected_less_after) << "template";
@@ -370,11 +369,12 @@ bool Parser::ParseTemplateParameters(uns
 /// template-parameter-list ',' template-parameter
 bool
 Parser::ParseTemplateParameterList(unsigned Depth,
- SmallVectorImpl ) {
+ SmallVectorImpl ) {
   while (1) {
+// FIXME: ParseTemplateParameter should probably just return a NamedDecl.
 if (Decl *TmpParam
   = ParseTemplateParameter(Depth, TemplateParams.size())) {
-  TemplateParams.push_back(TmpParam);
+  TemplateParams.push_back(dyn_cast(TmpParam));
 } else {
   // If we failed to parse a template parameter, skip until we find
   // a comma or closing brace.
@@ -569,7 +569,7 @@ Parser::ParseTemplateTemplateParameter(u
 
   // Handle the template <...> part.
   SourceLocation TemplateLoc = ConsumeToken();
-  SmallVector TemplateParams;
+  SmallVector TemplateParams;
   SourceLocation LAngleLoc, RAngleLoc;
   {
 ParseScope TemplateParmScope(this, Scope::TemplateParamScope);

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 

[PATCH] D37120: [analyzer] Fix modeling arithmetic

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:371-373
+return makeSymExprValNN(
+state, op, lhs.castAs(),
+rhs.castAs(), resultTy);

NoQ wrote:
> For now this code would return `UnknownVal` in most cases (pointer is not 
> tainted, or not symbolic, or contains an offset), and still construct an 
> invalid symbol in the rest of the cases (`makeSymExprValNN` would add the 
> number to the pointer symbol, instead of modelling an offset within the 
> pointed-to region).
> 
> Once D35450 finally lands (sorry for the delays...), it'd return `UnknownVal` 
> less often and crash more often.
@NoQ - many thanks for the code review!

i assume i might be missing smth, anyway, (replying to this comment and another 
one below)

  >This is probably the way to go: just take the Loc behind the LocAsInteger, 
cast it to char * (because 
  >pointer type shouldn't affect how much bytes of offset are added, anymore), 
add your integer to it, 
  >pack back into LocAsInteger

i'm not sure i understand how that would work here and somehow don't like it. 
For example, op can be MultiplicativeOp (i.e. long y = ((long)p) * 13;) 
extracting the Loc and doing smth with the Loc -  doesn't seem to be the right 
thing (if i understood your suggestion correctly)

  >For now this code would return UnknownVal in 
  >most cases (pointer is not tainted, or not symbolic, 
  >or contains an offset)

that was my understanding what this code should do (return UnknownVal in most 
cases unless we can reason about the actual (integer) values of LHS,RHS)




Repository:
  rL LLVM

https://reviews.llvm.org/D37120



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

r311787 - [Frontend] Fix printing policy for AST context loaded from file

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Fri Aug 25 11:07:03 2017
New Revision: 311787

URL: http://llvm.org/viewvc/llvm-project?rev=311787=rev
Log:
[Frontend] Fix printing policy for AST context loaded from file

In ASTUnit::LoadFromASTFile, the context object is set up using
default-constructed LangOptions (which only later get populated). As the
language options are used in the constructor of PrintingPolicy, this
needs to be updated explicitly after the language options are available.

Patch by Johann Klähn!

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

Added:
cfe/trunk/unittests/Frontend/ASTUnitTest.cpp
Modified:
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/unittests/Frontend/CMakeLists.txt

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=311787=311786=311787=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Fri Aug 25 11:07:03 2017
@@ -542,6 +542,9 @@ private:
 // Initialize the ASTContext
 Context->InitBuiltinTypes(*Target);
 
+// Adjust printing policy based on language options.
+Context->setPrintingPolicy(PrintingPolicy(LangOpt));
+
 // We didn't have access to the comment options when the ASTContext was
 // constructed, so register them now.
 Context->getCommentCommandTraits().registerCommentOptions(

Added: cfe/trunk/unittests/Frontend/ASTUnitTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/ASTUnitTest.cpp?rev=311787=auto
==
--- cfe/trunk/unittests/Frontend/ASTUnitTest.cpp (added)
+++ cfe/trunk/unittests/Frontend/ASTUnitTest.cpp Fri Aug 25 11:07:03 2017
@@ -0,0 +1,87 @@
+//===- unittests/Frontend/ASTUnitTest.cpp - ASTUnit tests 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+TEST(ASTUnit, SaveLoadPreservesLangOptionsInPrintingPolicy) {
+  // Check that the printing policy is restored with the correct language
+  // options when loading an ASTUnit from a file.  To this end, an ASTUnit
+  // for a C++ translation unit is set up and written to a temporary file.
+
+  // By default `UseVoidForZeroParams` is true for non-C++ language options,
+  // thus we can check this field after loading the ASTUnit to deduce whether
+  // the correct (C++) language options were used when setting up the printing
+  // policy.
+
+  {
+PrintingPolicy PolicyWithDefaultLangOpt(LangOptions{});
+EXPECT_TRUE(PolicyWithDefaultLangOpt.UseVoidForZeroParams);
+  }
+
+  int FD;
+  llvm::SmallString<256> InputFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("ast-unit", "cpp", FD, 
InputFileName));
+  tool_output_file input_file(InputFileName, FD);
+  input_file.os() << "";
+
+  const char* Args[] = {"clang", "-xc++", InputFileName.c_str()};
+
+  IntrusiveRefCntPtr Diags =
+  CompilerInstance::createDiagnostics(new DiagnosticOptions());
+
+  std::shared_ptr CInvok =
+  createInvocationFromCommandLine(Args, Diags);
+
+  if (!CInvok)
+FAIL() << "could not create compiler invocation";
+
+  FileManager *FileMgr =
+  new FileManager(FileSystemOptions(), vfs::getRealFileSystem());
+  auto PCHContainerOps = std::make_shared();
+
+  std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+  CInvok, PCHContainerOps, Diags, FileMgr);
+
+  if (!AST)
+FAIL() << "failed to create ASTUnit";
+
+  EXPECT_FALSE(AST->getASTContext().getPrintingPolicy().UseVoidForZeroParams);
+
+  llvm::SmallString<256> ASTFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("ast-unit", "ast", FD, 
ASTFileName));
+  tool_output_file ast_file(ASTFileName, FD);
+  AST->Save(ASTFileName.str());
+
+  EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
+
+  std::unique_ptr AU = ASTUnit::LoadFromASTFile(
+  ASTFileName.str(), PCHContainerOps->getRawReader(), 
ASTUnit::LoadEverything, Diags,
+  FileSystemOptions(), /*UseDebugInfo=*/false);
+
+  if (!AU)
+FAIL() << "failed to load ASTUnit";
+
+  EXPECT_FALSE(AU->getASTContext().getPrintingPolicy().UseVoidForZeroParams);
+}
+
+} // anonymous namespace

Modified: cfe/trunk/unittests/Frontend/CMakeLists.txt
URL: 

[PATCH] D35271: Fix printing policy for AST context loaded from file

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311787: [Frontend] Fix printing policy for AST context 
loaded from file (authored by vedantk).

Changed prior to commit:
  https://reviews.llvm.org/D35271?vs=112334=112718#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35271

Files:
  cfe/trunk/lib/Frontend/ASTUnit.cpp
  cfe/trunk/unittests/Frontend/ASTUnitTest.cpp
  cfe/trunk/unittests/Frontend/CMakeLists.txt

Index: cfe/trunk/lib/Frontend/ASTUnit.cpp
===
--- cfe/trunk/lib/Frontend/ASTUnit.cpp
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp
@@ -542,6 +542,9 @@
 // Initialize the ASTContext
 Context->InitBuiltinTypes(*Target);
 
+// Adjust printing policy based on language options.
+Context->setPrintingPolicy(PrintingPolicy(LangOpt));
+
 // We didn't have access to the comment options when the ASTContext was
 // constructed, so register them now.
 Context->getCommentCommandTraits().registerCommentOptions(
Index: cfe/trunk/unittests/Frontend/CMakeLists.txt
===
--- cfe/trunk/unittests/Frontend/CMakeLists.txt
+++ cfe/trunk/unittests/Frontend/CMakeLists.txt
@@ -3,6 +3,7 @@
   )
 
 add_clang_unittest(FrontendTests
+  ASTUnitTest.cpp
   FrontendActionTest.cpp
   CodeGenActionTest.cpp
   )
Index: cfe/trunk/unittests/Frontend/ASTUnitTest.cpp
===
--- cfe/trunk/unittests/Frontend/ASTUnitTest.cpp
+++ cfe/trunk/unittests/Frontend/ASTUnitTest.cpp
@@ -0,0 +1,87 @@
+//===- unittests/Frontend/ASTUnitTest.cpp - ASTUnit tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+TEST(ASTUnit, SaveLoadPreservesLangOptionsInPrintingPolicy) {
+  // Check that the printing policy is restored with the correct language
+  // options when loading an ASTUnit from a file.  To this end, an ASTUnit
+  // for a C++ translation unit is set up and written to a temporary file.
+
+  // By default `UseVoidForZeroParams` is true for non-C++ language options,
+  // thus we can check this field after loading the ASTUnit to deduce whether
+  // the correct (C++) language options were used when setting up the printing
+  // policy.
+
+  {
+PrintingPolicy PolicyWithDefaultLangOpt(LangOptions{});
+EXPECT_TRUE(PolicyWithDefaultLangOpt.UseVoidForZeroParams);
+  }
+
+  int FD;
+  llvm::SmallString<256> InputFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("ast-unit", "cpp", FD, InputFileName));
+  tool_output_file input_file(InputFileName, FD);
+  input_file.os() << "";
+
+  const char* Args[] = {"clang", "-xc++", InputFileName.c_str()};
+
+  IntrusiveRefCntPtr Diags =
+  CompilerInstance::createDiagnostics(new DiagnosticOptions());
+
+  std::shared_ptr CInvok =
+  createInvocationFromCommandLine(Args, Diags);
+
+  if (!CInvok)
+FAIL() << "could not create compiler invocation";
+
+  FileManager *FileMgr =
+  new FileManager(FileSystemOptions(), vfs::getRealFileSystem());
+  auto PCHContainerOps = std::make_shared();
+
+  std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+  CInvok, PCHContainerOps, Diags, FileMgr);
+
+  if (!AST)
+FAIL() << "failed to create ASTUnit";
+
+  EXPECT_FALSE(AST->getASTContext().getPrintingPolicy().UseVoidForZeroParams);
+
+  llvm::SmallString<256> ASTFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("ast-unit", "ast", FD, ASTFileName));
+  tool_output_file ast_file(ASTFileName, FD);
+  AST->Save(ASTFileName.str());
+
+  EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
+
+  std::unique_ptr AU = ASTUnit::LoadFromASTFile(
+  ASTFileName.str(), PCHContainerOps->getRawReader(), ASTUnit::LoadEverything, Diags,
+  FileSystemOptions(), /*UseDebugInfo=*/false);
+
+  if (!AU)
+FAIL() << "failed to load ASTUnit";
+
+  EXPECT_FALSE(AU->getASTContext().getPrintingPolicy().UseVoidForZeroParams);
+}
+
+} // anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37042#852733, @efriedma wrote:

> It would be nice to warn on any nullptr arithmetic, yes.  (We probably want 
> the wording of the warning to be a bit different if we're activating this 
> workaround.)


+1 (I'll likely want the ability to turn off the warning for one without 
turning off the warning for the other)


https://reviews.llvm.org/D37042



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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

It would be nice to warn on any nullptr arithmetic, yes.  (We probably want the 
wording of the warning to be a bit different if we're activating this 
workaround.)


https://reviews.llvm.org/D37042



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

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGCall.cpp:3861
< Align.getQuantity()) ||
 (ArgInfo.getIndirectByVal() && (RVAddrSpace != ArgAddrSpace))) {
   // Create an aligned temporary, and copy to it.

rjmccall wrote:
> This should be comparing AST address spaces.
The AST address space of RV cannot be obtained through 
`CGFunctionInfo::const_arg_iterator it` and `it->type` since `it->type` takes 
type of 


```
ImplicitCastExpr 0x60a9ff0  'struct S':'struct S' 
`-DeclRefExpr 0x60a9f28  '__global struct S':'__global struct S' 
lvalue Var 0x607efb0
```

and the original addr space is lost due to LValueToRValue cast.

To get the AST addr space of RV, it seems I need to save the argument Expr in 
CallArgList and get it from Expr.



https://reviews.llvm.org/D34367



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

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 112707.
johannes added a comment.

test ordering, class template


https://reviews.llvm.org/D36998

Files:
  include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  include/clang/AST/RecursiveASTVisitor.h
  unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
===
--- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
+++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
@@ -21,8 +21,9 @@
 : public LexicallyOrderedRecursiveASTVisitor {
 public:
   LexicallyOrderedDeclVisitor(DummyMatchVisitor ,
-  const SourceManager )
-  : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher) {}
+  const SourceManager , bool EmitIndices)
+  : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher),
+EmitIndices(EmitIndices) {}
 
   bool TraverseDecl(Decl *D) {
 TraversalStack.push_back(D);
@@ -35,15 +36,20 @@
 
 private:
   DummyMatchVisitor 
+  bool EmitIndices;
+  unsigned Index = 0;
   llvm::SmallVector TraversalStack;
 };
 
 class DummyMatchVisitor : public ExpectedLocationVisitor {
+  bool EmitIndices;
+
 public:
+  DummyMatchVisitor(bool EmitIndices = false) : EmitIndices(EmitIndices) {}
   bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) {
 const ASTContext  = TU->getASTContext();
 const SourceManager  = Context.getSourceManager();
-LexicallyOrderedDeclVisitor SubVisitor(*this, SM);
+LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitIndices);
 SubVisitor.TraverseDecl(TU);
 return false;
   }
@@ -64,9 +70,11 @@
   OS << ND->getNameAsString();
 else
   OS << "???";
-if (isa(D))
+if (isa(D) or isa(D))
   OS << "/";
   }
+  if (EmitIndices)
+OS << "@" << Index++;
   Matcher.match(OS.str(), D);
   return true;
 }
@@ -138,4 +146,18 @@
   EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
 }
 
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitTemplateDecl) {
+  StringRef Source = R"(
+template  T f();
+template  class Class {};
+)";
+  DummyMatchVisitor Visitor(/*EmitIndices=*/true);
+  Visitor.ExpectMatch("/f/T@0", 2, 11);
+  Visitor.ExpectMatch("/f/f/@1", 2, 20);
+  Visitor.ExpectMatch("/Class/U@2", 3, 11);
+  Visitor.ExpectMatch("/Class/@3", 3, 20);
+  Visitor.ExpectMatch("/Class/Class/@4", 3, 34);
+  EXPECT_TRUE(Visitor.runOver(Source));
+}
+
 } // end anonymous namespace
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -499,10 +499,10 @@
 
   bool canIgnoreChildDeclWhileTraversingDeclContext(const Decl *Child);
 
-private:
   // These are helper methods used by more than one Traverse* method.
   bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
 
+private:
   // Traverses template parameter lists of either a DeclaratorDecl or TagDecl.
   template 
   bool TraverseDeclTemplateParameterLists(T *D);
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
===
--- include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
+++ include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
@@ -111,6 +111,21 @@
 return true;
   }
 
+#define DEF_TRAVERSE_TEMPLATE_DECL(TMPLDECLKIND)   \
+  bool Traverse##TMPLDECLKIND##TemplateDecl(TMPLDECLKIND##TemplateDecl *TD) {  \
+if (!BaseType::TraverseTemplateParameterListHelper(\
+TD->getTemplateParameters()))  \
+  return false;\
+assert(!BaseType::getDerived().shouldVisitTemplateInstantiations() &&  \
+   "It does not make sense for most clients to visit template "\
+   "instantiations here.");\
+return BaseType::getDerived().TraverseDecl(TD->getTemplatedDecl());\
+  }
+  DEF_TRAVERSE_TEMPLATE_DECL(Class)
+  DEF_TRAVERSE_TEMPLATE_DECL(Var)
+  DEF_TRAVERSE_TEMPLATE_DECL(Function)
+#undef DEF_TRAVERSE_TEMPLATE_DECL
+
 private:
   bool TraverseAdditionalLexicallyNestedDeclarations() {
 // FIXME: Ideally the gathered declarations and the declarations in the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

In https://reviews.llvm.org/D37042#850676, @hfinkel wrote:

> I'd really like to see this done in some way such that we can issue a warning 
> along with generating well-defined code. The warning can specifically state 
> that the code is using an extension, etc.


Should it warn on any nullptr arithmetic, or just the cases that are being 
handled by this change?


https://reviews.llvm.org/D37042



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
   if (symLHS && symRHS &&
-  (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
+  (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
 return makeNonLoc(symLHS, Op, symRHS, ResultTy);

As a follow up to the previous version of this patch, I do not think we should 
set the default complexity limit to 1.

What is the relation between this limit and the limit in VisitNonLocSymbolVal? 
If they are related, would it be worthwhile turning these into an analyzer 
option?


https://reviews.llvm.org/D35450



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

[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:202
+
+  // Initializer of class constructors, that initialize owners.
+  if (OwnerInitializer) {

Can remove the comma.



Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:239
+diag(BadOwnerInitialization->getLocStart(),
+ "initializing non owner %0 with a newly created 'gsl::owner<>'")
+<< BadOwnerInitialization->getType()

s/non owner/non-owner



Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:256
+assert(BadOwnerParameter && "parameter for the problematic argument not 
found");
+diag(BadOwnerArgument->getLocStart(), "initializing non owner argument of "
+  "type %0 with a newly created "

s/non owner/non-owner



Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:292
+assert(DeclaredOwnerMember &&
+   "Match on class with bad destructor, but without an declared 
owner");
+

s/an declared/a declared

Also, you should either add punctuation or remove the capital M in Match.



Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:294
+
+diag(BadClass->getLocStart(),
+ "class with an 'gsl::owner<>' as member but without declared "

Instead of diagnosing this on the class, why not diagnose the individual 
offending members (and then you don't need a note, either)? (Note, that may 
change the suggested diagnostic wording below).



Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:295
+diag(BadClass->getLocStart(),
+ "class with an 'gsl::owner<>' as member but without declared "
+ "destructor to release the owner");

This diagnostic isn't grammatically correct; how about: `class with a member 
variable of type 'gsl::owner<>' but no destructor to release the owned 
resource`?


https://reviews.llvm.org/D36354



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

r311779 - [ObjC] Add a -Wobjc-messaging-id warning

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 25 09:12:17 2017
New Revision: 311779

URL: http://llvm.org/viewvc/llvm-project?rev=311779=rev
Log:
[ObjC] Add a -Wobjc-messaging-id warning

-Wobjc-messaging-id is a new, non-default warning that warns about
message sends to unqualified id in Objective-C. This warning is useful
for projects that would like to avoid any potential future compiler
errors/warnings, as the system frameworks might add a method with the same
selector which could make the message send to id ambiguous.

rdar://33303354

Added:
cfe/trunk/test/SemaObjC/warn-messaging-id.mm
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExprObjC.cpp

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=311779=311778=311779=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Fri Aug 25 09:12:17 2017
@@ -66,6 +66,12 @@ Improvements to Clang's diagnostics
 a non-default alignment that has been specified using a ``#pragma pack``
 directive prior to the ``#include``.
 
+- ``-Wobjc-messaging-id`` is a new, non-default warning that warns about
+  message sends to unqualified ``id`` in Objective-C. This warning is useful
+  for projects that would like to avoid any potential future compiler
+  errors/warnings, as the system frameworks might add a method with the same
+  selector which could make the message send to ``id`` ambiguous.
+
 Non-comprehensive list of changes in this release
 -
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=311779=311778=311779=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug 25 09:12:17 
2017
@@ -1211,6 +1211,10 @@ def err_objc_method_unsupported_param_re
   "%0 %select{parameter|return}1 type is unsupported; "
   "support for vector types for this target is introduced in %2">;
 
+def warn_messaging_unqualified_id : Warning<
+  "messaging unqualified id">, DefaultIgnore,
+  InGroup>;
+
 // C++ declarations
 def err_static_assert_expression_is_not_constant : Error<
   "static_assert expression is not an integral constant expression">;

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=311779=311778=311779=diff
==
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Fri Aug 25 09:12:17 2017
@@ -2705,6 +2705,9 @@ ExprResult Sema::BuildInstanceMessage(Ex
 }
   }
 
+  if (ReceiverType->isObjCIdType() && !isImplicit)
+Diag(Receiver->getExprLoc(), diag::warn_messaging_unqualified_id);
+
   // There's a somewhat weird interaction here where we assume that we
   // won't actually have a method unless we also don't need to do some
   // of the more detailed type-checking on the receiver.

Added: cfe/trunk/test/SemaObjC/warn-messaging-id.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/warn-messaging-id.mm?rev=311779=auto
==
--- cfe/trunk/test/SemaObjC/warn-messaging-id.mm (added)
+++ cfe/trunk/test/SemaObjC/warn-messaging-id.mm Fri Aug 25 09:12:17 2017
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class 
-Wobjc-messaging-id %s
+
+@interface CallMeMaybe
+
+- (void)doThing:(int)intThing;
+
+@property int thing;
+
+@end
+
+template
+void instantiate(const T ) {
+  [x setThing: 22]; // expected-warning {{messaging unqualified id}}
+}
+
+void fn() {
+  id myObject;
+  [myObject doThing: 10]; // expected-warning {{messaging unqualified id}}
+  [myObject setThing: 11]; // expected-warning {{messaging unqualified id}}
+  instantiate(myObject); // expected-note {{in instantiation}}
+}


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

[PATCH] D37150: [clangd] Command line arg to specify compile_commands.json path

[William Enright via Phabricator via cfe-commits]

Nebiroth created this revision.

Adds compileCommands command line argument to specify an absolute path directly 
to the requested compile_commands.json for flags.


https://reviews.llvm.org/D37150

Files:
  clangd/GlobalCompilationDatabase.cpp

Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -11,6 +11,24 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/Mutex.h"
+#include "llvm/Support/Timer.h"
+#include "llvm/Support/YAMLTraits.h"
+#include "llvm/Support/raw_ostream.h"
+using namespace llvm;
+
+namespace {
+
+static cl::opt CompileCommands("compileCommands",
+ cl::desc("Start with absolute path to compile_commands.json"));  
+}
 
 namespace clang {
 namespace clangd {
@@ -36,6 +54,8 @@
  /*Output=*/"");
 }
 
+
+
 std::vector
 DirectoryBasedGlobalCompilationDatabase::getCompileCommands(PathRef File) {
   std::vector Commands;
@@ -64,35 +84,62 @@
 tooling::CompilationDatabase *
 DirectoryBasedGlobalCompilationDatabase::getCompilationDatabase(PathRef File) {
   std::lock_guard Lock(Mutex);
-
   namespace path = llvm::sys::path;
+  // if --compileCommands arg was invoked, check value and override default path
+  std::size_t found = CompileCommands.find_first_of("/");
+  std::string TempString = CompileCommands;
+  if (found != std::string::npos)
+  {
+ File = TempString;
+  }
+
+  std::string Error;
+  bool badPath = false;
+  File = path::parent_path(File);
+  auto CachedIt = CompilationDatabases.find(File);
+  if (CachedIt != CompilationDatabases.end())
+return CachedIt->second.get();
+  auto CDB = tooling::CompilationDatabase::loadFromDirectory(File, Error);
+  if (!CDB) {
+  if (!Error.empty()) {
+badPath = true;
+  }  
+  }
 
   assert((path::is_absolute(File, path::Style::posix) ||
   path::is_absolute(File, path::Style::windows)) &&
  "path must be absolute");
 
-  for (auto Path = path::parent_path(File); !Path.empty();
+  if (badPath)
+  {
+for (auto Path = path::parent_path(File); !Path.empty();
Path = path::parent_path(Path)) {
 
 auto CachedIt = CompilationDatabases.find(Path);
 if (CachedIt != CompilationDatabases.end())
   return CachedIt->second.get();
-std::string Error;
 auto CDB = tooling::CompilationDatabase::loadFromDirectory(Path, Error);
 if (!CDB) {
   if (!Error.empty()) {
 // FIXME(ibiryukov): logging
 // Output.log("Error when trying to load compilation database from " +
 //Twine(Path) + ": " + Twine(Error) + "\n");
-  }
-  continue;
+  }  
 }
 
 // FIXME(ibiryukov): Invalidate cached compilation databases on changes
 auto result = CDB.get();
 CompilationDatabases.insert(std::make_pair(Path, std::move(CDB)));
 return result;
+}
   }
+  else
+  {
+auto result = CDB.get();
+CompilationDatabases.insert(std::make_pair(File, std::move(CDB)));
+return result;
+  }
+
 
   // FIXME(ibiryukov): logging
   // Output.log("Failed to find compilation database for " + Twine(File) +
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D37090: Implement CFG construction for __finally.

[Nico Weber via cfe-commits]

On Thu, Aug 24, 2017 at 1:04 PM, Reid Kleckner via Phabricator via
cfe-commits  wrote:

> rnk added a comment.
>
> Re: jumps out of __try, I wonder if you can tie __finally into whatever
> the CFG does for C++ destructors.
>

Given
$ cat test.cc
struct C {
  C();
  ~C();
};

bool g();
void h();

void f() {
  C c;
  if (g())
return;

  h();
}

Then `bin/clang-cl /c test.cc -Xclang -analyze -Xclang
-analyzer-checker=debug.ViewCFG` produces this CFG: http://imgur.com/SoGSFNY

So it looks like dtor calls just get duplicated into every branch end at
the moment, instead of producing proper cleanup blocks like codegen does
:-/ Since the __finally body isn't callable, this won't work. I think I'll
try to implement something like codegen's cleanup stuff
(EmitBranchThroughCleanup etc) and use that for __finally, and maybe we can
then use that for dtors some time later too. And maybe we can even use the
CFG for codegen one day.


>
>
>
> 
> Comment at: test/Sema/warn-unreachable-ms.c:49
>  __try {
> -  f();
> +  throw 1;
>  } __except (1) {
> 
> Nice. Would any noreteurn call work here to eliminate the re-run and ifdef?
>

Nice, that works. Just `f(); return;` instead of `throw 1;` does the trick.
(Call to f() isn't really needed but without it the test case is confusing
to humans – how could the __except possibly be entered?)


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

[PATCH] D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311778: [Basic] Add a DiagnosticError llvm::ErrorInfo 
subclass (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D36969?vs=112327=112700#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36969

Files:
  cfe/trunk/include/clang/Basic/DiagnosticError.h
  cfe/trunk/lib/Basic/Diagnostic.cpp
  cfe/trunk/unittests/Basic/DiagnosticTest.cpp

Index: cfe/trunk/lib/Basic/Diagnostic.cpp
===
--- cfe/trunk/lib/Basic/Diagnostic.cpp
+++ cfe/trunk/lib/Basic/Diagnostic.cpp
@@ -11,8 +11,9 @@
 //
 //===--===//
 
-#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/PartialDiagnostic.h"
@@ -1050,3 +1051,5 @@
   llvm::CrashRecoveryContext::isRecoveringFromCrash()) &&
  "A partial is on the lam");
 }
+
+char DiagnosticError::ID;
Index: cfe/trunk/unittests/Basic/DiagnosticTest.cpp
===
--- cfe/trunk/unittests/Basic/DiagnosticTest.cpp
+++ cfe/trunk/unittests/Basic/DiagnosticTest.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,25 @@
   }
 }
 
+TEST(DiagnosticTest, diagnosticError) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
+  new IgnoringDiagConsumer());
+  PartialDiagnostic::StorageAllocator Alloc;
+  llvm::Expected> Value = DiagnosticError::create(
+  SourceLocation(), PartialDiagnostic(diag::err_cannot_open_file, Alloc)
+<< "file"
+<< "error");
+  ASSERT_TRUE(!Value);
+  llvm::Error Err = Value.takeError();
+  Optional ErrDiag = DiagnosticError::take(Err);
+  llvm::cantFail(std::move(Err));
+  ASSERT_FALSE(!ErrDiag);
+  EXPECT_EQ(ErrDiag->first, SourceLocation());
+  EXPECT_EQ(ErrDiag->second.getDiagID(), diag::err_cannot_open_file);
+
+  Value = std::make_pair(20, 1);
+  ASSERT_FALSE(!Value);
+  EXPECT_EQ(*Value, std::make_pair(20, 1));
+  EXPECT_EQ(Value->first, 20);
+}
 }
Index: cfe/trunk/include/clang/Basic/DiagnosticError.h
===
--- cfe/trunk/include/clang/Basic/DiagnosticError.h
+++ cfe/trunk/include/clang/Basic/DiagnosticError.h
@@ -0,0 +1,61 @@
+//===--- DiagnosticError.h - Diagnostic payload for llvm::Error -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_DIAGNOSTIC_ERROR_H
+#define LLVM_CLANG_BASIC_DIAGNOSTIC_ERROR_H
+
+#include "clang/Basic/PartialDiagnostic.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+
+/// \brief Carries a Clang diagnostic in an llvm::Error.
+///
+/// Users should emit the stored diagnostic using the DiagnosticsEngine.
+class DiagnosticError : public llvm::ErrorInfo {
+public:
+  DiagnosticError(PartialDiagnosticAt Diag) : Diag(std::move(Diag)) {}
+
+  void log(raw_ostream ) const override { OS << "clang diagnostic"; }
+
+  PartialDiagnosticAt () { return Diag; }
+  const PartialDiagnosticAt () const { return Diag; }
+
+  /// Creates a new \c DiagnosticError that contains the given diagnostic at
+  /// the given location.
+  static llvm::Error create(SourceLocation Loc, PartialDiagnostic Diag) {
+return llvm::make_error(
+PartialDiagnosticAt(Loc, std::move(Diag)));
+  }
+
+  /// Extracts and returns the diagnostic payload from the given \c Error if
+  /// the error is a \c DiagnosticError. Returns none if the given error is not
+  /// a \c DiagnosticError.
+  static Optional take(llvm::Error ) {
+Optional Result;
+Err = llvm::handleErrors(std::move(Err), [&](DiagnosticError ) {
+  Result = std::move(E.getDiagnostic());
+});
+return Result;
+  }
+
+  static char ID;
+
+private:
+  // Users are not expected to use error_code.
+  std::error_code convertToErrorCode() const override {
+return llvm::inconvertibleErrorCode();
+  }
+
+  PartialDiagnosticAt Diag;
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_BASIC_DIAGNOSTIC_ERROR_H
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311778 - [Basic] Add a DiagnosticError llvm::ErrorInfo subclass

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 25 08:48:00 2017
New Revision: 311778

URL: http://llvm.org/viewvc/llvm-project?rev=311778=rev
Log:
[Basic] Add a DiagnosticError llvm::ErrorInfo subclass

Clang's DiagnosticError is an llvm::Error payload that stores a partial
diagnostic and its location. I'll be using it in the refactoring engine.

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

Added:
cfe/trunk/include/clang/Basic/DiagnosticError.h
Modified:
cfe/trunk/lib/Basic/Diagnostic.cpp
cfe/trunk/unittests/Basic/DiagnosticTest.cpp

Added: cfe/trunk/include/clang/Basic/DiagnosticError.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticError.h?rev=311778=auto
==
--- cfe/trunk/include/clang/Basic/DiagnosticError.h (added)
+++ cfe/trunk/include/clang/Basic/DiagnosticError.h Fri Aug 25 08:48:00 2017
@@ -0,0 +1,61 @@
+//===--- DiagnosticError.h - Diagnostic payload for llvm::Error -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_DIAGNOSTIC_ERROR_H
+#define LLVM_CLANG_BASIC_DIAGNOSTIC_ERROR_H
+
+#include "clang/Basic/PartialDiagnostic.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+
+/// \brief Carries a Clang diagnostic in an llvm::Error.
+///
+/// Users should emit the stored diagnostic using the DiagnosticsEngine.
+class DiagnosticError : public llvm::ErrorInfo {
+public:
+  DiagnosticError(PartialDiagnosticAt Diag) : Diag(std::move(Diag)) {}
+
+  void log(raw_ostream ) const override { OS << "clang diagnostic"; }
+
+  PartialDiagnosticAt () { return Diag; }
+  const PartialDiagnosticAt () const { return Diag; }
+
+  /// Creates a new \c DiagnosticError that contains the given diagnostic at
+  /// the given location.
+  static llvm::Error create(SourceLocation Loc, PartialDiagnostic Diag) {
+return llvm::make_error(
+PartialDiagnosticAt(Loc, std::move(Diag)));
+  }
+
+  /// Extracts and returns the diagnostic payload from the given \c Error if
+  /// the error is a \c DiagnosticError. Returns none if the given error is not
+  /// a \c DiagnosticError.
+  static Optional take(llvm::Error ) {
+Optional Result;
+Err = llvm::handleErrors(std::move(Err), [&](DiagnosticError ) {
+  Result = std::move(E.getDiagnostic());
+});
+return Result;
+  }
+
+  static char ID;
+
+private:
+  // Users are not expected to use error_code.
+  std::error_code convertToErrorCode() const override {
+return llvm::inconvertibleErrorCode();
+  }
+
+  PartialDiagnosticAt Diag;
+};
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_BASIC_DIAGNOSTIC_ERROR_H

Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=311778=311777=311778=diff
==
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Fri Aug 25 08:48:00 2017
@@ -11,8 +11,9 @@
 //
 
//===--===//
 
-#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/PartialDiagnostic.h"
@@ -1050,3 +1051,5 @@ PartialDiagnostic::StorageAllocator::~St
   llvm::CrashRecoveryContext::isRecoveringFromCrash()) &&
  "A partial is on the lam");
 }
+
+char DiagnosticError::ID;

Modified: cfe/trunk/unittests/Basic/DiagnosticTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/DiagnosticTest.cpp?rev=311778=311777=311778=diff
==
--- cfe/trunk/unittests/Basic/DiagnosticTest.cpp (original)
+++ cfe/trunk/unittests/Basic/DiagnosticTest.cpp Fri Aug 25 08:48:00 2017
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,25 @@ TEST(DiagnosticTest, suppressAfterFatalE
   }
 }
 
+TEST(DiagnosticTest, diagnosticError) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
+  new IgnoringDiagConsumer());
+  PartialDiagnostic::StorageAllocator Alloc;
+  llvm::Expected> Value = DiagnosticError::create(
+  SourceLocation(), PartialDiagnostic(diag::err_cannot_open_file, Alloc)
+<< "file"
+<< "error");
+  

r311777 - [OPENMP] Fix for PR34321: ustom OpenMP reduction in C++ template causes

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Aug 25 08:43:55 2017
New Revision: 311777

URL: http://llvm.org/viewvc/llvm-project?rev=311777=rev
Log:
[OPENMP] Fix for PR34321: ustom OpenMP reduction in C++ template causes
SEGFAULT at compile time

Compiler crashed when tried to rebuild non-template expression in
dependent context.

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

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=311777=311776=311777=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Aug 25 08:43:55 2017
@@ -9057,7 +9057,8 @@ buildDeclareReductionRef(Sema ,
   PrevD = D;
 }
   }
-  if (Ty->isDependentType() || Ty->isInstantiationDependentType() ||
+  if (SemaRef.CurContext->isDependentContext() || Ty->isDependentType() ||
+  Ty->isInstantiationDependentType() ||
   Ty->containsUnexpandedParameterPack() ||
   filterLookupForUDR(Lookups, [](ValueDecl *D) -> bool {
 return !D->isInvalidDecl() &&

Modified: cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp?rev=311777=311776=311777=diff
==
--- cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp Fri Aug 25 08:43:55 2017
@@ -92,6 +92,22 @@ T foo(T a) {
   return a;
 }
 
+struct Summary {
+  void merge(const Summary& other) {}
+};
+
+template 
+void work() {
+  Summary global_summary;
+#pragma omp declare reduction(+ : Summary : omp_out.merge(omp_in))
+#pragma omp parallel for reduction(+ : global_summary)
+  for (int k = 1; k <= 100; ++k) {
+  }
+}
+
+struct A {};
+
+
 // CHECK-LABEL: @main
 int main() {
   int i = 0;
@@ -110,6 +126,8 @@ int main() {
   // CHECK: call {{.*}}void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) 
@__kmpc_fork_call(
   // CHECK: call {{.*}}void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) 
@__kmpc_fork_call(
   // CHECK: call {{.*}}void (%ident_t*, i32, void (i32*, i32*, ...)*, ...) 
@__kmpc_fork_call({{[^@]*}} @{{[^@]*}}[[REGION:@[^ ]+]]
+  // CHECK-LABEL: work
+  work();
   // CHECK-LABEL: foo
   return foo(15);
 }


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

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: tools/clang-diff/ClangDiff.cpp:319
+   "A Binary operator is supposed to have two arguments.");
+for (int I : {1, 0, 2})
+  Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Children[I], Offset);

johannes wrote:
> arphaman wrote:
> > Please add a short comment that describes why this out-of-order traversal 
> > is required
> Should we do this in LexicallyOrderedRecursiveASTVisitor?
> 
> There are some other cases with CXXOperatorCallExpr where the order needs to 
> be changed, e.g. postfix operators, operator->, operator() and operator[].
> It can be done by sorting by SourceLocation of the first two elements, as the 
> operator is always the first one.
Sure, that would make sense. You probably don't need to sort, just figure out 
the right order from the specific combination of operator and infix/postfix.


https://reviews.llvm.org/D37004



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

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added inline comments.



Comment at: tools/clang-diff/ClangDiff.cpp:319
+   "A Binary operator is supposed to have two arguments.");
+for (int I : {1, 0, 2})
+  Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Children[I], Offset);

arphaman wrote:
> Please add a short comment that describes why this out-of-order traversal is 
> required
Should we do this in LexicallyOrderedRecursiveASTVisitor?

There are some other cases with CXXOperatorCallExpr where the order needs to be 
changed, e.g. postfix operators, operator->, operator() and operator[].
It can be done by sorting by SourceLocation of the first two elements, as the 
operator is always the first one.


https://reviews.llvm.org/D37004



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

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGCall.cpp:3851
+ ->getType()
+ ->getPointerAddressSpace();
 const unsigned ArgAddrSpace =

rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Hmm.  Is there actually a test case where 
> > > Addr.getType()->getAddressSpace() is not the lowering of LangAS::Default? 
> > >  The correctness of the performAddrSpaceCast call you make depends on 
> > > that.
> > > 
> > > If there isn't, I think it would be better to add a target hook to 
> > > attempt to peephole an addrspace cast:
> > >   llvm::Value *tryPeepholeAddrSpaceCast(llvm::Value*, unsigned valueAS, 
> > > unsigned targetAS);
> > > 
> > > And then you can just do
> > >   bool HasAddrSpaceMismatch = CGM.getASTAllocaAddrSpace() != 
> > > LangAS::Default);
> > >   if (HasAddrSpaceMismatch) {
> > > if (llvm::Value *ptr = tryPeepholeAddrSpaceCast(Addr.getPointer(), 
> > > LangAS::Default, getASTAllocAddrSpace())) {
> > >   Addr = Address(ptr, Addr.getAlignment()); // might need to cast the 
> > > element type for this
> > >   HasAddrSpaceMismatch = false;
> > > }
> > >   }
> > It is possible RVAddrSpace is not default addr space. e.g. in OpenCL
> > 
> > ```
> > global struct S g_s;
> > 
> > void f(struct S s);
> > 
> > void g() {
> >   f(g_s);
> > }
> > 
> > ```
> > 
> > One thing that confuses me is that why creating temp and copying can be 
> > skipped if RVAddrSpace equals alloca addr space. The callee is supposed to 
> > work on a copy of the arg, not the arg itself, right? Shouldn't the arg 
> > always be coped to a temp then pass to the callee?
> The result of emitting an agg expression should already be a temporary.  All 
> sorts of things would be broken if it we still had a direct reference to g_s 
> when we got here.
For the above example, the AST for the call statement `f(g_s)` is


```
`-CallExpr 0x60a9fc0  'void'
  |-ImplicitCastExpr 0x60a9fa8  'void (*)(struct S)' 

  | `-DeclRefExpr 0x60a9f00  'void (struct S)' Function 0x60a9d80 
'f' 'void (struct S)'
  `-ImplicitCastExpr 0x60a9ff0  'struct S':'struct S' 

`-DeclRefExpr 0x60a9f28  '__global struct S':'__global struct S' 
lvalue Var 0x607efb0 'g_s' '__global struct S':'__global struct S'

```

When clang executes line 3846 of CGCall.cpp, RV contains

```
@g_s = external addrspace(1) global %struct.S, align 4
```

Therefore the temporary var has not been created yet. It seems the temporary 
var will be created by line 3864.

So at line 3848, RVAddrSpace has non-default address space 1.




https://reviews.llvm.org/D34367



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

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added a comment.

In https://reviews.llvm.org/D37001#852442, @arphaman wrote:

> Can you remove `getNodeValue` now or do you still need it?


It is only used in the client, I think it makes sense to move it there, or to 
remove it altogether.
I think I'd keep it in the client for now, to disambiguate things with 
different names in the tests.


https://reviews.llvm.org/D37001



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

[PATCH] D36790: [ObjC] Messages to 'self' in class methods should use class method dispatch to avoid multiple method ambiguities

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@doug.gregor Ping


Repository:
  rL LLVM

https://reviews.llvm.org/D36790



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

[PATCH] D37138: Make run-clang-tidy compatible with Python 3.x

[Alexander Kornienko via Phabricator via cfe-commits]

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

Please don't forget go add cfe-commits to subscribers when you create a 
differential revision.

LG with one comment.




Comment at: clang-tidy/tool/run-clang-tidy.py:242
 # Spin up a bunch of tidy-launching threads.
-queue = Queue.Queue(max_task)
+q = queue.Queue(max_task)
 for _ in range(max_task):

let's give it a more meaningful name, e.g. file_queue or task_queue.


https://reviews.llvm.org/D37138



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

[PATCH] D37143: [clang-format] Fixed missing enter before bracket in typedef enum and extern

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

These look like they could be two separate patches: I like the `typedef enum` 
part, not convinced about the `extern` part. Also, please add unit tests.


https://reviews.llvm.org/D37143



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

[PATCH] D37005: Add include/clang/Tooling/ASTDiff/ASTPatch.h

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I don't think AST manipulation is the right way to do patching. You've already 
hit the limitations here in this patch where you can't really remove a 
statement unless its parent is a `CompoundStmt`.

I think the right way to do AST patching is to take a 3rd AST(lets call it a 
target AST), find the set of matching nodes that correspond to all nodes that 
have to be patched and then perform patching by rewriting the source for the 
target AST.
Let's say you want to start with `remove`. If you take the following files:

  $ cat src.cpp
  void printf(const char *, ...);
  void foo(int x) {
printf("%d", x, x);
  }
  $ cat dst.cpp
  void printf(const char *, ...);
  void foo(int x) {
printf("%d", x); // the second 'x' is removed.
  }
  $ cat target.cpp
  void printf(const char *, ...);
  void foo(int x) {
printf("different string %d", x, x);
  }

You'll find that the difference between src.cpp and dst.cpp is `Delete 
DeclRefExpr: x(10)`. Then you can take target.cpp, find the matching 
`DeclRefExpr` node, and create a source replacement (see tooling's Replacement) 
that removes ", x" from target.cpp.


https://reviews.llvm.org/D37005



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

[PATCH] D37143: [clang-format] Fixed missing enter before bracket in typedef enum and extern

[Pawel Maciocha via Phabricator via cfe-commits]

PriMee created this revision.
Herald added a subscriber: klimek.

Bug: https://bugs.llvm.org/show_bug.cgi?id=34016

**Problem:**

Clang format does not allow the flag **BraceWrapping.AfterEnum** control the 
case when our **enum** is preceded by **typedef** keyword (what is common in C 
language). 
Due to the lack of "brace wrapping extern" flag, clang format does parse the 
block after **extern** keyword moving the opening bracket to the header line 
**always**!

**Patch description:**

Added case to the **"AfterEnum"** flag when our enum does not start a line - is 
preceded by **typedef** keyword.
Added if statement handling the case when our **"extern block"** has the 
opening bracket in "non-header" line. Then forcing break before bracket.

**After fix:**

**BEFORE:**

  typedef enum 
  {
  a,
  b,
  c
  } SomeEnum;
  
  extern "C" 
  {
  #include 
  }

**AFTER:**

  typedef enum 
  {
  a,
  b,
  c
  } SomeEnum;
  
  extern "C" 
  {
  #include 
  }

**Remains the same!**


https://reviews.llvm.org/D37143

Files:
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineParser.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -986,6 +986,8 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+if (isOnNewLine(*FormatTok))
+  MustBreakBeforeNextToken = true;
 parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);
 addUnwrappedLine();
 return;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2647,6 +2647,7 @@
 return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
 return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+   (Line.startsWith(tok::kw_typedef, tok::kw_enum) && 
Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) 
||
(Line.startsWith(tok::kw_struct) && 
Style.BraceWrapping.AfterStruct);
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -986,6 +986,8 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+if (isOnNewLine(*FormatTok))
+  MustBreakBeforeNextToken = true;
 parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);
 addUnwrappedLine();
 return;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2647,6 +2647,7 @@
 return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
 return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+   (Line.startsWith(tok::kw_typedef, tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
(Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37142: clang-format: [JS] simplify template string wrapping.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
Herald added a subscriber: klimek.

Previously, clang-format would try to wrap template string substitutions
by indenting relative to the openening `${`. This helped with
indenting structured strings, such as strings containing HTML, as the
substitutions would be aligned according to the structure of the string.

However it turns out that the overwhelming majority of template string +
substitution usages are for substitutions into non-structured strings,
e.g. URLs or just plain messages. For these situations, clang-format
would often produce very ugly indents, in particular for strings
containing no line breaks:

  return `${file}(${
  row
},${
col
  }): `;

This change makes clang-format indent template string substitutions as
if they were string concatenation operations. It wraps +4 on overlong
lines and keeps all operands on the same line:

  return `${file}(${
  row},${col}): `;

While this breaks some lexical continuity between the `${` and `row}`
here, the overall effects are still a huge improvement, and users can
still manually break the string using `+` if desired.


https://reviews.llvm.org/D37142

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1793,43 +1793,39 @@
   verifyFormat("var x = someFunction(`${})`)  //\n"
".oon();");
   verifyFormat("var x = someFunction(`${}${\n"
-   "   a(  //\n"
-   "   a)\n"
-   " })`);");
+   "a(  //\n"
+   "a)})`);");
 }
 
 TEST_F(FormatTestJS, TemplateStringMultiLineExpression) {
   verifyFormat("var f = `aa: ${\n"
-   "   a +  //\n"
-   "   \n"
-   " }`;",
+   "a +  //\n"
+   "}`;",
"var f = `aa: ${a +  //\n"
"   }`;");
   verifyFormat("var f = `\n"
"  aa: ${\n"
-   "a +  //\n"
-   "\n"
-   "  }`;",
+   "a +  //\n"
+   "}`;",
"var f  =  `\n"
"  aa: ${   a  +  //\n"
" }`;");
   verifyFormat("var f = `\n"
"  aa: ${\n"
-   "someFunction(\n"
-   "a +  //\n"
-   ")\n"
-   "  }`;",
+   "someFunction(\n"
+   "a +  //\n"
+   ")}`;",
"var f  =  `\n"
"  aa: ${someFunction (\n"
"a  +   //\n"
")}`;");
 
   // It might be preferable to wrap before "someFunction".
   verifyFormat("var f = `\n"
"  aa: ${someFunction({\n"
-   "  : a,\n"
-   "  : b,\n"
-   "})}`;",
+   "  : a,\n"
+   "  : b,\n"
+   "})}`;",
"var f  =  `\n"
"  aa: ${someFunction ({\n"
"  :  a,\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -661,9 +661,7 @@
   // before the corresponding } or ].
   if (PreviousNonComment &&
   (PreviousNonComment->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
-   opensProtoMessageField(*PreviousNonComment, Style) ||
-   (PreviousNonComment->is(TT_TemplateString) &&
-PreviousNonComment->opensScope(
+   opensProtoMessageField(*PreviousNonComment, Style)))
 State.Stack.back().BreakBeforeClosingBrace = true;
 
   if (State.Stack.back().AvoidBinPacking) {
@@ -925,11 +923,6 @@
 
   moveStatePastFakeLParens(State, Newline);
   

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Can you remove `getNodeValue` now or do you still need it?


https://reviews.llvm.org/D37001



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

[PATCH] D37140: [clang-format] Fixed one-line if statement

[Pawel Maciocha via Phabricator via cfe-commits]

PriMee created this revision.
Herald added a subscriber: klimek.

**Short overview:**

Fixed bug: https://bugs.llvm.org/show_bug.cgi?id=34001
Clang-format bug resulting in a strange behavior of control statements short 
blocks. Different flags combinations do not guarantee expected result. Turned 
on option AllowShortBlocksOnASingleLine does not work as intended.

**Description of the problem:**

Cpp source file UnwrappedLineFormatter does not handle 
AllowShortBlocksOnASingleLine flag as it should. Putting a single-line control 
statement without any braces, clang-format works as expected (depending on 
AllowShortIfStatementOnASingleLine or AllowShortLoopsOnASingleLine value). 
Putting a single-line control statement in braces, we can observe strange and 
incorrect behavior.
Our short block is intercepted by tryFitMultipleLinesInOne function. The 
function returns a number of lines to be merged. Unfortunately, our control 
statement block is not covered properly. There are several if-return 
statements, but none of them handles our block. A block is identified by the 
line first token and by left and right braces. A function block works as 
expected, there is such an if-return statement doing proper job. A control 
statement block, from the other hand, falls into strange conditional construct, 
which depends on BraceWrapping.AfterFunction flag (with condition that the 
line’s last token is left brace, what is possible in our case) or goes even 
further. That should definitely not happen.

**Description of the patch:**

By adding three different if statements, we guarantee that our short control 
statement block, however it looks like (different brace wrapping flags may be 
turned on), is handled properly and does not fall into wrong conditional 
construct. Depending on appropriate options we return either 0 (when something 
disturbs our merging attempt) or let another function (tryMergeSimpleBlock) 
take the responsibility of returned result (number of merged lines). 
Nevertheless, one more correction is required in mentioned tryMergeSimpleBlock 
function. The function, previously, returned either 0 or 2. The problem was 
that this did not handle the case when our block had the left brace in a 
separate line, not the header one. After change, after adding condition, we 
return the result compatible with block’s structure. In case of left brace in 
the header’s line we do everything as before the patch. In case of left brace 
in a separate line we do the job similar to the one we do in case of a 
“non-header left brace” function short block. To be precise, we try to merge 
the block ignoring the header line. Then, if success, we increment our returned 
result.

**After fix:**

**CONFIG:**

  AllowShortBlocksOnASingleLine: true 
  AllowShortIfStatementsOnASingleLine: true 
  BreakBeforeBraces: Custom
  BraceWrapping: { 
  AfterClass: true, AfterControlStatement: true, AfterEnum: true, 
AfterFunction: true, AfterNamespace: false, AfterStruct: true, AfterUnion: 
true, BeforeCatch: true, BeforeElse: true 
  }

**BEFORE:**

  if (statement) doSomething();
  if (statement) { doSomething(); }
  if (statement) {
  doSomething();
  }
  if (statement)
  {
  doSomething();
  }
  if (statement)
  doSomething();
  if (statement) {
  doSomething1();
  doSomething2();
  }

**AFTER:**

  if (statement) doSomething();
  if (statement) { doSomething(); }
  if (statement) { doSomething(); }
  if (statement) { doSomething(); }
  if (statement) doSomething();
  if (statement)
  {
doSomething1();
doSomething2();
  }


https://reviews.llvm.org/D37140

Files:
  lib/Format/UnwrappedLineFormatter.cpp

Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -283,6 +283,21 @@
 TheLine->First != TheLine->Last) {
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
+if (TheLine->Last->is(tok::l_brace) &&
+TheLine->First != TheLine->Last &&
+TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) { 
+  return Style.AllowShortBlocksOnASingleLine ? tryMergeSimpleBlock(I, E, Limit) : 0;
+}
+if (I[1]->First->is(tok::l_brace) && 
+TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) { 
+  return Style.BraceWrapping.AfterControlStatement ? tryMergeSimpleBlock(I, E, Limit) : 0;
+}
+if (TheLine->First->is(tok::l_brace) &&
+   (TheLine->First == TheLine->Last) &&
+   (I != AnnotatedLines.begin()) &&
+   (I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for))) { 
+  return Style.AllowShortBlocksOnASingleLine ? tryMergeSimpleBlock(I-1, E, Limit) : 0;
+}
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction
  ? tryMergeSimpleBlock(I, E, Limit)
@@ -459,53 +474,74 @@
  

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

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

- Get rid of `RefactoringResult` in favour of different rule types.
- Rename `RefactoringOperation` to `RefactoringRuleContext`.
- Address a couple  more comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D36075

Files:
  include/clang/Basic/LLVM.h
  include/clang/Tooling/Refactoring/AtomicChange.h
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
  include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringRuleContext.h
  include/clang/Tooling/Refactoring/SourceSelectionConstraints.h
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/SourceSelectionConstraints.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- /dev/null
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -0,0 +1,165 @@
+//===- unittest/Tooling/RefactoringTestActionRulesTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ReplacementTest.h"
+#include "RewriterTestContext.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Errc.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace refactoring_action_rules;
+
+namespace {
+
+class RefactoringActionRulesTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+Context.Sources.setMainFileID(
+Context.createInMemoryFile("input.cpp", DefaultCode));
+  }
+
+  RewriterTestContext Context;
+  std::string DefaultCode = std::string(100, 'a');
+};
+
+Expected
+createReplacements(const std::unique_ptr ,
+   RefactoringRuleContext ) {
+  return cast(*Rule).createSourceReplacements(
+  Context);
+}
+
+TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
+  auto ReplaceAWithB =
+  [](std::pair Selection)
+  -> Expected {
+const SourceManager  = Selection.first.getSources();
+SourceLocation Loc = Selection.first.getRange().getBegin().getLocWithOffset(
+Selection.second);
+AtomicChange Change(SM, Loc);
+llvm::Error E = Change.replace(SM, Loc, 1, "b");
+if (E)
+  return std::move(E);
+return AtomicChanges{Change};
+  };
+  class SelectionRequirement : public selection::Requirement {
+  public:
+std::pair
+evaluateSelection(selection::SourceSelectionRange Selection) const {
+  return std::make_pair(Selection, 20);
+}
+  };
+  auto Rule = createRefactoringRule(ReplaceAWithB,
+requiredSelection(SelectionRequirement()));
+
+  // When the requirements are satisifed, the rule's function must be invoked.
+  {
+RefactoringRuleContext Operation(Context.Sources);
+SourceLocation Cursor =
+Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID())
+.getLocWithOffset(10);
+Operation.setSelectionRange({Cursor, Cursor});
+
+Expected ErrorOrResult =
+createReplacements(Rule, Operation);
+ASSERT_FALSE(!ErrorOrResult);
+ASSERT_FALSE(!*ErrorOrResult);
+AtomicChanges Result = std::move(**ErrorOrResult);
+ASSERT_EQ(Result.size(), 1u);
+std::string YAMLString =
+const_cast(Result[0]).toYAMLString();
+
+ASSERT_STREQ("---\n"
+ "Key: 'input.cpp:30'\n"
+ "FilePath:input.cpp\n"
+ "Error:   ''\n"
+ "InsertedHeaders: \n"
+ "RemovedHeaders:  \n"
+ "Replacements:\n" // Extra whitespace here!
+ "  - FilePath:input.cpp\n"
+ "Offset:  30\n"
+ "Length:  1\n"
+ "ReplacementText: b\n"
+ "...\n",
+ YAMLString.c_str());
+  }
+
+  // When one of the requirements is not satisfied, perform should return either
+  // None or a valid diagnostic.
+  {
+RefactoringRuleContext Operation(Context.Sources);
+Expected ErrorOrResult =
+createReplacements(Rule, Operation);
+
+ASSERT_FALSE(!ErrorOrResult);
+Optional Value = std::move(*ErrorOrResult);
+EXPECT_TRUE(!Value);
+  }
+}
+

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked 5 inline comments as done.
arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:33
+///
+///  - requiredSelection: The refactoring function won't be invoked unless the
+///   given selection requirement is satisfied.

ioeric wrote:
> We might want to document supported requirements somewhere else so that we 
> don't need to update this file every time a new requirement is added.
Do you think it should be in Clang's documentation? I can start on a new 
document there but I'd prefer to do it in a separate patch. WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added inline comments.



Comment at: lib/AST/ExprCXX.cpp:979
 
+SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const {
+  TemplateParameterList *List = getTemplateParameterList();

faisalv wrote:
> I think this should return an invalid range if getExplicitCount is 0.
> might assert that when not 0, langleloc does not equal rangleloc.
> 
I think it does return an invalid range in that case. Without explicit template 
parameters, getLAngleLoc and getRAngleLoc return invalid locations. And a range 
of invalid locations is an invalid range. But I suppose that could be asserted 
to make sure.



Comment at: lib/Sema/SemaLambda.cpp:495
+  reinterpret_cast(TParams.begin()),
+  reinterpret_cast(TParams.end()));
+  LSI->NumExplicitTemplateParams = TParams.size();

faisalv wrote:
> ack - avoid reinterpret cast please - why not just stick to Decl* for 
> TemplateParams for now  - and add some fixme's that suggest we should 
> consider refactoring ParseTemplateParameterList to accept a vector of 
> nameddecls - and update this when that gets updated ?
> 
> Perhaps add an assert here that iterates through and checks to make sure each 
> item in this list is some form of a template parameter decl - within an 
> #ifndef NDEBUG block (or your conversion check to NameDecl should suffice?)
Unfortunately `TemplateParameterList::Create` expects an array of `NamedDecl`, 
so there’ll need to be a forced downcast somewhere. By getting that over with 
as soon as possible, any errors will be caught straight away by that assertion 
instead of moving the problem down the line somewhere and making it more 
difficult to see where it went wrong.


https://reviews.llvm.org/D36527



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

[PATCH] D37120: [analyzer] Fix modeling arithmetic

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I guess we'd need more assertions to catch invalid symbols, will have a look.




Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:371-373
+return makeSymExprValNN(
+state, op, lhs.castAs(),
+rhs.castAs(), resultTy);

For now this code would return `UnknownVal` in most cases (pointer is not 
tainted, or not symbolic, or contains an offset), and still construct an 
invalid symbol in the rest of the cases (`makeSymExprValNN` would add the 
number to the pointer symbol, instead of modelling an offset within the 
pointed-to region).

Once D35450 finally lands (sorry for the delays...), it'd return `UnknownVal` 
less often and crash more often.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:375-377
   // FIXME: This only makes sense for comparisons. If we want to, say,
   // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
   // then pack it back into a LocAsInteger.

I guess you're trying to address this FIXME i recently added. This is probably 
the way to go: just take the `Loc` behind the `LocAsInteger`, cast it to `char 
*` (because pointer type shouldn't affect how much bytes of offset are added, 
anymore), add your integer to it, pack back into `LocAsInteger`.


Repository:
  rL LLVM

https://reviews.llvm.org/D37120



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/AST/ExprCXX.cpp:979
 
+SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const {
+  TemplateParameterList *List = getTemplateParameterList();

I think this should return an invalid range if getExplicitCount is 0.
might assert that when not 0, langleloc does not equal rangleloc.




Comment at: lib/Sema/SemaLambda.cpp:486
+ && "Already acted on explicit template parameters");
+  assert(LSI->TemplateParams.size() == 0
+ && "Explicit template parameters should come "

Perhaps also assert TParams.size should not be 0?



Comment at: lib/Sema/SemaLambda.cpp:495
+  reinterpret_cast(TParams.begin()),
+  reinterpret_cast(TParams.end()));
+  LSI->NumExplicitTemplateParams = TParams.size();

ack - avoid reinterpret cast please - why not just stick to Decl* for 
TemplateParams for now  - and add some fixme's that suggest we should consider 
refactoring ParseTemplateParameterList to accept a vector of nameddecls - and 
update this when that gets updated ?

Perhaps add an assert here that iterates through and checks to make sure each 
item in this list is some form of a template parameter decl - within an #ifndef 
NDEBUG block (or your conversion check to NameDecl should suffice?)


https://reviews.llvm.org/D36527



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

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Awesome, thanks!




Comment at: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp:146
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/f/T", 2, 11);
+  Visitor.ExpectMatch("/f/f/", 2, 20);

johannes wrote:
> This test also works before the change; what is the best way to enforce an 
> ordering here?
You can add a flag to `LexicallyOrderedDeclVisitor` that emits the ordering 
index when it's true and turn it on for this test. Then you'd should get 
something like `/f/T@0` and `/f/T@1`.

Please test a class template as well.




https://reviews.llvm.org/D36998



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

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

[Strahinja Petrovic via Phabricator via cfe-commits]

spetrovic added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D34878



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

[PATCH] D36876: [IRGen] Evaluate constant static variables referenced through member expressions

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311772: [IRGen] Evaluate constant static variables 
referenced through member (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D36876?vs=112360=112675#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36876

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprAgg.cpp
  cfe/trunk/lib/CodeGen/CGExprComplex.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/test/CodeGenCXX/member-expr-references-variable.cpp

Index: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp
@@ -124,24 +124,7 @@
   }
 
   // l-values.
-  void VisitDeclRefExpr(DeclRefExpr *E) {
-// For aggregates, we should always be able to emit the variable
-// as an l-value unless it's a reference.  This is due to the fact
-// that we can't actually ever see a normal l2r conversion on an
-// aggregate in C++, and in C there's no language standard
-// actively preventing us from listing variables in the captures
-// list of a block.
-if (E->getDecl()->getType()->isReferenceType()) {
-  if (CodeGenFunction::ConstantEmission result
-= CGF.tryEmitAsConstant(E)) {
-EmitFinalDestCopy(E->getType(), result.getReferenceLValue(CGF, E));
-return;
-  }
-}
-
-EmitAggLoadOfLValue(E);
-  }
-
+  void VisitDeclRefExpr(DeclRefExpr *E) { EmitAggLoadOfLValue(E); }
   void VisitMemberExpr(MemberExpr *ME) { EmitAggLoadOfLValue(ME); }
   void VisitUnaryDeref(UnaryOperator *E) { EmitAggLoadOfLValue(E); }
   void VisitStringLiteral(StringLiteral *E) { EmitAggLoadOfLValue(E); }
Index: cfe/trunk/lib/CodeGen/CGExprComplex.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprComplex.cpp
+++ cfe/trunk/lib/CodeGen/CGExprComplex.cpp
@@ -120,18 +120,22 @@
 return Visit(E->getSubExpr());
   }
 
+  ComplexPairTy emitConstant(const CodeGenFunction::ConstantEmission ,
+ Expr *E) {
+assert(Constant && "not a constant");
+if (Constant.isReference())
+  return EmitLoadOfLValue(Constant.getReferenceLValue(CGF, E),
+  E->getExprLoc());
+
+llvm::Constant *pair = Constant.getValue();
+return ComplexPairTy(pair->getAggregateElement(0U),
+ pair->getAggregateElement(1U));
+  }
 
   // l-values.
   ComplexPairTy VisitDeclRefExpr(DeclRefExpr *E) {
-if (CodeGenFunction::ConstantEmission result = CGF.tryEmitAsConstant(E)) {
-  if (result.isReference())
-return EmitLoadOfLValue(result.getReferenceLValue(CGF, E),
-E->getExprLoc());
-
-  llvm::Constant *pair = result.getValue();
-  return ComplexPairTy(pair->getAggregateElement(0U),
-   pair->getAggregateElement(1U));
-}
+if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E))
+  return emitConstant(Constant, E);
 return EmitLoadOfLValue(E);
   }
   ComplexPairTy VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) {
@@ -141,7 +145,14 @@
 return CGF.EmitObjCMessageExpr(E).getComplexVal();
   }
   ComplexPairTy VisitArraySubscriptExpr(Expr *E) { return EmitLoadOfLValue(E); }
-  ComplexPairTy VisitMemberExpr(const Expr *E) { return EmitLoadOfLValue(E); }
+  ComplexPairTy VisitMemberExpr(MemberExpr *ME) {
+if (CodeGenFunction::ConstantEmission Constant =
+CGF.tryEmitAsConstant(ME)) {
+  CGF.EmitIgnoredExpr(ME->getBase());
+  return emitConstant(Constant, ME);
+}
+return EmitLoadOfLValue(ME);
+  }
   ComplexPairTy VisitOpaqueValueExpr(OpaqueValueExpr *E) {
 if (E->isGLValue())
   return EmitLoadOfLValue(CGF.getOpaqueLValueMapping(E), E->getExprLoc());
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -3161,6 +3161,7 @@
   };
 
   ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr);
+  ConstantEmission tryEmitAsConstant(const MemberExpr *ME);
 
   RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e,
 AggValueSlot slot = AggValueSlot::ignored());
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -1339,6 +1339,25 @@
   return ConstantEmission::forValue(C);
 }
 
+static DeclRefExpr *tryToConvertMemberExprToDeclRefExpr(CodeGenFunction ,
+const MemberExpr *ME) {
+  if (auto *VD = dyn_cast(ME->getMemberDecl())) {
+// Try to emit static variable member expressions as DREs.
+return 

r311772 - [IRGen] Evaluate constant static variables referenced through member

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 25 03:07:00 2017
New Revision: 311772

URL: http://llvm.org/viewvc/llvm-project?rev=311772=rev
Log:
[IRGen] Evaluate constant static variables referenced through member
expressions

C++ allows us to reference static variables through member expressions. Prior to
this commit, non-integer static variables that were referenced using a member
expression were always emitted using lvalue loads. The old behaviour introduced
an inconsistency between regular uses of static variables and member expressions
uses. For example, the following program compiled and linked successfully:

struct Foo {
   constexpr static const char *name = "foo";
};
int main() {
  return Foo::name[0] == 'f';
}

but this program failed to link because "Foo::name" wasn't found:

struct Foo {
   constexpr static const char *name = "foo";
};
int main() {
  Foo f;
  return f.name[0] == 'f';
}

This commit ensures that constant static variables referenced through member
expressions are emitted in the same way as ordinary static variable references.

rdar://33942261

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

Added:
cfe/trunk/test/CodeGenCXX/member-expr-references-variable.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprComplex.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=311772=311771=311772=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Aug 25 03:07:00 2017
@@ -1339,6 +1339,25 @@ CodeGenFunction::tryEmitAsConstant(DeclR
   return ConstantEmission::forValue(C);
 }
 
+static DeclRefExpr *tryToConvertMemberExprToDeclRefExpr(CodeGenFunction ,
+const MemberExpr *ME) {
+  if (auto *VD = dyn_cast(ME->getMemberDecl())) {
+// Try to emit static variable member expressions as DREs.
+return DeclRefExpr::Create(
+CGF.getContext(), NestedNameSpecifierLoc(), SourceLocation(), VD,
+/*RefersToEnclosingVariableOrCapture=*/false, ME->getExprLoc(),
+ME->getType(), ME->getValueKind());
+  }
+  return nullptr;
+}
+
+CodeGenFunction::ConstantEmission
+CodeGenFunction::tryEmitAsConstant(const MemberExpr *ME) {
+  if (DeclRefExpr *DRE = tryToConvertMemberExprToDeclRefExpr(*this, ME))
+return tryEmitAsConstant(DRE);
+  return ConstantEmission();
+}
+
 llvm::Value *CodeGenFunction::EmitLoadOfScalar(LValue lvalue,
SourceLocation Loc) {
   return EmitLoadOfScalar(lvalue.getAddress(), lvalue.isVolatile(),
@@ -3540,6 +3559,11 @@ EmitExtVectorElementExpr(const ExtVector
 }
 
 LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) {
+  if (DeclRefExpr *DRE = tryToConvertMemberExprToDeclRefExpr(*this, E)) {
+EmitIgnoredExpr(E->getBase());
+return EmitDeclRefLValue(DRE);
+  }
+
   Expr *BaseExpr = E->getBase();
   // If this is s.x, emit s as an lvalue.  If it is s->x, emit s as a scalar.
   LValue BaseLV;
@@ -3566,9 +3590,6 @@ LValue CodeGenFunction::EmitMemberExpr(c
 return LV;
   }
 
-  if (auto *VD = dyn_cast(ND))
-return EmitGlobalVarDeclLValue(*this, E, VD);
-
   if (const auto *FD = dyn_cast(ND))
 return EmitFunctionDeclLValue(*this, E, FD);
 

Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=311772=311771=311772=diff
==
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Fri Aug 25 03:07:00 2017
@@ -124,24 +124,7 @@ public:
   }
 
   // l-values.
-  void VisitDeclRefExpr(DeclRefExpr *E) {
-// For aggregates, we should always be able to emit the variable
-// as an l-value unless it's a reference.  This is due to the fact
-// that we can't actually ever see a normal l2r conversion on an
-// aggregate in C++, and in C there's no language standard
-// actively preventing us from listing variables in the captures
-// list of a block.
-if (E->getDecl()->getType()->isReferenceType()) {
-  if (CodeGenFunction::ConstantEmission result
-= CGF.tryEmitAsConstant(E)) {
-EmitFinalDestCopy(E->getType(), result.getReferenceLValue(CGF, E));
-return;
-  }
-}
-
-EmitAggLoadOfLValue(E);
-  }
-
+  void VisitDeclRefExpr(DeclRefExpr *E) { EmitAggLoadOfLValue(E); }
   void VisitMemberExpr(MemberExpr *ME) { EmitAggLoadOfLValue(ME); }
   void VisitUnaryDeref(UnaryOperator *E) { EmitAggLoadOfLValue(E); }
   void VisitStringLiteral(StringLiteral *E) { EmitAggLoadOfLValue(E); }

Modified: cfe/trunk/lib/CodeGen/CGExprComplex.cpp
URL: 

[PATCH] D37003: [clang-diff] Support templates

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 112673.
johannes edited the summary of this revision.
johannes added a comment.

use LexicallyOrderedRecursiveASTVisitor


https://reviews.llvm.org/D37003

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/Inputs/clang-diff-basic-src.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-html.test

Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -1,4 +1,4 @@
-RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- | FileCheck %s
+RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- -std=c++11 | FileCheck %s
 
 CHECK: 
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-diff -dump-matches %S/Inputs/clang-diff-basic-src.cpp %s -- | FileCheck %s
+// RUN: clang-diff -dump-matches %S/Inputs/clang-diff-basic-src.cpp %s -- -std=c++11 | FileCheck %s
 
 // CHECK: Match TranslationUnitDecl{{.*}} to TranslationUnitDecl
 // CHECK: Match NamespaceDecl: src{{.*}} to NamespaceDecl: dst
@@ -68,5 +68,18 @@
   F(1, /*b=*/1);
 }
 
+// CHECK: Update TemplateTypeParmDecl: T{{.*}} to Type
+template 
+U visit(Type ) {
+  int x = t;
+  return U();
+}
+
+void tmp() {
+  long x;
+  // CHECK: Update TemplateArgument: int{{.*}} to long
+  visit(x);
+}
+
 // CHECK: Delete AccessSpecDecl: public
 // CHECK: Delete CXXMethodDecl
Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -92,3 +92,16 @@
 // CHECK-NEXT: FunctionDecl: sentinel
 void sentinel();
 #endif
+
+// CHECK-NEXT: ClassTemplateDecl: C
+// CHECK-NEXT: TemplateTypeParmDecl
+// CHECK-NEXT: CXXRecordDecl
+template  class C {
+  // CHECK-NEXT: FieldDecl
+  T t;
+};
+
+// CHECK-NEXT: CXXRecordDecl
+// CHECK-NEXT: TemplateName
+// CHECK-NEXT: TemplateArgument
+class I : C {};
Index: test/Tooling/Inputs/clang-diff-basic-src.cpp
===
--- test/Tooling/Inputs/clang-diff-basic-src.cpp
+++ test/Tooling/Inputs/clang-diff-basic-src.cpp
@@ -41,3 +41,16 @@
   M1;
   F(1, 1);
 }
+
+template 
+U visit(T ) {
+  int x = t;
+  return U();
+}
+
+void tmp() {
+  int x;
+  visit(x);
+}
+
+int x = 1;
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -15,7 +15,7 @@
 
 #include "clang/AST/DataCollection.h"
 #include "clang/AST/DeclVisitor.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
@@ -149,6 +149,7 @@
   // Maps preorder indices to postorder ones.
   std::vector PostorderIds;
   std::vector NodesBfs;
+  std::map TemplateArgumentLocations;
 
   int getSize() const { return Nodes.size(); }
   NodeId getRootId() const { return 0; }
@@ -186,6 +187,14 @@
 static bool isSpecializedNodeExcluded(CXXCtorInitializer *I) {
   return !I->isWritten();
 }
+static bool isSpecializedNodeExcluded(const TemplateArgumentLoc *S) {
+  return false;
+}
+
+static bool isNodeExcluded(ASTUnit , TemplateName *Template) {
+  // TODO what if it is from another file
+  return false;
+}
 
 template  static bool isNodeExcluded(ASTUnit , T *N) {
   const SourceManager  = AST.getSourceManager();
@@ -203,40 +212,39 @@
   return isSpecializedNodeExcluded(N);
 }
 
-static SourceRange getSourceRange(const ASTUnit , const DynTypedNode ) {
+static SourceRange getSourceExtent(const ASTUnit , SourceRange Range) {
   const SourceManager  = AST.getSourceManager();
-  SourceRange Range = DTN.getSourceRange();
   SourceLocation BeginLoc = Range.getBegin();
   SourceLocation EndLoc;
   if (BeginLoc.isMacroID())
 EndLoc = SrcMgr.getExpansionRange(BeginLoc).second;
   else
 EndLoc = Range.getEnd();
   EndLoc = Lexer::getLocForEndOfToken(EndLoc, /*Offset=*/0, SrcMgr,
   AST.getLangOpts());
-  if (auto *ThisExpr = DTN.get()) {
-if (ThisExpr->isImplicit())
-  EndLoc = BeginLoc;
-  }
   return {SrcMgr.getExpansionLoc(BeginLoc), SrcMgr.getExpansionLoc(EndLoc)};
 }
 
 namespace {
 // Sets Height, Parent and Children for each node.
-struct PreorderVisitor : public RecursiveASTVisitor {
+struct PreorderVisitor
+: public LexicallyOrderedRecursiveASTVisitor {
+  using BaseType = LexicallyOrderedRecursiveASTVisitor;
+
   int Id = 0, Depth = 0;
   NodeId Parent;
   SyntaxTree::Impl 
 
-  PreorderVisitor(SyntaxTree::Impl ) : Tree(Tree) {}
+  

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 112671.
johannes retitled this revision from "[AST] Make 
TraverseTemplateParameterListHelper public" to "[AST] Traverse templates in 
LexicallyOrderedRecursiveASTVisitor".
johannes edited the summary of this revision.
johannes added a reviewer: arphaman.
johannes added a comment.
Herald added a subscriber: klimek.

use LexicallyOrderedRecursiveASTVisitor


https://reviews.llvm.org/D36998

Files:
  include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  include/clang/AST/RecursiveASTVisitor.h
  unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp


Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
===
--- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
+++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
@@ -64,7 +64,7 @@
   OS << ND->getNameAsString();
 else
   OS << "???";
-if (isa(D))
+if (isa(D) or isa(D))
   OS << "/";
   }
   Matcher.match(OS.str(), D);
@@ -138,4 +138,14 @@
   EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
 }
 
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitTemplateDecl) {
+  StringRef Source = R"(
+template  T f();
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/f/T", 2, 11);
+  Visitor.ExpectMatch("/f/f/", 2, 20);
+  EXPECT_TRUE(Visitor.runOver(Source));
+}
+
 } // end anonymous namespace
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -499,10 +499,10 @@
 
   bool canIgnoreChildDeclWhileTraversingDeclContext(const Decl *Child);
 
-private:
   // These are helper methods used by more than one Traverse* method.
   bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
 
+private:
   // Traverses template parameter lists of either a DeclaratorDecl or TagDecl.
   template 
   bool TraverseDeclTemplateParameterLists(T *D);
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
===
--- include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
+++ include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
@@ -111,6 +111,21 @@
 return true;
   }
 
+#define DEF_TRAVERSE_TEMPLATE_DECL(TMPLDECLKIND)   
\
+  bool Traverse##TMPLDECLKIND##TemplateDecl(TMPLDECLKIND##TemplateDecl *TD) {  
\
+if (!BaseType::TraverseTemplateParameterListHelper(
\
+TD->getTemplateParameters()))  
\
+  return false;
\
+assert(!BaseType::getDerived().shouldVisitTemplateInstantiations() &&  
\
+   "It does not make sense for most clients to visit template "
\
+   "instantiations here.");
\
+return BaseType::getDerived().TraverseDecl(TD->getTemplatedDecl());
\
+  }
+  DEF_TRAVERSE_TEMPLATE_DECL(Class)
+  DEF_TRAVERSE_TEMPLATE_DECL(Var)
+  DEF_TRAVERSE_TEMPLATE_DECL(Function)
+#undef DEF_TRAVERSE_TEMPLATE_DECL
+
 private:
   bool TraverseAdditionalLexicallyNestedDeclarations() {
 // FIXME: Ideally the gathered declarations and the declarations in the


Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
===
--- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
+++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
@@ -64,7 +64,7 @@
   OS << ND->getNameAsString();
 else
   OS << "???";
-if (isa(D))
+if (isa(D) or isa(D))
   OS << "/";
   }
   Matcher.match(OS.str(), D);
@@ -138,4 +138,14 @@
   EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
 }
 
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitTemplateDecl) {
+  StringRef Source = R"(
+template  T f();
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/f/T", 2, 11);
+  Visitor.ExpectMatch("/f/f/", 2, 20);
+  EXPECT_TRUE(Visitor.runOver(Source));
+}
+
 } // end anonymous namespace
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -499,10 +499,10 @@
 
   bool canIgnoreChildDeclWhileTraversingDeclContext(const Decl *Child);
 
-private:
   // These are helper methods used by more than one Traverse* method.
   bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
 
+private:
   // Traverses template parameter lists of either a DeclaratorDecl or TagDecl.
   template 
   bool TraverseDeclTemplateParameterLists(T *D);
Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
===
--- 

[PATCH] D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added inline comments.



Comment at: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp:146
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/f/T", 2, 11);
+  Visitor.ExpectMatch("/f/f/", 2, 20);

This test also works before the change; what is the best way to enforce an 
ordering here?


https://reviews.llvm.org/D36998



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

[PATCH] D37000: [clang-diff] Remove NodeCountVisitor, NFC

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311770: [clang-diff] Remove NodeCountVisitor, NFC (authored 
by krobelus).

Changed prior to commit:
  https://reviews.llvm.org/D37000?vs=112132=112670#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37000

Files:
  cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp


Index: cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
+++ cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -127,6 +127,8 @@
 
   SyntaxTree *Parent;
   ASTContext 
+  /// Nodes in preorder.
+  std::vector Nodes;
   std::vector Leaves;
   // Maps preorder indices to postorder ones.
   std::vector PostorderIds;
@@ -155,9 +157,6 @@
   std::string getStmtValue(const Stmt *S) const;
 
 private:
-  /// Nodes in preorder.
-  std::vector Nodes;
-
   void initTree();
   void setLeftMostDescendants();
 };
@@ -182,32 +181,6 @@
 }
 
 namespace {
-/// Counts the number of nodes that will be compared.
-struct NodeCountVisitor : public RecursiveASTVisitor {
-  int Count = 0;
-  const SyntaxTree::Impl 
-  NodeCountVisitor(const SyntaxTree::Impl ) : Tree(Tree) {}
-  bool TraverseDecl(Decl *D) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), D))
-  return true;
-++Count;
-RecursiveASTVisitor::TraverseDecl(D);
-return true;
-  }
-  bool TraverseStmt(Stmt *S) {
-if (S)
-  S = S->IgnoreImplicit();
-if (isNodeExcluded(Tree.AST.getSourceManager(), S))
-  return true;
-++Count;
-RecursiveASTVisitor::TraverseStmt(S);
-return true;
-  }
-  bool TraverseType(QualType T) { return true; }
-};
-} // end anonymous namespace
-
-namespace {
 // Sets Height, Parent and Children for each node.
 struct PreorderVisitor : public RecursiveASTVisitor {
   int Id = 0, Depth = 0;
@@ -218,6 +191,7 @@
 
   template  std::tuple PreTraverse(T *ASTNode) {
 NodeId MyId = Id;
+Tree.Nodes.emplace_back();
 Node  = Tree.getMutableNode(MyId);
 N.Parent = Parent;
 N.Depth = Depth;
@@ -274,19 +248,13 @@
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Decl *N, ASTContext )
 : Parent(Parent), AST(AST) {
-  NodeCountVisitor NodeCounter(*this);
-  NodeCounter.TraverseDecl(N);
-  Nodes.resize(NodeCounter.Count);
   PreorderVisitor PreorderWalker(*this);
   PreorderWalker.TraverseDecl(N);
   initTree();
 }
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Stmt *N, ASTContext )
 : Parent(Parent), AST(AST) {
-  NodeCountVisitor NodeCounter(*this);
-  NodeCounter.TraverseStmt(N);
-  Nodes.resize(NodeCounter.Count);
   PreorderVisitor PreorderWalker(*this);
   PreorderWalker.TraverseStmt(N);
   initTree();


Index: cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
+++ cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -127,6 +127,8 @@
 
   SyntaxTree *Parent;
   ASTContext 
+  /// Nodes in preorder.
+  std::vector Nodes;
   std::vector Leaves;
   // Maps preorder indices to postorder ones.
   std::vector PostorderIds;
@@ -155,9 +157,6 @@
   std::string getStmtValue(const Stmt *S) const;
 
 private:
-  /// Nodes in preorder.
-  std::vector Nodes;
-
   void initTree();
   void setLeftMostDescendants();
 };
@@ -182,32 +181,6 @@
 }
 
 namespace {
-/// Counts the number of nodes that will be compared.
-struct NodeCountVisitor : public RecursiveASTVisitor {
-  int Count = 0;
-  const SyntaxTree::Impl 
-  NodeCountVisitor(const SyntaxTree::Impl ) : Tree(Tree) {}
-  bool TraverseDecl(Decl *D) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), D))
-  return true;
-++Count;
-RecursiveASTVisitor::TraverseDecl(D);
-return true;
-  }
-  bool TraverseStmt(Stmt *S) {
-if (S)
-  S = S->IgnoreImplicit();
-if (isNodeExcluded(Tree.AST.getSourceManager(), S))
-  return true;
-++Count;
-RecursiveASTVisitor::TraverseStmt(S);
-return true;
-  }
-  bool TraverseType(QualType T) { return true; }
-};
-} // end anonymous namespace
-
-namespace {
 // Sets Height, Parent and Children for each node.
 struct PreorderVisitor : public RecursiveASTVisitor {
   int Id = 0, Depth = 0;
@@ -218,6 +191,7 @@
 
   template  std::tuple PreTraverse(T *ASTNode) {
 NodeId MyId = Id;
+Tree.Nodes.emplace_back();
 Node  = Tree.getMutableNode(MyId);
 N.Parent = Parent;
 N.Depth = Depth;
@@ -274,19 +248,13 @@
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Decl *N, ASTContext )
 : Parent(Parent), AST(AST) {
-  NodeCountVisitor NodeCounter(*this);
-  NodeCounter.TraverseDecl(N);
-  Nodes.resize(NodeCounter.Count);
   PreorderVisitor PreorderWalker(*this);
   PreorderWalker.TraverseDecl(N);
   initTree();
 }
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Stmt *N, ASTContext )
 : Parent(Parent), AST(AST) {
-  NodeCountVisitor NodeCounter(*this);
-  NodeCounter.TraverseStmt(N);
-  

r311770 - [clang-diff] Remove NodeCountVisitor, NFC

[Johannes Altmanninger via cfe-commits]

Author: krobelus
Date: Fri Aug 25 02:49:49 2017
New Revision: 311770

URL: http://llvm.org/viewvc/llvm-project?rev=311770=rev
Log:
[clang-diff] Remove NodeCountVisitor, NFC

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp

Modified: cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp?rev=311770=311769=311770=diff
==
--- cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp (original)
+++ cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp Fri Aug 25 02:49:49 2017
@@ -127,6 +127,8 @@ public:
 
   SyntaxTree *Parent;
   ASTContext 
+  /// Nodes in preorder.
+  std::vector Nodes;
   std::vector Leaves;
   // Maps preorder indices to postorder ones.
   std::vector PostorderIds;
@@ -155,9 +157,6 @@ public:
   std::string getStmtValue(const Stmt *S) const;
 
 private:
-  /// Nodes in preorder.
-  std::vector Nodes;
-
   void initTree();
   void setLeftMostDescendants();
 };
@@ -182,32 +181,6 @@ static bool isNodeExcluded(const SourceM
 }
 
 namespace {
-/// Counts the number of nodes that will be compared.
-struct NodeCountVisitor : public RecursiveASTVisitor {
-  int Count = 0;
-  const SyntaxTree::Impl 
-  NodeCountVisitor(const SyntaxTree::Impl ) : Tree(Tree) {}
-  bool TraverseDecl(Decl *D) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), D))
-  return true;
-++Count;
-RecursiveASTVisitor::TraverseDecl(D);
-return true;
-  }
-  bool TraverseStmt(Stmt *S) {
-if (S)
-  S = S->IgnoreImplicit();
-if (isNodeExcluded(Tree.AST.getSourceManager(), S))
-  return true;
-++Count;
-RecursiveASTVisitor::TraverseStmt(S);
-return true;
-  }
-  bool TraverseType(QualType T) { return true; }
-};
-} // end anonymous namespace
-
-namespace {
 // Sets Height, Parent and Children for each node.
 struct PreorderVisitor : public RecursiveASTVisitor {
   int Id = 0, Depth = 0;
@@ -218,6 +191,7 @@ struct PreorderVisitor : public Recursiv
 
   template  std::tuple PreTraverse(T *ASTNode) {
 NodeId MyId = Id;
+Tree.Nodes.emplace_back();
 Node  = Tree.getMutableNode(MyId);
 N.Parent = Parent;
 N.Depth = Depth;
@@ -274,9 +248,6 @@ struct PreorderVisitor : public Recursiv
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Decl *N, ASTContext )
 : Parent(Parent), AST(AST) {
-  NodeCountVisitor NodeCounter(*this);
-  NodeCounter.TraverseDecl(N);
-  Nodes.resize(NodeCounter.Count);
   PreorderVisitor PreorderWalker(*this);
   PreorderWalker.TraverseDecl(N);
   initTree();
@@ -284,9 +255,6 @@ SyntaxTree::Impl::Impl(SyntaxTree *Paren
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Stmt *N, ASTContext )
 : Parent(Parent), AST(AST) {
-  NodeCountVisitor NodeCounter(*this);
-  NodeCounter.TraverseStmt(N);
-  Nodes.resize(NodeCounter.Count);
   PreorderVisitor PreorderWalker(*this);
   PreorderWalker.TraverseStmt(N);
   initTree();


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

[PATCH] D37136: [clang-format] Do not format likely xml

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a subscriber: klimek.

This patch detects the leading '<' in likely xml files and stops formatting in
that case. A recent use of a Qt xml file with a .ts extension triggered this:
http://doc.qt.io/qt-4.8/linguist-ts-file-format.html


https://reviews.llvm.org/D37136

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/SortIncludesTest.cpp


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -398,6 +398,17 @@
   EXPECT_EQ(26u, Ranges[0].getLength());
 }
 
+TEST_F(SortIncludesTest, DoNotSortLikelyXml) {
+  EXPECT_EQ("",
+sort(""));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11217,6 +11217,13 @@
   EXPECT_EQ("auto c = u8'a';", format("auto c = u8'a';"));
 }
 
+TEST_F(FormatTest, DoNotFormatLikelyXml) {
+  EXPECT_EQ("",
+format("", getGoogleStyle()));
+  EXPECT_EQ(" ",
+format(" ", getGoogleStyle()));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1539,14 +1539,19 @@
   return Code.size() > 188 && Code[0] == 0x47 && Code[188] == 0x47;
 }
 
+bool likelyXml(StringRef Code) {
+  return Code.ltrim().startswith("<");
+}
+
 tooling::Replacements sortIncludes(const FormatStyle , StringRef Code,
ArrayRef Ranges,
StringRef FileName, unsigned *Cursor) {
   tooling::Replacements Replaces;
   if (!Style.SortIncludes)
 return Replaces;
-  if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
-  isMpegTS(Code))
+  if (likelyXml(Code) ||
+  (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
+   isMpegTS(Code)))
 return Replaces;
   if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript)
 return sortJavaScriptImports(Style, Code, Ranges, FileName);
@@ -1894,7 +1899,8 @@
   FormatStyle Expanded = expandPresets(Style);
   if (Expanded.DisableFormat)
 return tooling::Replacements();
-  if (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code))
+  if (likelyXml(Code) ||
+  (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code)))
 return tooling::Replacements();
 
   typedef std::function


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -398,6 +398,17 @@
   EXPECT_EQ(26u, Ranges[0].getLength());
 }
 
+TEST_F(SortIncludesTest, DoNotSortLikelyXml) {
+  EXPECT_EQ("",
+sort(""));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11217,6 +11217,13 @@
   EXPECT_EQ("auto c = u8'a';", format("auto c = u8'a';"));
 }
 
+TEST_F(FormatTest, DoNotFormatLikelyXml) {
+  EXPECT_EQ("",
+format("", getGoogleStyle()));
+  EXPECT_EQ(" ",
+format(" ", getGoogleStyle()));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1539,14 +1539,19 @@
   return Code.size() > 188 && Code[0] == 0x47 && Code[188] == 0x47;
 }
 
+bool likelyXml(StringRef Code) {
+  return Code.ltrim().startswith("<");
+}
+
 tooling::Replacements sortIncludes(const FormatStyle , StringRef Code,
ArrayRef Ranges,
StringRef FileName, unsigned *Cursor) {
   tooling::Replacements Replaces;
   if (!Style.SortIncludes)
 return Replaces;
-  if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
-  isMpegTS(Code))
+  if (likelyXml(Code) ||
+  (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
+   isMpegTS(Code)))
 return Replaces;
   if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript)
 return sortJavaScriptImports(Style, Code, Ranges, FileName);
@@ -1894,7 +1899,8 @@
   FormatStyle Expanded = expandPresets(Style);
   if (Expanded.DisableFormat)
 return tooling::Replacements();
-  if (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code))
+  if (likelyXml(Code) ||
+  (Expanded.Language == FormatStyle::LK_JavaScript && 

[PATCH] D37134: [libc++] Rerun ranlib manually after merging the static libraries

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
Herald added a subscriber: mgorny.

In cross build setups, the ar used in merge_archives.py might not be the right 
one for the target (and the invocation doesn't include the 's' modifier for 
updating the archive index); make sure to rebuild the index for the static 
library.


https://reviews.llvm.org/D37134

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -272,6 +272,8 @@
   "$"
   "${MERGE_ARCHIVES_ABI_TARGET}"
   "${MERGE_ARCHIVES_SEARCH_PATHS}"
+COMMAND
+  ${CMAKE_RANLIB} $
 WORKING_DIRECTORY ${LIBCXX_BUILD_DIR}
 )
   endif()


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -272,6 +272,8 @@
   "$"
   "${MERGE_ARCHIVES_ABI_TARGET}"
   "${MERGE_ARCHIVES_SEARCH_PATHS}"
+COMMAND
+  ${CMAKE_RANLIB} $
 WORKING_DIRECTORY ${LIBCXX_BUILD_DIR}
 )
   endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37133: [libc++] Handle object files named *.obj in merge_archives.py

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.

https://reviews.llvm.org/D37133

Files:
  utils/merge_archives.py


Index: utils/merge_archives.py
===
--- utils/merge_archives.py
+++ utils/merge_archives.py
@@ -120,6 +120,8 @@
 
 files = glob.glob(os.path.join(temp_directory_root, '*.o'))
 if not files:
+files = glob.glob(os.path.join(temp_directory_root, '*.obj'))
+if not files:
 print_and_exit('Failed to glob for %s' % glob_path)
 cmd = [ar_exe, '-qc', args.output] + files
 execute_command_verbose(cmd, cwd=temp_directory_root, verbose=args.verbose)


Index: utils/merge_archives.py
===
--- utils/merge_archives.py
+++ utils/merge_archives.py
@@ -120,6 +120,8 @@
 
 files = glob.glob(os.path.join(temp_directory_root, '*.o'))
 if not files:
+files = glob.glob(os.path.join(temp_directory_root, '*.obj'))
+if not files:
 print_and_exit('Failed to glob for %s' % glob_path)
 cmd = [ar_exe, '-qc', args.output] + files
 execute_command_verbose(cmd, cwd=temp_directory_root, verbose=args.verbose)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37120: [analyzer] Fix modeling arithmetic

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thank you for the prompt fix!


Repository:
  rL LLVM

https://reviews.llvm.org/D37120



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

r311767 - Remove the old meeting links. Also available from the link: http://llvm.org/devmtg/

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Fri Aug 25 01:47:57 2017
New Revision: 311767

URL: http://llvm.org/viewvc/llvm-project?rev=311767=rev
Log:
Remove the old meeting links. Also available from the link: 
http://llvm.org/devmtg/

Modified:
cfe/trunk/www/menu.html.incl

Modified: cfe/trunk/www/menu.html.incl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/menu.html.incl?rev=311767=311766=311767=diff
==
--- cfe/trunk/www/menu.html.incl (original)
+++ cfe/trunk/www/menu.html.incl Fri Aug 25 01:47:57 2017
@@ -56,9 +56,7 @@
 
   
 Clang Events
-http://llvm.org/devmtg/2009-10/;>October 2009
-http://llvm.org/devmtg/2010-11/;>November 2010
-http://llvm.org/devmtg/;>LLVM events
+http://llvm.org/devmtg/;>LLVM Meeting
   
 
 


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

r311766 - clang also supports C++14 & 17 + remove trailing whitespaces

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Fri Aug 25 01:44:56 2017
New Revision: 311766

URL: http://llvm.org/viewvc/llvm-project?rev=311766=rev
Log:
clang also supports C++14 & 17 + remove trailing whitespaces

Modified:
cfe/trunk/www/index.html

Modified: cfe/trunk/www/index.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/index.html?rev=311766=311765=311766=diff
==
--- cfe/trunk/www/index.html (original)
+++ cfe/trunk/www/index.html Fri Aug 25 01:44:56 2017
@@ -1,4 +1,4 @@
-http://www.w3.org/TR/html4/strict.dtd;>
 
 
@@ -14,18 +14,18 @@
   
   clang: a C language family frontend for LLVM
   
-  
+
   The goal of the Clang project is to create a new C based language
   front-end: C, C++, Objective C/C++, OpenCL C and others for the
   http://www.llvm.org/;>LLVM compiler.  You can
   get and build the source  today.
-  
+
   
   Features and Goals
   
-  
+
   Some of the goals for the project include the following:
-  
+
   End-User Features:
 
   
@@ -34,7 +34,7 @@
   GCC compatibility
   
 
-  Utility and 
+  Utility and
  Applications:
 
   
@@ -45,7 +45,7 @@
   Use the LLVM 'BSD' License
   
 
-  Internal Design and 
+  Internal Design and
  Implementation:
 
   
@@ -56,60 +56,60 @@
   
 
   Of course this is only a rough outline of the goals and features of
- Clang.  To get a true sense of what it is all about, see the Features section, which breaks
  each of these down and explains them in more detail.
 
- 
+
   
   Why?
   
-  
+
   Development of the new front-end was started out of a need
  for a compiler that allows better diagnostics, better integration with
  IDEs, a license that is compatible with commercial products, and a
  nimble compiler that is easy to develop and maintain.  All of these were
  motivations for starting work on a new front-end that could
  meet these needs.
- 
+
   A good (but quite dated) introduction to Clang can be found in the
  following video lectures:
- 
+
   
 Clang Introduction
 (May 2007)
-Features and Performance of 
+Features and Performance of
 Clang  (July 2007)
   
-  
+
   For a more detailed comparison between Clang and other compilers, please
  see the clang comparison page.
-  
+
   
   Current Status
   
-  
+
   Clang is considered to
-   be a production quality C, Objective-C, C++ and Objective-C++ compiler when 
-   targeting X86-32, X86-64, and ARM (other targets may have caveats, but are 
+   be a production quality C, Objective-C, C++ and Objective-C++ compiler when
+   targeting X86-32, X86-64, and ARM (other targets may have caveats, but are
usually easy to fix).  If you are looking for source analysis or
source-to-source transformation tools, Clang is probably a great
-   solution for you.  Clang supports C++11, please see the C++ status page for more
information.
 
   
   Get it and get involved!
   
-  
+
   Start by getting the code, building it, and
  playing with it.  This will show you the sorts of things we can do
  today and will let you have the "Clang experience" first hand: hopefully
  it will "resonate" with you. :)
-  
+
   Once you've done that, please consider getting
  involved in the clang community.  The Clang developers include 
numerous
- volunteer contributors with a variety of backgrounds.  If you're 
+ volunteer contributors with a variety of backgrounds.  If you're
  interested in
  following the development of Clang, signing up for a mailing list is a 
good
  way to learn about how the project works.


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

[PATCH] D35271: Fix printing policy for AST context loaded from file

[Johann Klähn via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D35271#850472, @vsk wrote:

> Thanks, LGTM! This seems like a pretty straightforward bug fix. Since it's 
> not my usual area maybe it'd be worth waiting a day or so for more feedback.


Thanks! I do not have commit access so it would be great if you could commit it 
on my behalf.


https://reviews.llvm.org/D35271



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