Re: r348276 - [AST][NFC] Make ArrayTypeTraitExpr non polymorphic

[Mikael Holmén via cfe-commits]

Hi,

On 12/11/18 5:49 PM, Bruno Ricci wrote:
> Hi Mikael,
> 
> I can indeed reproduce this with gcc 5.5.0 when doing a Release
> build with assertions. I cannot reproduce this with gcc 6.5.0
> (also with a Release build with assertions), nor can I reproduce
> this with clang 7 (also with a Release build with assertions).
> 
> I tried to instrument StringRefCheckerVisitor::VisitChildren but this
> just causes the stack overflow to move to CGBuilder.
> 
> What is stranger is that the reproducer do not even causes
> an ArrayTypeTraitExpr node to be created.
> 
> Increasing the maximum stack size with ulimit by a small amount fixes
> the problem, so I don't think this is a mis-compilation either.

Oh, I didn't realize it actually passed with a larger stack, I thought 
it recursed forever.

I see now that before the change clang-tidy compiled with gcc 5.4 passed 
with stacksize limited to 2288k and after it needs 9372k.

If I use clang 3.6 to compile I needed 1747k before the change and 1750k 
after.

> 
> I think that this change is correct since no one is using the polymorphism
> of ArrayTypeTraitExpr. Indeed taking the blunt step of making it final do
> not causes any failure.
> 
> The only possible explaination I can think of is that for one reason
> or another, with this particular compiler + build settings, this change
> causes an increase of stack usage which exceeed the default limit ?
> 

Yes perhaps it's just some deficiency in old gcc versions that is 
exposed by this then.

Thanks,
Mikael


> Regards,
> Bruno
> 
> On 11/12/2018 14:13, Mikael Holmén wrote:
>> Hi Bruno,
>>
>> I've no idea if this is really related to your change, but with this
>> commit, the following starts crashing:
>>
>> clang-tidy -allow-enabling-analyzer-alpha-checkers
>> -checks='-*,clang-analyzer-*' ./reduced.c --
>>
>> It seems like it recurses forever for some reason, and then we run out
>> of stack and it crashes:
>>
>> elxhw7c132-n7[llvm]: build-all-bbigcc/bin/clang-tidy
>> -allow-enabling-analyzer-alpha-checkers -checks='-*,clang-analyzer-*'
>> ./reduced.c --
>> #0 0x0074fbda llvm::sys::PrintStackTrace(llvm::raw_ostream&)
>> (build-all-bbigcc/bin/clang-tidy+0x74fbda)
>> #1 0x0074e0aa llvm::sys::RunSignalHandlers()
>> (build-all-bbigcc/bin/clang-tidy+0x74e0aa)
>> #2 0x0074e1d7 SignalHandler(int)
>> (build-all-bbigcc/bin/clang-tidy+0x74e1d7)
>> #3 0x7f6cd7f84330 __restore_rt
>> (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
>> #4 0x00f06c63 clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c63)
>> #5 0x00f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #6 0x00f0527f clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf0527f)
>> #7 0x00f06c68 clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c68)
>> #8 0x00f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #9 0x00f03fcf clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf03fcf)
>> #10 0x00f06c68 clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c68)
>> #11 0x00f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #12 0x00f03fcf clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf03fcf)
>> #13 0x00f06c68 clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (build-all-bbigcc/bin/clang-tidy+0xf06c68)
>> #14 0x00f0a282 (anonymous
>> namespace)::StringRefCheckerVisitor::VisitChildren(clang::Stmt*)
>> (build-all-bbigcc/bin/clang-tidy+0xf0a282)
>> #15 0x00f03fcf clang::StmtVisitorBase> (anonymous namespace)::StringRefCheckerVisitor,
>> void>::Visit(clang::Stmt*) (.part.132)
>> (build-all-bbigcc/bin/clang-tidy+0xf03fcf)
>>
>> etc
>>
>> I've only seen this when I compile clang-tidy with gcc 5.4.0, assertions
>> on, with optimizations. Simply turning on debug info when compiling
>> clang-tidy makes the crash go away so it's quite fragile.
>>
>> Regards,
>> Mikael
>>
>> On 12/4/18 5:01 PM, Bruno Ricci via cfe-commits wrote:
>>> Author: brunoricci
>>> Date: Tue Dec  4 08:01:24 2018
>>> New Revision: 348276
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=348276=rev
>>> Log:
>>> [AST][NFC] Make 

[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

[Eric Christopher via Phabricator via cfe-commits]

echristo accepted this revision.
echristo added a comment.

LGTM. I'm quite a bit happier with this now. Thanks for going through the back 
and forth.


Repository:
  rC Clang

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

https://reviews.llvm.org/D51554



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

[PATCH] D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I'd tried this exact same patch back in https://reviews.llvm.org/D44619, but I 
was running into a bunch of check-clang failures with it, and I was never able 
to figure them out. It looks like it works now though, so I'm glad :) Richard's 
comment from that diff might still be relevant:

> Please add a test to ensure that we still destroy function parameters in the 
> right order and at the right times (for both the exceptional and 
> non-exceptional cleanup cases).

This will also resolve https://bugs.llvm.org/show_bug.cgi?id=36748


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

https://reviews.llvm.org/D55543



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

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai abandoned this revision.
smeenai added a comment.

https://reviews.llvm.org/D55543 does the same thing


Repository:
  rC Clang

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

https://reviews.llvm.org/D44619



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

r348911 - [CodeGen] Fix -DBUILD_SHARED_LIBS=on build after rC348907

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Dec 11 22:07:33 2018
New Revision: 348911

URL: http://llvm.org/viewvc/llvm-project?rev=348911=rev
Log:
[CodeGen] Fix -DBUILD_SHARED_LIBS=on build after rC348907

Modified:
cfe/trunk/lib/CodeGen/CMakeLists.txt

Modified: cfe/trunk/lib/CodeGen/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CMakeLists.txt?rev=348911=348910=348911=diff
==
--- cfe/trunk/lib/CodeGen/CMakeLists.txt (original)
+++ cfe/trunk/lib/CodeGen/CMakeLists.txt Tue Dec 11 22:07:33 2018
@@ -102,4 +102,5 @@ add_clang_library(clangCodeGen
   clangBasic
   clangFrontend
   clangLex
+  clangSerialization
   )


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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: CodeGen/CodeGenModule.cpp:2957-2958
   !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  !getTriple().isWindowsGNUEnvironment() &&
+  !getTriple().isWindowsMSVCEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);

rnk wrote:
> compnerd wrote:
> > rnk wrote:
> > > The condition here of `T.isOSBinFormatCOFF() && 
> > > !T.isWindowsGNUEnvironment() && !T.isWindowsMSVCEnvironment()` is 
> > > essentially equivalent to `T.isWindowsItaniumEnvironment()`. Please 
> > > simplify the condition to that. The other relevant environments are 
> > > Cygnus for Cygwin and CoreCLR, which probably want to follow the MSVC/GNU 
> > > behavior.
> > IIRC, one of the issues was that lldb couldn't deal very well with the 
> > thunks.  The other thing is that there is a small penalty for something 
> > that we know that we are going to redirect through.  However, I think that 
> > if lldb is able to deal with the thunks now, I would be okay with the 
> > penalty to make the behavior more similar to MSVC.  Basically, if lldb 
> > works, lets just get rid of the special behavior for the itanium 
> > environment.
> Can you elaborate on what didn't work in LLDB when the thunks were present? 
> What kind of functionality did you have to exercise to get it to misbehave?
Sure, it was single stepping through an indirected function call.  It would put 
you into the thunk rather than the destination.



Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

rnk wrote:
> compnerd wrote:
> > rnk wrote:
> > > mgrang wrote:
> > > > I had to remove dllimport in this and the next unit test. I am not sure 
> > > > of the wider implications of this change. Maybe controlling this via a 
> > > > flag (like -flto-visibility-public-std) is a better/more localized 
> > > > option?
> > > These are the ones that I think @compnerd really cares about since the 
> > > objc runtime is typically dynamically linked and frequently called. We 
> > > might want to arrange things such that we have a separate codepath for 
> > > ObjC runtime helpers. I'm surprised we don't already have such a code 
> > > path.
> > I think that @theraven would care more about this code path than I.  I 
> > think that this change may be wrong, because the load here is supposed to 
> > be in the ObjC runtime, and this becoming local to the module would break 
> > the global registration.
> We just set dso_local whenever something isn't dllimport, even if it's a lie 
> sometimes. I think everything would work as intended with a linker thunk. 
> It's "true" as far as LLVM is concerned for the purposes of emitting 
> relocations.
Ah, okay, then, this might be okay.  However, the use of `dllimport` here would 
force that the runtime to be called.  Then again it is possible to statically 
link ...


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

https://reviews.llvm.org/D55229



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

[PATCH] D55525: [Driver] Add support for -fembed-bitcode for assembly file

[Saleem Abdulrasool via Phabricator via cfe-commits]

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

Please do clang-format this.  If this is already in the wild, then, I suppose 
that we don't have the option, but, this seems like something that should be 
written by the author of the assembly file.  This is really the same as the 
emission of the emission of the directive for the GNU noexecstack, although, 
there is `--noexecstack`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55525



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

[PATCH] D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Nice catch.  This also fixes problems where there might be cleanups entered by 
`EmitParmDecl`, e.g. in ObjC++ with a parameter of type `struct A { __strong id 
x; }`; could you please add a test for that and verify that the parameter is 
destroyed at an appropriate point in the inlined call?


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

https://reviews.llvm.org/D55543



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:3441
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,

Can we more fundamentally restructure this entire handler so that, if the 
compound assignment's computation result type is an arithmetic type, we just 
promote both operands to that type, do the arithmetic there, and then coerce 
back down?  This is C++, so the LHS type imposes almost no restrictions on the 
RHS type; also, this is Clang, where we support way too many arithmetic types 
for our own good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55413



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

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Eric Fiselier via Phabricator via cfe-commits]

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



Comment at: docs/LibASTMatchersReference.html:2579-2581
+y(x); Matches
+NS::y(x); Doesn't match
+y(42); Doesn't match.

aaron.ballman wrote:
> EricWF wrote:
> > aaron.ballman wrote:
> > > This is not your bug to fix, but it seems the documentation generator is 
> > > stripping the comment markers. This impacts other matchers as well, such 
> > > as `throughUsingDecl()`.
> > I wonder if it strips `/**/` comments. I don't want to get blocked on this.
> > 
> Don't let it block you -- I'm poking at a fix currently, but this is not your 
> bug.
Thanks for the fix!


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

https://reviews.llvm.org/D55534



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

r348907 - Move PCHContainerOperations from Frontend to Serialization

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Tue Dec 11 18:53:59 2018
New Revision: 348907

URL: http://llvm.org/viewvc/llvm-project?rev=348907=rev
Log:
Move PCHContainerOperations from Frontend to Serialization

Fix a layering violation.  Frontend depends on Serialization, so anything used
by both should be in Serialization.

Added:
cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
cfe/trunk/include/clang/Serialization/PCHContainerOperations.h
  - copied, changed from r348906, 
cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
cfe/trunk/lib/Serialization/PCHContainerOperations.cpp
  - copied, changed from r348906, 
cfe/trunk/lib/Frontend/PCHContainerOperations.cpp
Removed:
cfe/trunk/lib/Frontend/PCHContainerOperations.cpp
Modified:
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/include/clang/module.modulemap
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CMakeLists.txt
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/CMakeLists.txt
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp

Added: cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PCHContainerOperations.h?rev=348907=auto
==
--- cfe/trunk/include/clang/Frontend/PCHContainerOperations.h (added)
+++ cfe/trunk/include/clang/Frontend/PCHContainerOperations.h Tue Dec 11 
18:53:59 2018
@@ -0,0 +1,15 @@
+//===--- Frontend/PCHContainerOperations.h - PCH Containers -*- 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_PCH_CONTAINER_OPERATIONS_H
+#define LLVM_CLANG_PCH_CONTAINER_OPERATIONS_H
+
+#include "clang/Serialization/PCHContainerOperations.h"
+
+#endif

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=348907=348906=348907=diff
==
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Tue Dec 11 18:53:59 2018
@@ -26,10 +26,10 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Sema/SemaConsumer.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTDeserializationListener.h"
+#include "clang/Serialization/PCHContainerOperations.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"

Copied: cfe/trunk/include/clang/Serialization/PCHContainerOperations.h (from 
r348906, cfe/trunk/include/clang/Frontend/PCHContainerOperations.h)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/PCHContainerOperations.h?p2=cfe/trunk/include/clang/Serialization/PCHContainerOperations.h=cfe/trunk/include/clang/Frontend/PCHContainerOperations.h=348906=348907=348907=diff
==
--- cfe/trunk/include/clang/Frontend/PCHContainerOperations.h (original)
+++ cfe/trunk/include/clang/Serialization/PCHContainerOperations.h Tue Dec 11 
18:53:59 2018
@@ -1,4 +1,4 @@
-//===--- Frontend/PCHContainerOperations.h - PCH Containers -*- C++ 
-*-===//
+//===--- Serialization/PCHContainerOperations.h - PCH Containers --*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -7,8 +7,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_PCH_CONTAINER_OPERATIONS_H
-#define LLVM_CLANG_PCH_CONTAINER_OPERATIONS_H
+#ifndef LLVM_CLANG_SERIALIZATION_PCHCONTAINEROPERATIONS_H
+#define LLVM_CLANG_SERIALIZATION_PCHCONTAINEROPERATIONS_H
 
 #include "clang/Basic/Module.h"
 #include "llvm/ADT/SmallVector.h"

Modified: cfe/trunk/include/clang/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/module.modulemap?rev=348907=348906=348907=diff
==
--- cfe/trunk/include/clang/module.modulemap (original)
+++ cfe/trunk/include/clang/module.modulemap Tue Dec 11 18:53:59 2018
@@ -103,9 +103,6 @@ module Clang_Frontend {
   textual header "Frontend/LangStandards.def"
 
   module * { export * }
-
-  // FIXME: This violates layers.
-  exclude header "Frontend/PCHContainerOperations.h"
 }
 
 module Clang_FrontendTool { requires cplusplus umbrella "FrontendTool" module 
* { export * } }

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

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

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



Comment at: docs/ReleaseNotes.rst:139
+
+  Warns for cases when pointer is cast and the pointed to type is
+  incompatible with allocated memory area type. This may lead to access memory

Eugene.Zelenko wrote:
> Please wrap up to 80 characters. Same in documentation.
Lines added by me are not longer than 80 characters.
It is also true for the documentation.



Comment at: docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst:8
+allocated type.
+For example char vs integer, long vs char etc.
+Also warns for cases when the pointed to type layout is different from the 

Eugene.Zelenko wrote:
> Integer -> int. Please highlight types with ``.
Done.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

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

hgabii updated this revision to Diff 177812.
hgabii added a comment.

Highlight type names in documentation.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/IncorrectPointerCastCheck.cpp
  clang-tidy/misc/IncorrectPointerCastCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst
  test/clang-tidy/misc-incorrect-pointer-cast.cpp

Index: test/clang-tidy/misc-incorrect-pointer-cast.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-incorrect-pointer-cast.cpp
@@ -0,0 +1,201 @@
+// RUN: %check_clang_tidy %s misc-incorrect-pointer-cast %t -- \
+// RUN:   -config="{CheckOptions: [{key: misc-incorrect-pointer-cast.WarnForDifferentSignedness, value: 1}]}" --
+
+char __attribute__((aligned(4))) a[16];
+
+struct S0 {
+  char a[16];
+};
+
+struct S01 {
+  char __attribute__((aligned(4))) a[16];
+  struct S0 __attribute__((aligned(4))) s0;
+};
+
+struct S1 {
+  int a;
+  int b;
+};
+
+struct S2 {
+  int a;
+};
+
+struct S3 {
+  int a;
+  double b;
+};
+
+struct S4 {
+  int x;
+  double y;
+};
+
+struct S5 {
+  double y;
+  int x;
+};
+
+struct __attribute__((aligned(16))) SAligned {
+  char buffer[16];
+};
+
+void initDouble(double *d) {
+  *d = 0.5;
+}
+
+void castCharToInt(void) {
+  char c = 'x';
+  int *i = (int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'char' to 'int' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castCharToSort() {
+  char c;
+  short *i = (short *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'char' to 'short' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castShortToInt() {
+  short s;
+  int *i = (int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'short' to 'int' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castWideCharToLong() {
+  wchar_t wc;
+  long *f = (long *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: cast from 'wchar_t' to 'long' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castFloatToDouble() {
+  float f;
+  initDouble((double *));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'float' to 'double' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castToS2(char *data, unsigned offset) {
+  struct S2 *tmp;
+  struct S2 header;
+
+  tmp = (struct S2 *)(data + offset);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: cast from 'char' to 'struct S2' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castS3ToS1() {
+  struct S3 s3;
+  struct S1 *s1 = (struct S1 *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: cast from 'struct S3' to 'struct S1' may lead to access memory based on invalid memory layout; struct members are incompatible [misc-incorrect-pointer-cast]
+}
+
+void castS4ToS5() {
+  struct S4 s4;
+  struct S5 *s5 = (struct S5 *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: cast from 'struct S4' to 'struct S5' may lead to access memory based on invalid memory layout; struct members are incompatible [misc-incorrect-pointer-cast]
+}
+
+void castULongToLong() {
+  unsigned long ul;
+  long *l = (long *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: cast from 'unsigned long' to 'long' may lead to access memory based on invalid memory layout; different signedness types [misc-incorrect-pointer-cast]
+}
+
+void castIntToUInt() {
+  int i;
+  unsigned int *ui = (unsigned int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: cast from 'int' to 'unsigned int' may lead to access memory based on invalid memory layout; different signedness types [misc-incorrect-pointer-cast]
+}
+
+void castToAlignedStruct(char *P) {
+  struct SAligned *a = (struct SAligned *)P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: cast from 'char' to 'struct SAligned' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castCharToIntWithReinterpretCast(void) {
+  char c = 'x';
+  int *i = reinterpret_cast();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'char' to 'int' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void TestDifferentAlignment() {
+
+ 

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

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

hgabii added a comment.

Check for different size and alignment as well.
Tests migrated from D33826 .
Warning in case of `reinterpret_cast`, but options is added to be able to turn 
it off.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:139
+
+  Warns for cases when pointer is cast and the pointed to type is
+  incompatible with allocated memory area type. This may lead to access memory

Please wrap up to 80 characters. Same in documentation.



Comment at: docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst:8
+allocated type.
+For example char vs integer, long vs char etc.
+Also warns for cases when the pointed to type layout is different from the 

Integer -> int. Please highlight types with ``.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

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

hgabii updated this revision to Diff 177810.
hgabii marked an inline comment as done.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/IncorrectPointerCastCheck.cpp
  clang-tidy/misc/IncorrectPointerCastCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst
  test/clang-tidy/misc-incorrect-pointer-cast.cpp

Index: test/clang-tidy/misc-incorrect-pointer-cast.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-incorrect-pointer-cast.cpp
@@ -0,0 +1,201 @@
+// RUN: %check_clang_tidy %s misc-incorrect-pointer-cast %t -- \
+// RUN:   -config="{CheckOptions: [{key: misc-incorrect-pointer-cast.WarnForDifferentSignedness, value: 1}]}" --
+
+char __attribute__((aligned(4))) a[16];
+
+struct S0 {
+  char a[16];
+};
+
+struct S01 {
+  char __attribute__((aligned(4))) a[16];
+  struct S0 __attribute__((aligned(4))) s0;
+};
+
+struct S1 {
+  int a;
+  int b;
+};
+
+struct S2 {
+  int a;
+};
+
+struct S3 {
+  int a;
+  double b;
+};
+
+struct S4 {
+  int x;
+  double y;
+};
+
+struct S5 {
+  double y;
+  int x;
+};
+
+struct __attribute__((aligned(16))) SAligned {
+  char buffer[16];
+};
+
+void initDouble(double *d) {
+  *d = 0.5;
+}
+
+void castCharToInt(void) {
+  char c = 'x';
+  int *i = (int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'char' to 'int' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castCharToSort() {
+  char c;
+  short *i = (short *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'char' to 'short' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castShortToInt() {
+  short s;
+  int *i = (int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'short' to 'int' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castWideCharToLong() {
+  wchar_t wc;
+  long *f = (long *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: cast from 'wchar_t' to 'long' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castFloatToDouble() {
+  float f;
+  initDouble((double *));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'float' to 'double' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castToS2(char *data, unsigned offset) {
+  struct S2 *tmp;
+  struct S2 header;
+
+  tmp = (struct S2 *)(data + offset);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: cast from 'char' to 'struct S2' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castS3ToS1() {
+  struct S3 s3;
+  struct S1 *s1 = (struct S1 *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: cast from 'struct S3' to 'struct S1' may lead to access memory based on invalid memory layout; struct members are incompatible [misc-incorrect-pointer-cast]
+}
+
+void castS4ToS5() {
+  struct S4 s4;
+  struct S5 *s5 = (struct S5 *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: cast from 'struct S4' to 'struct S5' may lead to access memory based on invalid memory layout; struct members are incompatible [misc-incorrect-pointer-cast]
+}
+
+void castULongToLong() {
+  unsigned long ul;
+  long *l = (long *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: cast from 'unsigned long' to 'long' may lead to access memory based on invalid memory layout; different signedness types [misc-incorrect-pointer-cast]
+}
+
+void castIntToUInt() {
+  int i;
+  unsigned int *ui = (unsigned int *)
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: cast from 'int' to 'unsigned int' may lead to access memory based on invalid memory layout; different signedness types [misc-incorrect-pointer-cast]
+}
+
+void castToAlignedStruct(char *P) {
+  struct SAligned *a = (struct SAligned *)P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: cast from 'char' to 'struct SAligned' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void castCharToIntWithReinterpretCast(void) {
+  char c = 'x';
+  int *i = reinterpret_cast();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'char' to 'int' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast]
+}
+
+void TestDifferentAlignment() {
+
+  struct S01 s;
+  int 

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked 3 inline comments as done.
hubert.reinterpretcast added inline comments.



Comment at: lib/AST/ExprConstant.cpp:6147-6148
+  return ZeroInitialization(E);
+if (!Result.checkNullPointerForFoldAccess(Info, E, AK_Read))
+  return false;
+QualType CharTy =

rsmith wrote:
> hubert.reinterpretcast wrote:
> > rsmith wrote:
> > > Why do we need to do this explicitly, rather than allowing to simply 
> > > happen as part of the first `handleLValueToRValueConversion` below?
> > We want the error message to be produced even if we bail early on not 
> > having a valid designator.
> If we have an invalid designator, we already produced a diagnostic when 
> setting it invalid, and generally don't need to diagnose anything else.
The null pointer diagnostic does not get produced if we do not do the check 
here on cases like the following:
```
static_assert(__builtin_memchr((int *)0, 0, 1) == 0, "");
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D55510



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

[PATCH] D54604: Automatic variable initialization

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

rsmith added a comment.

For the record: I'm OK with this direction. (I somewhat prefer removing the 
`-enable-long-wordy-thing` option and instead automatically disabling the 
`zero` option for clang release builds, but I'm OK with either approach.)




Comment at: include/clang/Basic/Attr.td:3134-3142
+
+def TrivialAutoInit : InheritableAttr {
+  let Spellings = [Clang<"trivial_auto_init", 1>];
+  let Args = [EnumArgument<"TrivialAutoVarInit", "TrivialAutoVarInitKind",
+   ["uninitialized", "zero", "pattern"],
+   ["Uninitialized", "Zero", "Pattern"]>];
+  let Subjects = SubjectList<[LocalVar]>;

I'm unconvinced that we should add this attribute; it seems insufficiently 
motivated to me, and harmful to portability. I think we should have some way of 
saying "leave this uninitialized", but zero- and pattern-initialization really 
seem like things that should simply be written in the source code in the normal 
way.

I think it would be reasonable to have a `__uninitialized__` keyword or similar 
that can be used as the initializer of an explicitly-uninitialized variable. If 
you want an attribute for this (for compatibility with other compilers), 
`[[clang::uninitialized]]` also seems reasonable. But absent a strong 
motivation, I do not think we should have an attribute that specifies a 
particular initial value.



Comment at: include/clang/Basic/AttrDocs.td:3710-3714
+The command-line parameter ``-ftrivial-auto-var-init=*`` can be used to
+automatically initialize trivial automatic stack variables. By default, trivial
+automatic stack variables are uninitialized. This attribute is used to override
+the command-line parameter, and accepts a single parameter that must be one of
+the following: ``uninitialized``, ``zero``, or ``pattern``.

If we keep the attribute, we should not document the `zero` mode. We don't need 
to make it discoverable.



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:409-412
+def warn_drv_trivial_auto_var_init_zero_disabled : Warning<
+  "-ftrivial-auto-var-init=zero hasn't been enabled, using pattern 
initialization instead. "
+  "Enable it at your own peril for benchmarking purpose only with "
+  "-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">;

I think this should be an error instead of a warning. We shouldn't silently do 
something different from what was requested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604



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

Re: r346789 - DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

[David Blaikie via cfe-commits]

(tiny factoid: turns out gold's 64 bit gdb-index support is also broken in
a different, more subtle way... - so best folks don't use this flag except
with lld, it seems, if you're using gdb-index)

On Tue, Nov 13, 2018 at 12:10 PM David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: dblaikie
> Date: Tue Nov 13 12:08:13 2018
> New Revision: 346789
>
> URL: http://llvm.org/viewvc/llvm-project?rev=346789=rev
> Log:
> DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier
> use.
>
> Summary:
> This saves a lot of relocations in optimized object files (at the cost
> of some cost/increase in linked executable bytes), but gold's 32 bit
> gdb-index support has a bug (
> https://sourceware.org/bugzilla/show_bug.cgi?id=21894 ) so we can't
> switch to this unconditionally. (& even if it weren't for that bug, one
> might argue that some users would want to optimize in one direction or
> the other - prioritizing object size or linked executable size)
>
> Differential Revision: https://reviews.llvm.org/D54243
>
> Added:
> cfe/trunk/test/CodeGen/debug-info-ranges-base-address.c
> Modified:
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/test/Driver/debug-options.c
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=346789=346788=346789=diff
>
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Tue Nov 13 12:08:13 2018
> @@ -1780,6 +1780,10 @@ def fdebug_types_section: Flag <["-"], "
>Flags<[CC1Option]>, HelpText<"Place debug types in their own section
> (ELF Only)">;
>  def fno_debug_types_section: Flag<["-"], "fno-debug-types-section">,
> Group,
>Flags<[CC1Option]>;
> +def fdebug_ranges_base_address: Flag <["-"],
> "fdebug-ranges-base-address">, Group,
> +  Flags<[CC1Option]>, HelpText<"Use DWARF base address selection entries
> in debug_ranges">;
> +def fno_debug_ranges_base_address: Flag <["-"],
> "fno-debug-ranges-base-address">, Group,
> +  Flags<[CC1Option]>;
>  def fsplit_dwarf_inlining: Flag <["-"], "fsplit-dwarf-inlining">,
> Group,
>Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the
> object/executable to facilitate online symbolication/stack traces in the
> absence of .dwo/.dwp files when using Split DWARF">;
>  def fno_split_dwarf_inlining: Flag<["-"], "fno-split-dwarf-inlining">,
> Group,
>
> Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=346789=346788=346789=diff
>
> ==
> --- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
> +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Tue Nov 13
> 12:08:13 2018
> @@ -331,6 +331,9 @@ CODEGENOPT(PreserveVec3Type, 1, 0)
>  /// Whether to emit .debug_gnu_pubnames section instead of
> .debug_pubnames.
>  CODEGENOPT(DebugNameTable, 2, 0)
>
> +/// Whether to use DWARF base address specifiers in .debug_ranges.
> +CODEGENOPT(DebugRangesBaseAddress, 1, 0)
> +
>  CODEGENOPT(NoPLT, 1, 0)
>
>  /// Whether to embed source in DWARF debug line section.
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=346789=346788=346789=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Nov 13 12:08:13 2018
> @@ -586,7 +586,8 @@ void CGDebugInfo::CreateCompileUnit() {
>CGM.getTarget().getTriple().isNVPTX()
>? llvm::DICompileUnit::DebugNameTableKind::None
>: static_cast(
> -CGOpts.DebugNameTable));
> +CGOpts.DebugNameTable),
> +  CGOpts.DebugRangesBaseAddress);
>  }
>
>  llvm::DIType *CGDebugInfo::CreateType(const BuiltinType *BT) {
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=346789=346788=346789=diff
>
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Nov 13 12:08:13 2018
> @@ -3188,6 +3188,11 @@ static void RenderDebugOptions(const Too
>  ? "-gpubnames"
>  : "-ggnu-pubnames");
>
> +  if (Args.hasFlag(options::OPT_fdebug_ranges_base_address,
> +   

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai marked an inline comment as done.
vsapsai added a comment.

In D48753#1323248 , @ldionne wrote:

> 2. Also, before this patch, the allocator's `construct` and `destroy` were 
> NEVER called in C++03, but were called when available in C++11. After this 
> patch, they are called when available in C++03 and in C++11. Is this correct?
>
>   If (2) is true, then I believe this patch is a worthwhile improvement in 
> terms of quality-of-implementation, even though it's not mandated by the 
> spec. People do have custom allocators, and this behavior change between 
> C++03 and C++11 is quite subtle and surprising.


The described situation is correct. And I agree it can be surprising and tricky.

> 1. Just to make sure I understand; this patch has nothing to do with 
> https://reviews.llvm.org/D48342, they are entirely orthogonal. Is this 
> correct?

They are not entirely orthogonal in sense that https://reviews.llvm.org/D48342 
doesn't account for difference in allocator behaviour between C++03 and C++11. 
So it is better to have uniform `construct` behaviour before using it in 
implementing other fixes.


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

https://reviews.llvm.org/D48753



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

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: libcxx/include/memory:1726
+#else  // _LIBCPP_HAS_NO_VARIADICS
+template 
+_LIBCPP_INLINE_VISIBILITY

vsapsai wrote:
> ldionne wrote:
> > Here,
> > 
> > ```
> > template  > enable_if<__has_construct::value>::type>
> > static void __construct(...);
> > ```
> > 
> > and then below
> > 
> > ```
> > template  > enable_if::value>::type>
> > static void __construct(...);
> > ```
> > 
> > This way you don't have to call `__has_construct` at the point of call and 
> > `__construct` is slightly more reusable.
> > 
> I've tried that (also updated how `static void construct` calls 
> `__construct`) and it works with C++2a but fails with C++03. The error is
> 
> ```
> llvm-project/libcxx/include/memory:1725:21: error: class member cannot be 
> redeclared
> static void __construct(allocator_type&, _Tp* __p, const _A0& __a0)
> ^
> llvm-project/libcxx/include/memory:1720:21: note: previous definition is here
> static void __construct(allocator_type& __a, _Tp* __p, const _A0& 
> __a0)
> ^
> ```
> 
> I haven't investigated further yet.
FWIW, I don't have a strong opinion here. I think Volodymyr's way instantiates 
slightly fewer intermediate types. It's also consistent with the definition of 
`__destroy` right below it. I don't know who we expect to be "reusing" 
`allocator_traits::__construct`.

OTOH, in this case I don't see any concrete benefit to making the dispatch 
explicit at the callsite. (Whereas in D49317 (Ctrl+F "Got it") I needed the 
dispatch to be explicit at the callsite.) So refactoring `__construct` would be 
reasonable, although perhaps slower, and open a can of worms as to whether 
`__destroy` should also be refactored.




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

https://reviews.llvm.org/D48753



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

Buildbot numbers for the week of 12/02/2018 - 12/08/2018

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 12/02/2018 -
12/08/2018.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
 buildername |  was_red
-+--
 clang-cmake-aarch64-lld | 102:41:51
 clang-cmake-aarch64-full| 85:35:58
 clang-s390x-linux-multistage| 81:37:42
 sanitizer-ppc64le-linux | 81:21:11
 clang-cmake-armv8-lld   | 44:20:10
 llvm-clang-x86_64-expensive-checks-win  | 25:00:13
 lldb-amd64-ninja-netbsd8| 21:15:33
 clang-cmake-x86_64-avx2-linux-perf  | 12:36:58
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 11:39:26
 clang-ppc64le-linux-multistage  | 11:21:40
 lld-perf-testsuite  | 09:59:15
 clang-lld-x86_64-2stage | 08:55:08
 clang-s390x-linux   | 08:23:58
 clang-cmake-armv7-selfhost  | 08:22:52
 clang-s390x-linux-lnt   | 08:09:59
 clang-cmake-armv7-full  | 08:07:22
 clang-with-lto-ubuntu   | 07:57:51
 clang-ppc64le-linux-lnt | 07:39:29
 clang-ppc64be-linux-multistage  | 07:29:18
 clang-cmake-armv8-full  | 07:03:49
 clang-cmake-thumbv8-full-sh | 06:59:56
 clang-ppc64be-linux-lnt | 06:59:54
 clang-cmake-thumbv7-full-sh | 06:59:33
 clang-cmake-armv8-selfhost-neon | 06:37:21
 clang-ppc64le-linux | 06:29:49
 clang-ppc64be-linux | 06:07:38
 clang-cmake-armv7-selfhost-neon | 05:48:49
 sanitizer-x86_64-linux-bootstrap| 05:32:37
 sanitizer-x86_64-linux-fast | 05:28:23
 clang-with-thin-lto-ubuntu  | 05:04:45
 sanitizer-x86_64-linux  | 05:02:25
 clang-bpf-build | 04:52:15
 clang-cmake-armv7-global-isel   | 04:30:51
 polly-amd64-linux   | 03:58:39
 clang-x86_64-linux-abi-test | 03:45:39
 clang-cmake-armv7-lnt   | 03:15:19
 clang-hexagon-elf   | 03:15:02
 lld-x86_64-darwin13 | 03:07:19
 clang-cmake-armv8-quick | 03:01:20
 clang-cmake-aarch64-quick   | 02:55:04
 sanitizer-ppc64be-linux | 02:54:08
 clang-cuda-build| 02:50:52
 clang-cmake-armv8-lnt   | 02:50:30
 sanitizer-x86_64-linux-bootstrap-ubsan  | 02:49:25
 clang-cmake-aarch64-global-isel | 02:31:24
 lldb-x86_64-ubuntu-14.04-buildserver| 02:29:06
 sanitizer-x86_64-linux-bootstrap-msan   | 02:04:02
 clang-x86_64-debian-fast| 01:41:29
 clang-cmake-x86_64-avx2-linux   | 01:36:45
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions | 01:36:13
 llvm-hexagon-elf| 01:21:33
 clang-cmake-armv7-quick | 01:18:04
 clang-cmake-armv8-global-isel   | 01:11:35
 sanitizer-x86_64-linux-android  | 01:09:18
 sanitizer-windows   | 00:52:58
 clang-cmake-x86_64-sde-avx512-linux | 00:50:21
 clang-atom-d525-fedora-rel  | 00:47:30
 polly-arm-linux | 00:37:38
 sanitizer-x86_64-linux-fuzzer   | 00:36:26
 sanitizer-x86_64-linux-autoconf | 00:31:24
 lld-x86_64-freebsd  | 00:27:34
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 00:26:20
 clang-armv7-linux-build-cache   | 00:25:31
 clang-aarch64-linux-build-cache | 00:23:05
 clang-sphinx-docs   | 00:11:32
(65 rows)


"Status change ratio" by active builder (percent of builds that 

Buildbot numbers for the week of 11/25/2018 - 12/01/2018

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 11/25/2018 - 12/01/2018.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 clang-tools-sphinx-docs | 57:51:26
 libcxx-libcxxabi-singlethreaded-x86_64-linux-debian | 46:16:24
 libcxx-libcxxabi-x86_64-linux-debian| 45:53:54
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions   | 45:30:27
 libcxx-libcxxabi-libunwind-x86_64-linux-debian  | 45:07:49
 llvm-clang-x86_64-expensive-checks-win  | 41:40:02
 clang-x86_64-debian-fast| 36:34:20
 openmp-gcc-x86_64-linux-debian  | 30:35:36
 openmp-clang-x86_64-linux-debian| 30:20:30
 clang-cmake-thumbv8-full-sh | 27:45:12
 clang-cmake-thumbv7-full-sh | 26:00:40
 clang-cmake-armv8-lld   | 25:25:08
 lldb-amd64-ninja-netbsd8| 25:05:09
 clang-with-lto-ubuntu   | 24:58:28
 clang-with-thin-lto-ubuntu  | 24:23:57
 aosp-O3-polly-before-vectorizer-unprofitable| 23:51:15
 clang-cmake-armv7-global-isel   | 23:13:43
 clang-cmake-armv8-global-isel   | 22:27:33
 clang-cmake-armv7-full  | 22:18:56
 clang-cmake-armv8-full  | 22:17:52
 clang-cmake-armv8-quick | 21:56:16
 clang-cmake-armv7-quick | 21:52:13
 clang-x86_64-linux-abi-test | 21:05:33
 clang-lld-x86_64-2stage | 19:19:32
 clang-cmake-aarch64-full| 17:34:39
 clang-cmake-armv8-selfhost-neon | 17:22:31
 sanitizer-x86_64-linux-fast | 16:53:44
 clang-hexagon-elf   | 16:53:38
 clang-cmake-armv7-selfhost  | 16:34:43
 clang-cmake-armv7-selfhost-neon | 15:38:56
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 14:46:08
 clang-cmake-aarch64-lld | 10:45:41
 sanitizer-x86_64-linux-bootstrap| 09:05:16
 sanitizer-x86_64-linux  | 07:25:09
 sanitizer-x86_64-linux-bootstrap-msan   | 07:03:30
 clang-s390x-linux-multistage| 06:55:56
 sanitizer-x86_64-linux-bootstrap-ubsan  | 06:01:10
 clang-s390x-linux-lnt   | 05:59:08
 clang-cmake-aarch64-global-isel | 05:57:13
 clang-s390x-linux   | 05:49:36
 clang-cmake-x86_64-avx2-linux   | 05:40:06
 clang-ppc64le-linux-multistage  | 04:59:38
 clang-cmake-aarch64-quick   | 04:47:49
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit  | 04:21:37
 clang-x64-windows-msvc  | 04:21:03
 sanitizer-windows   | 03:54:01
 sanitizer-ppc64be-linux | 03:37:08
 clang-cmake-x86_64-avx2-linux-perf  | 03:28:59
 lldb-x86_64-ubuntu-14.04-buildserver| 03:24:56
 clang-ppc64le-linux-lnt | 03:23:39
 sanitizer-ppc64le-linux | 03:21:41
 libcxx-libcxxabi-libunwind-armv8-linux  | 03:10:26
 libcxx-libcxxabi-libunwind-aarch64-linux| 03:09:03
 clang-cuda-build| 03:07:24
 clang-ppc64be-linux-multistage  | 03:07:03
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions | 03:04:51
 clang-cmake-x86_64-sde-avx512-linux | 03:04:22
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions   | 03:03:45
 clang-cmake-armv7-lnt   | 02:59:31
 clang-aarch64-linux-build-cache | 02:57:43
 clang-atom-d525-fedora-rel  | 02:55:38
 clang-armv7-linux-build-cache   | 02:50:37
 lld-perf-testsuite 

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai marked 3 inline comments as done.
vsapsai added a comment.

Regarding the tests. I've moved most of new tests for custom allocators to 
test/libcxx/*. And in destroy.pass.cpp I'm just checking `_LIBCPP_VERSION` to 
avoid copying the test to a different file.




Comment at: libcxx/include/memory:1466
+template 
+struct __has_construct<_Alloc, _Pointer, _Tp, typename enable_if<
+is_same

ldionne wrote:
> You can just say
> 
> ```
> template 
> struct __has_construct<_Alloc, _Pointer, _Tp, typename __void_t<
> decltype(_VSTD::declval<_Alloc>().construct(_VSTD::declval<_Pointer>(), 
> _VSTD::declval<_Tp>()))
> >::type> : std::true_type { };
> ```
Done. One of the reasons to use `is_same` was to mimic `__has_construct` for 
C++11 and later. But I don't think there is really value in keeping that.



Comment at: libcxx/include/memory:1726
+#else  // _LIBCPP_HAS_NO_VARIADICS
+template 
+_LIBCPP_INLINE_VISIBILITY

ldionne wrote:
> Here,
> 
> ```
> template  enable_if<__has_construct::value>::type>
> static void __construct(...);
> ```
> 
> and then below
> 
> ```
> template  enable_if::value>::type>
> static void __construct(...);
> ```
> 
> This way you don't have to call `__has_construct` at the point of call and 
> `__construct` is slightly more reusable.
> 
I've tried that (also updated how `static void construct` calls `__construct`) 
and it works with C++2a but fails with C++03. The error is

```
llvm-project/libcxx/include/memory:1725:21: error: class member cannot be 
redeclared
static void __construct(allocator_type&, _Tp* __p, const _A0& __a0)
^
llvm-project/libcxx/include/memory:1720:21: note: previous definition is here
static void __construct(allocator_type& __a, _Tp* __p, const _A0& __a0)
^
```

I haven't investigated further yet.


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

https://reviews.llvm.org/D48753



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

Buildbot numbers for the week of 11/18/2018 - 11/24/2018

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 11/18/2018 - 11/24/2018.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 lldb-amd64-ninja-netbsd8 | 34:43:22
 clang-s390x-linux-lnt| 32:49:11
 sanitizer-x86_64-linux-fast  | 22:13:32
 sanitizer-x86_64-linux-fuzzer| 19:46:11
 sanitizer-x86_64-linux-autoconf  | 19:14:46
 clang-x64-ninja-win7 | 15:51:25
 clang-x64-windows-msvc   | 13:41:06
 llvm-clang-x86_64-expensive-checks-win   | 13:34:42
 clang-ppc64le-linux-multistage   | 13:05:40
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc49-cxx11 | 13:00:50
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 12:50:47
 sanitizer-x86_64-linux   | 11:27:51
 clang-bpf-build  | 09:21:30
 clang-s390x-linux-multistage | 09:06:30
 clang-cmake-thumbv7-full-sh  | 08:19:24
 clang-s390x-linux| 08:06:39
 clang-with-lto-ubuntu| 08:04:28
 clang-cmake-armv7-selfhost-neon  | 07:54:59
 clang-ppc64be-linux-multistage   | 07:54:07
 clang-cmake-aarch64-lld  | 07:34:37
 sanitizer-x86_64-linux-bootstrap-msan| 07:16:30
 clang-ppc64be-linux-lnt  | 07:07:50
 clang-ppc64le-linux  | 07:04:03
 clang-cmake-thumbv8-full-sh  | 06:56:48
 clang-cmake-armv8-lld| 06:54:52
 clang-cuda-build | 06:54:36
 clang-ppc64be-linux  | 06:53:21
 clang-with-thin-lto-ubuntu   | 06:49:19
 clang-ppc64le-linux-lnt  | 06:48:25
 clang-atom-d525-fedora-rel   | 06:38:16
 clang-cmake-armv7-full   | 06:31:53
 clang-lld-x86_64-2stage  | 06:20:35
 sanitizer-ppc64le-linux  | 06:13:14
 sanitizer-x86_64-linux-android   | 05:56:32
 clang-cmake-aarch64-full | 05:27:20
 sanitizer-ppc64be-linux  | 05:13:36
 lld-x86_64-darwin13  | 04:24:47
 clang-hexagon-elf| 04:08:19
 clang-cmake-aarch64-quick| 03:26:52
 clang-cmake-armv7-global-isel| 03:12:42
 clang-cmake-armv8-full   | 02:57:40
 lld-perf-testsuite   | 02:18:28
 clang-cmake-armv7-selfhost   | 02:09:15
 clang-cmake-aarch64-global-isel  | 02:05:42
 clang-cmake-armv8-quick  | 02:01:28
 sanitizer-x86_64-linux-bootstrap | 02:00:58
 clang-cmake-armv8-global-isel| 02:00:31
 clang-cmake-armv7-quick  | 01:58:43
 clang-cmake-x86_64-sde-avx512-linux  | 01:46:50
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 01:46:07
 sanitizer-x86_64-linux-bootstrap-ubsan   | 01:38:51
 lldb-x86_64-ubuntu-14.04-buildserver | 01:12:17
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03   | 00:54:49
 polly-arm-linux  | 00:53:58
 llvm-hexagon-elf | 00:51:02
 clang-cmake-x86_64-avx2-linux| 00:50:10
 clang-x86_64-linux-abi-test  | 00:38:25
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17   | 00:37:10
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14   | 00:36:58
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11   | 00:36:55
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a   | 00:35:54
 sanitizer-windows| 00:29:06
 clang-armv7-linux-build-cache| 00:27:40
 clang-aarch64-linux-build-cache  | 00:26:11
(64 rows)


"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green):
   buildername   | builds | changes
| status_change_ratio
-++-+
 

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 177803.
vsapsai added a comment.

- Update `__has_construct`, `__has_destroy` according to review comments.
- Tighten tests: custom allocators aren't mandated by C++03 but libc++ has the 
support.


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

https://reviews.llvm.org/D48753

Files:
  libcxx/include/memory
  
libcxx/test/libcxx/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
  
libcxx/test/libcxx/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
  
libcxx/test/std/utilities/memory/allocator.traits/allocator.traits.members/destroy.pass.cpp
  libcxx/test/support/min_allocator.h

Index: libcxx/test/support/min_allocator.h
===
--- libcxx/test/support/min_allocator.h
+++ libcxx/test/support/min_allocator.h
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
@@ -131,6 +132,59 @@
 friend bool operator!=(malloc_allocator x, malloc_allocator y) {return !(x == y);}
 };
 
+template 
+struct cpp03_allocator : bare_allocator
+{
+typedef T value_type;
+typedef value_type* pointer;
+
+static bool construct_called;
+
+// Returned value is not used but it's not prohibited.
+pointer construct(pointer p, const value_type& val)
+{
+::new(p) value_type(val);
+construct_called = true;
+return p;
+}
+
+std::size_t max_size() const
+{
+return UINT_MAX / sizeof(T);
+}
+};
+template  bool cpp03_allocator::construct_called = false;
+
+template 
+struct cpp03_overload_allocator : bare_allocator
+{
+typedef T value_type;
+typedef value_type* pointer;
+
+static bool construct_called;
+
+void construct(pointer p, const value_type& val)
+{
+construct(p, val, std::is_class());
+}
+void construct(pointer p, const value_type& val, std::true_type)
+{
+::new(p) value_type(val);
+construct_called = true;
+}
+void construct(pointer p, const value_type& val, std::false_type)
+{
+::new(p) value_type(val);
+construct_called = true;
+}
+
+std::size_t max_size() const
+{
+return UINT_MAX / sizeof(T);
+}
+};
+template  bool cpp03_overload_allocator::construct_called = false;
+
 
 #if TEST_STD_VER >= 11
 
Index: libcxx/test/std/utilities/memory/allocator.traits/allocator.traits.members/destroy.pass.cpp
===
--- libcxx/test/std/utilities/memory/allocator.traits/allocator.traits.members/destroy.pass.cpp
+++ libcxx/test/std/utilities/memory/allocator.traits/allocator.traits.members/destroy.pass.cpp
@@ -73,7 +73,7 @@
   std::aligned_storage::type store;
   std::allocator_traits::destroy(a, (VT*));
 }
-#if TEST_STD_VER >= 11
+#if defined(_LIBCPP_VERSION) || TEST_STD_VER >= 11
 {
 A0::count = 0;
 b_destroy = 0;
Index: libcxx/test/libcxx/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
@@ -0,0 +1,57 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template  vector(InputIter first, InputIter last,
+//   const allocator_type& a);
+
+#include 
+#include 
+
+#include "min_allocator.h"
+
+void test_ctor_under_alloc() {
+  int arr1[] = {42};
+  int arr2[] = {1, 101, 42};
+  {
+typedef std::vector > C;
+typedef C::allocator_type Alloc;
+Alloc a;
+{
+  Alloc::construct_called = false;
+  C v(arr1, arr1 + 1, a);
+  assert(Alloc::construct_called);
+}
+{
+  Alloc::construct_called = false;
+  C v(arr2, arr2 + 3, a);
+  assert(Alloc::construct_called);
+}
+  }
+  {
+typedef std::vector > C;
+typedef C::allocator_type Alloc;
+Alloc a;
+{
+  Alloc::construct_called = false;
+  C v(arr1, arr1 + 1, a);
+  assert(Alloc::construct_called);
+}
+{
+  Alloc::construct_called = false;
+  C v(arr2, arr2 + 3, a);
+  assert(Alloc::construct_called);
+}
+  }
+}
+
+int main() {
+  test_ctor_under_alloc();
+}
Index: libcxx/test/libcxx/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
@@ -0,0 +1,54 @@

[PATCH] D55525: [Driver] Add support for -fembed-bitcode for assembly file

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D55525#1327742 , @compnerd wrote:

> This really feels odd.  Why not expect that the developer will add the 
> content themselves?  I'm not sure I understand the motivation for this change.


The main motivation for upstreaming this is to make -fembed-bitcode behaves the 
same as Apple clang.
The section is just a marker for ld64 to tell the linker there is no bitcode 
available for this specific module because it is built from assembly. If ld64 
sees this marker, it will pull the object file into the bitcode bundle, rather 
than error out and complaining about missing bitcode.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55525



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

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

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

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



Comment at: lib/AST/ExprConstant.cpp:6147-6148
+  return ZeroInitialization(E);
+if (!Result.checkNullPointerForFoldAccess(Info, E, AK_Read))
+  return false;
+QualType CharTy =

hubert.reinterpretcast wrote:
> rsmith wrote:
> > Why do we need to do this explicitly, rather than allowing to simply happen 
> > as part of the first `handleLValueToRValueConversion` below?
> We want the error message to be produced even if we bail early on not having 
> a valid designator.
If we have an invalid designator, we already produced a diagnostic when setting 
it invalid, and generally don't need to diagnose anything else.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55510



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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Thanks. I think what we really want to do here is reconsider our default for 
applying dllimport. Leaving things unannotated is a good safe default for every 
environment. In the absence of any flags, clang should assume runtime functions 
are statically linked. The linker will synthesize thunks for us. MSVC and GCC 
do this and it's fine, if slow, and perhaps confusing to past versions of LLDB.

Then, we should add new, runtime-specific, -cc1 flags (as @pcc and @mgrang said 
earlier) to indicate that certain runtimes will be dynamically linked. We 
should have separate -cc1 flags for the obj-c runtime and the CRT. The obj-c 
code should call some obj-c specific helper that respects the flag instead of 
CreateRuntimeFunction. We can do something like that in MicrosoftCXXABI for the 
msvc crt functions. As a straw man, I'd propose `-shared-crt` for consistency 
with `-shared-libgcc`, `-static-libc++`, and others as the flag spelling. I 
could imagine using that on mingw as an optimization for people who don't want 
the thunks.

Does that seem reasonable?




Comment at: CodeGen/CodeGenModule.cpp:2957-2958
   !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  !getTriple().isWindowsGNUEnvironment() &&
+  !getTriple().isWindowsMSVCEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);

compnerd wrote:
> rnk wrote:
> > The condition here of `T.isOSBinFormatCOFF() && 
> > !T.isWindowsGNUEnvironment() && !T.isWindowsMSVCEnvironment()` is 
> > essentially equivalent to `T.isWindowsItaniumEnvironment()`. Please 
> > simplify the condition to that. The other relevant environments are Cygnus 
> > for Cygwin and CoreCLR, which probably want to follow the MSVC/GNU behavior.
> IIRC, one of the issues was that lldb couldn't deal very well with the 
> thunks.  The other thing is that there is a small penalty for something that 
> we know that we are going to redirect through.  However, I think that if lldb 
> is able to deal with the thunks now, I would be okay with the penalty to make 
> the behavior more similar to MSVC.  Basically, if lldb works, lets just get 
> rid of the special behavior for the itanium environment.
Can you elaborate on what didn't work in LLDB when the thunks were present? 
What kind of functionality did you have to exercise to get it to misbehave?



Comment at: CodeGen/ms-symbol-linkage.cpp:1-3
+// RUN: %clangxx -target aarch64-windows \
+// RUN: -fcxx-exceptions -c -o - %s \
+// RUN: | llvm-objdump -syms - 2>&1 | FileCheck %s

Please do not write tests that emit object files in clang. Use %clang_cc1 
-emit-llvm like the other tests and check the IR for dllimport or the lack 
thereof.



Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

compnerd wrote:
> rnk wrote:
> > mgrang wrote:
> > > I had to remove dllimport in this and the next unit test. I am not sure 
> > > of the wider implications of this change. Maybe controlling this via a 
> > > flag (like -flto-visibility-public-std) is a better/more localized option?
> > These are the ones that I think @compnerd really cares about since the objc 
> > runtime is typically dynamically linked and frequently called. We might 
> > want to arrange things such that we have a separate codepath for ObjC 
> > runtime helpers. I'm surprised we don't already have such a code path.
> I think that @theraven would care more about this code path than I.  I think 
> that this change may be wrong, because the load here is supposed to be in the 
> ObjC runtime, and this becoming local to the module would break the global 
> registration.
We just set dso_local whenever something isn't dllimport, even if it's a lie 
sometimes. I think everything would work as intended with a linker thunk. It's 
"true" as far as LLVM is concerned for the purposes of emitting relocations.


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

https://reviews.llvm.org/D55229



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

[PATCH] D53238: [Driver] Add -static-{rtlib, stdlib} and make -static-{libgcc, libstdc++} their aliases

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

(sub)


Repository:
  rC Clang

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

https://reviews.llvm.org/D53238



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

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 177796.
hubert.reinterpretcast added a comment.

Recast representation-sensitive tests as SemaCXX tests using array bounds


Repository:
  rC Clang

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

https://reviews.llvm.org/D55510

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-string.cpp

Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -1,8 +1,11 @@
-// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++1z -fsyntax-only -verify -pedantic
-// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++1z -fsyntax-only -verify -pedantic -fno-signed-char
-// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++1z -fsyntax-only -verify -pedantic -fno-wchar -Dwchar_t=__WCHAR_TYPE__
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
-# 6 "/usr/include/string.h" 1 3 4
+# 9 "/usr/include/string.h" 1 3 4
 extern "C" {
   typedef decltype(sizeof(int)) size_t;
 
@@ -18,10 +21,13 @@
   extern void *memcpy(void *d, const void *s, size_t n);
   extern void *memmove(void *d, const void *s, size_t n);
 }
-# 22 "SemaCXX/constexpr-string.cpp" 2
+# 25 "SemaCXX/constexpr-string.cpp" 2
 
-# 24 "/usr/include/wchar.h" 1 3 4
+# 27 "/usr/include/wchar.h" 1 3 4
 extern "C" {
+#if NO_PREDEFINED_WCHAR_T
+  typedef decltype(L'0') wchar_t;
+#endif
   extern size_t wcslen(const wchar_t *p);
 
   extern int wcscmp(const wchar_t *s1, const wchar_t *s2);
@@ -35,7 +41,7 @@
   extern wchar_t *wmemmove(wchar_t *d, const wchar_t *s, size_t n);
 }
 
-# 39 "SemaCXX/constexpr-string.cpp" 2
+# 45 "SemaCXX/constexpr-string.cpp" 2
 namespace Strlen {
   constexpr int n = __builtin_strlen("hello"); // ok
   static_assert(n == 5);
@@ -95,11 +101,142 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memcmp(, "", 0u) == 0);
+  static_assert(__builtin_memcmp("", , 0u) == 0);
+  static_assert(__builtin_memcmp(, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+  static_assert(__builtin_memcmp("", , 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  constexpr unsigned char ku00fe00[] = {0x00, 0xfe, 0x00};
+  constexpr unsigned char ku00feff[] = {0x00, 0xfe, 0xff};
+  constexpr signed char ks00fe00[] = {0, -2, 0};
+  constexpr signed char ks00feff[] = {0, -2, -1};
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ku00fe00, ks00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ku00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 99) == -1);
+
+  struct Bool3Tuple { bool bb[3]; };
+  constexpr Bool3Tuple kb000100 = {{false, true, false}};
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 1) == 0);
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 2) == 1);
+
+  constexpr long ksl[] = {0, -1};
+  constexpr unsigned int kui[] = {0, 0u - 1};
+  constexpr unsigned long long kull[] = {0, 0ull - 1};
+  constexpr const auto *kuSizeofLong(void) {
+if constexpr(sizeof(long) == sizeof(int)) {
+  return kui;
+} else if constexpr(sizeof(long) == sizeof(long long)) {
+  return kull;
+} else {
+  return nullptr;
+}
+  }
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 1) == 0);
+  static_assert(__builtin_memcmp(ksl, 

[PATCH] D55546: [clang] Add AST matcher for block expressions 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Please let me know if I missed anything as part of this proposed change. I 
tried my best to determine how to make this change appropriately based on 
previous submissions and documentation but I recognize that I could have missed 
things 


Repository:
  rC Clang

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

https://reviews.llvm.org/D55546



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

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 177795.
stephanemoore marked 5 inline comments as done.
stephanemoore added a comment.

Changes:
• Drop const on local bool variable.
• Adopt the term "function in global namespace" in diagnostic messages.


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

https://reviews.llvm.org/D55482

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m
  test/clang-tidy/google-objc-function-naming.mm

Index: test/clang-tidy/google-objc-function-naming.mm
===
--- test/clang-tidy/google-objc-function-naming.mm
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -1,16 +1,19 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
 void printSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'printSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void PrintSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'PrintSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void ABCBad_Name() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'ABCBad_Name' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 namespace {
 
Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -3,28 +3,35 @@
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'ispositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
 
 static bool is_positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'is_positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool isPositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'isPositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool Is_Positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'Is_Positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool IsPositive(int a) { return a > 0; }
 
 bool ispalindrome(const char *str);
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'ispalindrome' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 static const char *md5(const char *str) { return 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: static function named 'md5' must be
+// in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
 
 static const char *MD5(const char *str) { return 0; }
@@ -38,12 +45,16 @@
 static const char *StringFromNSString(id str) { return ""; }
 
 void 

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:115
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "

aaron.ballman wrote:
> stephanemoore wrote:
> > benhamilton wrote:
> > > I know "global" is the correct name, but I feel like "non-static" might 
> > > be clearer to folks dealing with these error messages.
> > > 
> > > WDYT?
> > > 
> > I'm wary of saying "non-static" because namespaced functions in 
> > Objective-C++ are not subject to the cited rules. The term "non-static" 
> > seems like it could be interpreted to extend to namespaced functions which 
> > could potentially mislead someone into adding prefixes to namespaced 
> > functions in Objective-C++.
> How about "%select{static function|function in global namespace}1 named %0..."
Definitely better.


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

https://reviews.llvm.org/D55482



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

[PATCH] D55586: Basic: make `int_least64_t` and `int_fast64_t` match on Darwin

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd created this revision.
compnerd added reviewers: ahatanak, rjmccall.

The Darwin targets use `int64_t` and `uint64_t` to define the `int_least64_t`
and `int_fast64_t` types.  The underlying type is actually a `long long`.  Match
the types to allow the printf specifiers to work properly and have the compiler
vended macros match the implementation on the target.


Repository:
  rC Clang

https://reviews.llvm.org/D55586

Files:
  lib/Basic/Targets/OSTargets.h
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -1331,10 +1331,10 @@
 // AARCH64-DARWIN: #define __INT_FAST32_FMTi__ "i"
 // AARCH64-DARWIN: #define __INT_FAST32_MAX__ 2147483647
 // AARCH64-DARWIN: #define __INT_FAST32_TYPE__ int
-// AARCH64-DARWIN: #define __INT_FAST64_FMTd__ "ld"
-// AARCH64-DARWIN: #define __INT_FAST64_FMTi__ "li"
-// AARCH64-DARWIN: #define __INT_FAST64_MAX__ 9223372036854775807L
-// AARCH64-DARWIN: #define __INT_FAST64_TYPE__ long int
+// AARCH64-DARWIN: #define __INT_FAST64_FMTd__ "lld"
+// AARCH64-DARWIN: #define __INT_FAST64_FMTi__ "lli"
+// AARCH64-DARWIN: #define __INT_FAST64_MAX__ 9223372036854775807LL
+// AARCH64-DARWIN: #define __INT_FAST64_TYPE__ long long int
 // AARCH64-DARWIN: #define __INT_FAST8_FMTd__ "hhd"
 // AARCH64-DARWIN: #define __INT_FAST8_FMTi__ "hhi"
 // AARCH64-DARWIN: #define __INT_FAST8_MAX__ 127
@@ -1347,10 +1347,10 @@
 // AARCH64-DARWIN: #define __INT_LEAST32_FMTi__ "i"
 // AARCH64-DARWIN: #define __INT_LEAST32_MAX__ 2147483647
 // AARCH64-DARWIN: #define __INT_LEAST32_TYPE__ int
-// AARCH64-DARWIN: #define __INT_LEAST64_FMTd__ "ld"
-// AARCH64-DARWIN: #define __INT_LEAST64_FMTi__ "li"
-// AARCH64-DARWIN: #define __INT_LEAST64_MAX__ 9223372036854775807L
-// AARCH64-DARWIN: #define __INT_LEAST64_TYPE__ long int
+// AARCH64-DARWIN: #define __INT_LEAST64_FMTd__ "lld"
+// AARCH64-DARWIN: #define __INT_LEAST64_FMTi__ "lli"
+// AARCH64-DARWIN: #define __INT_LEAST64_MAX__ 9223372036854775807LL
+// AARCH64-DARWIN: #define __INT_LEAST64_TYPE__ long long int
 // AARCH64-DARWIN: #define __INT_LEAST8_FMTd__ "hhd"
 // AARCH64-DARWIN: #define __INT_LEAST8_FMTi__ "hhi"
 // AARCH64-DARWIN: #define __INT_LEAST8_MAX__ 127
@@ -1418,16 +1418,16 @@
 // AARCH64-DARWIN: #define __UINT_FAST16_TYPE__ unsigned short
 // AARCH64-DARWIN: #define __UINT_FAST32_MAX__ 4294967295U
 // AARCH64-DARWIN: #define __UINT_FAST32_TYPE__ unsigned int
-// AARCH64-DARWIN: #define __UINT_FAST64_MAX__ 18446744073709551615UL
-// AARCH64-DARWIN: #define __UINT_FAST64_TYPE__ long unsigned int
+// AARCH64-DARWIN: #define __UINT_FAST64_MAX__ 18446744073709551615ULL
+// AARCH64-DARWIN: #define __UINT_FAST64_TYPE__ long long unsigned int
 // AARCH64-DARWIN: #define __UINT_FAST8_MAX__ 255
 // AARCH64-DARWIN: #define __UINT_FAST8_TYPE__ unsigned char
 // AARCH64-DARWIN: #define __UINT_LEAST16_MAX__ 65535
 // AARCH64-DARWIN: #define __UINT_LEAST16_TYPE__ unsigned short
 // AARCH64-DARWIN: #define __UINT_LEAST32_MAX__ 4294967295U
 // AARCH64-DARWIN: #define __UINT_LEAST32_TYPE__ unsigned int
-// AARCH64-DARWIN: #define __UINT_LEAST64_MAX__ 18446744073709551615UL
-// AARCH64-DARWIN: #define __UINT_LEAST64_TYPE__ long unsigned int
+// AARCH64-DARWIN: #define __UINT_LEAST64_MAX__ 18446744073709551615ULL
+// AARCH64-DARWIN: #define __UINT_LEAST64_TYPE__ long long unsigned int
 // AARCH64-DARWIN: #define __UINT_LEAST8_MAX__ 255
 // AARCH64-DARWIN: #define __UINT_LEAST8_TYPE__ unsigned char
 // AARCH64-DARWIN: #define __USER_LABEL_PREFIX__ _
Index: lib/Basic/Targets/OSTargets.h
===
--- lib/Basic/Targets/OSTargets.h
+++ lib/Basic/Targets/OSTargets.h
@@ -133,6 +133,15 @@
   /// is very similar to ELF's "protected";  Darwin requires a "weak"
   /// attribute on declarations that can be dynamically replaced.
   bool hasProtectedVisibility() const override { return false; }
+
+  TargetInfo::IntType getLeastIntTypeByWidth(unsigned BitWidth,
+ bool IsSigned) const final {
+// Darwin uses `long long` for `int_least64_t` and `int_fast64_t`.
+return BitWidth == 64
+   ? (IsSigned ? TargetInfo::SignedLongLong
+   : TargetInfo::UnsignedLongLong)
+   : TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
+  }
 };

 // DragonFlyBSD Target


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -1331,10 +1331,10 @@
 // AARCH64-DARWIN: #define __INT_FAST32_FMTi__ "i"
 // AARCH64-DARWIN: #define __INT_FAST32_MAX__ 2147483647
 // AARCH64-DARWIN: #define __INT_FAST32_TYPE__ int
-// AARCH64-DARWIN: #define __INT_FAST64_FMTd__ "ld"
-// AARCH64-DARWIN: #define __INT_FAST64_FMTi__ "li"
-// AARCH64-DARWIN: #define __INT_FAST64_MAX__ 9223372036854775807L
-// 

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a subscriber: theraven.
compnerd added inline comments.



Comment at: CodeGen/CodeGenModule.cpp:2957-2958
   !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  !getTriple().isWindowsGNUEnvironment() &&
+  !getTriple().isWindowsMSVCEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);

rnk wrote:
> The condition here of `T.isOSBinFormatCOFF() && !T.isWindowsGNUEnvironment() 
> && !T.isWindowsMSVCEnvironment()` is essentially equivalent to 
> `T.isWindowsItaniumEnvironment()`. Please simplify the condition to that. The 
> other relevant environments are Cygnus for Cygwin and CoreCLR, which probably 
> want to follow the MSVC/GNU behavior.
IIRC, one of the issues was that lldb couldn't deal very well with the thunks.  
The other thing is that there is a small penalty for something that we know 
that we are going to redirect through.  However, I think that if lldb is able 
to deal with the thunks now, I would be okay with the penalty to make the 
behavior more similar to MSVC.  Basically, if lldb works, lets just get rid of 
the special behavior for the itanium environment.



Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

rnk wrote:
> mgrang wrote:
> > I had to remove dllimport in this and the next unit test. I am not sure of 
> > the wider implications of this change. Maybe controlling this via a flag 
> > (like -flto-visibility-public-std) is a better/more localized option?
> These are the ones that I think @compnerd really cares about since the objc 
> runtime is typically dynamically linked and frequently called. We might want 
> to arrange things such that we have a separate codepath for ObjC runtime 
> helpers. I'm surprised we don't already have such a code path.
I think that @theraven would care more about this code path than I.  I think 
that this change may be wrong, because the load here is supposed to be in the 
ObjC runtime, and this becoming local to the module would break the global 
registration.


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

https://reviews.llvm.org/D55229



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

[PATCH] D55525: [Driver] Add support for -fembed-bitcode for assembly file

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

This really feels odd.  Why not expect that the developer will add the content 
themselves?  I'm not sure I understand the motivation for this change.  I think 
that this should be easy to write a test case for as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55525



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

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

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

Szelethus added a comment.

Makes sense! I like the summary a lot, and the fact that you added a new debug 
checker. I feel like I'm not yet qualified to give meaningful feedback though, 
but if you are not in a hurry, I'll happily play around with this patch next 
week, and both learn a bit and possibly give a better review.

But looking at it right now, this looks great.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55566



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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Sorry, I was out sick for a week. We should ask @smeenai about this change, 
since he has been doing more work in this area recently.




Comment at: CodeGen/CodeGenModule.cpp:2957-2958
   !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  !getTriple().isWindowsGNUEnvironment() &&
+  !getTriple().isWindowsMSVCEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);

The condition here of `T.isOSBinFormatCOFF() && !T.isWindowsGNUEnvironment() && 
!T.isWindowsMSVCEnvironment()` is essentially equivalent to 
`T.isWindowsItaniumEnvironment()`. Please simplify the condition to that. The 
other relevant environments are Cygnus for Cygwin and CoreCLR, which probably 
want to follow the MSVC/GNU behavior.



Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

mgrang wrote:
> I had to remove dllimport in this and the next unit test. I am not sure of 
> the wider implications of this change. Maybe controlling this via a flag 
> (like -flto-visibility-public-std) is a better/more localized option?
These are the ones that I think @compnerd really cares about since the objc 
runtime is typically dynamically linked and frequently called. We might want to 
arrange things such that we have a separate codepath for ObjC runtime helpers. 
I'm surprised we don't already have such a code path.


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

https://reviews.llvm.org/D55229



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

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@courbet: On the cpplang Slack, Peter Feichtinger mentioned that defaulted 
template arguments should perhaps be treated differently. Is there any way to 
suppress the `, std::allocator` part of this diagnostic? 
https://godbolt.org/z/TM0UHc

Before your patches:

  :11:5: error: static_assert failed due to requirement 
'std::is_integral_v'
  static_assert(std::is_integral_v);
  ^ ~

After your patches:

  :11:5: error: static_assert failed due to requirement 
'std::is_integral_v > >'
  static_assert(std::is_integral_v);
  ^ ~ 

I don't think the new behavior is //worse//; but I don't think it's optimal, 
either.

I think Clang already has a feature to suppress printing these parameters; you 
would just have to figure out where it is and try to hook it into your thing. 
(And if you do, I'm going to ask for a test case where `T::t` is 
`std::pmr::vector`!)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55270



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

[PATCH] D55527: Normalize GlobalDecls when used with CPUDispatch

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CodeGenModule.cpp:910
 Out << ".resolver";
 }
 

rsmith wrote:
> Hmm, it looks like we don't have a unique name for a `GlobalDecl` naming the 
> `CPUOrdinal == 0` case of an overloaded `cpu_specific` function:
> ```
> __attribute__((cpu_specific(atom)))
> void f() {} // CPUOrdinal 0 GD mangled as _Z1fv.ifunc
> __attribute__((cpu_specific(haswell)))
> void f() {} // CPUOrdinal 0 GD also mangled as _Z1fv.ifunc
> ```
> Maybe we could append the CPU-specific manglings for all named CPUs before 
> the `.ifunc`?
Well shoot, thats true, since you can call the function at different points and 
have overload resolution 'choose' a different one (since it is just by 
ordering).

If we were to give it a different name, we would either have to replace it 
later, or make sure a call to 'not the ifunc' doesn't happen.  



Comment at: lib/CodeGen/CodeGenModule.cpp:2133-2138
+  // If this is a cpu_dispatch multiversion function, designate it for emission
+  // at the end of the Translation Unit.
+  if (Global->hasAttr()) {
+MultiVersionFuncs.push_back(GD);
+return;
+  }

rsmith wrote:
> Instead of adding a special-case list of `MultiVersionFuncs`, could we 
> instead change `MayBeEmittedEagerly` to return `false` for `CPUDispatch` 
> functions and use the normal `DeferredDecls` mechanism for this?
That list already would have to exist for GCC Multiversioning, since there 
isn't a single declaration that corresponds to the resolver.  I could 
definitely remove CPUDispatch from this mechanism and return it to a separate 
one, but I'm not sure what that buys us.

Unless you think this is something we can do that will fix the GCC 
Multiversioning as well?  



Comment at: lib/CodeGen/CodeGenModule.cpp:2637
+if (!ImplDecl.getDecl()) {
+  ImplDecl = GD.getWithMultiVersionOrdinal(Ordinal);
 

rsmith wrote:
> I think we should have a comment somewhere explaining the subtle use of the 
> multiversion ordinal:
> 
>  * (Resolver, 0) refers to the resolver itself
>  * (Resolver, N) refers to an external (outside this module) symbol for the 
> N'th CPU in Resolver's cpu_dispatch list
>  * (Specific, 0) is a placeholder used to refer to the set of function 
> symbols defined by a cpu_specific function declaration; no corresponding 
> symbol is ever emitted (right?)
>  * (Specific, N) refers to an internal (within this module) symbol for the 
> N'th CPU in Specific's cpu_specific list
(Specific,0) is a placeholder for a not-yet-existing CPU-Dispatch version of 
the function.  Otherwise, I think documenting that somewhere is a good idea.  
I'll look for a good spot for it.



Comment at: lib/CodeGen/CodeGenModule.cpp:2639-2644
+  std::string MangledName = getMangledNameImpl(*this, ImplDecl, FD, true) +
+getCPUSpecificMangling(*this, II->getName());
+  Func = cast(GetOrCreateLLVMFunction(
+  MangledName, DeclTy, ImplDecl,
   /*ForVTable=*/false, /*DontDefer=*/true,
+  /*IsThunk=*/false, llvm::AttributeList(), ForDefinition));

rsmith wrote:
> I think the first part of this should now be equivalent to 
> `getMangledName(...ImplDecl...)`, which should means that these 6 lines can 
> be replaced by `GetAddrOfFunction(ImplDecl, DeclTy, false, false, 
> NotForDefinition)`.
> 
> (Also I think you should pass `NotForDefinition` here because you just want 
> to reference the symbol, you don't want to define it yourself.)
I remember 'DontDefer=true' being set for a good reason, since we need this 
function emitted.

At the moment, this doesn't work because GetOrCreateLLVMFunction (called by 
GetAddressOfFunction) replaces NotForDefinition with the ifunc instead of the 
function, so calling that here ends up getting the ifunc/resolver instead of 
the function itself.  

I'll continue looking into that, but if you have alternative design decisions, 
I'd love to hear them.


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

https://reviews.llvm.org/D55527



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

[PATCH] D55447: [Sema] Fix Modified Type in address_space AttributedType

[Leonard Chan via Phabricator via cfe-commits]

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



Comment at: clang/tools/libclang/CXType.cpp:132
+  if (!(TU->ParsingOptions & CXTranslationUnit_IncludeAttributedTypes) &&
+  ATT->getAttrKind() != attr::AddressSpace) {
 return MakeCXType(ATT->getModifiedType(), TU);

aaron.ballman wrote:
> leonardchan wrote:
> > aaron.ballman wrote:
> > > leonardchan wrote:
> > > > aaron.ballman wrote:
> > > > > This change seems surprising -- if the parsing options say the caller 
> > > > > does not want attributed types, why are we returning one anyway for 
> > > > > address space?
> > > > This has to do with ensuring `clang_getAddressSpace` still returns the 
> > > > proper address_space. It does this by essentially checking the 
> > > > qualifiers of the type, which we now attach to the `AttributedType` 
> > > > whereas before it was attached to the modified type.
> > > > 
> > > > This extra condition is necessary for ensuring that calling 
> > > > `clang_getAddressSpace` points to the qualified AttributedType instead 
> > > > of the unqualified modified type.
> > > My fear is that this will be breaking assumptions in third-party code. If 
> > > someone disables `CXTranslationUnit_IncludeAttributedTypes`, they are 
> > > unlikely to expect to receive an `AttributedType` and may react poorly to 
> > > it.
> > Instead check if the type is address_space attributed and apply the 
> > qualifiers the modified type.
> Sure, they can always modify their code to handle it gracefully, but it's a 
> silent, breaking change to a somewhat stable API which is why I was prodding 
> about it.
> 
> After talking with @rsmith, we're thinking the correct change here is to 
> return the attributed type when the user asks for it, but return the 
> equivalent type rather than the modified type if the user doesn't want 
> attribute sugar (for all attributes, not just address spaces). This way, when 
> a user asks for CXType for one of these, they always get a correct type but 
> sometimes it has attribute type sugar and sometimes it doesn't, depending on 
> the parsing options.
> 
> This is still a silent, breaking change in behavior, which is unfortunate. It 
> should probably come with a mention in the release notes about the change to 
> the API and some unit tests as well.
Ok. In the case of qualifiers then, should the equivalent type still contain 
the address_space qualifiers when creating the AttributedType?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55447



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

r348899 - Replace Const-Member checking with non-recursive version.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Dec 11 13:54:52 2018
New Revision: 348899

URL: http://llvm.org/viewvc/llvm-project?rev=348899=rev
Log:
Replace Const-Member checking with non-recursive version.

As reported in PR39946, these two implementations cause stack overflows
to occur when a type recursively contains itself.  While this only
happens when an incomplete version of itself is used by membership (and
thus an otherwise invalid program), the crashes might be surprising.

The solution here is to replace the recursive implementation with one
that uses a std::vector as a queue.  Old values are kept around to
prevent re-checking already checked types.

Change-Id: I582bb27147104763d7daefcfee39d91f408b9fa8

Modified:
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/assign.c

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=348899=348898=348899=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Tue Dec 11 13:54:52 2018
@@ -3166,14 +3166,23 @@ bool TagType::isBeingDefined() const {
 }
 
 bool RecordType::hasConstFields() const {
-  for (FieldDecl *FD : getDecl()->fields()) {
-QualType FieldTy = FD->getType();
-if (FieldTy.isConstQualified())
-  return true;
-FieldTy = FieldTy.getCanonicalType();
-if (const auto *FieldRecTy = FieldTy->getAs())
-  if (FieldRecTy->hasConstFields())
+  std::vector RecordTypeList;
+  RecordTypeList.push_back(this);
+  unsigned NextToCheckIndex = 0;
+
+  while (RecordTypeList.size() > NextToCheckIndex) {
+for (FieldDecl *FD :
+ RecordTypeList[NextToCheckIndex]->getDecl()->fields()) {
+  QualType FieldTy = FD->getType();
+  if (FieldTy.isConstQualified())
 return true;
+  FieldTy = FieldTy.getCanonicalType();
+  if (const auto *FieldRecTy = FieldTy->getAs()) {
+if (llvm::find(RecordTypeList, FieldRecTy) == RecordTypeList.end())
+  RecordTypeList.push_back(FieldRecTy);
+  }
+}
+++NextToCheckIndex;
   }
   return false;
 }

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=348899=348898=348899=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Dec 11 13:54:52 2018
@@ -4,30 +4,39 @@ static void DiagnoseRecursiveConstFields
  const RecordType *Ty,
  SourceLocation Loc, SourceRange Range,
  OriginalExprKind OEK,
- bool ,
- bool IsNested = false) {
+ bool ) {
+  std::vector RecordTypeList;
+  RecordTypeList.push_back(Ty);
+  unsigned NextToCheckIndex = 0;
+  // TODO: MAKE THIS NOT RECURSIVE
   // We walk the record hierarchy breadth-first to ensure that we print
   // diagnostics in field nesting order.
-  // First, check every field for constness.
-  for (const FieldDecl *Field : Ty->getDecl()->fields()) {
-if (Field->getType().isConstQualified()) {
-  if (!DiagnosticEmitted) {
-S.Diag(Loc, diag::err_typecheck_assign_const)
-<< Range << NestedConstMember << OEK << VD
-<< IsNested << Field;
-DiagnosticEmitted = true;
+  while (RecordTypeList.size() > NextToCheckIndex) {
+bool IsNested = NextToCheckIndex > 0;
+for (const FieldDecl *Field :
+ RecordTypeList[NextToCheckIndex]->getDecl()->fields()) {
+  // First, check every field for constness.
+  QualType FieldTy = Field->getType();
+  if (FieldTy.isConstQualified()) {
+if (!DiagnosticEmitted) {
+  S.Diag(Loc, diag::err_typecheck_assign_const)
+  << Range << NestedConstMember << OEK << VD
+  << IsNested << Field;
+  DiagnosticEmitted = true;
+}
+S.Diag(Field->getLocation(), diag::note_typecheck_assign_const)
+<< NestedConstMember << IsNested << Field
+<< FieldTy << Field->getSourceRange();
+  }
+
+  // Then we append it to the list to check next in order.
+  FieldTy = FieldTy.getCanonicalType();
+  if (const auto *FieldRecTy = FieldTy->getAs()) {
+if (llvm::find(RecordTypeList, FieldRecTy) == RecordTypeList.end())
+  RecordTypeList.push_back(FieldRecTy);
   }
-  S.Diag(Field->getLocation(), diag::note_typecheck_assign_const)
-  << NestedConstMember << IsNested << Field
-  << Field->getType() << Field->getSourceRange();
 }
-  }
-  // Then, recurse.
-  for (const FieldDecl *Field : Ty->getDecl()->fields()) {
-QualType FTy = Field->getType();
-if (const RecordType *FieldRecTy = FTy->getAs())
-  

[PATCH] D55527: Normalize GlobalDecls when used with CPUDispatch

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

rsmith marked an inline comment as done.
rsmith added a comment.

Thanks, this makes a lot of sense to me.




Comment at: lib/CodeGen/CodeGenModule.cpp:907
+if (CGM.getTarget().supportsIFunc())
+Out << ".ifunc";
+  } else if (CGM.getTarget().supportsIFunc())

Missing indentation on this line.



Comment at: lib/CodeGen/CodeGenModule.cpp:910
 Out << ".resolver";
 }
 

Hmm, it looks like we don't have a unique name for a `GlobalDecl` naming the 
`CPUOrdinal == 0` case of an overloaded `cpu_specific` function:
```
__attribute__((cpu_specific(atom)))
void f() {} // CPUOrdinal 0 GD mangled as _Z1fv.ifunc
__attribute__((cpu_specific(haswell)))
void f() {} // CPUOrdinal 0 GD also mangled as _Z1fv.ifunc
```
Maybe we could append the CPU-specific manglings for all named CPUs before the 
`.ifunc`?



Comment at: lib/CodeGen/CodeGenModule.cpp:2133-2138
+  // If this is a cpu_dispatch multiversion function, designate it for emission
+  // at the end of the Translation Unit.
+  if (Global->hasAttr()) {
+MultiVersionFuncs.push_back(GD);
+return;
+  }

Instead of adding a special-case list of `MultiVersionFuncs`, could we instead 
change `MayBeEmittedEagerly` to return `false` for `CPUDispatch` functions and 
use the normal `DeferredDecls` mechanism for this?



Comment at: lib/CodeGen/CodeGenModule.cpp:2637
+if (!ImplDecl.getDecl()) {
+  ImplDecl = GD.getWithMultiVersionOrdinal(Ordinal);
 

I think we should have a comment somewhere explaining the subtle use of the 
multiversion ordinal:

 * (Resolver, 0) refers to the resolver itself
 * (Resolver, N) refers to an external (outside this module) symbol for the 
N'th CPU in Resolver's cpu_dispatch list
 * (Specific, 0) is a placeholder used to refer to the set of function symbols 
defined by a cpu_specific function declaration; no corresponding symbol is ever 
emitted (right?)
 * (Specific, N) refers to an internal (within this module) symbol for the N'th 
CPU in Specific's cpu_specific list



Comment at: lib/CodeGen/CodeGenModule.cpp:2639-2644
+  std::string MangledName = getMangledNameImpl(*this, ImplDecl, FD, true) +
+getCPUSpecificMangling(*this, II->getName());
+  Func = cast(GetOrCreateLLVMFunction(
+  MangledName, DeclTy, ImplDecl,
   /*ForVTable=*/false, /*DontDefer=*/true,
+  /*IsThunk=*/false, llvm::AttributeList(), ForDefinition));

I think the first part of this should now be equivalent to 
`getMangledName(...ImplDecl...)`, which should means that these 6 lines can be 
replaced by `GetAddrOfFunction(ImplDecl, DeclTy, false, false, 
NotForDefinition)`.

(Also I think you should pass `NotForDefinition` here because you just want to 
reference the symbol, you don't want to define it yourself.)



Comment at: lib/CodeGen/CodeGenModule.cpp:2645-2651
+} else {
+  if (ImplDecl.getDecl()->getAsFunction()->isDefined())
+EmitGlobalFunctionDefinition(ImplDecl, nullptr);
+  std::string MangledName = getMangledNameImpl(
+  *this, ImplDecl, ImplDecl.getDecl()->getAsFunction(), false);
+  Func = cast(GetGlobalValue(MangledName));
 }

This too should just be the same `GetAddrOfFunction` call, I think.


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

https://reviews.llvm.org/D55527



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

[PATCH] D55576: [libcxx] [test] [support] Use socket()+bind() to create unix sockets portably

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: EricWF, krytarowski.
Herald added subscribers: libcxx-commits, ldionne, emaste.

Replace the mknod() call with socket() + bind() for creating unix
sockets.  The mknod() method is not portable and does not work
on NetBSD while binding the socket should work on all systems supporting
unix sockets.

// NB: I think this should solve the issues w/ FreeBSD/Darwin too but I don't 
have those systems to test


Repository:
  rCXX libc++

https://reviews.llvm.org/D55576

Files:
  test/support/filesystem_dynamic_test_helper.py


Index: test/support/filesystem_dynamic_test_helper.py
===
--- test/support/filesystem_dynamic_test_helper.py
+++ test/support/filesystem_dynamic_test_helper.py
@@ -1,5 +1,6 @@
 import sys
 import os
+import socket
 import stat
 
 # Ensure that this is being run on a specific platform
@@ -76,8 +77,13 @@
 
 
 def create_socket(source):
-mode = 0o600 | stat.S_IFSOCK
-os.mknod(sanitize(source), mode)
+sock = socket.socket(socket.AF_UNIX)
+sanitized_source = sanitize(source)
+# AF_UNIX sockets may have very limited path length, so split it
+# into chdir call (with technically unlimited length) followed
+# by bind() relative to the directory
+os.chdir(os.path.dirname(sanitized_source))
+sock.bind(os.path.basename(sanitized_source))
 
 
 if __name__ == '__main__':


Index: test/support/filesystem_dynamic_test_helper.py
===
--- test/support/filesystem_dynamic_test_helper.py
+++ test/support/filesystem_dynamic_test_helper.py
@@ -1,5 +1,6 @@
 import sys
 import os
+import socket
 import stat
 
 # Ensure that this is being run on a specific platform
@@ -76,8 +77,13 @@
 
 
 def create_socket(source):
-mode = 0o600 | stat.S_IFSOCK
-os.mknod(sanitize(source), mode)
+sock = socket.socket(socket.AF_UNIX)
+sanitized_source = sanitize(source)
+# AF_UNIX sockets may have very limited path length, so split it
+# into chdir call (with technically unlimited length) followed
+# by bind() relative to the directory
+os.chdir(os.path.dirname(sanitized_source))
+sock.bind(os.path.basename(sanitized_source))
 
 
 if __name__ == '__main__':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Thanks for the feedback.

Once we decided on the style of the annotation, I will implement that and 
change the tests/documentation accordingly.




Comment at: include/clang/Basic/Attr.td:1204
+  VariadicUnsignedArgument<"PayloadIndices">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

aaron.ballman wrote:
> jdoerfert wrote:
> > aaron.ballman wrote:
> > > Should this also apply to Objective-C methods?
> > > 
> > > Why should the user specify this attribute on the function as opposed to 
> > > on the parameter? e.g.,
> > > ```
> > > // Why this:
> > > __attribute__((callback (1, 2, 3)))
> > > void* broker0(void* (*callee)(void *), void *payload, int otherPayload) {
> > >   return callee(payload);
> > > }
> > > 
> > > // Instead of this:
> > > void* broker0(void* (*callee)(void *) __attribute__((callback (2, 3))), 
> > > void *payload, int otherPayload) {
> > >   return callee(payload);
> > > }
> > > 
> > > // Or this:
> > > void* broker0(void* (*callee)(void *) __attribute__((callback (payload, 
> > > otherPayload))), void *payload, int otherPayload) {
> > >   return callee(payload);
> > > }
> > > ```
> > > I ask because these "use an index" attributes are really hard for users 
> > > to use in practice. They have to account for 0-vs-1 based indexing, 
> > > implicit this parameters, etc and if we can avoid that, it may be worth 
> > > the effort.
> > > Should this also apply to Objective-C methods?
> > 
> > I don't need it to and unless somebody does, I'd say no.
> > 
> > 
> > > I ask because these "use an index" attributes are really hard for users 
> > > to use in practice. They have to account for 0-vs-1 based indexing, 
> > > implicit this parameters, etc and if we can avoid that, it may be worth 
> > > the effort.
> > 
> > I was thinking that the function notation makes it clear that there is 
> > *only one callback per function* allowed right now. I don't expect many 
> > manual users of this feature until we improve the middle-end support, so it 
> > is unclear to me if this requirement needs to be removed as well.
> > 
> > Other than that, some thoughts: 
> > - I do not feel strongly about this.
> > - The middle requirement seems not much better n the first, we would still 
> > need to deal with index numbers (callbacks without arguments are not really 
> > interesting for now). 
> > - The last encoding requires us to define a symbol for "unknown argument" 
> > (maybe _ or ?).
> > I was thinking that the function notation makes it clear that there is 
> > *only one callback per function* allowed right now.
> 
> I don't see how that follows. Users may still try writing:
> ```
> __attribute__((callback (1, 3, 4)))
> __attribute__((callback (2, 3, 4)))
> void broker0(void (*cb1)(void *, int), void (*cb2)(void *, int), void 
> *payload, int otherPayload) {
>   cb1(payload, otherPayload);
>   cb2(payload, otherPayload);
> }
> ```
> and reasonably expect that to work (we should have this as a test case, and 
> probably warn on it).
> 
> I'm not strongly opposed to the current way this is exposed to users, just 
> wondering if we can find a better way to surface the feature.
> 
> > The last encoding requires us to define a symbol for "unknown argument" 
> > (maybe _ or ?).
> 
> Ah, I wasn't aware that this was part of the feature, but the documentation 
> you wrote helped to clarify for me. Personal preference is for `?`, but any 
> symbol will do (so long as we aren't hoping users can count commas, e.g., 
> `callback(frobble,,,foo)`).
> and reasonably expect that to work (we should have this as a test case, and 
> probably warn on it).

We have a test case and we'll spit out an error. (Sema/attr-callback-broken.c 
line 21 & 22)


> I'm not strongly opposed to the current way this is exposed to users, just 
> wondering if we can find a better way to surface the feature.

I can change it to the inlined style if nobody disagrees:

```
   void broker(int foo, void (*callback)(int, int, int, int) 
__attribute__((callback(foo, ?, bar, ?))), int bar);

```

As I mentioned, I don't have a strong opinion on this but I just don't want to 
change it back and forth :)



Repository:
  rC Clang

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

https://reviews.llvm.org/D55483



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

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 11.
jdoerfert marked 11 inline comments as done.
jdoerfert added a comment.

Fix and improve documentation


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483

Files:
  include/clang/AST/ASTContext.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/Builtins.def
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/Basic/Builtins.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Analysis/retain-release.m
  test/CodeGen/attr-callback.c
  test/CodeGen/callback_annotated.c
  test/CodeGen/callback_openmp.c
  test/CodeGen/callback_pthread_create.c
  test/CodeGen/callback_qsort_r.c
  test/CodeGenCXX/attr-callback.cpp
  test/Misc/pragma-attribute-supported-attributes-list.test
  test/OpenMP/parallel_codegen.cpp
  test/Sema/attr-callback-broken.c
  test/Sema/attr-callback.c
  test/SemaCXX/attr-callback-broken.cpp
  test/SemaCXX/attr-callback.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1275,6 +1275,8 @@
 Ptr = llvm::make_unique(Arg, Attr, "unsigned");
   else if (ArgName == "VariadicUnsignedArgument")
 Ptr = llvm::make_unique(Arg, Attr, "unsigned");
+  else if (ArgName == "VariadicSignedArgument")
+Ptr = llvm::make_unique(Arg, Attr, "int");
   else if (ArgName == "VariadicStringArgument")
 Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "VariadicEnumArgument")
Index: test/SemaCXX/attr-callback.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-callback.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+class C_in_class {
+#include "../Sema/attr-callback.c"
+};
+
+struct Base {
+
+  void no_args_1(void (*callback)(void));
+  __attribute__((callback(1))) void no_args_2(void (*callback)(void));
+  __attribute__((callback(1))) void no_args_3(void (*callback)(void)) {}
+
+  __attribute__((callback(1, 0))) virtual void
+  this_tr(void (*callback)(Base *));
+
+  __attribute__((callback(1, 0, -1, 0))) virtual void
+  this_unknown_this(void (*callback)(Base *, Base *, Base *));
+
+  __attribute__((callback(1))) virtual void
+  virtual_1(void (*callback)(void));
+
+  __attribute__((callback(1))) virtual void
+  virtual_2(void (*callback)(void));
+
+  __attribute__((callback(1))) virtual void
+  virtual_3(void (*callback)(void));
+};
+
+__attribute__((callback(1))) void
+Base::no_args_1(void (*callback)(void)) {
+}
+
+void Base::no_args_2(void (*callback)(void)) {
+}
+
+struct Derived_1 : public Base {
+
+  __attribute__((callback(1, 0))) virtual void
+  this_tr(void (*callback)(Base *)) override;
+
+  __attribute__((callback(1))) virtual void
+  virtual_1(void (*callback)(void)) override {}
+
+  virtual void
+  virtual_3(void (*callback)(void)) override {}
+};
+
+struct Derived_2 : public Base {
+
+  __attribute__((callback(1))) virtual void
+  virtual_1(void (*callback)(void)) override;
+
+  virtual void
+  virtual_2(void (*callback)(void)) override;
+
+  virtual void
+  virtual_3(void (*callback)(void)) override;
+};
+
+void Derived_2::virtual_1(void (*callback)(void)) {}
+
+__attribute__((callback(1))) void
+Derived_2::virtual_2(void (*callback)(void)) {}
+
+void Derived_2::virtual_3(void (*callback)(void)) {}
Index: test/SemaCXX/attr-callback-broken.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-callback-broken.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+class C_in_class {
+#define HAS_THIS
+#include "../Sema/attr-callback-broken.c"
+#undef HAS_THIS
+};
Index: test/Sema/attr-callback.c
===
--- /dev/null
+++ test/Sema/attr-callback.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+__attribute__((callback(1))) void no_args(void (*callback)(void));
+__attribute__((callback(1, 2, 3)))   void args_1(void (*callback)(int, double), int a, double b);
+__attribute__((callback(2, 3, 3)))   void args_2(int a, void (*callback)(double, double), double b);
+__attribute__((callback(2, -1, -1))) void args_3(int a, void (*callback)(double, double), double b);
Index: test/Sema/attr-callback-broken.c
===
--- /dev/null
+++ test/Sema/attr-callback-broken.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+__attribute__((callback())) void no_callee(void (*callback)(void)); // expected-error {{'callback' attribute takes at least 1 argument}}
+

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This header is used on systems without glibc. So please don't argue about 
behavior based only on that. Granted, most other libc implementation are less 
annoying when it comes to `free` and `malloc`, but still.


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

https://reviews.llvm.org/D43871



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

[PATCH] D55527: Normalize GlobalDecls when used with CPUDispatch

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 177767.
erichkeane marked 8 inline comments as done.
erichkeane added a comment.

Thanks @aaron.ballman .  This should fix all of your complaints.


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

https://reviews.llvm.org/D55527

Files:
  include/clang/AST/GlobalDecl.h
  include/clang/Basic/Attr.td
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGen/attr-cpuspecific.c

Index: test/CodeGen/attr-cpuspecific.c
===
--- test/CodeGen/attr-cpuspecific.c
+++ test/CodeGen/attr-cpuspecific.c
@@ -29,21 +29,7 @@
 
 ATTR(cpu_dispatch(ivybridge, knl))
 void TwoVersions(void);
-// LINUX: define void ()* @TwoVersions.resolver()
-// LINUX: call void @__cpu_indicator_init
-// LINUX: ret void ()* @TwoVersions.Z
-// LINUX: ret void ()* @TwoVersions.S
-// LINUX: call void @llvm.trap
-// LINUX: unreachable
-
-// WINDOWS: define dso_local void @TwoVersions()
-// WINDOWS: call void @__cpu_indicator_init()
-// WINDOWS: call void @TwoVersions.Z()
-// WINDOWS-NEXT: ret void
-// WINDOWS: call void @TwoVersions.S()
-// WINDOWS-NEXT: ret void
-// WINDOWS: call void @llvm.trap
-// WINDOWS: unreachable
+// Resolvers are emitted at the end, so the check lines are at the bottom.
 
 ATTR(cpu_specific(ivybridge))
 void TwoVersions(void){}
@@ -82,6 +68,59 @@
 // has an extra config to emit!
 ATTR(cpu_dispatch(ivybridge, knl, atom))
 void TwoVersionsSameAttr(void);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_dispatch(atom, ivybridge, knl))
+void ThreeVersionsSameAttr(void){}
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+// No Cpu Specific options.
+ATTR(cpu_dispatch(atom, ivybridge, knl))
+void NoSpecifics(void);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_dispatch(atom, generic, ivybridge, knl))
+void HasGeneric(void);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_dispatch(atom, generic, ivybridge, knl))
+void HasParams(int i, double d);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_dispatch(atom, generic, ivybridge, knl))
+int HasParamsAndReturn(int i, double d);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_dispatch(atom, generic, pentium))
+int GenericAndPentium(int i, double d);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_dispatch(atom, pentium))
+int DispatchFirst(void);
+// Resolvers are emitted at the end, so the check lines are at the bottom.
+
+ATTR(cpu_specific(atom))
+int DispatchFirst(void) {return 0;}
+ATTR(cpu_specific(pentium))
+int DispatchFirst(void) {return 1;}
+// Resolver emit causes these to be emited, so they happen later.
+
+// LINUX: define void ()* @TwoVersions.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @TwoVersions.Z
+// LINUX: ret void ()* @TwoVersions.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define dso_local void @TwoVersions()
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @TwoVersions.Z()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @TwoVersions.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 // LINUX: define void ()* @TwoVersionsSameAttr.resolver()
 // LINUX: ret void ()* @TwoVersionsSameAttr.Z
 // LINUX: ret void ()* @TwoVersionsSameAttr.S
@@ -99,8 +138,6 @@
 // WINDOWS: call void @llvm.trap
 // WINDOWS: unreachable
 
-ATTR(cpu_dispatch(atom, ivybridge, knl))
-void ThreeVersionsSameAttr(void){}
 // LINUX: define void ()* @ThreeVersionsSameAttr.resolver()
 // LINUX: call void @__cpu_indicator_init
 // LINUX: ret void ()* @ThreeVersionsSameAttr.Z
@@ -120,9 +157,16 @@
 // WINDOWS: call void @llvm.trap
 // WINDOWS: unreachable
 
-// No Cpu Specific options.
-ATTR(cpu_dispatch(atom, ivybridge, knl))
-void NoSpecifics(void);
+// LINUX: define i32 @DispatchFirst.O
+// LINUX: ret i32 0
+// LINUX: define i32 @DispatchFirst.B
+// LINUX: ret i32 1
+
+// WINDOWS: define dso_local i32 @DispatchFirst.O()
+// WINDOWS: ret i32 0
+// WINDOWS: define dso_local i32 @DispatchFirst.B
+// WINDOWS: ret i32 1
+
 // LINUX: define void ()* @NoSpecifics.resolver()
 // LINUX: call void @__cpu_indicator_init
 // LINUX: ret void ()* @NoSpecifics.Z
@@ -142,8 +186,6 @@
 // WINDOWS: call void @llvm.trap
 // WINDOWS: unreachable
 
-ATTR(cpu_dispatch(atom, generic, ivybridge, knl))
-void HasGeneric(void);
 // LINUX: define void ()* @HasGeneric.resolver()
 // LINUX: call void @__cpu_indicator_init
 // LINUX: ret void ()* @HasGeneric.Z
@@ -164,8 +206,6 @@
 // WINDOWS-NEXT: ret void
 // WINDOWS-NOT: call void @llvm.trap
 
-ATTR(cpu_dispatch(atom, generic, ivybridge, knl))
-void HasParams(int i, double d);
 // LINUX: define void (i32, double)* @HasParams.resolver()
 // LINUX: call 

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> What does it do with floating-point inputs?

Same as all the other atomics: run screaming for the hills (or error out, in 
more reasonable terms). The only way to implement floating atomics in LLVM IR 
would be with a compare-exchange loop and Clang doesn't expose anything to make 
that more convenient.

Min/max have at least some pedigree in integer terms. OpenCL has specified 
them, CPUs have implemented them, and we've had requests for builtins from 
users.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562



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

[PATCH] D55527: Normalize GlobalDecls when used with CPUDispatch

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:896
   let AdditionalMembers = [{
-IdentifierInfo *getCPUName(unsigned Index) const {
-  return *(cpus_begin() + Index);
+// gets the ordinal of the requested CPU name, or 0 if it isn't in the cpu
+// list.

gets -> Gets



Comment at: include/clang/Basic/Attr.td:901
+  for (const IdentifierInfo *II : cpus()) {
+if (II->getName() == Name)
+  return Ord;

`if (II->isStr(Name))`



Comment at: include/clang/Basic/Attr.td:908
+
+IdentifierInfo *getCPUName(unsigned Ordinal) const {
+  assert(Ordinal > 0 && "Invalid Ordinal");

Can this return a `const IdentifierInfo *` instead?



Comment at: lib/CodeGen/CodeGenModule.cpp:2141
+  if (Global->hasAttr() && GD.getMultiVersionOrdinal() == 0) {
+auto *Spec = Global->getAttr();
+for (unsigned I = 1; I <= Spec->cpus_size(); ++I)

Rather than use `hasAttr<>` followed by `getAttr<>`, just get the attribute and 
test it for null.

Also, `const auto *`.



Comment at: lib/CodeGen/CodeGenModule.cpp:2442
 // Requires multiple emits.
+auto *Spec = FD->getAttr();
+for (unsigned I = 1; I <= Spec->cpus_size(); ++I)

`const auto *`



Comment at: lib/CodeGen/CodeGenModule.cpp:2562
+  for (GlobalDecl GD : MultiVersionFuncs) {
+const FunctionDecl *FD = cast(GD.getDecl());
+assert(FD && FD->isMultiVersion() && "Not a multiversion function?");

`const auto *`



Comment at: lib/CodeGen/CodeGenModule.cpp:2584
+  llvm::Type *) {
+  const FunctionDecl *FD = cast(GD.getDecl());
   QualType CanonTy = Context.getCanonicalType(FD->getType());

`const auto *`



Comment at: lib/CodeGen/CodeGenModule.cpp:2612
   const TargetInfo  = getTarget();
-  unsigned Index = 0;
+  const FunctionDecl *FD = cast(GD.getDecl());
+  assert(FD->isCPUDispatchMultiVersion() &&

`const auto *`


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

https://reviews.llvm.org/D55527



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

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:1204
+  VariadicUnsignedArgument<"PayloadIndices">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

jdoerfert wrote:
> aaron.ballman wrote:
> > Should this also apply to Objective-C methods?
> > 
> > Why should the user specify this attribute on the function as opposed to on 
> > the parameter? e.g.,
> > ```
> > // Why this:
> > __attribute__((callback (1, 2, 3)))
> > void* broker0(void* (*callee)(void *), void *payload, int otherPayload) {
> >   return callee(payload);
> > }
> > 
> > // Instead of this:
> > void* broker0(void* (*callee)(void *) __attribute__((callback (2, 3))), 
> > void *payload, int otherPayload) {
> >   return callee(payload);
> > }
> > 
> > // Or this:
> > void* broker0(void* (*callee)(void *) __attribute__((callback (payload, 
> > otherPayload))), void *payload, int otherPayload) {
> >   return callee(payload);
> > }
> > ```
> > I ask because these "use an index" attributes are really hard for users to 
> > use in practice. They have to account for 0-vs-1 based indexing, implicit 
> > this parameters, etc and if we can avoid that, it may be worth the effort.
> > Should this also apply to Objective-C methods?
> 
> I don't need it to and unless somebody does, I'd say no.
> 
> 
> > I ask because these "use an index" attributes are really hard for users to 
> > use in practice. They have to account for 0-vs-1 based indexing, implicit 
> > this parameters, etc and if we can avoid that, it may be worth the effort.
> 
> I was thinking that the function notation makes it clear that there is *only 
> one callback per function* allowed right now. I don't expect many manual 
> users of this feature until we improve the middle-end support, so it is 
> unclear to me if this requirement needs to be removed as well.
> 
> Other than that, some thoughts: 
> - I do not feel strongly about this.
> - The middle requirement seems not much better n the first, we would still 
> need to deal with index numbers (callbacks without arguments are not really 
> interesting for now). 
> - The last encoding requires us to define a symbol for "unknown argument" 
> (maybe _ or ?).
> I was thinking that the function notation makes it clear that there is *only 
> one callback per function* allowed right now.

I don't see how that follows. Users may still try writing:
```
__attribute__((callback (1, 3, 4)))
__attribute__((callback (2, 3, 4)))
void broker0(void (*cb1)(void *, int), void (*cb2)(void *, int), void *payload, 
int otherPayload) {
  cb1(payload, otherPayload);
  cb2(payload, otherPayload);
}
```
and reasonably expect that to work (we should have this as a test case, and 
probably warn on it).

I'm not strongly opposed to the current way this is exposed to users, just 
wondering if we can find a better way to surface the feature.

> The last encoding requires us to define a symbol for "unknown argument" 
> (maybe _ or ?).

Ah, I wasn't aware that this was part of the feature, but the documentation you 
wrote helped to clarify for me. Personal preference is for `?`, but any symbol 
will do (so long as we aren't hoping users can count commas, e.g., 
`callback(frobble,,,foo)`).



Comment at: include/clang/Basic/AttrDocs.td:3711
+The ``callback`` attribute specifies that the annotated function may invoke the
+specified callback callee zero or more times. The callback callee, as well as
+the passed arguments, are identified by their parameter position (starting with

I'd remove `callee` here in both places.



Comment at: include/clang/Basic/AttrDocs.td:3714
+1!) in the annotated function. The first position identifies the callback 
callee,
+the following indices the forwarded arguments. The callback callee is required
+to be callable with the number, and order, of the specified arguments. To

and the following indicies are the forwarded arguments.



Comment at: include/clang/Basic/AttrDocs.td:3716
+to be callable with the number, and order, of the specified arguments. To
+represent unknown arguments a zero shall be used.
+

The index '0' is used to represent a callback callee argument which is not 
present in the declared parameter list.



Comment at: include/clang/Basic/AttrDocs.td:3718
+
+The ``callback`` attributes, which are directly translated to ``callback``
+metadata , allow to make

attributes -> attribute



Comment at: include/clang/Basic/AttrDocs.td:3719
+The ``callback`` attributes, which are directly translated to ``callback``
+metadata , allow to make
+the connection between the call to the annotated function and the callback

, allow to make the connection -> , make the 

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

That is, it is modified within the recursive call.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54921



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

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think that it is modified, at least, when `Memoization[C]` is evaluated. The 
object needs to be created and put into the map in order to obtain a reference 
to it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54921



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

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

@NoQ thanks, interesting! But I thought the underlying map is not actually 
modified while the reference is alive?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54921



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

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, i think i get it. `Cached` is a reference. Changing `Memoization` will 
invalidate references because `DenseMap` doesn't provide the respective 
guarantee.

Here's how the code should have looked:

  ~  77 Optional Cached = Memoization[C];
  ~  78 if (!Cached) {
 79   Cached = seenBeforeRec(C, A, B, Memoization);
  +  80   Memoization[C] = Cached;
  +  81 }


Repository:
  rC Clang

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

https://reviews.llvm.org/D54921



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

[PATCH] D55410: [clang-tidy] check for flagging using declarations in headers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: docs/clang-tidy/checks/abseil-alias-free-headers.rst:13
+The style guide  http://google.github.io/styleguide/cppguide.html#Aliases 
discusses this
+issue in more detail

Eugene.Zelenko wrote:
> Naysh wrote:
> > Eugene.Zelenko wrote:
> > > Missing link to guidelines,
> > @Eugene.Zelenko: Sorry, I'm not sure what you mean. Could you clarify what 
> > guidelines you're referring to? 
> Please see documentation in other using-related checks.
I think he's talking about the fact that this is text and not a hyperlink. It 
should look something more like
```
The `Abseil Style Guide 
`_ discusses this 
issue in more detail.
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55410



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

[PATCH] D55447: [Sema] Fix Modified Type in address_space AttributedType

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/tools/libclang/CXType.cpp:132
+  if (!(TU->ParsingOptions & CXTranslationUnit_IncludeAttributedTypes) &&
+  ATT->getAttrKind() != attr::AddressSpace) {
 return MakeCXType(ATT->getModifiedType(), TU);

leonardchan wrote:
> aaron.ballman wrote:
> > leonardchan wrote:
> > > aaron.ballman wrote:
> > > > This change seems surprising -- if the parsing options say the caller 
> > > > does not want attributed types, why are we returning one anyway for 
> > > > address space?
> > > This has to do with ensuring `clang_getAddressSpace` still returns the 
> > > proper address_space. It does this by essentially checking the qualifiers 
> > > of the type, which we now attach to the `AttributedType` whereas before 
> > > it was attached to the modified type.
> > > 
> > > This extra condition is necessary for ensuring that calling 
> > > `clang_getAddressSpace` points to the qualified AttributedType instead of 
> > > the unqualified modified type.
> > My fear is that this will be breaking assumptions in third-party code. If 
> > someone disables `CXTranslationUnit_IncludeAttributedTypes`, they are 
> > unlikely to expect to receive an `AttributedType` and may react poorly to 
> > it.
> Instead check if the type is address_space attributed and apply the 
> qualifiers the modified type.
Sure, they can always modify their code to handle it gracefully, but it's a 
silent, breaking change to a somewhat stable API which is why I was prodding 
about it.

After talking with @rsmith, we're thinking the correct change here is to return 
the attributed type when the user asks for it, but return the equivalent type 
rather than the modified type if the user doesn't want attribute sugar (for all 
attributes, not just address spaces). This way, when a user asks for CXType for 
one of these, they always get a correct type but sometimes it has attribute 
type sugar and sometimes it doesn't, depending on the parsing options.

This is still a silent, breaking change in behavior, which is unfortunate. It 
should probably come with a mention in the release notes about the change to 
the API and some unit tests as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55447



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

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 177753.
NoQ added a comment.

Add documentation, fix checker help message.


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

https://reviews.llvm.org/D55566

Files:
  docs/analyzer/DebugChecks.rst
  include/clang/Analysis/Analyses/LiveVariables.h
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  test/Analysis/live-stmts.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -784,6 +784,45 @@
   }
 }
 
+void checkMoreLoopZombies1(bool flag) {
+  while (flag) {
+Empty e{};
+if (true)
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
+bool coin();
+
+void checkMoreLoopZombies2(bool flag) {
+  while (flag) {
+Empty e{};
+while (coin())
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
+void checkMoreLoopZombies3(bool flag) {
+  while (flag) {
+Empty e{};
+do
+  e; // expected-warning {{expression result unused}}
+while (coin());
+Empty f = std::move(e); // no-warning
+  }
+}
+
+void checkMoreLoopZombies4(bool flag) {
+  while (flag) {
+Empty e{};
+for (; coin();)
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
 struct MoveOnlyWithDestructor {
   MoveOnlyWithDestructor();
   ~MoveOnlyWithDestructor();
Index: test/Analysis/live-stmts.cpp
===
--- /dev/null
+++ test/Analysis/live-stmts.cpp
@@ -0,0 +1,167 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveStmts %s 2>&1\
+// RUN:   | FileCheck %s
+
+int coin();
+
+
+int testThatDumperWorks(int x, int y, int z) {
+  return x ? y : z;
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 
+// CHECK-NEXT:   `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 
+// CHECK-NEXT:   `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 
+// CHECK-NEXT:   `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testIfBranchExpression(bool flag) {
+  // No expressions should be carried over from one block to another here.
+  while (flag) {
+int e = 1;
+if (true)
+  e;
+  }
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testWhileBodyExpression(bool flag) {
+  // No expressions should be carried over from one block to another here.
+  while (flag) {
+int e = 1;
+while (coin())
+  e;
+  }
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testDoWhileBodyExpression(bool flag) {
+  // No expressions should be carried over from one block to another here.
+  while (flag) {
+int e = 1;
+do
+  

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked an inline comment as done.
EricWF added a comment.

I think this is good to go. Any more comments?




Comment at: include/clang/Sema/Sema.h:2758
 bool AllowExplicit = false,
+bool IsADLCandidate = false,
 ConversionSequenceList EarlyConversions = None);

EricWF wrote:
> rsmith wrote:
> > Long lists of bool arguments make me nervous, especially with default 
> > arguments. Can you introduce an enum for the new param at least?
> Ack. I was thinking the same thing. 
And you were right to be nervous. The change found bugs.


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

https://reviews.llvm.org/D55534



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

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 177752.
EricWF marked 3 inline comments as done.
EricWF added a comment.

Address review comments.


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

https://reviews.llvm.org/D55534

Files:
  docs/LibASTMatchersReference.html
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/Stmt.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/AST/ast-dump-expr.cpp
  test/Import/call-expr/Inputs/F.cpp
  test/Import/call-expr/test.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,40 @@
"-fno-delayed-template-parsing"));
 }
 
+TEST(Matcher, ADLCall) {
+  StatementMatcher ADLMatch = callExpr(usesADL());
+  StatementMatcher ADLMatchOper = cxxOperatorCallExpr(usesADL());
+  auto NS_Str = R"cpp(
+  namespace NS {
+struct X {};
+void f(X);
+void operator+(X, X);
+  }
+  struct MyX {};
+  void f(...);
+  void operator+(MyX, MyX);
+)cpp";
+
+  auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+  };
+
+  EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+  EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+  EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+  EXPECT_TRUE(notMatches(MkStr("NS::X x; using NS::f; f(x);"), ADLMatch));
+
+  // Operator call expressions
+  EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatch));
+  EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatchOper));
+  EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatch));
+  EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatchOper));
+  EXPECT_TRUE(matches(MkStr("NS::X x; operator+(x, x);"), ADLMatch));
+  EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::operator+(x, x);"), ADLMatch));
+}
+
 TEST(Matcher, Call) {
   // FIXME: Do we want to overload Call() to directly take
   // Matcher, too?
Index: test/Import/call-expr/test.cpp
===
--- /dev/null
+++ test/Import/call-expr/test.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+void expr() {
+  f();
+}
+
+// CHECK: FunctionDecl 0x{{[^ ]*}} <{{[^>]*}}> line:{{.*}}:{{[^ ]*}} used f 'void ()'
+// CHECK: -CallExpr 0x{{[^ ]*}} <{{[^>]*}}> 'void' adl
+// CHECK: -CXXOperatorCallExpr 0x{{[^ ]*}} <{{[^>]*}}> 'void' adl
Index: test/Import/call-expr/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/call-expr/Inputs/F.cpp
@@ -0,0 +1,10 @@
+namespace NS {
+struct X {};
+void f(X) {}
+void operator+(X, X) {}
+} // namespace NS
+void f() {
+  NS::X x;
+  f(x);
+  x + x;
+}
Index: test/AST/ast-dump-expr.cpp
===
--- test/AST/ast-dump-expr.cpp
+++ test/AST/ast-dump-expr.cpp
@@ -508,3 +508,46 @@
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'Ts...' lvalue ParmVar 0x{{[^ ]*}} 'a' 'Ts...'
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'b' 'int'
 }
+
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+  NS::X x;
+  // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+  f(x);
+  // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+  y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+  NS::X x;
+  // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+  NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+  NS::X x;
+  using NS::f;
+  // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+  f(x);
+  // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+  y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+  X x;
+  // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+  f(x);
+}
+} // namespace test_adl_call_three
\ No newline at end of file
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -651,6 +651,7 @@
   for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
 Record.AddStmt(*Arg);
+  

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Aaron Ballman via Phabricator via cfe-commits]

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

I had to revert the commit as the changes caused some test failures.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/40784/steps/test/logs/stdio

  FAIL: Clang :: CodeGenOpenCL/printf.cl (8013 of 45545)
   TEST 'Clang :: CodeGenOpenCL/printf.cl' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang
 -cc1 -internal-isystem 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include
 -nostdsysteminc -cl-std=CL1.2 -cl-ext=-+cl_khr_fp64 -triple 
spir-unknown-unknown -disable-llvm-passes -emit-llvm -o - 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
 | 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/FileCheck
 -check-prefixes=FP64,ALL 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
  : 'RUN: at line 2';   
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang
 -cc1 -internal-isystem 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include
 -nostdsysteminc -cl-std=CL1.2 -cl-ext=-cl_khr_fp64 -triple 
spir-unknown-unknown -disable-llvm-passes -emit-llvm -o - 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
 | 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/FileCheck
 -check-prefixes=NOFP64,ALL 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:20:18:
 warning: format specifies type 'double __attribute__((ext_vector_type(2)))' 
but the argument has type 'float2' (vector of 2 'float' values)
printf("%v2f", arg);
   ^~~
%v2f
  clang: 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/AST/ASTContext.cpp:5554:
 FloatingRank getFloatingRank(clang::QualType): Assertion 
`T->getAs() && "getFloatingRank(): not a floating type"' failed.
  Stack dump:
  0.Program arguments: 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang
 -cc1 -internal-isystem 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include
 -nostdsysteminc -cl-std=CL1.2 -cl-ext=-cl_khr_fp64 -triple 
spir-unknown-unknown -disable-llvm-passes -emit-llvm -o - 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
 
  1.
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:30:21:
 current parser token ')'
  2.
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:29:42:
 parsing function body 'test_printf_half2'
  3.
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:29:42:
 in compound statement ('{}')
  #0 0x015d9934 PrintStackTraceSignalHandler(void*) 
(/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d9934)
  #1 0x015d769e llvm::sys::RunSignalHandlers() 
(/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d769e)
  #2 0x015d9af8 SignalHandler(int) 
(/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d9af8)
  #3 0x7f95cde44890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
  #4 0x7f95ccb0ae97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
  #5 0x7f95ccb0c801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
  #6 0x7f95ccafc39a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
  #7 0x7f95ccafc412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
  #8 0x032f872c getFloatingRank(clang::QualType) 
(/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x32f872c)
  #9 0x032f877e 
clang::ASTContext::getFloatingTypeOrder(clang::QualType, clang::QualType) const 
(/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x32f877e)
  #10 0x02bf9316 (anonymous 

r348892 - Revert r348889; it fails some tests.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Dec 11 11:42:04 2018
New Revision: 348892

URL: http://llvm.org/viewvc/llvm-project?rev=348892=rev
Log:
Revert r348889; it fails some tests.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/40784

Removed:
cfe/trunk/test/Sema/format-strings-bitfield-promotion.c
cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=348892=348891=348892=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 11 11:42:04 2018
@@ -7709,24 +7709,6 @@ shouldNotPrintDirectly(const ASTContext
   return std::make_pair(QualType(), StringRef());
 }
 
-/// Return true if \p ICE is an implicit argument promotion of an arithmetic
-/// type. Bit-field 'promotions' from a higher ranked type to a lower ranked
-/// type do not count.
-static bool
-isArithmeticArgumentPromotion(Sema , const ImplicitCastExpr *ICE) {
-  QualType From = ICE->getSubExpr()->getType();
-  QualType To = ICE->getType();
-  // It's a floating promotion if the source type is a lower rank.
-  if (ICE->getCastKind() == CK_FloatingCast &&
-  S.Context.getFloatingTypeOrder(From, To) < 0)
-return true;
-  // It's an integer promotion if the destination type is the promoted
-  // source type.
-  return ICE->getCastKind() == CK_IntegralCast &&
- From->isPromotableIntegerType() &&
- S.Context.getPromotedIntegerType(From) == To;
-}
-
 bool
 CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier ,
 const char *StartSpecifier,
@@ -7754,11 +7736,11 @@ CheckPrintfHandler::checkFormatExpr(cons
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
-  // and function pointer decay (seeing that an argument intended to be a
-  // string has type 'char [6]' is probably more confusing than 'char *') and
-  // certain bitfield promotions (bitfields can be 'demoted' to a lesser type).
+  // and function pointer decay; seeing that an argument intended to be a
+  // string has type 'char [6]' is probably more confusing than 'char *'.
   if (const ImplicitCastExpr *ICE = dyn_cast(E)) {
-if (isArithmeticArgumentPromotion(S, ICE)) {
+if (ICE->getCastKind() == CK_IntegralCast ||
+ICE->getCastKind() == CK_FloatingCast) {
   E = ICE->getSubExpr();
   ExprTy = E->getType();
 

Removed: cfe/trunk/test/Sema/format-strings-bitfield-promotion.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-bitfield-promotion.c?rev=348891=auto
==
--- cfe/trunk/test/Sema/format-strings-bitfield-promotion.c (original)
+++ cfe/trunk/test/Sema/format-strings-bitfield-promotion.c (removed)
@@ -1,18 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
-
-int printf(const char *restrict, ...);
-
-struct bitfields {
-  long a : 2;
-  unsigned long b : 2;
-  long c : 32;  // assumes that int is 32 bits
-  unsigned long d : 32; // assumes that int is 32 bits
-} bf;
-
-void bitfield_promotion() {
-  printf("%ld", bf.a); // expected-warning {{format specifies type 'long' but 
the argument has type 'int'}}
-  printf("%lu", bf.b); // expected-warning {{format specifies type 'unsigned 
long' but the argument has type 'int'}}
-  printf("%ld", bf.c); // expected-warning {{format specifies type 'long' but 
the argument has type 'int'}}
-  printf("%lu", bf.d); // expected-warning {{format specifies type 'unsigned 
long' but the argument has type 'unsigned int'}}
-}

Removed: cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx?rev=348891=auto
==
--- cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx (original)
+++ cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx (removed)
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
-
-// In C++, the bitfield promotion from long to int does not occur, unlike C.
-// expected-no-diagnostics
-
-int printf(const char *restrict, ...);
-
-struct bitfields {
-  long a : 2;
-  unsigned long b : 2;
-  long c : 32;  // assumes that int is 32 bits
-  unsigned long d : 32; // assumes that int is 32 bits
-} bf;
-
-void bitfield_promotion() {
-  printf("%ld", bf.a);
-  printf("%lu", 

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

[Artem Dergachev via Phabricator via cfe-commits]

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

For the first time in years, there seems to be a bug in our "live variables" 
analysis, which is an auxiliary data flow analysis that decides which variables 
and expressions are live, so that we could collect dead symbols, clean up the 
program state, and, ultimately, find memory leaks.

The bug shows up on the move-checker, as demonstrated on attached tests (except 
the do-while test, which worked fine, but i added it to make sure it stays that 
way). It was originally hidden by D54372  and 
seems to have been another reason why the hack that was removed in D54372 
 was introduced in the first place.

Consider `checkMoreLoopZombies1()` (other tests are similar). `LiveVariables` 
analysis reports that //expression// `e` that constitutes the true-branch of 
`if` is live throughout almost the whole body of the loop (!), namely, until 
the beginning of `if (true)` "on the next iteration" (quotes because it doesn't 
really make sense for live variables analysis, but kinda makes sense for us to 
think of it this way). There's a brief period when it doesn't live, but it 
quickly becomes live again. The expression, being an lvalue, evaluates to the 
`VarRegion` for variable `e`. This means that at least one of the two - the 
variable `e` itself and the expression that evaluates to the region `e` - is 
always live, and region `e` never becomes dead, and the moved-from state never 
gets cleaned up, which leads to an invalid use-after-move report on the second 
iteration of the loop.

Expressions don't need to be live after the first statement that contains them 
is evaluated. So the liveness analysis was overly conservative in this case, 
and it caused problems for a checker that relies on values being cleaned up 
perfectly.

It was essential that the expression `e` appears directly as a sub-statement of 
`if`, i.e. written without `{`...`}`. Otherwise the compound statement would 
have been marked as live instead, which is kinda fine, even if it doesn't make 
sense.

Because of that, when the `if`-statement is being evaluated as a terminator, 
the generic code that works for pretty much all statements was marking the 
expression as live:

  for (Stmt *Child : S->children()) {
if (Child)
  AddLiveStmt(val.liveStmts, LV.SSetFact, Child);

This caused `e` to be incorrectly marked as live at the end of block that 
terminates at `if (true)`, which is propagated to the beginning of that block 
(similarly to how in `x ? y : z` expressions `y` and `z` need to live at 
operator `x ?`, so that the operator could have been evaluated), which is then 
propagated to the while-loop terminator.

This is fixed by specializing the generic case for these control flow operators 
to avoid the unnecessary propagation of non-compound-statement branch liveness.

-

Because i wanted to add more direct tests for this stuff (as we had none), i 
had to introduce a new debug checker to dump statement liveness: 
`debug.DumpLiveStmts`. Dumps should be human-readable and testable.

These direct tests indicate that the same incorrect behavior does indeed occur 
in the do-while loop case, even if it doesn't have much consequences for the 
move-checker.

-

I'll add more tests if i notice that this change increases the number of leaks 
Static Analyzer finds.


Repository:
  rC Clang

https://reviews.llvm.org/D55566

Files:
  include/clang/Analysis/Analyses/LiveVariables.h
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  test/Analysis/live-stmts.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -784,6 +784,45 @@
   }
 }
 
+void checkMoreLoopZombies1(bool flag) {
+  while (flag) {
+Empty e{};
+if (true)
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
+bool coin();
+
+void checkMoreLoopZombies2(bool flag) {
+  while (flag) {
+Empty e{};
+while (coin())
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
+void checkMoreLoopZombies3(bool flag) {
+  while (flag) {
+Empty e{};
+do
+  e; // expected-warning {{expression result unused}}
+while (coin());
+Empty f = std::move(e); // no-warning
+  }
+}
+
+void checkMoreLoopZombies4(bool flag) {
+  while (flag) {
+Empty e{};
+for (; coin();)
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
 struct 

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 13 inline comments as done.
EricWF added inline comments.



Comment at: include/clang/Sema/Sema.h:2758
 bool AllowExplicit = false,
+bool IsADLCandidate = false,
 ConversionSequenceList EarlyConversions = None);

rsmith wrote:
> Long lists of bool arguments make me nervous, especially with default 
> arguments. Can you introduce an enum for the new param at least?
Ack. I was thinking the same thing. 



Comment at: lib/AST/ASTImporter.cpp:7387
   return new (Importer.getToContext()) CallExpr(
   Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(),
   ToRParenLoc);

rsmith wrote:
> Do we need to pass through the usesADL flag here too?
Seems like it. I'll add tests as well.



Comment at: lib/AST/Expr.cpp:1267
 : Expr(SC, Empty), NumArgs(NumArgs) {
+  CallExprBits.UsesADL = false;
   CallExprBits.NumPreArgs = NumPreArgs;

riccibruno wrote:
> riccibruno wrote:
> > I believe that msan can cope with bit level operations just fine.
> > What I am afraid is that initializing it here will hide the
> > fact that it is not initialized during deserialization if the
> > `E->setUsesADL(Record.readInt());` is removed by mistake.
> > 
> > At least not initializing it here will leave a chance for msan
> > to catch this.
> I don't think they do.
Regardless. I'll remove the initialization.

I think potentially getting a random value will make it easier to spot bugs.


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

https://reviews.llvm.org/D55534



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

[PATCH] D55561: Stop stripping comments from AST matcher example code

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Committed in r348891.


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

https://reviews.llvm.org/D55561



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

r348891 - Stop stripping comments from AST matcher example code.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Dec 11 11:30:49 2018
New Revision: 348891

URL: http://llvm.org/viewvc/llvm-project?rev=348891=rev
Log:
Stop stripping comments from AST matcher example code.

The AST matcher documentation dumping script was being a bit over-zealous about 
stripping comment markers, which ended up causing comments in example code to 
stop being comments. Fix that by only stripping comments at the start of a 
line, rather than removing any forward slash (which also impacts prose text).

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/docs/tools/dump_ast_matchers.py
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=348891=348890=348891=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Tue Dec 11 11:30:49 2018
@@ -499,7 +499,7 @@ Given
   int X;
   namespace NS {
   int Y;
-  }  namespace NS
+  }  // namespace NS
 decl(hasDeclContext(translationUnitDecl()))
   matches "int X", but not "int Y".
 
@@ -1152,7 +1152,7 @@ Example matches std::string()
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtfloatLiteralMatcherhttps://clang.llvm.org/doxygen/classclang_1_1FloatingLiteral.html;>FloatingLiteral...
-Matches float literals 
of all sizes encodings, e.g.
+Matches float literals 
of all sizes / encodings, e.g.
 1.0, 1.0f, 1.0L and 1e10.
 
 Does not match implicit conversions such as
@@ -1230,7 +1230,7 @@ initListExpr()
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtintegerLiteralMatcherhttps://clang.llvm.org/doxygen/classclang_1_1IntegerLiteral.html;>IntegerLiteral...
-Matches integer 
literals of all sizes encodings, e.g.
+Matches integer 
literals of all sizes / encodings, e.g.
 1, 1L, 0x1 and 1U.
 
 Does not match character-encoded integers such as L'a'.
@@ -1911,8 +1911,8 @@ Given
   template typename T
   class C { };
 
-  template class Cint;  A
-  Cchar var;B
+  template class Cint;  // A
+  Cchar var;// B
 
 templateSpecializationType() matches the type of the explicit
 instantiation in A and the type of the variable declaration in B.
@@ -2091,13 +2091,12 @@ Usable as: Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclisImplicit
 Matches a declaration 
that has been implicitly added
-by the compiler (eg. implicit defaultcopy constructors).
+by the compiler (eg. implicit default/copy constructors).
 
 
 
@@ -2942,7 +2941,7 @@ Given:
   class A { int operator*(); };
   const A operator(const A a, const A b);
   A a;
-  a  a;   -- This matches
+  a  a;   // -- This matches
 
 cxxOperatorCallExpr(hasOverloadedOperatorName(""))) matches the
 specified line and
@@ -2995,13 +2994,13 @@ functionDecl(isDefaulted())
 
 Example matches A, va, fa
   class A {};
-  class B;  Doesn't match, as it has no body.
+  class B;  // Doesn't match, as it has no body.
   int va;
-  extern int vb;  Doesn't match, as it doesn't define the variable.
+  extern int vb;  // Doesn't match, as it doesn't define the variable.
   void fa() {}
-  void fb();  Doesn't match, as it has no body.
+  void fb();  // Doesn't match, as it has no body.
   @interface X
-  - (void)ma; Doesn't match, interface is declaration.
+  - (void)ma; // Doesn't match, interface is declaration.
   @end
   @implementation X
   - (void)ma {}
@@ -3104,7 +3103,7 @@ functionDecl(isNoThrow()) and functionPr
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html;>FunctionDeclisStaticStorageClass
-Matches 
variablefunction declarations that have "static" storage
+Matches 
variable/function declarations that have "static" storage
 class specifier ("static" keyword) written in the source.
 
 Given:
@@ -3360,7 +3359,7 @@ Example matches X (regexp is one of "::X
 
 Given
   namespace n {
-  namespace {} #1
+  namespace {} // #1
   }
 namespaceDecl(isAnonymous()) will match #1 but not ::n.
 
@@ -3401,7 +3400,7 @@ message expression in
   CGRect bodyFrame = webView.frame;
   bodyFrame.size.height = self.bodyContentHeight;
   webView.frame = bodyFrame;
-  ^ matches here
+  // ^ matches here
 
 
 
@@ -3473,13 +3472,13 @@ a substring matched by the given RegExp.
 
 Example matches A, va, fa
   class A {};
-  class B;  Doesn't match, as it has no body.
+  class B;  // Doesn't match, as it has no body.
   int va;
-  extern int vb;  Doesn't match, as it doesn't define the variable.
+  extern int vb;  // Doesn't match, as it doesn't define the variable.
   void fa() {}
-  void fb();  Doesn't match, as it has no body.
+  void fb();  // Doesn't match, as it has no body.
   @interface X
-  - (void)ma; Doesn't match, interface is declaration.
+  - (void)ma; // Doesn't match, interface is declaration.
   @end
   @implementation X
   - (void)ma 

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 177749.
hwright marked 6 inline comments as done.
hwright added a comment.

Rebase


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

https://reviews.llvm.org/D55245

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/DurationSubtractionCheck.cpp
  clang-tidy/abseil/DurationSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-subtraction.cpp

Index: test/clang-tidy/abseil-duration-subtraction.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-subtraction.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s abseil-duration-subtraction %t -- -- -I %S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d, d1, d2;
+
+  x = absl::ToDoubleSeconds(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - d1);
+  x = absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(6.5)) - 8.0;
+  x = absl::ToDoubleHours(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleHours(d - absl::Hours(1))
+  x = absl::ToDoubleMinutes(d) - 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMinutes(d - absl::Minutes(1))
+  x = absl::ToDoubleMilliseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMilliseconds(d - absl::Milliseconds(9))
+  x = absl::ToDoubleMicroseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMicroseconds(d - absl::Microseconds(9))
+  x = absl::ToDoubleNanoseconds(d) - 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleNanoseconds(d - absl::Nanoseconds(42))
+
+  // We can rewrite the argument of the duration conversion
+#define THIRTY absl::Seconds(30)
+  x = absl::ToDoubleSeconds(THIRTY) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(THIRTY - absl::Seconds(1))
+#undef THIRTY
+
+  // Some other contexts
+  if (absl::ToDoubleSeconds(d) - 1.0 > 10) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: if (absl::ToDoubleSeconds(d - absl::Seconds(1)) > 10) {}
+
+  // A nested occurance
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::Seconds(5));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(5))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::Seconds(absl::ToDoubleSeconds(d1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1)))
+
+  // These should not match
+  x = 5 - 6;
+  x = 4 - absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  x = absl::ToDoubleSeconds(d) + 1.0;
+  x = absl::ToDoubleSeconds(d) * 1.0;
+  x = absl::ToDoubleSeconds(d) / 1.0;
+
+#define MINUS_FIVE(z) absl::ToDoubleSeconds(z) - 5
+  x = MINUS_FIVE(d);
+#undef MINUS_FIVE
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -8,6 +8,7 @@
abseil-duration-division
abseil-duration-factory-float
abseil-duration-factory-scale
+   abseil-duration-subtraction
abseil-faster-strsplit-delimiter
abseil-no-internal-dependencies
abseil-no-namespace
Index: docs/clang-tidy/checks/abseil-duration-subtraction.rst
===
--- /dev/null
+++ 

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: test/clang-tidy/abseil-duration-subtraction.cpp:12
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the 
duration domain [abseil-duration-subtraction]

JonasToth wrote:
> hwright wrote:
> > JonasToth wrote:
> > > From this example starting:
> > > 
> > > - The RHS should be a nested expression with function calls, as the RHS 
> > > is transformed to create the adversary example i mean in the 
> > > transformation function above.
> > > 
> > > ```
> > > absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::ToDoubleSeconds(d) 
> > > - absl::ToDoubleSeconds(d1));
> > > ```
> > > I think you need the proper conversion function, as the result of the 
> > > expression is `double` and you need a `Duration`, right?
> > > 
> > > But in principle starting from this idea the transformation might break.
> > I think there may be some confusion here (and that's entirely my fault. :) )
> > 
> > We should never get this expression as input to the check, since it doesn't 
> > compile (as you point out):
> > ```
> > absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::ToDoubleSeconds(d) - 
> > absl::ToDoubleSeconds(d1));
> > ```
> > 
> > Since `absl::ToDoubleSeconds` requires that its argument is an 
> > `absl::Duration`, but the expression `absl::ToDoubleSeconds(d) - 
> > absl::ToDoubleSeconds(d1)` results in a `double`, we can't get this as 
> > input.
> > 
> > There may be other expressions which could be input, but in practice they 
> > don't really happen.  I've added a contrived example to the tests, but at 
> > some point the tests get too complex and confuse the fix matching 
> > infrastructure.
> Your last sentence is the thing ;) Murphies Law will hit this check, too. In 
> my opinion wrong transformations are very unfortunate and should be avoided 
> if possible (in this case possible).
> You can simply require that the expression of type double does not contain 
> any duration subtraction calls.
> 
> This is even possible in the matcher-part of the check.
I've written a test (which the testing infrastructure fails to handle well, so 
I haven't included it in the diff), and it produces these results:

```
   //
   //
-  x = absl::ToDoubleSeconds(d) - (absl::ToDoubleSeconds(d1) - 5);
+  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1) - 5));
   //
   //
-  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1) - 5));
+  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1 - 
absl::Seconds(5;
```

Those results are correct.  There is a cosmetic issue of round tripping through 
the `double` conversion in the `absl::Seconds(absl::ToDoubleSeconds(...))` 
phrase, but untangling that is 1) difficult (because of order of operations 
issues) and thus; 2) probably the subject of a separate check.

This is still such a rare case (as in, we've not encountered it in Google's 
codebase), that I'm not really concerned.  But if it's worth it to explicitly 
exclude it from the traversal matcher, I can do that.


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

https://reviews.llvm.org/D55245



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

Re: r348889 - Emit -Wformat properly for bit-field promotions.

[Aaron Ballman via cfe-commits]

On Tue, Dec 11, 2018 at 2:21 PM Aaron Ballman via cfe-commits
 wrote:
>
> Author: aaronballman
> Date: Tue Dec 11 11:18:01 2018
> New Revision: 348889
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348889=rev
> Log:
> Emit -Wformat properly for bit-field promotions.
>
> Only explicitly look through integer and floating-point promotion where the 
> result type is actually a promotion, which is not always the case for 
> bit-fields in C.

Patch by Bevin Hansson. (Sorry for missing that in the commit message, Bevin!)

~Aaron

>
> Added:
> cfe/trunk/test/Sema/format-strings-bitfield-promotion.c
> cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx
> Modified:
> cfe/trunk/lib/Sema/SemaChecking.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=348889=34=348889=diff
> ==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 11 11:18:01 2018
> @@ -7709,6 +7709,24 @@ shouldNotPrintDirectly(const ASTContext
>return std::make_pair(QualType(), StringRef());
>  }
>
> +/// Return true if \p ICE is an implicit argument promotion of an arithmetic
> +/// type. Bit-field 'promotions' from a higher ranked type to a lower ranked
> +/// type do not count.
> +static bool
> +isArithmeticArgumentPromotion(Sema , const ImplicitCastExpr *ICE) {
> +  QualType From = ICE->getSubExpr()->getType();
> +  QualType To = ICE->getType();
> +  // It's a floating promotion if the source type is a lower rank.
> +  if (ICE->getCastKind() == CK_FloatingCast &&
> +  S.Context.getFloatingTypeOrder(From, To) < 0)
> +return true;
> +  // It's an integer promotion if the destination type is the promoted
> +  // source type.
> +  return ICE->getCastKind() == CK_IntegralCast &&
> + From->isPromotableIntegerType() &&
> + S.Context.getPromotedIntegerType(From) == To;
> +}
> +
>  bool
>  CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier 
> ,
>  const char *StartSpecifier,
> @@ -7736,11 +7754,11 @@ CheckPrintfHandler::checkFormatExpr(cons
>
>// Look through argument promotions for our error message's reported type.
>// This includes the integral and floating promotions, but excludes array
> -  // and function pointer decay; seeing that an argument intended to be a
> -  // string has type 'char [6]' is probably more confusing than 'char *'.
> +  // and function pointer decay (seeing that an argument intended to be a
> +  // string has type 'char [6]' is probably more confusing than 'char *') and
> +  // certain bitfield promotions (bitfields can be 'demoted' to a lesser 
> type).
>if (const ImplicitCastExpr *ICE = dyn_cast(E)) {
> -if (ICE->getCastKind() == CK_IntegralCast ||
> -ICE->getCastKind() == CK_FloatingCast) {
> +if (isArithmeticArgumentPromotion(S, ICE)) {
>E = ICE->getSubExpr();
>ExprTy = E->getType();
>
>
> Added: cfe/trunk/test/Sema/format-strings-bitfield-promotion.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-bitfield-promotion.c?rev=348889=auto
> ==
> --- cfe/trunk/test/Sema/format-strings-bitfield-promotion.c (added)
> +++ cfe/trunk/test/Sema/format-strings-bitfield-promotion.c Tue Dec 11 
> 11:18:01 2018
> @@ -0,0 +1,18 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
> +// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify 
> %s
> +
> +int printf(const char *restrict, ...);
> +
> +struct bitfields {
> +  long a : 2;
> +  unsigned long b : 2;
> +  long c : 32;  // assumes that int is 32 bits
> +  unsigned long d : 32; // assumes that int is 32 bits
> +} bf;
> +
> +void bitfield_promotion() {
> +  printf("%ld", bf.a); // expected-warning {{format specifies type 'long' 
> but the argument has type 'int'}}
> +  printf("%lu", bf.b); // expected-warning {{format specifies type 'unsigned 
> long' but the argument has type 'int'}}
> +  printf("%ld", bf.c); // expected-warning {{format specifies type 'long' 
> but the argument has type 'int'}}
> +  printf("%lu", bf.d); // expected-warning {{format specifies type 'unsigned 
> long' but the argument has type 'unsigned int'}}
> +}
>
> Added: cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx?rev=348889=auto
> ==
> --- cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx (added)
> +++ cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx Tue Dec 11 
> 11:18:01 2018
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
> +// RUN: 

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit in r348889, thank you for the patch!


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

https://reviews.llvm.org/D51211



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

r348889 - Emit -Wformat properly for bit-field promotions.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Dec 11 11:18:01 2018
New Revision: 348889

URL: http://llvm.org/viewvc/llvm-project?rev=348889=rev
Log:
Emit -Wformat properly for bit-field promotions.

Only explicitly look through integer and floating-point promotion where the 
result type is actually a promotion, which is not always the case for 
bit-fields in C.

Added:
cfe/trunk/test/Sema/format-strings-bitfield-promotion.c
cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=348889=34=348889=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 11 11:18:01 2018
@@ -7709,6 +7709,24 @@ shouldNotPrintDirectly(const ASTContext
   return std::make_pair(QualType(), StringRef());
 }
 
+/// Return true if \p ICE is an implicit argument promotion of an arithmetic
+/// type. Bit-field 'promotions' from a higher ranked type to a lower ranked
+/// type do not count.
+static bool
+isArithmeticArgumentPromotion(Sema , const ImplicitCastExpr *ICE) {
+  QualType From = ICE->getSubExpr()->getType();
+  QualType To = ICE->getType();
+  // It's a floating promotion if the source type is a lower rank.
+  if (ICE->getCastKind() == CK_FloatingCast &&
+  S.Context.getFloatingTypeOrder(From, To) < 0)
+return true;
+  // It's an integer promotion if the destination type is the promoted
+  // source type.
+  return ICE->getCastKind() == CK_IntegralCast &&
+ From->isPromotableIntegerType() &&
+ S.Context.getPromotedIntegerType(From) == To;
+}
+
 bool
 CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier ,
 const char *StartSpecifier,
@@ -7736,11 +7754,11 @@ CheckPrintfHandler::checkFormatExpr(cons
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
-  // and function pointer decay; seeing that an argument intended to be a
-  // string has type 'char [6]' is probably more confusing than 'char *'.
+  // and function pointer decay (seeing that an argument intended to be a
+  // string has type 'char [6]' is probably more confusing than 'char *') and
+  // certain bitfield promotions (bitfields can be 'demoted' to a lesser type).
   if (const ImplicitCastExpr *ICE = dyn_cast(E)) {
-if (ICE->getCastKind() == CK_IntegralCast ||
-ICE->getCastKind() == CK_FloatingCast) {
+if (isArithmeticArgumentPromotion(S, ICE)) {
   E = ICE->getSubExpr();
   ExprTy = E->getType();
 

Added: cfe/trunk/test/Sema/format-strings-bitfield-promotion.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-bitfield-promotion.c?rev=348889=auto
==
--- cfe/trunk/test/Sema/format-strings-bitfield-promotion.c (added)
+++ cfe/trunk/test/Sema/format-strings-bitfield-promotion.c Tue Dec 11 11:18:01 
2018
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
+
+int printf(const char *restrict, ...);
+
+struct bitfields {
+  long a : 2;
+  unsigned long b : 2;
+  long c : 32;  // assumes that int is 32 bits
+  unsigned long d : 32; // assumes that int is 32 bits
+} bf;
+
+void bitfield_promotion() {
+  printf("%ld", bf.a); // expected-warning {{format specifies type 'long' but 
the argument has type 'int'}}
+  printf("%lu", bf.b); // expected-warning {{format specifies type 'unsigned 
long' but the argument has type 'int'}}
+  printf("%ld", bf.c); // expected-warning {{format specifies type 'long' but 
the argument has type 'int'}}
+  printf("%lu", bf.d); // expected-warning {{format specifies type 'unsigned 
long' but the argument has type 'unsigned int'}}
+}

Added: cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx?rev=348889=auto
==
--- cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx (added)
+++ cfe/trunk/test/Sema/format-strings-bitfield-promotion.cxx Tue Dec 11 
11:18:01 2018
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
+
+// In C++, the bitfield promotion from long to int does not occur, unlike C.
+// expected-no-diagnostics
+
+int printf(const char *restrict, ...);
+
+struct bitfields {
+  long a : 2;
+  unsigned long b : 2;
+  long c : 32;  // assumes that int is 32 bits
+  unsigned long d : 32; // 

[PATCH] D55561: Stop stripping comments from AST matcher example code

[Jonas Toth via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D55561



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

[PATCH] D55488: Add utility for dumping a label with child nodes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/ASTDumper.cpp:89
 void dumpDecl(const Decl *D);
-void dumpStmt(const Stmt *S);
+void dumpStmt(const Stmt *S, const std::string  = {});
 

steveire wrote:
> aaron.ballman wrote:
> > Label
> > 
> > Rather than using `{}`, how about `""` (same behavior, but looks more 
> > idiomatic).
> > 
> > Why `std::string` instead of `StringRef`? I expect this will be called 
> > mostly with string literals, which saves an allocation. The other labels 
> > are using `const char *`, which would be another reasonable option. 
> > Whatever we go with, it'd be nice to make the label types agree across the 
> > calls.
> The actual print in TextTreeStructure is deferred, so it can't be `const 
> char*` or `StringRef`.
I think I've convinced myself there are not lifetime issues here, but it's 
subtle. In the case where the default argument is used, it creates a temporary 
object that is lifetime extended for the duration of the full expression 
including the call to `dumpStmt()`. The capture of `Label` in the lambda in 
`addChild()` captures the reference by copy and initializes the implicit field 
for the capture to the temporary value while its still within its lifetime. So 
I don't think this introduces UB.

However, if that lambda ever changes the capture list to use capture defaults 
(which we typically prefer when capturing multiple entities) and someone uses 
`&` or `this` as the capture default, they're going to have a very 
hard-to-discover bug on their hands. :-/

Another approach is to use `StringRef` in these calls, but within `addChild()` 
change the lambda to capture a local `std::string` by copy. e.g.,
```
template 
void addChild(StringRef Label, Fn doAddChild) {
  ...
  std::string LabelStr = Label.str();
  auto dumpWithIndent = [this, doAddChild, LabelStr](bool isLastChild) {
...
  };
  ...
```
(And when we upgrade to C++14 we can drop the local and just use an init 
capture instead.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55488



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

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 177746.
dmaclach added a comment.

Updated to fix Stephane's good catch of Objective C vs Objective-C


Repository:
  rC Clang

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

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,25 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are larger than
+// ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, );
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4025,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4788,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::warn_objc_method_encoding_too_large)
+  << (ObjCMethod->isInstanceMethod() ? "instance" : "class")
+  << ObjCMethod->getDeclName() << (unsigned)encoding.length()
+  << (unsigned)encodingSize;
+  }
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: 

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 177744.
dmaclach marked an inline comment as done.
dmaclach added a comment.

Full Diffs as requested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,25 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are larger than
+// ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, );
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4025,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4788,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::warn_objc_method_encoding_too_large)
+  << (ObjCMethod->isInstanceMethod() ? "instance" : "class")
+  << ObjCMethod->getDeclName() << (unsigned)encoding.length()
+  << (unsigned)encodingSize;
+  }
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: 

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks for working on this! Could you please post a patch with full context 
(git diff -U)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55544



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

[PATCH] D54592: [analyzer][CStringChecker] evaluate explicit_bzero

[David CARLIER via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348884: [analyzer][CStringChecker] evaluate explicit_bzero 
(authored by devnexen, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54592?vs=177171=177739#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54592

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  cfe/trunk/test/Analysis/string.c

Index: cfe/trunk/test/Analysis/string.c
===
--- cfe/trunk/test/Analysis/string.c
+++ cfe/trunk/test/Analysis/string.c
@@ -1184,11 +1184,14 @@
 }
 
 //===--===
-// memset()
+// memset() / explicit_bzero() / bzero()
 //===--===
 
 void *memset(void *dest, int ch, size_t count);
 
+void bzero(void *dst, size_t count);
+void explicit_bzero(void *dest, size_t count);
+
 void *malloc(size_t size);
 void free(void *);
 
@@ -1383,6 +1386,57 @@
   clang_analyzer_eval(array[4] == 0); // expected-warning{{TRUE}}
 }
 
+void bzero1_null() {
+  char *a = NULL;
+
+  bzero(a, 10); // expected-warning{{Null pointer argument in call to memory clearance function}}
+}
+
+void bzero2_char_array_null() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  bzero(str, 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+}
+
+void bzero3_char_ptr_null() {
+  char *str = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  bzero(str + 2, 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{FALSE}}
+}
+
+void explicit_bzero1_null() {
+  char *a = NULL;
+
+  explicit_bzero(a, 10); // expected-warning{{Null pointer argument in call to memory clearance function}}
+}
+
+void explicit_bzero2_clear_mypassword() {
+  char passwd[7] = "passwd";
+
+  explicit_bzero(passwd, sizeof(passwd)); // no-warning
+
+  clang_analyzer_eval(strlen(passwd) == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(passwd[0] == '\0'); // expected-warning{{TRUE}}
+}
+
+void explicit_bzero3_out_ofbound() {
+  char *privkey = (char *)malloc(7);
+  const char newprivkey[10] = "mysafekey";
+
+  strcpy(privkey, "random");
+  explicit_bzero(privkey, sizeof(newprivkey));
+#ifndef SUPPRESS_OUT_OF_BOUND
+  // expected-warning@-2 {{Memory clearance function accesses out-of-bound array element}}
+#endif
+  clang_analyzer_eval(privkey[0] == '\0');
+#ifdef SUPPRESS_OUT_OF_BOUND
+  // expected-warning@-2 {{UNKNOWN}}
+#endif
+  free(privkey);
+}
+
 //===--===
 // FIXMEs
 //===--===
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -124,6 +124,7 @@
   void evalStdCopyBackward(CheckerContext , const CallExpr *CE) const;
   void evalStdCopyCommon(CheckerContext , const CallExpr *CE) const;
   void evalMemset(CheckerContext , const CallExpr *CE) const;
+  void evalBzero(CheckerContext , const CallExpr *CE) const;
 
   // Utility methods
   std::pair
@@ -158,7 +159,7 @@
   static bool SummarizeRegion(raw_ostream , ASTContext ,
   const MemRegion *MR);
 
-  static bool memsetAux(const Expr *DstBuffer, const Expr *CharE,
+  static bool memsetAux(const Expr *DstBuffer, SVal CharE,
 const Expr *Size, CheckerContext ,
 ProgramStateRef );
 
@@ -1005,11 +1006,10 @@
   }
 }
 
-bool CStringChecker::memsetAux(const Expr *DstBuffer, const Expr *CharE,
+bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal,
const Expr *Size, CheckerContext ,
ProgramStateRef ) {
   SVal MemVal = C.getSVal(DstBuffer);
-  SVal CharVal = C.getSVal(CharE);
   SVal SizeVal = C.getSVal(Size);
   const MemRegion *MR = MemVal.getAsRegion();
   if (!MR)
@@ -2184,13 +2184,59 @@
   // According to the values of the arguments, bind the value of the second
   // argument to the destination buffer and set string length, or just
   // invalidate the destination buffer.
-  if (!memsetAux(Mem, CharE, Size, C, State))
+  if (!memsetAux(Mem, C.getSVal(CharE), Size, C, State))
 return;
 
   State = State->BindExpr(CE, LCtx, MemVal);
   C.addTransition(State);
 }
 
+void CStringChecker::evalBzero(CheckerContext , const CallExpr *CE) const {
+  if (CE->getNumArgs() != 2)
+return;
+
+  CurrentFunctionDescription = "memory clearance function";
+
+  const Expr *Mem = CE->getArg(0);
+  const Expr *Size = 

r348884 - [analyzer][CStringChecker] evaluate explicit_bzero

[David Carlier via cfe-commits]

Author: devnexen
Date: Tue Dec 11 10:57:07 2018
New Revision: 348884

URL: http://llvm.org/viewvc/llvm-project?rev=348884=rev
Log:
[analyzer][CStringChecker] evaluate explicit_bzero


- explicit_bzero has limited scope/usage only for security/crypto purposes but 
is non-optimisable version of memset/0 and bzero.
- explicit_memset has similar signature and semantics as memset but is also a 
non-optimisable version.

Reviewers: NoQ

Reviewed By: NoQ

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


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=348884=348883=348884=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Tue Dec 11 
10:57:07 2018
@@ -124,6 +124,7 @@ public:
   void evalStdCopyBackward(CheckerContext , const CallExpr *CE) const;
   void evalStdCopyCommon(CheckerContext , const CallExpr *CE) const;
   void evalMemset(CheckerContext , const CallExpr *CE) const;
+  void evalBzero(CheckerContext , const CallExpr *CE) const;
 
   // Utility methods
   std::pair
@@ -158,7 +159,7 @@ public:
   static bool SummarizeRegion(raw_ostream , ASTContext ,
   const MemRegion *MR);
 
-  static bool memsetAux(const Expr *DstBuffer, const Expr *CharE,
+  static bool memsetAux(const Expr *DstBuffer, SVal CharE,
 const Expr *Size, CheckerContext ,
 ProgramStateRef );
 
@@ -1005,11 +1006,10 @@ bool CStringChecker::SummarizeRegion(raw
   }
 }
 
-bool CStringChecker::memsetAux(const Expr *DstBuffer, const Expr *CharE,
+bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal,
const Expr *Size, CheckerContext ,
ProgramStateRef ) {
   SVal MemVal = C.getSVal(DstBuffer);
-  SVal CharVal = C.getSVal(CharE);
   SVal SizeVal = C.getSVal(Size);
   const MemRegion *MR = MemVal.getAsRegion();
   if (!MR)
@@ -2184,13 +2184,59 @@ void CStringChecker::evalMemset(CheckerC
   // According to the values of the arguments, bind the value of the second
   // argument to the destination buffer and set string length, or just
   // invalidate the destination buffer.
-  if (!memsetAux(Mem, CharE, Size, C, State))
+  if (!memsetAux(Mem, C.getSVal(CharE), Size, C, State))
 return;
 
   State = State->BindExpr(CE, LCtx, MemVal);
   C.addTransition(State);
 }
 
+void CStringChecker::evalBzero(CheckerContext , const CallExpr *CE) const {
+  if (CE->getNumArgs() != 2)
+return;
+
+  CurrentFunctionDescription = "memory clearance function";
+
+  const Expr *Mem = CE->getArg(0);
+  const Expr *Size = CE->getArg(1);
+  SVal Zero = C.getSValBuilder().makeZeroVal(C.getASTContext().IntTy);
+
+  ProgramStateRef State = C.getState();
+  
+  // See if the size argument is zero.
+  SVal SizeVal = C.getSVal(Size);
+  QualType SizeTy = Size->getType();
+
+  ProgramStateRef StateZeroSize, StateNonZeroSize;
+  std::tie(StateZeroSize, StateNonZeroSize) =
+assumeZero(C, State, SizeVal, SizeTy);
+
+  // If the size is zero, there won't be any actual memory access,
+  // In this case we just return.
+  if (StateZeroSize && !StateNonZeroSize) {
+C.addTransition(StateZeroSize);
+return;
+  }
+
+  // Get the value of the memory area.
+  SVal MemVal = C.getSVal(Mem);
+
+  // Ensure the memory area is not null.
+  // If it is NULL there will be a NULL pointer dereference.
+  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal);
+  if (!State)
+return;
+
+  State = CheckBufferAccess(C, State, Size, Mem);
+  if (!State)
+return;
+
+  if (!memsetAux(Mem, Zero, Size, C, State))
+return;
+
+  C.addTransition(State);
+}
+
 static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) {
   IdentifierInfo *II = FD->getIdentifier();
   if (!II)
@@ -2224,7 +2270,8 @@ bool CStringChecker::evalCall(const Call
 evalFunction =  ::evalMemcmp;
   else if (C.isCLibraryFunction(FDecl, "memmove"))
 evalFunction =  ::evalMemmove;
-  else if (C.isCLibraryFunction(FDecl, "memset"))
+  else if (C.isCLibraryFunction(FDecl, "memset") || 
+C.isCLibraryFunction(FDecl, "explicit_memset"))
 evalFunction =  ::evalMemset;
   else if (C.isCLibraryFunction(FDecl, "strcpy"))
 evalFunction =  ::evalStrcpy;
@@ -2262,6 +2309,9 @@ bool CStringChecker::evalCall(const Call
 evalFunction =  ::evalStdCopy;
   else if (isCPPStdLibraryFunction(FDecl, "copy_backward"))
 evalFunction =  ::evalStdCopyBackward;
+  else if (C.isCLibraryFunction(FDecl, "bzero") ||
+C.isCLibraryFunction(FDecl, "explicit_bzero"))
+evalFunction =  ::evalBzero;
 
   // If the callee isn't a string function, let 

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D54355#1327237 , @craig.topper 
wrote:

> Here's the test case that we have https://reviews.llvm.org/P8123   gcc seems 
> to accept it at O0


Smaller test case:

  extern unsigned long long __sdt_unsp;
  
  void foo() {
__asm__ __volatile__(
"" :: "n"( (__builtin_constant_p(__sdt_unsp) || 0) ? 1 : -1));
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D55562: Atomics: support min/max orthogonally

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

What does it do with floating-point inputs?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562



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

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

@joerg Yeah, we saw the commit explaining why the original fwd declaration 
patch was reverted. However, from what I can see, we only have three ways to 
fix the cyclic dependency between glibc and Clang's internal module:

1. We say that we don't support including mm_malloc.h from the module 
containing stdlib.h.
2. We don't include mm_malloc.h (and the internal headers that include 
mm_malloc.h) in Clang's internal module.
3. We add this workaround to allow us to forward declare free/malloc here.

Solution 1 essentially means that we won't support a glibc with modules until 
they work around this issue. And solution 2 means that all affected headers in 
clang's internal module have to be textually copied into modules that include 
them.

So after some offline discussion on how to proceed with this, the plan we came 
up with was to land this patch (which implements solution 3) and then see 
how/if we can work around the problems that users experience because of it 
(otherwise we would revert).

I don't have a strict preference between solution 2 and 3 at this point. So if 
@joerg thinks that this approach is not the right one, then I would just make a 
new patch that removes the affected headers from Clang's internal module.


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

https://reviews.llvm.org/D43871



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

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:113
 
+  const bool IsGlobal = MatchedDecl->getStorageClass() != SC_Static;
   diag(MatchedDecl->getLocation(),

Drop the top-level `const` qualifier, please.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:115
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "

stephanemoore wrote:
> benhamilton wrote:
> > I know "global" is the correct name, but I feel like "non-static" might be 
> > clearer to folks dealing with these error messages.
> > 
> > WDYT?
> > 
> I'm wary of saying "non-static" because namespaced functions in Objective-C++ 
> are not subject to the cited rules. The term "non-static" seems like it could 
> be interpreted to extend to namespaced functions which could potentially 
> mislead someone into adding prefixes to namespaced functions in Objective-C++.
How about "%select{static function|function in global namespace}1 named %0..."


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

https://reviews.llvm.org/D55482



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

[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Ping


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

https://reviews.llvm.org/D54923



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

[PATCH] D54592: [analyzer][CStringChecker] evaluate explicit_bzero

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks good, thanks!




Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2216
+  // In this case we just return.
+  if (StateZeroSize && !StateNonZeroSize) {
+C.addTransition(StateZeroSize);

`!StateNonZeroSize` implies `StateZeroSize`, you can drop the left-hand side of 
`&&`.



Comment at: test/Analysis/string.c:1405-1406
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  bzero(str + 2, 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{FALSE}}
+}

devnexen wrote:
> NoQ wrote:
> > Let's also add the true statement. I.e., do we know here that the actual 
> > length is 2?
> I think that s the limit of this checker (even with memset that does not 
> work).
I mean, even if it doesn't work, let's add a FIXME test.


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

https://reviews.llvm.org/D54592



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Here's the test case that we have https://reviews.llvm.org/P8123   gcc seems to 
accept it at O0


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: lib/AST/Expr.cpp:1267
 : Expr(SC, Empty), NumArgs(NumArgs) {
+  CallExprBits.UsesADL = false;
   CallExprBits.NumPreArgs = NumPreArgs;

riccibruno wrote:
> I believe that msan can cope with bit level operations just fine.
> What I am afraid is that initializing it here will hide the
> fact that it is not initialized during deserialization if the
> `E->setUsesADL(Record.readInt());` is removed by mistake.
> 
> At least not initializing it here will leave a chance for msan
> to catch this.
I don't think they do.


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

https://reviews.llvm.org/D55534



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

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh added a comment.

Hello!

I'm pinging since it's been a week. If someone can commit this patch on my 
behalf, that would be great.

Thank you :)


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

https://reviews.llvm.org/D55226



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

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

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

rsmith added inline comments.



Comment at: include/clang/Sema/Sema.h:2758
 bool AllowExplicit = false,
+bool IsADLCandidate = false,
 ConversionSequenceList EarlyConversions = None);

Long lists of bool arguments make me nervous, especially with default 
arguments. Can you introduce an enum for the new param at least?



Comment at: lib/AST/ASTImporter.cpp:7387
   return new (Importer.getToContext()) CallExpr(
   Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(),
   ToRParenLoc);

Do we need to pass through the usesADL flag here too?



Comment at: lib/AST/Expr.cpp:1267
 : Expr(SC, Empty), NumArgs(NumArgs) {
+  CallExprBits.UsesADL = false;
   CallExprBits.NumPreArgs = NumPreArgs;

EricWF wrote:
> riccibruno wrote:
> > It do not really matter but there is not point initializing this bit here.
> I'm a but nervous MSAN won't catch an uninitialized read because it's a 
> bitfield, so I would rather be safe than sorry?
IIRC all the ExprBits get initialized by the Expr(EmptyShell) ctor.



Comment at: lib/Sema/SemaExpr.cpp:5673
+  if (Config) {
 TheCall = new (Context)
 CUDAKernelCallExpr(Context, Fn, cast(Config), Args, ResultTy,

If you believe that CUDA kernel calls can't find functions with ADL, please add 
an assert here.


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

https://reviews.llvm.org/D55534



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

Re: r348858 - Revert "[PowerPC] Make no-PIC default to match GCC - CLANG"

[Aaron Ballman via cfe-commits]

On Tue, Dec 11, 2018 at 12:07 PM Stefan Pintilie  wrote:
>
> Hi Aaron,
>
> Sorry about giving so little info.

No worries!

> The commit was reverted because it broke two test cases on an internal 
> buildbot. The reason this was reverted so late was because this failure was 
> buried underneath another set of failures on that same buildbot which 
> initially hid the problem. I'm looking at the failures now and hopefully I'll 
> have the change back in soon.

Ah, thank you for the information.

~Aaron

>
> Stefan
>
>
>
> From:Aaron Ballman 
> To:stef...@ca.ibm.com
> Cc:cfe-commits 
> Date:2018/12/11 10:56 AM
> Subject:Re: r348858 - Revert "[PowerPC] Make no-PIC default to match 
> GCC - CLANG"
> 
>
>
>
> On Tue, Dec 11, 2018 at 10:50 AM Stefan Pintilie via cfe-commits
>  wrote:
> >
> > Author: stefanp
> > Date: Tue Dec 11 07:47:57 2018
> > New Revision: 348858
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=348858=rev
> > Log:
> > Revert "[PowerPC] Make no-PIC default to match GCC - CLANG"
> >
> > This reverts commit rL348299.
>
> When reverting a commit, you should explain why the commit was
> reverted as part of the commit message (this makes code archaeology
> much easier). Why was this reverted 500+ revisions after it landed?
>
> ~Aaron
>
> >
> > Modified:
> > cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> > cfe/trunk/test/Driver/clang-offload-bundler.c
> > cfe/trunk/test/Driver/ppc-abi.c
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=348858=348857=348858=diff
> > ==
> > --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Dec 11 07:47:57 2018
> > @@ -2435,7 +2435,7 @@ bool Generic_GCC::isPICDefault() const {
> >case llvm::Triple::x86_64:
> >  return getTriple().isOSWindows();
> >case llvm::Triple::ppc64:
> > -// Big endian PPC is PIC by default
> > +  case llvm::Triple::ppc64le:
> >  return !getTriple().isOSBinFormatMachO() && !getTriple().isMacOSX();
> >case llvm::Triple::mips64:
> >case llvm::Triple::mips64el:
> >
> > Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=348858=348857=348858=diff
> > ==
> > --- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
> > +++ cfe/trunk/test/Driver/clang-offload-bundler.c Tue Dec 11 07:47:57 2018
> > @@ -115,7 +115,7 @@
> >  // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLEEND__ openmp-x86_64-pc-linux-gnu
> >
> >  // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLESTART__ 
> > host-powerpc64le-ibm-linux-gnu
> > -// CK-TEXTLL: @A = dso_local global i32 0
> > +// CK-TEXTLL: @A = global i32 0
> >  // CK-TEXTLL: define {{.*}}@test_func()
> >  // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLEEND__ 
> > host-powerpc64le-ibm-linux-gnu
> >  // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLESTART__ 
> > openmp-powerpc64le-ibm-linux-gnu
> >
> > Modified: cfe/trunk/test/Driver/ppc-abi.c
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ppc-abi.c?rev=348858=348857=348858=diff
> > ==
> > --- cfe/trunk/test/Driver/ppc-abi.c (original)
> > +++ cfe/trunk/test/Driver/ppc-abi.c Tue Dec 11 07:47:57 2018
> > @@ -13,12 +13,12 @@
> >  // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
> >  // RUN:   -mcpu=a2q -mno-qpx | FileCheck -check-prefix=CHECK-ELFv1 %s
> >  // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
> > -// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2-BE %s
> > +// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
> >
> >  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
> >  // RUN:   | FileCheck -check-prefix=CHECK-ELFv2 %s
> >  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
> > -// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1-LE %s
> > +// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1 %s
> >  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
> >  // RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
> >  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
> > @@ -26,44 +26,8 @@
> >
> >  // CHECK-ELFv1: "-mrelocation-model" "pic" "-pic-level" "2"
> >  // CHECK-ELFv1: "-target-abi" "elfv1"
> > -// CHECK-ELFv1-LE: "-mrelocation-model" "static"
> > -// CHECK-ELFv1-LE: "-target-abi" "elfv1"
> >  // CHECK-ELFv1-QPX: "-mrelocation-model" "pic" "-pic-level" "2"
> >  // CHECK-ELFv1-QPX: "-target-abi" "elfv1-qpx"
> > -// CHECK-ELFv2: "-mrelocation-model" "static"
> > +// 

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: lib/AST/Expr.cpp:1267
 : Expr(SC, Empty), NumArgs(NumArgs) {
+  CallExprBits.UsesADL = false;
   CallExprBits.NumPreArgs = NumPreArgs;

I believe that msan can cope with bit level operations just fine.
What I am afraid is that initializing it here will hide the
fact that it is not initialized during deserialization if the
`E->setUsesADL(Record.readInt());` is removed by mistake.

At least not initializing it here will leave a chance for msan
to catch this.


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

https://reviews.llvm.org/D55534



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

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

In D55544#1326956 , @theraven wrote:

> It would probably be a good idea to have a similar check on properties, as 
> property encoding strings contain the type encoding (plus extra stuff).


Properties are already picked up based on the ivars and methods that they 
generate.

> I wonder if we also want an option in code generation to replace very long 
> type encodings (or encodings of specifically annotated ivars?) with `"?"` 
> ('unknown type')?

Yeah, this is the next step. I'm trying to decide how best to do this. For a 
large cross platform code base I don't necessarily want to have folks having to 
annotate every C++ class they use with a somewhat non-portable annotation. At 
the same time you can't make all structs/classes encode as ? because I think it 
will mess up some existing Objective C patterns such as NSCoding.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55544



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

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
t.p.northover added reviewers: delena, yaxunl.
Herald added subscribers: jfb, mcrosier.

We seem to have been gradually growing support for atomic min/max operations 
(exposing longstanding IR atomicrmw instructions). But until now there have 
been gaps in the expected intrinsics. This adds support for the C11-style 
intrinsics (i.e. taking _Atomic, rather than individually blessed by C11 
standard), and the variants that return the new value instead of the original 
one.

That way, people won't be misled by trying one form and it not working, and the 
front-end is more friendly to people using _Atomic types, as we recommend.


Repository:
  rC Clang

https://reviews.llvm.org/D55562

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/atomic-ops-libcall.c
  clang/test/CodeGen/atomic-ops.c
  clang/test/Sema/atomic-ops.c

Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -354,6 +354,20 @@
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_acq_rel);
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_seq_cst);
 
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_seq_cst);
+
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_seq_cst);
+
   (void)__c11_atomic_exchange(Ap, val, memory_order_relaxed);
   (void)__c11_atomic_exchange(Ap, val, memory_order_acquire);
   (void)__c11_atomic_exchange(Ap, val, memory_order_consume);
@@ -501,6 +515,20 @@
   (void)__atomic_nand_fetch(p, val, memory_order_acq_rel);
   (void)__atomic_nand_fetch(p, val, memory_order_seq_cst);
 
+  (void)__atomic_max_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_max_fetch(p, val, memory_order_acquire);
+  (void)__atomic_max_fetch(p, val, memory_order_consume);
+  (void)__atomic_max_fetch(p, val, memory_order_release);
+  (void)__atomic_max_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_max_fetch(p, val, memory_order_seq_cst);
+
+  (void)__atomic_min_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_min_fetch(p, val, memory_order_acquire);
+  (void)__atomic_min_fetch(p, val, memory_order_consume);
+  (void)__atomic_min_fetch(p, val, memory_order_release);
+  (void)__atomic_min_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_min_fetch(p, val, memory_order_seq_cst);
+
   (void)__atomic_exchange_n(p, val, memory_order_relaxed);
   (void)__atomic_exchange_n(p, val, memory_order_acquire);
   (void)__atomic_exchange_n(p, val, memory_order_consume);
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -661,4 +661,46 @@
   __atomic_compare_exchange(_a, _b, _c, 1, memory_order_seq_cst, memory_order_seq_cst);
 }
 
+void test_c11_minmax(_Atomic(int) *si, _Atomic(unsigned) *ui) {
+  // CHECK-LABEL: @test_c11_minmax
+
+  // CHECK: atomicrmw max
+  *si = __c11_atomic_fetch_max(si, 42, memory_order_acquire);
+  // CHECK: atomicrmw min
+  *si = __c11_atomic_fetch_min(si, 42, memory_order_acquire);
+  // CHECK: atomicrmw umax
+  *ui = __c11_atomic_fetch_max(ui, 42, memory_order_acquire);
+  // CHECK: atomicrmw umin
+  *ui = __c11_atomic_fetch_min(ui, 42, memory_order_acquire);
+}
+
+void test_minmax_postop(int *si, unsigned *ui) {
+  int val = 42;
+  // CHECK-LABEL: @test_minmax_postop
+
+  // CHECK: [[OLD:%.*]] = atomicrmw max i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp sgt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *si = __atomic_max_fetch(si, 42, memory_order_release);
+
+  // CHECK: [[OLD:%.*]] = atomicrmw min i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp slt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *si = __atomic_min_fetch(si, 42, memory_order_release);
+  
+  // CHECK: [[OLD:%.*]] = atomicrmw umax i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp ugt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select 

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 177732.
dmaclach added a comment.

Added some spacing around early exit as requested by theraven.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,25 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are larger than
+// ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, );
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4025,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4788,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::warn_objc_method_encoding_too_large)
+  << (ObjCMethod->isInstanceMethod() ? "instance" : "class")
+  << ObjCMethod->getDeclName() << (unsigned)encoding.length()
+  << (unsigned)encodingSize;
+  }
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: 

[PATCH] D55561: Stop stripping comments from AST matcher example code

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: JonasToth, klimek.

The AST matcher documentation dumping script was being a bit over-zealous about 
stripping comment markers, which ended up causing comments in example code to 
stop being comments. This patch fixes that by only stripping comments at the 
start of a line, rather than removing any forward slash (which also impacts 
prose text).

As a drive-by, this fixes a broken doxygen comment marker as well.


https://reviews.llvm.org/D55561

Files:
  docs/LibASTMatchersReference.html
  docs/tools/dump_ast_matchers.py
  include/clang/ASTMatchers/ASTMatchers.h

Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3528,7 +3528,7 @@
 ///   } catch (...) {
 /// // ...
 ///   }
-/// /endcode
+/// \endcode
 /// cxxCatchStmt(isCatchAll()) matches catch(...) but not catch(int).
 AST_MATCHER(CXXCatchStmt, isCatchAll) {
   return Node.getExceptionDecl() == nullptr;
Index: docs/tools/dump_ast_matchers.py
===
--- docs/tools/dump_ast_matchers.py
+++ docs/tools/dump_ast_matchers.py
@@ -354,7 +354,7 @@
 allowed_types += [m.group(1)]
 continue
   if line.strip() and line.lstrip()[0] == '/':
-comment += re.sub(r'/+\s?', '', line) + '\n'
+comment += re.sub(r'^/+\s?', '', line) + '\n'
   else:
 declaration += ' ' + line
 if ((not line.strip()) or 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -499,7 +499,7 @@
   int X;
   namespace NS {
   int Y;
-  }  namespace NS
+  }  // namespace NS
 decl(hasDeclContext(translationUnitDecl()))
   matches "int X", but not "int Y".
 
@@ -1152,7 +1152,7 @@
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtfloatLiteralMatcherhttps://clang.llvm.org/doxygen/classclang_1_1FloatingLiteral.html;>FloatingLiteral...
-Matches float literals of all sizes encodings, e.g.
+Matches float literals of all sizes / encodings, e.g.
 1.0, 1.0f, 1.0L and 1e10.
 
 Does not match implicit conversions such as
@@ -1230,7 +1230,7 @@
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtintegerLiteralMatcherhttps://clang.llvm.org/doxygen/classclang_1_1IntegerLiteral.html;>IntegerLiteral...
-Matches integer literals of all sizes encodings, e.g.
+Matches integer literals of all sizes / encodings, e.g.
 1, 1L, 0x1 and 1U.
 
 Does not match character-encoded integers such as L'a'.
@@ -1911,8 +1911,8 @@
   template typename T
   class C { };
 
-  template class Cint;  A
-  Cchar var;B
+  template class Cint;  // A
+  Cchar var;// B
 
 templateSpecializationType() matches the type of the explicit
 instantiation in A and the type of the variable declaration in B.
@@ -2091,13 +2091,12 @@
 
 Given
   try {
-...
+// ...
   } catch (int) {
-...
+// ...
   } catch (...) {
-...
+// ...
   }
-endcode
 cxxCatchStmt(isCatchAll()) matches catch(...) but not catch(int).
 
 
@@ -2136,9 +2135,9 @@
 
 Given
   struct S {
-S(); #1
-S(const S ); #2
-S(S ); #3
+S(); // #1
+S(const S ); // #2
+S(S ); // #3
   };
 cxxConstructorDecl(isCopyConstructor()) will match #2, but not #1 or #3.
 
@@ -2149,9 +2148,9 @@
 
 Given
   struct S {
-S(); #1
-S(const S ); #2
-S(S ); #3
+S(); // #1
+S(const S ); // #2
+S(S ); // #3
   };
 cxxConstructorDecl(isDefaultConstructor()) will match #1, but not #2 or #3.
 
@@ -2162,11 +2161,11 @@
 
 Given
   struct S {
-S(); #1
-S(int) {} #2
-S(S ) : S() {} #3
+S(); // #1
+S(int) {} // #2
+S(S ) : S() {} // #3
   };
-  S::S() : S(0) {} #4
+  S::S() : S(0) {} // #4
 cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
 #1 or #2.
 
@@ -2178,10 +2177,10 @@
 
 Given
   struct S {
-S(int); #1
-explicit S(double); #2
-operator int(); #3
-explicit operator bool(); #4
+S(int); // #1
+explicit S(double); // #2
+operator int(); // #3
+explicit operator bool(); // #4
   };
 cxxConstructorDecl(isExplicit()) will match #2, but not #1.
 cxxConversionDecl(isExplicit()) will match #4, but not #3.
@@ -2193,9 +2192,9 @@
 
 Given
   struct S {
-S(); #1
-S(const S ); #2
-S(S ); #3
+S(); // #1
+S(const S ); // #2
+S(S ); // #3
   };
 cxxConstructorDecl(isMoveConstructor()) will match #3, but not #1 or #2.
 
@@ -2207,10 +2206,10 @@
 
 Given
   struct S {
-S(int); #1
-explicit S(double); #2
-operator int(); #3
-explicit operator bool(); #4
+S(int); // #1
+explicit S(double); // #2
+operator int(); // #3
+explicit operator bool(); // #4
   };
 cxxConstructorDecl(isExplicit()) will match #2, but not #1.
 

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: docs/LibASTMatchersReference.html:2579-2581
+y(x); Matches
+NS::y(x); Doesn't match
+y(42); Doesn't match.

EricWF wrote:
> aaron.ballman wrote:
> > This is not your bug to fix, but it seems the documentation generator is 
> > stripping the comment markers. This impacts other matchers as well, such as 
> > `throughUsingDecl()`.
> I wonder if it strips `/**/` comments. I don't want to get blocked on this.
> 
Don't let it block you -- I'm poking at a fix currently, but this is not your 
bug.


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

https://reviews.llvm.org/D55534



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

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 3 inline comments as done.
EricWF added a comment.

Address more inline comments.




Comment at: docs/LibASTMatchersReference.html:2579-2581
+y(x); Matches
+NS::y(x); Doesn't match
+y(42); Doesn't match.

aaron.ballman wrote:
> This is not your bug to fix, but it seems the documentation generator is 
> stripping the comment markers. This impacts other matchers as well, such as 
> `throughUsingDecl()`.
I wonder if it strips `/**/` comments. I don't want to get blocked on this.




Comment at: lib/AST/Expr.cpp:1267
 : Expr(SC, Empty), NumArgs(NumArgs) {
+  CallExprBits.UsesADL = false;
   CallExprBits.NumPreArgs = NumPreArgs;

riccibruno wrote:
> It do not really matter but there is not point initializing this bit here.
I'm a but nervous MSAN won't catch an uninitialized read because it's a 
bitfield, so I would rather be safe than sorry?


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

https://reviews.llvm.org/D55534



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

Re: r348858 - Revert "[PowerPC] Make no-PIC default to match GCC - CLANG"

[Stefan Pintilie via cfe-commits]

Hi Aaron, 

Sorry about giving so little info.

The commit was reverted because it broke two test cases on an internal 
buildbot. The reason this was reverted so late was because this failure 
was buried underneath another set of failures on that same buildbot which 
initially hid the problem. I'm looking at the failures now and hopefully 
I'll have the change back in soon.

Stefan



From:   Aaron Ballman 
To: stef...@ca.ibm.com
Cc: cfe-commits 
Date:   2018/12/11 10:56 AM
Subject:Re: r348858 - Revert "[PowerPC] Make no-PIC default to 
match GCC - CLANG"



On Tue, Dec 11, 2018 at 10:50 AM Stefan Pintilie via cfe-commits
 wrote:
>
> Author: stefanp
> Date: Tue Dec 11 07:47:57 2018
> New Revision: 348858
>
> URL: 
http://llvm.org/viewvc/llvm-project?rev=348858=rev

> Log:
> Revert "[PowerPC] Make no-PIC default to match GCC - CLANG"
>
> This reverts commit rL348299.

When reverting a commit, you should explain why the commit was
reverted as part of the commit message (this makes code archaeology
much easier). Why was this reverted 500+ revisions after it landed?

~Aaron

>
> Modified:
> cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> cfe/trunk/test/Driver/clang-offload-bundler.c
> cfe/trunk/test/Driver/ppc-abi.c
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=348858=348857=348858=diff

> 
==
> --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Dec 11 07:47:57 2018
> @@ -2435,7 +2435,7 @@ bool Generic_GCC::isPICDefault() const {
>case llvm::Triple::x86_64:
>  return getTriple().isOSWindows();
>case llvm::Triple::ppc64:
> -// Big endian PPC is PIC by default
> +  case llvm::Triple::ppc64le:
>  return !getTriple().isOSBinFormatMachO() && 
!getTriple().isMacOSX();
>case llvm::Triple::mips64:
>case llvm::Triple::mips64el:
>
> Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
> URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=348858=348857=348858=diff

> 
==
> --- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
> +++ cfe/trunk/test/Driver/clang-offload-bundler.c Tue Dec 11 07:47:57 
2018
> @@ -115,7 +115,7 @@
>  // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLEEND__ 
openmp-x86_64-pc-linux-gnu
>
>  // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLESTART__ 
host-powerpc64le-ibm-linux-gnu
> -// CK-TEXTLL: @A = dso_local global i32 0
> +// CK-TEXTLL: @A = global i32 0
>  // CK-TEXTLL: define {{.*}}@test_func()
>  // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLEEND__ 
host-powerpc64le-ibm-linux-gnu
>  // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLESTART__ 
openmp-powerpc64le-ibm-linux-gnu
>
> Modified: cfe/trunk/test/Driver/ppc-abi.c
> URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ppc-abi.c?rev=348858=348857=348858=diff

> 
==
> --- cfe/trunk/test/Driver/ppc-abi.c (original)
> +++ cfe/trunk/test/Driver/ppc-abi.c Tue Dec 11 07:47:57 2018
> @@ -13,12 +13,12 @@
>  // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 
\
>  // RUN:   -mcpu=a2q -mno-qpx | FileCheck -check-prefix=CHECK-ELFv1 %s
>  // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 
\
> -// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2-BE %s
> +// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
>
>  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 
2>&1 \
>  // RUN:   | FileCheck -check-prefix=CHECK-ELFv2 %s
>  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 
2>&1 \
> -// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1-LE %s
> +// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1 %s
>  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 
2>&1 \
>  // RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
>  // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 
2>&1 \
> @@ -26,44 +26,8 @@
>
>  // CHECK-ELFv1: "-mrelocation-model" "pic" "-pic-level" "2"
>  // CHECK-ELFv1: "-target-abi" "elfv1"
> -// CHECK-ELFv1-LE: "-mrelocation-model" "static"
> -// CHECK-ELFv1-LE: "-target-abi" "elfv1"
>  // CHECK-ELFv1-QPX: "-mrelocation-model" "pic" "-pic-level" "2"
>  // CHECK-ELFv1-QPX: "-target-abi" "elfv1-qpx"
> -// CHECK-ELFv2: "-mrelocation-model" "static"
> +// CHECK-ELFv2: "-mrelocation-model" "pic" "-pic-level" "2"
>  // CHECK-ELFv2: "-target-abi" "elfv2"
> -// CHECK-ELFv2-BE: "-mrelocation-model" "pic" "-pic-level" "2"
> -// CHECK-ELFv2-BE: "-target-abi" "elfv2"
> -
> -// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o 
%t.o 2>&1 \
> -// RUN:   | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
> -// RUN: %clang -fPIC -target 

r348866 - Remove CGDebugInfo::getOrCreateFile() and use TheCU->getFile() directly.

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Tue Dec 11 08:58:46 2018
New Revision: 348866

URL: http://llvm.org/viewvc/llvm-project?rev=348866=rev
Log:
Remove CGDebugInfo::getOrCreateFile() and use TheCU->getFile() directly.

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348866=348865=348866=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Dec 11 08:58:46 2018
@@ -220,7 +220,7 @@ llvm::DIScope *CGDebugInfo::getContextDe
   if (const auto *RDecl = dyn_cast(Context))
 if (!RDecl->isDependentType())
   return getOrCreateType(CGM.getContext().getTypeDeclType(RDecl),
- getOrCreateMainFile());
+ TheCU->getFile());
   return Default;
 }
 
@@ -404,7 +404,7 @@ Optional CGDebugInfo::getSour
 llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
   if (!Loc.isValid())
 // If Location is not valid then use main input file.
-return getOrCreateMainFile();
+return TheCU->getFile();
 
   SourceManager  = CGM.getContext().getSourceManager();
   PresumedLoc PLoc = SM.getPresumedLoc(Loc);
@@ -412,7 +412,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
   StringRef FileName = PLoc.getFilename();
   if (PLoc.isInvalid() || FileName.empty())
 // If the location is not valid then use main input file.
-return getOrCreateMainFile();
+return TheCU->getFile();
 
   // Cache the results.
   auto It = DIFileCache.find(FileName.data());
@@ -471,13 +471,6 @@ CGDebugInfo::createFile(StringRef FileNa
   return F;
 }
 
-llvm::DIFile *CGDebugInfo::getOrCreateMainFile() {
-  return createFile(
-  remapDIPath(TheCU->getFilename()), remapDIPath(TheCU->getDirectory()),
-  TheCU->getFile()->getChecksum(),
-  CGM.getCodeGenOpts().EmbedSource ? TheCU->getSource() : None);
-}
-
 std::string CGDebugInfo::remapDIPath(StringRef Path) const {
   for (const auto  : DebugPrefixMap)
 if (Path.startswith(Entry.first))
@@ -641,9 +634,9 @@ llvm::DIType *CGDebugInfo::CreateType(co
 return nullptr;
   case BuiltinType::ObjCClass:
 if (!ClassTy)
-  ClassTy = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
-   "objc_class", TheCU,
-   getOrCreateMainFile(), 0);
+  ClassTy =
+  DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
+ "objc_class", TheCU, TheCU->getFile(), 0);
 return ClassTy;
   case BuiltinType::ObjCId: {
 // typedef struct objc_class *Class;
@@ -655,21 +648,21 @@ llvm::DIType *CGDebugInfo::CreateType(co
   return ObjTy;
 
 if (!ClassTy)
-  ClassTy = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
-   "objc_class", TheCU,
-   getOrCreateMainFile(), 0);
+  ClassTy =
+  DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
+ "objc_class", TheCU, TheCU->getFile(), 0);
 
 unsigned Size = CGM.getContext().getTypeSize(CGM.getContext().VoidPtrTy);
 
 auto *ISATy = DBuilder.createPointerType(ClassTy, Size);
 
-ObjTy = DBuilder.createStructType(
-TheCU, "objc_object", getOrCreateMainFile(), 0, 0, 0,
-llvm::DINode::FlagZero, nullptr, llvm::DINodeArray());
+ObjTy = DBuilder.createStructType(TheCU, "objc_object", TheCU->getFile(), 
0,
+  0, 0, llvm::DINode::FlagZero, nullptr,
+  llvm::DINodeArray());
 
 DBuilder.replaceArrays(
 ObjTy, DBuilder.getOrCreateArray(&*DBuilder.createMemberType(
-   ObjTy, "isa", getOrCreateMainFile(), 0, Size, 0, 0,
+   ObjTy, "isa", TheCU->getFile(), 0, Size, 0, 0,
llvm::DINode::FlagZero, ISATy)));
 return ObjTy;
   }
@@ -677,7 +670,7 @@ llvm::DIType *CGDebugInfo::CreateType(co
 if (!SelTy)
   SelTy = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
  "objc_selector", TheCU,
- getOrCreateMainFile(), 0);
+ TheCU->getFile(), 0);
 return SelTy;
   }
 
@@ -998,7 +991,7 @@ llvm::DIType *CGDebugInfo::getOrCreateSt
   if (Cache)
 return Cache;
   Cache = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type, Name,
- TheCU, getOrCreateMainFile(), 0);
+ TheCU, TheCU->getFile(), 0);
   unsigned Size = CGM.getContext().getTypeSize(CGM.getContext().VoidPtrTy);
   Cache = DBuilder.createPointerType(Cache, Size);
   return 

r348865 - Reuse code from CGDebugInfo::getOrCreateFile() when creating the file

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Tue Dec 11 08:58:43 2018
New Revision: 348865

URL: http://llvm.org/viewvc/llvm-project?rev=348865=rev
Log:
Reuse code from CGDebugInfo::getOrCreateFile() when creating the file
for the DICompileUnit.

This addresses post-commit feedback for D55085. Without this patch, a
main source file with an absolute paths may appear in different
DIFiles, once with the absolute path and once with the common prefix
between the absolute path and the current working directory.

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

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/test/CodeGen/debug-info-compilation-dir.c
cfe/trunk/test/PCH/debug-info-pch-path.c

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348865=348864=348865=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Dec 11 08:58:43 2018
@@ -429,7 +429,13 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
   Optional> CSInfo;
   if (CSKind)
 CSInfo.emplace(*CSKind, Checksum);
+  return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
+}
 
+llvm::DIFile *
+CGDebugInfo::createFile(StringRef FileName,
+Optional> CSInfo,
+Optional Source) {
   StringRef Dir;
   StringRef File;
   std::string RemappedFile = remapDIPath(FileName);
@@ -460,16 +466,13 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
 Dir = CurDir;
 File = RemappedFile;
   }
-  llvm::DIFile *F =
-  DBuilder.createFile(File, Dir, CSInfo,
-  getSource(SM, SM.getFileID(Loc)));
-
+  llvm::DIFile *F = DBuilder.createFile(File, Dir, CSInfo, Source);
   DIFileCache[FileName.data()].reset(F);
   return F;
 }
 
 llvm::DIFile *CGDebugInfo::getOrCreateMainFile() {
-  return DBuilder.createFile(
+  return createFile(
   remapDIPath(TheCU->getFilename()), remapDIPath(TheCU->getDirectory()),
   TheCU->getFile()->getChecksum(),
   CGM.getCodeGenOpts().EmbedSource ? TheCU->getSource() : None);
@@ -607,9 +610,7 @@ void CGDebugInfo::CreateCompileUnit() {
   auto  = CGM.getCodeGenOpts();
   TheCU = DBuilder.createCompileUnit(
   LangTag,
-  DBuilder.createFile(remapDIPath(MainFileName),
-  remapDIPath(getCurrentDirname()), CSInfo,
-  getSource(SM, SM.getMainFileID())),
+  createFile(MainFileName, CSInfo, getSource(SM, SM.getMainFileID())),
   CGOpts.EmitVersionIdentMetadata ? Producer : "",
   LO.Optimize || CGOpts.PrepareForLTO || CGOpts.PrepareForThinLTO,
   CGOpts.DwarfDebugFlags, RuntimeVers,

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=348865=348864=348865=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Tue Dec 11 08:58:43 2018
@@ -538,9 +538,16 @@ private:
   /// Get the source of the given file ID.
   Optional getSource(const SourceManager , FileID FID);
 
-  /// Get the file debug info descriptor for the input location.
+  /// Convenience function to get the file debug info descriptor for the input
+  /// location.
   llvm::DIFile *getOrCreateFile(SourceLocation Loc);
 
+  /// Create a file debug info descriptor for a source file.
+  llvm::DIFile *
+  createFile(StringRef FileName,
+ Optional> CSInfo,
+ Optional Source);
+
   /// Get the file info for main compile unit.
   llvm::DIFile *getOrCreateMainFile();
 

Modified: cfe/trunk/test/CodeGen/debug-info-compilation-dir.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-compilation-dir.c?rev=348865=348864=348865=diff
==
--- cfe/trunk/test/CodeGen/debug-info-compilation-dir.c (original)
+++ cfe/trunk/test/CodeGen/debug-info-compilation-dir.c Tue Dec 11 08:58:43 2018
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fdebug-compilation-dir /nonsense -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
+// RUN: mkdir -p %t.dir && cd %t.dir
+// RUN: cp %s rel.c
+// RUN: %clang_cc1 -fdebug-compilation-dir /nonsense -emit-llvm 
-debug-info-kind=limited rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
 // CHECK-NONSENSE: nonsense
 
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck 
-check-prefix=CHECK-DIR %s

Modified: cfe/trunk/test/PCH/debug-info-pch-path.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/debug-info-pch-path.c?rev=348865=348864=348865=diff
==
--- cfe/trunk/test/PCH/debug-info-pch-path.c (original)
+++