[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Michele Scandale via Phabricator via cfe-commits
michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2337
+<< "-ffp-contract=fast";
+optID = options::OPT_ffast_math;
+FPModel = Val;

Here it seems you are changing `optID` to `OPT_ffast_math` to reuse the logic 
specified below for that case to reset the state of the floating point options.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2345
+<< "-ffp-contract=fast";
+optID = options::OPT_ffp_contract;
+FPModel = Val;

Here the state of the floating point options seems unchanged except for 
`FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would 
expect the state of the floating point options to match the one of 
`-fno-fast-math` except for `FPContract` which you want to be set to "fast".

I think you might need to replicate the reset for all the option here as well, 
so at this point I don't know how much worth is to use the optID reset trick 
for the "fast" case only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

2019-11-19 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318
+ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal ) 
{
   if (auto Reg = Val.getAsRegion()) {
 Reg = Reg->getMostDerivedObjectRegion();
-return State->get(Reg);
+return State->remove(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
-return State->get(Sym);
+return State->remove(Sym);
   } else if (const auto LCVal = Val.getAs()) {

NoQ wrote:
> Maybe move this function to `Iterator.cpp` as well, and then move the 
> definitions for iterator maps from `Iterator.h` to `Iterator.cpp`, which will 
> allow you to use the usual `REGISTER_MAP_WITH_PROGRAMSTATE` macros, and 
> additionally guarantee that all access to the maps goes through the accessor 
> methods that you provide?
Hmm, I was trying hard to use these macros but failed so I reverted to the 
manual solution. I will retry now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70320



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


[PATCH] D70446: [clangd] Treat UserDefinedLiteral as a leaf in SelectionTree, sidestepping tokenization issues

2019-11-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70446



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Michele Scandale via Phabricator via cfe-commits
michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");

mibintc wrote:
> michele.scandale wrote:
> > With this change if I run `clang -### -ffast-math test.c` I don't see 
> > `-fno-trapping-math` passed to the CC1. 
> > This is changing the changes the value of the function level attribute 
> > "no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
> > Is this an intended change?
> > 
> > Moreover since with this patch the default value for trapping math changed, 
> > the "no-trapping-math" function level attribute is incorrect also for 
> > default case.
> Before this patch, ftrapping-math was added to the Driver and also a 
> bitfield, ``NoTrappingFPMath`` was created in the LLVM to describe the state 
> of trapping-math, but otherwise that bit wasn't consulted and the option had 
> no effect.  Gcc describes ftrapping-math as the default, but in llvm by 
> default floating point exceptions are masked and this corresponds to the 
> floating point Constrained Intrinsics having exception behavior set to 
> ignored.  This patch changed the llvm constructor to set the trapping bit to 
> "no trap".  In fact I'd like to get rid of the ``NoTrappingFPMath`` bitfield 
> since it's not being used, but I didn't make that change at this point. 
> 
> If I remember correctly, there are a bunch of driver tests that failed if 
> fno-trapping-math is output to cc1. I'd have to reconstruct the details.  
> Since fno-trapping-math is the default, it isn't passed through on the cc1 
> command line: the Clang.cpp driver doesn't pass through the positive and 
> negative for each existing option.
> 
> Thanks for pointing out the line in CGCall.cpp, it seems the CodeGenOpts 
> aren't getting set up perfectly I'll fix that in CompilerInvocation.cpp; I 
> don't see anything setting trapping-math as part of function level attribute, 
> @michele.scandale  did I overlook that/can you point out where that is?
I guess you are referring to the code in `TargetMachine.cpp` where the function 
level attributes are used to reset the `TargetOptions` state whenever we 
initiate the backend codegen for a given function. Considering that the 
trapping math option as stated in the documentation did not have any effect, 
I'm not surprised to see not many uses. The only one I can see is in 
`llvm/lib/Target/ARM/ARMAsmPrinter.cpp : 687` where the function level 
attribute affects the emission of some ARM specific attributes.

My only concern was that the change of the default value for trapping math was 
not propagated entirely causing this function level attribute to be initialized 
incorrectly.
Fixing the logic in `CompilerInvocation.cpp` considering the change of default 
it is fine for me.

Given that `ffp-exception-behavior={ignore,maytrap,strict}` supersedes 
`-f{,no-}trapping-math` I would expect long term to see the internal state of 
the compiler frontend to only care about the new state `FPExceptionBehavior` 
for both language and code generation options. And I guess the same would apply 
to the backend stage as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 230197.
arsenm added a comment.

Rebase


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

https://reviews.llvm.org/D59321

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/test/CodeGenOpenCL/amdgpu-debug-info-pointer-address-space.cl
  clang/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
  clang/test/Driver/Inputs/rocm-device-libs/lib/irif.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/ockl.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_off.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_803.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_900.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/ocml.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/opencl.amdgcn.bc
  clang/test/Driver/amdgpu-visibility.cl
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-device-libs.cl
  clang/test/Driver/rocm-not-found.cl

Index: clang/test/Driver/rocm-not-found.cl
===
--- /dev/null
+++ clang/test/Driver/rocm-not-found.cl
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+
+// Check that we raise an error if we're trying to compile OpenCL for amdhsa code but can't
+// find a ROCm install, unless -nodefaultlibs was passed.
+
+// RUN: %clang -### --sysroot=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// RUN: %clang -### --rocm-path=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// ERR: cannot find ROCm installation. Provide its path via --rocm-path, or pass -nodefaultlibs.
+
+// RUN: %clang -### -nodefaultlibs --rocm-path=%s/no-rocm-there %s 2>&1 | FileCheck %s --check-prefix OK
+// OK-NOT: cannot find ROCm installation.
Index: clang/test/Driver/rocm-device-libs.cl
===
--- /dev/null
+++ clang/test/Driver/rocm-device-libs.cl
@@ -0,0 +1,121 @@
+// REQUIRES: clang-driver
+// REQUIRES: amdgpu-registered-target
+
+// Test flush-denormals-to-zero enabled uses oclc_daz_opt_on
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900 %s
+
+
+
+// Make sure the different denormal default is respected for gfx8
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803 %s
+
+
+
+// Make sure the non-canonical name works
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=fiji \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   -cl-denorms-are-zero \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DAZ,GFX900 %s
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-denorms-are-zero \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DAZ,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-finite-math-only \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-FINITE-ONLY,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa\
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-fp32-correctly-rounded-divide-sqrt \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck 

[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm marked an inline comment as done.
arsenm added inline comments.



Comment at: lib/Driver/ToolChains/AMDGPU.h:25
+/// TODO: Generalize to handle libclc.
+class RocmInstallationDetector {
+private:

arsenm wrote:
> yaxunl wrote:
> > I don't think we should detect ROCm installation here. We are compiling 
> > code for amdgpu not only on ROCm, but also on amdgpu-pro and windows. Many 
> > cases, people want to compile code for amdgpu on systems without ROCm 
> > installed.
> > 
> > Compiilng code for amdgpu does not really depend on ROCm. We only depend on 
> > device library.
> > 
> > It makes more sense to have a AMDGPUDeviceLibDetector which works on ROCm, 
> > amdgpu-pro, and windows. Also we need an option -amdgpu-device-lib-path to 
> > override the default amdgpu device lib path.
> Being able to cross compile without a rocm installation is one of my primary 
> goals here. I don't know what the install paths look like for those other 
> drivers. It would be best if everything could agree on a single default 
> install location for these to search, which is mostly a naming problem. In 
> the patch as-is, the rocm-path is serving as -amdgpu-device-lib-path, but 
> this is a naming question
Except we don't have an omni-purpose bitcode library set. It's the 
ROCm-Device-Libs. rocm is the only semi-coherently defined platform at the 
moment


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

https://reviews.llvm.org/D59321



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


[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX added a comment.

In D70111#1751981 , @aprantl wrote:

> There are two options here:
>
> - leave the C bindings as is (fine with me)
> - add an overloaded function to the C bindings that has the extra argument 
> (also fine with me).


In my opinon, we should be doing both of these. Off-course Step 1 first and 
subsequently Step 2.
Otherwise consumers using / utilizing C bindings will again try to revert this, 
If we don't do Step 2.
As without an updated binding and backward compaitibilty -- this will break 
things for C-binding users ??


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX added a comment.

Some how, this{eric response} doesn't got it's way to phabricator, anyways FYI

> The C bindings, in general, don't have stability guarantees outside of a few 
> cases.



> Whitequark owns all of this now though :)



> -eric


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 230195.
nridge marked 4 inline comments as done.
nridge added a comment.

Address nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -140,7 +140,7 @@
   }
   for (auto  : ExpectedLines)
 ExpectedLinePairHighlighting.push_back(
-{LineTokens.first, LineTokens.second});
+{LineTokens.first, LineTokens.second, /*IsInactive = */ false});
 
   std::vector ActualDiffed =
   diffHighlightings(NewTokens, OldTokens);
@@ -493,11 +493,11 @@
 
   #define $Macro[[test]]
   #undef $Macro[[test]]
-  #ifdef $Macro[[test]]
-  #endif
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  #endif
 
-  #if defined($Macro[[test]])
-  #endif
+$InactiveCode[[]]  #if defined($Macro[[test]])
+$InactiveCode[[]]  #endif
 )cpp",
   R"cpp(
   struct $Class[[S]] {
@@ -598,6 +598,33 @@
 $Class[[Foo]]<$TemplateParameter[[TT]], $TemplateParameter[[TTs]]...>
   *$Field[[t]];
   }
+)cpp",
+  // Inactive code highlighting
+  R"cpp(
+  // Code in the preamble.
+  // Inactive lines get an empty InactiveCode token at the beginning.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  #endif
+
+  // A declaration to cause the preamble to end.
+  int $Variable[[EndPreamble]];
+
+  // Code after the preamble.
+  // Code inside inactive blocks does not get regular highlightings
+  // because it's not part of the AST.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive2;
+$InactiveCode[[]]  #endif
+
+  #ifndef $Macro[[test]]
+  int $Variable[[Active1]];
+  #endif
+
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive3;
+$InactiveCode[[]]  #else
+  int $Variable[[Active2]];
+  #endif
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
@@ -665,10 +692,12 @@
{{HighlightingKind::Variable,
  Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
 {HighlightingKind::Function,
- Range{CreatePosition(3, 4), CreatePosition(3, 7),
+ Range{CreatePosition(3, 4), CreatePosition(3, 7)}}},
+   /* IsInactive = */ false},
   {1,
{{HighlightingKind::Variable,
- Range{CreatePosition(1, 1), CreatePosition(1, 5)};
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)}}},
+   /* IsInactive = */ true}};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -57,6 +57,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
@@ -66,6 +69,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -81,10 +85,12 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -100,6 +106,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -115,6 +122,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": ""
 # CHECK-NEXT:

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-19 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  auto  = DiffedLines.back();
+  for (auto Iter = AddedLine.Tokens.begin();
+   Iter != AddedLine.Tokens.end();) {

hokein wrote:
> nridge wrote:
> > hokein wrote:
> > > it took me a while to understand this code,
> > > 
> > > If the NewLine is `IsInactive`, it just contains a single token whose 
> > > range is [0, 0), can't we just?
> > > 
> > > ```
> > > 
> > > if (NewLine.back().Tokens.empty()) continue;
> > > 
> > > bool InactiveLine = NewLine.back().Tokens.front().Kind == InactiveCode;
> > > assert(InactiveLine && NewLine.back().Tokens.size() == 1 && "IncativeCode 
> > > must have a single token");
> > > DiffedLines.back().IsInactive = true;
> > > ```
> > An inactive line can contain token highlightings as well. For example, we 
> > highlight macro references in the condition of an `#ifdef`, and that line 
> > is also inactive if the condition is false. Clients can merge the line 
> > style (which is typically a background color) with the token styles 
> > (typically a foreground color).
> > 
> > I did expand the comment to explain what the loop is doing more clearly.
> thanks, I see. 
> 
> I think we can still simplify the code like below, this could improve the 
> code readability, and avoid the comment explaining it.
> 
> ```
> llvm::erase_if(AddedLine.Tokens, [](const Token& T) { return T.Kind == 
> InactiveCode;});
> ```
Done. Note that we still need to set `AddedLine.IsInactive` appropriately. I 
did that in the lambda, which feels like a strange thing for an `erase_if` 
predicate to do. The alternative would be doing a `count_if` first (but then 
we're looping over the tokens twice).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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


[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Ben D. Jones via Phabricator via cfe-commits
bendjones added a comment.

In D70284#1752811 , @dexonsmith wrote:

> For some reason this revision is locked down and I'm not allowed to "edit" 
> it, which includes adding inline review comments.  Can you add me as a 
> reviewer?


Thought I did.

> The two comments:
> 
> - Please add a period at the end of the sentence in the comment.

Will do.

> - Can you give more context about what `objc_arc_inert` is doing, and why 
> it's necessary now that `no_dead_strip` is gone?

The objc_arc_inert was added to other similar things so in the spirt of making 
things match I added it here. I can keep it simple though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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


LLVM buildmaster will be updated and restarted tonight

2019-11-19 Thread Galina Kistanova via cfe-commits
 Hello everyone,

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

Thanks

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


[PATCH] D70477: [Clang] Enable RISC-V support for Fuchsia

2019-11-19 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added a reviewer: leonardchan.
Herald added subscribers: cfe-commits, apazos, sameer.abuasal, pzheng, 
s.egerton, lenary, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, mgorny.
Herald added a project: clang.

We don't have a full sysroot yet, so for now we only include compiler
support and compiler-rt builtins, the rest of the runtimes will get
enabled later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70477

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Basic/Targets.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/riscv64-fuchsia/libclang_rt.builtins.a
  clang/test/Driver/fuchsia.c
  clang/test/Driver/fuchsia.cpp

Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -1,9 +1,22 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes --target=x86_64-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 | FileCheck %s
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes --target=aarch64-fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes --target=riscv64-fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
-// CHECK: "-triple" "x86_64-fuchsia"
+// CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
+// CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
+// CHECK-RISCV64: "-triple" "riscv64-unknown-fuchsia"
 // CHECK: "-fuse-init-array"
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
@@ -23,7 +36,9 @@
 // CHECK: "-lc++"
 // CHECK: "-lm"
 // CHECK: "--pop-state"
-// CHECK: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-X86_64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-RISCV64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}riscv64-fuchsia{{/|}}libclang_rt.builtins.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
 // CHECK-NOT: crtn.o
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -6,7 +6,14 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: %clang %s -### -no-canonical-prefixes --target=riscv64-fuchsia \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
+// CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
+// CHECK-RISCV64: "-triple" "riscv64-unknown-fuchsia"
 // CHECK: "--mrelax-relocations"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
@@ -29,6 +36,7 @@
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
 // CHECK-X86_64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.builtins.a"
 // CHECK-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.builtins.a"
+// CHECK-RISCV64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}riscv64-fuchsia{{/|}}libclang_rt.builtins.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
 // CHECK-NOT: crtn.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -224,7 +224,7 @@
 std::string Fuchsia::ComputeEffectiveClangTriple(const ArgList ,
  types::ID InputType) const {
   llvm::Triple Triple(ComputeLLVMTriple(Args, InputType));
-  return (Triple.getArchName() + "-" + Triple.getOSName()).str();
+  

[clang] 6c6d348 - Revert "[clang][IFS] Fixing unsupported emulation mode on clang-ppc64be-linux bot."

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T21:59:22-05:00
New Revision: 6c6d34883a3003693cbf0ab3edf19eb999c1a62d

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

LOG: Revert "[clang][IFS] Fixing unsupported emulation mode on 
clang-ppc64be-linux bot."

This reverts commit 1b387484b9b38a4a1e98a9d22a9a26065b0d184e.

Added: 


Modified: 
clang/test/InterfaceStubs/driver-test.c

Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
index 425d87271f33..62d522fb7b1f 100644
--- a/clang/test/InterfaceStubs/driver-test.c
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -1,5 +1,4 @@
 // REQUIRES: x86-registered-target
-// REQUIRES: !powerpc-registered-target
 // REQUIRES: !system-darwin && !system-windows
 
 // RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \



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


[clang] 27b229d - Revert "[clang][IFS][test] Removing driver-test.c. Test is still too brittle."

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T21:59:10-05:00
New Revision: 27b229dc17b2ea1d06fe566df8631bb2fff7b1c8

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

LOG: Revert "[clang][IFS][test] Removing driver-test.c. Test is still too 
brittle."

This reverts commit f37356d6f60ae5db978611621d3a375ed87ec0f0.

Added: 
clang/test/InterfaceStubs/driver-test.c

Modified: 


Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
new file mode 100644
index ..425d87271f33
--- /dev/null
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -0,0 +1,15 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: !powerpc-registered-target
+// REQUIRES: !system-darwin && !system-windows
+
+// RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \
+// RUN:   -emit-interface-stubs -emit-merged-ifs %s %S/object.c %S/weak.cpp
+// RUN: cat %t1.ifs | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }



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


[clang] f37356d - [clang][IFS][test] Removing driver-test.c. Test is still too brittle.

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T21:42:17-05:00
New Revision: f37356d6f60ae5db978611621d3a375ed87ec0f0

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

LOG: [clang][IFS][test] Removing driver-test.c. Test is still too brittle.

Removing this test because if I add a triple then there are link falures
on targets like ppc and s390x. If I don't add a triple then on PS4
targets the clang driver tries to invoke orbis-ld which ends up being
not found.

Added: 


Modified: 


Removed: 
clang/test/InterfaceStubs/driver-test.c



diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
deleted file mode 100644
index 425d87271f33..
--- a/clang/test/InterfaceStubs/driver-test.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// REQUIRES: x86-registered-target
-// REQUIRES: !powerpc-registered-target
-// REQUIRES: !system-darwin && !system-windows
-
-// RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \
-// RUN:   -emit-interface-stubs -emit-merged-ifs %s %S/object.c %S/weak.cpp
-// RUN: cat %t1.ifs | FileCheck %s
-
-// CHECK-DAG: data
-// CHECK-DAG: foo
-// CHECK-DAG: strongFunc
-// CHECK-DAG: weakFunc
-
-int foo(int bar) { return 42 + 1844; }
-int main() { return foo(23); }



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


Re: [PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Akira Hatanaka via cfe-commits
Can you check the `#0` at the end of the globals and other strings that precede 
that? If you do so, we can also check that `no_dead_strip` isn’t added.

> On Nov 19, 2019, at 6:05 PM, Duncan P. N. Exon Smith via Phabricator via 
> cfe-commits  wrote:
> 
> dexonsmith added a comment.
> 
> For some reason this revision is locked down and I'm not allowed to "edit" 
> it, which includes adding inline review comments.  Can you add me as a 
> reviewer?
> 
> The two comments:
> 
> - Please add a period at the end of the sentence in the comment.
> - Can you give more context about what `objc_arc_inert` is doing, and why 
> it's necessary now that `no_dead_strip` is gone?
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D70284/new/
> 
> https://reviews.llvm.org/D70284
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {

xazax.hun wrote:
> NoQ wrote:
> > This is never necessary, just call all the methods directly on `QT` - it 
> > has an overloaded operator `->()` for this.
> `getPointeeOrArrayElementType` returns a `Type *`, so I cannot really 
> continue to use `QualType` and I am not interested in the qualifiers at all.
Whoops ^.^"



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+++PtrToHandleLevel;

xazax.hun wrote:
> NoQ wrote:
> > Ok, so what's the deal with arrays here? If the function receives an array 
> > of handles, do you ultimately plan to return multiple symbols - one for 
> > each element of the array?
> > 
> > Also, in the generic non-Fuchsia case, what if the handle itself is a 
> > pointer?
> I do not have a real plan with arrays just yet. Creating eagerly a symbol for 
> all the elements might look a bit wasteful but also users are probably not 
> expected to have large arrays of handles? Probably I should remove that code 
> for now.
> 
> I would expect non-Fuchsia checkers to introduce their own `getHandleSymbol` 
> logic. Does that make sense? Maybe I should rename this to 
> `getFuchsiaHandleSymbol`. 
But, i mean, why do you even consider arrays in this code? Why not simply `QT = 
QT->getPointeeType()`?


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

https://reviews.llvm.org/D70470



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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 230187.
xazax.hun added a comment.

- Addressed **some** of the review comments. More changes are planned :)


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

https://reviews.llvm.org/D70470

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.c

Index: clang/test/Analysis/fuchsia_handle.c
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_handle.c
@@ -0,0 +1,114 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.FuchsiaHandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define XZ_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define XZ_HANDLE_RELEASE  __attribute__((release_handle))
+#define XZ_HANDLE_USE  __attribute__((use_handle))
+#else
+#define XZ_HANDLE_ACQUIRE
+#define XZ_HANDLE_RELEASE
+#define XZ_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+XZ_HANDLE_ACQUIRE zx_handle_t* out0,
+zx_handle_t* out1 XZ_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+zx_handle_t handle XZ_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+
+void use1(const zx_handle_t *in XZ_HANDLE_USE);
+void use2(zx_handle_t in XZ_HANDLE_USE);
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++)
+zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkNoLeak07(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  if (tag) {
+escape1();
+escape2(sb);
+  }
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak08(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  use1();
+  if (tag)
+zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag)
+zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  use1(); // expected-warning {{Using a previously released handle}}
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -0,0 +1,436 @@
+//=== FuchsiaHandleChecker.cpp - Find handle leaks/double closes -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This checker checks if the handle of Fuchsia is properly used according to
+// following rules.
+//   - If a handle is acquired, it should be released before execution
+//ends.
+//   - If a handle is released, it should not be released again.
+//   - If a handle is released, it should not be used for other purposes
+//such as I/O.
+//
+// In this checker, each tracked handle is associated with a state. When the
+// handle variable is passed to different function calls or syscalls, its state
+// changes. The state changes can be generally represented by following ASCII
+// Art:
+//
+//+---+
+//|   |
+//|release_func failed|
+//|   +-+ |
+//|   | |  

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

2019-11-19 Thread Tim Wojtulewicz via Phabricator via cfe-commits
timwoj added a comment.

Thanks for the consideration on committer access, but I'm going to have to pass 
for the time being.

That said, can someone land this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346



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


[clang] 8700831 - clang/Modules: Early return in CompilerInstance::createModuleManager, NFC

2019-11-19 Thread Duncan P. N. Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2019-11-19T18:16:23-08:00
New Revision: 8700831734811cb89eafb72b75206f21e9f047e9

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

LOG: clang/Modules: Early return in CompilerInstance::createModuleManager, NFC

Reduce nesting with an early `return`.

Added: 


Modified: 
clang/lib/Frontend/CompilerInstance.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index a0663217453a..c9c66f011193 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1472,51 +1472,52 @@ static void pruneModuleCache(const HeaderSearchOptions 
) {
 }
 
 void CompilerInstance::createModuleManager() {
-  if (!ModuleManager) {
-if (!hasASTContext())
-  createASTContext();
-
-// If we're implicitly building modules but not currently recursively
-// building a module, check whether we need to prune the module cache.
-if (getSourceManager().getModuleBuildStack().empty() &&
-!getPreprocessor().getHeaderSearchInfo().getModuleCachePath().empty() 
&&
-getHeaderSearchOpts().ModuleCachePruneInterval > 0 &&
-getHeaderSearchOpts().ModuleCachePruneAfter > 0) {
-  pruneModuleCache(getHeaderSearchOpts());
-}
+  if (ModuleManager)
+return;
 
-HeaderSearchOptions  = getHeaderSearchOpts();
-std::string Sysroot = HSOpts.Sysroot;
-const PreprocessorOptions  = getPreprocessorOpts();
-std::unique_ptr ReadTimer;
-if (FrontendTimerGroup)
-  ReadTimer = std::make_unique("reading_modules",
- "Reading modules",
- *FrontendTimerGroup);
-ModuleManager = new ASTReader(
-getPreprocessor(), getModuleCache(), (),
-getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions,
-Sysroot.empty() ? "" : Sysroot.c_str(), PPOpts.DisablePCHValidation,
-/*AllowASTWithCompilerErrors=*/false,
-/*AllowConfigurationMismatch=*/false,
-HSOpts.ModulesValidateSystemHeaders,
-HSOpts.ValidateASTInputFilesContent,
-getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
-if (hasASTConsumer()) {
-  ModuleManager->setDeserializationListener(
-getASTConsumer().GetASTDeserializationListener());
-  getASTContext().setASTMutationListener(
-getASTConsumer().GetASTMutationListener());
-}
-getASTContext().setExternalSource(ModuleManager);
-if (hasSema())
-  ModuleManager->InitializeSema(getSema());
-if (hasASTConsumer())
-  ModuleManager->StartTranslationUnit(());
-
-for (auto  : DependencyCollectors)
-  Listener->attachToASTReader(*ModuleManager);
+  if (!hasASTContext())
+createASTContext();
+
+  // If we're implicitly building modules but not currently recursively
+  // building a module, check whether we need to prune the module cache.
+  if (getSourceManager().getModuleBuildStack().empty() &&
+  !getPreprocessor().getHeaderSearchInfo().getModuleCachePath().empty() &&
+  getHeaderSearchOpts().ModuleCachePruneInterval > 0 &&
+  getHeaderSearchOpts().ModuleCachePruneAfter > 0) {
+pruneModuleCache(getHeaderSearchOpts());
   }
+
+  HeaderSearchOptions  = getHeaderSearchOpts();
+  std::string Sysroot = HSOpts.Sysroot;
+  const PreprocessorOptions  = getPreprocessorOpts();
+  std::unique_ptr ReadTimer;
+  if (FrontendTimerGroup)
+ReadTimer = std::make_unique("reading_modules",
+"Reading modules",
+*FrontendTimerGroup);
+  ModuleManager = new ASTReader(
+  getPreprocessor(), getModuleCache(), (),
+  getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions,
+  Sysroot.empty() ? "" : Sysroot.c_str(), PPOpts.DisablePCHValidation,
+  /*AllowASTWithCompilerErrors=*/false,
+  /*AllowConfigurationMismatch=*/false,
+  HSOpts.ModulesValidateSystemHeaders,
+  HSOpts.ValidateASTInputFilesContent,
+  getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
+  if (hasASTConsumer()) {
+ModuleManager->setDeserializationListener(
+  getASTConsumer().GetASTDeserializationListener());
+getASTContext().setASTMutationListener(
+  getASTConsumer().GetASTMutationListener());
+  }
+  getASTContext().setExternalSource(ModuleManager);
+  if (hasSema())
+ModuleManager->InitializeSema(getSema());
+  if (hasASTConsumer())
+ModuleManager->StartTranslationUnit(());
+
+  for (auto  : DependencyCollectors)
+Listener->attachToASTReader(*ModuleManager);
 }
 
 bool CompilerInstance::loadModuleFile(StringRef FileName) {




[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 13 inline comments as done.
xazax.hun added a comment.

Thanks for the review!

I am not very well versed in Fuchsia's syscalls yet but my understanding is 
that not all of the syscalls can fail so we do not need all the users to check 
for errors. But this is something I will verify with the rest of the team, so 
please treat my answer with a grain of salt for now. An alternative would be to 
introduce an annotation to tell which APIs will never fail, but I am afraid 
that is also sometimes subjective. For example it is pretty much possible to 
have no available port in a system but is very unlikely to happen so some non 
mission critical applications might not check for errors there. But this is 
also something that I will follow up with the team.




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:99-100
+
+static const StringRef HandleTypeName = "zx_handle_t";
+static const StringRef ErrorTypeName = "zx_status_t";
+

NoQ wrote:
> So you plan to unhardcode these as well eventually?
I have no specific plans at this point. I do not see these types changing 
frequently, so I have no urge to unhardcode them. Are there any downsides I am 
not being aware of?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:123
+
+  // Only use this in checkDeadSymbols!
+  bool hasError(ProgramStateRef State) const {

NoQ wrote:
> For now it doesn't seem to be used at all.
Wow, indeed! This is from an eariler iteration :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:141-146
+#define CASE(ID)   
\
+  case ID: 
\
+OS << #ID; 
\
+break;
+  CASE(Kind::Allocated)
+  CASE(Kind::Released)

NoQ wrote:
> This macro has literally saved exactly as many lines as it has caused :)
Good point! Initially I had a separate escaped state, but now I guess I should 
just expand them :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:156-168
+  std::unique_ptr LeakBugType;
+  std::unique_ptr DoubleReleaseBugType;
+  std::unique_ptr UseAfterFreeBugType;
+
+public:
+  FuchsiaHandleChecker()
+  : LeakBugType(new BugType(this, "Fuchsia handle leak",

NoQ wrote:
> We've recently dumbed down this idiom to
> ```lang=c++
> class FuchsiaHandleChecker : ... {
>   LeakBugType{this, "Fuchsia handle leak",
>   "Fuchsia Handle Error", /*SuppressOnSink=*/true};
>   ...
> };
> ```
> :)
+1 :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {

NoQ wrote:
> This is never necessary, just call all the methods directly on `QT` - it has 
> an overloaded operator `->()` for this.
`getPointeeOrArrayElementType` returns a `Type *`, so I cannot really continue 
to use `QualType` and I am not interested in the qualifiers at all.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+++PtrToHandleLevel;

NoQ wrote:
> Ok, so what's the deal with arrays here? If the function receives an array of 
> handles, do you ultimately plan to return multiple symbols - one for each 
> element of the array?
> 
> Also, in the generic non-Fuchsia case, what if the handle itself is a pointer?
I do not have a real plan with arrays just yet. Creating eagerly a symbol for 
all the elements might look a bit wasteful but also users are probably not 
expected to have large arrays of handles? Probably I should remove that code 
for now.

I would expect non-Fuchsia checkers to introduce their own `getHandleSymbol` 
logic. Does that make sense? Maybe I should rename this to 
`getFuchsiaHandleSymbol`. 



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:302-303
+   Pred);
+// Avoid further reports on this symbol.
+State = State->remove(HandleSym);
+  } else

NoQ wrote:
> Why not generate a fatal error instead?
The motivation was to not to reduce coverage, but I have no strong feelings 
about making this a sink. But in case of leaks I certainly prefer non-fatal 
errors.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:336-338
+ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+ bool Assumption) 

[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

For some reason this revision is locked down and I'm not allowed to "edit" it, 
which includes adding inline review comments.  Can you add me as a reviewer?

The two comments:

- Please add a period at the end of the sentence in the comment.
- Can you give more context about what `objc_arc_inert` is doing, and why it's 
necessary now that `no_dead_strip` is gone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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


[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

2019-11-19 Thread Ben D. Jones via Phabricator via cfe-commits
bendjones added a comment.

Any additional thoughts @dexonsmith @erik.pilkington @ahatanak?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Yay, i see the "Schrödinger handle" pattern: until the return value of release 
is checked, the handle is believed to be both dead and alive at the same time.

We usually use this pattern because a lot of code that's already written 
doesn't check their return values. But do you really need this for a brand-new 
API? Maybe aggressively assume that the release may always fail and fix all the 
places in your code where the return value is not checked before use?




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:99-100
+
+static const StringRef HandleTypeName = "zx_handle_t";
+static const StringRef ErrorTypeName = "zx_status_t";
+

So you plan to unhardcode these as well eventually?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:123
+
+  // Only use this in checkDeadSymbols!
+  bool hasError(ProgramStateRef State) const {

For now it doesn't seem to be used at all.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:141-146
+#define CASE(ID)   
\
+  case ID: 
\
+OS << #ID; 
\
+break;
+  CASE(Kind::Allocated)
+  CASE(Kind::Released)

This macro has literally saved exactly as many lines as it has caused :)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:156-168
+  std::unique_ptr LeakBugType;
+  std::unique_ptr DoubleReleaseBugType;
+  std::unique_ptr UseAfterFreeBugType;
+
+public:
+  FuchsiaHandleChecker()
+  : LeakBugType(new BugType(this, "Fuchsia handle leak",

We've recently dumbed down this idiom to
```lang=c++
class FuchsiaHandleChecker : ... {
  LeakBugType{this, "Fuchsia handle leak",
  "Fuchsia Handle Error", /*SuppressOnSink=*/true};
  ...
};
```
:)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:207
+  int PtrToHandleLevel = 0;
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {

This is never necessary, just call all the methods directly on `QT` - it has an 
overloaded operator `->()` for this.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:208
+  const Type *T = QT.getTypePtr();
+  while (T->isAnyPointerType() || T->isReferenceType() || T->isArrayType()) {
+++PtrToHandleLevel;

Ok, so what's the deal with arrays here? If the function receives an array of 
handles, do you ultimately plan to return multiple symbols - one for each 
element of the array?

Also, in the generic non-Fuchsia case, what if the handle itself is a pointer?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:212
+  }
+  if (const auto *HandleType = dyn_cast(T)) {
+if (HandleType->getDecl()->getName() != HandleTypeName)

Here `QT->getAs()` respectively.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:302-303
+   Pred);
+// Avoid further reports on this symbol.
+State = State->remove(HandleSym);
+  } else

Why not generate a fatal error instead?



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:336-338
+ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State,
+ SVal Cond,
+ bool Assumption) const {

Ok, so you're basically saying that whenever the code under analysis does 
anything to check the return value, this callback will fire immediately? Thus 
guaranteeing that whenever the Schrödinger state is accessed, it's either 
already collapsed or indeed not checked yet?

This sounds right but i wish i had your confidence :D

My usual approach to Schrödinger states will be to do the same thing but in 
`checkDeadSymbols()`. I.e., when the return value symbol dies, see if it's 
constrained and collapse the state accordingly. If the access occurs earlier 
than symbol death, update the state according to the current constraints and 
forget about the symbol. If current constraints are insufficient, 
conservatively mark it as a successful release (as that's how non-paranoid code 
under analysis would behave).

Regardless of the approach, you still need to update the state upon access as 
if the release has succeeded (when the current state is a Schrödinger state). 
But with your approach you can avoid checking the constraints upon access, and 
instead blindly assume that the symbol is underconstrained.

I like this! Can we prove that it works by adding an 

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

NoQ wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > The next obvious step here would be to type-check the declaration to make 
> > > sure that it's actually a handle (and emit a warning if it isn't).
> > Yeah, I do agree, but I think this depends on the role of the attribute. 
> > Adding a type check would make this more restrictive in a sense other users 
> > who want to write for example a Posix API checker and want to reuse this 
> > attribute might not be able to do so without touching the type-check code. 
> > Which may or may not be good. I do not have any strong feelings about 
> > either of the directions. 
> So you envision this working on completely arbitrary types? Fair!
> 
> Maybe check that when `AcquireHandleAttr` is on a parameter, it's actually an 
> out-parameter?
Hmm, I think this makes a lot of sense! So basically, I would exclude 
primitive, non-pointer types that are taken by value. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

xazax.hun wrote:
> NoQ wrote:
> > The next obvious step here would be to type-check the declaration to make 
> > sure that it's actually a handle (and emit a warning if it isn't).
> Yeah, I do agree, but I think this depends on the role of the attribute. 
> Adding a type check would make this more restrictive in a sense other users 
> who want to write for example a Posix API checker and want to reuse this 
> attribute might not be able to do so without touching the type-check code. 
> Which may or may not be good. I do not have any strong feelings about either 
> of the directions. 
So you envision this working on completely arbitrary types? Fair!

Maybe check that when `AcquireHandleAttr` is on a parameter, it's actually an 
out-parameter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[clang] 1b38748 - [clang][IFS] Fixing unsupported emulation mode on clang-ppc64be-linux bot.

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T19:27:46-05:00
New Revision: 1b387484b9b38a4a1e98a9d22a9a26065b0d184e

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

LOG: [clang][IFS] Fixing unsupported emulation mode on clang-ppc64be-linux bot.

I am in another pickle here where if I specify a triple, I get the wrong elf
target arch on the PPC bot (error from the PPC elf Linker). To avoid this I am
going to turn this test off on the PPC bots for now.

Added: 


Modified: 
clang/test/InterfaceStubs/driver-test.c

Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
index 62d522fb7b1f..425d87271f33 100644
--- a/clang/test/InterfaceStubs/driver-test.c
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -1,4 +1,5 @@
 // REQUIRES: x86-registered-target
+// REQUIRES: !powerpc-registered-target
 // REQUIRES: !system-darwin && !system-windows
 
 // RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

NoQ wrote:
> The next obvious step here would be to type-check the declaration to make 
> sure that it's actually a handle (and emit a warning if it isn't).
Yeah, I do agree, but I think this depends on the role of the attribute. Adding 
a type check would make this more restrictive in a sense other users who want 
to write for example a Posix API checker and want to reuse this attribute might 
not be able to do so without touching the type-check code. Which may or may not 
be good. I do not have any strong feelings about either of the directions. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[clang] 69242e9 - clang/Modules: Sink ASTReadResult in ReadControlBlock, NFC

2019-11-19 Thread Duncan P. N. Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2019-11-19T16:10:44-08:00
New Revision: 69242e986823e3fdd11a8e51f47f36bec714363c

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

LOG: clang/Modules: Sink ASTReadResult in ReadControlBlock, NFC

Simplify the code by avoiding some state that wasn't being used.  The
function-level `Result` was only assigned a value other than `Success`
in the handler for `OPTIONS_BLOCK_ID`, but in that case it also hits an
early return.  Remove it at the function-level to make it obvious that
the normal case always returns `Success`.

Added: 


Modified: 
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 9111b60a7179..36c2952346a7 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2561,7 +2561,6 @@ ASTReader::ReadControlBlock(ModuleFile ,
 const ModuleFile *ImportedBy,
 unsigned ClientLoadCapabilities) {
   BitstreamCursor  = F.Stream;
-  ASTReadResult Result = Success;
 
   if (llvm::Error Err = Stream.EnterSubBlock(CONTROL_BLOCK_ID)) {
 Error(std::move(Err));
@@ -2652,7 +2651,7 @@ ASTReader::ReadControlBlock(ModuleFile ,
 }
   }
 
-  return Result;
+  return Success;
 }
 
 case llvm::BitstreamEntry::SubBlock:
@@ -2682,9 +2681,10 @@ ASTReader::ReadControlBlock(ModuleFile ,
   bool AllowCompatibleConfigurationMismatch =
   F.Kind == MK_ExplicitModule || F.Kind == MK_PrebuiltModule;
 
-  Result = ReadOptionsBlock(Stream, ClientLoadCapabilities,
-AllowCompatibleConfigurationMismatch,
-*Listener, SuggestedPredefines);
+  ASTReadResult Result =
+  ReadOptionsBlock(Stream, ClientLoadCapabilities,
+   AllowCompatibleConfigurationMismatch, *Listener,
+   SuggestedPredefines);
   if (Result == Failure) {
 Error("malformed block record in AST file");
 return Result;



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;

The next obvious step here would be to type-check the declaration to make sure 
that it's actually a handle (and emit a warning if it isn't).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70469



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


[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:279
+if (PredNode->getState() == ErrorNode->getState()) {
+  IsFalsePositiveFound = true;
+  PR->markInvalid(nullptr, nullptr);

Why is this a false positive?

You're bringing in a completely brand-new machinery here, could you explain how 
it works and why do you need it?


Repository:
  rC Clang

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

https://reviews.llvm.org/D70411



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


[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-19 Thread Zachary Turner via Phabricator via cfe-commits
zturner updated this revision to Diff 230162.
zturner added a comment.

Forgot to remove spurious `llvm::` namespace qualifications.


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

https://reviews.llvm.org/D70368

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
@@ -8,75 +8,51 @@
 template 
 bind_rt bind(Fp &&, Arguments &&...);
 }
+
+template 
+T ref(T );
 }
 
-int add(int x, int y) { return x + y; }
+namespace boost {
+template 
+class bind_rt {};
 
-void f() {
-  auto clj = std::bind(add, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
-  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
-}
+template 
+bind_rt bind(const Fp &, Arguments...);
+} // namespace boost
 
-void g() {
-  int x = 2;
-  int y = 2;
-  auto clj = std::bind(add, x, y);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
-}
+namespace C {
+int add(int x, int y) { return x + y; }
+} // namespace C
 
-struct placeholder {};
-placeholder _1;
-placeholder _2;
+struct Foo {
+  static int add(int x, int y) { return x + y; }
+};
 
-void h() {
-  int x = 2;
-  auto clj = std::bind(add, x, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
-}
+struct D {
+  D() = default;
+  void operator()(int x, int y) const {}
 
-struct A;
-struct B;
-bool ABTest(const A &, const B &);
+  void MemberFunction(int x) {}
 
-void i() {
-  auto BATest = std::bind(ABTest, _2, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
-}
+  static D *create();
+};
 
-void j() {
-  auto clj = std::bind(add, 2, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for argument mismatches.
-  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
-}
+struct F {
+  F(int x) {}
+  ~F() {}
 
-void k() {
-  auto clj = std::bind(add, _1, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for reused placeholders.
-  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
-}
+  int get() { return 42; }
+};
 
-void m() {
-  auto clj = std::bind(add, 1, add(2, 5));
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for nested calls.
-  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
-}
+void UseF(F);
 
-namespace C {
-  int add(int x, int y){ return x + y; }
-}
+struct placeholder {};
+placeholder _1;
+placeholder _2;
 
-void n() {
-  auto clj = std::bind(C::add, 1, 1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [] { return C::add(1, 1); };
-}
+int add(int x, int y) { return x + y; }
+int addThree(int x, int y, int z) { return x + y + z; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
@@ -114,10 +90,188 @@
   void Reset(std::function);
 };
 
-void test(Thing *t) {
+int GlobalVariable = 42;
+
+struct TestCaptureByValueStruct {
+  int MemberVariable;
+  static int StaticMemberVariable;
+  F MemberStruct;
+
+  void testCaptureByValue(int Param, F f) {
+int x = 3;
+int y = 4;
+auto AAA = std::bind(add, x, y);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto AAA = [x, y] { return add(x, y); };
+
+// When the captured variable is repeated, it should only appear in the capture list once.
+auto BBB = std::bind(add, x, x);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto BBB = [x] { return add(x, x); };
+
+int LocalVariable;
+// Global variables shouldn't be captured at all, and members should be captured through this.
+auto CCC = std::bind(add, MemberVariable, GlobalVariable);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto CCC = [this] { return add(MemberVariable, GlobalVariable); };
+
+// Static member variables shouldn't be captured, but locals should
+auto DDD = std::bind(add, TestCaptureByValueStruct::StaticMemberVariable, LocalVariable);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// 

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:67
+// functions:
+//  1. __attribute__((clang::acquire_handle)) |handle will be acquired
+//  2. __attribute__((clang::release_handle)) |handle will be released

Whoops, the spelling of the attribute is wrong here, will fix in a followup 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70470



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


[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:201
+std::tie(StateRetNotNull, StateRetNull) =
+CM.assumeDual(StateStreamNull, RetVal);
+if (StateRetNull) {

Mmm, wait, this doesn't look right to me. You cannot deduce from the presence 
of `freopen()` in the code that the argument may be null, so the split over the 
argument is not well-justified.

The right thing to do here will be to produce a "Schrödinger file descriptor" 
that's both `Opened` and `OpenFailed` until we observe the return value to be 
constrained to null or non-null later during analysis (cf. D32449), otherwise 
conservatively assume that the open succeeded when queried for the first time 
(because that's how non-paranoid code under analysis will behave).

Or you could drop the failed path immediately if the argument isn't known to be 
zero. That'll drop the coverage slightly but won't cause anything bad to happen.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948



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


[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-19 Thread Zachary Turner via Phabricator via cfe-commits
zturner updated this revision to Diff 230161.
zturner added a comment.

Updated with suggestions from @aaron.ballman


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

https://reviews.llvm.org/D70368

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
@@ -8,75 +8,51 @@
 template 
 bind_rt bind(Fp &&, Arguments &&...);
 }
+
+template 
+T ref(T );
 }
 
-int add(int x, int y) { return x + y; }
+namespace boost {
+template 
+class bind_rt {};
 
-void f() {
-  auto clj = std::bind(add, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind]
-  // CHECK-FIXES: auto clj = [] { return add(2, 2); };
-}
+template 
+bind_rt bind(const Fp &, Arguments...);
+} // namespace boost
 
-void g() {
-  int x = 2;
-  int y = 2;
-  auto clj = std::bind(add, x, y);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=] { return add(x, y); };
-}
+namespace C {
+int add(int x, int y) { return x + y; }
+} // namespace C
 
-struct placeholder {};
-placeholder _1;
-placeholder _2;
+struct Foo {
+  static int add(int x, int y) { return x + y; }
+};
 
-void h() {
-  int x = 2;
-  auto clj = std::bind(add, x, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
-}
+struct D {
+  D() = default;
+  void operator()(int x, int y) const {}
 
-struct A;
-struct B;
-bool ABTest(const A &, const B &);
+  void MemberFunction(int x) {}
 
-void i() {
-  auto BATest = std::bind(ABTest, _2, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
-}
+  static D *create();
+};
 
-void j() {
-  auto clj = std::bind(add, 2, 2, 2);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for argument mismatches.
-  // CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
-}
+struct F {
+  F(int x) {}
+  ~F() {}
 
-void k() {
-  auto clj = std::bind(add, _1, _1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for reused placeholders.
-  // CHECK-FIXES: auto clj = std::bind(add, _1, _1);
-}
+  int get() { return 42; }
+};
 
-void m() {
-  auto clj = std::bind(add, 1, add(2, 5));
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // No fix is applied for nested calls.
-  // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
-}
+void UseF(F);
 
-namespace C {
-  int add(int x, int y){ return x + y; }
-}
+struct placeholder {};
+placeholder _1;
+placeholder _2;
 
-void n() {
-  auto clj = std::bind(C::add, 1, 1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto clj = [] { return C::add(1, 1); };
-}
+int add(int x, int y) { return x + y; }
+int addThree(int x, int y, int z) { return x + y + z; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
@@ -114,10 +90,188 @@
   void Reset(std::function);
 };
 
-void test(Thing *t) {
+int GlobalVariable = 42;
+
+struct TestCaptureByValueStruct {
+  int MemberVariable;
+  static int StaticMemberVariable;
+  F MemberStruct;
+
+  void testCaptureByValue(int Param, F f) {
+int x = 3;
+int y = 4;
+auto AAA = std::bind(add, x, y);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto AAA = [x, y] { return add(x, y); };
+
+// When the captured variable is repeated, it should only appear in the capture list once.
+auto BBB = std::bind(add, x, x);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto BBB = [x] { return add(x, x); };
+
+int LocalVariable;
+// Global variables shouldn't be captured at all, and members should be captured through this.
+auto CCC = std::bind(add, MemberVariable, GlobalVariable);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto CCC = [this] { return add(MemberVariable, GlobalVariable); };
+
+// Static member variables shouldn't be captured, but locals should
+auto DDD = std::bind(add, TestCaptureByValueStruct::StaticMemberVariable, LocalVariable);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind]
+// CHECK-FIXES: auto DDD 

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision.
xazax.hun added reviewers: aaron.ballman, NoQ.
xazax.hun added a project: clang.
Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.

These are three new attributes that I plan to use in a Fuchsia specific check. 
I did not give them Fuchsia specific names as they might be useful for other 
APIs such as Posix as well. In case that is a concern I can rename them to be 
more specific to the check.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70469

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9,6 +9,7 @@
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
+// CHECK-NEXT: AcquireHandle (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7282,6 +7282,16 @@
   case ParsedAttr::AT_ArmMveAlias:
 handleArmMveAliasAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_AcquireHandle:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_ReleaseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_UseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
   }
 }
 
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4467,3 +4467,50 @@
   }
   }];
 }
+
+def AcquireHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed
+to fill the corresponding argument with a new handle.
+
+.. code-block:: c++
+
+  // Output arguments from Zircon.
+  zx_status_t zx_socket_create(uint32_t options,
+   zx_handle_t* out0 [[clang::acquire_handle]],
+   zx_handle_t* out1 [[clang::acquire_handle]]);
+
+
+  // Returned handle.
+  [[clang::acquire_handle]] int open(const char *path, int oflag, ... );
+  }];
+}
+
+def UseHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+A function taking a handle by value might close the handle. If a function is
+annotated with `use_handle` it is assumed to not to change the state of the
+handle. It is also assumed to require an open handle to work with.
+
+.. code-block:: c++
+
+  zx_status_t zx_port_wait(zx_handle_t handle [[clang::use_handle]],
+   zx_time_t deadline,
+   zx_port_packet_t* packet);
+  }];
+}
+
+def ReleaseHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+If a function is annotated with `release_handle` it is assumed to close the handle.
+It is also assumed to require an open handle to work with.
+
+.. code-block:: c++
+
+  zx_status_t zx_handle_close(zx_handle_t handle [[clang::release_handle]]);
+  }];
+}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3439,3 +3439,19 @@
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoBuiltinDocs];
 }
+
+def AcquireHandle : InheritableAttr {
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];
+}
+
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];
+}
+
+def ReleaseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"release_handle">];
+  let Documentation = [ReleaseHandleDocs];
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision.
xazax.hun added reviewers: NoQ, haowei.
xazax.hun added a project: clang.
Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity, 
mgorny.

This check is based on https://reviews.llvm.org/D36022 but it takes a bit 
different approach. It does less state splitting and tries to avoid the 
evalCall callback. The state machine is also a bit different, now the escaped 
and untracked states are merged.

There were some problems in the original patch with non-pointer escapes. I did 
not really see those problems with my current model (which is slightly 
different) but there might be some skeletons waiting to fall out.

Disclaimer: this patch will not apply cleanly on top of tree just yet. There 
are some dependencies that I plan to upload soon, but in the meantime I wanted 
this to be available for review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70470

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.c

Index: clang/test/Analysis/fuchsia_handle.c
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_handle.c
@@ -0,0 +1,114 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=fuchsia.FuchsiaHandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define XZ_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define XZ_HANDLE_RELEASE  __attribute__((release_handle))
+#define XZ_HANDLE_USE  __attribute__((use_handle))
+#else
+#define XZ_HANDLE_ACQUIRE
+#define XZ_HANDLE_RELEASE
+#define XZ_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+XZ_HANDLE_ACQUIRE zx_handle_t* out0,
+zx_handle_t* out1 XZ_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+zx_handle_t handle XZ_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+
+void use1(const zx_handle_t *in XZ_HANDLE_USE);
+void use2(zx_handle_t in XZ_HANDLE_USE);
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++)
+zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkNoLeak07(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  if (tag) {
+escape1();
+escape2(sb);
+  }
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak08(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  use1();
+  if (tag)
+zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag)
+zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  use1(); // expected-warning {{Using a previously released handle}}
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -0,0 +1,453 @@
+//=== FuchsiaHandleChecker.cpp - Find handle leaks/double closes -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This checker checks if the handle of Fuchsia is properly used according to
+// following rules.
+//   - If a handle is acquired, it should be released before 

[clang] 29fd1e1 - [clang][IFS] Attempting to fix missing 'orbis-ld' on scei-ps4-ubuntu bot.

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T18:34:49-05:00
New Revision: 29fd1e1f4a372f3870e054da24b57a4f45861808

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

LOG: [clang][IFS] Attempting to fix missing 'orbis-ld' on scei-ps4-ubuntu bot.

I want this test to run end to end, but I am still having trouble with
missing linkers on the scei-ps4 bot. Will remove this test if it
continues to be a source of brittle failures. Sorry for the noise.

Added: 


Modified: 
clang/test/InterfaceStubs/driver-test.c

Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
index b34536a29761..62d522fb7b1f 100644
--- a/clang/test/InterfaceStubs/driver-test.c
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -1,6 +1,8 @@
+// REQUIRES: x86-registered-target
 // REQUIRES: !system-darwin && !system-windows
 
-// RUN: %clang -o %t1 -emit-interface-stubs -emit-merged-ifs %s %S/object.c 
%S/weak.cpp
+// RUN: %clang -o %t1 -target x86_64-unknown-linux-gnu \
+// RUN:   -emit-interface-stubs -emit-merged-ifs %s %S/object.c %S/weak.cpp
 // RUN: cat %t1.ifs | FileCheck %s
 
 // CHECK-DAG: data



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


[PATCH] D69869: [clang-tools-extra] fix the check for if '-latomic' is necessary

2019-11-19 Thread Gokturk Yuksek via Phabricator via cfe-commits
gokturk added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69869



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


[PATCH] D70467: [Distro] Bypass distro detection on Windows

2019-11-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I'd rather avoid the ifdef if possible. I looked to see where these come from:

  $ git grep -l Distro ../clang/lib/Driver/
  ../clang/lib/Driver/CMakeLists.txt
  ../clang/lib/Driver/Distro.cpp
  ../clang/lib/Driver/ToolChains/Clang.cpp
  ../clang/lib/Driver/ToolChains/Cuda.cpp
  ../clang/lib/Driver/ToolChains/Linux.cpp

I think the only one that matters is in Clang.cpp, but it looks like it should 
never run on Windows:

  if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
   (TC.getTriple().isOSBinFormatELF() ||
TC.getTriple().isOSBinFormatCOFF()) &&
  !TC.getTriple().isPS4() &&
  !TC.getTriple().isOSNetBSD() &&
  !Distro(D.getVFS()).IsGentoo() &&
  !TC.getTriple().isAndroid() &&
   TC.useIntegratedAs()))
CmdArgs.push_back("-faddrsig");

Hm, I guess it does happen. I think that condition should be restructured to 
only do all that BSD, PS4, Android, Gentoo etc logic if the format is ELF, if 
COFF, then always default to -faddrsig.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70467



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


[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-19 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a subscriber: MyDeveloperDay.
poelmanc added a comment.

Just a quick update, I made some progress addressing the architectural 
limitation of `AnnotatedLines` and `evaluate()`. I changed the `AnnotatedLines` 
constructor to also take a `Line *PrevAnnotatedLine` argument and set 
`Line->First->Prev` to the `Last` of the previous line and `Line->Last->Next` 
to the `First` of the next line. So now the Tokens remain connected across 
lines, so individual `TokenAnalyzer` subclasses can easily peek ahead and 
behind the current changed Line if they wish. Places that previously iterated 
//e.g.// `while(Tok)` had to be changed to iterate `while(Tok && Tok != 
Line->Last->Next)` - I probably haven't found all of those places yet.

The good news is, with that architectural change my prior addition of:

  cleanupLeft(Line->First, tok::semi, tok::semi);

successfully removed redundant semicolons after `default`, even when the next 
semicolon was on the next line and/or separated by comments!

The bad news is this change also removed consecutive semicolons in //e.g.//:

  for( ;; )

which breaks some existing tests. I believe @MyDeveloperDay had thoughts on how 
to avoid replacing those meaningful consecutive semicolons? We have only tokens 
rather than an AST. We could search backwards for a `for` token, but I don't 
know how far backwards we would want to search to determine whether **this 
particular** `;;` is valid.

In D70144#1748881 , @JonasToth wrote:

> Hmm. I think this is fine, even though its not perfect.
>  @aaron.ballman wdyt?


The attached patch is nice and small: a 5-line generally useful utility 
function plus a few lines to call it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70144



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


[clang] 377d70c - [clang][IFS] Fixing failing bots that do not have PPC target or "orbis-ld"

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T18:12:07-05:00
New Revision: 377d70cdea733e36107e99d9148864d24797d51c

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

LOG: [clang][IFS] Fixing failing bots that do not have PPC target or "orbis-ld"

Added: 


Modified: 
clang/test/InterfaceStubs/driver-test.c
clang/test/InterfaceStubs/ppc.cpp

Removed: 




diff  --git a/clang/test/InterfaceStubs/driver-test.c 
b/clang/test/InterfaceStubs/driver-test.c
index ed5e5a04a226..b34536a29761 100644
--- a/clang/test/InterfaceStubs/driver-test.c
+++ b/clang/test/InterfaceStubs/driver-test.c
@@ -1,3 +1,5 @@
+// REQUIRES: !system-darwin && !system-windows
+
 // RUN: %clang -o %t1 -emit-interface-stubs -emit-merged-ifs %s %S/object.c 
%S/weak.cpp
 // RUN: cat %t1.ifs | FileCheck %s
 

diff  --git a/clang/test/InterfaceStubs/ppc.cpp 
b/clang/test/InterfaceStubs/ppc.cpp
index 3155bbd825c5..9a91697d9506 100644
--- a/clang/test/InterfaceStubs/ppc.cpp
+++ b/clang/test/InterfaceStubs/ppc.cpp
@@ -1,3 +1,5 @@
+// REQUIRES: powerpc-registered-target
+
 // RUN: %clang -x c++ -target powerpc64le-unknown-linux-gnu -o - %s \
 // RUN:   -emit-interface-stubs -emit-merged-ifs -S | \
 // RUN: FileCheck -check-prefix=CHECK-IFS %s



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


[PATCH] D70149: [C-index] Fix annotate-deep-statements test in Debug target

2019-11-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Thanks Reid!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70149



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


[clang] ea8e028 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (2)

2019-11-19 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T17:47:38-05:00
New Revision: ea8e02822341e2421b94167d828d3f224e767424

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

LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (2)

Second Landing Attempt:

Up until now, clang interface stubs has replaced the standard
PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
conjunction with it. So what when you build your code you will get an
a.out or lib.so as well as an interface stub file.

Example:

clang -shared -o libfoo.so -emit-interface-stubs ...

will generate both a libfoo.so and a libfoo.ifso. The .so file will
contain the code from the standard compilation pipeline and the .ifso
file will contain the ELF stub library.

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

Added: 
clang/test/InterfaceStubs/driver-test2.c
clang/test/InterfaceStubs/ppc.cpp

Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/InterfaceStubs.cpp
clang/lib/Driver/Types.cpp
clang/test/InterfaceStubs/driver-test.c
clang/test/InterfaceStubs/windows.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index cdf4a579f431..83b5db3b2530 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList ,
  (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
 FinalPhase = phases::Compile;
 
-  // clang interface stubs
-  } else if ((PhaseArg = DAL.getLastArg(options::OPT_emit_interface_stubs))) {
-FinalPhase = phases::IfsMerge;
-
   // -S only runs up to the backend.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
 FinalPhase = phases::Backend;
@@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation , DerivedArgList 
,
 Actions.push_back(
 C.MakeAction(MergerInputs, types::TY_Image));
 
+  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
+llvm::SmallVector PhaseList;
+if (Args.hasArg(options::OPT_c)) {
+  llvm::SmallVector 
CompilePhaseList;
+  types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
+  llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
+[&](phases::ID Phase) { return Phase <= phases::Compile; 
});
+} else {
+  types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
+}
+
+ActionList MergerInputs;
+
+for (auto  : Inputs) {
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+
+  // Currently clang and the llvm assembler do not support generating 
symbol
+  // stubs from assembly, so we skip the input on asm files. For ifs files
+  // we rely on the normal pipeline setup in the pipeline setup code above.
+  if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
+  InputType == types::TY_Asm)
+continue;
+
+  Action *Current = C.MakeAction(*InputArg, InputType);
+
+  for (auto Phase : PhaseList) {
+switch (Phase) {
+default:
+  llvm_unreachable(
+  "IFS Pipeline can only consist of Compile followed by 
IfsMerge.");
+case phases::Compile: {
+  // Only IfsMerge (llvm-ifs) can handle .o files by looking for ifs
+  // files where the .o file is located. The compile action can not
+  // handle this.
+  if (InputType == types::TY_Object)
+break;
+
+  Current = C.MakeAction(Current, types::TY_IFS_CPP);
+  break;
+}
+case phases::IfsMerge: {
+  assert(Phase == PhaseList.back() &&
+ "merging must be final compilation step.");
+  MergerInputs.push_back(Current);
+  Current = nullptr;
+  break;
+}
+}
+  }
+
+  // If we ended with something, add to the output list.
+  if (Current)
+Actions.push_back(Current);
+}
+
+// Add an interface stubs merge action if necessary.
+if (!MergerInputs.empty())
+  Actions.push_back(
+  C.MakeAction(MergerInputs, types::TY_Image));
+  }
+
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a 
custom
   // Compile phase that prints out supported cpu models and quits.
   if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {
@@ -3603,8 +3661,6 @@ Action *Driver::ConstructPhaseAction(
   return C.MakeAction(Input, types::TY_ModuleFile);
 if (Args.hasArg(options::OPT_verify_pch))
   return C.MakeAction(Input, types::TY_Nothing);
-if (Args.hasArg(options::OPT_emit_interface_stubs))
-  return C.MakeAction(Input, types::TY_IFS_CPP);
 return C.MakeAction(Input, types::TY_LLVM_BC);
   

[PATCH] D70149: [C-index] Fix annotate-deep-statements test in Debug target

2019-11-19 Thread 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

I was wondering if Richard would add something since he created this stack size 
management code, but in the absence of that, this looks right to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70149



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


[PATCH] D70467: [Distro] Bypass distro detection on Windows

2019-11-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision.
aganea added reviewers: sylvestre.ledru, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This skips distro detection on Windows, thus saving a few `stat` calls for each 
invocation of `clang.exe`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70467

Files:
  clang/lib/Driver/Distro.cpp
  clang/unittests/Driver/DistroTest.cpp


Index: clang/unittests/Driver/DistroTest.cpp
===
--- clang/unittests/Driver/DistroTest.cpp
+++ clang/unittests/Driver/DistroTest.cpp
@@ -337,4 +337,37 @@
   ASSERT_TRUE(Gentoo.IsGentoo());
 }
 
+#ifdef _WIN32
+TEST(DistroTest, DetectOnWindows) {
+
+  class CountingFileSystem : public llvm::vfs::ProxyFileSystem {
+  public:
+CountingFileSystem() : ProxyFileSystem(llvm::vfs::getRealFileSystem()) {}
+
+llvm::ErrorOr status(const llvm::Twine ) override {
+  ++Count;
+  return llvm::vfs::ProxyFileSystem::status(Path);
+}
+
+llvm::ErrorOr>
+openFileForRead(const llvm::Twine ) override {
+  ++Count;
+  return llvm::vfs::ProxyFileSystem::openFileForRead(Path);
+}
+
+unsigned Count{};
+  };
+
+  CountingFileSystem CFileSystem;
+  Distro WinVFS{ CFileSystem };
+  ASSERT_EQ(Distro(Distro::UnknownDistro), WinVFS);
+  ASSERT_GT(CFileSystem.Count, 0U);
+
+  llvm::IntrusiveRefCntPtr RFS =
+  llvm::vfs::getRealFileSystem();
+  Distro WinRFS{*RFS};
+  ASSERT_EQ(Distro(Distro::UnknownDistro), WinRFS);
+}
+#endif
+
 } // end anonymous namespace
Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -18,6 +18,12 @@
 using namespace clang;
 
 static Distro::DistroType DetectDistro(llvm::vfs::FileSystem ) {
+#ifdef _WIN32
+  IntrusiveRefCntPtr RealFS =
+  llvm::vfs::getRealFileSystem();
+  if ( == RealFS.get())
+return Distro::UnknownDistro;
+#endif
   llvm::ErrorOr> File =
   VFS.getBufferForFile("/etc/lsb-release");
   if (File) {


Index: clang/unittests/Driver/DistroTest.cpp
===
--- clang/unittests/Driver/DistroTest.cpp
+++ clang/unittests/Driver/DistroTest.cpp
@@ -337,4 +337,37 @@
   ASSERT_TRUE(Gentoo.IsGentoo());
 }
 
+#ifdef _WIN32
+TEST(DistroTest, DetectOnWindows) {
+
+  class CountingFileSystem : public llvm::vfs::ProxyFileSystem {
+  public:
+CountingFileSystem() : ProxyFileSystem(llvm::vfs::getRealFileSystem()) {}
+
+llvm::ErrorOr status(const llvm::Twine ) override {
+  ++Count;
+  return llvm::vfs::ProxyFileSystem::status(Path);
+}
+
+llvm::ErrorOr>
+openFileForRead(const llvm::Twine ) override {
+  ++Count;
+  return llvm::vfs::ProxyFileSystem::openFileForRead(Path);
+}
+
+unsigned Count{};
+  };
+
+  CountingFileSystem CFileSystem;
+  Distro WinVFS{ CFileSystem };
+  ASSERT_EQ(Distro(Distro::UnknownDistro), WinVFS);
+  ASSERT_GT(CFileSystem.Count, 0U);
+
+  llvm::IntrusiveRefCntPtr RFS =
+  llvm::vfs::getRealFileSystem();
+  Distro WinRFS{*RFS};
+  ASSERT_EQ(Distro(Distro::UnknownDistro), WinRFS);
+}
+#endif
+
 } // end anonymous namespace
Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -18,6 +18,12 @@
 using namespace clang;
 
 static Distro::DistroType DetectDistro(llvm::vfs::FileSystem ) {
+#ifdef _WIN32
+  IntrusiveRefCntPtr RealFS =
+  llvm::vfs::getRealFileSystem();
+  if ( == RealFS.get())
+return Distro::UnknownDistro;
+#endif
   llvm::ErrorOr> File =
   VFS.getBufferForFile("/etc/lsb-release");
   if (File) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68997: Allow searching for prebuilt implicit modules.

2019-11-19 Thread Alexandre Rames via Phabricator via cfe-commits
arames added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68997



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


[PATCH] D70245: [OPENMP50]Add device/kind context selector support.

2019-11-19 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

More comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70245



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


[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

2019-11-19 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment.

In D69950#1752044 , @gribozavr2 wrote:

> In D69950#1751894 , @eandrews wrote:
>
> > In D69950#1751859 , @gribozavr2 
> > wrote:
> >
> > > > hence not throwing the warning on any platform?
> > >
> > > The way I read the buildbot breakage, an existing ClangTidy test passed 
> > > before and after this change, but broke on Windows. The breakage was that 
> > > the warnings stopped being produced.
> >
> >
> > The existing test does not have this warning. I modified the test to add 
> > the check for this warning since it is generated on Linux after my patch. 
> > It is not generated on Windows because of delayed template parsing.
>
>
> Is it just that warning that is not generated, or all warnings in that test 
> file?


I am pretty sure it is just this warning but I will verify this again and push 
a new patch with "-fno-delayed-template-parsing" once I confirm . Thanks for 
all your help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69950



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


[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

I love how this checker is pioneering in so many ways.




Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318
+ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal ) 
{
   if (auto Reg = Val.getAsRegion()) {
 Reg = Reg->getMostDerivedObjectRegion();
-return State->get(Reg);
+return State->remove(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
-return State->get(Sym);
+return State->remove(Sym);
   } else if (const auto LCVal = Val.getAs()) {

Maybe move this function to `Iterator.cpp` as well, and then move the 
definitions for iterator maps from `Iterator.h` to `Iterator.cpp`, which will 
allow you to use the usual `REGISTER_MAP_WITH_PROGRAMSTATE` macros, and 
additionally guarantee that all access to the maps goes through the accessor 
methods that you provide?


Repository:
  rC Clang

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

https://reviews.llvm.org/D70320



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


[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

2019-11-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 230143.
aganea marked 10 inline comments as done.
aganea edited the summary of this revision.
aganea added a comment.

Addressed @hans' comments.

I've also added a script in the summary, if you'd like to reproduce the results 
on your end, and to ensure we all run the same thing.


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

https://reviews.llvm.org/D69825

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/include/llvm/Support/Signals.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -521,10 +521,13 @@
 extern "C" VOID WINAPI RtlCaptureContext(PCONTEXT ContextRecord);
 #endif
 
-void llvm::sys::PrintStackTrace(raw_ostream ) {
-  STACKFRAME64 StackFrame = {};
-  CONTEXT Context = {};
-  ::RtlCaptureContext();
+static void LocalPrintStackTrace(raw_ostream , PCONTEXT C) {
+  STACKFRAME64 StackFrame{};
+  CONTEXT Context{};
+  if (!C) {
+::RtlCaptureContext();
+C = 
+  }
 #if defined(_M_X64)
   StackFrame.AddrPC.Offset = Context.Rip;
   StackFrame.AddrStack.Offset = Context.Rsp;
@@ -546,9 +549,12 @@
   StackFrame.AddrStack.Mode = AddrModeFlat;
   StackFrame.AddrFrame.Mode = AddrModeFlat;
   PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(),
-   StackFrame, );
+   StackFrame, C);
 }
 
+void llvm::sys::PrintStackTrace(raw_ostream ) {
+  LocalPrintStackTrace(OS, nullptr);
+}
 
 void llvm::sys::SetInterruptFunction(void (*IF)()) {
   RegisterHandler();
@@ -792,6 +798,10 @@
   return std::error_code();
 }
 
+signed sys::InvokeExceptionHandler(int &, void *ExceptionInfo) {
+  return LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)ExceptionInfo);
+}
+
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
@@ -810,42 +820,9 @@
<< "\n";
   }
 
-  // Initialize the STACKFRAME structure.
-  STACKFRAME64 StackFrame = {};
-
-#if defined(_M_X64)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Rip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Rbp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_IX86)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Eip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Esp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_ARM64) || defined(_M_ARM)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Pc;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Sp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-#if defined(_M_ARM64)
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Fp;
-#else
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->R11;
-#endif
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#endif
-
-  HANDLE hProcess = GetCurrentProcess();
-  HANDLE hThread = GetCurrentThread();
-  PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame,
-   ep->ContextRecord);
+  LocalPrintStackTrace(llvm::errs(), ep ? ep->ContextRecord : nullptr);
 
-  _exit(ep->ExceptionRecord->ExceptionCode);
+  return EXCEPTION_EXECUTE_HANDLER;
 }
 
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -345,6 +345,17 @@
   FileToRemoveList::removeAllFiles(FilesToRemove);
 }
 
+signed sys::InvokeExceptionHandler(int , void *) {
+  SignalHandler(RetCode);
+  // llvm/lib/Support/Unix/Program.inc:Wait() returns -2 if a crash occurs,
+  // not the actual error code. If we want to diagnose a crash in the same
+  // way as invoking/forking a new process (in
+  // clang/tools/driver/driver.cpp), we need to do this here.
+  if (WIFSIGNALED(RetCode))
+RetCode = -2;
+  return 0;
+}
+
 // The signal handler that runs.
 static RETSIGTYPE SignalHandler(int Sig) {
   // Restore the signal behavior to default, so that the program actually
@@ -361,8 +372,8 @@
   {
 RemoveFilesToRemove();
 
-if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig)
-!= std::end(IntSigs)) {
+if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig) !=
+std::end(IntSigs)) {
   if (auto OldInterruptFunction = 

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

2019-11-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+CC1Main = Driver::CC1Main;

hans wrote:
> Now that we're not calling main() anymore, but cc1 directly -- is this 
> checking still necessary?
Yes it is - see my other comment just above.

The driver builds phases that do not always call the cc1 process. Simply 
stating `clang a.cpp` would invoke `clang -cc1`, then the linker, say 
`link.exe`. In this later case (invoking `link.exe`), even if we have 
`Driver::Main` it doesn't mean we should use it. There are a number of other 
edge cases of the same kind, such as `/fallback` or building cuda files, where 
a different executable is invoked along the way."


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

https://reviews.llvm.org/D69825



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


Re: [PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Eric Christopher via cfe-commits
The C bindings, in general, don't have stability guarantees outside of
a few cases.

Whitequark owns all of this now though :)

-eric

On Tue, Nov 19, 2019 at 1:38 PM Adrian Prantl via Phabricator
 wrote:
>
> aprantl added a subscriber: echristo.
> aprantl added a comment.
>
> In D70111#1752215 , @dblaikie wrote:
>
> > I have some vague recollection that the DWARF C bindings didn't have 
> > stability guarantees? Does that sound familiar to anyone? What sort of 
> > changes have been made to them in the past?
>
>
> That does sound familiar. I'm not sure if this was formalized anywhere or if 
> we just all agreed on it verbally. @echristo might know?
>
> In that case there is a 3rd option which is to
>
> - update the C bindings and the go bindings in one commit.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D70111/new/
>
> https://reviews.llvm.org/D70111
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a subscriber: echristo.
aprantl added a comment.

In D70111#1752215 , @dblaikie wrote:

> I have some vague recollection that the DWARF C bindings didn't have 
> stability guarantees? Does that sound familiar to anyone? What sort of 
> changes have been made to them in the past?


That does sound familiar. I'm not sure if this was formalized anywhere or if we 
just all agreed on it verbally. @echristo might know?

In that case there is a 3rd option which is to

- update the C bindings and the go bindings in one commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

2019-11-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

In D69330#1752089 , @ilya-biryukov 
wrote:

> In D69330#1746137 , @rsmith wrote:
>
> > I would prefer that we have dedicated tests for them rather than scattering 
> > the tests throughout the existing test suite (for example, consider adding 
> > `-Wno-unused` to the tests producing "result unused" warnings and adding a 
> > dedicated test).
>
>
> Most updated tests (including those with more "result unused" warnings) are 
> actually not intended to test recovery expressions, they just happen to 
> produce different results now and need to be updated.
>  The only new dedicated tests here are `clang/test/AST/ast-dump-recovery.cpp` 
> and `clang/test/Index/getcursor-recovery.cpp`.
>
> Could technically move them into the same directory, but wanted to make sure 
> I got your point first. Could you elaborate on what testing strategy you'd 
> prefer?


For the tests where you added `expected-warning {{unused}}` that are testing 
things other than parser recovery, I would instead add `-Wno-unused` or a cast 
to `void`. `expect`s unrelated to the intent of tests make them less readable 
and more fragile.

I think it's fine to use `test/AST/ast-dump-recovery.cpp` as the primary test 
for the various forms of recovery that you added here. I've not thought of 
anything better, at least :)




Comment at: clang/lib/Sema/TreeTransform.h:9470
+ExprResult TreeTransform::TransformRecoveryExpr(RecoveryExpr *E) {
+  return E;
+}

ilya-biryukov wrote:
> rsmith wrote:
> > We should either transform the subexpressions or just return `ExprError()` 
> > here. With this approach, we can violate AST invariants (eg, by having the 
> > same `Expr` reachable through two different code paths in the same function 
> > body, or by retaining `DeclRefExpr`s referring to declarations from the 
> > wrong context, etc).
> Thanks, I just blindly copied what TypoExpr was doing without considering the 
> consequences here.
> 
> 
> Added the code to rebuild `RecoveryExpr`.
> 
> Any good way to test this?
Hmm, testing that the old failure mode doesn't happen seems a bit tricky; any 
test for that is going to be testing second-order effects of the code under 
test, so will be fragile. But you can test that we're substituting into the 
`RecoveryExpr` easily enough: put some expression for which substitution will 
fail into a context where we'll build a `RecoveryExpr` inside a context that 
we'll `TreeTransform`. For example, maybe:

```
template int *p = (T::error); // should produce "cannot take 
address of void"
int *q = p; // should produce "'int' cannot be used prior to '::'" in 
instantiation
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69330



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


[PATCH] D70285: Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

2019-11-19 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision.
dexonsmith added a comment.

In D70285#1752361 , @dexonsmith wrote:

> In D70285#1752164 , @Bigcheese wrote:
>
> > lgtm as long as other compilers don't warn on unknown pragmas by default.
>
>
> godbolt.org says that GCC doesn't have `-Wunknown-pragmas` by default, but it 
> is in `-Wall`.  I'd better guard this more closely with `__clang__`.


Pushed as 8c48405069085a2c8b6b80816eda99e5dad31fc1 
 with this 
fixup.


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

https://reviews.llvm.org/D70285



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


[clang] 8c48405 - Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

2019-11-19 Thread Duncan P. N. Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2019-11-19T13:18:43-08:00
New Revision: 8c48405069085a2c8b6b80816eda99e5dad31fc1

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

LOG: Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

Force `-Werror=strict-prototypes` so that C API tests fail to compile if
we add a non-prototype declaration.  This should help avoid regressions
like bddecba4b333f7772029b4937d2c34f9f2fda6ca was fixing.

https://reviews.llvm.org/D70285
rdar://problem/57203137

Added: 
clang/include/clang-c/ExternC.h
llvm/include/llvm-c/ExternC.h

Modified: 
clang/include/clang-c/BuildSystem.h
clang/include/clang-c/CXCompilationDatabase.h
clang/include/clang-c/CXErrorCode.h
clang/include/clang-c/CXString.h
clang/include/clang-c/Documentation.h
clang/include/clang-c/FatalErrorHandler.h
clang/include/clang-c/Index.h
clang/include/clang-c/Platform.h
llvm/include/llvm-c/Analysis.h
llvm/include/llvm-c/BitReader.h
llvm/include/llvm-c/BitWriter.h
llvm/include/llvm-c/Comdat.h
llvm/include/llvm-c/Core.h
llvm/include/llvm-c/DebugInfo.h
llvm/include/llvm-c/Disassembler.h
llvm/include/llvm-c/Error.h
llvm/include/llvm-c/ErrorHandling.h
llvm/include/llvm-c/ExecutionEngine.h
llvm/include/llvm-c/IRReader.h
llvm/include/llvm-c/Initialization.h
llvm/include/llvm-c/LinkTimeOptimizer.h
llvm/include/llvm-c/Linker.h
llvm/include/llvm-c/Object.h
llvm/include/llvm-c/OrcBindings.h
llvm/include/llvm-c/Remarks.h
llvm/include/llvm-c/Support.h
llvm/include/llvm-c/Target.h
llvm/include/llvm-c/TargetMachine.h
llvm/include/llvm-c/Transforms/AggressiveInstCombine.h
llvm/include/llvm-c/Transforms/Coroutines.h
llvm/include/llvm-c/Transforms/IPO.h
llvm/include/llvm-c/Transforms/InstCombine.h
llvm/include/llvm-c/Transforms/PassManagerBuilder.h
llvm/include/llvm-c/Transforms/Scalar.h
llvm/include/llvm-c/Transforms/Utils.h
llvm/include/llvm-c/Transforms/Vectorize.h
llvm/include/llvm-c/Types.h
llvm/include/llvm-c/lto.h

Removed: 




diff  --git a/clang/include/clang-c/BuildSystem.h 
b/clang/include/clang-c/BuildSystem.h
index 8f26a8611719..4e9f6dee0279 100644
--- a/clang/include/clang-c/BuildSystem.h
+++ b/clang/include/clang-c/BuildSystem.h
@@ -14,13 +14,12 @@
 #ifndef LLVM_CLANG_C_BUILDSYSTEM_H
 #define LLVM_CLANG_C_BUILDSYSTEM_H
 
-#include "clang-c/Platform.h"
 #include "clang-c/CXErrorCode.h"
 #include "clang-c/CXString.h"
+#include "clang-c/ExternC.h"
+#include "clang-c/Platform.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_CLANG_C_EXTERN_C_BEGIN
 
 /**
  * \defgroup BUILD_SYSTEM Build system utilities
@@ -148,9 +147,7 @@ CINDEX_LINKAGE void 
clang_ModuleMapDescriptor_dispose(CXModuleMapDescriptor);
  * @}
  */
 
-#ifdef __cplusplus
-}
-#endif
+LLVM_CLANG_C_EXTERN_C_END
 
 #endif /* CLANG_C_BUILD_SYSTEM_H */
 

diff  --git a/clang/include/clang-c/CXCompilationDatabase.h 
b/clang/include/clang-c/CXCompilationDatabase.h
index 2669c1a792c1..2b336e5464a2 100644
--- a/clang/include/clang-c/CXCompilationDatabase.h
+++ b/clang/include/clang-c/CXCompilationDatabase.h
@@ -15,12 +15,11 @@
 #ifndef LLVM_CLANG_C_CXCOMPILATIONDATABASE_H
 #define LLVM_CLANG_C_CXCOMPILATIONDATABASE_H
 
-#include "clang-c/Platform.h"
 #include "clang-c/CXString.h"
+#include "clang-c/ExternC.h"
+#include "clang-c/Platform.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_CLANG_C_EXTERN_C_BEGIN
 
 /** \defgroup COMPILATIONDB CompilationDatabase functions
  * \ingroup CINDEX
@@ -169,8 +168,7 @@ 
clang_CompileCommand_getMappedSourceContent(CXCompileCommand, unsigned I);
  * @}
  */
 
-#ifdef __cplusplus
-}
-#endif
+LLVM_CLANG_C_EXTERN_C_END
+
 #endif
 

diff  --git a/clang/include/clang-c/CXErrorCode.h 
b/clang/include/clang-c/CXErrorCode.h
index fed195ec1f33..b3a0b9d66d5f 100644
--- a/clang/include/clang-c/CXErrorCode.h
+++ b/clang/include/clang-c/CXErrorCode.h
@@ -14,11 +14,10 @@
 #ifndef LLVM_CLANG_C_CXERRORCODE_H
 #define LLVM_CLANG_C_CXERRORCODE_H
 
+#include "clang-c/ExternC.h"
 #include "clang-c/Platform.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_CLANG_C_EXTERN_C_BEGIN
 
 /**
  * Error codes returned by libclang routines.
@@ -57,8 +56,7 @@ enum CXErrorCode {
   CXError_ASTReadError = 4
 };
 
-#ifdef __cplusplus
-}
-#endif
+LLVM_CLANG_C_EXTERN_C_END
+
 #endif
 

diff  --git a/clang/include/clang-c/CXString.h 
b/clang/include/clang-c/CXString.h
index 1eb3442ccb24..f117010c71a4 100644
--- a/clang/include/clang-c/CXString.h
+++ b/clang/include/clang-c/CXString.h
@@ -14,11 +14,10 @@
 #ifndef LLVM_CLANG_C_CXSTRING_H
 #define LLVM_CLANG_C_CXSTRING_H
 
+#include "clang-c/ExternC.h"
 #include "clang-c/Platform.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif

[PATCH] D70285: Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

2019-11-19 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In D70285#1752164 , @Bigcheese wrote:

> lgtm as long as other compilers don't warn on unknown pragmas by default.


godbolt.org says that GCC doesn't have `-Wunknown-pragmas` by default, but it 
is in `-Wall`.  I'd better guard this more closely with `__clang__`.


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

https://reviews.llvm.org/D70285



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


[clang] d08c056 - [OPENMP50]Add if clause in simd directive.

2019-11-19 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2019-11-19T15:58:19-05:00
New Revision: d08c056695a59fb1cfc7ccc9a2784bb9a6514551

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

LOG: [OPENMP50]Add if clause in simd directive.

According to OpenMP 5.0, if clause can be used in simd directive. If
condition in the if clause if false, the non-vectorized version of the
loop must be executed.

Added: 
clang/test/OpenMP/simd_if_messages.cpp

Modified: 
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/include/clang/Basic/OpenMPKinds.def
clang/include/clang/Basic/OpenMPKinds.h
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/simd_ast_print.cpp
clang/test/OpenMP/simd_codegen.cpp

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 72ae63f9a757..3a5d0c08337e 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6781,7 +6781,9 @@ AST_MATCHER(OMPDefaultClause, isSharedKind) {
 /// ``isAllowedToContainClauseKind("OMPC_default").``
 AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClauseKind,
   OpenMPClauseKind, CKind) {
-  return isAllowedClauseForDirective(Node.getDirectiveKind(), CKind);
+  return isAllowedClauseForDirective(
+  Node.getDirectiveKind(), CKind,
+  Finder->getASTContext().getLangOpts().OpenMP);
 }
 
 
////

diff  --git a/clang/include/clang/Basic/OpenMPKinds.def 
b/clang/include/clang/Basic/OpenMPKinds.def
index 9bba5e6b96ba..affa972529ea 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -366,6 +366,7 @@ OPENMP_SIMD_CLAUSE(simdlen)
 OPENMP_SIMD_CLAUSE(collapse)
 OPENMP_SIMD_CLAUSE(reduction)
 OPENMP_SIMD_CLAUSE(allocate)
+OPENMP_SIMD_CLAUSE(if)
 
 // Clauses allowed for directive 'omp for'.
 OPENMP_FOR_CLAUSE(private)

diff  --git a/clang/include/clang/Basic/OpenMPKinds.h 
b/clang/include/clang/Basic/OpenMPKinds.h
index 208e360c8a70..121c8bd2e48b 100644
--- a/clang/include/clang/Basic/OpenMPKinds.h
+++ b/clang/include/clang/Basic/OpenMPKinds.h
@@ -219,7 +219,8 @@ unsigned getOpenMPSimpleClauseType(OpenMPClauseKind Kind, 
llvm::StringRef Str);
 const char *getOpenMPSimpleClauseTypeName(OpenMPClauseKind Kind, unsigned 
Type);
 
 bool isAllowedClauseForDirective(OpenMPDirectiveKind DKind,
- OpenMPClauseKind CKind);
+ OpenMPClauseKind CKind,
+ unsigned OpenMPVersion);
 
 /// Checks if the specified directive is a directive with an associated
 /// loop construct.

diff  --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp
index 4e6c677f11db..b1e623b861de 100644
--- a/clang/lib/Basic/OpenMPKinds.cpp
+++ b/clang/lib/Basic/OpenMPKinds.cpp
@@ -447,7 +447,8 @@ const char 
*clang::getOpenMPSimpleClauseTypeName(OpenMPClauseKind Kind,
 }
 
 bool clang::isAllowedClauseForDirective(OpenMPDirectiveKind DKind,
-OpenMPClauseKind CKind) {
+OpenMPClauseKind CKind,
+unsigned OpenMPVersion) {
   assert(DKind <= OMPD_unknown);
   assert(CKind <= OMPC_unknown);
   switch (DKind) {
@@ -462,6 +463,8 @@ bool clang::isAllowedClauseForDirective(OpenMPDirectiveKind 
DKind,
 }
 break;
   case OMPD_simd:
+if (OpenMPVersion < 50 && CKind == OMPC_if)
+  return false;
 switch (CKind) {
 #define OPENMP_SIMD_CLAUSE(Name)   
\
   case OMPC_##Name:
\

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.h 
b/clang/lib/CodeGen/CGOpenMPRuntime.h
index d051170121ca..d806c27fdcba 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -241,17 +241,6 @@ class CGOpenMPRuntime {
 bool IsOffloadEntry,
 const RegionCodeGenTy 
);
 
-  /// Emits code for OpenMP 'if' clause using specified \a CodeGen
-  /// function. Here is the logic:
-  /// if (Cond) {
-  ///   ThenGen();
-  /// } else {
-  ///   ElseGen();
-  /// }
-  void emitIfClause(CodeGenFunction , const Expr *Cond,
-const RegionCodeGenTy ,
-const RegionCodeGenTy );
-
   /// Emits object of ident_t type with info for source location.
   /// \param Flags Flags for 

[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-19 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG586f65d31f32: Add a key method to Sema to optimize debug 
info size (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69586: [profile] Support online merging with continuous sync mode

2019-11-19 Thread Vedant Kumar via Phabricator via cfe-commits
vsk marked an inline comment as done.
vsk added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:419
+ * the open file object \p File. */
+static int writeProfileWithFileObject(const char *Filename, FILE *File) {
+  setProfileFile(File);

aganea wrote:
> These two functions are not used when targeting Windows, do you think it'll 
> be possible to `#ifdef` them out in a follow up patch? Thanks in advance!
> ```
> [550/4755] Building C object 
> projects\compiler-rt\lib\profile\CMakeFiles\clang_rt.profile-x86_64.dir\InstrProfilingFile.c.obj
> F:\llvm-project\compiler-rt\lib\profile\InstrProfilingFile.c(419,12): 
> warning: unused function 'writeProfileWithFileObject' [-Wunused-function]
> static int writeProfileWithFileObject(const char *Filename, FILE *File) {
>^
> F:\llvm-project\compiler-rt\lib\profile\InstrProfilingFile.c(429,13): 
> warning: unused function 'unlockProfile' [-Wunused-function]
> static void unlockProfile(int *ProfileRequiresUnlock, FILE *File) {
> ^
> 2 warnings generated.
> ```
Should be addressed by 0d4211f4e753057feec32938e596d037d4f5a6aa.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69586



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


[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done.
rnk added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

hans wrote:
> I worry that this is going to look obscure to most readers passing through. 
> Maybe it could be expanded to more explicitly spell out that it reduces the 
> size of the debug info?
I want to keep it concise, most readers shouldn't need to know what this is, 
and they can look up technical terms like "key method". I'll say "debug info" 
instead of "type info", though, that should be more obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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


[clang] 568db78 - [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

2019-11-19 Thread Vedant Kumar via cfe-commits

Author: Vedant Kumar
Date: 2019-11-19T12:49:27-08:00
New Revision: 568db780bb7267651a902da8e85bc59fc89aea70

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

LOG: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood 
(reland with fixes)

Currently, clang emits subprograms for declared functions when the
target debugger or DWARF standard is known to support entry values
(DW_OP_entry_value & the GNU equivalent).

Treat DW_AT_tail_call the same way to allow debuggers to follow cross-TU
tail calls.

Pre-patch debug session with a cross-TU tail call:

```
  * frame #0: 0x00010fa4 main`target at b.c:4:3 [opt]
frame #1: 0x00010f99 main`main at a.c:8:10 [opt]
```

Post-patch (note that the tail-calling frame, "helper", is visible):

```
  * frame #0: 0x00010fa4 main`target at b.c:4:3 [opt]
frame #1: 0x00010f80 main`helper [opt] [artificial]
frame #2: 0x00010f99 main`main at a.c:8:10 [opt]
```

This was reverted in 5b9a072c because it attached declaration
subprograms to inlinable builtin calls, which interacted badly with the
MergeICmps pass. The fix is to not attach declarations to builtins.

rdar://46577651

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

Added: 


Modified: 
clang/include/clang/Basic/IdentifierTable.h
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll

Removed: 




diff  --git a/clang/include/clang/Basic/IdentifierTable.h 
b/clang/include/clang/Basic/IdentifierTable.h
index 7aa1d074a591..ea5d7adeb2da 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -384,6 +384,17 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
 return getName().startswith("<#") && getName().endswith("#>");
   }
 
+  /// Determine whether \p this is a name reserved for the implementation (C99
+  /// 7.1.3, C++ [lib.global.names]).
+  bool isReservedName(bool doubleUnderscoreOnly = false) const {
+if (getLength() < 2)
+  return false;
+const char *Name = getNameStart();
+return Name[0] == '_' &&
+   (Name[1] == '_' ||
+(Name[1] >= 'A' && Name[1] <= 'Z' && !doubleUnderscoreOnly));
+  }
+
   /// Provide less than operator for lexicographical sorting.
   bool operator<(const IdentifierInfo ) const {
 return getName() < RHS.getName();

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 75c4b2ae2339..116517a9cb99 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3765,21 +3765,29 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, 
SourceLocation Loc,
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
   QualType CalleeType,
   const FunctionDecl *CalleeDecl) {
-  auto  = CGM.getCodeGenOpts();
-  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-  !CallOrInvoke)
+  if (!CallOrInvoke)
 return;
-
   auto *Func = CallOrInvoke->getCalledFunction();
   if (!Func)
 return;
+  if (Func->getSubprogram())
+return;
+
+  // Do not emit a declaration subprogram for a builtin or if call site info
+  // isn't required. Also, elide declarations for functions with reserved 
names,
+  // as call site-related features aren't interesting in this case (& also, the
+  // compiler may emit calls to these functions without debug locations, which
+  // makes the verifier complain).
+  if (CalleeDecl->getBuiltinID() != 0 ||
+  getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
+return;
+  if (const auto *Id = CalleeDecl->getIdentifier())
+if (Id->isReservedName())
+  return;
 
   // If there is no DISubprogram attached to the function being called,
   // create the one describing the function in order to have complete
   // call site debug info.
-  if (Func->getSubprogram())
-return;
-
   if (!CalleeDecl->isStatic() && !CalleeDecl->isInlined())
 EmitFunctionDecl(CalleeDecl, CalleeDecl->getLocation(), CalleeType, Func);
 }
@@ -4841,10 +4849,10 @@ llvm::DINode::DIFlags 
CGDebugInfo::getCallSiteRelatedAttrs() const {
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
   (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
-   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
-   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
 
-  if (!SupportsDWARFv4Ext 

[PATCH] D70259: [Error] Add source location to cantFail

2019-11-19 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 230131.
hintonda added a comment.

- Add back original llvm::cantFail signatures so they'll still be


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70259

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FormatTests.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/unittests/Basic/DiagnosticTest.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/unittests/Target/ExecutionContextTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp

Index: llvm/unittests/Support/ErrorTest.cpp
===
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -390,8 +390,8 @@
   };
 
   EXPECT_DEATH(FailToHandle(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << "Unhandled Error in handleAllErrors call did not cause an "
  "abort()";
 }
@@ -410,8 +410,8 @@
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << " Error returned from handler in handleAllErrors call did not "
  "cause abort()";
 }
@@ -515,8 +515,8 @@
   EXPECT_DEATH(cantFail(make_error("Original error message",
 inconvertibleErrorCode()),
 "Cantfail call failed"),
-   "Cantfail call failed\n"
-   "Original error message")
+   "Original error message\n"
+   "Cantfail call failed")
   << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
@@ -530,6 +530,24 @@
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -687,6 +687,48 @@
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
 bool gen_crash_diag = true);
 
+namespace detail {
+
+inline LLVM_ATTRIBUTE_NORETURN void handleError(Error Err, const char *Msg,
+const char *file = nullptr,
+unsigned line = 0) {
+  if (!Msg)
+Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << Err << "\n" << Msg;
+  if (file)
+OS << " at " << file << ":" << line;
+  Msg = OS.str().c_str();
+#endif
+  llvm_unreachable(Msg);
+}
+
+inline void cantFail(const char *file, unsigned line, Error Err,
+ const char *Msg = nullptr) {
+  if (Err)
+handleError(std::move(Err), Msg, file, line);
+}
+
+template 
+T cantFail(const char *file, unsigned line, Expected 

[clang] 586f65d - Add a key method to Sema to optimize debug info size

2019-11-19 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2019-11-19T12:42:33-08:00
New Revision: 586f65d31f32ca6bc8cfdb8a4f61bee5057bf6c8

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

LOG: Add a key method to Sema to optimize debug info size

It turns out that the debug info describing the Sema class is an
appreciable percentage of the total object file size of objects in Sema.
By adding a key function, clang is able to optimize the debug info size
by emitting a forward declaration in TUs that do not define the key
function.

On Windows, with clang-cl, these are the total sizes of object files in
Sema before and after this change, compiling with optimizations and
debug info:
  before: 335,012 KB
  after:  278,116 KB
  delta:  -56,896 KB
  percent: -17.0%

The effect on link time was negligible, despite having ~56MB less input.

On Linux, with clang, these are the same sizes using DWARF -g and
optimizations:
  before: 603,756 KB
  after:  515,340 KB
  delta:  -88,416 KB
  percent: -14.6%

I didn't use type units, DWARF-5, fission, or any other special flags.

Reviewed By: thakis

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

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 220b6c17835d..7d8ab59fe900 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@ class PreferredTypeBuilder {
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index bedea2167950..c3c6cad277ff 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@ Sema::Sema(Preprocessor , ASTContext , ASTConsumer 
,
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 230128.
mibintc added a comment.

Here's an update in response to comments from @michele.scandale 
I fixed the assertion error and added a test case
I fixed the setting of ftrapping-math in CodeGenOpts
I deleted an incorrect comment
I added a diagnostic when -fp-exception-behavior= is overridden on the command 
line by f[no-]trapping-math
I updated one of the test cases to work with some small modifications that will 
be made to IRBuilder.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -259,7 +259,7 @@
 assert(BB && "Must have a basic block to set any function attributes!");
 
 Function *F = BB->getParent();
-if (!F->hasFnAttribute(Attribute::StrictFP)) {
+if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
   F->addFnAttr(Attribute::StrictFP);
 }
   }
Index: clang/test/Driver/fp-model.c
===
--- /dev/null
+++ clang/test/Driver/fp-model.c
@@ -0,0 +1,137 @@
+// Test that incompatible combinations of -ffp-model= options
+// and other floating point options get a warning diagnostic.
+//
+// REQUIRES: clang-driver
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN %s
+// WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN1 %s
+// WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN2 %s
+// WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN3 %s
+// WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN4 %s
+// WARN4: warning: overriding '-ffp-model=strict' option with '-ffinite-math-only' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN5 %s
+// WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN6 %s
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN7 %s
+// WARN7: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN8 %s
+// WARN8: warning: overriding '-ffp-model=strict' option with '-fno-honor-infinities' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN9 %s
+// WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN:   | FileCheck 

[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 230130.
rnk added a comment.

- add final, tweak comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Oh, yeah, I forgot this causes tons of -Wdelete-non-virtual-dtor warnings, so 
I'll have to look into that before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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


[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

2019-11-19 Thread Vedant Kumar via Phabricator via cfe-commits
vsk marked an inline comment as done.
vsk added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:377
+#if defined(__Fuchsia__) || defined(_WIN32)
+  PROF_ERR("%s\n", "Continuous mode not yet supported on Fuchsia or Windows.");
+#else // defined(__Fuchsia__) || defined(_WIN32)

dmajor wrote:
> @vsk Doesn't this unconditionally break profiling on Fuschia/Windows? Should 
> there at least be a check for `__llvm_profile_is_continuous_mode_enabled()` 
> before erroring out? I'm currently testing my project's Windows build with 
> trunk, and it's failing here even though we didn't enable continuous mode.
Yes, sorry about the breakage, this should be addressed by 
1aacf58819a27f428a46ce839a0ee797af03c1fd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68351



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


[PATCH] D70259: [Error] Add source location to cantFail

2019-11-19 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 230125.
hintonda added a comment.
Herald added subscribers: lldb-commits, cfe-commits, usaxena95, kadircet, 
arphaman, jkorous.
Herald added projects: clang, LLDB.

- Replace macro magic with matching 3-parameter template functions.
- Refactor cantFail move into detail namespace.  Change macro name to


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70259

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FormatTests.cpp
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExternalASTMerger.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/unittests/Basic/DiagnosticTest.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/unittests/Target/ExecutionContextTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp

Index: llvm/unittests/Support/ErrorTest.cpp
===
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -390,8 +390,8 @@
   };
 
   EXPECT_DEATH(FailToHandle(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << "Unhandled Error in handleAllErrors call did not cause an "
  "abort()";
 }
@@ -410,8 +410,8 @@
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-   "Failure value returned from cantFail wrapped call\n"
-   "CustomError \\{7\\}")
+   "CustomError \\{7\\}\n"
+   "Failure value returned from cantFail wrapped call")
   << " Error returned from handler in handleAllErrors call did not "
  "cause abort()";
 }
@@ -515,8 +515,8 @@
   EXPECT_DEATH(cantFail(make_error("Original error message",
 inconvertibleErrorCode()),
 "Cantfail call failed"),
-   "Cantfail call failed\n"
-   "Original error message")
+   "Original error message\n"
+   "Cantfail call failed")
   << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
@@ -530,6 +530,24 @@
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(cantFail(make_error(1)), OS.str())
+  << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -687,6 +687,23 @@
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
 bool gen_crash_diag = true);
 
+namespace detail {
+
+inline LLVM_ATTRIBUTE_NORETURN void
+handleError(Error Err, const char *Msg, const char *file, unsigned line) {
+  if (!Msg)
+Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << Err << "\n" << Msg;
+  if (file)
+OS << " at " << file << ":" << line;
+  Msg = OS.str().c_str();
+#endif
+  llvm_unreachable(Msg);
+}
+
 /// Report a fatal error if Err is a failure value.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
@@ -700,18 +717,10 @@
 ///
 

[PATCH] D69586: [profile] Support online merging with continuous sync mode

2019-11-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:419
+ * the open file object \p File. */
+static int writeProfileWithFileObject(const char *Filename, FILE *File) {
+  setProfileFile(File);

These two functions are not used when targeting Windows, do you think it'll be 
possible to `#ifdef` them out in a follow up patch? Thanks in advance!
```
[550/4755] Building C object 
projects\compiler-rt\lib\profile\CMakeFiles\clang_rt.profile-x86_64.dir\InstrProfilingFile.c.obj
F:\llvm-project\compiler-rt\lib\profile\InstrProfilingFile.c(419,12): warning: 
unused function 'writeProfileWithFileObject' [-Wunused-function]
static int writeProfileWithFileObject(const char *Filename, FILE *File) {
   ^
F:\llvm-project\compiler-rt\lib\profile\InstrProfilingFile.c(429,13): warning: 
unused function 'unlockProfile' [-Wunused-function]
static void unlockProfile(int *ProfileRequiresUnlock, FILE *File) {
^
2 warnings generated.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69586



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


[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

See also 
https://llvm.org/docs/DeveloperPolicy.html#current-contributors-transfering-from-svn.


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

https://reviews.llvm.org/D59516



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


[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

2019-11-19 Thread dmajor via Phabricator via cfe-commits
dmajor added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:377
+#if defined(__Fuchsia__) || defined(_WIN32)
+  PROF_ERR("%s\n", "Continuous mode not yet supported on Fuchsia or Windows.");
+#else // defined(__Fuchsia__) || defined(_WIN32)

@vsk Doesn't this unconditionally break profiling on Fuschia/Windows? Should 
there at least be a check for `__llvm_profile_is_continuous_mode_enabled()` 
before erroring out? I'm currently testing my project's Windows build with 
trunk, and it's failing here even though we didn't enable continuous mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68351



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


[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

2019-11-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 230124.
MyDeveloperDay added a comment.

-update with missing files
-clang-format test


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/EastWestConstFixer.cpp
  clang/lib/Format/EastWestConstFixer.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -27,6 +27,10 @@
 
 FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
 
+#define VERIFYFORMAT(expect, style) verifyFormat(expect, style, __LINE__)
+#define VERIFYFORMAT2(expect, actual, style)   \
+  verifyFormat(expect, actual, style, __LINE__)
+
 class FormatTest : public ::testing::Test {
 protected:
   enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
@@ -66,10 +70,12 @@
   }
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle  = getLLVMStyle()) {
+const FormatStyle  = getLLVMStyle(),
+int line = __LINE__) {
 EXPECT_EQ(Expected.str(), format(Expected, Style))
-<< "Expected code is not stable";
-EXPECT_EQ(Expected.str(), format(Code, Style));
+<< "Expected code is not stable at " << __FILE__ << ":" << line;
+EXPECT_EQ(Expected.str(), format(Code, Style))
+<< " at " << __FILE__ << ":" << line;
 if (Style.Language == FormatStyle::LK_Cpp) {
   // Objective-C++ is a superset of C++, so everything checked for C++
   // needs to be checked for Objective-C++ as well.
@@ -80,8 +86,9 @@
   }
 
   void verifyFormat(llvm::StringRef Code,
-const FormatStyle  = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+const FormatStyle  = getLLVMStyle(),
+int line = __LINE__) {
+verifyFormat(Code, test::messUp(Code), Style, line);
   }
 
   void verifyIncompleteFormat(llvm::StringRef Code,
@@ -12609,6 +12616,15 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstStyle = FormatStyle::CS_Left;
+  CHECK_PARSE("ConstStyle: Leave", ConstStyle, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstStyle: East", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: West", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: Right", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Left", ConstStyle, FormatStyle::CS_Left);
+  CHECK_PARSE("ConstStyle: After", ConstStyle, FormatStyle::CS_Right);
+  CHECK_PARSE("ConstStyle: Before", ConstStyle, FormatStyle::CS_Left);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -14994,6 +15010,194 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const style unaltered
+  VERIFYFORMAT("const int a;", Style);
+  VERIFYFORMAT("const int *a;", Style);
+  VERIFYFORMAT("const int ", Style);
+  VERIFYFORMAT("const int &", Style);
+  VERIFYFORMAT("int const b;", Style);
+  VERIFYFORMAT("int const *b;", Style);
+  VERIFYFORMAT("int const ", Style);
+  VERIFYFORMAT("int const &", Style);
+  VERIFYFORMAT("int const *b const;", Style);
+  VERIFYFORMAT("int *const c;", Style);
+
+  VERIFYFORMAT("const Foo a;", Style);
+  VERIFYFORMAT("const Foo *a;", Style);
+  VERIFYFORMAT("const Foo ", Style);
+  VERIFYFORMAT("const Foo &", Style);
+  VERIFYFORMAT("Foo const b;", Style);
+  VERIFYFORMAT("Foo const *b;", Style);
+  VERIFYFORMAT("Foo const ", Style);
+  VERIFYFORMAT("Foo const &", Style);
+  VERIFYFORMAT("Foo const *b const;", Style);
+
+  VERIFYFORMAT("LLVM_NODISCARD const int ();", Style);
+  VERIFYFORMAT("LLVM_NODISCARD int const ();", Style);
+
+  VERIFYFORMAT("volatile const int *restrict;", Style);
+  VERIFYFORMAT("const volatile int *restrict;", Style);
+  VERIFYFORMAT("const int volatile *restrict;", Style);
+
+  Style.ConstStyle = FormatStyle::CS_Right;
+
+  VERIFYFORMAT("int const a;", Style);
+  VERIFYFORMAT("int const *a;", Style);
+  VERIFYFORMAT("int const ", Style);
+  VERIFYFORMAT("int const &", Style);
+  VERIFYFORMAT("int const b;", Style);
+  VERIFYFORMAT("int const *b;", Style);
+  VERIFYFORMAT("int const ", Style);
+  VERIFYFORMAT("int const &", Style);
+  VERIFYFORMAT("int const *b const;", Style);
+  VERIFYFORMAT("int *const c;", Style);
+
+  VERIFYFORMAT("Foo 

[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

2019-11-19 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:2721
+  From = ImpCastExprToType(From, DestASTy, CK_AddressSpaceConversion,
+   From->getValueKind()).get();
+}

Both the source and dest types here are off by a level if we're working with a 
pointer; you need to do:

```
  auto FromRecordTypeWithoutAS = 
Context.removeAddrSpaceQualType(FromRecordType);
  auto FromTypeWithDestAS = 
Context.getAddrSpaceQualType(FromRecordTypeWithoutAS, DestAS);
  if (PointerConversions)
   FromTypeWithDestAS = Context.getPointerType(FromTypeWithDestAS);
```


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

https://reviews.llvm.org/D69810



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor added inline comments.



Comment at: llvm/include/llvm/IR/IRBuilder.h:262
 Function *F = BB->getParent();
-if (!F->hasFnAttribute(Attribute::StrictFP)) {
+if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
   F->addFnAttr(Attribute::StrictFP);

kpn wrote:
> rjmccall wrote:
> > kpn wrote:
> > > This looks reasonable to me. 
> > > 
> > > It smells like there's a larger strictfp IRBuilder issue, but that's not 
> > > an issue for this patch here. The larger issue won't be hit since the new 
> > > options affect the entire compilation. It therefore shouldn't block this 
> > > patch.
> > Does IRBuilder actually support inserting into an unparented basic block?  
> > I feel like this is exposing a much more serious mis-use of IRBuilder.
> I suspect you are correct. If we let this "F && " change go in we'll have a 
> situation where whether or not a block is currently in a function when a 
> function call is emitted will affect whether or not the eventual function 
> definition gets the strictfp attribute. That seems like an unfortunate 
> inconsistency.
> 
> I'm still looking into it. I hope to have an IRBuilder review up today or 
> tomorrow.
As I just commented on the related patch @kpn posted, it appears that IRBuilder 
doesn't entirely support inserting into an unparented block. I was surprised by 
this, but there are places that need to be able to get to the Module from the 
BasicBlock. So, I think something problematic may be happening in the failing 
case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

2019-11-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

You're now supposed to push directly to github.
You might also have missed 
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063219.html.


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

https://reviews.llvm.org/D59516



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


[clang] c444a01 - fixe leak found by asan build bot

2019-11-19 Thread via cfe-commits

Author: Tyker
Date: 2019-11-19T21:11:37+01:00
New Revision: c444a01df3553d9405cbd4dac3c1074a71e6c2e1

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

LOG: fixe leak found by asan build bot

Added: 


Modified: 
clang/lib/AST/DeclCXX.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index bca560c40aef..6908b6a2b0a9 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -2818,8 +2818,10 @@ StorageDuration 
LifetimeExtendedTemporaryDecl::getStorageDuration() const {
 APValue *LifetimeExtendedTemporaryDecl::getOrCreateValue(bool MayCreate) const 
{
   assert(getStorageDuration() == SD_Static &&
  "don't need to cache the computed value for this temporary");
-  if (MayCreate && !Value)
+  if (MayCreate && !Value) {
 Value = (new (getASTContext()) APValue);
+getASTContext().addDestruction(Value);
+  }
   assert(Value && "may not be null");
   return Value;
 }



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-19 Thread Melanie Blower via Phabricator via cfe-commits
mibintc marked 4 inline comments as done.
mibintc added a comment.

inline replies to Michele, will upload a new patch shortly




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2408
+case options::OPT_frounding_math:
+  // The default setting for frounding-math is True and ffast-math
+  // sets fno-rounding-math, but we only want to use constrained

michele.scandale wrote:
> Isn't the default `-fno-rounding-math`?
Yes the default is no rounding math, I'll remove the comment.  Thank you. 



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2416
+  RoundingFPMath = false;
+  RoundingMathPresent = false;
+  break;

michele.scandale wrote:
> Shouldn't this be set to `true` similarly to what you do for 
> `TrappingMathPresent ` to track whether there is an explicit option related 
> to rounding math?
There's a switch statement above this that interprets the command line option 
-fp-model=strict as though frounding had appeared on the command line by 
assigning a new value to optID so that's why there is a discrepancy.  Also I'm 
using the *Present boolean variables to preserve the output from Driver so that 
pre-existing driver test cases don't need to be changed.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2574
+// FP Exception Behavior is also set to strict
+assert(FPExceptionBehavior.equals("strict"));
+CmdArgs.push_back("-ftrapping-math");

michele.scandale wrote:
> Running `clang -### -ftrapping-math -ffp-exception-behavior=ignore` lead to 
> this assertion to fail. As far as I can see `TrappingMath` is not changed in 
> the case FPExceptionBehavior is "ignore" or "maytrap".
> Clearly in the "ignore" case it should be safe to just set `TrappingMath` to 
> false, but I'm not sure about the "maytrap" case.
> It seems that `-ffp-exception-behavior` is more general than 
> `-f{,no-}trapping-math`, so it seems natural to me to see `ftrapping-math` 
> and `foo-trapping-math` as aliases for `ffp-exception-behavior=strict` and 
> `ffp-exception-behavior=ignore` respectively. If we agree on this, then I 
> would expect the reasoning inside the compiler only in terms of 
> `FPExceptionBehavior`.
Thanks for pointing out this assertion failure, I will upload a patch with fix. 
 Yes we could entirely express ftrapping-math and fno-trapping-math via the 
ffp-exception-behavior= option. That would probably be better--currently the 
trapping option becomes effective via the exception behavior parameter to the 
llvm floating point constrained intrinsics, and it can take 3 values. I thought 
it would be too radical at the moment, so I didn't propose that in this patch.

In the patch I'm about to add, I added a test case for the assertion that you 
saw.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");

michele.scandale wrote:
> With this change if I run `clang -### -ffast-math test.c` I don't see 
> `-fno-trapping-math` passed to the CC1. 
> This is changing the changes the value of the function level attribute 
> "no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
> Is this an intended change?
> 
> Moreover since with this patch the default value for trapping math changed, 
> the "no-trapping-math" function level attribute is incorrect also for default 
> case.
Before this patch, ftrapping-math was added to the Driver and also a bitfield, 
``NoTrappingFPMath`` was created in the LLVM to describe the state of 
trapping-math, but otherwise that bit wasn't consulted and the option had no 
effect.  Gcc describes ftrapping-math as the default, but in llvm by default 
floating point exceptions are masked and this corresponds to the floating point 
Constrained Intrinsics having exception behavior set to ignored.  This patch 
changed the llvm constructor to set the trapping bit to "no trap".  In fact I'd 
like to get rid of the ``NoTrappingFPMath`` bitfield since it's not being used, 
but I didn't make that change at this point. 

If I remember correctly, there are a bunch of driver tests that failed if 
fno-trapping-math is output to cc1. I'd have to reconstruct the details.  Since 
fno-trapping-math is the default, it isn't passed through on the cc1 command 
line: the Clang.cpp driver doesn't pass through the positive and negative for 
each existing option.

Thanks for pointing out the line in CGCall.cpp, it seems the CodeGenOpts aren't 
getting set up perfectly I'll fix that in CompilerInvocation.cpp; I don't see 
anything setting trapping-math as part of function level attribute, 
@michele.scandale  did I overlook that/can you point out where that is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731




[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

I have some vague recollection that the DWARF C bindings didn't have stability 
guarantees? Does that sound familiar to anyone? What sort of changes have been 
made to them in the past?




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:804-810
+  // attribute in DW_TAG_typedef DIE.
+  if (Tag == dwarf::DW_TAG_typedef && DD->getDwarfVersion() >= 5) {
+uint32_t AlignInBytes = DTy->getAlignInBytes();
+if (AlignInBytes > 0)
+  addUInt(Buffer, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
+  AlignInBytes);
+  }

Should there be some general utility/appropriate point to add alignment to any 
type? (generalized over the other places we're already adding alignment on 
types (such as in DwarfUnit::constructTypeDIE(DIE , const 
DICompositeType *CTy), maybe other places) - probably doesn't generalize over 
all places that add DW_AT_alignment, like global/static variables, etc, which 
is fine)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D70340#1751148 , @hans wrote:

> Nice!
>
> Silly questions, but for my own education: I thought the key function concept 
> only existed in the Itanium ABI, but from your numbers it sounds like it's a 
> concept, at least for debug info, also on Windows?


There's sort of two things going on:

- -flimit-debug-info: if a type has a vtable, debug info for the class is only 
emitted where the vtable is emitted, on the assumption that we believe the 
vtable will be in the program somewhere.
- key functions in the ABI: these optimize object file size by avoiding the 
need to emit the vtable in as many places.

The -flimit-debug-info behavior is cross-platform and happens regardless of 
whether the class has a key function. So, clang only emits a forward 
declaration of Foo in the debug info for this program, regardless of target:

  struct Foo {
Foo();
~Foo();
virtual void f() {}
  };
  Foo *makeFoo() { return new Foo(); }

-flimit-debug-info would emit complete type info if the constructor (which 
touches the vtable) was inline.

---

I'll try to land this today, I think it's worth doing. If anyone thinks it's 
too much of a hack, let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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


[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

2019-11-19 Thread Marcin Twardak via Phabricator via cfe-commits
twardakm marked 9 inline comments as done.
twardakm added a comment.

Thanks for the review!

@aaron.ballman take a look at new revision and let me know if something needs 
to be fixed. Thanks!




Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:50-51
+
+Check->addMacro(MacroNameTok.getIdentifierInfo()->getName().str(),
+Value.str());
+  }

aaron.ballman wrote:
> You only need to add the macro name in this case, not its value, which should 
> simplify this code considerably.
See comment above, now both value and definition is used



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(std::make_unique(PP, this));
+}

aaron.ballman wrote:
> Rather than making an entire new object for PP callbacks, why not make 
> `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it would 
> simplify the interface somewhat.
I think the check hast to inherit from ClangTidyCheck?

I duplicated the solution from other checks (e.g. IdentifierNamingCheck.cpp). 
Could you please point to some existing check which implements your idea?



Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h:44
+  // preprocessed define
+  std::vector> Macros;
 };

aaron.ballman wrote:
> I think a better approach is to use a set of some sort because the value of 
> the macro is never used in the check. Probably a `SmallPtrSet` over 
> `StringRef`.
After applying other comments, both value and definition is used, therefore I 
stayed with pair of strings. Let me know if you think this comment is still 
valid


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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


[PATCH] D70285: Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

2019-11-19 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

lgtm as long as other compilers don't warn on unknown pragmas by default.


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

https://reviews.llvm.org/D70285



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


[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

2019-11-19 Thread Marcin Twardak via Phabricator via cfe-commits
twardakm updated this revision to Diff 230113.
twardakm added a comment.

Apply Aaron's comments

- Fix missing namespace to macro definition instead of extension
- Print macro definition also in warning message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855

Files:
  clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
  clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s llvm-namespace-comment %t
+
+namespace n1 {
+namespace n2 {
+  void f();
+
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'n2' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+1]]:2: warning: namespace 'n1' not terminated with a closing comment [llvm-namespace-comment]
+}}
+// CHECK-FIXES: } // namespace n2
+// CHECK-FIXES: } // namespace n1
+
+#define MACRO macro_expansion
+namespace MACRO {
+  void f();
+  // CHECK-MESSAGES: :[[@LINE+1]]:1: warning: namespace 'MACRO' not terminated with a closing comment [llvm-namespace-comment]
+}
+// CHECK-FIXES: } // namespace MACRO
+
+namespace MACRO {
+  void g();
+} // namespace MACRO
+
+namespace MACRO {
+  void h();
+  // CHECK-MESSAGES: :[[@LINE+1]]:2: warning: namespace 'MACRO' ends with a comment that refers to an expansion of macro [llvm-namespace-comment]
+} // namespace macro_expansion
+// CHECK-FIXES: } // namespace MACRO
+
+namespace n1 {
+namespace MACRO {
+namespace n2 {
+  void f();
+  // CHECK-MESSAGES: :[[@LINE+3]]:1: warning: namespace 'n2' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'MACRO' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: namespace 'n1' not terminated with a closing comment [llvm-namespace-comment]
+}}}
+// CHECK-FIXES: } // namespace n2
+// CHECK-FIXES: } // namespace MACRO
+// CHECK-FIXES: } // namespace n1
Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
@@ -25,10 +25,10 @@
 // 5
 // 6
 // 7
-// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'macro_expansion' not terminated with
-// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts here
+// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'MACRO' not terminated with
+// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'MACRO' starts here
 }
-// CHECK-FIXES: }  // namespace macro_expansion
+// CHECK-FIXES: }  // namespace MACRO
 
 namespace short1 {
 namespace short2 {
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
===
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
+++ clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
@@ -26,14 +26,29 @@
   NamespaceCommentCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+
+  void addMacro(const std::string , const std::string ) noexcept;
 
 private:
   void storeOptions(ClangTidyOptions::OptionMap ) override;
+  std::string getNamespaceComment(const NamespaceDecl *ND,
+  bool InsertLineBreak);
+  std::string getNamespaceComment(const std::string ,
+  bool InsertLineBreak);
+  bool isNamespaceMacroDefinition(const StringRef NameSpaceName);
+  std::tuple
+  isNamespaceMacroExpansion(const StringRef NameSpaceName);
 
   llvm::Regex NamespaceCommentPattern;
   const unsigned ShortNamespaceLines;
   const unsigned SpacesBeforeComments;
   llvm::SmallVector Ends;
+
+  // Store macros to verify that warning is not thrown when namespace name is a
+  // preprocessed define.
+  std::vector> Macros;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
+++ 

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

2019-11-19 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments.



Comment at: clang/unittests/Driver/SanitizerArgsTest.cpp:100
+  Contains(StrEq(std::string("-fsanitize-system-blacklist=") +
+ ASanBlacklist)));
+  // User blacklists should also be added.

ilya-biryukov wrote:
> I'm pretty sure this is going to fail on Windows, looking for ideas on how to 
> unify this with minimal code...
> Let me know if there's some cool trick for this that I should know...
Would `llvm::sys::path::convert_to_slash()` help?



Comment at: clang/unittests/Driver/SanitizerArgsTest.cpp:133
+} // namespace
\ No newline at end of file


Nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70440



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


[PATCH] D69648: Add VFS support for sanitizers' blacklist' 2

2019-11-19 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment.

Ahh, right. I originally wanted to support `-ivfsoverlay` in Driver as it 
seemed reasonable to check the existence of blacklist in Driver and open it in 
cc1 using the same fs. I got talked out of it and I didn't touch the Driver but 
seems like I should've replaced the calls to native fs with VFS to keep things 
consistent. Thanks for picking this up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69648



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


[PATCH] D69620: Add AIX assembler support

2019-11-19 Thread Steven Wan via Phabricator via cfe-commits
stevewan marked an inline comment as done.
stevewan added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:45
+  // Acccept any mixture of instructions.
+  CmdArgs.push_back("-many");
+

Xiangling_L wrote:
> GCC invokes system assembler also with options `-mpwr4` and `-u`, I think you 
> need to verify that do we need those? And as far as I can recall, `-mpwr4` is 
> to pick up new version AIX instruction set, and `-u` is to suppress warning 
> for undefined symbols. 90% sure that we need `-mpwr4`(I could be wrong), but 
> not sure about `-u`.
For the `-u` option, I'll be adding it to this patch with a FIXME that we'd 
like to remove it in the future. The rationale is that, since currently we do 
not have the assembly writing ready that writes the extern(s), we'll pass in 
this `-u` to suppress any related warnings. Once the assembly writing is ready, 
we should remove this flag as it potentially covers up for undefined symbols 
that are actually problematic.

For the `-m` option, adding `-many` seemed to be more suitable at the moment. 
During code generation, the compiler should've already guaranteed that it used 
a correct and appropriate instruction set (i.e., conforms to the user specified 
driver option `-mcpu=`). The `-m` assembler option here double checks the same 
thing that's already been checked at codegen. That said, we can just pass in 
`-many` to accept all and rely on codegen to do the checking. On the other 
hand, potential problems might get away with it in one of the two scenarios I 
can think of right now,


  # User passes in a `.s` assembly file that uses instructions from the 
super-set of what's specified in `-mcpu=`. This is mostly on the user side 
because essentially they pass in contradictory inputs that are not going to fly.

  # User passes in a `.c` source file, but codegen hit some issue and falsely 
decides to use a super-set of what's specified in `-mcpu=`. Given that we don't 
have codegen ready yet, we don't know how reliable it might be. I would suggest 
that we keep `-many` as it is for now, and we may change it when needed once we 
are more clear on code generation. 


With regard to GCC's behaviour, GCC would append first `-mpwr4` then `-many` to 
the system assembler, which effectively means adding `-many` solely because the 
last `-m` flag would overwrite all its preceding one(s).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620



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


[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

2019-11-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 13 inline comments as done.
ilya-biryukov added a comment.

In D69330#1746137 , @rsmith wrote:

> I would prefer that we have dedicated tests for them rather than scattering 
> the tests throughout the existing test suite (for example, consider adding 
> `-Wno-unused` to the tests producing "result unused" warnings and adding a 
> dedicated test).


Most updated tests (including those with more "result unused" warnings) are 
actually not intended to test recovery expressions, they just happen to produce 
different results now and need to be updated.
The only new dedicated tests here are `clang/test/AST/ast-dump-recovery.cpp` 
and `clang/test/Index/getcursor-recovery.cpp`.

Could technically move them into the same directory, but wanted to make sure I 
got your point first. Could you elaborate on what testing strategy you'd 
prefer? Also eager for suggestions on more things I could test, but not sure 
what kinds of tests people normally add when adding new expressions. Any advice 
here is highly appreciated.




Comment at: clang/include/clang/Sema/Sema.h:3568-3569
+  /// Corrects typos inside \p SubExprs.
+  ExprResult ActOnSemanticError(SourceLocation Begin, SourceLocation End,
+ArrayRef SubExprs);
+

rsmith wrote:
> Generally the `ActOn*` interface is invoked by the parser in response to 
> syntactic constructs, so `ActOnSemanticError` is a surprising function name. 
> Maybe `CreateRecoveryExpr` would be better. Is that too narrow for the 
> intended semantics of this function?
`CreateRecoveryExpr` looks good, the only additional logic in the function is 
not producing recovery expressions in the C mode. But I would expect this to go 
away in the future.



Comment at: clang/lib/Sema/SemaExceptionSpec.cpp:1287
 // FIXME: Can any of the above throw?  If so, when?
 return CT_Cannot;
 

rsmith wrote:
> Should we return `CT_Dependent` for `RecoveryExprClass`, since we model it as 
> being dependent?
Makes total sense, thanks!



Comment at: clang/lib/Sema/SemaExpr.cpp:17978
+  }
+  return RecoveryExpr::Create(Context, Begin, End, NoTypos);
+}

rsmith wrote:
> We shouldn't create these in code synthesis contexts (eg, during SFINAE 
> checks in template instantiations).
Good point, thanks. Added the corresponding check. Also made sure that we 
always typo-correct in arguments before checking whether we can produce 
recovery expressions.

I don't think we have any code that calls into `CreateRecoveryExpression` 
(formerly `ActOnSemanticError`) from SFINAE context, all calls are currently in 
the parser. Not 100% certain this does not happen with 
`-fdelayed-template-parsing`, though, so ended up returning null instead of 
asserting we're not in SFINAE context.





Comment at: clang/lib/Sema/TreeTransform.h:9470
+ExprResult TreeTransform::TransformRecoveryExpr(RecoveryExpr *E) {
+  return E;
+}

rsmith wrote:
> We should either transform the subexpressions or just return `ExprError()` 
> here. With this approach, we can violate AST invariants (eg, by having the 
> same `Expr` reachable through two different code paths in the same function 
> body, or by retaining `DeclRefExpr`s referring to declarations from the wrong 
> context, etc).
Thanks, I just blindly copied what TypoExpr was doing without considering the 
consequences here.


Added the code to rebuild `RecoveryExpr`.

Any good way to test this?



Comment at: clang/test/SemaCXX/builtin-operator-new-delete.cpp:94
   void *p = __builtin_operator_new(42, std::align_val_t(2)); // expected-error 
{{call to '__builtin_operator_new' selects non-usual allocation function}}
-  __builtin_operator_delete(p, std::align_val_t(2)); // expected-error 
{{call to '__builtin_operator_delete' selects non-usual deallocation function}}
+  __builtin_operator_delete((void*)0, std::align_val_t(2)); // 
expected-error {{call to '__builtin_operator_delete' selects non-usual 
deallocation function}}
 #endif

rsmith wrote:
> What happens to the original testcase here? I wouldn't expect the invalid 
> call expression in the initializer of `p` to affect whether we diagnose uses 
> of `p`.
> 
> (Nit: remove spaces to realign the comments here.)
Sorry, that was a leftover from previous revisions. This was here by accident, 
I reverted this.

A bit more context on what happened here: if we mark any decl as invalid, 
references to those variables do not parse into `DeclRefExpr`, the 
corresponding sema function returns `null` instead.
This leads to diagnostics like this being lost. In one of my attempts, I marked 
more decls as invalid and, in turn, to these tests failing.



Comment at: clang/test/SemaOpenCLCXX/address-space-references.cl:15
+  bar(1) // expected-error{{binding reference of type 

[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

2019-11-19 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf55cd39f1913: [C-index] Fix test when using Debug target 
 MSVC STL (authored by aganea).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69959

Files:
  clang/include/clang/Frontend/FrontendOptions.h


Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -366,7 +366,7 @@
   std::string ARCMTMigrateReportOut;
 
   /// The input files and their types.
-  std::vector Inputs;
+  SmallVector Inputs;
 
   /// When the input is a module map, the original module map file from which
   /// that map was inferred, if any (for umbrella modules).


Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -366,7 +366,7 @@
   std::string ARCMTMigrateReportOut;
 
   /// The input files and their types.
-  std::vector Inputs;
+  SmallVector Inputs;
 
   /// When the input is a module map, the original module map file from which
   /// that map was inferred, if any (for umbrella modules).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

2019-11-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 230104.
ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added a comment.

- Rename ActOnSemanticError to CreateRecoveryExpr
- Do not print anything for recovery expr in TextNodeDumper
- canThrow(RecoveryExpr) now returns CT_Dependent
- Store Expr* instead of Stmt* in RecoveryExpr
- Transform subexpressions of RecoveryExpr in TreeTransform
- Do not produce RecoveryExpr in SFINAE contexts
- Updated some test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69330

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/AST/ast-dump-expr-errors.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/Index/getcursor-recovery.cpp
  clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
  clang/test/Parser/objcxx0x-lambda-expressions.mm
  clang/test/Parser/objcxx11-invalid-lambda.cpp
  clang/test/SemaCXX/builtins.cpp
  clang/test/SemaCXX/cast-conversion.cpp
  clang/test/SemaCXX/cxx1z-copy-omission.cpp
  clang/test/SemaCXX/decltype-crash.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/test/SemaOpenCLCXX/address-space-references.cl
  clang/test/SemaTemplate/instantiate-init.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -291,6 +291,7 @@
   case Stmt::ObjCDictionaryLiteralClass:
   case Stmt::ObjCBoxedExprClass:
   case Stmt::ObjCSubscriptRefExprClass:
+  case Stmt::RecoveryExprClass:
 K = CXCursor_UnexposedExpr;
 break;
 
Index: clang/test/SemaTemplate/instantiate-init.cpp
===
--- clang/test/SemaTemplate/instantiate-init.cpp
+++ clang/test/SemaTemplate/instantiate-init.cpp
@@ -100,7 +100,7 @@
 integral_c<1> ic1 = array_lengthof(Description::data);
 (void)sizeof(array_lengthof(Description::data));
 
-sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
+(void)sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
   Description::data // expected-note{{in instantiation of static data member 'PR7985::Description::data' requested here}}
   ));
 
Index: clang/test/SemaOpenCLCXX/address-space-references.cl
===
--- clang/test/SemaOpenCLCXX/address-space-references.cl
+++ clang/test/SemaOpenCLCXX/address-space-references.cl
@@ -11,5 +11,5 @@
 int bar(const unsigned int );
 
 void foo() {
-  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+  bar(1); // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
 }
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -22,7 +22,8 @@
 // default ctor.
 void record_context(int a, ...) {
   struct Foo {
-// expected-error@+1 {{'va_start' cannot be used outside a function}}
+// expected-error@+2 {{'va_start' cannot be used outside a function}}
+// expected-error@+1 {{default argument references parameter 'a'}}
 void meth(int a, int b = (__builtin_va_start(ap, a), 0)) {}
   };
 }
Index: clang/test/SemaCXX/decltype-crash.cpp
===
--- clang/test/SemaCXX/decltype-crash.cpp
+++ clang/test/SemaCXX/decltype-crash.cpp
@@ -3,5 +3,8 @@
 int& a();
 
 void f() {
-  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} expected-error {{use of undeclared identifier 'decltype'}}
+  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} \
+   // expected-error {{use of undeclared identifier 'decltype'}} \
+   // expected-error {{expected ';' after expression}} \
+   // expected-error {{use of undeclared identifier 'c'}}
 }
Index: clang/test/SemaCXX/cxx1z-copy-omission.cpp

[clang] f55cd39 - [C-index] Fix test when using Debug target & MSVC STL

2019-11-19 Thread Alexandre Ganea via cfe-commits

Author: Alexandre Ganea
Date: 2019-11-19T13:30:40-05:00
New Revision: f55cd39f19134392b16bc1fd6c558214778a3bb8

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

LOG: [C-index] Fix test when using Debug target & MSVC STL

Avoids a deadlock in "clang/test/Index/crash-recovery-modules.m" when building 
with the MSVC STL & _ITERATOR_DEBUG_LEVEL == 2 (meaning a DEBUG build)

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

Added: 


Modified: 
clang/include/clang/Frontend/FrontendOptions.h

Removed: 




diff  --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 09d83adf579b..70eb3425b0da 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -366,7 +366,7 @@ class FrontendOptions {
   std::string ARCMTMigrateReportOut;
 
   /// The input files and their types.
-  std::vector Inputs;
+  SmallVector Inputs;
 
   /// When the input is a module map, the original module map file from which
   /// that map was inferred, if any (for umbrella modules).



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


[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

2019-11-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

In D69950#1751894 , @eandrews wrote:

> In D69950#1751859 , @gribozavr2 
> wrote:
>
> > > hence not throwing the warning on any platform?
> >
> > The way I read the buildbot breakage, an existing ClangTidy test passed 
> > before and after this change, but broke on Windows. The breakage was that 
> > the warnings stopped being produced.
>
>
> The existing test does not have this warning. I modified the test to add the 
> check for this warning since it is generated on Linux after my patch. It is 
> not generated on Windows because of delayed template parsing.


Is it just that warning that is not generated, or all warnings in that test 
file?

Anyhow, adding `-fno-delayed-template-parsing` to a ClangTidy test is not 
unreasonable, and is indeed existing practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69950



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


[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

2019-11-19 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Yes, in that case copy-elision into the global variable is guaranteed.  You can 
write arbitrary expressions in global initializers, however, and those can use 
temporary lambdas.


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

https://reviews.llvm.org/D69938



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


[PATCH] D68155: [clang][NFC] Make various uses of Regex const

2019-11-19 Thread Nicolas Guillemot via Phabricator via cfe-commits
nlguillemot added a comment.

In D68155#1751826 , @thopre wrote:

> My sincere apologies, I forgot to change the author in the commit. I don't 
> often commit on behalf of others so didn't think enough.


No worries. Thanks again for helping with the commit!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68155



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


[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

2019-11-19 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh updated this revision to Diff 230096.
svenvh edited the summary of this revision.
svenvh added a comment.

Rework fix to insert an addrspace conversion node into the AST instead of 
catching the addrspace cast in CGCall.


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

https://reviews.llvm.org/D69810

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl


Index: clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -28,6 +28,7 @@
 class B2 {
   public:
 void baseMethod() const {  }
+int& getRef() { return bb; }
 int bb;
 };
 
@@ -46,7 +47,17 @@
 
 void pr43145_2(B *argB) {
   Derived *x = (Derived*)argB;
+  // CHECK-LABEL: @_Z9pr43145_2
+  // CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
 }
 
-// CHECK-LABEL: @_Z9pr43145_2
-// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
+// Assigning to reference returned by base class method through derived class.
+
+void pr43145_3(int n) {
+  Derived d;
+  d.getRef() = n;
+  // CHECK-LABEL: @_Z9pr43145_3
+  // CHECK: addrspacecast %class.Derived* %d to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2711,6 +2711,15 @@
   FromRecordType = FromType;
   DestType = DestRecordType;
 }
+
+LangAS FromAS = FromRecordType.getAddressSpace();
+LangAS DestAS = DestRecordType.getAddressSpace();
+if (FromAS != DestAS) {
+  auto FromTyWithoutAS = Context.removeAddrSpaceQualType(FromType);
+  auto DestASTy = Context.getAddrSpaceQualType(FromTyWithoutAS, DestAS);
+  From = ImpCastExprToType(From, DestASTy, CK_AddressSpaceConversion,
+   From->getValueKind()).get();
+}
   } else {
 // No conversion necessary.
 return From;


Index: clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -28,6 +28,7 @@
 class B2 {
   public:
 void baseMethod() const {  }
+int& getRef() { return bb; }
 int bb;
 };
 
@@ -46,7 +47,17 @@
 
 void pr43145_2(B *argB) {
   Derived *x = (Derived*)argB;
+  // CHECK-LABEL: @_Z9pr43145_2
+  // CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
 }
 
-// CHECK-LABEL: @_Z9pr43145_2
-// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
+// Assigning to reference returned by base class method through derived class.
+
+void pr43145_3(int n) {
+  Derived d;
+  d.getRef() = n;
+  // CHECK-LABEL: @_Z9pr43145_3
+  // CHECK: addrspacecast %class.Derived* %d to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2711,6 +2711,15 @@
   FromRecordType = FromType;
   DestType = DestRecordType;
 }
+
+LangAS FromAS = FromRecordType.getAddressSpace();
+LangAS DestAS = DestRecordType.getAddressSpace();
+if (FromAS != DestAS) {
+  auto FromTyWithoutAS = Context.removeAddrSpaceQualType(FromType);
+  auto DestASTy = Context.getAddrSpaceQualType(FromTyWithoutAS, DestAS);
+  From = ImpCastExprToType(From, DestASTy, CK_AddressSpaceConversion,
+   From->getValueKind()).get();
+}
   } else {
 // No conversion necessary.
 return From;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

There are two options here:

- leave the C bindings as is (fine with me)
- add an overloaded function to the C bindings that has the extra argument 
(also fine with me).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D69493: Add -fconvergent-functions flag

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

e531750c6cf9ab6ca987ffbfe100b1d766269eb5 



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

https://reviews.llvm.org/D69493



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


[clang] e531750 - clang: Add -fconvergent-functions flag

2019-11-19 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2019-11-19T23:20:15+05:30
New Revision: e531750c6cf9ab6ca987ffbfe100b1d766269eb5

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

LOG: clang: Add -fconvergent-functions flag

The CUDA builtin library is apparently compiled in C++ mode, so the
assumption of convergent needs to be made in a typically non-SPMD
language. The functions in the library should still be assumed
convergent. Currently they are not, which is potentially incorrect and
this happens to work after the library is linked.

Added: 
clang/test/CodeGen/convergent-functions.cpp

Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGenCUDA/propagate-metadata.cu

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index eba4f835d661..6ec55ded3c84 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -122,6 +122,7 @@ LANGOPT(WritableStrings   , 1, 0, "writable string support")
 LANGOPT(ConstStrings  , 1, 0, "const-qualified string support")
 ENUM_LANGOPT(LaxVectorConversions, LaxVectorConversionKind, 2,
  LaxVectorConversionKind::All, "lax vector conversions")
+LANGOPT(ConvergentFunctions, 1, 1, "Assume convergent functions")
 LANGOPT(AltiVec   , 1, 0, "AltiVec-style vector initializers")
 LANGOPT(ZVector   , 1, 0, "System z vector extensions")
 LANGOPT(Exceptions, 1, 0, "exception handling")

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 5f808f04e9ae..befe2f19792c 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -312,7 +312,7 @@ class LangOptions : public LangOptionsBase {
   }
 
   bool assumeFunctionsAreConvergent() const {
-return (CUDA && CUDAIsDevice) || OpenCL;
+return ConvergentFunctions;
   }
 
   /// Return the OpenCL C or C++ version as a VersionTuple.

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index ab629598eab9..1f0fc97b14e2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -546,6 +546,9 @@ def cxx_isystem : JoinedOrSeparate<["-"], "cxx-isystem">, 
Group,
   MetaVarName<"">;
 def c : Flag<["-"], "c">, Flags<[DriverOption]>, Group,
   HelpText<"Only run preprocess, compile, and assemble steps">;
+def fconvergent_functions : Joined<["-"], "fconvergent-functions">, 
Group, Flags<[CC1Option]>,
+  HelpText<"Assume functions may be convergent">;
+
 def cuda_device_only : Flag<["--"], "cuda-device-only">,
   HelpText<"Compile CUDA code for device only">;
 def cuda_host_only : Flag<["--"], "cuda-host-only">,

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 17ef037f3e6d..9ec242308116 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2776,6 +2776,9 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
   Opts.BlocksRuntimeOptional = Args.hasArg(OPT_fblocks_runtime_optional);
   Opts.Coroutines = Opts.CPlusPlus2a || Args.hasArg(OPT_fcoroutines_ts);
 
+  Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
+Args.hasArg(OPT_fconvergent_functions);
+
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,
OPT_fno_double_square_bracket_attributes,

diff  --git a/clang/test/CodeGen/convergent-functions.cpp 
b/clang/test/CodeGen/convergent-functions.cpp
new file mode 100644
index ..7ddb8d3f9450
--- /dev/null
+++ b/clang/test/CodeGen/convergent-functions.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple i386-pc-win32 -emit-llvm -fconvergent-functions -o 
- < %s | FileCheck -check-prefix=CONVFUNC %s
+// RUN: %clang_cc1 -triple i386-pc-win32 -emit-llvm -o - < %s | FileCheck 
-check-prefix=NOCONVFUNC %s
+
+// Test that the -fconvergent-functions flag works
+
+// CONVFUNC: attributes #0 = { convergent {{.*}} }
+// NOCONVFUNC-NOT: convergent
+void func() { }

diff  --git a/clang/test/CodeGenCUDA/propagate-metadata.cu 
b/clang/test/CodeGenCUDA/propagate-metadata.cu
index 773dd8afba81..52c88d3c8065 100644
--- a/clang/test/CodeGenCUDA/propagate-metadata.cu
+++ b/clang/test/CodeGenCUDA/propagate-metadata.cu
@@ -11,7 +11,7 @@
 
 // Build the bitcode library.  This is not built in CUDA mode, otherwise it
 // might have incompatible attributes.  This mirrors how libdevice is built.
-// RUN: %clang_cc1 -x c++ -emit-llvm-bc -ftrapping-math -DLIB \
+// RUN: %clang_cc1 

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

2019-11-19 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision.
vsk added a comment.

Thanks!


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

https://reviews.llvm.org/D68206



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


  1   2   3   >