Re: LLVM Lab power outage

[Galina Kistanova via cfe-commits]

Hello everyone,

The Lab is back to normal.
Thanks for you patience and understanding.

Thanks

Galina



On 10/5/18, Galina Kistanova  wrote:
> Hello everyone,
>
> PG&E experience a major power outage.
> LLVM Lab is affected. Buildbot is down.
> There is no ETA yet. I'll keep you posted.
>
> Thanks
>
> Galina
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In https://reviews.llvm.org/D52398#1257133, @aaronpuchert wrote:

> In https://reviews.llvm.org/D52398#1257092, @rupprecht wrote:
>
> > I patched the proposed fix-forward and it seems to have things building 
> > again. Is there an ETA on landing that? If it's going to take a bit, is 
> > there any chance we could revert this patch until then?
>
>
> The fix was just committed (https://reviews.llvm.org/rC343902). Generally I'm 
> for reverting if there are functional deficiencies, but here it's more that 
> an unrelated issue was accidentally uncovered by this patch.


Ah, I thought it was an issue with this patch. Clearly I shouldn't pretend to 
understand thread analysis :)
Thanks for the fix!


Repository:
  rL LLVM

https://reviews.llvm.org/D52398



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

[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter

[Artem Dergachev via Phabricator via cfe-commits]

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

Just add tests, i guess!

Also i'll have a look at whether the checker is in a good shape to be enabled 
by default. I suspect that mismatched iterators might be very safe to enable. 
With all these solver and rearrangement issues, if they only cause false 
negatives, it's not blocking enabling the checker by default. The same applies 
to the debate around `find`.

One thing that's most likely necessary to do before enabling the checker by 
default is to add a bug report visitor, so that it added notes when iterators 
appear for the first time or change their state. Without such notes it's 
usually very hard to understand warnings. Especially because we're dropping the 
path within inlined functions that have no interesting events, but when the 
iterator originates from or gets updated within such function, this information 
becomes crucial. The visitor might have to hop from one object to another 
similarly to `trackNullOrUndefValue()` (i.e., by adding more instances of 
itself that track objects that are being copied or moved into the object of 
interest at the program point that is currently being visited).




Comment at: test/Analysis/mismatched-iterator.cpp:157
+void bad_empty(std::vector &v1, std::vector &v2) {
+  is_cend(v1, v2.cbegin()); // expected-warning@149{{Iterators of different 
containers used where the same container is expected}}
+}

Maybe use `@-8` instead, so that we only had to update it when *this* test 
changes?


https://reviews.llvm.org/D32906



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

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In https://reviews.llvm.org/D52398#1257092, @rupprecht wrote:

> I patched the proposed fix-forward and it seems to have things building 
> again. Is there an ETA on landing that? If it's going to take a bit, is there 
> any chance we could revert this patch until then?


The fix was just committed (https://reviews.llvm.org/rC343902). Generally I'm 
for reverting if there are functional deficiencies, but here it's more that an 
unrelated issue was accidentally uncovered by this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D52398



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

LLVM Lab power outage

[Galina Kistanova via cfe-commits]

Hello everyone,

PG&E experience a major power outage.
LLVM Lab is affected. Buildbot is down.
There is no ETA yet. I'll keep you posted.

Thanks

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

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343902: Thread safety analysis: Handle conditional 
expression in getTrylockCallExpr (authored by aaronpuchert, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52888?vs=168295&id=168557#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52888

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


Index: cfe/trunk/lib/Analysis/ThreadSafety.cpp
===
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp
@@ -1435,6 +1435,17 @@
 if (BOP->getOpcode() == BO_LOr)
   return getTrylockCallExpr(BOP->getRHS(), C, Negate);
 return nullptr;
+  } else if (const auto *COP = dyn_cast(Cond)) {
+bool TCond, FCond;
+if (getStaticBooleanValue(COP->getTrueExpr(), TCond) &&
+getStaticBooleanValue(COP->getFalseExpr(), FCond)) {
+  if (TCond && !FCond)
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+  if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+  }
+}
   }
   return nullptr;
 }
@@ -1449,7 +1460,8 @@
   Result = ExitSet;
 
   const Stmt *Cond = PredBlock->getTerminatorCondition();
-  if (!Cond)
+  // We don't acquire try-locks on ?: branches, only when its result is used.
+  if (!Cond || isa(PredBlock->getTerminator()))
 return;
 
   bool Negate = false;
Index: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1873,6 +1873,23 @@
int i = a;
mu.Unlock();
   }
+
+  // Test with conditional operator
+  void foo13() {
+if (mu.TryLock() ? 1 : 0)
+  mu.Unlock();
+  }
+
+  void foo14() {
+if (mu.TryLock() ? 0 : 1)
+  return;
+mu.Unlock();
+  }
+
+  void foo15() {
+if (mu.TryLock() ? 0 : 1) // expected-note{{mutex acquired here}}
+  mu.Unlock();// expected-warning{{releasing mutex 'mu' that 
was not held}}
+  }   // expected-warning{{mutex 'mu' is not held on 
every path through here}}
 };  // end TestTrylock
 
 } // end namespace TrylockTest


Index: cfe/trunk/lib/Analysis/ThreadSafety.cpp
===
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp
@@ -1435,6 +1435,17 @@
 if (BOP->getOpcode() == BO_LOr)
   return getTrylockCallExpr(BOP->getRHS(), C, Negate);
 return nullptr;
+  } else if (const auto *COP = dyn_cast(Cond)) {
+bool TCond, FCond;
+if (getStaticBooleanValue(COP->getTrueExpr(), TCond) &&
+getStaticBooleanValue(COP->getFalseExpr(), FCond)) {
+  if (TCond && !FCond)
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+  if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+  }
+}
   }
   return nullptr;
 }
@@ -1449,7 +1460,8 @@
   Result = ExitSet;
 
   const Stmt *Cond = PredBlock->getTerminatorCondition();
-  if (!Cond)
+  // We don't acquire try-locks on ?: branches, only when its result is used.
+  if (!Cond || isa(PredBlock->getTerminator()))
 return;
 
   bool Negate = false;
Index: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1873,6 +1873,23 @@
int i = a;
mu.Unlock();
   }
+
+  // Test with conditional operator
+  void foo13() {
+if (mu.TryLock() ? 1 : 0)
+  mu.Unlock();
+  }
+
+  void foo14() {
+if (mu.TryLock() ? 0 : 1)
+  return;
+mu.Unlock();
+  }
+
+  void foo15() {
+if (mu.TryLock() ? 0 : 1) // expected-note{{mutex acquired here}}
+  mu.Unlock();// expected-warning{{releasing mutex 'mu' that was not held}}
+  }   // expected-warning{{mutex 'mu' is not held on every path through here}}
 };  // end TestTrylock
 
 } // end namespace TrylockTest
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r343902 - Thread safety analysis: Handle conditional expression in getTrylockCallExpr

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Fri Oct  5 18:09:28 2018
New Revision: 343902

URL: http://llvm.org/viewvc/llvm-project?rev=343902&view=rev
Log:
Thread safety analysis: Handle conditional expression in getTrylockCallExpr

Summary:
We unwrap conditional expressions containing try-lock functions.

Additionally we don't acquire on conditional expression branches, since
that is usually not helpful. When joining the branches we would almost
certainly get a warning then.

Hopefully fixes an issue that was raised in D52398.

Reviewers: aaron.ballman, delesley, hokein

Reviewed By: aaron.ballman

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=343902&r1=343901&r2=343902&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Oct  5 18:09:28 2018
@@ -1435,6 +1435,17 @@ const CallExpr* ThreadSafetyAnalyzer::ge
 if (BOP->getOpcode() == BO_LOr)
   return getTrylockCallExpr(BOP->getRHS(), C, Negate);
 return nullptr;
+  } else if (const auto *COP = dyn_cast(Cond)) {
+bool TCond, FCond;
+if (getStaticBooleanValue(COP->getTrueExpr(), TCond) &&
+getStaticBooleanValue(COP->getFalseExpr(), FCond)) {
+  if (TCond && !FCond)
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+  if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+  }
+}
   }
   return nullptr;
 }
@@ -1449,7 +1460,8 @@ void ThreadSafetyAnalyzer::getEdgeLockse
   Result = ExitSet;
 
   const Stmt *Cond = PredBlock->getTerminatorCondition();
-  if (!Cond)
+  // We don't acquire try-locks on ?: branches, only when its result is used.
+  if (!Cond || isa(PredBlock->getTerminator()))
 return;
 
   bool Negate = false;

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=343902&r1=343901&r2=343902&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Oct  5 18:09:28 
2018
@@ -1873,6 +1873,23 @@ struct TestTryLock {
int i = a;
mu.Unlock();
   }
+
+  // Test with conditional operator
+  void foo13() {
+if (mu.TryLock() ? 1 : 0)
+  mu.Unlock();
+  }
+
+  void foo14() {
+if (mu.TryLock() ? 0 : 1)
+  return;
+mu.Unlock();
+  }
+
+  void foo15() {
+if (mu.TryLock() ? 0 : 1) // expected-note{{mutex acquired here}}
+  mu.Unlock();// expected-warning{{releasing mutex 'mu' that 
was not held}}
+  }   // expected-warning{{mutex 'mu' is not held on 
every path through here}}
 };  // end TestTrylock
 
 } // end namespace TrylockTest


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

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

It looks like you ran clang-format on all of lib/Sema/SemaExpr.cpp and changed 
many lines that are irrelevant to your patch. Can you undo that, please?


https://reviews.llvm.org/D52949



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

[PATCH] D52957: [analyzer] Teach CallEvent about C++17 aligned new.

[Artem Dergachev via Phabricator via cfe-commits]

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

In C++17, when class `C` has large alignment value, a special case of overload 
resolution rule kicks in for expression `new C` that causes the aligned version 
of `operator new()` to be called. The aligned `new` has two arguments: size and 
alignment. However, the new-expression has only one argument: the 
construct-expression for `C()`. This causes a false positive in 
`core.CallAndMessage`'s check for matching number of arguments and number of 
parameters.

Update `CXXAllocatorCall`, which is a `CallEvent` sub-class for operator new 
calls within new-expressions, so that the number of arguments always matched 
the number of parameters.

Dunno, maybe we should instead abandon the idea of reserving a whole 
argument/parameter index for each of those implicit arguments that aren't even 
represented by an expression in the AST.

Side note: Ugh, we never supported passing size into `operator new()` calls, 
even though it's known in compile time. And now also alignment. They are 
currently symbolic (unconstrained) within allocator calls.


Repository:
  rC Clang

https://reviews.llvm.org/D52957

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/new-aligned.cpp


Index: test/Analysis/new-aligned.cpp
===
--- /dev/null
+++ test/Analysis/new-aligned.cpp
@@ -0,0 +1,14 @@
+//RUN: %clang_analyze_cc1 -std=c++17 -analyze -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+// Notice the weird alignment.
+struct alignas(1024) S {};
+
+void foo() {
+  // Operator new() here is the C++17 aligned new that takes two arguments:
+  // size and alignment. Size is passed implicitly as usual, and alignment
+  // is passed implicitly in a similar manner.
+  S *s = new S; // no-warning
+  delete s;
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -503,10 +503,14 @@
 const ParmVarDecl *ParamDecl = *I;
 assert(ParamDecl && "Formal parameter has no decl?");
 
+// TODO: Support allocator calls.
 if (Call.getKind() != CE_CXXAllocator)
   if (Call.isArgumentConstructedDirectly(Idx))
 continue;
 
+// TODO: Allocators should receive the correct size and possibly alignment,
+// determined in compile-time but not represented as arg-expressions,
+// which makes getArgSVal() fail and return UnknownVal.
 SVal ArgVal = Call.getArgSVal(Idx);
 if (!ArgVal.isUnknown()) {
   Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx));
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -921,15 +921,28 @@
 return getOriginExpr()->getOperatorNew();
   }
 
+  // Size and maybe implicit alignment in C++17. Instead of size, the AST
+  // contains the construct-expression. Alignment is always hidden.
+  // We pretend that argument 0 is size and argument 1 is alignment (if passed
+  // implicitly) and the rest are placement args. This makes sure that the
+  // number of arguments is always the same as the number of parameters.
+  unsigned getNumImplicitArgs() const {
+return getOriginExpr()->passAlignment() ? 2 : 1;
+  }
+
   unsigned getNumArgs() const override {
-return getOriginExpr()->getNumPlacementArgs() + 1;
+return getOriginExpr()->getNumPlacementArgs() + getNumImplicitArgs();
   }
 
   const Expr *getArgExpr(unsigned Index) const override {
 // The first argument of an allocator call is the size of the allocation.
-if (Index == 0)
+if (Index < getNumImplicitArgs())
   return nullptr;
-return getOriginExpr()->getPlacementArg(Index - 1);
+return getOriginExpr()->getPlacementArg(Index - getNumImplicitArgs());
+  }
+
+  const Expr *getPlacementArgExpr(unsigned Index) const {
+return getOriginExpr()->getPlacementArg(Index);
   }
 
   Kind getKind() const override { return CE_CXXAllocator; }


Index: test/Analysis/new-aligned.cpp
===
--- /dev/null
+++ test/Analysis/new-aligned.cpp
@@ -0,0 +1,14 @@
+//RUN: %clang_analyze_cc1 -std=c++17 -analyze -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+// Notice the weird alignment.
+struct alignas(1024) S {};
+
+void foo() {
+  // Operator new() here is the C++17 aligned new that takes two arguments:
+  // size and alignment. Size is passed implicitly as usual, and alignment
+  // is passed implicitly in a similar

[PATCH] D52920: Introduce code_model macros

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: lib/Basic/Targets/X86.cpp:865
+  if (CodeModel == "default")
+// When the user has not explicitly specified anything,
+// the default code model to use is small.

I'm not sure if the comment is useful here... or you can comment that this 
piece of code should be consistent with X86TargetMachine.cpp#L208


Repository:
  rC Clang

https://reviews.llvm.org/D52920



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

[PATCH] D52956: Support `-fno-visibility-inlines-hidden`

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added subscribers: cfe-commits, eraman.

Undoes `-fvisibility-inlines-hidden`.

Test Plan: added test


Repository:
  rC Clang

https://reviews.llvm.org/D52956

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/visibility-inlines-hidden.cpp


Index: test/Driver/visibility-inlines-hidden.cpp
===
--- /dev/null
+++ test/Driver/visibility-inlines-hidden.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -S -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-1 %s < %t.log
+// CHECK-1: "-fvisibility-inlines-hidden"
+// CHECK-1-NOT: "-fno-visibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-2 %s < %t.log
+// CHECK-2: "-fno-visibility-inlines-hidden"
+// CHECK-2-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fvisibility-inlines-hidden 
-fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-3 %s < %t.log
+// CHECK-3: "-fno-visibility-inlines-hidden"
+// CHECK-3-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden 
-fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-4 %s < %t.log
+// CHECK-4: "-fvisibility-inlines-hidden"
+// CHECK-4-NOT: "-fno-visibility-inlines-hidden"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2310,8 +2310,11 @@
 Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
-  if (Args.hasArg(OPT_fvisibility_inlines_hidden))
+  if (Args.hasFlag(OPT_fvisibility_inlines_hidden,
+   OPT_fno_visibility_inlines_hidden,
+   false)) {
 Opts.InlineVisibilityHidden = 1;
+  }
 
   if (Args.hasArg(OPT_ftrapv)) {
 Opts.setSignedOverflowBehavior(LangOptions::SOB_Trapping);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4167,7 +4167,8 @@
 }
   }
 
-  Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden);
+  Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden,
+  options::OPT_fno_visibility_inlines_hidden);
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1714,6 +1714,9 @@
 def fvisibility_inlines_hidden : Flag<["-"], "fvisibility-inlines-hidden">, 
Group,
   HelpText<"Give inline C++ member functions hidden visibility by default">,
   Flags<[CC1Option]>;
+def fno_visibility_inlines_hidden : Flag<["-"], 
"fno-visibility-inlines-hidden">, Group,
+  HelpText<"Do not give inline C++ member functions hidden visibility by 
default">,
+  Flags<[CC1Option]>;
 def fvisibility_ms_compat : Flag<["-"], "fvisibility-ms-compat">, 
Group,
   HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;


Index: test/Driver/visibility-inlines-hidden.cpp
===
--- /dev/null
+++ test/Driver/visibility-inlines-hidden.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -S -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-1 %s < %t.log
+// CHECK-1: "-fvisibility-inlines-hidden"
+// CHECK-1-NOT: "-fno-visibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-2 %s < %t.log
+// CHECK-2: "-fno-visibility-inlines-hidden"
+// CHECK-2-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fvisibility-inlines-hidden -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-3 %s < %t.log
+// CHECK-3: "-fno-visibility-inlines-hidden"
+// CHECK-3-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-4 %s < %t.log
+// CHECK-4: "-fvisibility-inlines-hidden"
+// CHECK-4-NOT: "-fno-visibility-inlines-hidden"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2310,8 +2310,11 @@
 Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
-  if (Args.hasArg(OPT_fvisibility_inlines_hidden))
+  if (Args.hasFlag(OPT_fvisibility_inlines_hidden,
+   OPT_fno_visibility_inlines_hidden,
+   false)) {
 Opts.InlineVisibilityHidden = 1;
+  }
 
   if

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

[Aaron Ballman via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D52888#1256625, @aaronpuchert wrote:

> In https://reviews.llvm.org/D52888#1256395, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D52888#1255862, @aaronpuchert wrote:
> >
> > > Additional changes (including some non-tail recursion unfortunately) 
> > > would allow the following to work:
> > >
> > >   void foo16() {
> > > if (cond ? mu.TryLock() : false)
> > >   mu.Unlock();
> > >   }
> > >   
> > >   void foo17() {
> > > if (cond ? true : !mu.TryLock())
> > >   return;
> > > mu.Unlock();
> > >   }
> > >   
> > >
> > > Worth the effort, or is that too exotic?
> >
> >
> > `foo16()` looks like code I could plausibly see in the wild. Consider the 
> > case where the mutex is a pointer and you want to test it for null before 
> > calling `TryLock()` on it (`M ? M->TryLock() : false`). However, I don't 
> > have a good feeling for how much work it would be to support that case.
>
>
> It's relatively short, but not exactly straightforward, because we have to 
> check if both branches are "compatible". However, thinking about this again I 
> think most programmers would write `foo16` as
>
>   void foo16() {
> if (cond && mu.TryLock())
>   mu.Unlock();
>   }
>
>
> which is functionally equivalent, and which we already support. So I'd 
> propose to add support for this only when someone asks for it, and leave it 
> as it is for now.


I think that's reasonable. LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52888



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

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In https://reviews.llvm.org/D52398#1255290, @aaronpuchert wrote:

> @hokein Please have a look at https://reviews.llvm.org/D52888, maybe you can 
> try it out already. The problem was that `?:` expressions are considered a 
> branch point and when merging both branches the warning was emitted. Before 
> this change, we couldn't look into `__builtin_expect` and so we didn't see 
> that the result of `TryLock` is used to branch.


I patched the proposed fix-forward and it seems to have things building again. 
Is there an ETA on landing that? If it's going to take a bit, is there any 
chance we could revert this patch until then?


Repository:
  rL LLVM

https://reviews.llvm.org/D52398



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

[PATCH] D51388: [analyzer] NFC: Legalize state manager factory injection.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h:33
   /// introduce a type named \c NameTy.
-  /// The macro should not be used inside namespaces, or for traits that must
-  /// be accessible from more than one translation unit.
+  /// The macro should not be used inside namespaces.
   #define REGISTER_TRAIT_WITH_PROGRAMSTATE(Name, Type) \

george.karpenkov wrote:
> What happened to the comment about multiple TUs?
> One still has to be very careful here, right?
D52905#1257040.


Repository:
  rL LLVM

https://reviews.llvm.org/D51388



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

[PATCH] D52905: CSA: fix accessing GDM data from shared libraries

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added subscribers: george.karpenkov, dcoughlin, NoQ.
NoQ added a comment.

Hmmm, interesting. A checker doesn't usually need to access these specific 
static locals, at least not directly. These are usually accessed through 
functions in .cpp files that are supposed to be compiled with a pointer to the 
correct instance of the static local, and it doesn't seem to be necessary to 
expose them to plugins, simply because i don't immediately see why would a 
plugin want to use them. In this sense, i believe that the entire definition of 
these traits should be moved to .cpp files and be made private, accessed only 
via public methods of respective classes. But i guess it's more difficult and 
it's a separate chunk of work, so i totally approve this patch.

Also, i guess that's what they meant when they were saying that 
REGISTER_TRAIT_WITH_PROGRAMSTATE [and similar] macros shouldn't be used in 
headers (https://reviews.llvm.org/D51388?id=162968#inline-455495).

Did you use any sort of tool to find those? I.e., are there more such bugs, and 
can we prevent this from regressing and breaking your workflow later?

You didn't add reviewers. Does it mean that you are still planning to work on 
this patch further, or do you want this patch to be committed in its current 
shape? Static Analyzer patches are usually prefixed with [analyzer] (a few 
people auto-subscribe to those) and please feel free to add me and 
@george.karpenkov as reviewers, and the code owner is @dcoughlin.


Repository:
  rC Clang

https://reviews.llvm.org/D52905



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

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343894: [COFF, ARM64] Add _InterlockedAdd intrinsic 
(authored by mgrang, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52811?vs=168518&id=168543#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52811

Files:
  cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c


Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -8517,6 +8517,15 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRMWInst::Add, Arg0, Arg1,
+  llvm::AtomicOrdering::SequentiallyConsistent);
+return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 
Index: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
@@ -94,6 +94,7 @@
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
Index: cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
===
--- cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
+++ cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }


Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 \**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 /**\
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -8517,6 +8517,15 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRMWInst::Add, Arg0, Arg1,
+  llvm::AtomicOrdering::SequentiallyConsistent);
+return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 
Index: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
+++ cfe/trunk/include/clang/Basic/BuiltinsA

r343894 - [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Fri Oct  5 14:57:41 2018
New Revision: 343894

URL: http://llvm.org/viewvc/llvm-project?rev=343894&view=rev
Log:
[COFF, ARM64] Add _InterlockedAdd intrinsic

Reviewers: rnk, mstorsjo, compnerd, TomTan, haripul, javed.absar, efriedma

Reviewed By: efriedma

Subscribers: efriedma, kristof.beyls, chrib, jfb, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Fri Oct  5 14:57:41 2018
@@ -94,6 +94,7 @@ TARGET_HEADER_BUILTIN(_BitScanReverse, "
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  5 14:57:41 2018
@@ -8517,6 +8517,15 @@ Value *CodeGenFunction::EmitAArch64Built
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRMWInst::Add, Arg0, Arg1,
+  llvm::AtomicOrdering::SequentiallyConsistent);
+return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Fri Oct  5 14:57:41 2018
@@ -869,6 +869,7 @@ __nop(void) {
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\

Modified: cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c Fri Oct  5 14:57:41 2018
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }


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

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343892: Emit CK_NoOp casts in C mode, not just C++. 
(authored by jyknight, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52918?vs=168477&id=168542#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

Files:
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/c-casts.c


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: cfe/trunk/test/Sema/c-casts.c
===
--- cfe/trunk/test/Sema/c-casts.c
+++ cfe/trunk/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace()

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343892: Emit CK_NoOp casts in C mode, not just C++. 
(authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52918?vs=168477&id=168541#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/c-casts.c


Index: test/Sema/c-casts.c
===
--- test/Sema/c-casts.c
+++ test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 


Index: test/Sema/c-casts.c
===
--- test/Sema/c-casts.c
+++ test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Resu

r343892 - Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via cfe-commits]

Author: jyknight
Date: Fri Oct  5 14:53:51 2018
New Revision: 343892

URL: http://llvm.org/viewvc/llvm-project?rev=343892&view=rev
Log:
Emit CK_NoOp casts in C mode, not just C++.

Previously, it had been using CK_BitCast even for casts that only
change const/restrict/volatile. Now it will use CK_Noop where
appropriate.

This is an alternate solution to r336746.

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

Added:
cfe/trunk/test/Sema/c-casts.c
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=343892&r1=343891&r2=343892&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Oct  5 14:53:51 2018
@@ -5864,11 +5864,7 @@ bool PointerExprEvaluator::VisitCastExpr
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=343892&r1=343891&r2=343892&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Oct  5 14:53:51 2018
@@ -5862,6 +5862,8 @@ CastKind Sema::PrepareScalarCast(ExprRes
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@ Sema::CheckAssignmentConstraints(QualTyp
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 

Added: cfe/trunk/test/Sema/c-casts.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/c-casts.c?rev=343892&view=auto
==
--- cfe/trunk/test/Sema/c-casts.c (added)
+++ cfe/trunk/test/Sema/c-casts.c Fri Oct  5 14:53:51 2018
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}


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

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

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

xbolva00 added inline comments.



Comment at: test/Sema/div-sizeof-ptr.c:9
+
+int e = sizeof(int *) / sizeof(int);
+int f = sizeof(p) / sizeof(p);

GCC warns also in this case, but it is weird...


https://reviews.llvm.org/D52949



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

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

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

xbolva00 updated this revision to Diff 168539.

https://reviews.llvm.org/D52949

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/div-sizeof-ptr.c

Index: test/Sema/div-sizeof-ptr.c
===
--- test/Sema/div-sizeof-ptr.c
+++ test/Sema/div-sizeof-ptr.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+void test(int *p, int **q) {
+int a = sizeof(p) / sizeof(*p);  // expected-warning {{division produces the incorrect number of array elements}}
+int b = sizeof p / sizeof *p;// expected-warning {{division produces the incorrect number of array elements}}
+int c = sizeof(*q) / sizeof(**q);// expected-warning {{division produces the incorrect number of array elements}}
+int d = sizeof(p) / sizeof(int); // expected-warning {{division produces the incorrect number of array elements}}
+
+int e = sizeof(int *) / sizeof(int);
+int f = sizeof(p) / sizeof(p);
+int g = sizeof(*q) / sizeof(q);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8670,6 +8670,41 @@
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  UnaryExprOrTypeTraitExpr *LUE =
+  dyn_cast_or_null(LHS);
+  UnaryExprOrTypeTraitExpr *RUE =
+  dyn_cast_or_null(RHS);
+
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy;
+  QualType RHSTy;
+  if (LUE->isArgumentType())
+return;
+
+  LHS = LUE->getArgumentExpr()->IgnoreParens();
+  LHSTy = LHS->getType();
+
+  if (RUE->isArgumentType()) {
+RHSTy = RUE->getArgumentType();
+  } else {
+RHS = RUE->getArgumentExpr()->IgnoreParens();
+RHSTy = RHS->getType();
+  }
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8700,8 +8735,10 @@
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3291,6 +3291,10 @@
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "division produces the incorrect number of array elements">,
+  InGroup;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -142,6 +142,7 @@
 def : DiagGroup<"discard-qual">;
 def DivZero : DiagGroup<"division-by-zero">;
 def : DiagGroup<"div-by-zero", [DivZero]>;
+def DivSizeofPtr : DiagGroup<"sizeof-pointer-div">;
 
 def DocumentationHTML : DiagGroup<"documentation-html">;
 def DocumentationUnknownCommand : DiagGroup<"documentation-unknown-command">;
@@ -785,6 +786,7 @@
 SelfMove,
 SizeofArrayArgument,
 SizeofArrayDecay,
+DivSizeofPtr,
 StringPlusInt,
 Trigraphs,
 Uninitialized,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

xbolva00 created this revision.
Herald added a subscriber: cfe-commits.

void test(int *arr) {

  int arr_len = sizeof(arr) / sizeof(*arr);  // warn, incorrect way to compute 
number of array elements

}

Enabled under -Wall (same behaviour as GCC)


Repository:
  rC Clang

https://reviews.llvm.org/D52949

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/div-sizeof-ptr.c

Index: test/Sema/div-sizeof-ptr.c
===
--- test/Sema/div-sizeof-ptr.c
+++ test/Sema/div-sizeof-ptr.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+void test(int *p, int **q) {
+int a = sizeof(p) / sizeof(*p);  // expected-warning {{division produces the incorrect number of array elements}}
+int b = sizeof p / sizeof *p;// expected-warning {{division produces the incorrect number of array elements}}
+int c = sizeof(*q) / sizeof(**q);// expected-warning {{division produces the incorrect number of array elements}}
+int d = sizeof(p) / sizeof(int); // expected-warning {{division produces the incorrect number of array elements}}
+int e = sizeof(int *) / sizeof(int); // expected-warning {{division produces the incorrect number of array elements}}
+  
+int f = sizeof(p) / sizeof(p);
+int g = sizeof(*q) / sizeof(q);
+}
+
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8670,6 +8670,42 @@
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  UnaryExprOrTypeTraitExpr *LUE =
+  dyn_cast_or_null(LHS);
+  UnaryExprOrTypeTraitExpr *RUE =
+  dyn_cast_or_null(RHS);
+
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy;
+  QualType RHSTy;
+  if (LUE->isArgumentType()) {
+LHSTy = LUE->getArgumentType();
+  } else {
+LHS = LUE->getArgumentExpr()->IgnoreParens();
+LHSTy = LHS->getType();
+  }
+
+  if (RUE->isArgumentType()) {
+RHSTy = RUE->getArgumentType();
+  } else {
+RHS = RUE->getArgumentExpr()->IgnoreParens();
+RHSTy = RHS->getType();
+  }
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8700,8 +8736,10 @@
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3291,6 +3291,10 @@
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "division produces the incorrect number of array elements">,
+  InGroup;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -142,6 +142,7 @@
 def : DiagGroup<"discard-qual">;
 def DivZero : DiagGroup<"division-by-zero">;
 def : DiagGroup<"div-by-zero", [DivZero]>;
+def DivSizeofPtr : DiagGroup<"sizeof-pointer-div">;
 
 def DocumentationHTML : DiagGroup<"documentation-html">;
 def DocumentationUnknownCommand : DiagGroup<"documentation-unknown-command">;
@@ -785,6 +786,7 @@
 SelfMove,
 SizeofArrayArgument,
 SizeofArrayDecay,
+DivSizeofPtr,
 StringPlusInt,
 Trigraphs,
 Uninitialized,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r343887 - [llvm-nm] Write "no symbol" output to stderr

[Petr Hosek via cfe-commits]

Author: phosek
Date: Fri Oct  5 14:10:03 2018
New Revision: 343887

URL: http://llvm.org/viewvc/llvm-project?rev=343887&view=rev
Log:
[llvm-nm] Write "no symbol" output to stderr

This matches the output of binutils' nm and ensures that any scripts
or tools that use nm and expect empty output in case there no symbols
don't break.

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

Modified:
cfe/trunk/test/CodeGen/thinlto_backend.ll

Modified: cfe/trunk/test/CodeGen/thinlto_backend.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto_backend.ll?rev=343887&r1=343886&r2=343887&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto_backend.ll (original)
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll Fri Oct  5 14:10:03 2018
@@ -25,7 +25,7 @@
 ; be empty file.
 ; RUN: opt -o %t5.o %s
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c 
-fthinlto-index=%t.thinlto.bc
-; RUN: llvm-nm %t4.o | FileCheck %s -check-prefix=NO-SYMBOLS
+; RUN: llvm-nm %t4.o 2>&1 | FileCheck %s -check-prefix=NO-SYMBOLS
 ; NO-SYMBOLS: no symbols
 
 ; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].


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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In https://reviews.llvm.org/D52193#1243456, @thakis wrote:

> ...and to reword this a bit: Clang taking a long time to start up in some 
> configurations is a bug we should profile and fix :-)


This is time spent in `ntdll.dll` loading various low-level libraries like 
`kernel32.dll`, `KernelBase.dll`, `combase.dll` and so on. So at least 20ms 
just getting to main(). Then, about ~10ms spent loading `dbghelp.dll` while 
installing the exception handler in `llvm::RegisterHandler`. So a lot of wasted 
time. The easiest would be to just not invoke a new child process!



New timings without the child `clang-cl.exe` being invoked (hacked from 
https://reviews.llvm.org/D52411). The improvement **is significant**. Tested at 
r343846.

**Config 1:** Intel Xeon Haswell 6 cores / 12 HW threads, 3.5 GHz, 15M cache, 
128 GB RAM, SSD 550 MB/s

__Ninja:__

| MSVC cl  | (37min 19sec)  
   |
| clang-cl //(built with Clang)//  | //(in progress-will update a bit 
later)// |
| clang-cl no child //(built with Clang)// | //(in progress-will update a bit 
later)// |
|

**Config 2:** Intel Xeon Skylake 18 cores / 36 HW threads, x2 (Dual CPU), 72 HW 
threads total, 2.3 GHz, 24.75M cache, 128 GB RAM, NVMe SSD 4.6 GB/s

__Ninja:__

| MSVC cl  | (7min 33sec)|
| clang-cl //(built with Clang)//  | (9min 2sec) |
| **clang-cl no child //(built with Clang)//** | **(7min 9sec)** |
|

This asks whether the improvement will be of the same order, if invoking just 
one `clang-cl.exe` for the whole compilation process. A sort of local 
compilation-as-a-service.
In that scenario, Ninja could invoke `clang-cl.exe` and pass it all the files 
to be compiled, and let clang iterate and multi-thread internally. That could 
be quite a significant change, however the improvements //could be// important.
However that would probably break the concept behind //Goma //I suppose. Unless 
you split the invocations to precisely one `clang-cl` per available (remote) 
agent.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

r343883 - [DebugInfo] Add support for DWARF5 call site-related attributes

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Fri Oct  5 13:37:17 2018
New Revision: 343883

URL: http://llvm.org/viewvc/llvm-project?rev=343883&view=rev
Log:
[DebugInfo] Add support for DWARF5 call site-related attributes

DWARF v5 introduces DW_AT_call_all_calls, a subprogram attribute which
indicates that all calls (both regular and tail) within the subprogram
have call site entries. The information within these call site entries
can be used by a debugger to populate backtraces with synthetic tail
call frames.

Tail calling frames go missing in backtraces because the frame of the
caller is reused by the callee. Call site entries allow a debugger to
reconstruct a sequence of (tail) calls which led from one function to
another. This improves backtrace quality. There are limitations: tail
recursion isn't handled, variables within synthetic frames may not
survive to be inspected, etc. This approach is not novel, see:

  
https://gcc.gnu.org/wiki/summit2010?action=AttachFile&do=get&target=jelinek.pdf

This patch adds an IR-level flag (DIFlagAllCallsDescribed) which lowers
to DW_AT_call_all_calls. It adds the minimal amount of DWARF generation
support needed to emit standards-compliant call site entries. For easier
deployment, when the debugger tuning is LLDB, the DWARF requirement is
adjusted to v4.

Testing: Apart from check-{llvm, clang}, I built a stage2 RelWithDebInfo
clang binary. Its dSYM passed verification and grew by 1.4% compared to
the baseline. 151,879 call site entries were added.

rdar://42001377

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

Added:
cfe/trunk/test/CodeGenCXX/dbg-info-all-calls-described.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=343883&r1=343882&r2=343883&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Oct  5 13:37:17 2018
@@ -3168,6 +3168,7 @@ llvm::DISubprogram *CGDebugInfo::getFunc
   QualType FnType = CGM.getContext().getFunctionType(
   FD->getReturnType(), ArgTypes, FunctionProtoType::ExtProtoInfo(CC));
   if (Stub) {
+Flags |= getCallSiteRelatedAttrs();
 return DBuilder.createFunction(
 DContext, Name, LinkageName, Unit, Line,
 getOrCreateFunctionType(GD.getDecl(), FnType, Unit),
@@ -3407,6 +3408,8 @@ void CGDebugInfo::EmitFunctionStart(Glob
   if (CurFuncIsThunk)
 Flags |= llvm::DINode::FlagThunk;
 
+  llvm::DINode::DIFlags FlagsForDef = Flags | getCallSiteRelatedAttrs();
+
   unsigned LineNo = getLineNumber(Loc);
   unsigned ScopeLine = getLineNumber(ScopeLoc);
 
@@ -3418,7 +3421,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
   llvm::DISubprogram *SP = DBuilder.createFunction(
   FDContext, Name, LinkageName, Unit, LineNo,
   getOrCreateFunctionType(D, FnType, Unit), Fn->hasLocalLinkage(),
-  true /*definition*/, ScopeLine, Flags, CGM.getLangOpts().Optimize,
+  true /*definition*/, ScopeLine, FlagsForDef, CGM.getLangOpts().Optimize,
   TParamsArray.get(), getFunctionDeclaration(D));
   Fn->setSubprogram(SP);
   // We might get here with a VarDecl in the case we're generating
@@ -4422,3 +4425,22 @@ llvm::DebugLoc CGDebugInfo::SourceLocToD
   llvm::MDNode *Scope = LexicalBlockStack.back();
   return llvm::DebugLoc::get(getLineNumber(Loc), getColumnNumber(Loc), Scope);
 }
+
+llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
+  // Call site-related attributes are only useful in optimized programs, and
+  // when there's a possibility of debugging backtraces.
+  if (!CGM.getLangOpts().Optimize || DebugKind == codegenoptions::NoDebugInfo 
||
+  DebugKind == codegenoptions::LocTrackingOnly)
+return llvm::DINode::FlagZero;
+
+  // Call site-related attributes are available in DWARF v5. Some debuggers,
+  // while not fully DWARF v5-compliant, may accept these attributes as if they
+  // were part of DWARF v4.
+  bool SupportsDWARFv4Ext =
+  CGM.getCodeGenOpts().DwarfVersion == 4 &&
+  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
+return llvm::DINode::FlagZero;
+
+  return llvm::DINode::FlagAllCallsDescribed;
+}

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=343883&r1=343882&r2=343883&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Fri Oct  5 13:37:17 2018
@@ -608,6 +608,11 @@ private:
  unsigned LineNo, StringRef LinkageName,
  llvm::GlobalVariable *Var, llvm::DIScope *DContext);
 
+
+  /// Return flags which enable debug info e

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D52811



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

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 168518.

https://reviews.llvm.org/D52811

Files:
  include/clang/Basic/BuiltinsAArch64.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/arm64-microsoft-intrinsics.c


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8514,6 +8514,15 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRMWInst::Add, Arg0, Arg1,
+  llvm::AtomicOrdering::SequentiallyConsistent);
+return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 
Index: include/clang/Basic/BuiltinsAArch64.def
===
--- include/clang/Basic/BuiltinsAArch64.def
+++ include/clang/Basic/BuiltinsAArch64.def
@@ -94,6 +94,7 @@
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 \**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 /**\
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8514,6 +8514,15 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRM

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 168517.
mgrang added a comment.
Herald added a reviewer: javed.absar.

Limited the intrinsic only for AArch64 and fixed the implementation to return 
the sum instead of the old value of the Addend. Thanks @efriedma for the 
pointers.


https://reviews.llvm.org/D52811

Files:
  include/clang/Basic/BuiltinsAArch64.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/arm64-microsoft-intrinsics.c


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,19 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: [[ADDEND:%[0-9]+]] = load i32*, i32** %Addend.addr, align 8
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[ATOMICADD:%[0-9]+]] = atomicrmw add i32* [[ADDEND:%[0-9]+]], 
i32 [[VALUE:%[0-9]+]] seq_cst
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[RESULT:%[0-9]+]] = add i32 [[ATOMICADD:%[0-9]+]], 
[[VALUE:%[0-9]+]]
+// CHECK-MSVC: ret i32 [[RESULT:%[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8514,6 +8514,11 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *RMWI = MakeBinaryAtomicValue(*this, AtomicRMWInst::Add, E);
+return Builder.CreateAdd(RMWI, EmitScalarExpr(E->getArg(1)));
+  }
   }
 }
 
Index: include/clang/Basic/BuiltinsAArch64.def
===
--- include/clang/Basic/BuiltinsAArch64.def
+++ include/clang/Basic/BuiltinsAArch64.def
@@ -94,6 +94,7 @@
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,19 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
+// CHECK-MSVC: [[ADDEND:%[0-9]+]] = load i32*, i32** %Addend.addr, align 8
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[ATOMICADD:%[0-9]+]] = atomicrmw add i32* [[ADDEND:%[0-9]+]], i32 [[VALUE:%[0-9]+]] seq_cst
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[RESULT:%[0-9]+]] = add i32 [[ATOMICADD:%[0-9]+]], [[VALUE:%[0-9]+]]
+// CHECK-MSVC: ret i32 [[RESULT:%[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 \**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 /*

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

[Victor Costan via Phabricator via cfe-commits]

pwnall added a comment.

Thank you for clarifying, Aaron!

I probably used Phabricator incorrectly. My intent was to state that the tests 
LGTM and express support for the functionality in this patch. Please definitely 
address all review comments before landing.


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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

[PATCH] D52807: [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:3003
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;

efriedma wrote:
> efriedma wrote:
> > rnk wrote:
> > > mgrang wrote:
> > > > rnk wrote:
> > > > > Is the no fence version really equivalent to this seq_cst version 
> > > > > here?
> > > > I don't see InterlockedCompareExchangePointer creating a fence but it 
> > > > should. (since it is the fence version). So maybe that needs to be 
> > > > fixed (and possibly other fence functions).
> > > > 
> > > > InterlockedCompareExchangePointer_nf should not create a fence so the 
> > > > current implementation seems correct.
> > > Hm, if we're supposed to have fences, we're probably missing them 
> > > everywhere. I thought the atomicrmw orderings were enough, but that shows 
> > > what I know about the C++ memory model. =p
> > > 
> > > We don't generate a fence for this intrinsic when MSVC does:
> > > ```
> > > unsigned char test_arm(long *base, long idx) {
> > >   return _interlockedbittestandreset_acq(base, idx);
> > > }
> > > ```
> > > 
> > > @efriedma, is MSVC over-emitting extra fences, or are we under-emitting? 
> > > If this is their "bug", should we be bug-for-bug compatible with it so we 
> > > get the same observable memory states?
> > "create a fence" is a little confusing because the AArch64 ldaxr/stlxr have 
> > builtin fences... but presumably the "nf" version should use ldxr/stxr 
> > instead.  At the IR level, that corresponds to "monotonic".
> In terms of emitting fences, maybe we are actually missing a fence.
> 
> An ldaxr+stlxr pair isn't a complete fence, at least in theory; it stops 
> loads from being hosted past it, and stores from being sunk past it, but 
> stores can be hoisted and loads can be sunk.  That said, a stronger fence 
> isn't necessary to model C++11 atomics; C++11 only requires sequential 
> consistency with other sequentially consistent operations, plus an 
> acquire/release barrier.  And a program that isn't using C++11 relaxed 
> atomics can't depend on a stronger barrier without triggering a data race.
> 
> A program written before C++11 atomics were generally available could 
> theoretically depend on the barrier through a series of operations that cause 
> a data race under the C++11 rules.  Since there were no clear rules before 
> C++11, programs did things like emulate relaxed atomics on top of volatile 
> pointers; using such a construct, a program could depend on the implied 
> barrier.  For this reason, gcc emits a fence on AArch64 after `__sync_*` (but 
> not `__atomic_*`).  And I guess MSVC does something similar for 
> `_Interlocked*`.
> 
> That said, LLVM has never emitted a dmb for `__sync_*` operations on AArch64, 
> and it hasn't led to any practical problems as far as I know.
> it stops loads from being hosted past it, and stores from being sunk past it, 
> but stores can be hoisted and loads can be sunk.

Looking over it again, this bit is a really bad description of what actually 
happens. Really there are two issues: one, an operation from after an 
ldaxr/stlxr pair could end up betwen the ldaxr and stlxr, and two, a cmpxchg 
doesn't always execute the stlxr.


Repository:
  rC Clang

https://reviews.llvm.org/D52807



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

r343881 - [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Fri Oct  5 12:49:36 2018
New Revision: 343881

URL: http://llvm.org/viewvc/llvm-project?rev=343881&view=rev
Log:
[COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

Reviewers: rnk, mstorsjo, compnerd, TomTan, haripul, efriedma

Reviewed By: efriedma

Subscribers: efriedma, kristof.beyls, chrib, jfb, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=343881&r1=343880&r2=343881&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Fri Oct  5 12:49:36 2018
@@ -786,6 +786,7 @@ LANGBUILTIN(_InterlockedCompareExchange1
 LANGBUILTIN(_InterlockedCompareExchange,"NiNiD*NiNi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchange64,  "LLiLLiD*LLiLLi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchangePointer, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedCompareExchangePointer_nf, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement16,"ssD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement,  "NiNiD*",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedExchange,   "NiNiD*Ni", "n", 
ALL_MS_LANGUAGES)

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=343881&r1=343880&r2=343881&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  5 12:49:36 2018
@@ -2999,7 +2999,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   case Builtin::BI_InterlockedExchangePointer:
 return RValue::get(
 EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
-  case Builtin::BI_InterlockedCompareExchangePointer: {
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;
 llvm::IntegerType *IntType =
   IntegerType::get(getLLVMContext(),
@@ -3016,10 +3017,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 llvm::Value *Comparand =
   Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);
 
-auto Result =
-Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
-AtomicOrdering::SequentiallyConsistent,
-AtomicOrdering::SequentiallyConsistent);
+auto Ordering =
+  BuiltinID == Builtin::BI_InterlockedCompareExchangePointer_nf ?
+  AtomicOrdering::Monotonic : AtomicOrdering::SequentiallyConsistent;
+
+auto Result = Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
+  Ordering, Ordering);
 Result->setVolatile(true);
 
 return 
RValue::get(Builder.CreateIntToPtr(Builder.CreateExtractValue(Result,

Modified: cfe/trunk/test/CodeGen/ms-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics.c?rev=343881&r1=343880&r2=343881&view=diff
==
--- cfe/trunk/test/CodeGen/ms-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c Fri Oct  5 12:49:36 2018
@@ -235,6 +235,21 @@ void *test_InterlockedCompareExchangePoi
 // CHECK:   ret i8* %[[RESULT:[0-9]+]]
 // CHECK: }
 
+void *test_InterlockedCompareExchangePointer_nf(void * volatile *Destination,
+ void *Exchange, void *Comparand) {
+  return _InterlockedCompareExchangePointer_nf(Destination, Exchange, 
Comparand);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer_nf(i8** 
{{[a-z_ ]*}}%Destination, i8* {{[a-z_ ]*}}%Exchange, i8* {{[a-z_ 
]*}}%Comparand){{.*}}{
+// CHECK:   %[[DEST:[0-9]+]] = bitcast i8** %Destination to [[iPTR]]*
+// CHECK:   %[[EXCHANGE:[0-9]+]] = ptrtoint i8* %Exchange to [[iPTR]]
+// CHECK:   %[[COMPARAND:[0-9]+]] = ptrtoint i8* %Comparand to [[iPTR]]
+// CHECK:   %[[XCHG:[0-9]+]] = cmpxchg volatile [[iPTR]]* %[[DEST:[0-9]+]], 
[[iPTR]] %[[COMPARAND:[0-9]+]], [[iPTR]] %[[EXCHANGE:[0-9]+]] monotonic 
monotonic
+// CHECK:   %[[EXTRACT:[0-9]+]] = extractvalue { [[iPTR]], i1 } %[[XCHG]], 0
+// CHECK:   %[[RESULT:[0-9]+]] = inttoptr [[iPTR]] %[[EXTRACT]] to i8*
+// CHECK:   ret i8* %[[RESULT:[0-9]+]]
+// CHECK: }
+
 char test_InterlockedExchange8(char volatile *value, char mask) {
   return _InterlockedExchange8(value, mask);
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/

[PATCH] D52807: [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343881: [COFF, ARM64] Add 
_InterlockedCompareExchangePointer_nf intrinsic (authored by mgrang, committed 
by ).

Repository:
  rC Clang

https://reviews.llvm.org/D52807

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/ms-intrinsics.c


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2999,7 +2999,8 @@
   case Builtin::BI_InterlockedExchangePointer:
 return RValue::get(
 EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
-  case Builtin::BI_InterlockedCompareExchangePointer: {
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;
 llvm::IntegerType *IntType =
   IntegerType::get(getLLVMContext(),
@@ -3016,10 +3017,12 @@
 llvm::Value *Comparand =
   Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);
 
-auto Result =
-Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
-AtomicOrdering::SequentiallyConsistent,
-AtomicOrdering::SequentiallyConsistent);
+auto Ordering =
+  BuiltinID == Builtin::BI_InterlockedCompareExchangePointer_nf ?
+  AtomicOrdering::Monotonic : AtomicOrdering::SequentiallyConsistent;
+
+auto Result = Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
+  Ordering, Ordering);
 Result->setVolatile(true);
 
 return 
RValue::get(Builder.CreateIntToPtr(Builder.CreateExtractValue(Result,
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -786,6 +786,7 @@
 LANGBUILTIN(_InterlockedCompareExchange,"NiNiD*NiNi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchange64,  "LLiLLiD*LLiLLi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchangePointer, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedCompareExchangePointer_nf, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement16,"ssD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement,  "NiNiD*",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedExchange,   "NiNiD*Ni", "n", 
ALL_MS_LANGUAGES)
Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -235,6 +235,21 @@
 // CHECK:   ret i8* %[[RESULT:[0-9]+]]
 // CHECK: }
 
+void *test_InterlockedCompareExchangePointer_nf(void * volatile *Destination,
+ void *Exchange, void *Comparand) {
+  return _InterlockedCompareExchangePointer_nf(Destination, Exchange, 
Comparand);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer_nf(i8** 
{{[a-z_ ]*}}%Destination, i8* {{[a-z_ ]*}}%Exchange, i8* {{[a-z_ 
]*}}%Comparand){{.*}}{
+// CHECK:   %[[DEST:[0-9]+]] = bitcast i8** %Destination to [[iPTR]]*
+// CHECK:   %[[EXCHANGE:[0-9]+]] = ptrtoint i8* %Exchange to [[iPTR]]
+// CHECK:   %[[COMPARAND:[0-9]+]] = ptrtoint i8* %Comparand to [[iPTR]]
+// CHECK:   %[[XCHG:[0-9]+]] = cmpxchg volatile [[iPTR]]* %[[DEST:[0-9]+]], 
[[iPTR]] %[[COMPARAND:[0-9]+]], [[iPTR]] %[[EXCHANGE:[0-9]+]] monotonic 
monotonic
+// CHECK:   %[[EXTRACT:[0-9]+]] = extractvalue { [[iPTR]], i1 } %[[XCHG]], 0
+// CHECK:   %[[RESULT:[0-9]+]] = inttoptr [[iPTR]] %[[EXTRACT]] to i8*
+// CHECK:   ret i8* %[[RESULT:[0-9]+]]
+// CHECK: }
+
 char test_InterlockedExchange8(char volatile *value, char mask) {
   return _InterlockedExchange8(value, mask);
 }


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2999,7 +2999,8 @@
   case Builtin::BI_InterlockedExchangePointer:
 return RValue::get(
 EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
-  case Builtin::BI_InterlockedCompareExchangePointer: {
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;
 llvm::IntegerType *IntType =
   IntegerType::get(getLLVMContext(),
@@ -3016,10 +3017,12 @@
 llvm::Value *Comparand =
   Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);
 
-auto Result =
-Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
-AtomicOrdering::SequentiallyConsistent,
-AtomicOrdering::SequentiallyConsistent);
+auto Ordering =
+  BuiltinID == Builtin::BI_InterlockedCompareExchangePointer_nf ?
+  AtomicOrder

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

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert planned changes to this revision.
aaronpuchert added a comment.

I think I'll try to simplify this and address @delesley's comments before we 
commit this. I'll admit that the semantics are somewhat counter-intuitive, but 
as I explained I think it's more consistent this way. Because the scoped unlock 
releases a lock on construction, the operation on the underlying lock mirrors 
the operation on the scoped capability: releasing the scoped capability (on 
destruction, for example) acquires the lock again.


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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

[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 168505.
aaronpuchert added a comment.

Rebase on top of https://reviews.llvm.org/D52443. We also check the move 
constructor argument for write access, as suggested in a review.

This isn't intended to be merged (yet?), it should be seen as an RFC.


Repository:
  rC Clang

https://reviews.llvm.org/D52395

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5058,27 +5058,27 @@
 
   void test1() {
 copy(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
-write1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-write2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+write1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+write2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 read1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 read2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-destroy(mymove(foo));  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+destroy(mymove(foo));  // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 
 copyVariadic(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
 readVariadic(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-writeVariadic(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+writeVariadic(foo);// expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 copyVariadicC(1, foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
 
 FooRead reader(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-FooWrite writer(foo);  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+FooWrite writer(foo);  // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 
-mwrite1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-mwrite2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+mwrite1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+mwrite2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 mread1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 mread2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
-smwrite1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-smwrite2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+smwrite1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+smwrite2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 smread1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 smread2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
@@ -5094,12 +5094,13 @@
 (*this) << foo;  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
 copy(*foop); // expected-warning {{reading the value pointed to by 'foop' requires holding mutex 'mu'}}
-write1(*foop);   // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
-write2(10, *foop);   // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
+write1(*foop);   // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+write2

[PATCH] D52807: [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D52807



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

[PATCH] D51809: [CUDA][HIP] Fix ShouldDeleteSpecialMember for inherited constructors

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 168500.
yaxunl added a comment.

fix a typo.


https://reviews.llvm.org/D51809

Files:
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCUDA/implicit-member-target-inherited.cu
  test/SemaCUDA/inherited-ctor.cu

Index: test/SemaCUDA/inherited-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/inherited-ctor.cu
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+// Inherit a valid non-default ctor.
+namespace NonDefaultCtorValid {
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {}
+  };
+
+  void test() {
+B b(0);
+C c;
+  }
+}
+
+// Inherit an invalid non-default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace NonDefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {} // expected-error{{constructor inherited by 'B' from base class 'A' is implicitly deleted}}
+  // expected-note@-6{{constructor inherited by 'B' is implicitly deleted because field 's' has a deleted corresponding constructor}}
+  // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
+
+// Inherit a valid default ctor.
+namespace DefaultCtorValid {
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() {}
+  };
+
+  void test() {
+B b;
+C c;
+  }
+}
+
+// Inherit an invalid default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace DefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() {} // expected-error{{call to implicitly-deleted default constructor of 'struct B'}}
+   // expected-note@-6{{default constructor of 'B' is implicitly deleted because field 's' has a deleted default constructor}}
+   // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
Index: test/SemaCUDA/implicit-member-target-inherited.cu
===
--- /dev/null
+++ test/SemaCUDA/implicit-member-target-inherited.cu
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -Wno-defaulted-function-deleted
+
+#include "Inputs/cuda.h"
+
+//--
+// Test 1: infer inherited default ctor to be host.
+
+struct A1_with_host_ctor {
+  A1_with_host_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+// The inherited default constructor is inferred to be host, so we'll encounter
+// an error when calling it from a __device__ function, but not from a __host__
+// function.
+struct B1_with_implicit_default_ctor : A1_with_host_ctor {
+  using A1_with_host_ctor::A1_with_host_ctor;
+};
+
+// expected-note@-4 {{call to __host__ function from __device__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A1_with_host_ctor' inherited here}}
+
+void hostfoo() {
+  B1_with_implicit_default_ctor b;
+}
+
+__device__ void devicefoo() {
+  B1_with_implicit_default_ctor b; // expected-error {{no matching constructor}}
+}
+
+//--
+// Test 2: infer inherited default ctor to be device.
+
+struct A2_with_device_ctor {
+  __device__ A2_with_device_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct B2_with_implicit_default_ctor : A2_with_device_ctor {
+  using A2_with_device_ctor::A2_with_device_ctor;
+};
+
+// expected-note@-4 {{call to __device__ function from __host__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A2_with_device_ctor' inherited here}}
+
+void hostfoo2() {
+  B2_with_implicit_default_ctor b;  // expected-error {{no matching constructor}}
+}
+
+__device__ void devicefoo2() {
+  B2_with_implicit_default_ctor b;
+}
+
+//--
+// Test 3: infer inherited copy ctor
+
+struct A3_with_device_ctors {
+  __host__ A3_with_device_ctors

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for adding the test and the TODO!


https://reviews.llvm.org/D52918



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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343875: [CUDA] Use all 64 bits of GUID in __nv_module_id 
(authored by tra, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52938?vs=168489&id=168498#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52938

Files:
  cfe/trunk/lib/CodeGen/CGCUDANV.cpp


Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 


Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r343875 - [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via cfe-commits]

Author: tra
Date: Fri Oct  5 11:39:58 2018
New Revision: 343875

URL: http://llvm.org/viewvc/llvm-project?rev=343875&view=rev
Log:
[CUDA] Use all 64 bits of GUID in __nv_module_id

getGUID() returns an uint64_t and "%x" only prints 32 bits of it.
Use PRIx64 format string to print all 64 bits.

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

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

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=343875&r1=343874&r2=343875&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Fri Oct  5 11:39:58 2018
@@ -520,7 +520,7 @@ llvm::Function *CGNVCUDARuntime::makeMod
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 


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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

This particular change is largely cosmetic. I've just spotted this nit while I 
was debugging a different problem.

It's also related to module ID.

We're trying to compile NCCL 2.3 with -fcuda-rdc and we were getting duplicate 
symbols when we tried to link multiple object files compiled from the same 
source file.
E.g.

  clang++ -DPART1 -o foo-1.o  foo.cu
  clang++ -DPART2 -o foo-2.o  foo.cu
  ...
  nvlink 

The stubs generated by nvlink ended up with conflicting names (based on module 
ID) that were identical for all foo-*.o.

It appears that clang generates ID based on the source file name only, so all 
foo-*.o end up with identical ID. 
NVCC, on the other hand, appears to generate ID based on some other factors 
(compiler flags? preprocessed TU source?) so each object file gets a unique ID.

For now we've worked around this by renaming the source files before 
compilation of each part, but we will need to find a better solution.


https://reviews.llvm.org/D52938



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

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343872: clang-format: Don't insert spaces in front of 
:: for Java 8 Method References. (authored by nico, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52842?vs=168161&id=168495#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52842

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJava.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2555,8 +2555,11 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Left.MatchingParen &&
   Left.MatchingParen->Previous &&
-  Left.MatchingParen->Previous->is(tok::period))
+  (Left.MatchingParen->Previous->is(tok::period) ||
+   Left.MatchingParen->Previous->is(tok::coloncolon)))
+// Java call to generic function with explicit type:
 // A.>>DoSomething();
+// A::>>DoSomething();  // With a Java 8 method reference.
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
@@ -2776,6 +2779,9 @@
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
 return false;
+  if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
+  (Left.is(tok::identifier) || Left.is(tok::kw_this)))
+return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
 // Generally don't remove existing spaces between an identifier and "::".
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -443,6 +443,22 @@
getStyleWithColumns(40));
 }
 
+TEST_F(FormatTestJava, MethodReference) {
+  EXPECT_EQ(
+  "private void foo() {\n"
+  "  f(this::methodReference);\n"
+  "  f(C.super::methodReference);\n"
+  "  Consumer c = System.out::println;\n"
+  "  Iface mRef = Ty::meth;\n"
+  "}",
+  format("private void foo() {\n"
+ "  f(this ::methodReference);\n"
+ "  f(C.super ::methodReference);\n"
+ "  Consumer c = System.out ::println;\n"
+ "  Iface mRef = Ty ::  meth;\n"
+ "}"));
+}
+
 TEST_F(FormatTestJava, CppKeywords) {
   verifyFormat("public void union(Type a, Type b);");
   verifyFormat("public void struct(Object o);");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2555,8 +2555,11 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Left.MatchingParen &&
   Left.MatchingParen->Previous &&
-  Left.MatchingParen->Previous->is(tok::period))
+  (Left.MatchingParen->Previous->is(tok::period) ||
+   Left.MatchingParen->Previous->is(tok::coloncolon)))
+// Java call to generic function with explicit type:
 // A.>>DoSomething();
+// A::>>DoSomething();  // With a Java 8 method reference.
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
@@ -2776,6 +2779,9 @@
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
 return false;
+  if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
+  (Left.is(tok::identifier) || Left.is(tok::kw_this)))
+return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
 // Generally don't remove existing spaces between an identifier and "::".
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -443,6 +443,22 @@
getStyleWithColumns(40));
 }
 
+TEST_F(FormatTestJava, MethodReference) {
+  EXPECT_EQ(
+  "private void foo() {\n"
+  "  f(this::methodReference);\n"
+  "  f(C.super::methodReference);\n"
+  "  Consumer c = System.out::println;\n"
+  "  Iface mRef = Ty::meth;\n"
+  "}",
+  format("private void foo() {\n"
+ "  f(this ::methodReference);\n"
+ "  f(C.super ::methodReference);\n"
+ "  Consumer c = System.out ::println;\n"
+ "  Iface mRef = Ty ::  meth;\n"
+ "}"));
+}
+
 TEST_F(FormatTestJava, CppKeywords) {
   verifyFormat("public void union(Type 

r343872 - clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Nico Weber via cfe-commits]

Author: nico
Date: Fri Oct  5 11:22:21 2018
New Revision: 343872

URL: http://llvm.org/viewvc/llvm-project?rev=343872&view=rev
Log:
clang-format: Don't insert spaces in front of :: for Java 8 Method References.

The existing code kept the space if it was there for identifiers, and it didn't
handle `this`. After this patch, for Java `this` is handled in addition to
identifiers, and existing space is always stripped between identifier and `::`.

Also accept `::` in addition to `.` in front of `<` in `foo::bar` generic
calls.

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJava.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=343872&r1=343871&r2=343872&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct  5 11:22:21 2018
@@ -2555,8 +2555,11 @@ bool TokenAnnotator::spaceRequiredBetwee
 return false;
   if (Left.is(TT_TemplateCloser) && Left.MatchingParen &&
   Left.MatchingParen->Previous &&
-  Left.MatchingParen->Previous->is(tok::period))
+  (Left.MatchingParen->Previous->is(tok::period) ||
+   Left.MatchingParen->Previous->is(tok::coloncolon)))
+// Java call to generic function with explicit type:
 // A.>>DoSomething();
+// A::>>DoSomething();  // With a Java 8 method reference.
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
@@ -2776,6 +2779,9 @@ bool TokenAnnotator::spaceRequiredBefore
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
 return false;
+  if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
+  (Left.is(tok::identifier) || Left.is(tok::kw_this)))
+return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
 // Generally don't remove existing spaces between an identifier and "::".
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If

Modified: cfe/trunk/unittests/Format/FormatTestJava.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJava.cpp?rev=343872&r1=343871&r2=343872&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJava.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp Fri Oct  5 11:22:21 2018
@@ -443,6 +443,22 @@ TEST_F(FormatTestJava, MethodDeclaration
getStyleWithColumns(40));
 }
 
+TEST_F(FormatTestJava, MethodReference) {
+  EXPECT_EQ(
+  "private void foo() {\n"
+  "  f(this::methodReference);\n"
+  "  f(C.super::methodReference);\n"
+  "  Consumer c = System.out::println;\n"
+  "  Iface mRef = Ty::meth;\n"
+  "}",
+  format("private void foo() {\n"
+ "  f(this ::methodReference);\n"
+ "  f(C.super ::methodReference);\n"
+ "  Consumer c = System.out ::println;\n"
+ "  Iface mRef = Ty ::  meth;\n"
+ "}"));
+}
+
 TEST_F(FormatTestJava, CppKeywords) {
   verifyFormat("public void union(Type a, Type b);");
   verifyFormat("public void struct(Object o);");


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

[PATCH] D52920: Introduce code_model macros

[Ali Tamur via Phabricator via cfe-commits]

tamur updated this revision to Diff 168494.

Repository:
  rC Clang

https://reviews.llvm.org/D52920

Files:
  include/clang/Basic/TargetOptions.h
  lib/Basic/Targets/X86.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -47,21 +47,21 @@
 // CXX11:#define __cplusplus 201103L
 // CXX11:#define __private_extern__ extern
 //
-// 
+//
 // RUN: %clang_cc1 -x c++ -std=c++98 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX98 %s
-// 
+//
 // CXX98:#define __GNUG__ {{.*}}
 // CXX98:#define __GXX_RTTI 1
 // CXX98:#define __GXX_WEAK__ 1
 // CXX98:#define __cplusplus 199711L
 // CXX98:#define __private_extern__ extern
 //
-// 
+//
 // RUN: %clang_cc1 -fdeprecated-macro -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix DEPRECATED %s
 //
 // DEPRECATED:#define __DEPRECATED 1
 //
-// 
+//
 // RUN: %clang_cc1 -std=c99 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C99 %s
 //
 // C99:#define __STDC_VERSION__ 199901L
@@ -71,7 +71,7 @@
 // C99-NOT: __GXX_WEAK__
 // C99-NOT: __cplusplus
 //
-// 
+//
 // RUN: %clang_cc1 -std=c11 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C11 %s
 // RUN: %clang_cc1 -std=c1x -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C11 %s
 // RUN: %clang_cc1 -std=iso9899:2011 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C11 %s
@@ -86,7 +86,7 @@
 // C11-NOT: __GXX_WEAK__
 // C11-NOT: __cplusplus
 //
-// 
+//
 // RUN: %clang_cc1 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix COMMON %s
 //
 // COMMON:#define __CONSTANT_CFSTRINGS__ 1
@@ -113,7 +113,7 @@
 // RUN: %clang_cc1 -E -dM -triple=x86_64-pc-linux-gnu < /dev/null | FileCheck -match-full-lines -check-prefix C-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -triple=x86_64-apple-darwin < /dev/null | FileCheck -match-full-lines -check-prefix C-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -triple=armv7a-apple-darwin < /dev/null | FileCheck -match-full-lines -check-prefix C-DEFAULT %s
-// 
+//
 // C-DEFAULT:#define __STDC_VERSION__ 201112L
 //
 // RUN: %clang_cc1 -ffreestanding -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix FREESTANDING %s
@@ -158,12 +158,12 @@
 // GXX98:#define __cplusplus 199711L
 // GXX98:#define __private_extern__ extern
 //
-// 
+//
 // RUN: %clang_cc1 -std=iso9899:199409 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C94 %s
 //
 // C94:#define __STDC_VERSION__ 199409L
 //
-// 
+//
 // RUN: %clang_cc1 -fms-extensions -triple i686-pc-win32 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix MSEXT %s
 //
 // MSEXT-NOT:#define __STDC__
@@ -185,7 +185,7 @@
 // MSEXT-CXX-NOWCHAR-NOT:#define _WCHAR_T_DEFINED 1
 // MSEXT-CXX-NOWCHAR:#define __BOOL_DEFINED 1
 //
-// 
+//
 // RUN: %clang_cc1 -x objective-c -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix OBJC %s
 //
 // OBJC:#define OBJC_NEW_PROPERTIES 1
@@ -197,7 +197,7 @@
 //
 // OBJCGC:#define __OBJC_GC__ 1
 //
-// 
+//
 // RUN: %clang_cc1 -x objective-c -fobjc-exceptions -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NONFRAGILE %s
 //
 // NONFRAGILE:#define OBJC_ZEROCOST_EXCEPTIONS 1
@@ -246,9 +246,9 @@
 //
 // PASCAL:#define __PASCAL_STRINGS__ 1
 //
-// 
+//
 // RUN: %clang_cc1 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix SCHAR %s
-// 
+//
 // SCHAR:#define __STDC__ 1
 // SCHAR-NOT:#define __UNSIGNED_CHAR__
 // SCHAR:#define __clang__ 1
@@ -7978,6 +7978,7 @@
 // X86_64:#define __WINT_WIDTH__ 32
 // X86_64:#define __amd64 1
 // X86_64:#define __amd64__ 1
+// X86_64:#define __code_model_small_ 1
 // X86_64:#define __x86_64 1
 // X86_64:#define __x86_64__ 1
 //
@@ -7987,7 +7988,10 @@
 // X86_64H:#define __x86_64__ 1
 // X86_64H:#define __x86_64h 1
 // X86_64H:#define __x86_64h__ 1
-
+//
+// RUN: %clang -xc - -E -dM -mcmodel=medium --target=i386-unknown-linux < /dev/null | FileCheck -match-full-lines -check-prefix X86_MEDIUM %s
+// X86_MEDIUM:#define __code_model_medium_ 1
+//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines -check-prefix X32 %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines -check-prefix X32 -check-prefix X32-CXX %s
 //
@@ -9830,16 +9834,16 @@
 // AVR:#define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
 // AVR:#define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
 // AVR:#define __GXX_ABI_VERSION 1002
-// AVR:#define __INT16_C_SUFFIX__ 
+// AVR:#define __INT16_C_SUFFIX__
 // AVR:#define __INT16_MAX__ 32767
 // AVR:#define __INT16_TYPE__ short
 // AVR:#define __INT32_C_SUFFIX__ L
 // AVR:#define __INT32_MAX__ 2147483647L
 // AVR:#define __INT32_TYPE__ long int
 // AVR:#define __INT64_C_SUFFIX__ LL
 // AVR:#define __INT64_MAX__ 9223372036854775807LL
 // AVR:#define __INT64_TYPE__ long long int
-/

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks! Will land with tweaked comment.




Comment at: lib/Format/TokenAnnotator.cpp:2559
 // A.>>DoSomething();
+// A::>>DoSomething();
 return false;

krasimir wrote:
> nit: please add a comment that this example comes from Java.
Will do. I believe the existing example is Java too.


https://reviews.llvm.org/D52842



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

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

The constant evaluation now returns false whenever indicating failure
would be appropriate for the requested mode, instead of returning
"true" for success, and depending on the caller examining the various
status variables after the fact, in some circumstances.

In EM_ConstantExpression mode, evaluation is aborted as soon as a
diagnostic note is generated, and therefore, a "true" return value now
actually indicates that the expression was a constexpr. (You no longer
have to additionally check for a lack of diagnostic messages.)

In EM_ConstantFold, evaluation is now aborted upon running into a
side-effect -- so a "true" return value now actually indicates that
there were no side-effects

Also -- update and clarify documentation for the evaluation modes.

This is a cleanup, not intending to change the functionality, as
callers had been checking those properties manually before.


https://reviews.llvm.org/D52939

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ExprConstant.cpp

Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -412,52 +412,10 @@
   MostDerivedArraySize = 2;
   MostDerivedPathLength = Entries.size();
 }
-void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
 void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
const APSInt &N);
 /// Add N to the address of this subobject.
-void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
-  if (Invalid || !N) return;
-  uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
-  if (isMostDerivedAnUnsizedArray()) {
-diagnoseUnsizedArrayPointerArithmetic(Info, E);
-// Can't verify -- trust that the user is doing the right thing (or if
-// not, trust that the caller will catch the bad behavior).
-// FIXME: Should we reject if this overflows, at least?
-Entries.back().ArrayIndex += TruncatedN;
-return;
-  }
-
-  // [expr.add]p4: For the purposes of these operators, a pointer to a
-  // nonarray object behaves the same as a pointer to the first element of
-  // an array of length one with the type of the object as its element type.
-  bool IsArray = MostDerivedPathLength == Entries.size() &&
- MostDerivedIsArrayElement;
-  uint64_t ArrayIndex =
-  IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
-  uint64_t ArraySize =
-  IsArray ? getMostDerivedArraySize() : (uint64_t)1;
-
-  if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
-// Calculate the actual index in a wide enough type, so we can include
-// it in the note.
-N = N.extend(std::max(N.getBitWidth() + 1, 65));
-(llvm::APInt&)N += ArrayIndex;
-assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
-diagnosePointerArithmetic(Info, E, N);
-setInvalid();
-return;
-  }
-
-  ArrayIndex += TruncatedN;
-  assert(ArrayIndex <= ArraySize &&
- "bounds check succeeded for out-of-bounds index");
-
-  if (IsArray)
-Entries.back().ArrayIndex = ArrayIndex;
-  else
-IsOnePastTheEnd = (ArrayIndex != 0);
-}
+bool adjustIndex(EvalInfo &Info, const Expr *E, APSInt N);
   };
 
   /// A stack frame in the constexpr call stack.
@@ -721,43 +679,68 @@
 bool IsSpeculativelyEvaluating;
 
 enum EvaluationMode {
-  /// Evaluate as a constant expression. Stop if we find that the expression
-  /// is not a constant expression.
+  /* The various EvaluationMode kinds vary in terms of what sorts of
+   * expressions they accept.
+
+ Key for the following table:
+ * D = Diagnostic notes (about non-constexpr, but still evaluable
+   constructs) are OK (keepEvaluatingAfterNote)
+ * S = Failure to evaluate which only occurs in expressions used for
+   side-effect are okay.  (keepEvaluatingAfterSideEffect)
+ * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)
+ * E = Eagerly evaluate certain builtins to a value which would normally
+   defer until after optimizations.
+
+ +-+---+---+---+---+
+ | | D | S | F | E |
+ +-+---+---+---+---+
+ |EM_ConstantExpression| . | . | . | . |
+ |EM_ConstantFold  | Y | . | . | . |
+ |EM_ConstantFoldUnevaluated   | Y | . | . | Y |
+ |EM_IgnoreSideEffects | Y | Y | . | . |
+ |EM_PotentialConstantExpression   | . | Y | Y | . |
+ |EM_PotentialConstan

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

[Victor Costan via Phabricator via cfe-commits]

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

test/SemaCXX/warn-thread-safety-analysis.cpp LGTM -- this is the functionality 
that we need in Chrome to use thread safety annotations for AutoUnlock.

very non-authoritative opinion - I think the tests would be a bit easier to 
follow if Lock() would be the EXCLUSIVE_LOCK_FUNCTION and Unlock() would be the 
EXCLUSIVE_UNLOCK_FUNCTION.

aaronpuchert: Thank you for the patch! I'd be happy and grateful to have this 
functionality in Chrome.
delesley: Thank you for your review and guidance!


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Jonas Hahnfeld via Phabricator via cfe-commits]

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

LG.

Out of interest: Is this fixing a particular issue?


https://reviews.llvm.org/D52938



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

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

Looks good!
I didn't find any instances where this messes-up C++ code (and it looks fine to 
me for Java code).


https://reviews.llvm.org/D52842



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

[PATCH] D52920: Introduce code_model macros

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: test/Preprocessor/init.c:7992
+//
+// RUN: %clang -xc - -E -dD -mcmodel=medium --target=i386-unknown-linux < 
/dev/null | FileCheck -match-full-lines -check-prefix X86_MEDIUM %s
+// X86_MEDIUM:#define __code_model_medium_ 1

`-dM` here is better because it suppresses other CPP output 
(https://github.com/llvm-mirror/clang/tree/BindingDecl/lib/Frontend/CompilerInvocation.cpp#L2962)



Repository:
  rC Clang

https://reviews.llvm.org/D52920



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

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:2559
 // A.>>DoSomething();
+// A::>>DoSomething();
 return false;

nit: please add a comment that this example comes from Java.


https://reviews.llvm.org/D52842



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

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343867: Emit diagnostic note when calling an invalid 
function declaration. (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52919?vs=168417&id=168490#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52919

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/enable_if.cpp


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a 
constant expression}} expected-error@-2{{no matching function for call to 
'templated'}} expected-note{{in instantiation of function template}} 
expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the 
problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant 
expression}} expected-error@-3{{no matching function for call to 'templated'}} 
expected-note{{in instantiation of function template}} 
expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52891: [AMDGPU] Add -fvisibility-amdgpu-non-kernel-functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Can you also fix HIP toolchain? It is in HIPToolChain::addClangTargetOptions. 
Thanks.


https://reviews.llvm.org/D52891



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

r343867 - Emit diagnostic note when calling an invalid function declaration.

[James Y Knight via cfe-commits]

Author: jyknight
Date: Fri Oct  5 10:49:48 2018
New Revision: 343867

URL: http://llvm.org/viewvc/llvm-project?rev=343867&view=rev
Log:
Emit diagnostic note when calling an invalid function declaration.

The comment said it was intentionally not emitting any diagnostic
because the declaration itself was already diagnosed. However,
everywhere else that wants to not emit a diagnostic without an extra
note emits note_invalid_subexpr_in_const_expr instead, which gets
suppressed later.

This was the only place which did not emit a diagnostic note.

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

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/enable_if.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=343867&r1=343866&r2=343867&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Oct  5 10:49:48 2018
@@ -4330,10 +4330,13 @@ static bool CheckConstexprFunction(EvalI
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&

Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=343867&r1=343866&r2=343867&view=diff
==
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Fri Oct  5 10:49:48 2018
@@ -414,7 +414,8 @@ static_assert(templated<1>() == 1, "");
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a 
constant expression}} expected-error@-2{{no matching function for call to 
'templated'}} expected-note{{in instantiation of function template}} 
expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the 
problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant 
expression}} expected-error@-3{{no matching function for call to 'templated'}} 
expected-note{{in instantiation of function template}} 
expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 


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

[PATCH] D52891: [AMDGPU] Add -fvisibility-amdgpu-non-kernel-functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D52891#1256207, @arsenm wrote:

> I think the name needs work, but I'm not sure what it should be. I think it 
> should avoid using "non" and "amdgpu"


I think dropping amdgpu is fine since we can add (AMDGUP only) to the 
description of the option, following the precedence of

  -ffixed-r9  Reserve the r9 register (ARM only)

However it is difficult to coin a different term for 'non-kernel-function'. 
Also, I saw precedence of using 'non' in option name:

  -objcmt-ns-nonatomic-iosonly

So, probably we could use `-fvisibility-nonkernel-function` ?


https://reviews.llvm.org/D52891



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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: Hahnfeld.
Herald added subscribers: bixia, jlebar, sanjoy.

getGUID() returns an uint64_t and "%x" only prints 32 bits of it.
Use PRIx64 format string to print all 64 bits.


https://reviews.llvm.org/D52938

Files:
  clang/lib/CodeGen/CGCUDANV.cpp


Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 


Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52920: Introduce code_model macros

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: include/clang/Basic/TargetOptions.h:72
+  // The code model to be used as specified by the user. Corresponds to
+  // Model enum defined in include/llvm/Support/CodeGen.h, plus "default" for
+  // the case when the user has not explicitly specified a code model.

`Model` -> `CodeModel::Model` because there are `Model` in other namespaces

```
  namespace Reloc {
  enum Model { Static, PIC_, DynamicNoPIC, ROPI, RWPI, ROPI_RWPI };
  }

  // Code model types.
  namespace CodeModel {
// Sync changes with CodeGenCWrappers.h.
  enum Model { Tiny, Small, Kernel, Medium, Large };
  }
```



Comment at: lib/Basic/Targets/X86.cpp:866
+// For compatibility with gcc.
+CodeModel = "small";
+  Builder.defineMacro("__code_model_" + CodeModel + "_");

The comment `// For compatibility with gcc.` needs changing.

Small code model is the fastest and expected to be suitable for vast majority 
of programs.

It is chosen here:

https://github.com/llvm-mirror/llvm/tree/master/lib/Target/X86/X86TargetMachine.cpp#L208

static CodeModel::Model getEffectiveCodeModel(Optional CM,
  bool JIT, bool Is64Bit) {
  if (CM)
return *CM;
  if (JIT)
return Is64Bit ? CodeModel::Large : CodeModel::Small;
  return CodeModel::Small;
}



Repository:
  rC Clang

https://reviews.llvm.org/D52920



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

[PATCH] D52445: [Index] Use locations to uniquify function-scope BindingDecl USR

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 168486.
MaskRay added a comment.

Add test to Core/index-source.cpp


Repository:
  rC Clang

https://reviews.llvm.org/D52445

Files:
  lib/Index/USRGeneration.cpp
  test/Index/Core/index-source.cpp


Index: test/Index/Core/index-source.cpp
===
--- test/Index/Core/index-source.cpp
+++ test/Index/Core/index-source.cpp
@@ -1,4 +1,5 @@
 // RUN: c-index-test core -print-source-symbols -- %s -std=c++1z -target 
x86_64-apple-macosx10.7 | FileCheck %s
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s 
-std=c++1z -target x86_64-apple-macosx10.7 | FileCheck -check-prefix=LOCAL %s
 
 // CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] |  | Def 
| rel: 0
 class Cls { public:
@@ -493,6 +494,7 @@
 // CHECK: [[@LINE-1]]:69 | variable/C++ | structuredBinding2 | 
c:@N@cpp17structuredBinding@structuredBinding2 |  | Ref,Read,RelCont 
| rel: 1
 // CHECK-NEXT: RelCont | localStructuredBindingAndRef | 
c:@N@cpp17structuredBinding@F@localStructuredBindingAndRef#
 // CHECK-NOT: localBinding
+// LOCAL: [[@LINE-4]]:9 | variable(local)/C++ | localBinding1 | 
c:index-source.cpp@25382@N@cpp17structuredBinding@F@localStructuredBindingAndRef#@localBinding1
 }
 
 }
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -97,6 +97,7 @@
   void VisitTypedefDecl(const TypedefDecl *D);
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D);
   void VisitVarDecl(const VarDecl *D);
+  void VisitBindingDecl(const BindingDecl *D);
   void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D);
   void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D);
   void VisitUnresolvedUsingValueDecl(const UnresolvedUsingValueDecl *D);
@@ -334,6 +335,12 @@
   }
 }
 
+void USRGenerator::VisitBindingDecl(const BindingDecl *D) {
+  if (D->getParentFunctionOrMethod() && GenLoc(D, /*IncludeOffset=*/true))
+return;
+  VisitNamedDecl(D);
+}
+
 void USRGenerator::VisitNonTypeTemplateParmDecl(
 const NonTypeTemplateParmDecl *D) {
   GenLoc(D, /*IncludeOffset=*/true);


Index: test/Index/Core/index-source.cpp
===
--- test/Index/Core/index-source.cpp
+++ test/Index/Core/index-source.cpp
@@ -1,4 +1,5 @@
 // RUN: c-index-test core -print-source-symbols -- %s -std=c++1z -target x86_64-apple-macosx10.7 | FileCheck %s
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s -std=c++1z -target x86_64-apple-macosx10.7 | FileCheck -check-prefix=LOCAL %s
 
 // CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] |  | Def | rel: 0
 class Cls { public:
@@ -493,6 +494,7 @@
 // CHECK: [[@LINE-1]]:69 | variable/C++ | structuredBinding2 | c:@N@cpp17structuredBinding@structuredBinding2 |  | Ref,Read,RelCont | rel: 1
 // CHECK-NEXT: RelCont | localStructuredBindingAndRef | c:@N@cpp17structuredBinding@F@localStructuredBindingAndRef#
 // CHECK-NOT: localBinding
+// LOCAL: [[@LINE-4]]:9 | variable(local)/C++ | localBinding1 | c:index-source.cpp@25382@N@cpp17structuredBinding@F@localStructuredBindingAndRef#@localBinding1
 }
 
 }
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -97,6 +97,7 @@
   void VisitTypedefDecl(const TypedefDecl *D);
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D);
   void VisitVarDecl(const VarDecl *D);
+  void VisitBindingDecl(const BindingDecl *D);
   void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D);
   void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D);
   void VisitUnresolvedUsingValueDecl(const UnresolvedUsingValueDecl *D);
@@ -334,6 +335,12 @@
   }
 }
 
+void USRGenerator::VisitBindingDecl(const BindingDecl *D) {
+  if (D->getParentFunctionOrMethod() && GenLoc(D, /*IncludeOffset=*/true))
+return;
+  VisitNamedDecl(D);
+}
+
 void USRGenerator::VisitNonTypeTemplateParmDecl(
 const NonTypeTemplateParmDecl *D) {
   GenLoc(D, /*IncludeOffset=*/true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r343862 - [clang-format] Java import sorting in clang-format

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Fri Oct  5 10:19:26 2018
New Revision: 343862

URL: http://llvm.org/viewvc/llvm-project?rev=343862&view=rev
Log:
[clang-format] Java import sorting in clang-format

Contributed by SamMaier!

Added:
cfe/trunk/unittests/Format/SortImportsTestJava.cpp
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/CMakeLists.txt

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=343862&r1=343861&r2=343862&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Fri Oct  5 10:19:26 2018
@@ -1130,6 +1130,35 @@ struct FormatStyle {
   /// \endcode
   bool IndentWrappedFunctionNames;
 
+  /// A vector of prefixes ordered by the desired groups for Java imports.
+  ///
+  /// Each group is seperated by a newline. Static imports will also follow the
+  /// same grouping convention above all non-static imports. One group's prefix
+  /// can be a subset of another - the longest prefix is always matched. Within
+  /// a group, the imports are ordered lexicographically.
+  ///
+  /// In the .clang-format configuration file, this can be configured like:
+  /// \code{.yaml}
+  ///   JavaImportGroups: ['com.example', 'com', 'org']
+  /// \endcode
+  /// Which will result in imports being formatted as so:
+  /// \code{.java}
+  ///import static com.example.function1;
+  ///
+  ///import static com.test.function2;
+  ///
+  ///import static org.example.function3;
+  ///
+  ///import com.example.ClassA;
+  ///import com.example.Test;
+  ///import com.example.a.ClassB;
+  ///
+  ///import com.test.ClassC;
+  ///
+  ///import org.example.ClassD;
+  /// \endcode
+  std::vector JavaImportGroups;
+
   /// Quotation styles for JavaScript strings. Does not affect template
   /// strings.
   enum JavaScriptQuoteStyle {
@@ -1734,6 +1763,7 @@ struct FormatStyle {
IndentPPDirectives == R.IndentPPDirectives &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
+   JavaImportGroups == R.JavaImportGroups &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=343862&r1=343861&r2=343862&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri Oct  5 10:19:26 2018
@@ -414,6 +414,7 @@ template <> struct MappingTraitshttps://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md#Import-Order
+ChromiumStyle.JavaImportGroups = {
+"android",
+"com",
+"dalvik",
+"junit",
+"org",
+"com.google.android.apps.chrome",
+"org.chromium",
+"java",
+"javax",
+};
+ChromiumStyle.SortIncludes = true;
   } else if (Language == FormatStyle::LK_JavaScript) {
 ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
 ChromiumStyle.AllowShortLoopsOnASingleLine = false;
@@ -1608,6 +1623,14 @@ struct IncludeDirective {
   int Category;
 };
 
+struct JavaImportDirective {
+  StringRef Identifier;
+  StringRef Text;
+  unsigned Offset;
+  std::vector AssociatedCommentLines;
+  bool IsStatic;
+};
+
 } // end anonymous namespace
 
 // Determines whether 'Ranges' intersects with ('Start', 'End').
@@ -1726,7 +1749,7 @@ static void sortCppIncludes(const Format
 
 namespace {
 
-const char IncludeRegexPattern[] =
+const char CppIncludeRegexPattern[] =
 R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
 
 } // anonymous namespace
@@ -1738,7 +1761,7 @@ tooling::Replacements sortCppIncludes(co
   unsigned *Cursor) {
   unsigned Prev = 0;
   unsigned SearchFrom = 0;
-  llvm::Regex IncludeRegex(IncludeRegexPattern);
+  llvm::Regex IncludeRegex(CppIncludeRegexPattern);
   SmallVector Matches;
   SmallVector IncludesInBlock;
 
@@ -1797,6 +1820,149 @@ tooling::Replacements sortCppIncludes(co
   return Replaces;
 }
 
+// Returns group number to use as a first order sort on imports. Gives UINT_MAX
+// if the import does not match any given groups.
+static unsigned findJavaImportGroup(const FormatStyle &Style,
+StringRef ImportIdentifier) {
+  unsigned LongestMatchIndex = UINT_MAX;
+  unsigned LongestMatchLength = 0;
+  for (unsigned I = 0; I < Style.JavaImportGroups.size(); I++) {
+std::string GroupPrefix = Style.JavaImportGroups[I];
+if (ImportIdentifier.startswith(GroupPrefix) &&
+GroupPre

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

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

In https://reviews.llvm.org/D52676#1251391, @oleg.smolsky wrote:

> In https://reviews.llvm.org/D52676#1251342, @krasimir wrote:
>
> > Digging a bit further, seems like the behavior you're looking for could be 
> > achieved by setting the `AlignAfterOpenBracket` option to `DontAlign` or 
> > `AlwaysBreak`:
> >
> >   % clang-format -style='{BasedOnStyle: google, AlignAfterOpenBracket: 
> > AlwaysBreak}' test.cc
> >void f() {
> >  something->Method2s(
> >  1,
> >  [this] {
> >Do1();
> >Do2();
> >  },
> >  1);
> >}
> >% clang-format -style='{BasedOnStyle: google, AlignAfterOpenBracket: 
> > DontAlign}' test.cc
> >void f() {
> >  something->Method2s(1,
> >  [this] {
> >Do1();
> >Do2();
> >  },
> >  1);
> >}
> >
> > Does this work for you?
>
>
> This is interesting. It obviously does what I want with the lambda, but 
> forces this:
>
>   void f() {
> auto stub = PopulateContextHereAndHereSomethi(GetSomethingHere(),
>   GetSomethingElseHere());
>   }
>
>
> into this:
>
>   void f() {
> auto stub = PopulateContextHereAndHereSomethi(
> GetSomethingHere(), GetSomethingElseHere());
>   }
>
> The former looks better to me and that's what Emacs does when you press Tab. 
> I think people here at work will balk at this formatting...
>
> > I don't think modifying the behavior as posed in this change based on the 
> > existence of multiline lambdas and the value of the `BinPackArguments` 
> > option is better than what we have now, especially when 
> > `AlignAfterOpenBracket=Align`.
>
> Yeah, I hear you. The patch is intended to work with 
> `AlignAfterOpenBracket=Align` and `BinPackArguments=false` and only tweaks 
> the lambdas' placement.
>
> How about I key the new behavior off a new value of `AlignAfterOpenBracket`, 
> such as `AlignAfterOpenBracket=AlignExceptLambda`?


Sorry, I don't think we want to support something this specific. I'd like to 
hear djasper@ and klimek@ opinions about this.


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1893
+
+IsCtor = isa(TargetDecl);
   }

I feel like you should just use `TargetDecl && 
isa(TargetDecl)` below; it's more obvious.

Is there not an analogous rule for destructors?


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:62
+CompilerInstance &Compiler) {
+  if (this->getLangOpts().CPlusPlus) {
+Compiler.getPreprocessor().addPPCallbacks(

JonasToth wrote:
> you dont need `this->` and please use the same return early pattern as in the 
> other registering call
By the word, see PR32774 for such check suggestion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52892



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm fine with being more aggressive about this, and I agree that the standard 
should be making aliasing UB here.  We use a similarly aggressive rule with 
return values: NRVO can allow direct access to the return slot, which we mark 
`noalias`, but which can in fact be aliased if we're doing copy-elision in the 
caller.  IIRC the standard has analogously weak wording about what's supposed 
to happen in that case, but UB is really the only sensible rule.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

I'll take a look. I'm a bit worried that this might potentially affect C++ 
files too: I'll run an experiment over some random files to confirm that we're 
not missing something.


https://reviews.llvm.org/D52842



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

Re: [PATCH] Add some automatic tests for DivideZero checker

[Tamás Zolnai via cfe-commits]

Hi,

I uploaded this patch to phabricator:
https://reviews.llvm.org/D52936

Best Regards,
Tamás

Tamás Zolnai  ezt írta (időpont: 2018. okt. 5.,
P, 14:14):

> Hi all,
>
> I'm a new contributor in clang / llvm. I'm planning to work on clang
> static analyzer: maybe add new checker, imporve the exisiting one, etc. As
> the first step I checked how core.DivideZero checker works now and added
> some test cases for regression testing (see attached patch).
> The patch contains not only test cases when the checker catches an issue,
> but also use cases when the checker misses a division by zero situation,
> showing that there is some points where the checker can be improved.
>
> Best Regards,
> Tamás Zolnai
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52598: [OpenCL] Fixed address space cast in C style cast of C++ parsing

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2288
+  SrcType->isPointerType()) {
+const PointerType *DestPtr = DestType->getAs();
+if (!DestPtr->isAddressSpaceOverlapping(*SrcType->getAs())) {

Anastasia wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Please test the result of `getAs` instead of separately testing 
> > > `isPointerType`.
> > > 
> > > Why is this check OpenCL-specific?  Address spaces are a general language 
> > > feature.
> > I think mainly because I am factoring out from the existing OpenCL check. 
> > But it probably makes sense that the semantics of this is not different in 
> > other languages. I will update it! Thanks!
> After playing with this for a bit longer I discovered that I have to keep the 
> OpenCL check unfortunately.
> 
> I found this old commit (`d4c5f84`/`r129583`) that says:
>   C-style casts can add/remove/change address spaces through the 
> reinterpret_cast mechanism.
> That's not the same as in OpenCL because it seems for C++ you can cast any AS 
> to any other AS. Therefore, no checks are needed at all. I am not sure if we 
> can come up with a common function for the moment.
> 
> The following  tests are checking this:
>CodeGenCXX/address-space-cast.cpp
>SemaCXX/address-space-conversion.cpp
> 
> Do you think it would make sense to rename this method with OpenCL-something 
> or keep in case may be CUDA or some other languages might need similar 
> functionality...
> 
I think you can leave it alone for now, but please add a comment explaining the 
reasoning as best you see it, and feel free to express uncertainty about the 
right rule.

Don't take `QualType` by `const &`, by the way.  It's a cheap-to-copy value 
type and should always be passed by value.


https://reviews.llvm.org/D52598



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

[PATCH] D52937: [clangd] Add clangd.serverInfo command to inspect server state.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.

Initially just export the information that's easily available.
(I want to measure changes in dynamic index size, so this is good enough for 
now)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52937

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -661,6 +661,8 @@
 struct ExecuteCommandParams {
   // Command to apply fix-its. Uses WorkspaceEdit as argument.
   const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
+  // Retrieve information about the server state. No arguments.
+  const static llvm::StringLiteral CLANGD_SERVER_INFO;
 
   /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
   std::string command;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -408,6 +408,8 @@
 
 const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
 "clangd.applyFix";
+const llvm::StringLiteral ExecuteCommandParams::CLANGD_SERVER_INFO =
+"clangd.serverInfo";
 bool fromJSON(const json::Value &Params, ExecuteCommandParams &R) {
   json::ObjectMapper O(Params);
   if (!O || !O.map("command", R.command))
@@ -417,6 +419,8 @@
   if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
 return Args && Args->size() == 1 &&
fromJSON(Args->front(), R.workspaceEdit);
+  } else if (R.command == ExecuteCommandParams::CLANGD_SERVER_INFO) {
+return true; // No args.
   }
   return false; // Unrecognized command.
 }
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -12,6 +12,7 @@
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -214,6 +215,28 @@
 
 reply("Fix applied.");
 ApplyEdit(*Params.workspaceEdit);
+  } else if (Params.command == ExecuteCommandParams::CLANGD_SERVER_INFO) {
+json::Object Info{
+{"version", getClangToolFullVersion("clangd")},
+};
+if (const auto *DynIndex = Server->dynamicIndex()) {
+  Info["dynamicIndex"] = json::Object{
+  {"memory", int64_t(DynIndex->estimateMemoryUsage())},
+  };
+}
+json::Object TrackedFiles;
+auto MemUse = Server->getUsedBytesPerFile();
+DenseMap MemUseMap = {MemUse.begin(), MemUse.end()};
+for (const auto &File : DraftMgr.getActiveFiles()) {
+  json::Object FileInfo;
+  if (auto Data = DraftMgr.getDraft(File))
+FileInfo["size"] = int64_t(Data->size());
+  if (auto Mem = MemUseMap.lookup(File))
+FileInfo["memory"] = int64_t(Mem);
+  TrackedFiles[File] = std::move(FileInfo);
+}
+Info["files"] = std::move(TrackedFiles);
+reply(std::move(Info));
   } else {
 // We should not get here because ExecuteCommandParams would not have
 // parsed in the first place and this handler should not be called. But if


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -661,6 +661,8 @@
 struct ExecuteCommandParams {
   // Command to apply fix-its. Uses WorkspaceEdit as argument.
   const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
+  // Retrieve information about the server state. No arguments.
+  const static llvm::StringLiteral CLANGD_SERVER_INFO;
 
   /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
   std::string command;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -408,6 +408,8 @@
 
 const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
 "clangd.applyFix";
+const llvm::StringLiteral ExecuteCommandParams::CLANGD_SERVER_INFO =
+"clangd.serverInfo";
 bool fromJSON(const json::Value &Params, ExecuteCommandParams &R) {
   json::ObjectMapper O(Params);
   if (!O || !O.map("command", R.command))
@@ -417,6 +419,8 @@
   if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
 return Args && Args->size() == 1 &&
fromJSON(Args->front(), R.workspaceEdit);
+  } else if (R.command == ExecuteCommandParams::CLANGD_SERVER_INFO) {
+return true; // No args.
   }
   return false; // Unrecognized command.
 }
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -12,6 +12,7 @@
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
 #include "URI.h

[PATCH] D52936: Add some automatic tests for DivideZero checker

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D52936

Files:
  test/Analysis/div-zero.cpp

Index: test/Analysis/div-zero.cpp
===
--- test/Analysis/div-zero.cpp
+++ test/Analysis/div-zero.cpp
@@ -11,3 +11,301 @@
   return (a % (qX-1)); // expected-warning {{Division by zero}}
 
 }
+
+
+void testDiv1() {
+  (void)(42 / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv2() {
+  (void)(42 / false);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv3() {
+  (void)(42 / !1);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv4() {
+  (void)(42 / (1 - 1));
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv5() {
+  (void)(42 / !(1 + 1));
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv6a() {
+  (void)(42 / (int)0.0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // Missing Division by zero warning
+}
+
+
+void testDiv6b() {
+  int x = (int)0.0;
+  (void)(42 / x);
+  // Missing Division by zero warning
+}
+
+
+void testDiv6c() {
+  float y = 0.2;
+  float z = 0.1;
+  int x = (int)(z + y);
+  (void)(42 / x);
+  // Missing Division by zero warning
+}
+
+
+void testDiv7() {
+  (void)(true / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv8() {
+  (void)(!1 / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv9() {
+  (void)((int)(9.0) / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv10() {
+  (void)((10 - 1) / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testRem() {
+  (void)(42 % 0);
+  // expected-warning@-1 {{remainder by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+int testDivAssign(int x) {
+  return (x /= 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+int testRemAssign(int x) {
+  return (x %= 0);
+  // expected-warning@-1 {{remainder by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testFloatDiv() {
+  (void)(42.0 / 0);
+  // No warning, the checker handles scalar types only
+}
+
+
+void testDivPath() {
+  int x = 2;
+  int y = 2;
+  int z = 10;
+
+  x = y / z;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testDivPath2() {
+  int x = 2;
+  int y = 0;
+  int z = 10;
+
+  x = y / z;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testRemPath() {
+  int x = 2;
+  int y = 2;
+  int z = 10;
+
+  x = z % y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testRemPath2() {
+  int x = 2;
+  int y = 2;
+  int z = 0;
+
+  x = z % y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testAdditionPath() {
+  int x = 2;
+  int y = 2;
+  int z = -2;
+
+  x = z + y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testSubtractionPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  x = z - y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testNegationPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  x = - y;
+  x = x + z;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testIfThenPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  if(y > 0)
+x = z - y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testIfElsePath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  if(y < 0) {
+x = z - y + 1;
+  } else {
+x = z - y;
+  }
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testSwitchPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  switch(y) {
+case 1: x = y; break;
+case 3: x = z; break;
+default: x %= z; break;
+  }
+
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testForLoopPath1() {
+  int x = 5;
+
+  for(int i = 0; i < 5; ++i) {
+x = x - 1;
+  }
+  (void)(42 / x); // Missing warning
+}
+
+
+void testForLoopPath2() {
+  int x = 0;
+
+  for(int i = 0; i < 5; ++i) {}
+  (void)(42 / x); // Missing warning
+}
+
+
+void testForLoopPath3() {
+  int x = 0;
+
+  for(int i = 0; i < 1; ++i) {}
+
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testWhileLoopPath() {
+  int x = 0;
+
+  int i = 0;
+  while(i < 5) {
+++i;
+  }
+  (void)(42

[PATCH] D51809: [CUDA][HIP] Fix ShouldDeleteSpecialMember for inherited constructors

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 168479.
yaxunl retitled this revision from "[CUDA][HIP] Fix assertion in 
LookupSpecialMember" to "[CUDA][HIP] Fix ShouldDeleteSpecialMember for 
inherited constructors".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Revised by Justin's comments.

Handle the situation where an inherited constructor is faked to be a default 
constructor but it is really not.


https://reviews.llvm.org/D51809

Files:
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCUDA/implicit-member-target-inherited.cu
  test/SemaCUDA/inherited-ctor.cu

Index: test/SemaCUDA/inherited-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/inherited-ctor.cu
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+// Inherit a valid non-default ctor.
+namespace NonDefaultCtorValid {
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {}
+  };
+
+  void test() {
+B b(0);
+C c;
+  }
+}
+
+// Inherit an invalid non-default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace NonDefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {} // expected-error{{constructor inherited by 'B' from base class 'A' is implicitly deleted}}
+  // expected-note@-6{{constructor inherited by 'B' is implicitly deleted because field 's' has a deleted corresponding constructor}}
+  // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
+
+// Inherit a valid default ctor.
+namespace DefaultCtorValid {
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() {}
+  };
+
+  void test() {
+B b;
+C c;
+  }
+}
+
+// Inherit an invalid default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace DefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() {} // expected-error{{call to implicitly-deleted default constructor of 'struct B'}}
+   // expected-note@-6{{default constructor of 'B' is implicitly deleted because field 's' has a deleted default constructor}}
+   // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
Index: test/SemaCUDA/implicit-member-target-inherited.cu
===
--- /dev/null
+++ test/SemaCUDA/implicit-member-target-inherited.cu
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -Wno-defaulted-function-deleted
+
+#include "Inputs/cuda.h"
+
+//--
+// Test 1: infer inherited default ctor to be host.
+
+struct A1_with_host_ctor {
+  A1_with_host_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+// The inherited default constructor is inferred to be host, so we'll encounter
+// an error when calling it from a __device__ function, but not from a __host__
+// function.
+struct B1_with_implicit_default_ctor : A1_with_host_ctor {
+  using A1_with_host_ctor::A1_with_host_ctor;
+};
+
+// expected-note@-4 {{call to __host__ function from __device__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A1_with_host_ctor' inherited here}}
+
+void hostfoo() {
+  B1_with_implicit_default_ctor b;
+}
+
+__device__ void devicefoo() {
+  B1_with_implicit_default_ctor b; // expected-error {{no matching constructor}}
+}
+
+//--
+// Test 2: infer inherited default ctor to be device.
+
+struct A2_with_device_ctor {
+  __device__ A2_with_device_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct B2_with_implicit_default_ctor : A2_with_device_ctor {
+  using A2_with_device_ctor::A2_with_device_ctor;
+};
+
+// expected-note@-4 {{call to __device__ function from __host__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A2_with_device_ctor' inherited here}}
+
+void hostfoo2() {
+  B2_wi

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In https://reviews.llvm.org/D52888#1256395, @aaron.ballman wrote:

> In https://reviews.llvm.org/D52888#1255862, @aaronpuchert wrote:
>
> > Additional changes (including some non-tail recursion unfortunately) would 
> > allow the following to work:
> >
> >   void foo16() {
> > if (cond ? mu.TryLock() : false)
> >   mu.Unlock();
> >   }
> >   
> >   void foo17() {
> > if (cond ? true : !mu.TryLock())
> >   return;
> > mu.Unlock();
> >   }
> >   
> >
> > Worth the effort, or is that too exotic?
>
>
> `foo16()` looks like code I could plausibly see in the wild. Consider the 
> case where the mutex is a pointer and you want to test it for null before 
> calling `TryLock()` on it (`M ? M->TryLock() : false`). However, I don't have 
> a good feeling for how much work it would be to support that case.


It's relatively short, but not exactly straightforward, because we have to 
check if both branches are "compatible". However, thinking about this again I 
think most programmers would write `foo16` as

  void foo16() {
if (cond && mu.TryLock())
  mu.Unlock();
  }

which is functionally equivalent, and which we already support. So I'd propose 
to add support for this only when someone asks for it, and leave it as it is 
for now.


Repository:
  rC Clang

https://reviews.llvm.org/D52888



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

Re: r343790 - [clang] Add the exclude_from_explicit_instantiation attribute

[Louis Dionne via cfe-commits]

I just saw this. Simon Pilgrim already fixed it in r343846. Thanks Simon!

Louis

> On Oct 4, 2018, at 22:02, Galina Kistanova  wrote:
> 
> Hello Louis,
> 
> This commit broke build step on one of our builders:
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/13042
>  
> 
> 
> . . .
> FAILED: tools/clang/lib/Sema/CMakeFiles/clangSema.dir/SemaExprCXX.cpp.obj 
> C:\PROGRA~2\MICROS~1.0\VC\bin\amd64\cl.exe  /nologo /TP -DEXPENSIVE_CHECKS 
> -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
> -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE 
> -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_DEBUG -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 
> -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -Itools\clang\lib\Sema 
> -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\lib\Sema
>  
> -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\include
>  -Itools\clang\include -Iinclude 
> -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\include 
> /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 
> -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 
> -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 
> -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 
> -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 
> -wd4592 -wd4319 -wd4324 -w14062 -we4238 /MDd /Zi /Ob0 /Od /RTC1/EHs-c- 
> /GR- /showIncludes 
> /Fotools\clang\lib\Sema\CMakeFiles\clangSema.dir\SemaExprCXX.cpp.obj 
> /Fdtools\clang\lib\Sema\CMakeFiles\clangSema.dir\clangSema.pdb /FS -c 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp
>  : fatal error C1128: number of sections exceeded object file format limit: 
> compile with /bigobj
> 
> 
> Please have a look?
> The builder was already red and did not send notifications on this.
> 
> Thanks
> 
> Galina
> 
> On Thu, Oct 4, 2018 at 8:51 AM Louis Dionne via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: ldionne
> Date: Thu Oct  4 08:49:42 2018
> New Revision: 343790
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=343790&view=rev 
> 
> Log:
> [clang] Add the exclude_from_explicit_instantiation attribute
> 
> Summary:
> This attribute allows excluding a member of a class template from being part
> of an explicit template instantiation of that class template. This also makes
> sure that code using such a member will not take for granted that an external
> instantiation exists in another translation unit. The attribute was discussed
> on cfe-dev at [1] and is primarily motivated by the removal of always_inline
> in libc++ to control what's part of the ABI (see links in [1]).
> 
> [1]: http://lists.llvm.org/pipermail/cfe-dev/2018-August/059024.html 
> 
> 
> rdar://problem/43428125
> 
> Reviewers: rsmith
> 
> Subscribers: dexonsmith, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D51789 
> 
> 
> Added:
> 
> cfe/trunk/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.dont_assume_extern_instantiation.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.diagnose_on_undefined_entity.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.explicit_instantiation.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/Sema/Sema.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
> 
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=343790&r1=343789&r2=343790&view=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Thu Oct  4 08:49:42 2018
> @@ -3042,6 +3042,13 @@ def InternalLinkage : InheritableAttr {
>let Documentation = [InternalLinkageDocs];
>  }
> 
> +def ExcludeFromExplicitInstantiation : InheritableAttr {
> +  let Spellings = [C

[PATCH] D52800: Java import sorting in clang-format

[Krasimir Georgiev via Phabricator via cfe-commits]

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

This gooks great! Thanks for the contribution!
If you don't have commit access to Clang I can commit this for you.


Repository:
  rC Clang

https://reviews.llvm.org/D52800



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

[PATCH] D52879: Derive builtin return type from its definition

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

LGTM from OpenCL side!


Repository:
  rC Clang

https://reviews.llvm.org/D52879



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

[PATCH] D52873: Remove unwanted signedness conversion from tests

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52873



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

[PATCH] D52658: [OpenCL] Remove PIPE_RESERVE_ID_VALID_BIT from opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52658



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

r343857 - [OPENMP][NVPTX] Fix emission of __kmpc_global_thread_num() for non-SPMD

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct  5 08:27:47 2018
New Revision: 343857

URL: http://llvm.org/viewvc/llvm-project?rev=343857&view=rev
Log:
[OPENMP][NVPTX] Fix emission of __kmpc_global_thread_num() for non-SPMD
mode.

__kmpc_global_thread_num() should be called before initialization of the
runtime.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=343857&r1=343856&r2=343857&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Fri Oct  5 08:27:47 2018
@@ -1083,12 +1083,15 @@ void CGOpenMPRuntimeNVPTX::emitNonSPMDKe
  CGOpenMPRuntimeNVPTX::WorkerFunctionState &WST)
 : EST(EST), WST(WST) {}
 void Enter(CodeGenFunction &CGF) override {
-  static_cast(CGF.CGM.getOpenMPRuntime())
-  .emitNonSPMDEntryHeader(CGF, EST, WST);
+  auto &RT = static_cast(CGF.CGM.getOpenMPRuntime());
+  RT.emitNonSPMDEntryHeader(CGF, EST, WST);
+  // Skip target region initialization.
+  RT.setLocThreadIdInsertPt(CGF, /*AtCurrentPoint=*/true);
 }
 void Exit(CodeGenFunction &CGF) override {
-  static_cast(CGF.CGM.getOpenMPRuntime())
-  .emitNonSPMDEntryFooter(CGF, EST);
+  auto &RT = static_cast(CGF.CGM.getOpenMPRuntime());
+  RT.clearLocThreadIdInsertPt(CGF);
+  RT.emitNonSPMDEntryFooter(CGF, EST);
 }
   } Action(EST, WST);
   CodeGen.setAction(Action);

Modified: cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp?rev=343857&r1=343856&r2=343857&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp Fri Oct  5 08:27:47 2018
@@ -105,7 +105,6 @@ int main (int argc, char **argv) {
 // CK2: [[AADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[BADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[ARGCADDR:%.+]] = alloca i{{[0-9]+}},
-// CK2:  {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2: store i{{[0-9]+}} [[A_IN]], i{{[0-9]+}}* [[AADDR]],
 // CK2: store i{{[0-9]+}} [[B_IN]], i{{[0-9]+}}* [[BADDR]],
 // CK2: store i{{[0-9]+}} [[ARGC_IN]], i{{[0-9]+}}* [[ARGCADDR]],
@@ -117,6 +116,7 @@ int main (int argc, char **argv) {
 // CK2-32:  [[ARG:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[ARGCADDR]]
 // CK2:  [[ARGCADDR:%.+]] = getelementptr inbounds %struct.{{.*}}, 
%struct.{{.*}}* %{{.*}}, i{{[0-9]+}} 0, i{{[0-9]+}} 0
 // CK2:  store i{{[0-9]+}} [[ARG]], i{{[0-9]+}}* [[ARGCADDR]],
+// CK2:  {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2:  store i{{[0-9]+}}* [[ARGCADDR]], i{{[0-9]+}}** [[ARGCADDR_PTR]],
 // CK2:  [[ARGCADDR_PTR_REF:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** 
[[ARGCADDR_PTR]],
 // CK2: store i{{[0-9]+}} 0, i{{[0-9]+}}* [[ARGCADDR_PTR_REF]],
@@ -129,7 +129,6 @@ int main (int argc, char **argv) {
 // CK2: [[AADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[BADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[ARGCADDR:%.+]] = alloca i{{[0-9]+}}**,
-// CK2: {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2: store i{{[0-9]+}} [[A_IN]], i{{[0-9]+}}* [[AADDR]],
 // CK2: store i{{[0-9]+}} [[B_IN]], i{{[0-9]+}}* [[BADDR]],
 // CK2: store i{{[0-9]+}}** [[ARGC]], i{{[0-9]+}}*** [[ARGCADDR]],
@@ -137,6 +136,7 @@ int main (int argc, char **argv) {
 // CK2: [[ARG:%.+]] = load i{{[0-9]+}}**, i{{[0-9]+}}*** [[ARGCADDR]]
 // CK2: [[ARGCADDR:%.+]] = getelementptr inbounds %struct.{{.*}}, 
%struct.{{.*}}* %{{.*}}, i{{[0-9]+}} 0, i{{[0-9]+}} 0
 // CK2: store i{{[0-9]+}}** [[ARG]], i{{[0-9]+}}*** [[ARGCADDR]],
+// CK2: {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2: store i{{[0-9]+}}*** [[ARGCADDR]], i{{[0-9]+}} [[ARGCADDR_PTR]],
 // CK2: [[ARGCADDR_PTR_REF:%.+]] = load i{{[0-9]+}}***, i{{[0-9]+}} 
[[ARGCADDR_PTR]],
 // CK2: store i{{[0-9]+}}** null, i{{[0-9]+}}*** [[ARGCADDR_PTR_REF]],


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

r343856 - [OPENMP] Fix emission of the __kmpc_global_thread_num.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct  5 08:08:53 2018
New Revision: 343856

URL: http://llvm.org/viewvc/llvm-project?rev=343856&view=rev
Log:
[OPENMP] Fix emission of the __kmpc_global_thread_num.

Fixed emission of the __kmpc_global_thread_num() so that it is not
messed up with alloca instructions anymore. Plus, fixes emission of the
__kmpc_global_thread_num() functions in the target outlined regions so
that they are not called before runtime is initialized.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp

cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
cfe/trunk/test/OpenMP/parallel_if_codegen.cpp
cfe/trunk/test/OpenMP/single_codegen.cpp
cfe/trunk/test/OpenMP/single_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskgroup_task_reduction_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=343856&r1=343855&r2=343856&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Oct  5 08:08:53 2018
@@ -1485,6 +1485,31 @@ Address CGOpenMPRuntime::getOrCreateDefa
   return Address(Entry, Align);
 }
 
+void CGOpenMPRuntime::setLocThreadIdInsertPt(CodeGenFunction &CGF,
+ bool AtCurrentPoint) {
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  assert(!Elem.second.ServiceInsertPt && "Insert point is set already.");
+
+  llvm::Value *Undef = llvm::UndefValue::get(CGF.Int32Ty);
+  if (AtCurrentPoint) {
+Elem.second.ServiceInsertPt = new llvm::BitCastInst(
+Undef, CGF.Int32Ty, "svcpt", CGF.Builder.GetInsertBlock());
+  } else {
+Elem.second.ServiceInsertPt =
+new llvm::BitCastInst(Undef, CGF.Int32Ty, "svcpt");
+Elem.second.ServiceInsertPt->insertAfter(CGF.AllocaInsertPt);
+  }
+}
+
+void CGOpenMPRuntime::clearLocThreadIdInsertPt(CodeGenFunction &CGF) {
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  if (Elem.second.ServiceInsertPt) {
+llvm::Instruction *Ptr = Elem.second.ServiceInsertPt;
+Elem.second.ServiceInsertPt = nullptr;
+Ptr->eraseFromParent();
+  }
+}
+
 llvm::Value *CGOpenMPRuntime::emitUpdateLocation(CodeGenFunction &CGF,
  SourceLocation Loc,
  unsigned Flags) {
@@ -1511,8 +1536,10 @@ llvm::Value *CGOpenMPRuntime::emitUpdate
 Elem.second.DebugLoc = AI.getPointer();
 LocValue = AI;
 
+if (!Elem.second.ServiceInsertPt)
+  setLocThreadIdInsertPt(CGF);
 CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
-CGF.Builder.SetInsertPoint(CGF.AllocaInsertPt);
+CGF.Builder.SetInsertPoint(Elem.second.ServiceInsertPt);
 CGF.Builder.CreateMemCpy(LocValue, getOrCreateDefaultLocation(Flags),
  CGF.getTypeSize(IdentQTy));
   }
@@ -1582,21 +1609,25 @@ llvm::Value *CGOpenMPRuntime::getThreadI
   // kmpc_global_thread_num(ident_t *loc).
   // Generate thread id value and cache this value for use across the
   // function.
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  if (!Elem.second.ServiceInsertPt)
+setLocThreadIdInsertPt(CGF);
   CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
-  CGF.Builder.SetInsertPoint(CGF.AllocaInsertPt);
+  CGF.Builder.SetInsertPoint(Elem.second.ServiceInsertPt);
   llvm::CallInst *Call = CGF.Builder.CreateCall(
   createRuntimeFunction(OMPRTL__kmpc_global_thread_num),
   emitUpdateLocation(CGF, Loc));
   Call->setCallingConv(CGF.getRuntimeCC());
-  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
   Elem.second.ThreadID = Call;
   return Call;
 }
 
 void CGOpenMPRuntime::functionFinished(CodeGenFunction &CGF) {
   assert(CGF.CurFn && "No function in current CodeGenFunction.");
-  if (OpenMPLocThreadIDMap.count(CGF.CurFn))
+  if (OpenMPLocThreadIDMap.count(CGF.CurFn)) {
+clearLocThreadIdInsertPt(CGF);
 OpenMPLocThreadIDMap.erase(CGF.CurFn);
+  }
   if (FunctionUDRMap.count(CGF.CurFn) > 0) {
 for(auto *D : FunctionUDRMap[CGF.CurFn])
   UDRMap.erase(D);

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=343856&r1=343855&r2=343856&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Fri Oct  5 08:08:53 2018
@@ -278,6 +278,10 @@ protected:
   /// stored.
   virtual Address emitThreadIDAddress(CodeGenFunction &CGF, SourceLocation 
Loc);
 
+  void setL

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 168477.
jyknight added a comment.

Added test.


https://reviews.llvm.org/D52918

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c-casts.c


Index: clang/test/Sema/c-casts.c
===
--- /dev/null
+++ clang/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();


Index: clang/test/Sema/c-casts.c
===
--- /dev/null
+++ clang/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSTyp

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D52918#1256420, @aaron.ballman wrote:

> Patch is missing tests -- perhaps you could dump the AST and check the 
> casting notation from the output?


It would appear that which casts get emitted in C mode is almost completely 
untested. I added tests just for this change in a new file, and left a TODO to 
fill it out for the remainder of those functions.


https://reviews.llvm.org/D52918



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

[PATCH] D52598: [OpenCL] Fixed address space cast in C style cast of C++ parsing

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2288
+  SrcType->isPointerType()) {
+const PointerType *DestPtr = DestType->getAs();
+if (!DestPtr->isAddressSpaceOverlapping(*SrcType->getAs())) {

Anastasia wrote:
> rjmccall wrote:
> > Please test the result of `getAs` instead of separately testing 
> > `isPointerType`.
> > 
> > Why is this check OpenCL-specific?  Address spaces are a general language 
> > feature.
> I think mainly because I am factoring out from the existing OpenCL check. But 
> it probably makes sense that the semantics of this is not different in other 
> languages. I will update it! Thanks!
After playing with this for a bit longer I discovered that I have to keep the 
OpenCL check unfortunately.

I found this old commit (`d4c5f84`/`r129583`) that says:
  C-style casts can add/remove/change address spaces through the 
reinterpret_cast mechanism.
That's not the same as in OpenCL because it seems for C++ you can cast any AS 
to any other AS. Therefore, no checks are needed at all. I am not sure if we 
can come up with a common function for the moment.

The following  tests are checking this:
   CodeGenCXX/address-space-cast.cpp
   SemaCXX/address-space-conversion.cpp

Do you think it would make sense to rename this method with OpenCL-something or 
keep in case may be CUDA or some other languages might need similar 
functionality...



https://reviews.llvm.org/D52598



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

[PATCH] D49244: Always search sysroot for GCC installs

[David Greene via Phabricator via cfe-commits]

greened updated this revision to Diff 168472.
greened added a comment.

Updated to implement option 2.  I'm not totally happy with passing a StringRef 
just to check if it's non-empty but opted to reduce the size of the diff rather 
than refactor a bunch of stuff.


Repository:
  rC Clang

https://reviews.llvm.org/D49244

Files:
  lib/Driver/ToolChains/Gnu.cpp


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1618,10 +1618,18 @@
   return GoodVersion;
 }
 
-static llvm::StringRef getGCCToolchainDir(const ArgList &Args) {
+static llvm::StringRef getGCCToolchainDir(const ArgList &Args,
+  llvm::StringRef SysRoot) {
   const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain);
   if (A)
 return A->getValue();
+
+  // If we have a SysRoot, ignore GCC_INSTALL_PREFIX.
+  // GCC_INSTALL_PREFIX specifies the gcc installation for the default
+  // sysroot and is likely not valid with a different sysroot.
+  if (!SysRoot.empty())
+return "";
+
   return GCC_INSTALL_PREFIX;
 }
 
@@ -1653,7 +1661,7 @@
   SmallVector Prefixes(D.PrefixDirs.begin(),
D.PrefixDirs.end());
 
-  StringRef GCCToolchainDir = getGCCToolchainDir(Args);
+  StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
   if (GCCToolchainDir != "") {
 if (GCCToolchainDir.back() == '/')
   GCCToolchainDir = GCCToolchainDir.drop_back(); // remove the /


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1618,10 +1618,18 @@
   return GoodVersion;
 }
 
-static llvm::StringRef getGCCToolchainDir(const ArgList &Args) {
+static llvm::StringRef getGCCToolchainDir(const ArgList &Args,
+  llvm::StringRef SysRoot) {
   const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain);
   if (A)
 return A->getValue();
+
+  // If we have a SysRoot, ignore GCC_INSTALL_PREFIX.
+  // GCC_INSTALL_PREFIX specifies the gcc installation for the default
+  // sysroot and is likely not valid with a different sysroot.
+  if (!SysRoot.empty())
+return "";
+
   return GCC_INSTALL_PREFIX;
 }
 
@@ -1653,7 +1661,7 @@
   SmallVector Prefixes(D.PrefixDirs.begin(),
D.PrefixDirs.end());
 
-  StringRef GCCToolchainDir = getGCCToolchainDir(Args);
+  StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
   if (GCCToolchainDir != "") {
 if (GCCToolchainDir.back() == '/')
   GCCToolchainDir = GCCToolchainDir.drop_back(); // remove the /
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52598: [OpenCL] Fixed address space cast in C style cast of C++ parsing

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 168471.
Anastasia added a comment.

Change AS checking function to be a method of  `CastOperation`.


https://reviews.llvm.org/D52598

Files:
  lib/Sema/SemaCast.cpp
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl

Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -9,46 +9,47 @@
   __local int lj = 2; // expected-error {{'__local' variable cannot have an initializer}}
 
   int *ip;
-// FIXME: Temporarily disable part of the test that doesn't work for C++ yet.
-#if !__OPENCL_CPP_VERSION__
-#if __OPENCL_C_VERSION__ < 200
+#if ((!__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ < 200))
   ip = gip; // expected-error {{assigning '__global int *' to 'int *' changes address space of pointer}}
   ip = &li; // expected-error {{assigning '__local int *' to 'int *' changes address space of pointer}}
   ip = &ci; // expected-error {{assigning '__constant int *' to 'int *' changes address space of pointer}}
 #else
   ip = gip;
   ip = &li;
-  ip = &ci; // expected-error {{assigning '__constant int *' to '__generic int *' changes address space of pointer}}
+  ip = &ci;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{assigning '__constant int *' to '__generic int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{assigning to '__generic int *' from incompatible type '__constant int *'}}
+#endif
 #endif
 }
 
-void explicit_cast(global int* g, local int* l, constant int* c, private int* p, const constant int *cc)
-{
-  g = (global int*) l;// expected-error {{casting '__local int *' to type '__global int *' changes address space of pointer}}
-  g = (global int*) c;// expected-error {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
-  g = (global int*) cc;   // expected-error {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
-  g = (global int*) p;// expected-error {{casting 'int *' to type '__global int *' changes address space of pointer}}
+void explicit_cast(__global int *g, __local int *l, __constant int *c, __private int *p, const __constant int *cc) {
+  g = (__global int *)l;  // expected-error {{casting '__local int *' to type '__global int *' changes address space of pointer}}
+  g = (__global int *)c;  // expected-error {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
+  g = (__global int *)cc; // expected-error {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
+  g = (__global int *)p;  // expected-error {{casting 'int *' to type '__global int *' changes address space of pointer}}
 
-  l = (local int*) g; // expected-error {{casting '__global int *' to type '__local int *' changes address space of pointer}}
-  l = (local int*) c; // expected-error {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
-  l = (local int*) cc;// expected-error {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
-  l = (local int*) p; // expected-error {{casting 'int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)g;  // expected-error {{casting '__global int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)c;  // expected-error {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)cc; // expected-error {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)p;  // expected-error {{casting 'int *' to type '__local int *' changes address space of pointer}}
 
-  c = (constant int*) g;  // expected-error {{casting '__global int *' to type '__constant int *' changes address space of pointer}}
-  c = (constant int*) l;  // expected-error {{casting '__local int *' to type '__constant int *' changes address space of pointer}}
-  c = (constant int*) p;  // expected-error {{casting 'int *' to type '__constant int *' changes address space of pointer}}
+  c = (__constant int *)g; // expected-error {{casting '__global int *' to type '__constant int *' changes address space of pointer}}
+  c = (__constant int *)l; // expected-error {{casting '__local int *' to type '__constant int *' changes address space of pointer}}
+  c = (__constant int *)p; // expected-error {{casting 'int *' to type '__constant int *' changes address space of pointer}}
 
-  p = (private int*) g;   // expected-error {{casting '__global int *' to type 'int *' changes address space of pointer}}
-  p = (private int*) l;   // expected-error {{casting '__local int *' to type 'int *' changes address space of pointer}}
-  p = (private int*) c;   // expected-error {{casting '__constant int *' 

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

C.131 seems to imply a minimal amount of trivial getters/setters before
diagnosing.

I feel that the CPPCG just want to force the programmer to think twice
instead of forbidding it totally.
It might be worth to have a more chatty/specific check for the CPPCG and
a strict "Don't do it" for HICPP.

From my experience with asking the CPPCG authors it takes a while for a
response and that in the end is
not necessarily telling which way to go. Experience might differ
depending on topic though.

> Hmm, I don't know that it would help with code like this:
> 
>   class Base {
> int f; // Suggested by C.133/C.9
>   
>   protected:
>   //  int f; // Disallowed by C.133
>   
> int getF() const { return f; } // Suggested by C.133, disallowed by C.131
> void setF(int NewF) { f = NewF; }  // Suggested by C.133, disallowed by 
> C.131
>   };
> 
>> Still worth asking the authors.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils to be usable without ASTContext

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343850: [clang-tidy] NFC refactor lexer-utils to be usable 
without ASTContext (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52684

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h

Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -91,8 +91,9 @@
 getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
   std::vector> Comments;
   while (Loc.isValid()) {
-clang::Token Tok =
-utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false);
+clang::Token Tok = utils::lexer::getPreviousToken(
+Loc, Ctx->getSourceManager(), Ctx->getLangOpts(),
+/*SkipComments=*/false);
 if (Tok.isNot(tok::comment))
   break;
 Loc = Tok.getLocation();
Index: clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
@@ -40,7 +40,8 @@
 return;
 
   ASTContext &Ctxt = *Result.Context;
-  auto Token = utils::lexer::getPreviousToken(Ctxt, LocStart);
+  auto Token = utils::lexer::getPreviousToken(LocStart, Ctxt.getSourceManager(),
+  Ctxt.getLangOpts());
   auto &SM = *Result.SourceManager;
   unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
 
Index: clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
@@ -19,8 +19,8 @@
 namespace lexer {
 
 /// Returns previous token or ``tok::unknown`` if not found.
-Token getPreviousToken(const ASTContext &Context, SourceLocation Location,
-   bool SkipComments = true);
+Token getPreviousToken(SourceLocation Location, const SourceManager &SM,
+   const LangOptions &LangOpts, bool SkipComments = true);
 
 } // namespace lexer
 } // namespace utils
Index: clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
@@ -14,19 +14,15 @@
 namespace utils {
 namespace lexer {
 
-Token getPreviousToken(const ASTContext &Context, SourceLocation Location,
-   bool SkipComments) {
-  const auto &SourceManager = Context.getSourceManager();
+Token getPreviousToken(SourceLocation Location, const SourceManager &SM,
+   const LangOptions &LangOpts, bool SkipComments) {
   Token Token;
   Token.setKind(tok::unknown);
   Location = Location.getLocWithOffset(-1);
-  auto StartOfFile =
-  SourceManager.getLocForStartOfFile(SourceManager.getFileID(Location));
+  auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
   while (Location != StartOfFile) {
-Location = Lexer::GetBeginningOfToken(Location, SourceManager,
-  Context.getLangOpts());
-if (!Lexer::getRawToken(Location, Token, SourceManager,
-Context.getLangOpts()) &&
+Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
+if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
 (!SkipComments || !Token.is(tok::comment))) {
   break;
 }
Index: clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
@@ -18,7 +18,8 @@
 
 FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
   SourceLocation AmpLocation = Var.getLocation();
-  auto Token = utils::lexer::getPreviousToken(Context, AmpLocation);
+  auto Token = utils::lexer::getPreviousToken(
+  AmpLocation, Context.getSourceManager(), Context.getLangOpts());
   if (!Token.is(tok::unknown))
 AmpLocation = Lexer::getLocForEndOfToken(Token.getLocation(), 0,
  Context.getSou

[clang-tools-extra] r343850 - [clang-tidy] NFC refactor lexer-utils to be usable without ASTContext

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Fri Oct  5 07:15:19 2018
New Revision: 343850

URL: http://llvm.org/viewvc/llvm-project?rev=343850&view=rev
Log:
[clang-tidy] NFC refactor lexer-utils to be usable without ASTContext

Summary:
This patch is a small refactoring necessary for
'readability-isolate-declaration' and does not introduce functional changes.
It allows to use the utility functions without a full `ASTContext` and requires 
only the `SourceManager` and the `LangOpts`.

Reviewers: alexfh, aaron.ballman, hokein

Reviewed By: alexfh

Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits

Tags: #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp Fri 
Oct  5 07:15:19 2018
@@ -91,8 +91,9 @@ static std::vector> Comments;
   while (Loc.isValid()) {
-clang::Token Tok =
-utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false);
+clang::Token Tok = utils::lexer::getPreviousToken(
+Loc, Ctx->getSourceManager(), Ctx->getLangOpts(),
+/*SkipComments=*/false);
 if (Tok.isNot(tok::comment))
   break;
 Loc = Tok.getLocation();

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp 
Fri Oct  5 07:15:19 2018
@@ -40,7 +40,8 @@ void SuspiciousSemicolonCheck::check(con
 return;
 
   ASTContext &Ctxt = *Result.Context;
-  auto Token = utils::lexer::getPreviousToken(Ctxt, LocStart);
+  auto Token = utils::lexer::getPreviousToken(LocStart, 
Ctxt.getSourceManager(),
+  Ctxt.getLangOpts());
   auto &SM = *Result.SourceManager;
   unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
 

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
Fri Oct  5 07:15:19 2018
@@ -120,12 +120,14 @@ struct IntializerInsertion {
 switch (Placement) {
 case InitializerPlacement::New:
   Location = utils::lexer::getPreviousToken(
- Context, Constructor.getBody()->getBeginLoc())
+ Constructor.getBody()->getBeginLoc(),
+ Context.getSourceManager(), Context.getLangOpts())
  .getLocation();
   break;
 case InitializerPlacement::Before:
   Location = utils::lexer::getPreviousToken(
- Context, Where->getSourceRange().getBegin())
+ Where->getSourceRange().getBegin(),
+ Context.getSourceManager(), Context.getLangOpts())
  .getLocation();
   break;
 case InitializerPlacement::After:

Modified: clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp Fri Oct  5 
07:15:19 2018
@@ -18,7 +18,8 @@ namespace fixit {
 
 FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
   SourceLocation AmpLocation = Var.getLocation();
-  auto Token = utils::lexer::getPreviousToken(Context, AmpLocation);
+  auto Token = utils::lexe

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use

[Alexander Kornienko via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D52684#1250885, @JonasToth wrote:

> This patch is related to https://reviews.llvm.org/D51949
>
> To isolate variable declarations (split `int * p, v;` up) it is necessary to 
> do a lot of work with source location and requires some forward and backwards 
> lexing. The functions there just use the LangOpts and the SourceManager and 
> don't have a `ASTContext` available, that why I changed this interface.


Makes sense. But then I'd change "to be easier to use" to "to be callable 
without ASTContext" or something similar in the patch description.

Otherwise LG.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52684



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52771#1256511, @JonasToth wrote:

> In https://reviews.llvm.org/D52771#1256432, @aaron.ballman wrote:
>
> > I can't help but notice how badly C.133 and C.9 interact with C.131 and I'm 
> > worried we will wind up with clang-tidy checks that leave the user in an 
> > impossible situation where they need to make data members private and 
> > provide trivial accessors for them. Do you have thoughts on how to avoid 
> > that? The C++ Core Guidelines seem silent on the matter -- this might be 
> > worth raising with the authors.
>
>
> I have an idea to avoid trivial set/get methods: Flag public data if it is 
> modified internally. This could serve as a heuristic for pre/postconditions 
> on that variable. 
>  In general one could resolve all data-dependencies inside the class and 
> figure out if a private variable would need to change if a public variable 
> got changed. But that is way more complex!


Hmm, I don't know that it would help with code like this:

  class Base {
int f; // Suggested by C.133/C.9
  
  protected:
  //  int f; // Disallowed by C.133
  
int getF() const { return f; } // Suggested by C.133, disallowed by C.131
void setF(int NewF) { f = NewF; }  // Suggested by C.133, disallowed by 
C.131
  };

> Still worth asking the authors.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[clang-tools-extra] r343849 - [clangd] Remove last usage of ast matchers from SymbolCollector. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct  5 07:03:04 2018
New Revision: 343849

URL: http://llvm.org/viewvc/llvm-project?rev=343849&view=rev
Log:
[clangd] Remove last usage of ast matchers from SymbolCollector. NFC

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=343849&r1=343848&r2=343849&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri Oct  5 
07:03:04 2018
@@ -19,7 +19,6 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Index/IndexSymbol.h"
@@ -229,10 +228,10 @@ getTokenLocation(SourceLocation TokLoc,
 // the first seen declaration as canonical declaration is not a good enough
 // heuristic.
 bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) {
-  using namespace clang::ast_matchers;
+  const auto& SM = ND.getASTContext().getSourceManager();
   return (Roles & static_cast(index::SymbolRole::Definition)) &&
  llvm::isa(&ND) &&
- match(decl(isExpansionInMainFile()), ND, ND.getASTContext()).empty();
+ !SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getLocation()));
 }
 
 RefKind toRefKind(index::SymbolRoleSet Roles) {
@@ -260,7 +259,6 @@ void SymbolCollector::initialize(ASTCont
 bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
   ASTContext &ASTCtx,
   const Options &Opts) {
-  using namespace clang::ast_matchers;
   if (ND.isImplicit())
 return false;
   // Skip anonymous declarations, e.g (anonymous enum/class/struct).


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

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/misc-misplaced-const.c:18
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a 
const-qualified typedef type; results in the type being 'int *const' instead of 
'const int *'
+  // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
 

alexfh wrote:
> JonasToth wrote:
> > alexfh wrote:
> > > These notes are also just marginally useful and make it harder to change 
> > > the test. I wonder whether an absolute line number would make more sense 
> > > here. Or maybe just add a test for one of the notes and leave out the 
> > > rest (and keep CHECK-MESSAGES)?
> > Absolute line number makes sense. IMHO the tests should cover all generated 
> > diagnostics including the notes. Would you accept sticking with 
> > `CHECK-NOTES` but with absolute line numbers?
> > IMHO the tests should cover all generated diagnostics including the notes.
> 
> I wouldn't call this the most important goal. I'd say that tests should cover 
> important aspects of the output, not every single character of it. Another 
> useful feature is that tests should be easy to create, read, and change.  In 
> cases like this - where the benefit of the change is not obvious - I would 
> leave the decision to the author of the check.
> 
> Aaron, WDYT?
@aaron.ballman not sure if you overlooked that note


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52690



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D52771#1256432, @aaron.ballman wrote:

> I can't help but notice how badly C.133 and C.9 interact with C.131 and I'm 
> worried we will wind up with clang-tidy checks that leave the user in an 
> impossible situation where they need to make data members private and provide 
> trivial accessors for them. Do you have thoughts on how to avoid that? The 
> C++ Core Guidelines seem silent on the matter -- this might be worth raising 
> with the authors.


I have an idea to avoid trivial set/get methods: Flag public data if it is 
modified internally. This could serve as a heuristic for pre/postconditions on 
that variable. 
In general one could resolve all data-dependencies inside the class and figure 
out if a private variable would need to change if a public variable got 
changed. But that is way more complex!

Still worth asking the authors.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:26
+
+AST_MATCHER(clang::CXXRecordDecl, hasNonStaticMethod) {
+  return hasMethod(unless(isStaticStorageClass()))

lebedev.ri wrote:
> JonasToth wrote:
> > I think this and the next matcher can be a normal variable.
> > 
> > ```
> > auto HasNonStaticMethod = hasMethod(unless(isStaticStorageClass()));
> > 
> > ...
> > ... allOf(anyOf(isStruct(), isClass()), HasMethods, HasNonStaticMethod)
> > ...
> > ```
> They could, but these have a very meaningful meaning, so i'd argue it would 
> not be worse to keep them 
> out here refactored as proper matchers, in case someone else needs them 
> elsewhere (so he could just
> move them from here into more general place, as opposed to writing a 
> duplicating matcher.)
Ok.



Comment at: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp:1
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: 
[{key: 
misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, 
value: 0}, {key: 
misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic,
 value: 0}]}' --

lebedev.ri wrote:
> JonasToth wrote:
> > I would prefer multiple test-files instead of mixing them all together.
> Hmm, no. Then the tests will have to be duplicated in N files,
> because i really do want to see what each of these 4 configurations do on the 
> *same* input.
> 
> It only looks ugly because the script only supports `-check-suffix=`,
> and not `-check-suffixes=`, which would significantly cut down on check-lines.
Ok, if you prefer this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

[Jonas Toth via Phabricator via cfe-commits]

JonasToth closed this revision.
JonasToth added a comment.

Commited in https://reviews.llvm.org/rL343848.
Thank you for the patch!


https://reviews.llvm.org/D51332



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

[clang-tools-extra] r343848 - [clang-tidy] Replace deprecated std::ios_base aliases

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Fri Oct  5 06:36:00 2018
New Revision: 343848

URL: http://llvm.org/viewvc/llvm-project?rev=343848&view=rev
Log:
[clang-tidy] Replace deprecated std::ios_base aliases

This check warns the uses of the deprecated member types of std::ios_base
and replaces those that have a non-deprecated equivalent.

Patch by andobence!

Reviewd by: alexfh

Revision ID: https://reviews.llvm.org/D51332

Added:

clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-ios-base-aliases.rst

clang-tools-extra/trunk/test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt?rev=343848&r1=343847&r2=343848&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt Fri Oct  5 
06:36:00 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyModernizeModu
   AvoidBindCheck.cpp
   ConcatNestedNamespacesCheck.cpp
   DeprecatedHeadersCheck.cpp
+  DeprecatedIosBaseAliasesCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeSharedCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp?rev=343848&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp 
Fri Oct  5 06:36:00 2018
@@ -0,0 +1,80 @@
+//===--- DeprecatedIosBaseAliasesCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "DeprecatedIosBaseAliasesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+static const std::array DeprecatedTypes = {
+"::std::ios_base::io_state",  "::std::ios_base::open_mode",
+"::std::ios_base::seek_dir",  "::std::ios_base::streamoff",
+"::std::ios_base::streampos",
+};
+
+static const llvm::StringMap ReplacementTypes = {
+{"io_state", "iostate"},
+{"open_mode", "openmode"},
+{"seek_dir", "seekdir"}};
+
+void DeprecatedIosBaseAliasesCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  auto IoStateDecl = typedefDecl(hasAnyName(DeprecatedTypes)).bind("TypeDecl");
+  auto IoStateType =
+  qualType(hasDeclaration(IoStateDecl), unless(elaboratedType()));
+
+  Finder->addMatcher(typeLoc(loc(IoStateType)).bind("TypeLoc"), this);
+}
+
+void DeprecatedIosBaseAliasesCheck::check(
+const MatchFinder::MatchResult &Result) {
+  SourceManager &SM = *Result.SourceManager;
+
+  const auto *Typedef = Result.Nodes.getNodeAs("TypeDecl");
+  StringRef TypeName = Typedef->getName();
+  bool HasReplacement = ReplacementTypes.count(TypeName);
+
+  const auto *TL = Result.Nodes.getNodeAs("TypeLoc");
+  SourceLocation IoStateLoc = TL->getBeginLoc();
+
+  // Do not generate fixits for matches depending on template arguments and
+  // macro expansions.
+  bool Fix = HasReplacement && !TL->getType()->isDependentType();
+  if (IoStateLoc.isMacroID()) {
+IoStateLoc = SM.getSpellingLoc(IoStateLoc);
+Fix = false;
+  }
+
+  SourceLocation EndLoc = IoStateLoc.getLocWithOffset(TypeName.size() - 1);
+
+  if (HasReplacement) {
+auto FixName = ReplacementTypes.lookup(TypeName);
+auto Builder = diag(IoStateLoc, "'std::ios_base::%0' is deprecated; use "
+"'std::ios_base::%1' instead")
+   << TypeName << FixName;
+
+if (Fix)
+  Builder << FixItHint::CreateReplacement(SourceRange(IoStateLoc, EndLoc),
+  FixName);
+  } else
+diag(IoStateLoc, "'std::ios_base::%0' is deprecated") << TypeNa

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I can't help but notice how badly C.133 and C.9 interact with C.131 and I'm 
worried we will wind up with clang-tidy checks that leave the user in an 
impossible situation where they need to make data members private and provide 
trivial accessors for them. Do you have thoughts on how to avoid that? The C++ 
Core Guidelines seem silent on the matter -- this might be worth raising with 
the authors.

The HIC++ standard has an exception that requires the data type to be in an 
`extern "C"` context, but I don't see that reflected in the tests or code.




Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:22
+
+AST_MATCHER(clang::CXXRecordDecl, hasMethods) {
+  return std::distance(Node.method_begin(), Node.method_end()) != 0;

Can drop the `clang::` qualifiers -- you're in the `namespace clang` scope 
already.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:56
+  auto RecordIsInteresting =
+  allOf(anyOf(isStruct(), isClass()), hasMethods(), hasNonStaticMethod(),
+IgnoreClassesWithAllMemberVariablesBeingPublic

Neither the C++ Core Guidelines nor HIC++ carve out an exception for union 
types. I think you should talk to the guideline authors to see what they think. 
Perhaps first try to diagnose unions the same as structs and classes and see 
how the results look to you -- if they look bad, you can incorporate those as 
examples when discussing with the authors.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:82
+   "member variable '%0' of %1 '%2' has %3 visibility")
+  << Field->getName() << Record->getKindName() << Record->getName()
+  << Field->getAccess();

Drop the single quotes above and just pass in `Field` and `Record` directly -- 
the diagnostic printer will do the right thing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

r343846 - Fix llvm-clang-x86_64-expensive-checks-win build by setting bigobj flag.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Fri Oct  5 05:33:57 2018
New Revision: 343846

URL: http://llvm.org/viewvc/llvm-project?rev=343846&view=rev
Log:
Fix llvm-clang-x86_64-expensive-checks-win build by setting bigobj flag.

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

Modified: cfe/trunk/lib/Sema/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/CMakeLists.txt?rev=343846&r1=343845&r2=343846&view=diff
==
--- cfe/trunk/lib/Sema/CMakeLists.txt (original)
+++ cfe/trunk/lib/Sema/CMakeLists.txt Fri Oct  5 05:33:57 2018
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 if (MSVC)
   set_source_files_properties(SemaDeclAttr.cpp PROPERTIES COMPILE_FLAGS 
/bigobj)
   set_source_files_properties(SemaExpr.cpp PROPERTIES COMPILE_FLAGS /bigobj)
+  set_source_files_properties(SemaExprCXX.cpp PROPERTIES COMPILE_FLAGS /bigobj)
   set_source_files_properties(SemaTemplate.cpp PROPERTIES COMPILE_FLAGS 
/bigobj)
 endif()
 


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

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Patch is missing tests -- perhaps you could dump the AST and check the casting 
notation from the output?


https://reviews.llvm.org/D52918



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

Re: [clang-tools-extra] r343778 - [clangd] clangd-indexer gathers refs and stores them in index files.

[Sam McCall via cfe-commits]

Thanks! There was actually some old debugging code that got left over in a
test. Sorry about that!
Fixed in r343845.

On Fri, Oct 5, 2018 at 11:34 AM  wrote:

> Hi Sam,
>
> Our internal build bot hit an assertion failure in the changes you made in
> this commit that I was able to reproduce by building your change on my
> machine on Windows 7 using Visual Studio 2015. None of the public build
> bots seem to have hit this issue, though I am not sure why at the moment.
>
> Here is the assertion failure that is hit while running ClangdTests.exe:
>
> [--] 2 tests from SerializationTest
> [ RUN  ] SerializationTest.YAMLConversions
> [   OK ] SerializationTest.YAMLConversions (0 ms)
> [ RUN  ] SerializationTest.BinaryConversions
> Assertion failed: OutBufCur == OutBufStart && "raw_ostream destructor
> called with non-empty buffer!", file
> C:\src\upstream\llvm2\lib\Support\raw_ostream.cpp, line 73
> 0x00013FA74405 (0x0016 0x0200
> 0x06D100230006 0x07FEEA9BAA51), HandleAbort() + 0x5 bytes(s),
> c:\src\upstream\llvm2\lib\support\windows\signals.inc, line 409
> 0x07FEEAA1DC17 (0x0001 0x0001
> 0x 0x009BF460), raise() + 0x1E7 bytes(s)
> 0x07FEEAA1EAA1 (0x07FE0003 0x07FE0003
> 0x0001413AF810 0x0001413AF790), abort() + 0x31 bytes(s)
> 0x07FEEAA20751 (0x0049 0x0001413AF810
> 0x0122 0x07FEEA9C05D6), _get_wpgmptr() + 0x1BE1 bytes(s)
> 0x07FEEAA20A5F (0x009BF890 0x009BF680
> 0x00B05AB0 0x00014126F8C5), _wassert() + 0x3F bytes(s)
> 0x00013FA40677 (0x 0x00A8
> 0x009BF890 0x00013FA41A2E), llvm::raw_ostream::~raw_ostream() +
> 0x37 bytes(s), c:\src\upstream\llvm2\lib\support\raw_ostream.cpp, line 75
> 0x00013FA4057C (0x04BA1FF0 0x009BF680
> 0x00B05AB0 0x00B05AB0),
> llvm::raw_fd_ostream::~raw_fd_ostream() + 0x11C bytes(s),
> c:\src\upstream\llvm2\lib\support\raw_ostream.cpp, line 617
> 0x00013F96A4CC (0x04BA1FF0 0x00B05AB0
> 0x00B05AB0 0x04BA1FF0), clang::clangd::`anonymous
> namespace'::SerializationTest_BinaryConversions_Test::TestBody() + 0x34C
> bytes(s),
> c:\src\upstream\llvm2\tools\clang\tools\extra\unittests\clangd\serializationtests.cpp,
> line 166
> 0x00013FAE201C (0x04BA1FF0 0x00B05AB0
> 0x0001413C4B68 0x07FEEAA954B8),
> testing::internal::HandleSehExceptionsInMethodIfSupported()
> + 0xC bytes(s),
> c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2387 +
> 0x2 byte(s)
> 0x00013FB09E5B (0x04BA1FF0 0x00B05AB0
> 0x0001413C4D30 0x009BF9E8), testing::Test::Run() + 0x7B
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 2481
> 0x00013FB0A0DB (0x016641B94CAF 0x00B05AB0
> 0x00B1DE30 0x00AFF560), testing::TestInfo::Run() + 0xAB
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 2660
> 0x00013FB09F72 (0x0023 0x
> 0x00B05AB0 0x00AFF560), testing::TestCase::Run() + 0xB2
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 2774 + 0x12 byte(s)
> 0x00013FB0A529 (0x00AFF410 0x
> 0x0001 0x07FE0001),
> testing::internal::UnitTestImpl::RunAllTests() + 0x229 bytes(s),
> c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 4649 +
> 0x39 byte(s)
> 0x00013FAE213C (0x00AFF410 0x
> 0x0001413C54A8 0x),
> testing::internal::HandleSehExceptionsInMethodIfSupported()
> + 0xC bytes(s),
> c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2387 +
> 0x2 byte(s)
> 0x00013FB0A27C (0x8002 0x
> 0x 0x), testing::UnitTest::Run() + 0xEC
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 4257 + 0x17 byte(s)
> 0x000141299397 (0x00010001 0x
> 0x 0x01D45C4625BC735E), main() + 0xB7 bytes(s),
> c:\src\upstream\llvm2\utils\unittest\unittestmain\testmain.cpp, line 52
> 0x0001412707C1 (0x 0x
> 0x 0x), __scrt_common_main_seh() + 0x11D
> bytes(s), f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl, line 253
> + 0x22 byte(s)
> 0x76C159CD (0x 0x
> 0x 0x), BaseThreadInitThunk() + 0xD bytes(s)
> 0x76E7385D (0x 0x
> 0x 0x), RtlUserThreadStart() + 0x1D bytes(s)
>
> Could you take a look into this?
>
> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> > Of Sam McCall via cfe-c

[clang-tools-extra] r343845 - [clangd] Remove debugging output in test

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct  5 05:22:40 2018
New Revision: 343845

URL: http://llvm.org/viewvc/llvm-project?rev=343845&view=rev
Log:
[clangd] Remove debugging output in test

Modified:
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp?rev=343845&r1=343844&r2=343845&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp Fri Oct  5 
05:22:40 2018
@@ -157,11 +157,6 @@ TEST(SerializationTest, BinaryConversion
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
   std::string Serialized = llvm::to_string(Out);
-  {
-std::error_code EC;
-llvm::raw_fd_ostream F("/tmp/foo", EC);
-F << Serialized;
-  }
 
   auto In2 = readIndexFile(Serialized);
   ASSERT_TRUE(bool(In2)) << In.takeError();


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

[PATCH] Add some automatic tests for DivideZero checker

[Tamás Zolnai via cfe-commits]

Hi all,

I'm a new contributor in clang / llvm. I'm planning to work on clang static
analyzer: maybe add new checker, imporve the exisiting one, etc. As the
first step I checked how core.DivideZero checker works now and added some
test cases for regression testing (see attached patch).
The patch contains not only test cases when the checker catches an issue,
but also use cases when the checker misses a division by zero situation,
showing that there is some points where the checker can be improved.

Best Regards,
Tamás Zolnai


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

[PATCH] D52928: [clangd] Fix a subtle case for GetBeginningOfIdentifier.

[Sam McCall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343844: [clangd] Fix a subtle case for 
GetBeginningOfIdentifier. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52928?vs=168445&id=168455#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52928

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdUnitTests.cpp


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,23 +205,40 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct Bar { int func(); };
+#define MACRO(X) void f() { X; }
+Bar* bar;
+  )cpp";
   // First ^ is the expected beginning, last is the search position.
-  for (const char *Text : {
+  for (std::string Text : std::vector{
"int ^f^oo();", // inside identifier
"int ^foo();",  // beginning of identifier
"int ^foo^();", // end of identifier
"int foo(^);",  // non-identifier
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
"int ^λλ^λ();", // UTF-8 handled properly when backing up
+
+   // identifier in macro arg
+   "MACRO(bar->^func())",  // beginning of identifier
+   "MACRO(bar->^fun^c())", // inside identifier
+   "MACRO(bar->^func^())", // end of identifier
+   "MACRO(^bar->func())",  // begin identifier
+   "MACRO(^bar^->func())", // end identifier
+   "^MACRO(bar->func())",  // beginning of macro name
+   "^MAC^RO(bar->func())", // inside macro name
+   "^MACRO^(bar->func())", // end of macro name
}) {
-Annotations TestCase(Text);
+std::string WithPreamble = Preamble + Text;
+Annotations TestCase(WithPreamble);
 auto AST = TestTU::withCode(TestCase.code()).build();
 const auto &SourceMgr = AST.getASTContext().getSourceManager();
 SourceLocation Actual = getBeginningOfIdentifier(
 AST, TestCase.points().back(), SourceMgr.getMainFileID());
-Position ActualPos =
-offsetToPosition(TestCase.code(), SourceMgr.getFileOffset(Actual));
+Position ActualPos = offsetToPosition(
+TestCase.code(),
+SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
 EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
   }
 }
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -390,7 +390,6 @@
 log("getBeginningOfIdentifier: {0}", Offset.takeError());
 return SourceLocation();
   }
-  SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
 
   // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
   // if the cursor is at the end of the identifier.
@@ -401,15 +400,16 @@
   //  3) anywhere outside an identifier, we'll get some non-identifier thing.
   // We can't actually distinguish cases 1 and 3, but returning the original
   // location is correct for both!
+  SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
   if (*Offset == 0) // Case 1 or 3.
 return SourceMgr.getMacroArgExpandedLocation(InputLoc);
-  SourceLocation Before =
-  SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1));
+  SourceLocation Before = SourceMgr.getComposedLoc(FID, *Offset - 1);
+
   Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
   Token Tok;
   if (Before.isValid() &&
   !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
   Tok.is(tok::raw_identifier))
-return Before;// Case 2.
+return SourceMgr.getMacroArgExpandedLocation(Before); // Case 2.
   return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
 }


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,23 +205,40 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct Bar { int func(); };
+#define MACRO(X) void f() { X; }
+Bar* bar;
+  )cpp";
   // First ^ is the expected beginning, last is the search position.
-  for (const char *Text : {
+  for (std::string Text : std::vector{
"int ^f^oo();", // inside identifier
"int ^foo();",  // beginning of identifier
"int ^foo^();", // end of identifier
"int foo(^);",  // non-identifier
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)