[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)

[Xing via Phabricator via cfe-commits]

Higuoxing added a comment.

In https://reviews.llvm.org/D47687#1266893, @vsapsai wrote:

> Sorry about the delay. The change seems to be correct but `ninja check-clang` 
> reveals the test "Misc/caret-diags-macros.c" is failing. Can you please look 
> into that?


Hi, It's because the expression is given in multiple lines and my 
`SuggestParentheses` cannot give proper fix-it hint. But I could give proper 
`ParenRange` in macros. Shall we reserve current `SuggestParentheses` and just 
give Parenrange hit in macros ? I have no idea about this ...

> Appreciate your contribution, Xing, and thanks for verifying your change with 
> tests.

My pleasure :) Thanks for your review.


https://reviews.llvm.org/D47687



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

[PATCH] D53472: Add gfx904 and gfx906 to GPU Arch

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344996: Add gfx904 and gfx906 to GPU Arch (authored by 
yaxunl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53472?vs=170311=170560#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53472

Files:
  cfe/trunk/include/clang/Basic/Cuda.h
  cfe/trunk/lib/Basic/Cuda.cpp
  cfe/trunk/lib/Basic/Targets/NVPTX.cpp


Index: cfe/trunk/lib/Basic/Targets/NVPTX.cpp
===
--- cfe/trunk/lib/Basic/Targets/NVPTX.cpp
+++ cfe/trunk/lib/Basic/Targets/NVPTX.cpp
@@ -188,6 +188,8 @@
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
   case CudaArch::LAST:
 break;
   case CudaArch::UNKNOWN:
Index: cfe/trunk/lib/Basic/Cuda.cpp
===
--- cfe/trunk/lib/Basic/Cuda.cpp
+++ cfe/trunk/lib/Basic/Cuda.cpp
@@ -90,6 +90,10 @@
 return "gfx900";
   case CudaArch::GFX902: // TBA
 return "gfx902";
+  case CudaArch::GFX904: // TBA
+return "gfx904";
+  case CudaArch::GFX906: // TBA
+return "gfx906";
   }
   llvm_unreachable("invalid enum");
 }
@@ -124,6 +128,8 @@
   .Case("gfx810", CudaArch::GFX810)
   .Case("gfx900", CudaArch::GFX900)
   .Case("gfx902", CudaArch::GFX902)
+  .Case("gfx904", CudaArch::GFX904)
+  .Case("gfx906", CudaArch::GFX906)
   .Default(CudaArch::UNKNOWN);
 }
 
@@ -233,6 +239,8 @@
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
 return CudaVirtualArch::COMPUTE_AMDGCN;
   }
   llvm_unreachable("invalid enum");
@@ -277,6 +285,8 @@
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
 return CudaVersion::CUDA_70;
   }
   llvm_unreachable("invalid enum");
Index: cfe/trunk/include/clang/Basic/Cuda.h
===
--- cfe/trunk/include/clang/Basic/Cuda.h
+++ cfe/trunk/include/clang/Basic/Cuda.h
@@ -62,6 +62,8 @@
   GFX810,
   GFX900,
   GFX902,
+  GFX904,
+  GFX906,
   LAST,
 };
 const char *CudaArchToString(CudaArch A);


Index: cfe/trunk/lib/Basic/Targets/NVPTX.cpp
===
--- cfe/trunk/lib/Basic/Targets/NVPTX.cpp
+++ cfe/trunk/lib/Basic/Targets/NVPTX.cpp
@@ -188,6 +188,8 @@
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
   case CudaArch::LAST:
 break;
   case CudaArch::UNKNOWN:
Index: cfe/trunk/lib/Basic/Cuda.cpp
===
--- cfe/trunk/lib/Basic/Cuda.cpp
+++ cfe/trunk/lib/Basic/Cuda.cpp
@@ -90,6 +90,10 @@
 return "gfx900";
   case CudaArch::GFX902: // TBA
 return "gfx902";
+  case CudaArch::GFX904: // TBA
+return "gfx904";
+  case CudaArch::GFX906: // TBA
+return "gfx906";
   }
   llvm_unreachable("invalid enum");
 }
@@ -124,6 +128,8 @@
   .Case("gfx810", CudaArch::GFX810)
   .Case("gfx900", CudaArch::GFX900)
   .Case("gfx902", CudaArch::GFX902)
+  .Case("gfx904", CudaArch::GFX904)
+  .Case("gfx906", CudaArch::GFX906)
   .Default(CudaArch::UNKNOWN);
 }
 
@@ -233,6 +239,8 @@
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
 return CudaVirtualArch::COMPUTE_AMDGCN;
   }
   llvm_unreachable("invalid enum");
@@ -277,6 +285,8 @@
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
 return CudaVersion::CUDA_70;
   }
   llvm_unreachable("invalid enum");
Index: cfe/trunk/include/clang/Basic/Cuda.h
===
--- cfe/trunk/include/clang/Basic/Cuda.h
+++ cfe/trunk/include/clang/Basic/Cuda.h
@@ -62,6 +62,8 @@
   GFX810,
   GFX900,
   GFX902,
+  GFX904,
+  GFX906,
   LAST,
 };
 const char *CudaArchToString(CudaArch A);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r344996 - Add gfx904 and gfx906 to GPU Arch

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Mon Oct 22 19:05:31 2018
New Revision: 344996

URL: http://llvm.org/viewvc/llvm-project?rev=344996=rev
Log:
Add gfx904 and gfx906 to GPU Arch

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

Modified:
cfe/trunk/include/clang/Basic/Cuda.h
cfe/trunk/lib/Basic/Cuda.cpp
cfe/trunk/lib/Basic/Targets/NVPTX.cpp

Modified: cfe/trunk/include/clang/Basic/Cuda.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Cuda.h?rev=344996=344995=344996=diff
==
--- cfe/trunk/include/clang/Basic/Cuda.h (original)
+++ cfe/trunk/include/clang/Basic/Cuda.h Mon Oct 22 19:05:31 2018
@@ -62,6 +62,8 @@ enum class CudaArch {
   GFX810,
   GFX900,
   GFX902,
+  GFX904,
+  GFX906,
   LAST,
 };
 const char *CudaArchToString(CudaArch A);

Modified: cfe/trunk/lib/Basic/Cuda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Cuda.cpp?rev=344996=344995=344996=diff
==
--- cfe/trunk/lib/Basic/Cuda.cpp (original)
+++ cfe/trunk/lib/Basic/Cuda.cpp Mon Oct 22 19:05:31 2018
@@ -90,6 +90,10 @@ const char *CudaArchToString(CudaArch A)
 return "gfx900";
   case CudaArch::GFX902: // TBA
 return "gfx902";
+  case CudaArch::GFX904: // TBA
+return "gfx904";
+  case CudaArch::GFX906: // TBA
+return "gfx906";
   }
   llvm_unreachable("invalid enum");
 }
@@ -124,6 +128,8 @@ CudaArch StringToCudaArch(llvm::StringRe
   .Case("gfx810", CudaArch::GFX810)
   .Case("gfx900", CudaArch::GFX900)
   .Case("gfx902", CudaArch::GFX902)
+  .Case("gfx904", CudaArch::GFX904)
+  .Case("gfx906", CudaArch::GFX906)
   .Default(CudaArch::UNKNOWN);
 }
 
@@ -233,6 +239,8 @@ CudaVirtualArch VirtualArchForCudaArch(C
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
 return CudaVirtualArch::COMPUTE_AMDGCN;
   }
   llvm_unreachable("invalid enum");
@@ -277,6 +285,8 @@ CudaVersion MinVersionForCudaArch(CudaAr
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
 return CudaVersion::CUDA_70;
   }
   llvm_unreachable("invalid enum");

Modified: cfe/trunk/lib/Basic/Targets/NVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/NVPTX.cpp?rev=344996=344995=344996=diff
==
--- cfe/trunk/lib/Basic/Targets/NVPTX.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/NVPTX.cpp Mon Oct 22 19:05:31 2018
@@ -188,6 +188,8 @@ void NVPTXTargetInfo::getTargetDefines(c
   case CudaArch::GFX810:
   case CudaArch::GFX900:
   case CudaArch::GFX902:
+  case CudaArch::GFX904:
+  case CudaArch::GFX906:
   case CudaArch::LAST:
 break;
   case CudaArch::UNKNOWN:


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

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

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

rsmith added a comment.

I'm very happy with reducing our configuration space like this, if the 
distinction is indeed unused (which it appears to be). (If compiling in ObjC1 
mode actually is useful, we should instead have a command-line flag for that 
and tests; the fact that we don't tells us almost everything we need to know 
here.)




Comment at: clang/include/clang/Basic/Features.def:95
+FEATURE(objc_kindof, LangOpts.ObjC)
+FEATURE(objc_modules, LangOpts.ObjC &)
 FEATURE(objc_nonfragile_abi, LangOpts.ObjCRuntime.isNonFragile())

You touched this line last, so it's now your fault that there's missing space 
after the `&&` :)



Comment at: clang/lib/Basic/IdentifierTable.cpp:166-167
   // in non-arc mode.
-  if (LangOpts.ObjC2 && (Flags & KEYARC)) return KS_Enabled;
-  if (LangOpts.ObjC2 && (Flags & KEYOBJC2)) return KS_Enabled;
+  if (LangOpts.ObjC && (Flags & KEYARC)) return KS_Enabled;
+  if (LangOpts.ObjC && (Flags & KEYOBJC)) return KS_Enabled;
   if (LangOpts.ConceptsTS && (Flags & KEYCONCEPTS)) return KS_Enabled;

Would it make sense to fold `KEYOBJC` and `KEYARC` together?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1365
 Results.AddResult(Result("bool", CCP_Type +
- (LangOpts.ObjC1? CCD_bool_in_ObjC : 0)));
+ (LangOpts.ObjC? CCD_bool_in_ObjC : 0)));
 Results.AddResult(Result("class", CCP_Type));

Space before `?` please.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3335
   else if (MacroName.equals("bool"))
-Priority = CCP_Type + (LangOpts.ObjC1? CCD_bool_in_ObjC : 0);
+Priority = CCP_Type + (LangOpts.ObjC? CCD_bool_in_ObjC : 0);
 

Space before `?` please.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:408
 // non-Apple platforms, but for now it is needed.
-m_compiler->getLangOpts().ObjC1 = true;
+m_compiler->getLangOpts().ObjC = true;
 break;

Curious, this looks like it was the *only* way we previously ever got into 
`ObjC1` mode. Any idea why this path turns on `ObjC1` but not `ObjC2`?


Repository:
  rC Clang

https://reviews.llvm.org/D53547



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

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:202
+  void finalizeConstraints() {
+Constraints.clear();
+  }

These constraints are conceptually part of the visitor, not part of the 
constraint manager. Could they be simply stored in the visitor?



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:235
+  /// Check if the state has changed based on the constraint has changed.
+  bool isChanged(const Stmt *Cond, StringRef Message) const;
+

Probably should be replaced by the expression above and inlined.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:187
 
-void
-BugReporterVisitor::finalizeVisitor(BugReporterContext &,
-const ExplodedNode *, BugReport &) {}
+void BugReporterVisitor::finalizeVisitor(BugReporterContext &,
+ const ExplodedNode *, BugReport &) {}

spurious change



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1831
+  const Stmt *Cond = srcBlk->getTerminatorCondition();
+  auto piece = VisitTerminator(term, N, srcBlk, BE->getDst(), BR, BRC);
+  if (piece && State->isChanged(Cond, piece->getString()))

capital letter


https://reviews.llvm.org/D53076



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

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:80
 class ConstraintManager {
+  using ConstraintMap = std::map;
+  ConstraintMap Constraints;

Traditionally LLVM projects use `llvm::DenseMap`



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:183
+  bool isChanged(const Stmt *Cond, StringRef Message) {
+ConstraintMap::iterator I = Constraints.find(Cond);
+

`return Constraints.insert(make_pair(Cond, Message)).second` ?


https://reviews.llvm.org/D53076



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

[PATCH] D53476: [clang-cl] Allowed -fopenmp work and link openmp library from per-runtime library directory

[Peiyuan Song via Phabricator via cfe-commits]

SquallATF updated this revision to Diff 170559.
SquallATF added a comment.

- add test


Repository:
  rC Clang

https://reviews.llvm.org/D53476

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/MSVC.cpp
  test/Driver/cl-options.c
  test/OpenMP/Inputs/resource_dir/x86_64-msvc-win32/lib/.keep
  test/OpenMP/linking.c


Index: test/OpenMP/linking.c
===
--- test/OpenMP/linking.c
+++ test/OpenMP/linking.c
@@ -83,11 +83,12 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libomp -target x86_64-msvc-win32 -rtlib=platform \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
 // CHECK-MSVC-LINK-64: link.exe
 // CHECK-MSVC-LINK-64-SAME: -nodefaultlib:vcomp.lib
 // CHECK-MSVC-LINK-64-SAME: -nodefaultlib:vcompd.lib
-// CHECK-MSVC-LINK-64-SAME: -libpath:{{.+}}/../lib
+// CHECK-MSVC-LINK-64-SAME: -libpath:{{.+[/\\]}}x86_64-msvc-win32{{/|}}lib
 // CHECK-MSVC-LINK-64-SAME: -defaultlib:libomp.lib
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 -fopenmp-simd 
-target x86_64-msvc-win32 -rtlib=platform | FileCheck --check-prefix 
SIMD-ONLY11 %s
@@ -97,6 +98,7 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 -rtlib=platform \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 -fopenmp-simd 
-target x86_64-msvc-win32 -rtlib=platform | FileCheck --check-prefix 
SIMD-ONLY11 %s
@@ -106,6 +108,6 @@
 // CHECK-MSVC-ILINK-64: link.exe
 // CHECK-MSVC-ILINK-64-SAME: -nodefaultlib:vcomp.lib
 // CHECK-MSVC-ILINK-64-SAME: -nodefaultlib:vcompd.lib
-// CHECK-MSVC-ILINK-64-SAME: -libpath:{{.+}}/../lib
+// CHECK-MSVC-ILINK-64-SAME: -libpath:{{.+[/\\]}}x86_64-msvc-win32{{/|}}lib
 // CHECK-MSVC-ILINK-64-SAME: -defaultlib:libiomp5md.lib
 //
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -572,6 +572,11 @@
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck 
-check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 
+// RUN: %clang_cl -fopenmp -### -- %s 2>&1 | FileCheck -check-prefix=OPENMP %s
+// RUN: %clang_cl -fopenmp=libomp -### -- %s 2>&1 | FileCheck 
-check-prefix=OPENMP %s
+// RUN: %clang_cl -fopenmp=libiomp5 -### -- %s 2>&1 | FileCheck 
-check-prefix=OPENMP %s
+// OPENMP: -fopenmp
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
@@ -611,6 +616,8 @@
 // RUN: -fmerge-all-constants \
 // RUN: -no-canonical-prefixes \
 // RUN: -march=skylake \
+// RUN: -fopenmp \
+// RUN: -fopenmp=libomp \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -422,8 +422,9 @@
options::OPT_fno_openmp, false)) {
 CmdArgs.push_back("-nodefaultlib:vcomp.lib");
 CmdArgs.push_back("-nodefaultlib:vcompd.lib");
-CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
- TC.getDriver().Dir + "/../lib"));
+for (const auto  : TC.getLibraryPaths()) {
+  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") + 
LibPath));
+}
 switch (TC.getDriver().getOpenMPRuntime(Args)) {
 case Driver::OMPRT_OMP:
   CmdArgs.push_back("-defaultlib:libomp.lib");
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1516,11 +1516,11 @@
 
 def fobjc_sender_dependent_dispatch : Flag<["-"], 
"fobjc-sender-dependent-dispatch">, Group;
 def fomit_frame_pointer : Flag<["-"], "fomit-frame-pointer">, Group;
-def fopenmp : Flag<["-"], "fopenmp">, Group, Flags<[CC1Option, 
NoArgumentUnused]>,
+def fopenmp : Flag<["-"], "fopenmp">, Group, Flags<[CC1Option, 
CoreOption, NoArgumentUnused]>,
   HelpText<"Parse OpenMP pragmas and generate parallel code.">;
 def fno_openmp : Flag<["-"], "fno-openmp">, Group, 
Flags<[NoArgumentUnused]>;
 def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, 
Flags<[CC1Option, NoArgumentUnused]>;
-def fopenmp_EQ : Joined<["-"], "fopenmp=">, Group;
+def fopenmp_EQ : Joined<["-"], "fopenmp=">, Group, 
Flags<[CoreOption]>;
 def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group,
   Flags<[NoArgumentUnused, HelpHidden]>;
 def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group,


Index: test/OpenMP/linking.c
===
--- 

r344992 - [analyzer] [NFC] Correct comment on RetainSummaryManager

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Oct 22 18:31:08 2018
New Revision: 344992

URL: http://llvm.org/viewvc/llvm-project?rev=344992=rev
Log:
[analyzer] [NFC] Correct comment on RetainSummaryManager

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=344992=344991=344992=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Mon Oct 
22 18:31:08 2018
@@ -56,7 +56,7 @@ enum ArgEffect {
 
   /// The argument has its reference count decreased by 1.  This is as
   /// if a -release message has been sent to the argument.  This differs
-  /// in behavior from DecRef when GC is enabled.
+  /// in behavior from DecRef when ARC is enabled.
   DecRefMsg,
 
   /// The argument has its reference count decreased by 1 to model
@@ -65,7 +65,7 @@ enum ArgEffect {
 
   /// The argument has its reference count increased by 1.  This is as
   /// if a -retain message has been sent to the argument.  This differs
-  /// in behavior from IncRef when GC is enabled.
+  /// in behavior from IncRef when ARC is enabled.
   IncRefMsg,
 
   /// The argument has its reference count increased by 1.  This is as


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

r344991 - [analyzer] [www] Drop references to GC mode, which was deprecated years ago

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Oct 22 18:30:45 2018
New Revision: 344991

URL: http://llvm.org/viewvc/llvm-project?rev=344991=rev
Log:
[analyzer] [www] Drop references to GC mode, which was deprecated years ago

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

Removed:
cfe/trunk/www/analyzer/images/example_cf_returns_retained_gc.png
Modified:
cfe/trunk/www/analyzer/annotations.html

Modified: cfe/trunk/www/analyzer/annotations.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/annotations.html?rev=344991=344990=344991=diff
==
--- cfe/trunk/www/analyzer/annotations.html (original)
+++ cfe/trunk/www/analyzer/annotations.html Mon Oct 22 18:30:45 2018
@@ -152,16 +152,6 @@ analyzer typically does not make any ass
 is returned retained. Explicitly adding the 'ns_returns_retained' attribute to 
C
 functions allows the analyzer to perform extra checking.
 
-Important note when using Garbage Collection: Note that the analyzer
-interprets this attribute slightly differently when using Objective-C garbage
-collection (available on Mac OS 10.5+). When analyzing Cocoa code that uses
-garbage collection, alloc methods are assumed to return an object
-that is managed by the garbage collector (and thus doesn't have a retain count
-the caller must balance). These same assumptions are applied to methods or
-functions annotated with 'ns_returns_retained'. If you are returning a Core
-Foundation object (which may not be managed by the garbage collector) you 
should
-use 'cf_returns_retained'.
-
 Example
 
 
@@ -243,16 +233,15 @@ that is functionally equivalent to the o
 Placing on Objective-C methods: With respect to Objective-C methods.,
 this attribute is identical in its behavior and usage to 'ns_returns_retained'
 except for the distinction of returning a Core Foundation object instead of a
-Cocoa object. This distinction is important for two reasons:
+Cocoa object.
 
-
-  Core Foundation objects are not automatically managed by the Objective-C
-  garbage collector.
-  Because Core Foundation is a C API, the analyzer cannot always tell that 
a
-  pointer return value refers to a Core Foundation object. In contrast, it is
-  trivial for the analyzer to recognize if a pointer refers to a Cocoa object
-  (given the Objective-C type system).
-
+This distinction is important for the following reason:
+as Core Foundation is a C API,
+the analyzer cannot always tell that a pointer return value refers to a
+Core Foundation object.
+In contrast, it is
+trivial for the analyzer to recognize if a pointer refers to a Cocoa object
+(given the Objective-C type system).
 
 Placing on C functions: When placing the attribute
 'cf_returns_retained' on the declarations of C functions, the analyzer
@@ -264,17 +253,11 @@ interprets the function as:
 contained the keywords create or copy. This means the
 returned object as a +1 retain count that must be released by the caller, 
either
 by sending a release message (via toll-free bridging to an Objective-C
-object pointer), calling CFRelease (or similar function), or using
-CFMakeCollectable to register the object with the Objective-C garbage
-collector.
+object pointer), or calling CFRelease or a similar function.
 
 
 Example
 
-In this example, observe the difference in output when the code is compiled
-to not use garbage collection versus when it is compiled to only use garbage
-collection (-fobjc-gc-only).
-
 
 $ cat test.m
 $ cat test.m
@@ -322,15 +305,6 @@ CFDateRef returnsRetainedCFDate()  {
 
 
 
-When the above code is compiled using Objective-C garbage collection (i.e.,
-code is compiled with the flag -fobjc-gc or -fobjc-gc-only),
-scan-build produces both the above error (with slightly different text
-to indicate the code uses garbage collection) as well as the following warning,
-which indicates a leak that occurs only when using garbage
-collection:
-
-
-
 Attribute 'cf_returns_not_retained'
 (Clang-specific)
 
@@ -372,13 +346,6 @@ static analyzer that a release
 parameter upon completion of the call to the given function or method. The 
 Foundation framework defines a macro NS_RELEASES_ARGUMENT that 
 is functionally equivalent to the NS_CONSUMED macro shown below.
-  
-Important note when using Garbage Collection: Note that the analyzer
-essentially ignores this attribute when code is compiled to use Objective-C
-garbage collection.  This is because the release message does nothing
-when using GC.  If the underlying function/method uses something like
-CFRelease to decrement the reference count, consider using
-the cf_consumed attribute instead.
 
 Example
 
@@ -432,10 +399,7 @@ to the given function or method. The Cor
 CF_RELEASES_ARGUMENT that is functionally equivalent to the
 CF_CONSUMED macro shown below.
 
-Operationally this attribute is nearly identical to 'ns_consumed' with the
-main difference that the reference count decrement still occurs when using

r344990 - [analyzer] [testing] Compute data on path length, compute percentiles

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Oct 22 18:30:26 2018
New Revision: 344990

URL: http://llvm.org/viewvc/llvm-project?rev=344990=rev
Log:
[analyzer] [testing] Compute data on path length, compute percentiles

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

Modified:
cfe/trunk/utils/analyzer/CmpRuns.py

Modified: cfe/trunk/utils/analyzer/CmpRuns.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/CmpRuns.py?rev=344990=344989=344990=diff
==
--- cfe/trunk/utils/analyzer/CmpRuns.py (original)
+++ cfe/trunk/utils/analyzer/CmpRuns.py Mon Oct 22 18:30:26 2018
@@ -281,9 +281,21 @@ def compareResults(A, B, opts):
 
 return res
 
+def computePercentile(l, percentile):
+"""
+Return computed percentile.
+"""
+return sorted(l)[int(round(percentile * len(l) + 0.5)) - 1]
+
 def deriveStats(results):
 # Assume all keys are the same in each statistics bucket.
 combined_data = defaultdict(list)
+
+# Collect data on paths length.
+for report in results.reports:
+for diagnostic in report.diagnostics:
+combined_data['PathsLength'].append(diagnostic.getPathLength())
+
 for stat in results.stats:
 for key, value in stat.iteritems():
 combined_data[key].append(value)
@@ -293,6 +305,8 @@ def deriveStats(results):
 "max": max(values),
 "min": min(values),
 "mean": sum(values) / len(values),
+"90th %tile": computePercentile(values, 0.9),
+"95th %tile": computePercentile(values, 0.95),
 "median": sorted(values)[len(values) / 2],
 "total": sum(values)
 }


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

[PATCH] D52844: [analyzer] [testing] Compute data on path length, compute percentiles

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344990: [analyzer] [testing] Compute data on path length, 
compute percentiles (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52844?vs=168160=170556#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52844

Files:
  utils/analyzer/CmpRuns.py


Index: utils/analyzer/CmpRuns.py
===
--- utils/analyzer/CmpRuns.py
+++ utils/analyzer/CmpRuns.py
@@ -281,9 +281,21 @@
 
 return res
 
+def computePercentile(l, percentile):
+"""
+Return computed percentile.
+"""
+return sorted(l)[int(round(percentile * len(l) + 0.5)) - 1]
+
 def deriveStats(results):
 # Assume all keys are the same in each statistics bucket.
 combined_data = defaultdict(list)
+
+# Collect data on paths length.
+for report in results.reports:
+for diagnostic in report.diagnostics:
+combined_data['PathsLength'].append(diagnostic.getPathLength())
+
 for stat in results.stats:
 for key, value in stat.iteritems():
 combined_data[key].append(value)
@@ -293,6 +305,8 @@
 "max": max(values),
 "min": min(values),
 "mean": sum(values) / len(values),
+"90th %tile": computePercentile(values, 0.9),
+"95th %tile": computePercentile(values, 0.95),
 "median": sorted(values)[len(values) / 2],
 "total": sum(values)
 }


Index: utils/analyzer/CmpRuns.py
===
--- utils/analyzer/CmpRuns.py
+++ utils/analyzer/CmpRuns.py
@@ -281,9 +281,21 @@
 
 return res
 
+def computePercentile(l, percentile):
+"""
+Return computed percentile.
+"""
+return sorted(l)[int(round(percentile * len(l) + 0.5)) - 1]
+
 def deriveStats(results):
 # Assume all keys are the same in each statistics bucket.
 combined_data = defaultdict(list)
+
+# Collect data on paths length.
+for report in results.reports:
+for diagnostic in report.diagnostics:
+combined_data['PathsLength'].append(diagnostic.getPathLength())
+
 for stat in results.stats:
 for key, value in stat.iteritems():
 combined_data[key].append(value)
@@ -293,6 +305,8 @@
 "max": max(values),
 "min": min(values),
 "mean": sum(values) / len(values),
+"90th %tile": computePercentile(values, 0.9),
+"95th %tile": computePercentile(values, 0.95),
 "median": sorted(values)[len(values) / 2],
 "total": sum(values)
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I hope the cleanup makes the code more easily digestible, and to some extent 
might also transport why I think this is the most elegant approach.

I think we should document the semantics of scoped capabilities in more detail, 
and I will do so once this is either merged, or we decided that we don't want 
to merge it.




Comment at: lib/Analysis/ThreadSafety.cpp:893
 private:
-  SmallVector UnderlyingMutexes;
+  enum UnderlyingCapabilityKind {
+UCK_Acquired,  ///< Any kind of acquired capability.

aaronpuchert wrote:
> delesley wrote:
> > aaronpuchert wrote:
> > > delesley wrote:
> > > > IMHO, it would make more sense to break this into two properties (or 
> > > > bits): acquired/released and shared/exclusive. 
> > > We don't use the information (shared/exclusive) for acquired locks, but 
> > > we need two bits anyway, so why not.
> > The normal unlock doesn't distinguish between shared/exclusive, but there 
> > is a release_shared_capability which does...
> Right, but see [bug 33504](https://bugs.llvm.org/show_bug.cgi?id=33504): 
> currently `release_shared_capability` does not work with scoped capabilities. 
> If we allow it, we would have to discuss when it is allowed. A scoped 
> capability can lock multiple locks, some of them shared, some of them 
> exclusive.
> 
> My assumption, as I wrote in the bug: scoped capabilities are always 
> exclusive, hence can only be unlocked exclusively, but automatically release 
> underlying mutexes in the right mode.
I've thought about splitting up `UCK_Acquired` into a shared and exclusive 
variant, but not done it. Here's why: we don't need to know how the mutex was 
initially acquired for releasing it, and for re-acquisition we use the 
attribute on the "relock" method of the scoped capability. With released 
capabilities however we need to know in which mode they should be acquired 
again. Another reason is that we might want a fourth enumeration value to 
implement something in the direction of 
[`std::defer_lock_t`](https://en.cppreference.com/w/cpp/thread/unique_lock/unique_lock),
 and we might only have two bits.



Comment at: lib/Analysis/ThreadSafety.cpp:951
+}
   } else {
+// We're removing the underlying mutex. Warn on double unlocking.

aaronpuchert wrote:
> delesley wrote:
> > aaronpuchert wrote:
> > > delesley wrote:
> > > > I find this very confusing.  A lock here unlocks the underlying mutex, 
> > > > and vice versa.  At the very least, some methods need to be renamed, or 
> > > > maybe we can have separate classes for ScopedLockable and 
> > > > ScopedUnlockable. 
> > > I agree that it's confusing, but it follows what I think was the idea 
> > > behind scoped capabilities: that they are also capabilities that can be 
> > > acquired/released. Now since the scoped capability releases a mutex on 
> > > construction (i.e. when it is acquired), it has to acquire the mutex when 
> > > released. So the way it handles the released mutexes mirrors what happens 
> > > on the scoped capability itself.
> > > 
> > > It's definitely not very intuitive, but I feel it's the most consistent 
> > > variant with what we have already.
> > > 
> > > The nice thing about this is that it works pretty well with the existing 
> > > semantics and allows constructs such as `MutexLockUnlock` (see the tests) 
> > > that unlocks one mutex and locks another. Not sure if anyone needs this, 
> > > but why not?
> > A scoped_lockable object is not a capability, it is an object that manages 
> > capabilities.  IMHO, conflating those two concepts is a bad idea, and 
> > recipe for confusion.  You would not, for example, use a scoped_lockable 
> > object in a guarded_by attribute.  
> > 
> > Unfortunately, the existing code tends to conflate "capabilities" and 
> > "facts", which is my fault.  A scoped_lockable object is a valid fact -- 
> > the fact records whether the object is in scope -- it's just not a valid 
> > capability. 
> > 
> > Simply renaming the methods from handleLock/Unlock to  addFact/removeFact 
> > would be a nice first step in making the code clearer, and would 
> > distinguish between facts that refer to capabilities, and facts that refer 
> > to scoped objects.  
> > 
> > 
> > 
> > 
> Makes sense to me. I'll see if I can refactor some of the code to clarify 
> this.
> 
> I think that we should also separate between unlocking the underlying mutexes 
> and destructing the scoped_lockable, which is currently handled via the extra 
> parameter `FullyRemove` of `handleUnlock`.
Scoped capabilities are not the same as capabilities, but they are also more 
than just facts. (Except if you follow 
[Wittgenstein](https://en.wikipedia.org/wiki/Tractatus_Logico-Philosophicus#Proposition_1),
 perhaps.) They can be released, and reacquired, which makes them more than 
mere immutable facts. I think we can consider them as proxy capabilities. They 

r344987 - [CodeGen] Attach InlineHint to more functions

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Mon Oct 22 18:26:28 2018
New Revision: 344987

URL: http://llvm.org/viewvc/llvm-project?rev=344987=rev
Log:
[CodeGen] Attach InlineHint to more functions

For instantiated functions, search the template pattern to see if it marked
inline to determine if InlineHint attribute should be added to the function.


Added:
cfe/trunk/test/CodeGenCXX/inline-template-hint.cpp
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344987=344986=344987=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 18:26:28 2018
@@ -1299,9 +1299,19 @@ void CodeGenModule::SetLLVMFunctionAttri
 // Otherwise, propagate the inline hint attribute and potentially use its
 // absence to mark things as noinline.
 if (auto *FD = dyn_cast(D)) {
-  if (any_of(FD->redecls(), [&](const FunctionDecl *Redecl) {
-return Redecl->isInlineSpecified();
-  })) {
+  // Search function and template pattern redeclarations for inline.
+  auto CheckForInline = [](const FunctionDecl *FD) {
+auto CheckRedeclForInline = [](const FunctionDecl *Redecl) {
+  return Redecl->isInlineSpecified();
+};
+if (any_of(FD->redecls(), CheckRedeclForInline))
+  return true;
+const FunctionDecl *Pattern = FD->getTemplateInstantiationPattern();
+if (!Pattern)
+  return false;
+return any_of(Pattern->redecls(), CheckRedeclForInline);
+  };
+  if (CheckForInline(FD)) {
 B.addAttribute(llvm::Attribute::InlineHint);
   } else if (CodeGenOpts.getInlining() ==
  CodeGenOptions::OnlyHintInlining &&

Added: cfe/trunk/test/CodeGenCXX/inline-template-hint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/inline-template-hint.cpp?rev=344987=auto
==
--- cfe/trunk/test/CodeGenCXX/inline-template-hint.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/inline-template-hint.cpp Mon Oct 22 18:26:28 2018
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 %s -std=c++11 -triple=x86_64-linux -O2 \
+// RUN:   -finline-functions -emit-llvm -disable-llvm-passes -o - \
+// RUN: | FileCheck -allow-deprecated-dag-overlap %s \
+// RUN:   --check-prefix=CHECK --check-prefix=SUITABLE
+// RUN: %clang_cc1 %s -std=c++11 -triple=x86_64-linux -O2 \
+// RUN:   -finline-hint-functions -emit-llvm -disable-llvm-passes -o - \
+// RUN: | FileCheck -allow-deprecated-dag-overlap %s \
+// RUN:   --check-prefix=CHECK --check-prefix=HINTED
+// RUN: %clang_cc1 %s -std=c++11 -triple=x86_64-linux -O2 \
+// RUN:   -fno-inline -emit-llvm -disable-llvm-passes -o - \
+// RUN: | FileCheck -allow-deprecated-dag-overlap %s \
+// RUN:   --check-prefix=CHECK --check-prefix=NOINLINE
+
+struct A {
+  inline void int_run(int);
+
+  template 
+  inline void template_run(T);
+};
+
+// CHECK: @_ZN1A7int_runEi({{.*}}) [[ATTR:#[0-9]+]]
+void A::int_run(int) {}
+// CHECK: @_ZN1A12template_runIiEEvT_({{.*}}) [[ATTR]]
+template 
+void A::template_run(T) {}
+
+void bar() {
+  A().int_run(1);
+  A().template_run(1);
+}
+
+// SUITABLE: attributes [[ATTR]] = { {{.*}}inlinehint{{.*}} }
+//   HINTED: attributes [[ATTR]] = { {{.*}}inlinehint{{.*}} }
+// NOINLINE: attributes [[ATTR]] = { {{.*}}noinline{{.*}} }


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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

@rjmccall I prototyped the ForRTTI parameter approach in 
https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, but 
it demonstrates the problems I saw with the added parameter. Namely, 
`mangleType(QualType, SourceRange, QualifierMangleMode)` has a bunch of 
additional logic which is needed for correctness, so we need to thread the 
parameter through the entire chain, and the only way I could think of doing 
that without adding the parameter to every single `mangleType` overload was to 
have an additional switch to dispatch to the overloads with the added ForRTTI 
parameter, which is pretty ugly.

As I see it, there are a few ways to proceed here:

- The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I think 
it's pretty ugly, but you may have suggestions on how to do it better.
- In the `mangleType` overload for `ObjCObjectPointerType`, skipping the 
generic `mangleType` dispatch and going directly to either the `ObjCObjectType` 
or `ObjCInterfaceType` overloads. That avoids the ugliness in the generic 
`mangleType`, but it also requires us to duplicate some logic from it in the 
`ObjCObjectPointerType` overload, which doesn't seem great either.
- Maintaining `ForRTTI` state in the mangler instead of threading a parameter 
through (I'm generally not a fan of state like that, but it might be cleaner 
than the threading in this case?)
- Just sticking with what I'm doing in this patch.

What do you think?


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, rsmith.
Herald added a subscriber: dexonsmith.

We haven't supported compiling ObjC1 for a long time (and never will again), so 
there isn't any reason to keep these separate. This patch replaces 
LangOpts::ObjC1 and LangOpts::ObjC2 with LangOpts::ObjC. See a recent thread on 
cfe-dev here: http://lists.llvm.org/pipermail/cfe-dev/2018-September/059468.html

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D53547

Files:
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/objc/AvoidNSErrorInitCheck.cpp
  clang-tools-extra/clang-tidy/objc/ForbiddenSubclassingCheck.cpp
  clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Parse/Parser.h
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/Analysis/FormatString.cpp
  clang/lib/Basic/Builtins.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/test/Modules/module_file_info.m
  clang/tools/arcmt-test/arcmt-test.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/libclang/CXType.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
  lldb/source/Symbol/ClangASTContext.cpp

Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -459,7 +459,7 @@
   if (IK.getLanguage() == InputKind::Asm) {
 Opts.AsmPreprocessor = 1;
   } else if (IK.isObjectiveC()) {
-Opts.ObjC1 = Opts.ObjC2 = 1;
+Opts.ObjC = 1;
   }
 
   LangStandard::Kind LangStd = LangStandard::lang_unspecified;
Index: lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
===
--- lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
+++ lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
@@ -155,7 +155,8 @@
   // Let's just enable the latest ObjC and C++ which should get most tokens
   // right.
   LangOptions Opts;
-  Opts.ObjC2 = true;
+  Opts.ObjC = true;
+  // FIXME: This should probably set CPlusPlus, CPlusPlus11, ... too
   Opts.CPlusPlus17 = true;
   Opts.LineComment = true;
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -381,8 +381,7 @@
 m_compiler->getLangOpts().CPlusPlus = true;
 break;
   case lldb::eLanguageTypeObjC:
-m_compiler->getLangOpts().ObjC1 = true;
-m_compiler->getLangOpts().ObjC2 = true;
+m_compiler->getLangOpts().ObjC = true;
 // FIXME: the following language option is a temporary workaround,
 // to "ask for ObjC, get ObjC++" (see comment above).
 m_compiler->getLangOpts().CPlusPlus = true;

[PATCH] D52418: [driver][mips] Enable integrated assembler for MIPS64 except N32 ABI selected

[Brad Smith via Phabricator via cfe-commits]

brad added a comment.

How is it going with regard to N32 fixes / testing?


Repository:
  rC Clang

https://reviews.llvm.org/D52418



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

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 170545.
aaronpuchert marked 3 inline comments as done.
aaronpuchert added a comment.
This revision is now accepted and ready to land.

Addressed some review comments and simplified the code.

There is a lot less duplication and maybe it's even easier to understand.


Repository:
  rC Clang

https://reviews.llvm.org/D52578

Files:
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2787,6 +2787,110 @@
 } // end namespace RelockableScopedLock
 
 
+namespace ScopedUnlock {
+
+class SCOPED_LOCKABLE MutexUnlock {
+public:
+  MutexUnlock(Mutex *mu) EXCLUSIVE_UNLOCK_FUNCTION(mu);
+  ~MutexUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
+
+  void Lock() EXCLUSIVE_UNLOCK_FUNCTION();
+  void Unlock() EXCLUSIVE_LOCK_FUNCTION();
+};
+
+class SCOPED_LOCKABLE ReaderMutexUnlock {
+public:
+  ReaderMutexUnlock(Mutex *mu) SHARED_UNLOCK_FUNCTION(mu);
+  ~ReaderMutexUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
+
+  void Lock() EXCLUSIVE_UNLOCK_FUNCTION();
+  void Unlock() EXCLUSIVE_LOCK_FUNCTION();
+};
+
+Mutex mu;
+int x GUARDED_BY(mu);
+bool c;
+void print(int);
+
+void simple() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+  x = 1;
+  MutexUnlock scope();
+  x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void simpleShared() SHARED_LOCKS_REQUIRED(mu) {
+  print(x);
+  ReaderMutexUnlock scope();
+  print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
+}
+
+void innerUnlock() {
+  MutexLock outer();
+  if (x == 0) {
+MutexUnlock inner();
+x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+  }
+  x = 2;
+}
+
+void innerUnlockShared() {
+  ReaderMutexLock outer();
+  if (x == 0) {
+ReaderMutexUnlock inner();
+print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
+  }
+  print(x);
+}
+
+void manual() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+  MutexUnlock scope();
+  scope.Lock();
+  x = 2;
+  scope.Unlock();
+  x = 3; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void join() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+  MutexUnlock scope();
+  if (c) {
+scope.Lock(); // expected-note{{mutex acquired here}}
+  }
+  // expected-warning@+1{{mutex 'mu' is not held on every path through here}}
+  scope.Lock();
+}
+
+void doubleLock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+  MutexUnlock scope();
+  scope.Lock();
+  scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
+}
+
+void doubleUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+  MutexUnlock scope();
+  scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
+}
+
+class SCOPED_LOCKABLE MutexLockUnlock {
+public:
+  MutexLockUnlock(Mutex *mu1, Mutex *mu2) EXCLUSIVE_UNLOCK_FUNCTION(mu1) EXCLUSIVE_LOCK_FUNCTION(mu2);
+  ~MutexLockUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
+
+  void Release() EXCLUSIVE_UNLOCK_FUNCTION();
+  void Acquire() EXCLUSIVE_LOCK_FUNCTION();
+};
+
+Mutex other;
+void fn() EXCLUSIVE_LOCKS_REQUIRED(other);
+
+void lockUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+  MutexLockUnlock scope(, );
+  fn();
+  x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+} // end namespace ScopedUnlock
+
+
 namespace TrylockFunctionTest {
 
 class Foo {
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -42,6 +42,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -890,45 +891,81 @@
 
 class ScopedLockableFactEntry : public FactEntry {
 private:
-  SmallVector UnderlyingMutexes;
+  enum UnderlyingCapabilityKind {
+UCK_Acquired,  ///< Any kind of acquired capability.
+UCK_ReleasedShared,///< Shared capability that was released.
+UCK_ReleasedExclusive, ///< Exclusive capability that was released.
+  };
+
+  static LockKind getUnlockKind(UnderlyingCapabilityKind kind) {
+switch (kind) {
+case UCK_Acquired:
+  return LK_Exclusive;
+case UCK_ReleasedShared:
+  return LK_Shared;
+case UCK_ReleasedExclusive:
+  return LK_Exclusive;
+}
+llvm_unreachable("Unknown UnderlyingCapabilityKind");
+  }
+
+  using UnderlyingCapability =
+  llvm::PointerIntPair;
+
+  SmallVector UnderlyingMutexes;
 
 public:
   ScopedLockableFactEntry(const CapabilityExpr , SourceLocation Loc,
-  const CapExprSet , const CapExprSet )
+  const CapExprSet , const 

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

[Artem Dergachev via Phabricator via cfe-commits]

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

Remove weird flags that i used for debugging from the test run-lines.


https://reviews.llvm.org/D53543

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/NewDelete-custom.cpp
  test/Analysis/NewDelete-sized-deallocation.cpp

Index: test/Analysis/NewDelete-sized-deallocation.cpp
===
--- /dev/null
+++ test/Analysis/NewDelete-sized-deallocation.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -fsized-deallocation
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -DTEST_INLINABLE_ALLOCATORS
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -DTEST_INLINABLE_ALLOCATORS -fsized-deallocation
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void leak() {
+  int *x = new int; // expected-note{{Memory is allocated}}
+} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+  // expected-note@-1{{Potential leak of memory pointed to by 'x'}}
+
+// This function was incorrectly diagnosed as leak under -fsized-deallocation
+// because the sized operator delete was mistaken for a custom delete.
+void no_leak() {
+  int *x = new int; // no-note
+  delete x;
+} // no-warning
Index: test/Analysis/NewDelete-custom.cpp
===
--- test/Analysis/NewDelete-custom.cpp
+++ test/Analysis/NewDelete-custom.cpp
@@ -4,9 +4,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
 #include "Inputs/system-header-simulator-cxx.h"
 
-#if !(LEAKS && !ALLOCATOR_INLINING)
 // expected-no-diagnostics
-#endif
 
 
 void *allocator(std::size_t size);
@@ -24,50 +22,37 @@
 void testNewMethod() {
   void *p1 = C::operator new(0); // no warn
 
-  C *p2 = new C; // no warn
+  C *p2 = new C; // no-warning
 
-  C *c3 = ::new C;
+  C *c3 = ::new C; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'c3'}}
-#endif
 
 void testOpNewArray() {
   void *p = operator new[](0); // call is inlined, no warn
 }
 
 void testNewExprArray() {
-  int *p = new int[0];
+  int *p = new int[0]; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
 
 
 //- Custom non-placement operators
 void testOpNew() {
   void *p = operator new(0); // call is inlined, no warn
 }
 
 void testNewExpr() {
-  int *p = new int;
+  int *p = new int; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
-
 
 //- Custom NoThrow placement operators
 void testOpNewNoThrow() {
   void *p = operator new(0, std::nothrow); // call is inlined, no warn
 }
 
 void testNewExprNoThrow() {
-  int *p = new(std::nothrow) int;
+  int *p = new(std::nothrow) int; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
 
 //- Custom placement operators
 void testOpNewPlacement() {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -712,10 +712,8 @@
   return false;
 }
 
-// Tells if the callee is one of the following:
-// 1) A global non-placement new/delete operator function.
-// 2) A global placement operator function with the single placement argument
-//of type std::nothrow_t.
+// Tells if the callee is one of the builtin new/delete operators, including
+// placement operators and other standard overloads.
 bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD,
 ASTContext ) const {
   if (!FD)
@@ -726,23 +724,9 @@
   Kind != OO_Delete && Kind != OO_Array_Delete)
 return false;
 
-  // Skip all operator new/delete methods.
-  if (isa(FD))
-return false;
-
-  // Return true if tested operator is a standard placement nothrow operator.
-  if (FD->getNumParams() == 2) {
-QualType T = FD->getParamDecl(1)->getType();
-if (const IdentifierInfo *II = T.getBaseTypeIdentifier())
-  return II->getName().equals("nothrow_t");
-  }
-
-  // Skip placement operators.
-  if (FD->getNumParams() != 1 || FD->isVariadic())
-return false;
-
-  // One of the standard new/new[]/delete/delete[] non-placement operators.
-  return true;
+  // This is standard iff it's not defined in a user file.
+  SourceLocation L = FD->getLocation();
+  return !L.isValid() || 

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

[Artem Dergachev via Phabricator via cfe-commits]

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

Nono, that was an old patch, sry. Here's the correct patch.


https://reviews.llvm.org/D53543

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/NewDelete-custom.cpp
  test/Analysis/NewDelete-sized-deallocation.cpp

Index: test/Analysis/NewDelete-sized-deallocation.cpp
===
--- /dev/null
+++ test/Analysis/NewDelete-sized-deallocation.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text -analyzer-config prune-paths=false %s
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text -analyzer-config prune-paths=false -fsized-deallocation %s
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text -analyzer-config prune-paths=false -DTEST_INLINABLE_ALLOCATORS %s
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text -analyzer-config prune-paths=false -DTEST_INLINABLE_ALLOCATORS -fsized-deallocation %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void leak() {
+  int *x = new int; // expected-note{{Memory is allocated}}
+} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+  // expected-note@-1{{Potential leak of memory pointed to by 'x'}}
+
+// This function was incorrectly diagnosed as leak under -fsized-deallocation
+// because the sized operator delete was mistaken for a custom delete.
+void no_leak() {
+  int *x = new int; // no-note
+  delete x;
+} // no-warning
Index: test/Analysis/NewDelete-custom.cpp
===
--- test/Analysis/NewDelete-custom.cpp
+++ test/Analysis/NewDelete-custom.cpp
@@ -4,9 +4,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
 #include "Inputs/system-header-simulator-cxx.h"
 
-#if !(LEAKS && !ALLOCATOR_INLINING)
 // expected-no-diagnostics
-#endif
 
 
 void *allocator(std::size_t size);
@@ -24,50 +22,37 @@
 void testNewMethod() {
   void *p1 = C::operator new(0); // no warn
 
-  C *p2 = new C; // no warn
+  C *p2 = new C; // no-warning
 
-  C *c3 = ::new C;
+  C *c3 = ::new C; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'c3'}}
-#endif
 
 void testOpNewArray() {
   void *p = operator new[](0); // call is inlined, no warn
 }
 
 void testNewExprArray() {
-  int *p = new int[0];
+  int *p = new int[0]; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
 
 
 //- Custom non-placement operators
 void testOpNew() {
   void *p = operator new(0); // call is inlined, no warn
 }
 
 void testNewExpr() {
-  int *p = new int;
+  int *p = new int; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
-
 
 //- Custom NoThrow placement operators
 void testOpNewNoThrow() {
   void *p = operator new(0, std::nothrow); // call is inlined, no warn
 }
 
 void testNewExprNoThrow() {
-  int *p = new(std::nothrow) int;
+  int *p = new(std::nothrow) int; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
 
 //- Custom placement operators
 void testOpNewPlacement() {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -712,10 +712,8 @@
   return false;
 }
 
-// Tells if the callee is one of the following:
-// 1) A global non-placement new/delete operator function.
-// 2) A global placement operator function with the single placement argument
-//of type std::nothrow_t.
+// Tells if the callee is one of the builtin new/delete operators, including
+// placement operators and other standard overloads.
 bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD,
 ASTContext ) const {
   if (!FD)
@@ -726,23 +724,9 @@
   Kind != OO_Delete && Kind != OO_Array_Delete)
 return false;
 
-  // Skip all operator new/delete methods.
-  if (isa(FD))
-return false;
-
-  // Return true if tested operator is a standard placement nothrow operator.
-  if (FD->getNumParams() == 2) {
-QualType T = FD->getParamDecl(1)->getType();
-if (const IdentifierInfo *II = T.getBaseTypeIdentifier())
-  return II->getName().equals("nothrow_t");
-  }
-
-  // Skip placement operators.
-  if (FD->getNumParams() != 1 || FD->isVariadic())
-return false;
-
-  // One of the standard new/new[]/delete/delete[] non-placement operators.
-  return 

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

[Artem Dergachev via Phabricator via cfe-commits]

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

Fixes https://bugs.llvm.org/show_bug.cgi?id=39348

Under `-fsized-deallocation`, the new test case in 
`NewDelete-sized-deallocation.cpp` produces a call to operator delete with an 
explicitly passed size argument. MallocChecker didn't expect that and 
identified this as a custom operator delete, which lead to a leak false 
positive on extremely simple code.

Identify custom operators by source locations instead of guessing by the number 
of parameters. This sounds much more correct.

Additionally, it exposes a regression in `NewDelete-intersections.mm`'s 
`testStandardPlacementNewAfterDelete()` test, where the diagnostic is delayed 
from before the call of placement new into the code of placement new in the 
header. This happens because the check for pass-into-function-after-free for 
placement arguments is located in `checkNewAllocator()`, which happens after 
the allocator is inlined, which is too late. Move this use-after-free check 
into `checkPreCall` instead, where it works automagically because the guard 
that prevents it from working is useless and can be removed as well.

I didn't bother looking at `NewDelete-custom.cpp` because they only manifest 
under the non-default-and-never-planned-to-be-default-anymore option 
`-analyzer-config c++-allocator-inlining=false`. I believe that this flag can 
be removed entirely. I think we've previously been thinking that these are 
false negatives in the new mode that are potentially undesired. Did anybody 
actually end up using it? If a more aggressive behavior towards custom 
allocators is desired, would it make sense to re-implement it directly, without 
relying on side effects of broken operator new modeling (?)


Repository:
  rC Clang

https://reviews.llvm.org/D53543

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/NewDelete-custom.cpp
  test/Analysis/NewDelete-sized-deallocation.cpp

Index: test/Analysis/NewDelete-sized-deallocation.cpp
===
--- /dev/null
+++ test/Analysis/NewDelete-sized-deallocation.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify %s
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -fsized-deallocation %s
+
+void foo() {
+  int *x = new int;
+} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+
+void bar() {
+  int *x = new int;
+  delete x;
+} // no-warning
Index: test/Analysis/NewDelete-custom.cpp
===
--- test/Analysis/NewDelete-custom.cpp
+++ test/Analysis/NewDelete-custom.cpp
@@ -4,9 +4,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
 #include "Inputs/system-header-simulator-cxx.h"
 
-#if !(LEAKS && !ALLOCATOR_INLINING)
 // expected-no-diagnostics
-#endif
 
 
 void *allocator(std::size_t size);
@@ -24,50 +22,37 @@
 void testNewMethod() {
   void *p1 = C::operator new(0); // no warn
 
-  C *p2 = new C; // no warn
+  C *p2 = new C; // no-warning
 
-  C *c3 = ::new C;
+  C *c3 = ::new C; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'c3'}}
-#endif
 
 void testOpNewArray() {
   void *p = operator new[](0); // call is inlined, no warn
 }
 
 void testNewExprArray() {
-  int *p = new int[0];
+  int *p = new int[0]; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
 
 
 //- Custom non-placement operators
 void testOpNew() {
   void *p = operator new(0); // call is inlined, no warn
 }
 
 void testNewExpr() {
-  int *p = new int;
+  int *p = new int; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
-
 
 //- Custom NoThrow placement operators
 void testOpNewNoThrow() {
   void *p = operator new(0, std::nothrow); // call is inlined, no warn
 }
 
 void testNewExprNoThrow() {
-  int *p = new(std::nothrow) int;
+  int *p = new(std::nothrow) int; // no-warning
 }
-#if LEAKS && !ALLOCATOR_INLINING
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
 
 //- Custom placement operators
 void testOpNewPlacement() {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -164,6 +164,7 @@
  check::EndFunction,
  check::PreCall,
   

[PATCH] D53541: [COFF, ARM64] Add aarch64_seh_recoverfp intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.
mgrang added reviewers: rnk, compnerd, mstorsjo, TomTan, efriedma.
Herald added subscribers: chrib, kristof.beyls, javed.absar.

This patch mimics X86 behavior and add aarch64_seh_recoverfp similar to 
x86_seh_recoverfp.
This patch is needed by https://reviews.llvm.org/D53540.


Repository:
  rC Clang

https://reviews.llvm.org/D53541

Files:
  lib/CodeGen/CGException.cpp
  test/CodeGen/exceptions-seh.c


Index: test/CodeGen/exceptions-seh.c
===
--- test/CodeGen/exceptions-seh.c
+++ test/CodeGen/exceptions-seh.c
@@ -99,7 +99,7 @@
 // X64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture 
to i8*), i8* %[[fp]], i32 0)
 //
 // ARM64-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"(i8* 
%exception_pointers, i8* %frame_pointer)
-// ARM64: %[[fp:[^ ]*]] = call i8* @llvm.x86.seh.recoverfp(i8* bitcast (i32 
()* @filter_expr_capture to i8*), i8* %frame_pointer)
+// ARM64: %[[fp:[^ ]*]] = call i8* @llvm.aarch64.seh.recoverfp(i8* bitcast 
(i32 ()* @filter_expr_capture to i8*), i8* %frame_pointer)
 // ARM64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* 
@filter_expr_capture to i8*), i8* %[[fp]], i32 0)
 //
 // X86-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"()
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1777,6 +1777,8 @@
 // frame pointer of the parent function. We only need to do this in 
filters,
 // since finally funclets recover the parent FP for us.
 llvm::Function *RecoverFPIntrin =
+CGM.getTarget().getTriple().getArch() == llvm::Triple::aarch64 ?
+CGM.getIntrinsic(llvm::Intrinsic::aarch64_seh_recoverfp) :
 CGM.getIntrinsic(llvm::Intrinsic::x86_seh_recoverfp);
 llvm::Constant *ParentI8Fn =
 llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);


Index: test/CodeGen/exceptions-seh.c
===
--- test/CodeGen/exceptions-seh.c
+++ test/CodeGen/exceptions-seh.c
@@ -99,7 +99,7 @@
 // X64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %[[fp]], i32 0)
 //
 // ARM64-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"(i8* %exception_pointers, i8* %frame_pointer)
-// ARM64: %[[fp:[^ ]*]] = call i8* @llvm.x86.seh.recoverfp(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %frame_pointer)
+// ARM64: %[[fp:[^ ]*]] = call i8* @llvm.aarch64.seh.recoverfp(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %frame_pointer)
 // ARM64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %[[fp]], i32 0)
 //
 // X86-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"()
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1777,6 +1777,8 @@
 // frame pointer of the parent function. We only need to do this in filters,
 // since finally funclets recover the parent FP for us.
 llvm::Function *RecoverFPIntrin =
+CGM.getTarget().getTriple().getArch() == llvm::Triple::aarch64 ?
+CGM.getIntrinsic(llvm::Intrinsic::aarch64_seh_recoverfp) :
 CGM.getIntrinsic(llvm::Intrinsic::x86_seh_recoverfp);
 llvm::Constant *ParentI8Fn =
 llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r344978 - [X86] Remove 'rtm' feature from KNL.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Oct 22 17:15:37 2018
New Revision: 344978

URL: http://llvm.org/viewvc/llvm-project?rev=344978=rev
Log:
[X86] Remove 'rtm' feature from KNL.

I'm unsure if KNL has this feature, but the backend never thought it did, only 
clang did. The predefined-arch-macros test lost the check for __RTM__ on KNL 
when it was removed Skylake CPUs in r344117.

I think we want to drop it from KNL for consistency with Skylake anyway 
regardless of how we got here.

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=344978=344977=344978=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Oct 22 17:15:37 2018
@@ -280,7 +280,6 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "lzcnt", true);
 setFeatureEnabledImpl(Features, "bmi", true);
 setFeatureEnabledImpl(Features, "bmi2", true);
-setFeatureEnabledImpl(Features, "rtm", true);
 setFeatureEnabledImpl(Features, "fma", true);
 setFeatureEnabledImpl(Features, "rdrnd", true);
 setFeatureEnabledImpl(Features, "f16c", true);


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

Re: r342827 - Fix modules build with shared library.

[Richard Smith via cfe-commits]

On Mon, 22 Oct 2018 at 14:57, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Richard - any further thoughts here (re: layering/dependencies, etc)?
> Would love to get the layering oddity fixed rather than having it get more
> embedded over time.
>

Here's the intended current directory layout and layering as I understand
it:

 * include/clang/Analysis and lib/Analysis are the parts of the static
analysis engine that are depended on by both Sema and StaticAnalyzer, and
include common functionality / infrastructure
 * include/clang/Analysis/Analyses and lib/Analysis/Analyses are the
specific Sema analyses (that don't depend on the static analyzer);
StaticAnalyzer should not need to depend on this
 * the StaticAnalyzer library should contain all the pieces that are
specific to the static analyzer
 * Sema should not depend on Tooling, but it's fine for StaticAnalyzer to
depend on Tooling

Compared to where we are today, there are two differences:

1) The implementations of the headers in include/clang/Analysis are in
lib/Analysis, not in lib/Analysis/Analyses
2) ExprMutationAnalyzer is in lib/Analysis despite not being part of the
common infrastructure depended on by Sema and StaticAnalyzer

For point 1, we should change the lib/Analysis directory layout to match
the headers.
For point 2, we should find a home for this ExprMutationAnalyzer code that
makes sense. Given that the intent is to use it from the static analyzer,
that seems like a reasonable home for it, but libTooling would also make
some sense (perhaps a new subdirectory Tooling/Analysis), since it's only
performing AST matching, not a flow-sensitive / path-sensitive static
analysis.

On Tue, Oct 2, 2018 at 2:44 PM Shuai Wang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Mon, Oct 1, 2018 at 4:58 PM Richard Smith 
>> wrote:
>>
>>> On Mon, 1 Oct 2018 at 16:10, George Karpenkov via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Hi Richard,

 On Oct 1, 2018, at 2:50 PM, Richard Smith via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

 This looks like the wrong fix to me, but I don't really know enough
 about what's being done with ExprMutationAnalyzer to have an opinion on
 what the right fix is.

 Shuai, what is the goal here? Why is this code being moved to Analysis/?


 I’ve asked for this possibility, as I wanted to use it from the Clang
 static analyzer.

 Do you intend to call it from the compiler frontend at some point? I
 can see a code review for the move itself, but no discussion of a plan or
 overall direction being followed here. Did I miss it?

 We have historically decided to not use the tooling interfaces
 (ASTMatcher, ParentMap, etc) from the frontend,


 Clang analyzer uses ASTMatcher all over the place.

 because they're generally a poor fit for our goals (we aim for a
 largely-single-pass compilation with good locality, and the AST matchers
 make different design choices)


 That’s totally good for the analyzer though, right?

>>>
>>> Yes, that all seems fine for the static analyzer.
>>>
>>>
 In any case, in future it might make sense to move the analyzer out of
 Clang proper.
 But for know the only way to use clang-tidy utilities from the analyzer
 is to move them into Clang.

>>>
>>> Right. I think we should try to maintain the idea that all the Clang
>>> Static Analyzer-specific things are in lib/StaticAnalyzer, and lib/Analysis
>>> only contains things depended on by the frontend. (That is: the things a
>>> clang binary would use if we didn't link in the static analyzer)
>>>
>>> Given the above, I think the best approach would be to move this code
>>> out of lib/Analysis and into somewhere in lib/StaticAnalyzer. There
>>> shouldn't be any problem with clang-tidy using it from there, since it
>>> already depends on the static analyzer.
>>>
>> I'm happy to do the move.
>> Could you (or someone) help point out where exactly I should move it to
>> though?
>>
>>> . If you want to change that, we'll need to discuss that decision.

 If the idea is to move this code into clang proper so that it can be
 used by various different tooling clients, we'd need to discuss the right
 place for it. Perhaps lib/Tooling/Analysis would make sense?


 On Mon, 1 Oct 2018 at 13:13, David Blaikie via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> I can't really reproduce this - when I try to build clang/llvm with
> LLVM_ENABLE_MODULES in CMake I'm still seeing an error I reported March on
> a cfe-dev thread - something to do with unique_ptr instantiations for
> MappedBlockStream in the PDB parsing code.
>
> So, I'm wondering what error you hit, Eric/where/how, etc...
>
> On Sun, Sep 30, 2018 at 10:23 AM Eric Fiselier  wrote:
>
>> +rsmith (actually this time)
>>
>> 

[PATCH] D53295: Mark store and load of block invoke function as invariant.group

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Btw, blocks w/o captures are already optimized into regular calls?


https://reviews.llvm.org/D53295



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

[clang-tools-extra] r344968 - [clang-tidy] Fix typo abls->absl.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Oct 22 15:43:17 2018
New Revision: 344968

URL: http://llvm.org/viewvc/llvm-project?rev=344968=rev
Log:
[clang-tidy] Fix typo abls->absl.

Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-str-cat-append.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-str-cat-append.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-str-cat-append.rst?rev=344968=344967=344968=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-str-cat-append.rst 
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-str-cat-append.rst 
Mon Oct 22 15:43:17 2018
@@ -13,5 +13,5 @@ them makes the code smaller and faster.
 
   a = absl::StrCat(a, b); // Use absl::StrAppend(, b) instead.
 
-Does not diagnose cases where ``abls::StrCat()`` is used as a template 
+Does not diagnose cases where ``absl::StrCat()`` is used as a template
 argument for a functor.


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

[PATCH] D51204: [COFF, ARM64] Add MS intrinsics: __getReg, _ReadStatusReg, _WriteStatusReg

[David Major via Phabricator via cfe-commits]

dmajor added a comment.

In https://reviews.llvm.org/D51204#1253230, @mgrang wrote:

> Will abandon this patch since I have implementations of these which I will 
> upstream soon.


Just to link up the reviews: these landed in https://reviews.llvm.org/D52838 
and https://reviews.llvm.org/D53115. (Thanks @mgrang!)


https://reviews.llvm.org/D51204



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

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky updated this revision to Diff 170515.
oleg.smolsky edited the summary of this revision.
oleg.smolsky added a comment.

Added another test case.


Repository:
  rC Clang

https://reviews.llvm.org/D52676

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3277,25 +3277,27 @@
  "});");
   FormatStyle Style = getGoogleStyle();
   Style.ColumnLimit = 45;
-  verifyFormat("Debug(a,\n"
-   "  {\n"
-   "if () return;\n"
-   "  },\n"
-   "  a);",
+  verifyFormat("Debug(\n"
+   "a,\n"
+   "{\n"
+   "  if () return;\n"
+   "},\n"
+   "a);",
Style);
 
   verifyFormat("SomeFunction({MACRO({ return output; }), b});");
 
   verifyNoCrash("^{v^{a}}");
 }
 
 TEST_F(FormatTest, FormatNestedBlocksInMacros) {
-  EXPECT_EQ("#define MACRO() \\\n"
-"  Debug(aaa, /* force line break */ \\\n"
-"{   \\\n"
-"  int i;\\\n"
-"  int j;\\\n"
-"})",
+  EXPECT_EQ("#define MACRO()   \\\n"
+"  Debug(  \\\n"
+"  aaa, /* force line break */ \\\n"
+"  {   \\\n"
+"int i;\\\n"
+"int j;\\\n"
+"  })",
 format("#define   MACRO()   Debug(aaa,  /* force line break */ \\\n"
"  {  int   i;  int  j;   })",
getGoogleStyle()));
@@ -11735,9 +11737,10 @@
"  other(x.begin(), x.end(), [&](int, int) { return 1; });\n"
"}\n");
   verifyFormat("void f() {\n"
-   "  other(x.begin(), //\n"
-   "x.end(),   //\n"
-   "[&](int, int) { return 1; });\n"
+   "  other(\n"
+   "  x.begin(), //\n"
+   "  x.end(),   //\n"
+   "  [&](int, int) { return 1; });\n"
"}\n");
   verifyFormat("SomeFunction([]() { // A cool function...\n"
"  return 43;\n"
@@ -11790,9 +11793,9 @@
   verifyFormat("SomeFunction({[&] {\n"
"  // comment\n"
"}});");
-  verifyFormat("virtual (std::function  =\n"
-   " [&]() { return true; },\n"
-   " a a);");
+  verifyFormat("virtual (\n"
+   "std::function  = [&]() { return true; },\n"
+   "a a);");
 
   // Lambdas with return types.
   verifyFormat("int c = []() -> int { return 2; }();\n");
@@ -11819,17 +11822,78 @@
"  return 1; //\n"
"};");
 
-  // Multiple lambdas in the same parentheses change indentation rules.
+  // Multiple lambdas in the same parentheses change indentation rules. These
+  // lambdas are forced to start on new lines.
   verifyFormat("SomeFunction(\n"
"[]() {\n"
-   "  int i = 42;\n"
-   "  return i;\n"
+   "  //\n"
"},\n"
"[]() {\n"
-   "  int j = 43;\n"
-   "  return j;\n"
+   "  //\n"
"});");
 
+  // A lambda passed as arg0 is always pushed to the next line.
+  verifyFormat("SomeFunction(\n"
+   "[this] {\n"
+   "  //\n"
+   "},\n"
+   "1);\n");
+
+  // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
+  // case above.
+  {
+auto Style = getGoogleStyle();
+Style.BinPackArguments = false;
+verifyFormat("SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ "  //\n"
+ "},\n"
+ "b);\n",
+ Style);
+  }
+  {
+verifyFormat("SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ "  //\n"
+ "},\n"
+ "b);\n");
+  }
+
+  // A lambda with a very long line forces arg0 to be pushed out irrespective of
+  // the BinPackArguments value (as long as the code is wide enough).
+  verifyFormat("something->SomeFunction(\n"
+   "a,\n"
+   "

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky added inline comments.



Comment at: unittests/Format/FormatTest.cpp:11604
+   "  x.end(),   //\n"
+   "  [&](int, int) { return 1; });\n"
"}\n");

krasimir wrote:
> This looks a bit suspicious: I'd expect a break before the first arg to be 
> forced only when there exists a multiline (after formatting) lambda 
> expression arg. Is this (multiline vs. lambdas fitting 1 line) something that 
> we (can) differentiate with respect to? djasper@ might have an insight on 
> this.
Well, yes, I can see where you are coming from - the lambda is short and would 
fit. Unfortunately, I am not sure how to implement this nuance... I think I'd 
need to get the length of the unwrapped line and then check whether it fits in 
TokenUnnotator.cc

Also, I personally favor less indentation (i.e. full width for the lambda) as 
that prevents drastic reformat when the lambda body changes. (that's why this 
patch exists)



Comment at: unittests/Format/FormatTest.cpp:11657
"}});");
-  verifyFormat("virtual (std::function  
=\n"
-   " [&]() { return true; },\n"
-   " a a);");
+  verifyFormat("virtual (\n"
+   "std::function  = [&]() { return true; 
},\n"

krasimir wrote:
> Similar to my previous comment, this forcing the std::function on a newline 
> here might not be what we want to end up with?
I think this change is in line with the updated/extended semantics - the extra 
arg forces the lambda to its own line and the introducer is kept with the 
preceding tokens.



Comment at: unittests/Format/FormatTest.cpp:11736
+  // line and there are no further args.
+  verifyFormat("function(1, [this, that] {\n"
+   "  //\n"

krasimir wrote:
> Could we please have a test case where there are several args packed on the 
> first line, then a line break, then an arg, then a multiline lambda as a last 
> arg (illustrating that we don't pull the first arg down if there's only a 
> multiline lambda as the last arg):
> ```
> function(a, b, ccc,
>  d, [] () {
>   body
> });
> ```
Sure, that seems to work, but not in the way you expected :) I'll update the 
patch...

```
  verifyFormat("function(a, b, c, //\n"
   " d, [this, that] {\n"
   "   //\n"
   " });\n");
```


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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

[PATCH] D52441: [CodeGen] Update min-legal-vector width based on function argument and return types

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D52441#1258317, @craig.topper wrote:

> Address Reid's comments. Add a comment with a list of all things that 
> currently effect the vector width attribute emitted in IR.
>
> For inlining, we update the caller's attribute during merging to ensure it is 
> at least as large as the callee that is being inlined. This is required for 
> always_inline of the intrinsics. We probably want a way to limit inlining, 
> but that would effect the inlining decision. If the decision has been made to 
> inline we have to take the max.
>
> For LTO I don't have an answer. What do we do for things like target features 
> and cpu today?


I think your comments about the behavior w.r.t. inlining are enough to describe 
what happens during LTO. I don't want to speak for Eric, but I think you've 
answered his questions.


https://reviews.llvm.org/D52441



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

[PATCH] D52441: [CodeGen] Update min-legal-vector width based on function argument and return types

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Ping


https://reviews.llvm.org/D52441



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

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

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

Szelethus updated this revision to Diff 170509.
Szelethus added a comment.

Added a new `macro_expansions` key on the same level at `path` and `notes`, 
under which the macro expansions are listed, as suggested by @NoQ.

There were a couple ways to make this happen. I could've changed how macro 
pieces are made in `BugReporter`, so that they are created but the path isn't 
compacted, but then the  `BugReporter`'s implementation would be complicated by 
doing specific things specific output formats. I instead decided to manually 
flatten macro pieces in `PlistDiagnostics`.

Since `FlushDiagnosticsImpl` was already super long, I decided to move both the 
logic of this patch and the logic for `notes` into a separate function.


https://reviews.llvm.org/D52742

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  test/Analysis/plist-macros-with-expansion.cpp

Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- /dev/null
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core %s  \
+// RUN:   -analyzer-output=plist -o %t.plist \
+// RUN:   -analyzer-config expand-macros=true
+//
+// Check the actual plist output.
+//   RUN: cat %t.plist | %diff_plist \
+//   RUN:   %S/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+//
+// Check the macro expansions from the plist output here, to make the test more
+// understandable.
+//   RUN: FileCheck --input-file=%t.plist %s
+
+void print(const void*);
+
+//===--===//
+// Tests for non-function-like macro expansions.
+//===--===//
+
+#define SET_PTR_VAR_TO_NULL \
+  ptr = 0
+
+void nonFunctionLikeMacroTest() {
+  int *ptr;
+  SET_PTR_VAR_TO_NULL;
+  *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: name
+// CHECK-NEXT: expansion
+
+#define NULL 0
+#define SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO \
+  ptr = NULL
+
+void nonFunctionLikeNestedMacroTest() {
+  int *ptr;
+  SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO;
+  *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: name
+// CHECK-NEXT: expansion
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- /dev/null
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -0,0 +1,419 @@
+
+http://www.apple.com/DTDs/PropertyList-1.0.dtd;>
+
+
+ diagnostics
+ 
+  
+   macro_expansions
+   
+
+ location
+ 
+  line26
+  col3
+  file0
+ 
+ name
+ expansion
+
+   
+   path
+   
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line25
+   col3
+   file0
+  
+  
+   line25
+   col5
+   file0
+  
+ 
+end
+ 
+  
+   line26
+   col3
+   file0
+  
+  
+   line26
+   col21
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line26
+  col3
+  file0
+ 
+ ranges
+ 
+   
+
+ line26
+ col3
+ file0
+
+
+ line26
+ col21
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Null pointer value stored to ptr
+ message
+ Null pointer value stored to ptr
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line26
+   col3
+   file0
+  
+  
+   line26
+   col21
+   file0
+  
+ 
+end
+ 
+  
+   line27
+   col3
+   file0
+  
+  
+   line27
+   col3
+   file0
+  
+ 
+   
+  
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line27
+   col3
+   file0
+  
+  
+   line27
+   col3
+   file0
+  
+ 
+end
+ 
+  
+   line27
+   col8
+   file0
+  
+  
+   line27
+   col8
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line27
+  col8
+  file0
+ 
+ ranges
+ 
+   
+
+ 

Re: r344915 - Ensure sanitizer check function calls have a !dbg location

[Vlad Tsyrklevich via cfe-commits]

This change causes build failures on the UBSan bot, like so

:

3.  
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/include/llvm/Support/Allocator.h:98:40:
Generating code for declaration 'llvm::MallocAllocator::Allocate'

#0 0x55c1695ee77a llvm::sys::PrintStackTrace(llvm::raw_ostream&)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x265677a)
#1 0x55c1695ecc55 llvm::sys::RunSignalHandlers()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2654c55)
#2 0x55c1695ecd6c SignalHandler(int)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2654d6c)
#3 0x7f1e75f880c0 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0)
#4 0x55c169018c69 llvm::DebugLoc::get(unsigned int, unsigned int,
llvm::MDNode const*, llvm::MDNode const*, bool)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2080c69)
#5 0x55c1697ddb31
clang::CodeGen::CGDebugInfo::EmitLocation(clang::CodeGen::CGBuilderTy&,
clang::SourceLocation)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2845b31)
#6 0x55c1697ddcd6
clang::CodeGen::ApplyDebugLocation::init(clang::SourceLocation, bool)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2845cd6)
#7 0x55c1699defa6
emitCheckHandlerCall(clang::CodeGen::CodeGenFunction&,
llvm::FunctionType*, llvm::ArrayRef,
clang::CodeGen::SanitizerHandler, (anonymous
namespace)::CheckRecoverableKind, bool, llvm::BasicBlock*)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2a46fa6)
#8 0x55c1699e155a
clang::CodeGen::CodeGenFunction::EmitCheck(llvm::ArrayRef >, clang::CodeGen::SanitizerHandler,
llvm::ArrayRef, llvm::ArrayRef)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2a4955a)
#9 0x55c169991463
clang::CodeGen::CodeGenFunction::EmitReturnValueCheck(llvm::Value*)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x29f9463)
#10 0x55c16999f536
clang::CodeGen::CodeGenFunction::EmitFunctionEpilog(clang::CodeGen::CGFunctionInfo
const&, bool, clang::SourceLocation)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2a07536)
#11 0x55c16985070a
clang::CodeGen::CodeGenFunction::FinishFunction(clang::SourceLocation)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x28b870a)
#12 0x55c16985959f
clang::CodeGen::CodeGenFunction::GenerateCode(clang::GlobalDecl,
llvm::Function*, clang::CodeGen::CGFunctionInfo const&)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x28c159f)
#13 0x55c169895ac5
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl,
llvm::GlobalValue*)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x28fdac5)
#14 0x55c169892c39
clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl,
llvm::GlobalValue*)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x28fac39)
#15 0x55c169898992 clang::CodeGen::CodeGenModule::EmitDeferred()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2900992)
#16 0x55c1698989ac clang::CodeGen::CodeGenModule::EmitDeferred()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x29009ac)
#17 0x55c1698989ac clang::CodeGen::CodeGenModule::EmitDeferred()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x29009ac)
#18 0x55c1698989ac clang::CodeGen::CodeGenModule::EmitDeferred()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x29009ac)
#19 0x55c1698989ac clang::CodeGen::CodeGenModule::EmitDeferred()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x29009ac)
#20 0x55c169898af3 clang::CodeGen::CodeGenModule::Release()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2900af3)
#21 0x55c169fae527 (anonymous
namespace)::CodeGeneratorImpl::HandleTranslationUnit(clang::ASTContext&)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x3016527)
#22 0x55c169fad156
clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x3015156)
#23 0x55c16a7b9569 clang::ParseAST(clang::Sema&, bool, bool)
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x3821569)
#24 0x55c169fac339 clang::CodeGenAction::ExecuteAction()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x3014339)
#25 0x55c169c2d25e clang::FrontendAction::Execute()
(/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/bin/clang-8+0x2c9525e)
#26 0x55c169bf7b0e
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)

[PATCH] D53475: Create ConstantExpr class

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

rsmith added a comment.

Thanks, this is looking good.




Comment at: include/clang/AST/Expr.h:874-875
+
+/// FullExpression - Represents a "full-expression" node.
+class FullExpression : public Expr {
+protected:

All of our expression classes have names ending `Expr`; I don't think we should 
deviate from that convention here.



Comment at: include/clang/AST/Expr.h:886-889
+  virtual const Expr *getSubExpr() const = 0;
+  virtual Expr *getSubExpr() = 0;
+
+  virtual unsigned getNumObjects() const { return 0; }

These should not be `virtual`; this class hierarchy uses LLVM-style RTTI rather 
than vtables. If you want to provide these convenience methods on `FullExpr`, 
you can manually dispatch on the dynamic type using `dyn_cast` or similar.



Comment at: include/clang/AST/Expr.h:924-925
+  // Iterators
+  child_range children() { return Val->children(); }
+  const_child_range children() const { return Val->children(); }
+};

The children range for a `ConstantExpr` should comprise `Val` itself, not 
`Val`'s children.



Comment at: include/clang/Basic/StmtNodes.td:97
+// Wrapper expressions
+def ConstantExpr : DStmt;
+

Please add a `def FullExpr : DStmt` and change this and 
`ExprWithCleanups` to be `: DStmt` so that our visitors will be able 
to visit the `FullExpr` base class.



Comment at: lib/AST/Expr.cpp:3100-3101
+  case ConstantExprClass:
+return cast(this)->getSubExpr()->HasSideEffects(
+Ctx, IncludePossibleEffects);
+

Please add a FIXME here to move this into the `return false;` block. (Keeping 
this functionally identical to an `ExprWithCleanups` with no cleanups for now 
seems fine.)


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53066: [RFC] [Driver] Use forward slashes in most linker arguments

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


https://reviews.llvm.org/D53066



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

r344963 - Revert "[Driver] Reland again: Default Android toolchains to libc++."

[Dan Albert via cfe-commits]

Author: danalbert
Date: Mon Oct 22 14:58:22 2018
New Revision: 344963

URL: http://llvm.org/viewvc/llvm-project?rev=344963=rev
Log:
Revert "[Driver] Reland again: Default Android toolchains to libc++."

More compiler-rt test bot breakages...

Removed:

cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c++/v1/.keep
Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.h
cfe/trunk/test/Driver/android-ndk-standalone.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=344963=344962=344963=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Mon Oct 22 14:58:22 2018
@@ -443,12 +443,6 @@ Linux::Linux(const Driver , const llvm
   addPathIfExists(D, SysRoot + "/usr/lib", Paths);
 }
 
-ToolChain::CXXStdlibType Linux::GetDefaultCXXStdlibType() const {
-  if (getTriple().isAndroid())
-return ToolChain::CST_Libcxx;
-  return ToolChain::CST_Libstdcxx;
-}
-
 bool Linux::HasNativeLLVMSupport() const { return true; }
 
 Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); }

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.h?rev=344963=344962=344963=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.h Mon Oct 22 14:58:22 2018
@@ -37,7 +37,6 @@ public:
   llvm::opt::ArgStringList ) const override;
   void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  CXXStdlibType GetDefaultCXXStdlibType() const override;
   bool isPIEDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;

Removed: 
cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c++/v1/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c%2B%2B/v1/.keep?rev=344962=auto
==
(empty)

Modified: cfe/trunk/test/Driver/android-ndk-standalone.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-ndk-standalone.cpp?rev=344963=344962=344963=diff
==
--- cfe/trunk/test/Driver/android-ndk-standalone.cpp (original)
+++ cfe/trunk/test/Driver/android-ndk-standalone.cpp Mon Oct 22 14:58:22 2018
@@ -2,13 +2,21 @@
 // toolchain.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi21 \
+// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
 // RUN: -B%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN:   | FileCheck  %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/v1"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
+// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/backward"
 // CHECK: "-internal-isystem" "{{.*}}/sysroot/usr/local/include"
 // CHECK: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
 // CHECK: "-internal-externc-isystem" 
"{{.*}}/sysroot/usr/include/arm-linux-androideabi"
@@ -41,47 +49,21 @@
 // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
-// RUN: -B%S/Inputs/basic_android_ndk_tree \
-// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
-// RUN:   | FileCheck --check-prefix=CHECK-STDCXX %s
-// CHECK-STDCXX: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-STDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-STDCXX: "-internal-isystem" "{{.*}}/include/c++/4.9"
-// CHECK-STDCXX-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// CHECK-STDCXX-NOT: 

Re: r342827 - Fix modules build with shared library.

[David Blaikie via cfe-commits]

Richard - any further thoughts here (re: layering/dependencies, etc)? Would
love to get the layering oddity fixed rather than having it get more
embedded over time.

On Tue, Oct 2, 2018 at 2:44 PM Shuai Wang via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Oct 1, 2018 at 4:58 PM Richard Smith 
> wrote:
>
>> On Mon, 1 Oct 2018 at 16:10, George Karpenkov via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Hi Richard,
>>>
>>> On Oct 1, 2018, at 2:50 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
>>> This looks like the wrong fix to me, but I don't really know enough
>>> about what's being done with ExprMutationAnalyzer to have an opinion on
>>> what the right fix is.
>>>
>>> Shuai, what is the goal here? Why is this code being moved to Analysis/?
>>>
>>>
>>> I’ve asked for this possibility, as I wanted to use it from the Clang
>>> static analyzer.
>>>
>>> Do you intend to call it from the compiler frontend at some point? I can
>>> see a code review for the move itself, but no discussion of a plan or
>>> overall direction being followed here. Did I miss it?
>>>
>>> We have historically decided to not use the tooling interfaces
>>> (ASTMatcher, ParentMap, etc) from the frontend,
>>>
>>>
>>> Clang analyzer uses ASTMatcher all over the place.
>>>
>>> because they're generally a poor fit for our goals (we aim for a
>>> largely-single-pass compilation with good locality, and the AST matchers
>>> make different design choices)
>>>
>>>
>>> That’s totally good for the analyzer though, right?
>>>
>>
>> Yes, that all seems fine for the static analyzer.
>>
>>
>>> In any case, in future it might make sense to move the analyzer out of
>>> Clang proper.
>>> But for know the only way to use clang-tidy utilities from the analyzer
>>> is to move them into Clang.
>>>
>>
>> Right. I think we should try to maintain the idea that all the Clang
>> Static Analyzer-specific things are in lib/StaticAnalyzer, and lib/Analysis
>> only contains things depended on by the frontend. (That is: the things a
>> clang binary would use if we didn't link in the static analyzer)
>>
>> Given the above, I think the best approach would be to move this code out
>> of lib/Analysis and into somewhere in lib/StaticAnalyzer. There shouldn't
>> be any problem with clang-tidy using it from there, since it already
>> depends on the static analyzer.
>>
> I'm happy to do the move.
> Could you (or someone) help point out where exactly I should move it to
> though?
>
>> . If you want to change that, we'll need to discuss that decision.
>>>
>>> If the idea is to move this code into clang proper so that it can be
>>> used by various different tooling clients, we'd need to discuss the right
>>> place for it. Perhaps lib/Tooling/Analysis would make sense?
>>>
>>>
>>> On Mon, 1 Oct 2018 at 13:13, David Blaikie via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 I can't really reproduce this - when I try to build clang/llvm with
 LLVM_ENABLE_MODULES in CMake I'm still seeing an error I reported March on
 a cfe-dev thread - something to do with unique_ptr instantiations for
 MappedBlockStream in the PDB parsing code.

 So, I'm wondering what error you hit, Eric/where/how, etc...

 On Sun, Sep 30, 2018 at 10:23 AM Eric Fiselier  wrote:

> +rsmith (actually this time)
>
> On Sun, Sep 30, 2018 at 12:09 PM Eric Fiselier  wrote:
>
>> +rsmith
>>
>> Hi All,
>>
>> Sorry, I'm not actually sure why this fix is correct.I stole both the
>> fix and the comment from a similar one on L150 of the module map left by
>> Richard Smith.
>>
>> /Eric
>>
>> On Tue, Sep 25, 2018 at 8:36 PM Shuai Wang 
>> wrote:
>>
>>> I'd like to understand this better as well, in particular what would
>>> be a proper fix?
>>>
>>> On Tue, Sep 25, 2018 at 2:15 PM David Blaikie 
>>> wrote:
>>>
 +Shuai Wang

 On Tue, Sep 25, 2018 at 2:14 PM David Blaikie 
 wrote:

> Hey Eric - thanks for the fix - but could you explain the issue
> here in a bit more detail, as I'm a bit confused (& really interested 
> in
> understanding any layering problems in LLVM - and fixing them/making 
> sure
> they're fixed/holding the line/etc)
>
> What do you mean by "pull all of the AST matchers library into
> clang" - how does including a header ever add a link dependency?
>
> - Dave
>
>
> On Sat, Sep 22, 2018 at 5:49 PM Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ericwf
>> Date: Sat Sep 22 17:48:05 2018
>> New Revision: 342827
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=342827=rev
>> Log:
>> Fix modules build with shared library.
>>
>> r341994 caused 

r344961 - Revert "Ensure sanitizer check function calls have a !dbg location"

[Vlad Tsyrklevich via cfe-commits]

Author: vlad.tsyrklevich
Date: Mon Oct 22 14:51:58 2018
New Revision: 344961

URL: http://llvm.org/viewvc/llvm-project?rev=344961=rev
Log:
Revert "Ensure sanitizer check function calls have a !dbg location"

This reverts commit r344915. It was causing exceptions on the
x86_64-linux-ubsan bot.

Removed:
cfe/trunk/test/CodeGenCXX/ubsan-check-debuglocs.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=344961=344960=344961=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Oct 22 14:51:58 2018
@@ -2867,9 +2867,6 @@ static void emitCheckHandlerCall(CodeGen
  CheckRecoverableKind RecoverKind, bool 
IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
-  auto *DI = CGF.getDebugInfo();
-  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
-  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;

Removed: cfe/trunk/test/CodeGenCXX/ubsan-check-debuglocs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-check-debuglocs.cpp?rev=344960=auto
==
--- cfe/trunk/test/CodeGenCXX/ubsan-check-debuglocs.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/ubsan-check-debuglocs.cpp (removed)
@@ -1,17 +0,0 @@
-// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
-// RUN:   -fsanitize=null %s -o - | FileCheck %s
-
-// Check that santizer check calls have a !dbg location.
-// CHECK: define {{.*}}acquire{{.*}} !dbg
-// CHECK-NOT: define
-// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1
-// CHECK-SAME: !dbg
-
-struct SourceLocation {
-  SourceLocation acquire() {};
-};
-extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
-static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
-void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
-  handleTypeMismatchImpl(Loc);
-}


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

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: unittests/Format/FormatTest.cpp:11604
+   "  x.end(),   //\n"
+   "  [&](int, int) { return 1; });\n"
"}\n");

This looks a bit suspicious: I'd expect a break before the first arg to be 
forced only when there exists a multiline (after formatting) lambda expression 
arg. Is this (multiline vs. lambdas fitting 1 line) something that we (can) 
differentiate with respect to? djasper@ might have an insight on this.



Comment at: unittests/Format/FormatTest.cpp:11657
"}});");
-  verifyFormat("virtual (std::function  
=\n"
-   " [&]() { return true; },\n"
-   " a a);");
+  verifyFormat("virtual (\n"
+   "std::function  = [&]() { return true; 
},\n"

Similar to my previous comment, this forcing the std::function on a newline 
here might not be what we want to end up with?



Comment at: unittests/Format/FormatTest.cpp:11736
+  // line and there are no further args.
+  verifyFormat("function(1, [this, that] {\n"
+   "  //\n"

Could we please have a test case where there are several args packed on the 
first line, then a line break, then an arg, then a multiline lambda as a last 
arg (illustrating that we don't pull the first arg down if there's only a 
multiline lambda as the last arg):
```
function(a, b, ccc,
 d, [] () {
  body
});
```


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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

[PATCH] D53529: [Driver] fix broken test

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344958: [Driver] fix broken test (authored by 
nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53529

Files:
  cfe/trunk/test/Driver/android-gcc-toolchain.c


Index: cfe/trunk/test/Driver/android-gcc-toolchain.c
===
--- cfe/trunk/test/Driver/android-gcc-toolchain.c
+++ cfe/trunk/test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9


Index: cfe/trunk/test/Driver/android-gcc-toolchain.c
===
--- cfe/trunk/test/Driver/android-gcc-toolchain.c
+++ cfe/trunk/test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r344958 - [Driver] fix broken test

[Nick Desaulniers via cfe-commits]

Author: nickdesaulniers
Date: Mon Oct 22 14:25:53 2018
New Revision: 344958

URL: http://llvm.org/viewvc/llvm-project?rev=344958=rev
Log:
[Driver] fix broken test

Summary:
Fixes test from r344941 which was broken on Windows. We want to check
the selected toolchain rather than the found toolchain anyways.

Reviewers: srhines, danalbert

Reviewed By: srhines

Subscribers: cfe-commits, bogner, pirama

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

Modified:
cfe/trunk/test/Driver/android-gcc-toolchain.c

Modified: cfe/trunk/test/Driver/android-gcc-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-gcc-toolchain.c?rev=344958=344957=344958=diff
==
--- cfe/trunk/test/Driver/android-gcc-toolchain.c (original)
+++ cfe/trunk/test/Driver/android-gcc-toolchain.c Mon Oct 22 14:25:53 2018
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9


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

r344957 - Give Multiversion-inline functions linkonce linkage

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Oct 22 14:20:45 2018
New Revision: 344957

URL: http://llvm.org/viewvc/llvm-project?rev=344957=rev
Log:
Give Multiversion-inline functions linkonce linkage

Since multiversion variant functions can be inline, in C they become
available-externally linkage.  This ends up causing the variants to not
be emitted, and not available to the linker.

The solution is to make sure that multiversion functions are always
emitted by marking them linkonce.

Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGen/attr-target-mv.c

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957=344956=344957=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018
@@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM
   return llvm::GlobalVariable::WeakAnyLinkage;
   }
 
+  if (const auto *FD = D->getAsFunction())
+if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
+  return llvm::GlobalVariable::LinkOnceAnyLinkage;
+
   // We are guaranteed to have a strong definition somewhere else,
   // so we can use available_externally linkage.
   if (Linkage == GVA_AvailableExternally)

Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957=344956=344957=diff
==
--- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
+++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018
@@ -88,19 +88,19 @@ void bar4() {
 
 // CHECK: declare i32 @foo.arch_sandybridge()
 
-// CHECK: define available_externally i32 @foo_inline.sse4.2()
+// CHECK: define linkonce i32 @foo_inline.sse4.2()
 // CHECK: ret i32 0
 
 // CHECK: declare i32 @foo_inline.arch_sandybridge()
 //
-// CHECK: define available_externally i32 @foo_inline.arch_ivybridge()
+// CHECK: define linkonce i32 @foo_inline.arch_ivybridge()
 // CHECK: ret i32 1
-// CHECK: define available_externally i32 @foo_inline()
+// CHECK: define linkonce i32 @foo_inline()
 // CHECK: ret i32 2
 
-// CHECK: define available_externally void @foo_decls()
-// CHECK: define available_externally void @foo_decls.sse4.2()
+// CHECK: define linkonce void @foo_decls()
+// CHECK: define linkonce void @foo_decls.sse4.2()
 
-// CHECK: define available_externally void @foo_multi.avx_sse4.2()
-// CHECK: define available_externally void @foo_multi.fma4_sse4.2()
-// CHECK: define available_externally void 
@foo_multi.arch_ivybridge_fma4_sse4.2()
+// CHECK: define linkonce void @foo_multi.avx_sse4.2()
+// CHECK: define linkonce void @foo_multi.fma4_sse4.2()
+// CHECK: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2()


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

[PATCH] D53529: [Driver] fix broken test

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, danalbert.
Herald added a subscriber: cfe-commits.

Fixes test from r344941 which was broken on Windows. We want to check
the selected toolchain rather than the found toolchain anyways.


Repository:
  rC Clang

https://reviews.llvm.org/D53529

Files:
  test/Driver/android-gcc-toolchain.c


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53482: Add clang-format stability check with FormatTests

[Krasimir Georgiev via Phabricator via cfe-commits]

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

Sadly I tried this out a year ago and hit the same thing. A root cause is that 
tabs have variable column length depending on their start column, and in 
clang-format tokens are modeled as entities having a fixed column width. There 
are FIXMEs about this in the codebase 
(https://github.com/llvm-mirror/clang/blob/41e165b659ec7d8262153a6e0ed9130431c886de/lib/Format/FormatTokenLexer.cpp#L641).
A patch that adds stability to tests would have to address the underlying root 
cause which requires quite a lot of stuff to be threaded over clang-format and 
for practical purposes folks using clang-format are not complaining about this 
enough to be worth spending the effort and added complexity fixing this. I 
know, it sucks, but that's the situation right now.


Repository:
  rC Clang

https://reviews.llvm.org/D53482



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

[PATCH] D53520: Update the example of BS_Stroustrup to match what is done by clang-format

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir requested changes to this revision.
krasimir added inline comments.
This revision now requires changes to proceed.



Comment at: docs/ClangFormatStyleOptions.rst:904
   * ``BS_Stroustrup`` (in configuration: ``Stroustrup``)
-Like ``Attach``, but break before function definitions, ``catch``, and
-``else``.
+Always break before braces.
 

Looking into this, I think your update of the example below is correct, but the 
description should stay as-is: 
```Like ``Attach``, but break before function definitions,...```
The `Always break before braces` should still apply to the `BS_Allman` option 
below.
Am I missing something here?


Repository:
  rC Clang

https://reviews.llvm.org/D53520



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

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> Also, the template version (whether trailing or normal return) does not 
> trigger the matchers at all, from which I surmise that clang doesn't consider 
> the type const qualified when it is not materialized with an actual type. 
> I've noted as much in the comments, but please correct me if I've 
> misunderstood.

If the template is not instantiated no information on `const` exists and
that might vary between instantiations anyway (if you don't explicitly
write `const T`). It would be an interesting test to try and break the
check with an explicit `const` template and instantiaions with `const`.
(e.g. `template  const T my_function();` and an
instantiation like `gsl::span`)

> Finally, as for triggering: I ran this over Google's C++ codebase and found 
> quite a few hits (10k < X < 100k;  not sure I'm allowed to specify a more 
> precise value for X, but if you need it,  let me know and I'll ask).  I 
> sampled 100 of them and confirmed that all were correct.
> 
> I also looked at LLVM and found ~130 hits. I sampled 10 and again found them 
> all correct. I'm happy to post all of the llvm hits if you think that would 
> be helpful.

Did you try the transformation and is it correct (does not break code)?
Enough to check on LLVM :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025



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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D53524#1271357, @pcc wrote:

> The reason why LTO unit is always enabled is so that you can link translation 
> units compiled with `-fsanitize=cfi` and/or `-fwhole-program-vtables` against 
> translation units compiled without CFI/WPD. With this change we will see 
> miscompiles in the translation units compiled with CFI/WPD if they use 
> vtables in the translation units compiled without CFI/WPD. If we really need 
> this option I think it should be an opt out.


Is there an important use case for support thing mixing and matching? The issue 
is that it comes at a cost to all ThinLTO compiles for codes with vtables by 
requiring them all to process IR during the thin link. Can we detect that TUs 
compiled with -flto-unit are being mixed with those not built without 
-flto-unit at the thin link time and issue an error?


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53476: [clang-cl] Allowed -fopenmp work and link openmp library from per-runtime library directory

[Reid Kleckner via Phabricator via cfe-commits]

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

Actually, can you please add -fopenmp to `test/Driver/cl-options.c` to verify 
that we accept `-fopenmp`?


Repository:
  rC Clang

https://reviews.llvm.org/D53476



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

[PATCH] D53476: [clang-cl] Allowed -fopenmp work and link openmp library from per-runtime library directory

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D53476



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

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D53025#1270784, @ymandel wrote:

> Correction: the code *catches* the trailing return cases, I just couldn't 
> find a way to *fix* them; specifically, I was not able to get the necessary 
> source ranges to re-lex the trailing return type.


There is `fuchsia/TrailingReturnCheck.cpp` which you look if there is 
something, I am currently not aware of other checks that handle trailing return 
types in any way.
In principle it is ok to leave these for future improvements, but should be 
noted as limitation in the docs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025



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

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:51
+  hasArgument(0,
+  anyOf(cxxStaticCastExpr(
+hasDestinationType(realFloatingPointType()),

What about c-style casts?



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:67
+  const Expr *Arg = MatchedCall->getArg(0)->IgnoreImpCasts();
+  // Macros are ignored.
+  if (Arg->getBeginLoc().isMacroID())

Please clarfiy this comment a bit more, like `Marcros as arguments are 
ignored.` or the like.



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:86
+  Result.Nodes.getNodeAs("float_literal")) {
+// Attempt to simplify a Duration factory call with a literal argument.
+if (llvm::Optional IntValue = truncateIfIntegral(*LitFloat)) 
{

please highlight the code construct ('Duration' in this case) here and in other 
comments to clarify its about the class in user-code.



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.h:19
+
+/// Prefer integer Duration factories when possible.
+///

Please add more to the doc here, like `This check finds ... and transforms 
these calls into ...` or similar.



Comment at: docs/clang-tidy/checks/abseil-duration-factory-float.rst:6
+
+Finds cases where callers of ``absl::Duration`` factory functions (such as
+``absl::Seconds`` or ``absl::Hours``) are providing a floating point value

Please synchronize the first paragraph with the release notes (I would prefer 
the release notes version)


https://reviews.llvm.org/D53339



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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Peter Collingbourne via Phabricator via cfe-commits]

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

The reason why LTO unit is always enabled is so that you can link translation 
units compiled with `-fsanitize=cfi` and/or `-fwhole-program-vtables` against 
translation units compiled without CFI/WPD. With this change we will see 
miscompiles in the translation units compiled with CFI/WPD if they use vtables 
in the translation units compiled without CFI/WPD. If we really need this 
option I think it should be an opt out.


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53527: Fix range length comparison in DraftStore::UpdateDraft when Unicode characters are removed from the document

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer created this revision.
DaanDeMeyer added a reviewer: sammccall.
DaanDeMeyer added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, 
ilya-biryukov.

See http://lists.llvm.org/pipermail/clangd-dev/2018-October/000171.html for 
context.

I kept the error (instead of downgrading to a log message) since the range 
lengths differing does indicate either a bug in the client or server range 
calculation or the buffers being out of sync (which both seems serious enough 
to me to be an error). If any existing clients aside from VSCode break they 
should only break when accidentally typing a Unicode character which should 
only be a minor nuisance for a little while until the bug is fixed in the 
respective client.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53527

Files:
  clangd/DraftStore.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h


Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -67,13 +67,14 @@
   return std::min(Result, U8.size());
 }
 
-// Counts the number of UTF-16 code units needed to represent a string.
-// Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units). Like most strings in clangd,
+// the input is UTF-8 encoded.
+size_t lspLength(StringRef Code) {
   // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
   // Astral codepoints are encoded as 4 bytes in UTF-8, starting with 0xxx.
   size_t Count = 0;
-  iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+  iterateCodepoints(Code, [&](int U8Len, int U16Len) {
 Count += U16Len;
 return false;
   });
@@ -123,7 +124,7 @@
   size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
   Position Pos;
   Pos.line = Lines;
-  Pos.character = utf16Len(Before.substr(StartOfLine));
+  Pos.character = lspLength(Before.substr(StartOfLine));
   return Pos;
 }
 
@@ -139,7 +140,7 @@
   if (!Invalid) {
 auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
 auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
-P.character = utf16Len(LineSoFar);
+P.character = lspLength(LineSoFar);
   }
   return P;
 }
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -23,6 +23,9 @@
 
 namespace clangd {
 
+/// Calculate the length of Code according to the LSP specification.
+size_t lspLength(StringRef Code);
+
 /// Turn a [line, column] pair into an offset in Code.
 ///
 /// If P.character exceeds the line length, returns the offset at end-of-line.
Index: clangd/DraftStore.cpp
===
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -77,8 +77,21 @@
   End, Start),
   llvm::errc::invalid_argument);
 
-if (Change.rangeLength &&
-(ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)
+// Since the range length between two LSP positions is dependent on the
+// contents of the buffer we compute the range length between the start and
+// end position ourselves and compare it to the range length of the LSP
+// message to verify the buffers of the client and server are in sync.
+
+// The difference between EndIndex and StartIndex gives the range length in
+// bytes. However, the LSP range length is specified in UTF-16 code units
+// which means directly comparing them will give faulty results. To solve
+// this we convert the computed range length in bytes back to UTF-16 code
+// units by counting the amount of UTF-16 code units in the [StartIndex,
+// EndIndex) substring of the document buffer.
+ssize_t ComputedRangeLength =
+lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex));
+
+if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength)
   return make_error(
   formatv("Change's rangeLength ({0}) doesn't match the "
   "computed range length ({1}).",


Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -67,13 +67,14 @@
   return std::min(Result, U8.size());
 }
 
-// Counts the number of UTF-16 code units needed to represent a string.
-// Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units). Like most strings in clangd,
+// the input is UTF-8 encoded.
+size_t lspLength(StringRef Code) {
   // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
   // Astral 

[PATCH] D53483: [analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

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

Szelethus added a comment.

> (though I would also prefer if even checkers could pre-register their options 
> somehow)

Good news, I've also successfully modified the tblgen file `Checkers.td` to be 
able to register checker options (it took wy more effort and I thought it'd 
take), so a list of non-plugin checker options should only be a patch away. 
Still struggling with the plugin part, but I'm making progress.




Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:157
+ .getAsInteger(10, Ret);
+  assert(!HasFailed && "analyzer-config option should be numeric");
+  (void)HasFailed;

xazax.hun wrote:
> Can this assert be triggered using a bad invocation of the analyzer? I wonder 
> if it is a good idea to use asserts to validate user input. Maybe it would be 
> better to generate a warning and return the default value?
This is the current state of things, but I intend to change it later in a 
followup patch. I am not even sure what the thinking was behind this solution 
myself.


Repository:
  rC Clang

https://reviews.llvm.org/D53483



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

[clang-tools-extra] r344947 - [clang-tidy] tryfix windows build

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Mon Oct 22 13:29:15 2018
New Revision: 344947

URL: http://llvm.org/viewvc/llvm-project?rev=344947=rev
Log:
[clang-tidy] tryfix windows build

Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp?rev=344947=344946=344947=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp 
Mon Oct 22 13:29:15 2018
@@ -13,6 +13,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Regex.h"
 #include 
+#include 
 
 namespace clang {
 namespace tidy {


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

r344946 - [Driver] Reland again: Default Android toolchains to libc++.

[Dan Albert via cfe-commits]

Author: danalbert
Date: Mon Oct 22 13:16:21 2018
New Revision: 344946

URL: http://llvm.org/viewvc/llvm-project?rev=344946=rev
Log:
[Driver] Reland again: Default Android toolchains to libc++.

Some of the test data went missing last time I tried to submit this,
causing the tests to fail when the build did not include libc++.

Original review was https://reviews.llvm.org/D53109.

Added:
cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c++/

cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c++/v1/

cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c++/v1/.keep
Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.h
cfe/trunk/test/Driver/android-ndk-standalone.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=344946=344945=344946=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Mon Oct 22 13:16:21 2018
@@ -443,6 +443,12 @@ Linux::Linux(const Driver , const llvm
   addPathIfExists(D, SysRoot + "/usr/lib", Paths);
 }
 
+ToolChain::CXXStdlibType Linux::GetDefaultCXXStdlibType() const {
+  if (getTriple().isAndroid())
+return ToolChain::CST_Libcxx;
+  return ToolChain::CST_Libstdcxx;
+}
+
 bool Linux::HasNativeLLVMSupport() const { return true; }
 
 Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); }

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.h?rev=344946=344945=344946=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.h Mon Oct 22 13:16:21 2018
@@ -37,6 +37,7 @@ public:
   llvm::opt::ArgStringList ) const override;
   void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
   bool isPIEDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;

Added: 
cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c++/v1/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/basic_android_ndk_tree/sysroot/usr/include/c%2B%2B/v1/.keep?rev=344946=auto
==
(empty)

Modified: cfe/trunk/test/Driver/android-ndk-standalone.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-ndk-standalone.cpp?rev=344946=344945=344946=diff
==
--- cfe/trunk/test/Driver/android-ndk-standalone.cpp (original)
+++ cfe/trunk/test/Driver/android-ndk-standalone.cpp Mon Oct 22 13:16:21 2018
@@ -2,21 +2,13 @@
 // toolchain.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -target arm-linux-androideabi21 \
 // RUN: -B%S/Inputs/basic_android_ndk_tree \
 // RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
 // RUN:   | FileCheck  %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/arm-linux-androideabi"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a/thumb"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/armv7-a"
-// CHECK-NOT: "-internal-isystem" 
"{{.*}}/include/c++/4.9/arm-linux-androideabi/thumb"
-// CHECK: "-internal-isystem" "{{.*}}/include/c++/4.9/backward"
+// CHECK: "-internal-isystem" "{{.*}}/include/c++/v1"
 // CHECK: "-internal-isystem" "{{.*}}/sysroot/usr/local/include"
 // CHECK: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
 // CHECK: "-internal-externc-isystem" 
"{{.*}}/sysroot/usr/include/arm-linux-androideabi"
@@ -49,21 +41,47 @@
 // CHECK-14: "-L{{.*}}/sysroot/usr/lib/arm-linux-androideabi"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target armv7a-none-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: 

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:58
+   const QualType Rhs) {
+  assert(Lhs->isRealType()); // Either integer or floating point.
+  assert(Rhs->isFloatingType()); // Floating point only.

Couldn't be the conversion from an `int` to an `enum` be considered narrowing 
as well? (Not sure about the word of the standard) I think it makes sense to 
change the `assert` to `return false`



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:82
+  << RhsType << LhsType;
   }
 }

could you please add a final `llvm_unreachable()` to ensure that all 
possibilites are properly handled?
If that would not fit I would prefer ealry returns instead to show "the end" 
for each logical path


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

r344944 - [analyzer] Fix dumping for SymbolConjured conjured at no particular statement.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Oct 22 13:11:10 2018
New Revision: 344944

URL: http://llvm.org/viewvc/llvm-project?rev=344944=rev
Log:
[analyzer] Fix dumping for SymbolConjured conjured at no particular statement.

Added:
cfe/trunk/test/Analysis/dump_egraph.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=344944=344943=344944=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Mon Oct 22 13:11:10 2018
@@ -83,10 +83,13 @@ void SymbolCast::dumpToStream(raw_ostrea
 }
 
 void SymbolConjured::dumpToStream(raw_ostream ) const {
-  os << "conj_$" << getSymbolID() << '{' << T.getAsString()
-<< ", LC" << LCtx->getID() << ", S" << S->getID(
-  LCtx->getDecl()->getASTContext()) << ", #" << Count
-<< '}';
+  os << "conj_$" << getSymbolID() << '{' << T.getAsString() << ", LC"
+ << LCtx->getID();
+  if (S)
+os << ", S" << S->getID(LCtx->getDecl()->getASTContext());
+  else
+os << ", no stmt";
+  os << ", #" << Count << '}';
 }
 
 void SymbolDerived::dumpToStream(raw_ostream ) const {

Added: cfe/trunk/test/Analysis/dump_egraph.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dump_egraph.cpp?rev=344944=auto
==
--- cfe/trunk/test/Analysis/dump_egraph.cpp (added)
+++ cfe/trunk/test/Analysis/dump_egraph.cpp Mon Oct 22 13:11:10 2018
@@ -0,0 +1,15 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
%s
+// RUN: cat %t.dot | FileCheck %s
+// REQUIRES: asserts
+
+
+struct S {
+  ~S();
+};
+
+void foo() {
+  // Test that dumping symbols conjured on null statements doesn't crash.
+  S s;
+}
+
+// CHECK: conj_$0\{int, LC1, no stmt, #1\}


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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added a reviewer: pcc.
Herald added subscribers: dexonsmith, steven_wu, inglorion, mehdi_amini.

Currently, -flto-unit is specified whenever LTO options are used
(unless using the old LTO API). This causes vtable defs to be processed
using regular LTO, which is needed for CFI and whole program vtable
optimizations, since they need to modify the vtables in a whole program
manner.

However, this causes non-negligible overhead due to the regular
LTO processing. Since this isn't needed when not using CFI or
-fwhole-program-vtables, only enable -flto-unit in those cases.
Otherwise all ThinLTO compiles pay the overhead, even when not needed.


Repository:
  rC Clang

https://reviews.llvm.org/D53524

Files:
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/lto-unit.c


Index: test/Driver/lto-unit.c
===
--- test/Driver/lto-unit.c
+++ test/Driver/lto-unit.c
@@ -1,9 +1,9 @@
-// RUN: %clang -target x86_64-unknown-linux -### %s -flto=full 2>&1 | 
FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | 
FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=full 2>&1 | 
FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 2>&1 | 
FileCheck --check-prefix=NOUNIT %s
-// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=full 2>&1 | FileCheck 
--check-prefix=UNIT %s
-// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin 2>&1 | FileCheck 
--check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=full 
-fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 
-fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=full 
-fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 
-fwhole-program-vtables 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=full 
-fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin 
-fwhole-program-vtables 2>&1 | FileCheck --check-prefix=NOUNIT %s
 
 // UNIT: "-flto-unit"
 // NOUNIT-NOT: "-flto-unit"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3364,21 +3364,6 @@
 // the use-list order, since serialization to bitcode is part of the flow.
 if (JA.getType() == types::TY_LLVM_BC)
   CmdArgs.push_back("-emit-llvm-uselists");
-
-// Device-side jobs do not support LTO.
-bool isDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
-   JA.isDeviceOffloading(Action::OFK_Host));
-
-if (D.isUsingLTO() && !isDeviceOffloadAction) {
-  Args.AddLastArg(CmdArgs, options::OPT_flto, options::OPT_flto_EQ);
-
-  // The Darwin and PS4 linkers currently use the legacy LTO API, which
-  // does not support LTO unit features (CFI, whole program vtable opt)
-  // under ThinLTO.
-  if (!(RawTriple.isOSDarwin() || RawTriple.isPS4()) ||
-  D.getLTOMode() == LTOK_Full)
-CmdArgs.push_back("-flto-unit");
-}
   }
 
   if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) {
@@ -4980,6 +4965,25 @@
 CmdArgs.push_back("-fwhole-program-vtables");
   }
 
+  // Device-side jobs do not support LTO.
+  bool isDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
+ JA.isDeviceOffloading(Action::OFK_Host));
+
+  if (D.isUsingLTO() &&
+  (isa(JA) || isa(JA)) &&
+  !isDeviceOffloadAction) {
+Args.AddLastArg(CmdArgs, options::OPT_flto, options::OPT_flto_EQ);
+
+// Enable LTO unit if need for CFI or whole program vtable optimization.
+// The Darwin and PS4 linkers currently use the legacy LTO API, which
+// does not support LTO unit features (CFI, whole program vtable opt)
+// under ThinLTO.
+bool SupportsLTOUnit = !(RawTriple.isOSDarwin() || RawTriple.isPS4()) ||
+   D.getLTOMode() == LTOK_Full;
+if ((WholeProgramVTables || Sanitize.needsLTO()) && SupportsLTOUnit)
+  CmdArgs.push_back("-flto-unit");
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_fexperimental_isel,
options::OPT_fno_experimental_isel)) {
 CmdArgs.push_back("-mllvm");
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -207,6 +207,10 @@
   return Sanitizers.Mask & NeedsUnwindTables;
 }
 

[clang-tools-extra] r344943 - [Documentation] Fix grammar related to Clang-tidy cppcoreguidelines-macro-usage.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Mon Oct 22 12:55:52 2018
New Revision: 344943

URL: http://llvm.org/viewvc/llvm-project?rev=344943=rev
Log:
[Documentation] Fix grammar related to Clang-tidy cppcoreguidelines-macro-usage.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=344943=344942=344943=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Mon Oct 22 12:55:52 2018
@@ -106,7 +106,7 @@ Improvements to clang-tidy
 - New :doc:`cppcoreguidelines-macro-usage
   ` check.
 
-  Find macro usage that is considered problematic because better language
+  Finds macro usage that is considered problematic because better language
   constructs exist for the task.
 
 - New :doc:`misc-non-private-member-variables-in-classes

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst?rev=344943=344942=344943=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
 Mon Oct 22 12:55:52 2018
@@ -3,7 +3,7 @@
 cppcoreguidelines-macro-usage
 =
 
-Find macro usage that is considered problematic because better language
+Finds macro usage that is considered problematic because better language
 constructs exist for the task.
 
 The relevant sections in the C++ Core Guidelines are 


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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Please add a short sentence to the release notes and adjust the documentation 
to that check to cover the changes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53463: [Driver] allow Android triples to alias for non Android targets

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344941: [Driver] allow Android triples to alias for non 
Android targets (authored by nickdesaulniers, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53463?vs=170283=170478#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53463

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/android-gcc-toolchain.c


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -0,0 +1,8 @@
+// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
+// triple.
+//
+// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
+// RUN: | FileCheck %s
+//
+// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1847,19 +1847,21 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux"};
+  "aarch64-suse-linux", "aarch64-linux-android"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
+   "arm-linux-androideabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
+ "armeb-linux-androideabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -1870,22 +1872,24 @@
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux"};
+  "x86_64-amazon-linux","x86_64-linux-android"};
   static const char *const X32LibDirs[] = {"/libx32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
   "i686-linux-gnu",   "i686-pc-linux-gnu", "i486-linux-gnu",
   "i386-linux-gnu",   "i386-redhat-linux6E",   "i686-redhat-linux",
   "i586-redhat-linux","i386-redhat-linux", "i586-suse-linux",
-  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu"};
+  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu",
+  "i686-linux-android"};
 
   static const char *const MIPSLibDirs[] = {"/lib"};
   static const char *const MIPSTriples[] = {
   "mips-linux-gnu", "mips-mti-linux", "mips-mti-linux-gnu",
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
+  "mipsel-linux-android"};
 
   static const char *const MIPS64LibDirs[] = {"/lib64", "/lib"};
   static const char *const MIPS64Triples[] = {
@@ -1896,7 +1900,8 @@
   static const char *const MIPS64ELTriples[] = {
   "mips64el-linux-gnu",  "mips-mti-linux-gnu",
   "mips-img-linux-gnu",  "mips64el-linux-gnuabi64",
-  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64"};
+  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64",
+  "mips64el-linux-android"};
 
   static const char *const MIPSN32LibDirs[] = {"/lib32"};
   static const char *const MIPSN32Triples[] = {"mips64-linux-gnuabin32",


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -0,0 +1,8 @@
+// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
+// triple.
+//
+// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
+// RUN:   

r344941 - [Driver] allow Android triples to alias for non Android targets

[Nick Desaulniers via cfe-commits]

Author: nickdesaulniers
Date: Mon Oct 22 12:48:08 2018
New Revision: 344941

URL: http://llvm.org/viewvc/llvm-project?rev=344941=rev
Log:
[Driver] allow Android triples to alias for non Android targets

Summary:
Partial revert of r330873 ('[Driver] Reland "Android triples are not
aliases for other triples."')

While we don't want `-target *-linux-android` to alias to non
*-linux-android libs and binaries, it turns out we do want the
opposite. Ie. We would like for `-target *-linux-gnu` to still be
able to use *-android libs and binaries.

In fact, this is used to cross assemble and link the Linux kernel for
Android devices.

`-target *-linux-gnu` needs to be used for the Linux kernel when
using the android binutils prebuilts (*-linux-android).

The use of `-target *-linux-android` on C source files will cause
Clang to perform optimizations based on the presence of bionic (due to
r265481 ('Faster stack-protector for Android/AArch64.')) which is
invalid within the Linux kernel and will produce a non-bootable kernel
image.

Of course, you could just use the standard binutils (*-linux-gnu),
but Android does not distribute these.  So this patch fixes a problem
that only occurs when cross assembling and linking a Linux kernel with
the Android provided binutils, which is what is done within Android's
build system.

Reviewers: srhines, pirama, danalbert

Reviewed By: srhines, danalbert

Subscribers: javed.absar, kristof.beyls, cfe-commits

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

Added:
cfe/trunk/test/Driver/android-gcc-toolchain.c
Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=344941=344940=344941=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Mon Oct 22 12:48:08 2018
@@ -1847,19 +1847,21 @@ void Generic_GCC::GCCInstallationDetecto
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux"};
+  "aarch64-suse-linux", "aarch64-linux-android"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
+   "arm-linux-androideabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
+ "armeb-linux-androideabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -1870,14 +1872,15 @@ void Generic_GCC::GCCInstallationDetecto
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux"};
+  "x86_64-amazon-linux","x86_64-linux-android"};
   static const char *const X32LibDirs[] = {"/libx32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
   "i686-linux-gnu",   "i686-pc-linux-gnu", "i486-linux-gnu",
   "i386-linux-gnu",   "i386-redhat-linux6E",   "i686-redhat-linux",
   "i586-redhat-linux","i386-redhat-linux", "i586-suse-linux",
-  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu"};
+  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu",
+  "i686-linux-android"};
 
   static const char *const MIPSLibDirs[] = {"/lib"};
   static const char *const MIPSTriples[] = {
@@ -1885,7 +1888,8 @@ void Generic_GCC::GCCInstallationDetecto
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
+  "mipsel-linux-android"};
 
   static const char *const MIPS64LibDirs[] = {"/lib64", "/lib"};
   static const char *const MIPS64Triples[] = 

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: bruno, rsmith.
Herald added a subscriber: dexonsmith.

This patch makes clang include headers found in modulemap files in the `.d` 
file. This is necessary to capture symlinked headers, which previously were 
ignored, since only the canonical version of the file makes it into the pcm. 
This is a corollary to the -module-dependency-dir version from r264971.

rdar://44604913

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D53522

Files:
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Modules/Inputs/dfg-symlinks.modulemap
  clang/test/Modules/dependency-file-symlinks.c
  clang/test/Modules/dependency-gen-pch.m
  clang/test/Modules/dependency-gen.m
  clang/test/Modules/dependency-gen.modulemap

Index: clang/test/Modules/dependency-gen.modulemap
===
--- clang/test/Modules/dependency-gen.modulemap
+++ clang/test/Modules/dependency-gen.modulemap
@@ -27,6 +27,7 @@
 // For an explicit use of a module via -fmodule-file=, the other module maps
 // and included headers are not dependencies of this target (they are instead
 // dependencies of the explicitly-specified .pcm input).
+// FIXME: Shouldn't we include all transitive dependencies here?
 //
 // EXPLICIT: {{^}}explicit.pcm:
 // EXPLICIT-NOT: dependency-gen-
Index: clang/test/Modules/dependency-gen.m
===
--- clang/test/Modules/dependency-gen.m
+++ clang/test/Modules/dependency-gen.m
@@ -4,19 +4,19 @@
 // RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.1 -MT %s.o -I %S/Inputs -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
 // RUN: FileCheck %s < %t.d.1
 // CHECK: dependency-gen.m
-// CHECK: Inputs{{.}}module.map
 // CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
 // CHECK-NOT: usr{{.}}include{{.}}module.map
 // CHECK-NOT: stdint.h
 
 
 // RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.2 -MT %s.o -I %S/Inputs -sys-header-deps -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
 // RUN: FileCheck %s -check-prefix=CHECK-SYS < %t.d.2
 // CHECK-SYS: dependency-gen.m
-// CHECK-SYS: Inputs{{.}}module.map
 // CHECK-SYS: Inputs{{.}}diamond_top.h
-// CHECK-SYS: usr{{.}}include{{.}}module.map
+// CHECK-SYS: Inputs{{.}}module.map
 // CHECK-SYS: stdint.h
+// CHECK-SYS: usr{{.}}include{{.}}module.map
 
 #import "diamond_top.h"
 #import "stdint.h" // inside sysroot
Index: clang/test/Modules/dependency-gen-pch.m
===
--- clang/test/Modules/dependency-gen-pch.m
+++ clang/test/Modules/dependency-gen-pch.m
@@ -5,9 +5,9 @@
 // RUN: %clang_cc1 -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -module-file-deps -dependency-file %t.d -MT %s.o -I %S/Inputs -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-mcp -emit-pch -o %t.pch %s
 // RUN: FileCheck %s < %t.d
 // CHECK: dependency-gen-pch.m.o
-// CHECK-NEXT: dependency-gen-pch.m
-// CHECK-NEXT: Inputs{{.}}module.map
-// CHECK-NEXT: diamond_top.pcm
-// CHECK-NEXT: Inputs{{.}}diamond_top.h
+// CHECK: dependency-gen-pch.m
+// CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
+// CHECK: diamond_top.pcm
 
 #import "diamond_top.h"
Index: clang/test/Modules/dependency-file-symlinks.c
===
--- /dev/null
+++ clang/test/Modules/dependency-file-symlinks.c
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -fr %t
+// RUN: mkdir -p %t/cache
+
+// Set up %t as follows:
+//  * misc.h #includes target.h
+//  * target.h is empty
+//  * link.h is a symlink to target.h
+
+// RUN: cp %S/Inputs/dfg-symlinks.modulemap %t/module.modulemap
+// RUN: echo "int foo();" > %t/target.h
+// RUN: ln -s %t/target.h %t/link.h
+// RUN: echo '#include "target.h"' >> %t/misc.h
+// RUN: echo 'int foo();'  >> %t/misc.h
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN:   -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck %s
+
+// Run clang again, to make sure that we get identical output with the cached
+// modules.
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN:   -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck %s
+
+#include "misc.h"
+
+int main() {
+  void *p = foo;
+}
+
+// CHECK:  dependency-file-symlinks.o:
+// CHECK-NEXT:   {{.*}}dependency-file-symlinks.c
+// CHECK-NEXT:   {{.*}}link.h
+// CHECK-NEXT:   {{.*}}misc.h
+// CHECK-NEXT:   {{.*}}module.modulemap
+// CHECK-NEXT:   {{.*}}target.h
Index: 

[PATCH] D52857: [clang-query] Add non-exclusive output API

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

> This is already not the case with set output diag etc.

Oh, I'm wrong here. I misread your post, sorry.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52857



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

[PATCH] D53513: [OPENMP] Add support for 'atomic_default_mem_order' clause on requires directive

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

https://reviews.llvm.org/D53513



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

[PATCH] D52857: [clang-query] Add non-exclusive output API

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

> This seems confusing to me because I would expect set foo bar to set the 
> value of a variable foo to bar,

This is already not the case with `set output diag` etc.

That said, I don't mind spelling this ` output foo`. It's not 
my decision. It's up to the reviewers.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52857



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

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Max Bernstein via Phabricator via cfe-commits]

tekknolagi updated this revision to Diff 170475.
tekknolagi added a comment.

Make the test case a little easier to read


Repository:
  rC Clang

https://reviews.llvm.org/D53206

Files:
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/Analysis/padding_inherit.cpp

Index: test/Analysis/padding_inherit.cpp
===
--- /dev/null
+++ test/Analysis/padding_inherit.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=20 -verify %s
+
+// A class that has no fields and one base class should visit that base class
+// instead.
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+
+struct AnotherIntSandwich : FakeIntSandwich { // no-warning
+};
+
+// But we don't yet support multiple base classes.
+struct IntSandwich {};
+struct TooManyBaseClasses : FakeIntSandwich, IntSandwich { // no-warning
+};
+
+AnotherIntSandwich ais[100];
+
+struct Empty {};
+struct DoubleEmpty : Empty { // no-warning
+Empty e;
+};
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -75,6 +75,19 @@
 if (shouldSkipDecl(RD))
   return;
 
+// We need to be looking at a definition, not just any pointer to the
+// declaration.
+if (!(RD = RD->getDefinition()))
+  return;
+
+// This is the simplest correct case: a class with no fields and one base
+// class. Other cases are more complicated because of how the base classes
+// & fields might interact, so we don't bother dealing with them.
+if (auto *CXXRD = dyn_cast(RD))
+  if (CXXRD->field_empty() && CXXRD->getNumBases() == 1)
+return visitRecord(CXXRD->bases().begin()->getType()->getAsRecordDecl(),
+   PadMultiplier);
+
 auto  = RD->getASTContext();
 const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
 assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
@@ -112,8 +125,7 @@
 if (RT == nullptr)
   return;
 
-// TODO: Recurse into the fields and base classes to see if any
-// of those have excess padding.
+// TODO: Recurse into the fields to see if they have excess padding.
 visitRecord(RT->getDecl(), Elts);
   }
 
@@ -132,13 +144,17 @@
 // Not going to attempt to optimize unions.
 if (RD->isUnion())
   return true;
-// How do you reorder fields if you haven't got any?
-if (RD->field_empty())
-  return true;
 if (auto *CXXRD = dyn_cast(RD)) {
+  // TODO: Figure out why we are going through declarations and not only
+  // definitions.
+  if (!(CXXRD = CXXRD->getDefinition()))
+return true;
   // Tail padding with base classes ends up being very complicated.
-  // We will skip objects with base classes for now.
-  if (CXXRD->getNumBases() != 0)
+  // We will skip objects with base classes for now, unless they do not
+  // have fields.
+  if (!CXXRD->field_empty() && CXXRD->getNumBases() != 0)
+return true;
+  if (CXXRD->field_empty() && CXXRD->getNumBases() != 1)
 return true;
   // Virtual bases are complicated, skipping those for now.
   if (CXXRD->getNumVBases() != 0)
@@ -150,6 +166,10 @@
   if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
 return true;
 }
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+  return true;
+
 auto IsTrickyField = [](const FieldDecl *FD) -> bool {
   // Bitfield layout is hard.
   if (FD->isBitField())
@@ -323,7 +343,7 @@
 BR->emitReport(std::move(Report));
   }
 };
-}
+} // namespace
 
 void ento::registerPaddingChecker(CheckerManager ) {
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Max Bernstein via Phabricator via cfe-commits]

tekknolagi added inline comments.



Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+

tekknolagi wrote:
> bcraig wrote:
> > Looking at this again... what new cases does this catch?  FakeIntSandwich 
> > was caught before (it is identical to 'PaddedA', and AnotherIntSandwich 
> > generated no warning before.  So what is different?
> > 
> > EBO1 and EBO2 are cases above that would be nice to catch, but aren't being 
> > caught right now.
> Ah, you're right. I'll just keep the one in `padding_inherit`.
@bcraig I updated the description of the diff to be more informative about the 
particular cases this change catches.


Repository:
  rC Clang

https://reviews.llvm.org/D53206



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

[PATCH] D53463: [Driver] allow Android triples to alias for non Android targets

[Dan Albert via Phabricator via cfe-commits]

danalbert accepted this revision.
danalbert added a comment.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D53463



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

[PATCH] D53520: Update the example of BS_Stroustrup to match what is done by clang-format

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: krasimir.

reported here https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=911561

clang-format-7 -style="{BreakBeforeBraces: Stroustrup}" wasn't doing
the same as the doc


Repository:
  rC Clang

https://reviews.llvm.org/D53520

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h


Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -528,25 +528,21 @@
 ///   enum X : int { A, B };
 /// \endcode
 BS_Mozilla,
-/// Like ``Attach``, but break before function definitions, ``catch``, and
-/// ``else``.
+/// Always break before braces.
 /// \code
 ///   try {
 /// foo();
-///   } catch () {
+///   }
+///   catch () {
 ///   }
 ///   void foo() { bar(); }
-///   class foo
-///   {
+///   class foo {
 ///   };
 ///   if (foo()) {
-///   } else {
 ///   }
-///   enum X : int
-///   {
-/// A,
-/// B
-///   };
+///   else {
+///   }
+///   enum X : int { A, B };
 /// \endcode
 BS_Stroustrup,
 /// Always break before braces.
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -901,27 +901,23 @@
   enum X : int { A, B };
 
   * ``BS_Stroustrup`` (in configuration: ``Stroustrup``)
-Like ``Attach``, but break before function definitions, ``catch``, and
-``else``.
+Always break before braces.
 
 .. code-block:: c++
 
   try {
 foo();
-  } catch () {
+  }
+  catch () {
   }
   void foo() { bar(); }
-  class foo
-  {
+  class foo {
   };
   if (foo()) {
-  } else {
   }
-  enum X : int
-  {
-A,
-B
-  };
+  else {
+  }
+  enum X : int { A, B };
 
   * ``BS_Allman`` (in configuration: ``Allman``)
 Always break before braces.


Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -528,25 +528,21 @@
 ///   enum X : int { A, B };
 /// \endcode
 BS_Mozilla,
-/// Like ``Attach``, but break before function definitions, ``catch``, and
-/// ``else``.
+/// Always break before braces.
 /// \code
 ///   try {
 /// foo();
-///   } catch () {
+///   }
+///   catch () {
 ///   }
 ///   void foo() { bar(); }
-///   class foo
-///   {
+///   class foo {
 ///   };
 ///   if (foo()) {
-///   } else {
 ///   }
-///   enum X : int
-///   {
-/// A,
-/// B
-///   };
+///   else {
+///   }
+///   enum X : int { A, B };
 /// \endcode
 BS_Stroustrup,
 /// Always break before braces.
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -901,27 +901,23 @@
   enum X : int { A, B };
 
   * ``BS_Stroustrup`` (in configuration: ``Stroustrup``)
-Like ``Attach``, but break before function definitions, ``catch``, and
-``else``.
+Always break before braces.
 
 .. code-block:: c++
 
   try {
 foo();
-  } catch () {
+  }
+  catch () {
   }
   void foo() { bar(); }
-  class foo
-  {
+  class foo {
   };
   if (foo()) {
-  } else {
   }
-  enum X : int
-  {
-A,
-B
-  };
+  else {
+  }
+  enum X : int { A, B };
 
   * ``BS_Allman`` (in configuration: ``Allman``)
 Always break before braces.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344940: [clang-tidy] implement cppcoreguidelines macro 
rules (authored by JonasToth, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41648?vs=170472=170473#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: clang-tidy/cppcoreguidelines/MacroUsageCheck.h
===
--- clang-tidy/cppcoreguidelines/MacroUsageCheck.h
+++ clang-tidy/cppcoreguidelines/MacroUsageCheck.h
@@ -0,0 +1,48 @@
+//===--- MacroUsageCheck.h - clang-tidy--*- 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_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
+
+#include "../ClangTidy.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Find macro usage that is considered problematic because better language
+/// constructs exist for the task.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-macro-usage.html
+class MacroUsageCheck : public ClangTidyCheck {
+public:
+  MacroUsageCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context),
+AllowedRegexp(Options.get("AllowedRegexp", "^DEBUG_*")),
+CheckCapsOnly(Options.get("CheckCapsOnly", 0)) {}
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
+  void warnMacro(const MacroDirective *MD);
+  void warnNaming(const MacroDirective *MD);
+
+private:
+  /// A regular expression that defines how allowed macros must look like.
+  std::string AllowedRegexp;
+  /// Control if only the check shall only test on CAPS_ONLY macros.
+  bool CheckCapsOnly;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@
   AvoidGotoCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
+  MacroUsageCheck.cpp
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp
Index: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
===
--- clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -0,0 +1,95 @@
+//===--- MacroUsageCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "MacroUsageCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Regex.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+namespace {
+
+bool isCapsOnly(StringRef Name) {
+  return std::all_of(Name.begin(), Name.end(), [](const char c) {
+if (std::isupper(c) || std::isdigit(c) || c == '_')
+  return true;
+return false;
+  });
+}
+
+class MacroUsageCallbacks : public PPCallbacks {
+public:
+  MacroUsageCallbacks(MacroUsageCheck *Check, StringRef RegExp, bool CapsOnly)
+  : Check(Check), RegExp(RegExp), CheckCapsOnly(CapsOnly) {}
+  void MacroDefined(const Token ,
+const MacroDirective *MD) override {
+if (MD->getMacroInfo()->isUsedForHeaderGuard() ||
+MD->getMacroInfo()->getNumTokens() == 0)
+  return;
+
+StringRef MacroName = MacroNameTok.getIdentifierInfo()->getName();
+if (!CheckCapsOnly && !llvm::Regex(RegExp).match(MacroName))
+  Check->warnMacro(MD);
+
+if (CheckCapsOnly && 

[clang-tools-extra] r344940 - [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Mon Oct 22 12:20:01 2018
New Revision: 344940

URL: http://llvm.org/viewvc/llvm-project?rev=344940=rev
Log:
[clang-tidy] implement cppcoreguidelines macro rules

Summary:
In short macros are discouraged by multiple rules (and sometimes reference 
randomly). [Enum.1], [ES.30], [ES.31]
This check allows only headerguards and empty macros for annotation.

Reviewers: aaron.ballman, hokein

Reviewed By: aaron.ballman

Subscribers: jbcoe, Eugene.Zelenko, klimek, nemanjai, mgorny, xazax.hun, 
kbarton, cfe-commits

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

Added:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=344940=344939=344940=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Mon Oct 
22 12:20:01 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyCppCoreGuidel
   AvoidGotoCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
+  MacroUsageCheck.cpp
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=344940=344939=344940=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 Mon Oct 22 12:20:01 2018
@@ -15,6 +15,7 @@
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
+#include "MacroUsageCheck.h"
 #include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
@@ -45,6 +46,8 @@ public:
 "cppcoreguidelines-avoid-magic-numbers");
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
+CheckFactories.registerCheck(
+"cppcoreguidelines-macro-usage");
 CheckFactories.registerCheck(
 "cppcoreguidelines-narrowing-conversions");
 CheckFactories.registerCheck("cppcoreguidelines-no-malloc");

Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp?rev=344940=auto
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp 
Mon Oct 22 12:20:01 2018
@@ -0,0 +1,95 @@
+//===--- MacroUsageCheck.cpp - 
clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "MacroUsageCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Regex.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+namespace {
+
+bool isCapsOnly(StringRef Name) {
+  return std::all_of(Name.begin(), Name.end(), [](const char c) {
+if (std::isupper(c) || std::isdigit(c) || c == '_')
+  return true;
+return false;
+  });
+}
+
+class MacroUsageCallbacks : public PPCallbacks {
+public:
+  MacroUsageCallbacks(MacroUsageCheck *Check, StringRef RegExp, bool CapsOnly)
+  : Check(Check), RegExp(RegExp), CheckCapsOnly(CapsOnly) {}
+  void MacroDefined(const Token ,
+const MacroDirective *MD) override {
+if 

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 170472.
JonasToth added a comment.

- [Doc] add missing release notes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+#define TEST_VARIADIC2(x, ...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+//
+#define problematic_variadic2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define OKISH_CONSTANT 42
+#define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define OKISH_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -89,6 +89,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 170471.
JonasToth added a comment.

- add missing private


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+#define TEST_VARIADIC2(x, ...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+//
+#define problematic_variadic2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define OKISH_CONSTANT 42
+#define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define OKISH_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -89,6 +89,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-non-private-member-variables-in-classes (redirects to 

[PATCH] D53513: [OPENMP] Add support for 'atomic_default_mem_order' clause on requires directive

[Patrick Lyster via Phabricator via cfe-commits]

patricklyster added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:930
+  /// Returns location of clause kind.
+  SourceLocation getAtomicDefaultMemOrderKindKwLoc() const { return KindKwLoc; 
}
+

ABataev wrote:
> Again, probably too long line.
None of these lines are longer than 80 symbols - hard to see this on 
Phabricator though. All this code was formatted using the clang-format tool. 
Would you still like me to change it?


Repository:
  rC Clang

https://reviews.llvm.org/D53513



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 170470.
JonasToth marked 4 inline comments as done.
JonasToth added a comment.

- address review nits


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+#define TEST_VARIADIC2(x, ...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+//
+#define problematic_variadic2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define OKISH_CONSTANT 42
+#define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define OKISH_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -89,6 +89,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.h:34-35
+  void registerPPCallbacks(CompilerInstance ) override;
+  void warnMacro(const MacroDirective *MD);
+  void warnNaming(const MacroDirective *MD);
+

aaron.ballman wrote:
> Nit: these can be private functions instead.
They are used by the PPCallbacks. I think adding a `friend` is not worth it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

r344939 - Hopefully fix the documentation generation issue

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Mon Oct 22 12:07:29 2018
New Revision: 344939

URL: http://llvm.org/viewvc/llvm-project?rev=344939=rev
Log:
Hopefully fix the documentation generation issue

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=344939=344938=344939=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Mon Oct 22 12:07:29 2018
@@ -1406,12 +1406,14 @@ the configuration (without a prefix: ``A
   can be a subset of another - the longest prefix is always matched. Within
   a group, the imports are ordered lexicographically.
 
-  In the .clang-format configuration file, this can be configured like:
+  In the .clang-format configuration file, this can be configured like
+  in the following yaml example. This will result in imports being
+  formatted as in the Java example below.
 
   .. code-block:: yaml
 
 JavaImportGroups: ['com.example', 'com', 'org']
-  Which will result in imports being formatted as so:
+
 
   .. code-block:: java
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=344939=344938=344939=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Mon Oct 22 12:07:29 2018
@@ -1139,11 +1139,13 @@ struct FormatStyle {
   /// can be a subset of another - the longest prefix is always matched. Within
   /// a group, the imports are ordered lexicographically.
   ///
-  /// In the .clang-format configuration file, this can be configured like:
+  /// In the .clang-format configuration file, this can be configured like
+  /// in the following yaml example. This will result in imports being
+  /// formatted as in the Java example below.
   /// \code{.yaml}
   ///   JavaImportGroups: ['com.example', 'com', 'org']
   /// \endcode
-  /// Which will result in imports being formatted as so:
+  ///
   /// \code{.java}
   ///import static com.example.function1;
   ///


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

[PATCH] D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2026
+return EmitScalarConversion(Visit(E), E->getType(), DestTy,
+CE->getExprLoc());
 

ebevhan wrote:
> rjmccall wrote:
> > Why are you pushing these casts through `EmitScalarConversion` when the 
> > cast kind already tells you which operation you're doing?
> It could be useful to enable EmitScalarConversion to handle any of the 
> conversions so it can be used in other contexts than expanding a cast.
> It could be useful to enable EmitScalarConversion to handle any of the 
> conversions so it can be used in other contexts than expanding a cast.

This was also the reason I did this since `EmitScalarConversion` seems to get 
called in many places as a generic dispatch for different conversions.


Repository:
  rC Clang

https://reviews.llvm.org/D53308



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

[PATCH] D52857: [clang-query] Add non-exclusive output API

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I'm not really involved in clang-query development these days, so this is more 
of a drive-by comment. It wasn't really obvious to me what the wording of the 
help text for `set enable-output` meant:

> Set whether to enable  content non-exclusively.

I figured out from the name of the option and from reading the source code that 
it just means "Enable output in format ." This seems confusing to me 
because I would expect `set foo bar` to set the value of a variable `foo` to 
`bar`, which doesn't match what this command does. What do you think about 
spelling it as something like `enable output `?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52857



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

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think `RetainCountChecker` is the only checker that maintains such maps and 
does such cleanup: 
https://github.com/llvm-mirror/clang/blob/efe41bf98/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h#L288

I didn't know it does that until today.


https://reviews.llvm.org/D51531



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

[PATCH] D53066: [RFC] [Driver] Use forward slashes in most linker arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping @rnk


https://reviews.llvm.org/D53066



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

r344934 - Generate ClangFormatStyleOptions.rst from Format.h (using docs/tools/dump_format_style.py)

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Mon Oct 22 11:48:58 2018
New Revision: 344934

URL: http://llvm.org/viewvc/llvm-project?rev=344934=rev
Log:
Generate ClangFormatStyleOptions.rst from Format.h (using 
docs/tools/dump_format_style.py)

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=344934=344933=344934=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Mon Oct 22 11:48:58 2018
@@ -1134,21 +1134,17 @@ the configuration (without a prefix: ``A
   .. code-block:: c++
 
 true:
-FitsOnOneLine::Constructor()
-: a(aa), a(aa) {}
-
-DoesntFit::Constructor()
-: a(aa),
-  a(aa),
-  a(aa) {}
+SomeClass::Constructor()
+: (), (), 
(a) {
+  return 0;
+}
 
 false:
-FitsOnOneLine::Constructor()
-: a(aa), a(aa) {}
-
-DoesntFit::Constructor()
-: a(aa), a(aa),
-  a(aa) {}
+SomeClass::Constructor()
+: (), (),
+  (a) {
+  return 0;
+}
 
 **ConstructorInitializerIndentWidth** (``unsigned``)
   The number of characters to use for indentation of constructor
@@ -1402,6 +1398,37 @@ the configuration (without a prefix: ``A
  LngReturnType
  LngFunctionDeclaration();
 
+**JavaImportGroups** (``std::vector``)
+  A vector of prefixes ordered by the desired groups for Java imports.
+
+  Each group is seperated by a newline. Static imports will also follow the
+  same grouping convention above all non-static imports. One group's prefix
+  can be a subset of another - the longest prefix is always matched. Within
+  a group, the imports are ordered lexicographically.
+
+  In the .clang-format configuration file, this can be configured like:
+
+  .. code-block:: yaml
+
+JavaImportGroups: ['com.example', 'com', 'org']
+  Which will result in imports being formatted as so:
+
+  .. code-block:: java
+
+ import static com.example.function1;
+
+ import static com.test.function2;
+
+ import static org.example.function3;
+
+ import com.example.ClassA;
+ import com.example.Test;
+ import com.example.a.ClassB;
+
+ import com.test.ClassC;
+
+ import org.example.ClassD;
+
 **JavaScriptQuotes** (``JavaScriptQuoteStyle``)
   The JavaScriptQuoteStyle to use for JavaScript strings.
 
@@ -1980,7 +2007,8 @@ the configuration (without a prefix: ``A
 
 
 **StatementMacros** (``std::vector``)
-  A vector of macros that should be interpreted as complete statements.
+  A vector of macros that should be interpreted as complete
+  statements.
 
   Typical macros are expressions, and require a semi-colon to be
   added; sometimes this is not the case, and this allows to make


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

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Max Bernstein via Phabricator via cfe-commits]

tekknolagi updated this revision to Diff 170464.
tekknolagi edited the summary of this revision.
tekknolagi added a comment.

Remove useless test case in `padding_cpp.cpp`


Repository:
  rC Clang

https://reviews.llvm.org/D53206

Files:
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/Analysis/padding_inherit.cpp

Index: test/Analysis/padding_inherit.cpp
===
--- /dev/null
+++ test/Analysis/padding_inherit.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=20 -verify %s
+
+// A class that has no fields and one base class should visit that base class
+// instead.
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+
+struct AnotherIntSandwich : FakeIntSandwich { // no-warning
+};
+
+struct IntSandwich {};
+
+// But we don't yet support multiple base classes.
+struct TooManyBaseClasses : FakeIntSandwich, IntSandwich { // no-warning
+};
+
+AnotherIntSandwich ais[100];
+
+struct Empty {};
+struct DoubleEmpty : Empty { // no-warning
+Empty e;
+};
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -75,6 +75,19 @@
 if (shouldSkipDecl(RD))
   return;
 
+// We need to be looking at a definition, not just any pointer to the
+// declaration.
+if (!(RD = RD->getDefinition()))
+  return;
+
+// This is the simplest correct case: a class with no fields and one base
+// class. Other cases are more complicated because of how the base classes
+// & fields might interact, so we don't bother dealing with them.
+if (auto *CXXRD = dyn_cast(RD))
+  if (CXXRD->field_empty() && CXXRD->getNumBases() == 1)
+return visitRecord(CXXRD->bases().begin()->getType()->getAsRecordDecl(),
+   PadMultiplier);
+
 auto  = RD->getASTContext();
 const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
 assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
@@ -112,8 +125,7 @@
 if (RT == nullptr)
   return;
 
-// TODO: Recurse into the fields and base classes to see if any
-// of those have excess padding.
+// TODO: Recurse into the fields to see if they have excess padding.
 visitRecord(RT->getDecl(), Elts);
   }
 
@@ -132,13 +144,17 @@
 // Not going to attempt to optimize unions.
 if (RD->isUnion())
   return true;
-// How do you reorder fields if you haven't got any?
-if (RD->field_empty())
-  return true;
 if (auto *CXXRD = dyn_cast(RD)) {
+  // TODO: Figure out why we are going through declarations and not only
+  // definitions.
+  if (!(CXXRD = CXXRD->getDefinition()))
+return true;
   // Tail padding with base classes ends up being very complicated.
-  // We will skip objects with base classes for now.
-  if (CXXRD->getNumBases() != 0)
+  // We will skip objects with base classes for now, unless they do not
+  // have fields.
+  if (!CXXRD->field_empty() && CXXRD->getNumBases() != 0)
+return true;
+  if (CXXRD->field_empty() && CXXRD->getNumBases() != 1)
 return true;
   // Virtual bases are complicated, skipping those for now.
   if (CXXRD->getNumVBases() != 0)
@@ -150,6 +166,10 @@
   if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
 return true;
 }
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+  return true;
+
 auto IsTrickyField = [](const FieldDecl *FD) -> bool {
   // Bitfield layout is hard.
   if (FD->isBitField())
@@ -323,7 +343,7 @@
 BR->emitReport(std::move(Report));
   }
 };
-}
+} // namespace
 
 void ento::registerPaddingChecker(CheckerManager ) {
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Max Bernstein via Phabricator via cfe-commits]

tekknolagi added inline comments.



Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+

bcraig wrote:
> Looking at this again... what new cases does this catch?  FakeIntSandwich was 
> caught before (it is identical to 'PaddedA', and AnotherIntSandwich generated 
> no warning before.  So what is different?
> 
> EBO1 and EBO2 are cases above that would be nice to catch, but aren't being 
> caught right now.
Ah, you're right. I'll just keep the one in `padding_inherit`.


Repository:
  rC Clang

https://reviews.llvm.org/D53206



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

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

@bcraig comments look valid, marking as "Needs Changes"


Repository:
  rC Clang

https://reviews.llvm.org/D53206



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

[PATCH] D53483: [analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

OK, so the overall direction makes sense: unregistered options are restricted 
to checkers, and for others, an explicit getter must be maintained.
(though I would also prefer if even checkers could pre-register their options 
somehow)

@NoQ does this make sense to you?




Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:243
+  /// specified.
+  StringRef getStringOption(StringRef Name, StringRef DefaultVal);
 

xazax.hun wrote:
> If you want the devs to maintain an explicit getter for each analyzer option 
> rather than making this one public at some point, please document expectation 
> this somewhere.
+1


Repository:
  rC Clang

https://reviews.llvm.org/D53483



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

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Ping


https://reviews.llvm.org/D52949



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

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Ping


https://reviews.llvm.org/D52835



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.
Herald added subscribers: dkrupp, donat.nagy.

I'm marking it as "needs changes" for now -- the idea seems reasonable, but 
it's hard to tell without a thorough evaluation on at least a few codebases.
Additionally, we should figure out a checker group for this - if "alpha" means 
"work in progress", then it should be under something else, like "linting" or 
similar.


https://reviews.llvm.org/D50488



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

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.



Comment at: www/analyzer/available_checks.html:459
 
+
 

Spurious newline



Comment at: www/analyzer/available_checks.html:496
+
+
+

If we don't have a description, let's just drop it.



Comment at: www/analyzer/available_checks.html:677
+
+void use_semaphor_antipattern() {
+  dispatch_semaphore_t sema = dispatch_semaphore_create(0);

I have a longer description:

This checker finds a common performance anti-pattern in a code that uses Grand 
Central dispatch.
The anti-pattern involves emulating a synchronous call from an asynchronous API 
using semaphores,
as in the snippet below, where the `requestCurrentTaskName` function makes an 
XPC call and then uses the semaphore to
block until the XPC call returns:

```
+ (NSString *)requestCurrentTaskName {
__block NSString *taskName = nil;
dispatch_semaphore_t sema = dispatch_semaphore_create(0);
NSXPCConnection *connection = [[NSXPCConnection alloc] 
initWithServiceName:@"MyConnection"];
id remoteObjectProxy = connection.remoteObjectProxy;
[remoteObjectProxy requestCurrentTaskName:^(NSString *task) {
taskName = task;
dispatch_semaphore_signal(sema);
}];
dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER);
return taskName;
}
```

Usage of such a pattern in production code running on the main thread is 
discouraged, as the main queue gets blocked waiting for the background queue,
which could be running at a lower priority, and unnecessary threads are spawned 
in the process.

In order to avoid the anti-pattern, the available alternatives are:

 - Use the synchronous version of the API, if available.
In the example above, the synchronous XPC proxy object can be used:

```
+ (NSString *)requestCurrentTaskName {
__block NSString *taskName = nil;
NSXPCConnection *connection = [[NSXPCConnection alloc] 
initWithServiceName:@"MyConnection"];
id remoteObjectProxy = [connection 
synchronousRemoteObjectProxyWithErrorHandler:^(NSError *error) {
  NSLog(@"Error = %@", error);

}];
[remoteObjectProxy requestCurrentTaskName:^(NSString *task) {
taskName = task;
}];
return taskName;
}
```

 - Alternatively, the API can be used in the asynchronous way.




Comment at: www/analyzer/available_checks.html:768
+Check for proper uses of Objective-C properties
+
+

If we don't have proper description, let's drop.



Comment at: www/analyzer/available_checks.html:877
+(ObjC)
+Warn about potentially crashing writes to autoreleasing objects from different
+autoreleasing pools in Objective-C.

I have a longer description:

Under ARC, function parameters which are pointers to pointers (e.g. NSError**) 
are `__autoreleasing`.
Writing to such parameters inside autoreleasing pools might crash whenever the 
parameter outlives the pool. Detecting such crashes may be difficult, as usage 
of autorelease pool is usually hidden inside the called functions 
implementation. Examples include:

```
BOOL writeToErrorWithIterator(NSError *__autoreleasing* error, NSArray *a) { [a 
enumerateObjectsUsingBlock:^{
*error = [NSError errorWithDomain:1];
}];
}
```

and

```
BOOL writeToErrorInBlockFromCFunc(NSError *__autoreleasing* error) {
dispatch_semaphore_t sem = dispatch_semaphore_create(0l);
dispatch_async(queue, ^{
if (error) {
*error = [NSError errorWithDomain:1];
}
dispatch_semaphore_signal(sem);
});
 
dispatch_semaphore_wait(sem, 100);
  return 0;
}
```




Comment at: www/analyzer/available_checks.html:1071
+(ObjC)
+Model the APIs that are guaranteed to return a non-nil value.
+

Probably should be dropped.



Comment at: www/analyzer/available_checks.html:1119
+
+
+

Top of the checker file has a somewhat reasonable description:

// A checker for detecting leaks resulting from allocating temporary
// autoreleased objects before starting the main run loop.
//
// Checks for two antipatterns:
// 1. ObjCMessageExpr followed by [[NSRunLoop mainRunLoop] run] in the same
// autorelease pool.
// 2. ObjCMessageExpr followed by [[NSRunLoop mainRunLoop] run] in no
// autorelease pool.
//
// Any temporary objects autoreleased in code called in those expressions
// will not be deallocated until the program exits, and are effectively leaks.



https://reviews.llvm.org/D53069



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

[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.

[Neeraj K. Singh via Phabricator via cfe-commits]

neerajksingh added a comment.

In https://reviews.llvm.org/D53457#1269769, @hans wrote:

> I'm not completely convinced that we want this. So far we've used the 
> strategy of promoting clang options that are also useful in clang-cl to core 
> options, and if someone wants to use more clang than that, maybe clang-cl 
> isn't the right driver for that use-case.
>
> But I suppose an argument could be made for having an escape hatch from 
> clang-cl if it doesn't add too much complexity to the code.


This is a good point. However, having this escape hatch gets you and Reid and 
others out of the business of having to promote options. Also, as new flags are 
added to the compiler people might need one revision of the official builds to 
realize they need a flag and one revision to get the flag added to the binary 
release. This obviously isn't a problem for people building Clang from source, 
but it does add unnecessary friction as I found myself.

> I'm not sure I'm a fan of calling it /Xdriver: though, because what does it 
> mean - clang-cl is the driver, but the option refers to the clang driver. The 
> natural name would of course be -Xclang but that already means something 
> else.  Maybe we could just call it /clang:

At the conference last week I discussed this with Reid Kleckner. One of the 
options we discussed was trying to make things work such that -Xclang serves 
both purposes.  We quickly decided that this wouldn't work.  /clang: would be 
fine, but it might be more confusing since people will wonder what's the 
difference between /Xclang and /clang:.   We could use something more verbose 
like /Xclang-driver:.  I'd be happy to change the flag to whatever spelling 
will build consensus.


Repository:
  rC Clang

https://reviews.llvm.org/D53457



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

r344929 - [X86] Add new features to the priority list for target attribute multiversioning.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Oct 22 10:59:58 2018
New Revision: 344929

URL: http://llvm.org/viewvc/llvm-project?rev=344929=rev
Log:
[X86] Add new features to the priority list for target attribute 
multiversioning.

Modified:
cfe/trunk/include/clang/Basic/X86Target.def

Modified: cfe/trunk/include/clang/Basic/X86Target.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/X86Target.def?rev=344929=344928=344929=diff
==
--- cfe/trunk/include/clang/Basic/X86Target.def (original)
+++ cfe/trunk/include/clang/Basic/X86Target.def Mon Oct 22 10:59:58 2018
@@ -283,6 +283,11 @@ FEATURE(FEATURE_AVX512IFMA)
 FEATURE(FEATURE_AVX5124VNNIW)
 FEATURE(FEATURE_AVX5124FMAPS)
 FEATURE(FEATURE_AVX512VPOPCNTDQ)
+FEATURE(FEATURE_AVX512VBMI2)
+FEATURE(FEATURE_GFNI)
+FEATURE(FEATURE_VPCLMULQDQ)
+FEATURE(FEATURE_AVX512VNNI)
+FEATURE(FEATURE_AVX512BITALG)
 
 
 // FIXME: When commented out features are supported in LLVM, enable them here.


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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D38061#1271021, @twoh wrote:

> @aprantl It is a debug info. If you compile 
> test/CodeGenCXX/debug-info-anonymous.cpp file with `clang -g2 -emit-llvm -S 
> `, you will find debug metadata something like `distinct !DISubprogram(name: 
> "foo /home/twoh/llvms/llvm/tools/clang/test/CodeGenCXX/debug-info-anonymous.cpp:9:3)>",
>  linkageName: "_Z3fooIN1XUt_EEiT_" ...`, which will eventually be included in 
> .debug_info section.


For comparison, GCC names these "foo >" - this is somewhat 
of a compatibility problem, since the template function definition's names 
won't match between GCC and Clang, but I guess this is by far not the only 
instance of that. GCC keeps that naming scheme even when it's clearly ambiguous 
(ie: it's not just making a short name when there's no other X::anonymous enum, 
it does it even when there's two of them, etc)


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

[George Karpenkov via Phabricator via cfe-commits]

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

Good to go, with minor nits inline.




Comment at: lib/FrontendTool/ExecuteCompilerInvocation.cpp:255
+ento::printAnalyzerConfigList(llvm::outs());
+return true;
+  }

Hm, should we return here?
If there are errors, which are only printed in the next part, should they still 
be printed?



Comment at: test/Analysis/analyzer-list-configs.c:13
+//
+// CHECK: OPTIONS:

Just to be sure, I would hardcode checking for at least the first option here.


https://reviews.llvm.org/D53296



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

[PATCH] D53513: [OPENMP] Add support for 'atomic_default_mem_order' clause on requires directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Check the code formatting, please. There should be lines longer than 80 symbols.




Comment at: clang/include/clang/AST/OpenMPClause.h:886
+  /// \param K Kind of clause.
+  void setAtomicDefaultMemOrderKind(OpenMPAtomicDefaultMemOrderClauseKind K) {
+Kind = K;

Check formatting here



Comment at: clang/include/clang/AST/OpenMPClause.h:906
+  /// \param EndLoc Ending location of the clause.
+  OMPAtomicDefaultMemOrderClause(OpenMPAtomicDefaultMemOrderClauseKind A,
+ SourceLocation ALoc, SourceLocation StartLoc,

Also, formatting



Comment at: clang/include/clang/AST/OpenMPClause.h:930
+  /// Returns location of clause kind.
+  SourceLocation getAtomicDefaultMemOrderKindKwLoc() const { return KindKwLoc; 
}
+

Again, probably too long line.


Repository:
  rC Clang

https://reviews.llvm.org/D53513



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

@aprantl It is a debug info. If you compile 
test/CodeGenCXX/debug-info-anonymous.cpp file with `clang -g2 -emit-llvm -S `, 
you will find debug metadata something like `distinct !DISubprogram(name: 
"foo",
 linkageName: "_Z3fooIN1XUt_EEiT_" ...`, which will eventually be included in 
.debug_info section.


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

The size of an os_log buffer is known at any stage of compilation, so making it 
a constant expression means that the common idiom of declaring a buffer for it 
won't result in a VLA. That allows the compiler to skip saving and restoring 
the stack pointer around such buffers.

There can be hundreds or even thousands of such calls, even in firmware 
projects, so the code size savings add up.


Repository:
  rC Clang

https://reviews.llvm.org/D53514

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* 
%[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8* %[[ARGSIZE]], align 1
 // CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3594,13 +3594,6 @@
   case Builtin::BI__builtin_os_log_format:
 return emitBuiltinOSLogFormat(*E);
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-analyze_os_log::OSLogBufferLayout Layout;
-analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
-return RValue::get(ConstantInt::get(ConvertType(E->getType()),
-Layout.size().getQuantity()));
-  }
-
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -33,6 +33,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/Analyses/OSLog.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
@@ -8112,6 +8113,12 @@
 llvm_unreachable("unexpected EvalMode");
   }
 
+  case Builtin::BI__builtin_os_log_format_buffer_size: {
+analyze_os_log::OSLogBufferLayout Layout;
+analyze_os_log::computeOSLogBufferLayout(Info.Ctx, E, Layout);
+return Success(Layout.size().getQuantity(), E);
+  }
+
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
   case Builtin::BI__builtin_bswap64: {


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* %[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: 

[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D53457#1269769, @hans wrote:

> I haven't started looking at the code yet.
>
> I'm not completely convinced that we want this. So far we've used the 
> strategy of promoting clang options that are also useful in clang-cl to core 
> options, and if someone wants to use more clang than that, maybe clang-cl 
> isn't the right driver for that use-case.
>
> But I suppose an argument could be made for having an escape hatch from 
> clang-cl if it doesn't add too much complexity to the code.


Personally, I really want to get out of the role of being a gatekeeper for 
GCC-style driver flags. I want users to be able to solve their own problems 
without building clang-cl from scratch or waiting for the next 6 month release.

I'm personally in favor of moving clang-cl to a blacklist model instead of a 
whitelist model so that we go even further in this direction, but it doesn't 
address what to do about conflicting or ambiguous options like `-MD`. An escape 
hatch like `[-/]Xdriver:` seems like a useful thing to have when that arises.

> I'm not sure I'm a fan of calling it /Xdriver: though, because what does it 
> mean - clang-cl is the driver, but the option refers to the clang driver. The 
> natural name would of course be -Xclang but that already means something 
> else.  Maybe we could just call it /clang:

Huh, I like that. You'd also be able to spell it `-clang:`, so the leading `/` 
is less of a disambiguator, but the colon is clearly a sign that this is a 
CL-style option.


Repository:
  rC Clang

https://reviews.llvm.org/D53457



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

LLVM buildmaster will be restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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