[PATCH] D59407: [clangd] Add RelationSlab

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.

Still LG, thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.
Herald added a subscriber: ebevhan.
Herald added a project: clang.

Do you know when this will be merged?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54258



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

[PATCH] D62471: [clangd] SymbolCollector support for relations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:298
+
+  processRelations(*ND, *ID, Relations);
+

nridge wrote:
> kadircet wrote:
> > why do we want to process these relations for references?
> The `RelationBaseOf` is not present in the `Relations` parameter of the 
> `handleDeclOccurrence()` call for the base class's declaration. It's only 
> present for the specific reference that's in the base-specifier.
yeah just checked the indexing code, that's unfortunate :/.

could you add a comment explaining the situation ?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:431
+  for (const auto &R : Relations) {
+if (!(R.Roles & static_cast(index::SymbolRole::RelationBaseOf))) 
{
+  continue;

kadircet wrote:
> this might get complicated, maybe isolate it to a `bool 
> shouldIndexRelation(const Relation&)`
nit: no need for braces



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:453
+  this->Relations.insert(
+  Relation{ID, index::SymbolRole::RelationBaseOf, *ParentID});
+}

nridge wrote:
> nridge wrote:
> > kadircet wrote:
> > > kadircet wrote:
> > > > I believe subject is `ParentID` and object is `ID` in a baseof relation 
> > > > so these should be replaced ?
> > > also we should generalize this with something like 
> > > `index::applyForEachSymbolRole`, which should also get rid of the check 
> > > at top(`shouldIndexRelation`)
> > > I believe subject is ParentID and object is ID in a baseof relation so 
> > > these should be replaced ?
> > 
> > The code was functionally correct, but the name `ParentID` was wrong -- the 
> > `RelatedSymbol` for a `BaseOf` relation is the child.
> > 
> > Anyways, I'm now calling it `ObjectID`.
> > also we should generalize this with something like 
> > index::applyForEachSymbolRole, which should also get rid of the check at 
> > top(shouldIndexRelation)
> 
> I don't fully understand this suggestion; could you elaborate?
> 
> Are you thinking ahead to a future where we want to index additional kinds of 
> relations? Can we refactor things at that time?
yes I was thinking ahead. just wanted to make sure we can add other relation 
types without changing the logic inside `processrelations`.

but it is not that important, let's keep it that way, we can refactor it later 
on.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:441
+auto ObjectID = getSymbolID(Object);
+if (!ObjectID) {
+  continue;

nit: braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62471



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

[PATCH] D62623: Reduce memory consumption of coverage dumps

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

In D62623#1522575 , @serge-sans-paille 
wrote:

> @kwk: looks like you're still compiling with clang-7, this patch is to be 
> applied on the master version of LLVM/clang, then you can install it and use 
> it to recompile llvm/lld with coverage info.
>
> Does it make sense to you?


@serge-sans-paille oh yes. Absolutely. Thank you! I will try that out right 
away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62623



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

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 202388.
SjoerdMeijer added a comment.

This addresses @t.p.northover comment.


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

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -484,22 +484,100 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef &Name) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName &InputFPU = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName &CandidateFPU : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector &Features) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  bool Negated = stripNegationPrefix(ArchExt);
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+
+if (ArchExt == "fp.dp") {
+  unsigned DoubleFPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+
+  // If the default FPU already supports double-precision, or if
+  // there is no double-prec FPU that extends it, then "fp.dp"
+  // doesn't have a separate meaning, and we treat it as an
+  // invalid extension name.
+  if (DoubleFPUKind == FK_INVALID)
+return false;
+
+  // If there _is_ a separate double-precision FPU, then "nofp.dp"
+  // should disable just the double-precision extension, leaving
+  // the base FPU still enabled if it previously was.
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+
+  // Otherwise, select the double-precision FPU.
+  FPUKind = DoubleFPUKind;
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+  if (FPUKind == ARM::FK_NONE)
+return false;
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -240,6 +240,8 @@
 StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(unsigned ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
+bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+   std::vector &Features);
 StringRef getHWDivName(unsigned HWDivKind);
 
 // Information by Name
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "l

[PATCH] D62459: [clangd] Serialization support for RelationSlab

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LG, thanks for the patch!




Comment at: clang-tools-extra/clangd/index/Serialization.cpp:34
+  switch (Role) {
+  case index::SymbolRole::RelationBaseOf: {
+return RelationKind::BaseOf;

nit: no need for braces



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:44
+  switch (Kind) {
+  case RelationKind::BaseOf: {
+return index::SymbolRole::RelationBaseOf;

nit: braces



Comment at: clang-tools-extra/clangd/index/Serialization.h:81
+// Used for serializing SymbolRole as used in Relation.
+enum class RelationKind : uint8_t { ChildOf = 1, BaseOf };
+llvm::Expected symbolRoleToRelationKind(index::SymbolRole);

nridge wrote:
> nridge wrote:
> > nridge wrote:
> > > kadircet wrote:
> > > > kadircet wrote:
> > > > > why not start from zero?
> > > > > 
> > > > > Also let's change the `index::SymbolRole` in `Relation` with this one.
> > > > why not just store baseof ? do we need both directions for 
> > > > typehierarchy?
> > > > Also let's change the index::SymbolRole in Relation with this one.
> > > 
> > > You previously asked for the opposite change in [this 
> > > comment](https://reviews.llvm.org/D59407?id=190785#inline-525734) :)
> > > 
> > > Did your opinion change?
> > > why not start from zero?
> > 
> > The original motivation was so that we had a distinct value to return in 
> > case of an error. However, now that we use `llvm_unreachable` that doesn't 
> > seem to be necessary any more.
> > why not just store baseof ? do we need both directions for typehierarchy?
> 
> In the future, we may want to be able to look up supertypes in the index as 
> well (for example, to handle scenarios where a type that is only 
> forward-declared in the current TU is queried).
> 
> However, we can add that later, and just focus on subtypes for now, which 
> only require BaseOf.
ahaha good catch :D for some reason I thought we were also storing our own 
struct in `Symbol` for `SmybolInfo` but apparently we are not. Just ignore that 
one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62459



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

[PATCH] D62722: [Driver] Simplify Assemble and Backend action collapsing

[Kévin Petit via Phabricator via cfe-commits]

kpet created this revision.
kpet added reviewers: tra, ABataev, echristo, jlebar, hfinkel, rsmith.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

The action collapsing logic introduced in
https://reviews.llvm.org/D21840 generally works by replacing
action A followed by B by an action that uses the tool for A
with action B.

The logic in combineAssembleBackend however does something
different and selects a tool for an action that is not part
of the list of actions under consideration for collapsing,
specifically it selects the tool for the CompileJobAction on
which the BackendJobAction depends.

There doesn't seem to be any good reason that I could find for
doing this. My understanding is that all cases of interest rely
on the Clang tool being selected for the CompileJobAction.
However the same Clang tool would be selected for a
BackendJobAction and doing so would make the collapsing logic
more symmetric and make it easier to further refactor into
something more generic.

The reason I'm looking at this is that I am currently working
on a toolchain that uses a single tool with combined
BackendJobAction and AssembleJobAction that is not Clang but
uses Clang as the tool for CompileJobAction.

Am I missing something? Are we missing a test?

Signed-off-by: Kevin Petit 


Repository:
  rC Clang

https://reviews.llvm.org/D62722

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3733,18 +3733,8 @@
 if (!AJ || !BJ)
   return nullptr;
 
-// Retrieve the compile job, backend action must always be preceded by one.
-ActionList CompileJobOffloadActions;
-auto *CJ = getPrevDependentAction(BJ->getInputs(), 
CompileJobOffloadActions,
-  /*CanBeCollapsed=*/false);
-if (!AJ || !BJ || !CJ)
-  return nullptr;
-
-assert(isa(CJ) &&
-   "Expecting compile job preceding backend job.");
-
-// Get compiler tool.
-const Tool *T = TC.SelectTool(*CJ);
+// Get backend tool.
+const Tool *T = TC.SelectTool(*BJ);
 if (!T)
   return nullptr;
 


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3733,18 +3733,8 @@
 if (!AJ || !BJ)
   return nullptr;
 
-// Retrieve the compile job, backend action must always be preceded by one.
-ActionList CompileJobOffloadActions;
-auto *CJ = getPrevDependentAction(BJ->getInputs(), CompileJobOffloadActions,
-  /*CanBeCollapsed=*/false);
-if (!AJ || !BJ || !CJ)
-  return nullptr;
-
-assert(isa(CJ) &&
-   "Expecting compile job preceding backend job.");
-
-// Get compiler tool.
-const Tool *T = TC.SelectTool(*CJ);
+// Get backend tool.
+const Tool *T = TC.SelectTool(*BJ);
 if (!T)
   return nullptr;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62724: [Analyzer] Iterator Checkers - Model `size()` method of containers

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, gamesh411, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, xazax.hun, whisperity.

Modeling of the `size()` method is essential for more accurate reporting 
past-the-end iterator access.


Repository:
  rC Clang

https://reviews.llvm.org/D62724

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -236,3 +236,19 @@
 *i0; // no-warning
   }
 }
+
+void size0(const std::vector& V) {
+  if (V.size() != 0)
+return;
+
+  *V.begin(); // expected-warning{{Past-the-end iterator dereferenced}}
+}
+
+void size1(const std::vector& V) {
+  if(V.size() != 1)
+return;
+  *V.begin(); // no-warning
+  auto i = V.cbegin();
+  ++i;
+  *i; // expected-warning{{Past-the-end iterator dereferenced}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -182,6 +182,8 @@
   void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
 const SVal &LVal, const SVal &RVal,
 OverloadedOperatorKind Op) const;
+  void handleSize(CheckerContext &C, const Expr *CE, const SVal &Cont,
+  const SVal &RetVal) const;
   void processComparison(CheckerContext &C, ProgramStateRef State,
  SymbolRef Sym1, SymbolRef Sym2, const SVal &RetVal,
  OverloadedOperatorKind Op) const;
@@ -272,6 +274,7 @@
 
 bool isIteratorType(const QualType &Type);
 bool isIterator(const CXXRecordDecl *CRD);
+bool isContainerType(const QualType &Type);
 bool isComparisonOperator(OverloadedOperatorKind OK);
 bool isBeginCall(const FunctionDecl *Func);
 bool isEndCall(const FunctionDecl *Func);
@@ -287,6 +290,7 @@
 bool isEraseCall(const FunctionDecl *Func);
 bool isEraseAfterCall(const FunctionDecl *Func);
 bool isEmplaceCall(const FunctionDecl *Func);
+bool isSizeCall(const FunctionDecl *Func);
 bool isAssignmentOperator(OverloadedOperatorKind OK);
 bool isSimpleComparisonOperator(OverloadedOperatorKind OK);
 bool isAccessOperator(OverloadedOperatorKind OK);
@@ -314,6 +318,8 @@
 ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val);
 ProgramStateRef assumeNoOverflow(ProgramStateRef State, SymbolRef Sym,
  long Scale);
+const llvm::APSInt* calculateSize(ProgramStateRef State, SymbolRef Begin,
+  SymbolRef End);
 ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
 ProgramStateRef
@@ -691,6 +697,16 @@
 if (!OrigExpr)
   return;
 
+if (const auto *InstCall = dyn_cast(&Call)) {
+  if (isContainerType(InstCall->getCXXThisExpr()->getType())) {
+if (isSizeCall(Func)) {
+  handleSize(C, OrigExpr, InstCall->getCXXThisVal(),
+ Call.getReturnValue());
+  return;
+}
+  }
+}
+
 if (!isIteratorType(Call.getResultType()))
   return;
 
@@ -918,6 +934,68 @@
   }
 }
 
+void IteratorChecker::handleSize(CheckerContext &C, const Expr *CE,
+ const SVal &Cont, const SVal &RetVal) const {
+  const auto *ContReg = Cont.getAsRegion();
+  if (!ContReg)
+return;
+
+  ContReg = ContReg->getMostDerivedObjectRegion();
+ 
+  auto State = C.getState();
+
+  State = createContainerBegin(State, ContReg, CE, C.getASTContext().LongTy,
+   C.getLocationContext(), C.blockCount());
+  auto BeginSym = getContainerBegin(State, ContReg);
+  State = createContainerEnd(State, ContReg, CE, C.getASTContext().LongTy,
+ C.getLocationContext(), C.blockCount());
+  auto EndSym = getContainerEnd(State, ContReg);
+  const llvm::APSInt *CalcSize = calculateSize(State, BeginSym, EndSym);
+
+  auto &SVB = C.getSValBuilder();
+  auto &BVF = State->getBasicVals();
+  const llvm::APSInt *RetSize = nullptr;
+  if (const auto *KnownSize = SVB.getKnownValue(State, RetVal)) {
+RetSize = &BVF.Convert(C.getASTContext().LongTy, *KnownSize);
+  }
+
+  if (RetSize) {
+// If the return value is a concrete integer, then try to adjust the size
+// of the container (the difference between its `begin()` and `end()` to
+// this size. Function `relateSymbols()` returns null if it contradits
+// the current size.
+const auto CalcEnd =
+  SVB.evalBinOp(State, BO_Add, nonloc::SymbolVal(EndSym),
+nonloc::ConcreteInt(*RetSize), C.getASTConte

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31
+  Result,
+  "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC",
+  ReplacementText);

the message doesn't seem to explain the reason, especially to the people who 
are not familiar with the `pipe` API, maybe `prefer pipe2() to pipe() to avoid 
file descriptor leakage` is clearer?

Ah, it looks like you are following the existing `CloexecCreatCheck` check, no 
need to do it in this patch. 



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:17
+  pipe(pipefd);
+  // CHECK-MESSAGES-NOT: warning:
+}

nit: no need to do it explicitly, if a warning is shown unexpectedly, the test 
will fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:60
+  TEMP_FAILURE_RETRY(pipe2(pipefd, O_NONBLOCK | O_CLOEXEC));
+  // CHECK-MESSAGES-NOT: warning:
+}

no need to verify it explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62580: [OpenCL] Use long instead of long long in x86 builtins

[Alexander Batashev via Phabricator via cfe-commits]

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

@Ka-Ka good point. Thank you.
@Anastasia Would such tests be ok with you?
@erichkeane Thank you very much. I think I don't have permissions to commit 
changes and will need someone's help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62580



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

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

This change broke compiling Qt on MinGW, see 
https://bugs.llvm.org/show_bug.cgi?id=42089. Trivially reproducible by trying 
to compile a snippet that looks like this:

  class Foo {
  public:
  __attribute__((nothrow)) void __attribute__((__stdcall__)) Bar();
  };
  
  void Foo::Bar() {
  }

Reproducible by compiling the same snippet for a windows-msvc target as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62435



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

[PATCH] D62729: [ARM] Fix recent breakage of -mfpu=none.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: SjoerdMeijer, dmgreen.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls, 
javed.absar.
Herald added projects: clang, LLVM.

The recent change D60691  introduced a bug in 
clang when handling
option combinations such as `-mcpu=cortex-m4 -mfpu=none`. Those
options together should select Cortex-M4 but disable all use of
hardware FP, but in fact, now hardware FP instructions can still be
generated in that mode.

The reason is because the handling of FPUVersion::NONE disables all
the same feature names it used to, of which the base one is `vfp2`.
But now there are further features below that, like `vfp2d16fp` and
(following D60694 ) `fpregs`, which also need 
to be turned off to
disable hardware FP completely.

Added a tiny test which double-checks that compiling a simple FP
function doesn't access the FP registers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62729

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/CodeGen/arm-mfpu-none.c
  llvm/lib/Support/ARMTargetParser.cpp


Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -198,6 +198,7 @@
 Features.push_back("-fp-armv8");
 break;
   case FPUVersion::NONE:
+Features.push_back("-fpregs");
 Features.push_back("-vfp2");
 Features.push_back("-vfp3");
 Features.push_back("-fp16");
Index: clang/test/CodeGen/arm-mfpu-none.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-mfpu-none.c
@@ -0,0 +1,8 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | 
FileCheck %s
+
+// CHECK-LABEL: compute
+// CHECK-NOT: {{s[0-9]}}
+float compute(float a, float b) {
+  return (a+b) * (a-b);
+}
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -433,8 +433,8 @@
 llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
 
 // Disable hardware FP features which have been enabled.
-// FIXME: Disabling vfp2 and neon should be enough as all the other
-//features are dependent on these 2 features in LLVM. However
+// FIXME: Disabling fpregs should be enough all by itself, since all
+//the other FP features are dependent on it. However
 //there is currently no easy way to test this in clang, so for
 //now just be explicit and disable all known dependent features
 //as well.
@@ -442,6 +442,11 @@
 "neon", "crypto", "dotprod", "fp16fml"})
   if (std::find(std::begin(Features), std::end(Features), "+" + Feature) 
!= std::end(Features))
 Features.push_back(Args.MakeArgString("-" + Feature));
+
+// Disable the base feature unconditionally, even if it was not
+// explicitly in the features list (e.g. if we had +vfp3, which
+// implies it).
+Features.push_back("-fpregs");
   }
 
   // En/disable crc code generation.


Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -198,6 +198,7 @@
 Features.push_back("-fp-armv8");
 break;
   case FPUVersion::NONE:
+Features.push_back("-fpregs");
 Features.push_back("-vfp2");
 Features.push_back("-vfp3");
 Features.push_back("-fp16");
Index: clang/test/CodeGen/arm-mfpu-none.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-mfpu-none.c
@@ -0,0 +1,8 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | FileCheck %s
+
+// CHECK-LABEL: compute
+// CHECK-NOT: {{s[0-9]}}
+float compute(float a, float b) {
+  return (a+b) * (a-b);
+}
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -433,8 +433,8 @@
 llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
 
 // Disable hardware FP features which have been enabled.
-// FIXME: Disabling vfp2 and neon should be enough as all the other
-//features are dependent on these 2 features in LLVM. However
+// FIXME: Disabling fpregs should be enough all by itself, since all
+//the other FP features are dependent on it. However
 //there is currently no easy way to test this in clang, so for
 //now just be explicit and disable all known dependent featur

[PATCH] D62729: [ARM] Fix recent breakage of -mfpu=none.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/test/CodeGen/arm-mfpu-none.c:2
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | 
FileCheck %s
+

Generally clang codegen tests should test `-emit-llvm -S` output


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62729



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

[PATCH] D62729: [ARM] Fix recent breakage of -mfpu=none.

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: clang/test/CodeGen/arm-mfpu-none.c:2
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | 
FileCheck %s
+

lebedev.ri wrote:
> Generally clang codegen tests should test `-emit-llvm -S` output
Perhaps add to the tests in clang/test/Driver/arm-mfpu.c instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62729



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

[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 202414.
balazske added a comment.

- Using size_t instead of int.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60463

Files:
  unittests/AST/ASTImporterFixtures.cpp
  unittests/AST/ASTImporterFixtures.h
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -49,9 +49,8 @@
   llvm::raw_svector_ostream ToNothing(ImportChecker);
   ToCtx.getTranslationUnitDecl()->print(ToNothing);
 
-  // This traverses the AST to catch certain bugs like poorly or not
-  // implemented subtrees.
-  (*Imported)->dump(ToNothing);
+  // Additional SourceLocation checks.
+  checkImportedSourceLocations(Node, *Imported);
 }
 
 return Imported;
Index: unittests/AST/ASTImporterFixtures.h
===
--- unittests/AST/ASTImporterFixtures.h
+++ unittests/AST/ASTImporterFixtures.h
@@ -42,6 +42,8 @@
 void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
StringRef Code);
 
+void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD);
+
 // Common base for the different families of ASTImporter tests that are
 // parameterized on the compiler options which may result a different AST. E.g.
 // -fms-compatibility or -fdelayed-template-parsing.
Index: unittests/AST/ASTImporterFixtures.cpp
===
--- unittests/AST/ASTImporterFixtures.cpp
+++ unittests/AST/ASTImporterFixtures.cpp
@@ -18,6 +18,8 @@
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/Tooling.h"
 
+#include 
+
 namespace clang {
 namespace ast_matchers {
 
@@ -38,6 +40,82 @@
llvm::MemoryBuffer::getMemBuffer(Code));
 }
 
+void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD) {
+  // Check AST dump for matching source locations in From and To AST.
+  // The first line of the AST dump contains information about the root node and
+  // some of the SourceLocation info, but certainly not everything.
+  // Still it is worth to check these. The test can be extended to check the
+  // whole AST dump but for this to work the ordering of imported Decls must be
+  // the same which is not the case now.
+  // The AST dump additionally traverses the AST and can catch certain bugs like
+  // poorly or not implemented subtrees.
+
+  // Print debug information?
+  const bool Print = false;
+
+  SmallString<1024> ToPrinted;
+  SmallString<1024> FromPrinted;
+  llvm::raw_svector_ostream ToStream(ToPrinted);
+  llvm::raw_svector_ostream FromStream(FromPrinted);
+
+  ToD->dump(ToStream);
+  FromD->dump(FromStream);
+
+  // search for SourceLocation strings:
+  // ::
+  // or
+  // line::
+  // or
+  // col:
+  // or
+  // ''
+  // If a component (filename or line) is same as in the last location
+  // it is not printed.
+  // Filename component is grouped into sub-expression to make it extractable.
+  std::regex MatchSourceLoc(
+  "|((\\w|\\.)+):\\d+:\\d+|line:\\d+:\\d+|col:\\d+");
+
+  std::string ToString(ToStream.str());
+  std::string FromString(FromStream.str());
+  size_t ToEndl = ToString.find('\n');
+  if (ToEndl == std::string::npos)
+ToEndl = ToString.size();
+  size_t FromEndl = FromString.find('\n');
+  if (FromEndl == std::string::npos)
+FromEndl = FromString.size();
+  auto ToLoc = std::sregex_iterator(ToString.begin(), ToString.begin() + ToEndl,
+MatchSourceLoc);
+  auto FromLoc = std::sregex_iterator(
+  FromString.begin(), FromString.begin() + FromEndl, MatchSourceLoc);
+  if (Print) {
+llvm::errs() << ToString << "\n\n\n" << FromString << "\n";
+llvm::errs() << "\n";
+  }
+  if (ToLoc->size() > 1 && FromLoc->size() > 1 && (*ToLoc)[1] != (*FromLoc)[1])
+// Different filenames in To and From.
+// This should mean that a to-be-imported decl was mapped to an existing
+// (these normally reside in different files) and the check is
+// not applicable.
+return;
+
+  bool SourceLocationMismatch = false;
+  while (ToLoc != std::sregex_iterator() && FromLoc != std::sregex_iterator()) {
+if (Print)
+  llvm::errs() << ToLoc->str() << "|" << FromLoc->str() << "\n";
+SourceLocationMismatch =
+SourceLocationMismatch || (ToLoc->str() != FromLoc->str());
+++ToLoc;
+++FromLoc;
+  }
+  if (Print)
+llvm::errs() << "\n";
+
+  if (FromLoc != std::sregex_iterator() || ToLoc != std::sregex_iterator())
+SourceLocationMismatch = true;
+
+  EXPECT_FALSE(SourceLocationMismatch);
+}
+
 ASTImporterTestBase::TU::TU(StringRef Code, StringRef FileName, ArgVector Args,
 ImporterConstructor C)
 : Code(Code), FileName(FileNa

[PATCH] D62729: [ARM] Fix recent breakage of -mfpu=none.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/arm-mfpu-none.c:2
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | 
FileCheck %s
+

dmgreen wrote:
> lebedev.ri wrote:
> > Generally clang codegen tests should test `-emit-llvm -S` output
> Perhaps add to the tests in clang/test/Driver/arm-mfpu.c instead?
@lebedev.ri : looking at the IR doesn't show up what I'm trying to test in this 
case. And I modelled this on existing tests in test/CodeGen, such as 
`arm-eabi.c`, which also need to test the real assembler output in order to do 
their job.

@dmgreen: it's true that I could also make `test/Driver/arm-mfpu.c` check that 
the extra feature name is appearing in the cc1 command line where I currently 
think it should be, but I think that wouldn't replace this test. The real point 
of this test is that the //currently// right feature names might not stay 
right, if the feature set is reorganised again in future – and if that happens, 
then whoever does it will probably rewrite most of `arm-mfpu.c` anyway.

So this is an end-to-end check that //whatever// detailed command line is 
generated by the driver in these circumstances, it is sufficient to ensure that 
the final generated code really doesn't include FP instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62729



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1017
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Anastasia wrote:
> Undocumented -> SYCLKernelDocs
Oh, Thank you for that!



Comment at: clang/lib/Parse/ParseAST.cpp:171
 
+  if (S.getLangOpts().SYCLIsDevice) {
+for (Decl *D : S.SyclDeviceFuncs()) {

Anastasia wrote:
> Do you also need to prevent generation of non-device functions somehow?
I think It's already prevented by change to CodeGenModule.cpp in this patch. 
CodeGen just ignores declarations without SYCL device attribute now.



Comment at: clang/lib/Sema/SemaSYCL.cpp:23
+
+  bool VisitCallExpr(CallExpr *e) {
+if (FunctionDecl *Callee = e->getDirectCallee()) {

Anastasia wrote:
> Anastasia wrote:
> > bader wrote:
> > > Anastasia wrote:
> > > > This is probably not something we can change at this point but I wish 
> > > > we could avoid complexities like this. :(
> > > > 
> > > > I think this is also preventing traditional linking of translation 
> > > > units. That is somewhat unfortunate.
> > > > 
> > > > It is good direction however to keep this logic in a separate dedicated 
> > > > compilation unit.
> > > > 
> > > > I would suggest to document it a bit more including any current 
> > > > limitations/assumption that you can mark under FIXME i.e. does your 
> > > > code handle lambdas yet, what if lambdas are used in function 
> > > > parameters, etc...
> > > > I think this is also preventing traditional linking of translation 
> > > > units.
> > > 
> > > Could you elaborate more on this topic, please?
> > > What do you mean by "traditional linking of translation units" and what 
> > > exactly "is preventing" it?
> > > Do you compare with the linking of regular C++ code (i.e. which do not 
> > > split into host and device code)?
> > > If so, SYCL is different from this model and more similar to CUDA/OpenMP 
> > > models, which also skip "linking" of irrelevant part (e.g. host code is 
> > > not linked by the device compiler).
> > > Mariya added Justin (@jlebar) and Alexey (@ABataev), who work on 
> > > single-source programming models to make them aware and provide feedback 
> > > if any.
> > Yes indeed, I mean linking of modules in C/C++ even though it doesn't 
> > necessarily mean linking of object files. So you don't plan to support 
> > `SYCL_EXTERNAL` in clang?
> > 
> > In CUDA the functions executed on device are annotated manually using 
> > `__device__` hence separate translation units can specify external device 
> > function... although I don't know if CUDA implementation in clang support 
> > this.
> > 
> > I guess OpenMP is allowed to fall back to run on host?
> Ping!
> 
> 
> 
> > I would suggest to document it a bit more including any current 
> > limitations/assumption that you can mark under FIXME i.e. does your code 
> > handle lambdas yet, what if lambdas are used in function parameters, etc...
> 
> 
Oh, sorry, I missed this comment when I updated patch last time.
Could you please advise in which form I can document it?



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5520
 DefinitionRequired, true);
-  if (CurFD->isDefined())
+  if (CurFD->isDefined()) {
+// Because all SYCL kernel functions are template functions - 
they

Anastasia wrote:
> May be this should go into a helper function as it seems to be now a bigger 
> chunk of code that is repeated?
> 
> Although, I am not very familiar with this code. You can try to get someone 
> to review who has contributed to this more recently.
I think this chunk of code seems big because of big repeated comment.



Comment at: clang/test/CodeGenSYCL/device-functions.cpp:24
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar

Anastasia wrote:
> I can't see where the SPIR calling convention is currently set for SYCL?
If I understand correct it's set automatically on AST level because we use 
SPIR-based triple for device code. Only in case of C++ methods clang doesn't 
set SPIR calling convention. We did a modification in our codebase to get SPIR 
calling convention for C++ methods too (available [[ 
https://github.com/intel/llvm/blob/c13cb8df84418cb5d682f3bbee89090ebb0d00be/clang/lib/AST/ASTContext.cpp#L10015
 | here ]] ) 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D62399: [clang] Add storage for APValue in ConstantExpr

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.
Herald added a subscriber: rnkovacs.

ping @rsmith


Repository:
  rC Clang

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

https://reviews.llvm.org/D62399



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

[PATCH] D62729: [ARM] Fix recent breakage of -mfpu=none.

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: clang/test/CodeGen/arm-mfpu-none.c:2
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | 
FileCheck %s
+

simon_tatham wrote:
> dmgreen wrote:
> > lebedev.ri wrote:
> > > Generally clang codegen tests should test `-emit-llvm -S` output
> > Perhaps add to the tests in clang/test/Driver/arm-mfpu.c instead?
> @lebedev.ri : looking at the IR doesn't show up what I'm trying to test in 
> this case. And I modelled this on existing tests in test/CodeGen, such as 
> `arm-eabi.c`, which also need to test the real assembler output in order to 
> do their job.
> 
> @dmgreen: it's true that I could also make `test/Driver/arm-mfpu.c` check 
> that the extra feature name is appearing in the cc1 command line where I 
> currently think it should be, but I think that wouldn't replace this test. 
> The real point of this test is that the //currently// right feature names 
> might not stay right, if the feature set is reorganised again in future – and 
> if that happens, then whoever does it will probably rewrite most of 
> `arm-mfpu.c` anyway.
> 
> So this is an end-to-end check that //whatever// detailed command line is 
> generated by the driver in these circumstances, it is sufficient to ensure 
> that the final generated code really doesn't include FP instructions.
I believe that the idea is that if we test each step, so make sure we have 
tests from clang driver -> clang-cl and clang -> llvm-ir and llvm -> asm, then 
it should all be tested. And because each has a well defined interfaces along 
each step of the way it should keep working. Any integration tests are often 
left to external tests like the test suite.

But I do see some precedent for this in other places. If you add the extra 
checks in test/Driver/arm-mfpu.c, then I think this is fine. (And I guess if 
someone objects we can always take it out :) )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62729



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

[PATCH] D60763: Prototype OpenCL BIFs using Tablegen

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Looks good overall, I'd like to propose a few more minor changes to this patch 
before landing:

- isPointer can be a `bit` / `bool` instead of an `int` / `unsigned`.
- Renaming the command line option from `-fadd-declare-builtins` to 
`-fdeclare-opencl-builtins` should hopefully convey the meaning of the option a 
bit better.
- Drop the spurious `;` in `OCL2Qual`.
- Emit spaces instead of tabs for `OCL2Qual`, like the rest of the generated 
file.
- Bring the format of OpenCL spec references into alignment with the rest of 
clang (e.g. OpenCL v2.0 s9.17.3).
- Adding //(experimental)// to the help text of `-fdeclare-opencl-builtins`.


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

https://reviews.llvm.org/D60763



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

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D62435#1524824 , @mstorsjo wrote:

> This change broke compiling Qt on MinGW, see 
> https://bugs.llvm.org/show_bug.cgi?id=42089. Trivially reproducible by trying 
> to compile a snippet that looks like this:
>
>   class Foo {
>   public:
>   __attribute__((nothrow)) void __attribute__((__stdcall__)) Bar();
>   };
>  
>   void Foo::Bar() {
>   }
>
>
> Reproducible by compiling the same snippet for a windows-msvc target as well.


Yikes, thanks for the report!  I'll look at it this morning.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62435



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

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

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

This still needs tests adding.


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

https://reviews.llvm.org/D60697



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

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Keane, Erich via cfe-commits]

Presumably the right choice for you on this one is to separate the diagnostic 
into a new group.  Copying Aaron since he might have an idea of an existing 
group, otherwise I’ll push a commit to put it in a new one.

Thanks for the report!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Thursday, May 30, 2019 6:15 PM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Hello,

this causes this diagnostic when building on Windows:

In file included from 
../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): 
error: 'nothrow' attribute conflicts with exception specification; attribute 
ignored [-Werror,-Wignored-attributes]
  END_COM_MAP()
  ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
 note: expanded from macro 'END_COM_MAP'
STDMETHOD(QueryInterface)( \
^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
 note: expanded from macro 'STDMETHOD'
#define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT 
STDMETHODCALLTYPE method
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
 note: expanded from macro 'COM_DECLSPEC_NOTHROW'
#define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
 note: expanded from macro 'DECLSPEC_NOTHROW'
#define DECLSPEC_NOTHROW   __declspec(nothrow)
  ^


Suggestions? We don't want to use -Wno-ignored-attributes since that disables 
this warning for all attributes.

All the involved macros are in system headers and we have no control over them.

On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu May 30 10:31:54 2019
New Revision: 362119

URL: http://llvm.org/viewvc/llvm-project?rev=362119&view=rev
Log:
Add Attribute NoThrow as an Exception Specifier Type

In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
clear that the current mechanism of hacking through checks for the
exception specification of a function gets confused really quickly when
there are alternate exception specifiers.

This patch introcues EST_NoThrow, which is the equivilent of
EST_noexcept when caused by EST_noThrow. The existing implementation is
left in place to cover functions with no FunctionProtoType.

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

Added:
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119&r1=362118&r2=362119&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 30 10:31:54 2019
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 57
+#define CINDEX_VERSION_MINOR 58

 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -221,7 +221,12 @@ enum CXCursor_ExceptionSpecificationKind
   /**
* The exception specification has not been parsed yet.
*/
-  CXCursor_ExceptionSpecificationKind_Unparsed
+  CXCursor_ExceptionSpecificationKind_Unparsed,
+
+  /**
+   * The cursor has a __declspec(nothrow) exception specification.
+   */
+  CXCursor_ExceptionSpecificationKind_NoThrow
 };

 /**

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=362119&r1=362118&r2=362119&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu May 30 10:31:54 2019
@@ -2330,6 +2330,14 @@ public:
 return T->castAs()-

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Ah yes, the school boy error! ;-) Actually, there was a test, but in a 
different patch; I will move it to here.


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

https://reviews.llvm.org/D60697



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

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: andrew.w.kaylor, kpn.
Herald added a subscriber: mgorny.
Herald added a project: LLVM.

This is part of RFC to support language options -fp-model and -fp-speculation.

This makes a small modification to D53157  to 
pass some arguments using an enumeration type instead of MDNode.

In addition I created the FPState.h file which declares 2 enumeration types 
corresponding to the option settings.  I want to put them into llvm so it will 
be available not only to clang but to other language front ends that might want 
to offer floating point control using option settings.  The language options 
are combined using certain semantics into IRBuilder settings.  There's a member 
function in FPState.h that does this.

There's a clang patch that goes along with this patch.

Here's the RFC pasted from email to llvm-dev and clang-dev: 
Intel would like to contribute a patch to implement support for these Intel- 
and Microsoft -fp options:
-fp-model=[precise|strict|fast|except[-]] and -fp-speculation=[fast|strict|safe]

This contribution would dovetail with the llvm patch "Teach the IRBuilder about 
constrained fadd and friends" which is under review here, 
https://reviews.llvm.org/D53157/new/.  I have a patch ready to review that 
works with D53157 .  The motivation for 
providing these is that having a single option to control most basic FP options 
is better and easier to understand for users.

The option settings -fp-model=[precise|strict|fast|except] are supported by 
both ICC and CL. The fp-speculation option is supported only by ICC.  The CL 
and ICC -fp-model option is documented on these pages:

  
https://docs.microsoft.com/en-us/cpp/build/reference/fp-specify-floating-point-behavior?view=vs-2019
  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-model-fp

Currently, clang's default behavior corresponds to -fp-model=precise.  
Clang/llvm support for -fp-model=[strict|except] is being developed in the 
D53157  patch, and there is current llvm 
support for the fast settings by using the fast math flags llvm::FastMathFlags. 
 Note: the clang-cl wrapper to support Microsoft options has simplified support 
for these options by mapping /fp-model=except to ftrapping-math, fp-mdel=fast 
to ffast-math, fp-model=precise and fp-model=strict to fno-fast-math (see 
clang/Driver/CLCompatOptions.td).

According to the online options documentation, you can combine except[-] with 
[precise|strict|fast], but combining both fast and except is not a valid 
setting (the Driver will emit an error).

  precise - Disables optimizations that are not value-safe on floating-point 
data, although FP contraction is enabled.
  strict - Enables precise and except, disables contractions (FMA), and enables 
pragma stdc fenv_access.   
  fast - Equivalent to -ffast-math
  except/except- - Determines whether strict floating-point exception semantics 
are honored.

The ICC option -fp-speculation is described here,

  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-speculation-qfp-speculation

These are the meanings of the fp-speculation settings:

  fast - Tells the compiler to speculate on floating-point operations.  This is 
equivalent to “fpexcept.ignore” in the constrained intrinsics review D53157.
  strict - Tells the compiler to disable speculation on floating-point 
operations.  This is equivalent to “fpexcept.strict” in the constrained 
intrinsics review D53157.
  safe - Tells the compiler to disable speculation if there is a possibility 
that the speculation may cause a floating-point exception.  This is equivalent 
to “fpexcept.maytrap” in the constrained intrinsics review D53157.


Repository:
  rL LLVM

https://reviews.llvm.org/D62730

Files:
  include/llvm/IR/FPState.h
  include/llvm/IR/IRBuilder.h
  lib/IR/CMakeLists.txt
  lib/IR/FPState.cpp
  unittests/IR/IRBuilderTest.cpp

Index: unittests/IR/IRBuilderTest.cpp
===
--- unittests/IR/IRBuilderTest.cpp
+++ unittests/IR/IRBuilderTest.cpp
@@ -10,6 +10,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/FPState.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
@@ -122,6 +123,159 @@
   EXPECT_FALSE(II->hasNoNaNs());
 }
 
+TEST_F(IRBuilderTest, ConstrainedFP) {
+  IRBuilder<> Builder(BB);
+  Value *V;
+  CallInst *Call;
+  IntrinsicInst *II;
+
+  V = Builder.CreateLoad(GV);
+
+  Call = Builder.CreateConstrainedFAdd(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  Call = Builder.CreateConstrainedFSub(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  Call = Builder.CreateCon

Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Nico Weber via cfe-commits]

I'm not sure this is the best fix. From the patch description, it sounds
like this is a Microsoft-specific change. So splitting this into a new
diag group that then everyone using the Microsoft headers has to disable
seems a bit roundabout – don't we get the same behavior by not emitting
this warning in the first place, except that we don't require everyone
using clang-cl to explicitly disable this warning then?

On Fri, May 31, 2019 at 9:38 AM Keane, Erich  wrote:

> Presumably the right choice for you on this one is to separate the
> diagnostic into a new group.  Copying Aaron since he might have an idea of
> an existing group, otherwise I’ll push a commit to put it in a new one.
>
>
>
> Thanks for the report!
>
> -Erich
>
>
>
> *From:* Nico Weber [mailto:tha...@chromium.org]
> *Sent:* Thursday, May 30, 2019 6:15 PM
> *To:* Keane, Erich 
> *Cc:* cfe-commits 
> *Subject:* Re: r362119 - Add Attribute NoThrow as an Exception Specifier
> Type
>
>
>
> Hello,
>
>
>
> this causes this diagnostic when building on Windows:
>
>
>
> In file included from
> ../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
> ../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3):
> error: 'nothrow' attribute conflicts with exception specification;
> attribute ignored [-Werror,-Wignored-attributes]
>   END_COM_MAP()
>   ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
> note: expanded from macro 'END_COM_MAP'
> STDMETHOD(QueryInterface)( \
> ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
> note: expanded from macro 'STDMETHOD'
> #define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT
> STDMETHODCALLTYPE method
>  ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
> note: expanded from macro 'COM_DECLSPEC_NOTHROW'
> #define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
>  ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
> note: expanded from macro 'DECLSPEC_NOTHROW'
> #define DECLSPEC_NOTHROW   __declspec(nothrow)
>   ^
>
>
>
>
>
> Suggestions? We don't want to use -Wno-ignored-attributes since that
> disables this warning for all attributes.
>
>
>
> All the involved macros are in system headers and we have no control over
> them.
>
>
>
> On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Thu May 30 10:31:54 2019
> New Revision: 362119
>
> URL: http://llvm.org/viewvc/llvm-project?rev=362119&view=rev
> Log:
> Add Attribute NoThrow as an Exception Specifier Type
>
> In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
> clear that the current mechanism of hacking through checks for the
> exception specification of a function gets confused really quickly when
> there are alternate exception specifiers.
>
> This patch introcues EST_NoThrow, which is the equivilent of
> EST_noexcept when caused by EST_noThrow. The existing implementation is
> left in place to cover functions with no FunctionProtoType.
>
> Differential Revision: https://reviews.llvm.org/D62435
>
> Added:
> cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
> Modified:
> cfe/trunk/include/clang-c/Index.h
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/include/clang/AST/Type.h
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/JSONNodeDumper.cpp
> cfe/trunk/lib/AST/Type.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaType.cpp
> cfe/trunk/tools/libclang/CXType.cpp
>
> Modified: cfe/trunk/include/clang-c/Index.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119&r1=362118&r2=362119&view=diff
>
> ==
> --- cfe/trunk/include/clang-c/Index.h (original)
> +++ cfe/trunk/include/clang-c/Index.h Thu May 30 10:31:54 2019
> @@ -32,7 +32,7 @@
>   * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
>   */
>  #define CINDEX_VERSION_MAJOR 0
> -#define CINDEX_VERSION_MINOR 57
> +#define CINDEX_VERSION_MINOR 58
>
>  #define CINDEX_VERSION_ENCODE(major, minor) ( \
>((major) * 1)   \
> @@ -221,7 +221,12 @@ enum CXCursor_ExceptionSpecificationKind
>/**
> * The exce

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added a reviewer: chandlerc.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Intel would like to contribute a patch to implement support for these Intel- 
and Microsoft -fp options.  This message is to describe the options and request 
feedback from the community.
-fp-model=[precise|strict|fast|except[-]] and -fp-speculation=[fast|strict|safe]

This contribution would dovetail with the llvm patch "Teach the IRBuilder about 
constrained fadd and friends" which is under review here, 
https://reviews.llvm.org/D53157/new/.  I have a patch ready to review that 
works with D53157 .  The motivation for 
providing these is that having a single option to control most basic FP options 
is better and easier to understand for users.

The companion llvm patch is available for review here: 
https://reviews.llvm.org/D62730 ; I made a couple small changes to D53157 
.

The option settings -fp-model=[precise|strict|fast|except] are supported by 
both ICC and CL. The fp-speculation option is supported only by ICC.  The CL 
and ICC -fp-model option is documented on these pages:

  
https://docs.microsoft.com/en-us/cpp/build/reference/fp-specify-floating-point-behavior?view=vs-2019
  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-model-fp

Currently, clang's default behavior corresponds to -fp-model=precise.  
Clang/llvm support for -fp-model=[strict|except] is being developed in the 
D53157  patch, and there is current llvm 
support for the fast settings by using the fast math flags llvm::FastMathFlags. 
 Note: the clang-cl wrapper to support Microsoft options has simplified support 
for these options by mapping /fp-model=except to ftrapping-math, fp-mdel=fast 
to ffast-math, fp-model=precise and fp-model=strict to fno-fast-math (see 
clang/Driver/CLCompatOptions.td).

According to the online options documentation, you can combine except[-] with 
[precise|strict|fast], but combining both fast and except is not a valid 
setting (the Driver will emit an error).

  precise - Disables optimizations that are not value-safe on floating-point 
data, although FP contraction is enabled.
  strict - Enables precise and except, disables contractions (FMA), and enables 
pragma stdc fenv_access.   
  fast - Equivalent to -ffast-math
  except/except- - Determines whether strict floating-point exception semantics 
are honored.

The ICC option -fp-speculation is described here,

  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-speculation-qfp-speculation

These are the meanings of the fp-speculation settings:

  fast - Tells the compiler to speculate on floating-point operations.  This is 
equivalent to “fpexcept.ignore” in the constrained intrinsics review D53157.
  strict - Tells the compiler to disable speculation on floating-point 
operations.  This is equivalent to “fpexcept.strict” in the constrained 
intrinsics review D53157.
  safe - Tells the compiler to disable speculation if there is a possibility 
that the speculation may cause a floating-point exception.  This is equivalent 
to “fpexcept.maytrap” in the constrained intrinsics review D53157.


Repository:
  rL LLVM

https://reviews.llvm.org/D62731

Files:
  include/clang/Basic/LangOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/fpconstrained.c
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -217,6 +217,11 @@
 // RUN: %clang -### -S -fexec-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
 // CHECK-INVALID-INPUT-CHARSET: error: invalid value 'iso-8859-1' in '-fexec-charset=iso-8859-1'
 
+// RUN: %clang -### -S -fp-model=fast -fp-model=except -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FAST-EXCEPT %s
+// RUN: %clang -### -S -fp-model=fast -fp-model=except -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FAST-EXCEPT %s
+// CHECK-INVALID-FAST-EXCEPT: error: invalid argument 'fp-model=fast' not allowed with 'fp-model=except'
+//
+
 // Test that we don't error on these.
 // RUN: %clang -### -S -Werror\
 // RUN: -falign-functions -falign-functions=2 -fno-align-functions\
Index: test/CodeGen/fpconstrained.c
===
--- /dev/null
+++ test/CodeGen/fpconstrained.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fp-model=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICT
+// RUN: %clang_cc1 -fp-model=strict -fp-model-except -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICT

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Keane, Erich via cfe-commits]

It IS an attribute that works on Linux as well (about ½ way down this page: 
https://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Function-Attributes.html), though 
used rarely (at least in C++ code) as far as I understand.

The fact that a mistake like this is in the ATL headers is frustrating, but I’m 
not sure the best way to deal with this.  Aaron and I both believed the warning 
to be meaningful/useful (since this conflict is the same as saying throw() 
noexcept(false)), so I’m not sure removing it is a good idea (if that’s what 
you’re suggesting).

However, if this is something reasonably common on Microsoft implementations, I 
wonder if it is a warning we can make off-by-default on clang-cl.  It would be 
a shame to lose it, but if this is a commonly enough used problem we might be 
stuck with it.

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Friday, May 31, 2019 6:58 AM
To: Keane, Erich 
Cc: Aaron Ballman ; cfe-commits 

Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

I'm not sure this is the best fix. From the patch description, it sounds like 
this is a Microsoft-specific change. So splitting this into a new diag group 
that then everyone using the Microsoft headers has to disable seems a bit 
roundabout – don't we get the same behavior by not emitting this warning in the 
first place, except that we don't require everyone using clang-cl to explicitly 
disable this warning then?

On Fri, May 31, 2019 at 9:38 AM Keane, Erich 
mailto:erich.ke...@intel.com>> wrote:
Presumably the right choice for you on this one is to separate the diagnostic 
into a new group.  Copying Aaron since he might have an idea of an existing 
group, otherwise I’ll push a commit to put it in a new one.

Thanks for the report!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Thursday, May 30, 2019 6:15 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Hello,

this causes this diagnostic when building on Windows:

In file included from 
../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): 
error: 'nothrow' attribute conflicts with exception specification; attribute 
ignored [-Werror,-Wignored-attributes]
  END_COM_MAP()
  ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
 note: expanded from macro 'END_COM_MAP'
STDMETHOD(QueryInterface)( \
^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
 note: expanded from macro 'STDMETHOD'
#define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT 
STDMETHODCALLTYPE method
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
 note: expanded from macro 'COM_DECLSPEC_NOTHROW'
#define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
 note: expanded from macro 'DECLSPEC_NOTHROW'
#define DECLSPEC_NOTHROW   __declspec(nothrow)
  ^


Suggestions? We don't want to use -Wno-ignored-attributes since that disables 
this warning for all attributes.

All the involved macros are in system headers and we have no control over them.

On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu May 30 10:31:54 2019
New Revision: 362119

URL: http://llvm.org/viewvc/llvm-project?rev=362119&view=rev
Log:
Add Attribute NoThrow as an Exception Specifier Type

In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
clear that the current mechanism of hacking through checks for the
exception specification of a function gets confused really quickly when
there are alternate exception specifiers.

This patch introcues EST_NoThrow, which is the equivilent of
EST_noexcept when caused by EST_noThrow. The existing implementation is
left in place to cover functions with no FunctionProtoType.

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

Added:
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/lib/AST/Type.cpp
c

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Keane, Erich via cfe-commits]

To Update: Nico and I discussed this on IRC and believe the best way to fix 
this is to simply suppress this when it comes from a macro-expansion in a 
system header ala r221172 (or r220992).

I intend to work on that fix next after my current (fairly short) fix.

Thanks!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Friday, May 31, 2019 6:58 AM
To: Keane, Erich 
Cc: Aaron Ballman ; cfe-commits 

Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

I'm not sure this is the best fix. From the patch description, it sounds like 
this is a Microsoft-specific change. So splitting this into a new diag group 
that then everyone using the Microsoft headers has to disable seems a bit 
roundabout – don't we get the same behavior by not emitting this warning in the 
first place, except that we don't require everyone using clang-cl to explicitly 
disable this warning then?

On Fri, May 31, 2019 at 9:38 AM Keane, Erich 
mailto:erich.ke...@intel.com>> wrote:
Presumably the right choice for you on this one is to separate the diagnostic 
into a new group.  Copying Aaron since he might have an idea of an existing 
group, otherwise I’ll push a commit to put it in a new one.

Thanks for the report!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Thursday, May 30, 2019 6:15 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Hello,

this causes this diagnostic when building on Windows:

In file included from 
../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): 
error: 'nothrow' attribute conflicts with exception specification; attribute 
ignored [-Werror,-Wignored-attributes]
  END_COM_MAP()
  ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
 note: expanded from macro 'END_COM_MAP'
STDMETHOD(QueryInterface)( \
^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
 note: expanded from macro 'STDMETHOD'
#define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT 
STDMETHODCALLTYPE method
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
 note: expanded from macro 'COM_DECLSPEC_NOTHROW'
#define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
 note: expanded from macro 'DECLSPEC_NOTHROW'
#define DECLSPEC_NOTHROW   __declspec(nothrow)
  ^


Suggestions? We don't want to use -Wno-ignored-attributes since that disables 
this warning for all attributes.

All the involved macros are in system headers and we have no control over them.

On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu May 30 10:31:54 2019
New Revision: 362119

URL: http://llvm.org/viewvc/llvm-project?rev=362119&view=rev
Log:
Add Attribute NoThrow as an Exception Specifier Type

In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
clear that the current mechanism of hacking through checks for the
exception specification of a function gets confused really quickly when
there are alternate exception specifiers.

This patch introcues EST_NoThrow, which is the equivilent of
EST_noexcept when caused by EST_noThrow. The existing implementation is
left in place to cover functions with no FunctionProtoType.

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

Added:
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119&r1=362118&r2=362119&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 30 1

r362225 - Fix for PR42089, regression from r362119

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 07:26:19 2019
New Revision: 362225

URL: http://llvm.org/viewvc/llvm-project?rev=362225&view=rev
Log:
Fix for PR42089, regression from r362119

The implementation of the NoThrow ExceptionSpecificationType missed a
switch statement for forming the diagnostic when an out-of-line member
redeclaration misses the exception specification.  This patch adds the
correct case statement.

Modified:
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp

Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=362225&r1=362224&r2=362225&view=diff
==
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Fri May 31 07:26:19 2019
@@ -381,6 +381,11 @@ bool Sema::CheckEquivalentExceptionSpec(
 // when declaring a replaceable global allocation function.
 DiagID = diag::ext_missing_exception_specification;
 ReturnValueOnError = false;
+  } else if (ESI.Type == EST_NoThrow) {
+// Allow missing attribute 'nothrow' in redeclarations, since this is a 
very
+// common omission.
+DiagID = diag::ext_missing_exception_specification;
+ReturnValueOnError = false;
   } else {
 DiagID = diag::err_missing_exception_specification;
 ReturnValueOnError = true;
@@ -421,7 +426,9 @@ bool Sema::CheckEquivalentExceptionSpec(
 OldProto->getNoexceptExpr()->printPretty(OS, nullptr, getPrintingPolicy());
 OS << ")";
 break;
-
+  case EST_NoThrow:
+OS <<"__attribute__((nothrow))";
+break;
   default:
 llvm_unreachable("This spec type is compatible with none.");
   }

Modified: cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=362225&r1=362224&r2=362225&view=diff
==
--- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Fri May 31 07:26:19 2019
@@ -517,6 +517,15 @@ void PR34109(int* a) {
   delete a;
 }
 
+namespace PR42089 {
+  struct S {
+__attribute__((nothrow)) void Foo(); // expected-note {{previous 
declaration is here}}
+__attribute__((nothrow)) void Bar();
+  };
+  void S::Foo(){} // expected-warning {{is missing exception specification}}
+  __attribute__((nothrow)) void S::Bar(){}
+}
+
 #elif TEST2
 
 // Check that __unaligned is not recognized if MS extensions are not enabled


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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

I've posted a small change to this patch here, https://reviews.llvm.org/D62730 
and there's a patch to add clang fp options here, 
https://reviews.llvm.org/D62731




Comment at: include/llvm/IR/IRBuilder.h:234
+  /// Set the exception handling to be used with constrained floating point
+  void setDefaultConstrainedExcept(MDNode *NewExcept) { 
+DefaultConstrainedExcept = NewExcept; 

andrew.w.kaylor wrote:
> I think it would be better to add some enumerated type to describe the 
> exception semantic and rounding modes. The MDNode is an implementation detail 
> and an awkward one for the front end to have to deal with.
I posted a patch showing the rounding and exception information being passed as 
enumeration not MDNode. I've uploaded the patch here, 
https://reviews.llvm.org/D62730


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

https://reviews.llvm.org/D53157



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

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

New files are missing header blurbs; the **precise** explanation of what is 
what is missing (should be in `doc/`, not patch description)




Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+FPM_Off,
+FPM_Precise,
+FPM_Strict,
+FPM_Fast
+  };
+

All this needs comments, and possibly better names.
`FPM_Off`,etc is very non-self-explanatory.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: gribozavr.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Previously, we intended to omit the check fix to the case when constructor has
any braced-init-list argument. But the HasListInitializedArgument was not
correct to handle all cases (Foo(Bar{1, 2}) will return false in C++14
mode).

This patch fixes it, corrects the tests, and makes the check to run at C++17 
mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62736

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +454,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +468,10 @@
 
   std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 
1});
   PJ4.reset(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES:  PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ4.reset(new J{E{1, 2}, 1});
 
   std::unique_ptr FF = std::unique_ptr(new Foo());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -298,11 +298,20 @@
 return true;
   // Check whether we implicitly construct a class from a
   // std::initializer_list.
-  if (const auto *ImplicitCE = dyn_cast(Arg)) {
-if (ImplicitCE->isStdInitListInitialization())
+  if (const auto *CEArg = dyn_cast(Arg)) {
+// Strip the ediable move constructor, it is presented in C++11/14 
mode.
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+// an elidable move constructor.
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =
+dyn_cast(TempExp->IgnoreImplicit()))
+  CEArg = UnwrappedCE;
+  }
+}
+if (CEArg->isStdInitListInitialization())
   return true;
   }
-  return false;
 }
 return false;
   };


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +454,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +4

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Documentation missing. All the blurb from patch description should be in `doc/`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[clang-tools-extra] r362226 - [clangd] Add missing license for rename.cpp, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri May 31 07:38:16 2019
New Revision: 362226

URL: http://llvm.org/viewvc/llvm-project?rev=362226&view=rev
Log:
[clangd] Add missing license for rename.cpp, NFC.

Modified:
clang-tools-extra/trunk/clangd/refactor/Rename.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Rename.cpp?rev=362226&r1=362225&r2=362226&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/Rename.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp Fri May 31 07:38:16 2019
@@ -1,3 +1,11 @@
+//===--- Rename.cpp - Symbol-rename refactorings -*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "refactor/Rename.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
@@ -65,7 +73,7 @@ renameWithinFile(ParsedAST &AST, llvm::S
   // Right now we only support renaming the main file, so we
   // drop replacements not for the main file. In the future, we might
   // also support rename with wider scope.
-  // Rename sometimes returns duplicate edits (which is a bug). A side-effect 
of 
+  // Rename sometimes returns duplicate edits (which is a bug). A side-effect 
of
   // adding them to a single Replacements object is these are deduplicated.
   for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
 for (const auto &Rep : Change.getReplacements()) {


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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:302
+  if (const auto *CEArg = dyn_cast(Arg)) {
+// Strip the ediable move constructor, it is presented in C++11/14 
mode.
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around

s/ediable/elidable/

s/presented/present in the AST/



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:304
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+// an elidable move constructor.
+if (CEArg->isElidable()) {

Isn't it the other way round -- the move constructor is around the init-list 
constructor?



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:1
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
 

"-std=c++14-or-later" (will also cover c++2a etc.)



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));

Why do we print the warning if the fixit is not fixing anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 202433.
SjoerdMeijer added a comment.

This time with tests.


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

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/armv8.1m.main.c
  clang/test/Driver/armv8.1m.main.s
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -484,22 +484,100 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef &Name) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName &InputFPU = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName &CandidateFPU : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector &Features) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  bool Negated = stripNegationPrefix(ArchExt);
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+
+if (ArchExt == "fp.dp") {
+  unsigned DoubleFPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+
+  // If the default FPU already supports double-precision, or if
+  // there is no double-prec FPU that extends it, then "fp.dp"
+  // doesn't have a separate meaning, and we treat it as an
+  // invalid extension name.
+  if (DoubleFPUKind == FK_INVALID)
+return false;
+
+  // If there _is_ a separate double-precision FPU, then "nofp.dp"
+  // should disable just the double-precision extension, leaving
+  // the base FPU still enabled if it previously was.
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+
+  // Otherwise, select the double-precision FPU.
+  FPUKind = DoubleFPUKind;
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+  if (FPUKind == ARM::FK_NONE)
+return false;
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -240,6 +240,8 @@
 StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(unsigned ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
+bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+   std::vector &Features);
 StringRef getHWDivName(unsigned HWDivKind);
 
 // Information by Name
Index: clang/test/Driver/armv8.1m.main.s
===
--- clang/test/Driver/armv8.1m.main.s
+++ clang/test/Driver/armv8.1m.main.s
@@ -5,8 +5,14 @@
 # RUN:  FileCheck --check-prefi

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 202434.
hokein marked 6 inline comments as done.
hokein added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
+// FIXME: Fix the test code in C++2a mode.
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +455,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +469,10 @@
 
   std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 
1});
   PJ4.reset(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES:  PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ4.reset(new J{E{1, 2}, 1});
 
   std::unique_ptr FF = std::unique_ptr(new Foo());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -298,11 +298,20 @@
 return true;
   // Check whether we implicitly construct a class from a
   // std::initializer_list.
-  if (const auto *ImplicitCE = dyn_cast(Arg)) {
-if (ImplicitCE->isStdInitListInitialization())
+  if (const auto *CEArg = dyn_cast(Arg)) {
+// Strip the elidable move constructor, it is present in the AST for
+// C++11/14, e.g. Foo(Bar{1, 2}), the move constructor is around the
+// init-list constructor.
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =
+dyn_cast(TempExp->IgnoreImplicit()))
+  CEArg = UnwrappedCE;
+  }
+}
+if (CEArg->isStdInitListInitialization())
   return true;
   }
-  return false;
 }
 return false;
   };


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
+// FIXME: Fix the test code in C++2a mode.
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +455,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +469,10 @@
 
   std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr P

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:304
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+// an elidable move constructor.
+if (CEArg->isElidable()) {

gribozavr wrote:
> Isn't it the other way round -- the move constructor is around the init-list 
> constructor?
yeah, this is what I mean, updated the comment.



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:1
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
 

gribozavr wrote:
> "-std=c++14-or-later" (will also cover c++2a etc.)
hmm, the test code has some compiler errors on c++2a mode,  it is a different 
issue (I will address it in a follow-up), added a FIXME. 



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));

gribozavr wrote:
> Why do we print the warning if the fixit is not fixing anything?
We may still want to inform users, these cases can still be converted to 
`make_unique`, but the check currently is not smart enough to give correct 
fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn updated this revision to Diff 202436.
kpn marked 4 inline comments as done.
kpn added a comment.

Address review comments.


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

https://reviews.llvm.org/D53157

Files:
  include/llvm/IR/IRBuilder.h
  unittests/IR/IRBuilderTest.cpp

Index: unittests/IR/IRBuilderTest.cpp
===
--- unittests/IR/IRBuilderTest.cpp
+++ unittests/IR/IRBuilderTest.cpp
@@ -122,6 +122,100 @@
   EXPECT_FALSE(II->hasNoNaNs());
 }
 
+TEST_F(IRBuilderTest, ConstrainedFP) {
+  IRBuilder<> Builder(BB);
+  Value *V;
+  CallInst *Call;
+  IntrinsicInst *II;
+
+  V = Builder.CreateLoad(GV);
+
+  Call = Builder.CreateConstrainedFAdd(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  Call = Builder.CreateConstrainedFSub(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  Call = Builder.CreateConstrainedFMul(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fmul);
+
+  Call = Builder.CreateConstrainedFDiv(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fdiv);
+
+  Call = Builder.CreateConstrainedFRem(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_frem);
+
+  // Now see if we get constrained intrinsics instead of non-constrained
+  // instructions.
+  Builder.setIsConstrainedFP(true);
+
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  V = Builder.CreateFSub(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  V = Builder.CreateFMul(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fmul);
+  
+  V = Builder.CreateFDiv(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fdiv);
+  
+  V = Builder.CreateFRem(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_frem);
+
+  // Verify the codepaths for setting and overriding the default metadata.
+  MDNode *ExceptStr = MDNode::get(Builder.getContext(), 
+  MDString::get(Builder.getContext(), 
+"fpexcept.strict"));
+  MDNode *ExceptIgn = MDNode::get(Builder.getContext(), 
+  MDString::get(Builder.getContext(), 
+"fpexcept.ignore"));
+  MDNode *RoundDyn = MDNode::get(Builder.getContext(), 
+ MDString::get(Builder.getContext(),
+   "round.dynamic"));
+  MDNode *RoundUp = MDNode::get(Builder.getContext(), 
+MDString::get(Builder.getContext(),
+  "round.upward"));
+
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  auto *CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebStrict);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDynamic);
+
+  Builder.setDefaultConstrainedExcept(ExceptIgn);
+  Builder.setDefaultConstrainedRounding(RoundUp);
+  V = Builder.CreateFAdd(V, V);
+  CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebIgnore);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmUpward);
+
+  // Now override the defaults.
+  Call = Builder.CreateConstrainedFAdd(V, V, nullptr, "", RoundDyn, ExceptStr);
+  CII = cast(Call);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebStrict);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDynamic);
+
+  Builder.CreateRetVoid();
+  EXPECT_FALSE(verifyModule(*M));
+}
+
 TEST_F(IRBuilderTest, Lifetime) {
   IRBuilder<> Builder(BB);
   AllocaInst *Var1 = Builder.CreateAlloca(Builder.getInt8Ty());
Index: include/llvm/IR/IRBuilder.h
===
--- include/llvm/IR/IRBuilder.h
+++ include/llvm/IR/IRBuilder.h
@@ -96,13 +96,17 @@
   MDNode *DefaultFPMathTag;
   FastMathFlags FMF;
 
+  bool IsFPConstrained;
+  MDNode *DefaultConstrainedExcept = nullptr;
+  MDNode *DefaultConstrainedRounding = nullptr;
+
   ArrayRef DefaultOperandBundles;
 
 public:
   IRBuilderBase(LLVMContext &context, MDNode *FPMathTag = nullptr,
 ArrayRef OpBundles = None)
   : Context(context), DefaultFPMathTag(FPMathTag),
-DefaultOperandBundles(OpBundles) {
+IsFPConstrained(false), DefaultConstrainedExcept(nullptr) {
 ClearInsertionPoint();
   }
 
@@ -218,6 +222,40 @@
   /// Set t

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Incorporating feedback from D53157  is 
probably a good idea. Looks like that hasn't been done yet here completely.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 202438.
balazske added a comment.

- Import BraceRange of EnumDecl.
- Added lit tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60499

Files:
  lib/AST/ASTImporter.cpp
  test/Import/enum/test.cpp
  test/Import/namespace/Inputs/NS.cpp
  test/Import/namespace/test.cpp
  test/Import/struct-and-var/test.cpp
  test/Import/template-specialization/test.cpp

Index: test/Import/template-specialization/test.cpp
===
--- test/Import/template-specialization/test.cpp
+++ test/Import/template-specialization/test.cpp
@@ -1,4 +1,7 @@
-// RUN: clang-import-test -import %S/Inputs/T.cpp -expression %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/T.cpp -expression %s | FileCheck %s
+
+// CHECK: |-ClassTemplateSpecializationDecl
+// CHECK-SAME:  line:4:20 struct A
 
 void expr() {
   A::B b1;
Index: test/Import/struct-and-var/test.cpp
===
--- test/Import/struct-and-var/test.cpp
+++ test/Import/struct-and-var/test.cpp
@@ -1,4 +1,8 @@
-// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s
+// RUN: clang-import-test -dump-ast --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s | FileCheck %s
+
+// CHECK: `-CXXRecordDecl
+// CHECK-SAME: Inputs/S2.cpp:1:1, line:3:1> line:1:8 struct F
+
 void expr() {
   struct F f;
   int x = f.a;
Index: test/Import/namespace/test.cpp
===
--- /dev/null
+++ test/Import/namespace/test.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/NS.cpp -expression %s | FileCheck %s
+
+// CHECK: `-NamespaceDecl
+// CHECK-SAME: Inputs/NS.cpp:1:1, line:5:1> line:1:11 NS
+
+void expr() {
+  static_assert(NS::A == 3);
+}
Index: test/Import/namespace/Inputs/NS.cpp
===
--- /dev/null
+++ test/Import/namespace/Inputs/NS.cpp
@@ -0,0 +1,5 @@
+namespace NS {
+  void f1();
+  void f2();
+  const int A = 3;
+};
Index: test/Import/enum/test.cpp
===
--- test/Import/enum/test.cpp
+++ test/Import/enum/test.cpp
@@ -1,5 +1,7 @@
 // RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s | FileCheck %s
 
+// CHECK: |-EnumDecl
+// CHECK-SAME: Inputs/S.cpp:1:1, line:4:1> line:1:6 E
 // CHECK: OpaqueWithType 'long'
 
 void expr() {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2158,6 +2158,9 @@
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
 return BeginLocOrErr.takeError();
+  ExpectedSLoc RBraceLocOrErr = import(D->getRBraceLoc());
+  if (!RBraceLocOrErr)
+return RBraceLocOrErr.takeError();
 
   // Create the "to" namespace, if needed.
   NamespaceDecl *ToNamespace = MergeWithNamespace;
@@ -2167,6 +2170,7 @@
 *BeginLocOrErr, Loc, Name.getAsIdentifierInfo(),
 /*PrevDecl=*/nullptr))
   return ToNamespace;
+ToNamespace->setRBraceLoc(*RBraceLocOrErr);
 ToNamespace->setLexicalDeclContext(LexicalDC);
 LexicalDC->addDeclInternal(ToNamespace);
 
@@ -2464,9 +2468,10 @@
   SourceLocation ToBeginLoc;
   NestedNameSpecifierLoc ToQualifierLoc;
   QualType ToIntegerType;
-  if (auto Imp = importSeq(
-  D->getBeginLoc(), D->getQualifierLoc(), D->getIntegerType()))
-std::tie(ToBeginLoc, ToQualifierLoc, ToIntegerType) = *Imp;
+  SourceRange ToBraceRange;
+  if (auto Imp = importSeq(D->getBeginLoc(), D->getQualifierLoc(),
+   D->getIntegerType(), D->getBraceRange()))
+std::tie(ToBeginLoc, ToQualifierLoc, ToIntegerType, ToBraceRange) = *Imp;
   else
 return Imp.takeError();
 
@@ -2480,6 +2485,7 @@
 
   D2->setQualifierInfo(ToQualifierLoc);
   D2->setIntegerType(ToIntegerType);
+  D2->setBraceRange(ToBraceRange);
   D2->setAccess(D->getAccess());
   D2->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(D2);
@@ -2712,6 +2718,10 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
   if (auto QualifierLocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*QualifierLocOrErr);
   else
@@ -5181,6 +5191,11 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
+
   // Import the qualifier, if any.
   if (auto LocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*LocOrErr);
@@ -6174,7 +6189,8 @@
   TemplateArgumentListInfo *ToResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) 

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I don't know which patch will proceed; but they weren't added already, this 
seems to be missing documentation changes.


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

https://reviews.llvm.org/D53157



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

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Added lit tests for the simple (`Decl`) cases. The `Expr` and type related 
changes are more tricky to do. But I do not like this approach to add new tests 
because the test function in D60463  does 
almost the same thing for every import in ASTTests without adding lot of new 
checks manually.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60499



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

Re: r278642 - P0217R3: code generation support for decomposition declarations.

[Nico Weber via cfe-commits]

The invented mangling for clang-cl turns out to not be correct :)
https://bugs.llvm.org/show_bug.cgi?id=42093

Maybe it's better to add a "can't yet mangle" error, so that we don't
silently do the wrong thing?

On Sun, Aug 14, 2016 at 9:41 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Sun Aug 14 20:33:41 2016
> New Revision: 278642
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278642&view=rev
> Log:
> P0217R3: code generation support for decomposition declarations.
>
> Added:
> cfe/trunk/test/CodeGenCXX/cxx1z-decomposition.cpp
> Modified:
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/ItaniumMangle.cpp
> cfe/trunk/lib/AST/MicrosoftMangle.cpp
> cfe/trunk/lib/CodeGen/CGDecl.cpp
> cfe/trunk/lib/CodeGen/CGExpr.cpp
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/SemaCXX/cxx1z-decomposition.cpp
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=278642&r1=278641&r2=278642&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Sun Aug 14 20:33:41 2016
> @@ -8721,6 +8721,14 @@ bool ASTContext::DeclMustBeEmitted(const
>!VD->evaluateValue())
>  return true;
>
> +  // Likewise, variables with tuple-like bindings are required if their
> +  // bindings have side-effects.
> +  if (auto *DD = dyn_cast(VD))
> +for (auto *BD : DD->bindings())
> +  if (auto *BindingVD = BD->getHoldingVar())
> +if (DeclMustBeEmitted(BindingVD))
> +  return true;
> +
>return false;
>  }
>
>
> Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=278642&r1=278641&r2=278642&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
> +++ cfe/trunk/lib/AST/ItaniumMangle.cpp Sun Aug 14 20:33:41 2016
> @@ -1195,18 +1195,21 @@ void CXXNameMangler::mangleUnqualifiedNa
>case DeclarationName::Identifier: {
>  const IdentifierInfo *II = Name.getAsIdentifierInfo();
>
> -// We mangle decomposition declarations as the name of their first
> binding.
> +// We mangle decomposition declarations as the names of their
> bindings.
>  if (auto *DD = dyn_cast(ND)) {
> -  auto B = DD->bindings();
> -  if (B.begin() == B.end()) {
> -// FIXME: This is ill-formed but we accept it as an extension.
> -DiagnosticsEngine &Diags = Context.getDiags();
> -unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
> -"cannot mangle global empty decomposition decl");
> -Diags.Report(DD->getLocation(), DiagID);
> -break;
> -  }
> -  II = (*B.begin())->getIdentifier();
> +  // FIXME: Non-standard mangling for decomposition declarations:
> +  //
> +  //   ::= DC * E
> +  //
> +  // These can never be referenced across translation units, so we do
> +  // not need a cross-vendor mangling for anything other than
> demanglers.
> +  // Proposed on cxx-abi-dev on 2016-08-12
> +  Out << "DC";
> +  for (auto *BD : DD->bindings())
> +mangleSourceName(BD->getDeclName().getAsIdentifierInfo());
> +  Out << 'E';
> +  writeAbiTags(ND, AdditionalAbiTags);
> +  break;
>  }
>
>  if (II) {
>
> Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=278642&r1=278641&r2=278642&view=diff
>
> ==
> --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
> +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Sun Aug 14 20:33:41 2016
> @@ -394,7 +394,8 @@ bool MicrosoftMangleContextImpl::shouldM
>if (!getASTContext().getLangOpts().CPlusPlus)
>  return false;
>
> -  if (const VarDecl *VD = dyn_cast(D)) {
> +  const VarDecl *VD = dyn_cast(D);
> +  if (VD && !isa(D)) {
>  // C variables are not mangled.
>  if (VD->isExternC())
>return false;
> @@ -780,6 +781,21 @@ void MicrosoftCXXNameMangler::mangleUnqu
>  }
>}
>
> +  if (const DecompositionDecl *DD = dyn_cast(ND)) {
> +// FIXME: Invented mangling for decomposition declarations:
> +//   [X,Y,Z]
> +// where X,Y,Z are the names of the bindings.
> +llvm::SmallString<128> Name("[");
> +for (auto *BD : DD->bindings()) {
> +  if (Name.size() > 1)
> +Name += ',';
> +  Name += BD->getDeclName().getAsIdentifierInfo()->getName();
> +}
> +Name += ']';
> +mangleSourceName(Name);
> +break;
> +  }
> +
>if (const VarDecl *VD = dyn_cast(ND)) {
>  // We must have an anonymous

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

I wanted to get the API straight before working on documentation.




Comment at: include/llvm/IR/IRBuilder.h:228
+  /// Enable/Disable use of constrained floating point math
+  void setIsConstrainedFP(bool IsCon) { IsFPConstrained = IsCon; }
+

erichkeane wrote:
> kpn wrote:
> > kbarton wrote:
> > > This is a minor quibble, but the method is setIsConstrainedFP, while the 
> > > member is IsFPConstrained. 
> > > I'm not sure if that is intentionally different, or an oversight. 
> > Yeah, that's an oversight. Fixed.
> IS this fixed?
In my working copy, yes.



Comment at: include/llvm/IR/IRBuilder.h:234
+  /// Set the exception handling to be used with constrained floating point
+  void setDefaultConstrainedExcept(MDNode *NewExcept) { 
+DefaultConstrainedExcept = NewExcept; 

mibintc wrote:
> andrew.w.kaylor wrote:
> > I think it would be better to add some enumerated type to describe the 
> > exception semantic and rounding modes. The MDNode is an implementation 
> > detail and an awkward one for the front end to have to deal with.
> I posted a patch showing the rounding and exception information being passed 
> as enumeration not MDNode. I've uploaded the patch here, 
> https://reviews.llvm.org/D62730
It's a good idea. D62730 beat me to the punch.



Comment at: include/llvm/IR/IRBuilder.h:1249
+if (IsFPConstrained)
+  return CreateConstrainedFAdd(L, R, FMFSource, Name, nullptr, nullptr);
+

erichkeane wrote:
> Why set the last 2 to nullptr when you have defaults for these?
Eh, no really good reason.



Comment at: include/llvm/IR/IRBuilder.h:1259
+  Instruction *FMFSource = nullptr,
   const Twine &Name = "",
+  MDNode *RoundingMD = nullptr,   
+  MDNode *ExceptMD = nullptr) {

erichkeane wrote:
> The last 2 parameters are never actually used except in the test.  Are these 
> really important to have if they are never used in source?
They're part of the intrinsics. It seems like there should be a way to emit 
them as part of the instruction when creating the instruction.



Comment at: include/llvm/IR/IRBuilder.h:1408
 
+  CallInst *CreateConstrainedFRem(Value *L, Value *R, 
+  Instruction *FMFSource = nullptr, 

erichkeane wrote:
> All these CreateConstrainedXXX should be distilled down to a single function 
> that takes the intrinsic as a parameter.
That moves us further down the road towards having modes. I haven't seen a 
front-end guy sign off on it yet, but it seems like a good idea.


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

https://reviews.llvm.org/D53157



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

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added inline comments.



Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+FPM_Off,
+FPM_Precise,
+FPM_Strict,
+FPM_Fast
+  };
+

lebedev.ri wrote:
> All this needs comments, and possibly better names.
> `FPM_Off`,etc is very non-self-explanatory.
Shouldn't there be an "accept default" value? That way 
CreateConstrainedFAdd()/CreateConstrainedBinOp() can accept enumerations and 
avoid having front ends having to deal with MDNode*.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:234
+  /// Set the exception handling to be used with constrained floating point
+  void setDefaultConstrainedExcept(MDNode *NewExcept) { 
+DefaultConstrainedExcept = NewExcept; 

kpn wrote:
> mibintc wrote:
> > andrew.w.kaylor wrote:
> > > I think it would be better to add some enumerated type to describe the 
> > > exception semantic and rounding modes. The MDNode is an implementation 
> > > detail and an awkward one for the front end to have to deal with.
> > I posted a patch showing the rounding and exception information being 
> > passed as enumeration not MDNode. I've uploaded the patch here, 
> > https://reviews.llvm.org/D62730
> It's a good idea. D62730 beat me to the punch.
BTW I created FPState.h because if I put the enumeration types in IRBuilder it 
dragged in too many include files in clang and required other random source 
changes for disambigutation.  Anyway I don't need to comandeer your patch -- I 
don't want it to die from neglect.  I can close mine? 


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

https://reviews.llvm.org/D53157



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

r362232 - Revise test case due to the change from CUDA 10+.

[Michael Liao via cfe-commits]

Author: hliao
Date: Fri May 31 08:29:55 2019
New Revision: 362232

URL: http://llvm.org/viewvc/llvm-project?rev=362232&view=rev
Log:
Revise test case due to the change from CUDA 10+.

Modified:
cfe/trunk/test/Driver/offloading-interoperability.c

Modified: cfe/trunk/test/Driver/offloading-interoperability.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/offloading-interoperability.c?rev=362232&r1=362231&r2=362232&view=diff
==
--- cfe/trunk/test/Driver/offloading-interoperability.c (original)
+++ cfe/trunk/test/Driver/offloading-interoperability.c Fri May 31 08:29:55 2019
@@ -11,7 +11,7 @@
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE:  clang{{.*}}" "-cc1" "-triple" 
"nvptx64-nvidia-cuda"
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NOT:  -fopenmp
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: ptxas" "-m64"
-// NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: fatbinary" "--cuda" "-64"
+// NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: fatbinary"{{( "--cuda")?}} "-64"
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: clang{{.*}}" "-cc1" "-triple" 
"powerpc64le-unknown-linux-gnu"
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE:  -fopenmp
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: {{ld(.exe)?"}} {{.*}}"-m" "elf64lppc"


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

[PATCH] D62582: [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 202441.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Handle methods without "normal" identifier names.
Add comments.
Trigger in both directions (dominated and dominated-by).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62582

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.cpp

Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -210,3 +210,66 @@
 // CHECK-CC9: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>) (requires fix-it: {181:4-181:5} to "->")
 // CHECK-CC9: memfun3 : [#int#]memfun3(<#int#>) (requires fix-it: {181:4-181:5} to "->")
 // CHECK-CC9: operator-> : [#Derived *#]operator->()[# const#]
+
+// These overload sets differ only by return type and this-qualifiers.
+// So for any given callsite, only one is available.
+struct Overloads {
+  double ConstOverload(char);
+  int ConstOverload(char) const;
+
+  int RefOverload(char) &;
+  double RefOverload(char) const&;
+  char RefOverload(char) &&;
+};
+void testLValue(Overloads& Ref) {
+  Ref.
+}
+void testConstLValue(const Overloads& ConstRef) {
+  ConstRef.
+}
+void testRValue() {
+  Overloads().
+}
+void testXValue(Overloads& X) {
+  static_cast(X).
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:225:7 %s -o - | FileCheck -check-prefix=CHECK-LVALUE %s \
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \
+// RUN: --implicit-check-not="[#double#]RefOverload(" \
+// RUN: --implicit-check-not="[#char#]RefOverload("
+// CHECK-LVALUE-DAG: [#double#]ConstOverload(
+// CHECK-LVALUE-DAG: [#int#]RefOverload(
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:228:12 %s -o - | FileCheck -check-prefix=CHECK-CONSTLVALUE %s \
+// RUN: --implicit-check-not="[#double#]ConstOverload(" \
+// RUN: --implicit-check-not="[#int#]RefOverload(" \
+// RUN: --implicit-check-not="[#char#]RefOverload("
+// CHECK-CONSTLVALUE: [#int#]ConstOverload(
+// CHECK-CONSTLVALUE: [#double#]RefOverload(
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:231:15 %s -o - | FileCheck -check-prefix=CHECK-PRVALUE %s \
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \
+// RUN: --implicit-check-not="[#int#]RefOverload(" \
+// RUN: --implicit-check-not="[#double#]RefOverload("
+// CHECK-PRVALUE: [#double#]ConstOverload(
+// CHECK-PRVALUE: [#char#]RefOverload(
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:234:31 %s -o - | FileCheck -check-prefix=CHECK-XVALUE %s \
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \
+// RUN: --implicit-check-not="[#int#]RefOverload(" \
+// RUN: --implicit-check-not="[#double#]RefOverload("
+// CHECK-XVALUE: [#double#]ConstOverload(
+// CHECK-XVALUE: [#char#]RefOverload(
+
+void testOverloadOperator() {
+  struct S {
+char operator=(int) const;
+int operator=(int);
+  } s;
+  return s.
+}
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:270:12 %s -o - | FileCheck -check-prefix=CHECK-OPER %s \
+// RUN: --implicit-check-not="[#char#]operator=("
+// CHECK-OPER: [#int#]operator=(
+
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -16,7 +16,9 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/QualTypeNames.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
@@ -152,9 +154,16 @@
   /// different levels of, e.g., the inheritance hierarchy.
   std::list ShadowMaps;
 
+  /// Overloaded C++ member functions found by SemaLookup.
+  /// Used to determine when one overload is dominated by another.
+  llvm::DenseMap, ShadowMapEntry>
+  OverloadMap;
+
   /// If we're potentially referring to a C++ member function, the set
   /// of qualifiers applied to the object type.
   Qualifiers ObjectTypeQualifiers;
+  /// The kind of the object expression, for rvalue/lvalue overloads.
+  ExprValueKind ObjectKind;
 
   /// Whether the \p ObjectTypeQualifiers field is active.
   bool HasObjectTypeQualifiers;
@@ -230,8 +239,9 @@
   /// out member functions that aren't available (because there will be a
   /// cv-qualifier mismatch) or prefer functions with an exact qualifier
   /// match.
-  void setObjectTypeQualifiers(Qualifiers Quals) {
+  void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {
 ObjectTypeQualifiers = Quals;
+ObjectKind = Kind;
 HasObjectTypeQualifiers = true;
   }
 
@@ -1157,6 +1167,53 @@
   R.InBaseClass = true;
 }
 
+enum class OverloadCompare { BothViable, Dominates, Dominated };
+// Will Candidate ever be called on the object, when ove

[PATCH] D62582: [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Oops, we weren't handling the case when the newly-added overload is dominated 
by an existing one. Fixed.




Comment at: lib/Sema/SemaCodeComplete.cpp:1300
+auto &OverloadSet =
+OverloadMap[std::make_pair(CurContext, Method->getName())];
+for (const DeclIndexPair& Entry : OverloadSet) {

ilya-biryukov wrote:
> Won't this crash on `Method->getName()`? Could we add a test for that?
> Completing in a class that has a user-defined `operator==()` would probably 
> do the trick.
Right! Fixed and test added. We now no longer hash the strings which is nice 
too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62582



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

[PATCH] D62738: [HIP] Support texture type

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a reviewer: a.sidorin.

This patch handles `__attribute__((device_builtin_vector_type))`  for HIP.

If a class or struct type has this attribute, any variables with this type will
be emitted as global symbol in device code with undef initializer, and
registered by `__hipRegisterVar` with shadow variable in host code.

This allows HIP runtime to implement support of HIP texture type.

For CUDA, there is no change for codegen since it depends on implementation.


https://reviews.llvm.org/D62738

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCUDA/texture.cu
  test/SemaCUDA/attr-declspec.cu
  test/SemaCUDA/attributes-on-non-cuda.cu

Index: test/SemaCUDA/attributes-on-non-cuda.cu
===
--- test/SemaCUDA/attributes-on-non-cuda.cu
+++ test/SemaCUDA/attributes-on-non-cuda.cu
@@ -7,11 +7,12 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c %s
 
 #if defined(EXPECT_WARNINGS)
-// expected-warning@+12 {{'device' attribute ignored}}
-// expected-warning@+12 {{'global' attribute ignored}}
-// expected-warning@+12 {{'constant' attribute ignored}}
-// expected-warning@+12 {{'shared' attribute ignored}}
-// expected-warning@+12 {{'host' attribute ignored}}
+// expected-warning@+13 {{'device' attribute ignored}}
+// expected-warning@+13 {{'global' attribute ignored}}
+// expected-warning@+13 {{'constant' attribute ignored}}
+// expected-warning@+13 {{'shared' attribute ignored}}
+// expected-warning@+13 {{'host' attribute ignored}}
+// expected-warning@+20 {{'device_builtin_texture_type' attribute ignored}}
 //
 // NOTE: IgnoredAttr in clang which is used for the rest of
 // attributes ignores LangOpts, so there are no warnings.
Index: test/SemaCUDA/attr-declspec.cu
===
--- test/SemaCUDA/attr-declspec.cu
+++ test/SemaCUDA/attr-declspec.cu
@@ -6,11 +6,12 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fms-extensions -fsyntax-only -verify -x c %s
 
 #if defined(EXPECT_WARNINGS)
-// expected-warning@+12 {{'__device__' attribute ignored}}
-// expected-warning@+12 {{'__global__' attribute ignored}}
-// expected-warning@+12 {{'__constant__' attribute ignored}}
-// expected-warning@+12 {{'__shared__' attribute ignored}}
-// expected-warning@+12 {{'__host__' attribute ignored}}
+// expected-warning@+13 {{'__device__' attribute ignored}}
+// expected-warning@+13 {{'__global__' attribute ignored}}
+// expected-warning@+13 {{'__constant__' attribute ignored}}
+// expected-warning@+13 {{'__shared__' attribute ignored}}
+// expected-warning@+13 {{'__host__' attribute ignored}}
+// expected-warning@+19 {{'__device_builtin_texture_type__' attribute ignored}}
 //
 // (Currently we don't for the other attributes. They are implemented with
 // IgnoredAttr, which is ignored irrespective of any LangOpts.)
Index: test/CodeGenCUDA/texture.cu
===
--- /dev/null
+++ test/CodeGenCUDA/texture.cu
@@ -0,0 +1,28 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -std=c++11 -fcuda-is-device \
+// RUN: -emit-llvm -o - %s | FileCheck -check-prefixes=CUDADEV %s
+// RUN: %clang_cc1 -triple x86_64 -std=c++11 \
+// RUN: -emit-llvm -o - %s | FileCheck -check-prefixes=CUDAHOST %s
+
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -std=c++11 -fvisibility hidden -fapply-global-visibility-to-externs \
+// RUN: -emit-llvm -o - -x hip %s | FileCheck -check-prefixes=HIPDEV %s
+// RUN: %clang_cc1 -triple x86_64 -std=c++11 \
+// RUN: -emit-llvm -o - -x hip %s | FileCheck -check-prefixes=HIPHOST %s
+
+struct textureReference {
+  int a;
+};
+
+template 
+struct __attribute__((device_builtin_texture_type)) texture : public textureReference {
+texture() { a = 1; }
+};
+
+texture tex;
+// CUDADEV-NOT: @tex
+// CUDAHOST-NOT: call i32 @__hipRegisterVar{{.*}}@tex
+// HIPDEV: @tex = protected addrspace(1) global %struct.texture undef
+// HIPDEV-NOT: declare{{.*}}void @_ZN7textureIfLi2ELi1EEC1Ev
+// HIPHOST:  define{{.*}}@_ZN7textureIfLi2ELi1EEC1Ev
+// HIPHOST:  call i32 @__hipRegisterVar{{.*}}@tex
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6786,6 +6786,9 @@
   case ParsedAttr::AT_CUDAHost:
 handleSimpleAttributeWithExclusions(S, D, AL);
 break;
+  case ParsedAttr::AT_CUDADeviceBuiltinTextureType:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_GNUInline:
 handleGNUInlineAttr(S, D, AL);
 break;
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGe

r362236 - Suppress nothrow/exception spec conflict warning when ES is parsed.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 08:56:27 2019
New Revision: 362236

URL: http://llvm.org/viewvc/llvm-project?rev=362236&view=rev
Log:
Suppress nothrow/exception spec conflict warning when ES is parsed.

The previously added warning ended up causing false positives when
nothrow was used on member functions, where the exception specification
wasn't yet parsed.  So, throw() and noexcept(true) both were incorrectly
warning.  There doesn't seem to be a good way to force these to be parsed
to identify which they are (and likely should not be), so suppress the warning.

For now, unevaluated/uninstantiated are left as warnings as I am not
creative enough to find a reproducer that causes a false positive for
either.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=362236&r1=362235&r2=362236&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri May 31 08:56:27 2019
@@ -6976,7 +6976,10 @@ static bool handleFunctionTypeAttr(TypeP
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
   case EST_NoThrow:
+  case EST_Unparsed:
 // Exception spec doesn't conflict with nothrow, so don't warn.
+// Unparsed is included in this, since method signatures aren't parsed
+// until after the fact.
 break;
 
   case EST_Dynamic:
@@ -6985,7 +6988,6 @@ static bool handleFunctionTypeAttr(TypeP
   case EST_DependentNoexcept:
   case EST_Unevaluated:
   case EST_Uninstantiated:
-  case EST_Unparsed:
 S.Diag(attr.getLoc(), diag::warn_nothrow_attribute_ignored);
 break;
   }

Modified: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp?rev=362236&r1=362235&r2=362236&view=diff
==
--- cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp (original)
+++ cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp Fri May 31 08:56:27 
2019
@@ -53,3 +53,16 @@ __declspec(nothrow) void foo4() noexcept
 __declspec(nothrow) void foo5() noexcept(noexcept(foo2()));
 // expected-warning@+1{{'nothrow' attribute conflicts with exception 
specification; attribute ignored}}
 __declspec(nothrow) void foo6() noexcept(noexcept(foo3()));
+
+// FIXME: It would be nice to be able to warn on these, however at the time we
+// evaluate the nothrow, these have yet to be parsed, so the data is not yet
+// there.
+struct S {
+  __declspec(nothrow) void f1();
+#ifndef CPP17
+  __declspec(nothrow) void f2() throw();
+  __declspec(nothrow) void f3() throw(int);
+#endif
+  __declspec(nothrow) void f4() noexcept(true);
+  __declspec(nothrow) void f5() noexcept(false);
+};


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

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 3 inline comments as done.
mibintc added inline comments.



Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+FPM_Off,
+FPM_Precise,
+FPM_Strict,
+FPM_Fast
+  };
+

kpn wrote:
> lebedev.ri wrote:
> > All this needs comments, and possibly better names.
> > `FPM_Off`,etc is very non-self-explanatory.
> Shouldn't there be an "accept default" value? That way 
> CreateConstrainedFAdd()/CreateConstrainedBinOp() can accept enumerations and 
> avoid having front ends having to deal with MDNode*.
By "off" I just meant, not specified, and therefore the default setting would 
prevail.  If you prefer "default" instead of "off" that's fine by me. 



Comment at: include/llvm/IR/IRBuilder.h:247
+switch (DefaultConstrainedExcept) {
+case FPState::CE_Off:
+case FPState::CE_Strict:

In order to match the code that @kpn wrote, I should set 
DefaultConstrainedExcept = strict instead of just leaving it off.



Comment at: include/llvm/IR/IRBuilder.h:261
+switch (DefaultConstrainedRounding) {
+case FPState::CR_Off:
+case FPState::CR_Dynamic:

to match @kpn patch, I should set DefaultConstrainedRounding to dynamic instead 
of leaving it off


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Nikolaus Wittenstein via Phabricator via cfe-commits]

adzenith added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

I just came across this case-insensitive behavior when trying to regroup 
include blocks and it was surprising. I'm trying to put system headers after 
other angle-bracket includes (like for Qt), and I was trying:

  - Regex:   '^<[a-z_]*>'
Priority:5
  - Regex:   '^<[A-Za-z_]*>'
Priority:4

Unfortunately, `#include ` matches the first regex and so Qt headers 
get sorted in with system headers. Would it be possible to add the option to do 
case-sensitive matching after all? Should I open a new issue?

For anyone else who wants case-sensitive matching, I just did discover that 
`(?-i)` seems to do the trick. I thought it wouldn't because the docs mention 
extended regex, but I guess it's more extended than I thought! Using 
`'(?-i)^<[a-z_]*>'` does the trick for me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33932



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:311
+  }
+}
+if (CEArg->isStdInitListInitialization())

So `Foo(Bar{1, 2})` is not problematic, it is just that we can't tell it apart 
from `Foo({1,2})`?



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));

hokein wrote:
> gribozavr wrote:
> > Why do we print the warning if the fixit is not fixing anything?
> We may still want to inform users, these cases can still be converted to 
> `make_unique`, but the check currently is not smart enough to give correct 
> fixes.
Sorry I don't understand -- so what the file used to say, 
`std::make_unique(E{1, 2}, 1)`, is correct, and this patch introduces a 
regression?  Or is some other syntax necessary?

Please add FIXMEs anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: t-tye, b-sumner.
Herald added subscribers: tpr, dstuttard, yaxunl, nhaehnle, wdng, jvesely, 
kzhuravl.

The backend default maximum should be the hardware maximum, so the
frontend should set the implementation defined default maximum.


https://reviews.llvm.org/D62739

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenOpenCL/amdgpu-attrs.cl


Index: test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- test/CodeGenOpenCL/amdgpu-attrs.cl
+++ test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -143,6 +143,10 @@
 // CHECK: define void @a_function() [[A_FUNCTION:#[0-9]+]]
 }
 
+kernel void default_kernel() {
+// CHECK: define amdgpu_kernel void @default_kernel() 
[[DEFAULT_KERNEL_ATTRS:#[0-9]+]]
+}
+
 
 // Make sure this is silently accepted on other targets.
 // X86-NOT: "amdgpu-flat-work-group-size"
@@ -161,20 +165,20 @@
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_64_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="64,64" 
"amdgpu-implicitarg-num-bytes"="48"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_16_128]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="16,128" 
"amdgpu-implicitarg-num-bytes"="48"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[NUM_VGPR_64]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2,4"
+// CHECK-DAG: attributes [[NUM_SGPR_32]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32"
+// CHECK-DAG: attributes [[NUM_VGPR_64]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64"
 
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2,4"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_SGPR_32]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_VGPR_64]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_NUM_SGPR_32]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_NUM_VGPR_64]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32_NUM_VGPR_64]] = { convergent noinline 
nounwind optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32" 
"amdgpu-num-vgpr"="64"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32" 
"amdgpu-waves-per-eu"="2"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64" 
"amd

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Nikolaus Wittenstein via Phabricator via cfe-commits]

adzenith added a comment.

It appears I was mistaken - the `(?-i)` caused the regex match to fail 
entirely, which made the system header move all the way to the end. It didn't 
actually solve my problem.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33932



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

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61697



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

r362243 - Suppress nothrow/Exception spec conflict warning when we dont know the ES.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 09:46:38 2019
New Revision: 362243

URL: http://llvm.org/viewvc/llvm-project?rev=362243&view=rev
Log:
Suppress nothrow/Exception spec conflict warning when we dont know the ES.

In any situation where the Exception Spec isn't clear, suppress the
warning to avoid false positives.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=362243&r1=362242&r2=362243&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri May 31 09:46:38 2019
@@ -6976,18 +6976,18 @@ static bool handleFunctionTypeAttr(TypeP
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
   case EST_NoThrow:
-  case EST_Unparsed:
 // Exception spec doesn't conflict with nothrow, so don't warn.
-// Unparsed is included in this, since method signatures aren't parsed
-// until after the fact.
+LLVM_FALLTHROUGH;
+  case EST_Unparsed:
+  case EST_Uninstantiated:
+  case EST_DependentNoexcept:
+  case EST_Unevaluated:
+// We don't have enough information to properly determine if there is a
+// conflict, so suppress the warning.
 break;
-
   case EST_Dynamic:
   case EST_MSAny:
   case EST_NoexceptFalse:
-  case EST_DependentNoexcept:
-  case EST_Unevaluated:
-  case EST_Uninstantiated:
 S.Diag(attr.getLoc(), diag::warn_nothrow_attribute_ignored);
 break;
   }

Modified: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp?rev=362243&r1=362242&r2=362243&view=diff
==
--- cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp (original)
+++ cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp Fri May 31 09:46:38 
2019
@@ -54,6 +54,9 @@ __declspec(nothrow) void foo5() noexcept
 // expected-warning@+1{{'nothrow' attribute conflicts with exception 
specification; attribute ignored}}
 __declspec(nothrow) void foo6() noexcept(noexcept(foo3()));
 
+template
+__declspec(nothrow) void foo7() noexcept(noexcept(F()));
+
 // FIXME: It would be nice to be able to warn on these, however at the time we
 // evaluate the nothrow, these have yet to be parsed, so the data is not yet
 // there.


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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Very nice testing approach!




Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:20
+/// Provides notifications for file changes in a directory.
+
+/// Invokes client-provided function on every filesystem event in the watched

Looks like triple slashes on empty lines got removed, splitting the doc comment.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

Three slashes for doc comments?



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

gribozavr wrote:
> Three slashes for doc comments?
Don't write what something is used for currently, such comments go out of date 
quickly.  (Just delete the last sentence.)



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:41
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {
+  // Expecting two file-descriptors opened as a pipe in the canonical POSIX

Semaphor*e*



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:42
+struct SemaphorPipe {
+  // Expecting two file-descriptors opened as a pipe in the canonical POSIX
+  // order: pipefd[0] refers to the read end of the pipe. pipefd[1] refers to

"Expects"

Three slashes for doc comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:49
+assert(Result != -1);
+  };
+  ~SemaphorPipe() {

Extra semicolon.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:52
+close(FDWrite);
+close(FDRead);
+  }

Since it closes the file descriptors in the destructor, I feel like it should 
also be responsible for calling `pipe` in the constructor.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:58
+
+// Mutex-protected queue of Events.
+class EventQueue {

Three slashes for doc comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:120
+
+  // Consuming inotify events and pushing events to the Queue.
+  void InotifyPollingLoop();

"consumes", "pushes"



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:109
+
+  // This method is used by DirectoryWatcher
+  void consume(DirectoryWatcher::Event E, bool IsInitial) {

Please add a period at the end of the comment (everywhere in the patch).



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:153
+  // Not locking - caller has to lock Mtx
+  llvm::Optional Result() const {
+if (ExpectedInitial.empty() && ExpectedNonInitial.empty() &&

Please name functions consistently -- there's both `consume()` that starts with 
lowercase, and `Result()` that starts with uppercase.

Please refer to the current naming rules in the style guide and apply 
everywhere in the patch.





Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:210
+
+  // TestConsumer didn't reach the expected state in given time.
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==

"If the following assertions fail, it is a sign that ..."

Also you can stream the message into the EXPECT_TRUE, it will be printed if the 
assertion fails.

EXPECT_TRUE(...) << "whatever";



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:224
+
+TEST(DirectoryWatcherTest, initialScanSync) {
+  DirectoryWatcherTestFixture fixture;

Test names start with an uppercase letter (`InitialScanSync`).  Please apply 
everywhere in the patch.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:243
+  },
+  true);
+

Add /*waitForInitialSync=*/ ?  (everywhere in the patch)



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:304
+  {
+  {DirectoryWatcher::Event::EventKind::Modified, "a"},
+  }};

Delete the comma and wrap onto one line.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:419
+
+std::error_code setTimeRes = 
llvm::sys::fs::setLastAccessAndModificationTime(FD, NewTimePt,
+NewTimePt);

80 columns.


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

https://reviews.llvm.org/D58418



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Oh, this ticket is not going to die from neglect. It is true that D43515 
 is a higher priority, but I need this 
IRBuilder work done as well. My department head wanted it done by the end of 
_last_ year. It's not going to die.

How about I merge your changes into this ticket and we continue work over here? 
There is the issue of the documentation that lebedev.ri asked you to write. Can 
I talk you into putting that together and sending it to me 
? I'll work on the documentation that I was asked to write. 
Between the two of us we should be in pretty good shape. Does that work for you?

I am still waiting for feedback from an actual consumer of the IRBuilder who 
will be using this new functionality. If someone clang-side could chime in on 
this ticket I'd very much appreciate it.


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

https://reviews.llvm.org/D53157



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

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31
+  Result,
+  "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC",
+  ReplacementText);

hokein wrote:
> the message doesn't seem to explain the reason, especially to the people who 
> are not familiar with the `pipe` API, maybe `prefer pipe2() to pipe() to 
> avoid file descriptor leakage` is clearer?
> 
> Ah, it looks like you are following the existing `CloexecCreatCheck` check, 
> no need to do it in this patch. 
"prefer pipe2() with O_CLOEXEC to avoid leaking file descriptors to child 
processes"

No need to change in this patch if you're following an existing pattern, but 
please do update the message in all checks in a separate patch to explain 
things better.



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h:18
+
+/// pipe() is better to be replaced by pipe2().
+///

"Suggests to replace calls to pipe() with calls to pipe2()."



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:6
+
+Detects usage of ``pipe()``. The usage of ``pipe()`` is not recommended, it's 
better to use ``pipe2()``.
+Without this flag, an opened sensitive file descriptor would remain open across

WDYT about this rewrite?

"""
This check detects usage of pipe().  Using pipe() is not recommended, pipe2() 
is the suggested replacement.

The check also adds the O_CLOEXEC flag that marks the file descriptor to be 
closed in child processes.  Without this flag a sensitive file descriptor can 
be leaked to a child process, potentially into a lower-privileged SELinux 
domain.
"""



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:16
+
+  // becomes
+

"Suggested replacement"

Also use a separate code block for the replacement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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

r362245 - Replace 'default' in an enum-over-a-switch with the missing list.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 10:00:48 2019
New Revision: 362245

URL: http://llvm.org/viewvc/llvm-project?rev=362245&view=rev
Log:
Replace 'default' in an enum-over-a-switch with the missing list.

This suppressed the Wswitch warning causing me to miss it and write an
assertion failure.

Modified:
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp

Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=362245&r1=362244&r2=362245&view=diff
==
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Fri May 31 10:00:48 2019
@@ -429,7 +429,11 @@ bool Sema::CheckEquivalentExceptionSpec(
   case EST_NoThrow:
 OS <<"__attribute__((nothrow))";
 break;
-  default:
+  case EST_None:
+  case EST_MSAny:
+  case EST_Unevaluated:
+  case EST_Uninstantiated:
+  case EST_Unparsed:
 llvm_unreachable("This spec type is compatible with none.");
   }
 


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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:7885
+// By default, restrict the maximum size to 256.
+F->addFnAttr("amdgpu-flat-work-group-size", "128,256");
   }

Theoretically, shouldn't the minimum be 1?


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

https://reviews.llvm.org/D62739



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:6
+
+Checks if the required file flag ``O_CLOEXEC`` is present in the argument of
+``pipe2()``. ``pipe2()`` should include ``O_CLOEXEC`` in its type argument to

This checks ensures that pipe2() is called with the O_CLOEXEC flag.  This flag 
helps to avoid accidentally leaking file descriptors to child processes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:17
+
+  // becomes
+

"Suggested replacement"

Also use a separate code block for the replacement.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:48
+  pipe2(pipefd, O_NONBLOCK);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(pipe2(pipefd, O_NONBLOCK));

srhines wrote:
> Much like line 39 (which covers both lines 37 and 38), you can delete this 
> CHECK-MESSAGES-NOT. The one on line 50 will cover both of these.
Not done -- you don't need any CHECK-MESSAGES-NOT at all.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:20
+  pipe2(pipefd, O_NONBLOCK);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC 
where possible [android-cloexec-pipe2]
+  // CHECK-FIXES: pipe2(pipefd, O_NONBLOCK | O_CLOEXEC);

Same comment about the message as in D61967 -- the message should briefly 
explain why the user should make this change. Users tend to ignore warnings 
they don't understand.

"pipe2 should be called with O_CLOEXEC to avoid leaking file descriptors to 
child processes"



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:27
+
+void f() {
+  int pipefd[2];

Please give the tests informative names instead of `a`, `f`, `g` etc.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54
+
+void e() {
+  int pipefd[2];

How is `e` different from `a`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Return type is not mentioned explicitly, so auto should not be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert removed a reviewer: bkramer.
aaronpuchert added a comment.

I discovered an issue with variables, because there can be multiple definitions 
with the same `TypeLoc`:

  extern int a;
  int a, b;

We emit the warning for `b`, but we can't just put static in front of the 
declaration, as that would affect `a` as well. I haven't found a way to 
determine whether a `VarDecl` shares its type with another declaration, 
especially since global declarations don't even belong to a `DeclStmt`. We just 
get

  TranslationUnitDecl 0x1d0b598 <> 
  |-...
  |-VarDecl 0x1d47b60 <:1:1, col:12> col:12 a 'int' extern
  |-VarDecl 0x1d47c18 prev 0x1d47b60  col:5 a 'int'
  `-VarDecl 0x1d47c90  col:8 b 'int'

So I think I have to drop the fix-it hint there, but I can still emit a note.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D53157#1525311 , @kpn wrote:

> Oh, this ticket is not going to die from neglect. It is true that D43515 
>  is a higher priority, but I need this 
> IRBuilder work done as well. My department head wanted it done by the end of 
> _last_ year. It's not going to die.
>
> How about I merge your changes into this ticket and we continue work over 
> here? There is the issue of the documentation that lebedev.ri asked you to 
> write. Can I talk you into putting that together and sending it to me 
> ? I'll work on the documentation that I was asked to 
> write. Between the two of us we should be in pretty good shape. Does that 
> work for you?


Yes I'll do that. Thanks.

> I am still waiting for feedback from an actual consumer of the IRBuilder who 
> will be using this new functionality. If someone clang-side could chime in on 
> this ticket I'd very much appreciate it.

I wrote a clang patch that works with your IRBuilder modifications.  I also 
checked with the Intel fortran team and they think this interface will be 
workable for them too. From Intel perspective it's +1.


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

https://reviews.llvm.org/D53157



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Matt Arsenault via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/TargetInfo.cpp:7885
+// By default, restrict the maximum size to 256.
+F->addFnAttr("amdgpu-flat-work-group-size", "128,256");
   }

b-sumner wrote:
> Theoretically, shouldn't the minimum be 1?
That's what I thought, but the backend is defaulting to 2 * wave size now


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

https://reviews.llvm.org/D62739



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

r362249 - [analyzer] print() JSONify: ExplodedNode revision

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Fri May 31 10:54:12 2019
New Revision: 362249

URL: http://llvm.org/viewvc/llvm-project?rev=362249&view=rev
Log:
[analyzer] print() JSONify: ExplodedNode revision

Summary: Revert node-ID removal.

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp,
 cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=362249&r1=362248&r2=362249&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri May 31 10:54:12 2019
@@ -3075,8 +3075,8 @@ struct DOTGraphTraits :
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
-Out << "{ \"node_id\": \"" << (const void *)N
-<< "\", \"state_id\": " << State->getID()
+Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
+<< (const void *)N << "\", \"state_id\": " << State->getID()
 << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
 << ",\\l";
 
@@ -3094,7 +3094,7 @@ struct DOTGraphTraits :
   else
 Out << "null }";
 },
-   // Adds a comma and a new-line between each program point.
+// Adds a comma and a new-line between each program point.
 [&](const ExplodedNode *) { Out << ",\\l"; },
 [&](const ExplodedNode *) { return false; });
 


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

[PATCH] D62658: [analyzer] print() JSONify: ExplodedNode revision

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362249: [analyzer] print() JSONify: ExplodedNode revision 
(authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62658?vs=202173&id=202462#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62658

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3075,8 +3075,8 @@
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
-Out << "{ \"node_id\": \"" << (const void *)N
-<< "\", \"state_id\": " << State->getID()
+Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
+<< (const void *)N << "\", \"state_id\": " << State->getID()
 << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
 << ",\\l";
 
@@ -3094,7 +3094,7 @@
   else
 Out << "null }";
 },
-   // Adds a comma and a new-line between each program point.
+// Adds a comma and a new-line between each program point.
 [&](const ExplodedNode *) { Out << ",\\l"; },
 [&](const ExplodedNode *) { return false; });
 


Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3075,8 +3075,8 @@
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
-Out << "{ \"node_id\": \"" << (const void *)N
-<< "\", \"state_id\": " << State->getID()
+Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
+<< (const void *)N << "\", \"state_id\": " << State->getID()
 << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
 << ",\\l";
 
@@ -3094,7 +3094,7 @@
   else
 Out << "null }";
 },
-	// Adds a comma and a new-line between each program point.
+// Adds a comma and a new-line between each program point.
 [&](const ExplodedNode *) { Out << ",\\l"; },
 [&](const ExplodedNode *) { return false; });
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62279: Use LTO capable linker

[Chris Bieneman via Phabricator via cfe-commits]

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

This is good to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62279



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Eugene.Zelenko wrote:
> Return type is not mentioned explicitly, so auto should not be used.
An explicit type is not needed for readability here.  The rule is to use auto 
when it improves readability, not when the type is not spelled in immediate 
vicinity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

gribozavr wrote:
> Eugene.Zelenko wrote:
> > Return type is not mentioned explicitly, so auto should not be used.
> An explicit type is not needed for readability here.  The rule is to use auto 
> when it improves readability, not when the type is not spelled in immediate 
> vicinity.
I think it's reasonable to follow modernize-use-auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

jkorous marked 20 inline comments as done.
jkorous added inline comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

gribozavr wrote:
> gribozavr wrote:
> > Three slashes for doc comments?
> Don't write what something is used for currently, such comments go out of 
> date quickly.  (Just delete the last sentence.)
Sure, no problem.

I naturally tend to err on the side of over-commenting as I think I'd 
appreciate it myself if I had to understand the code without prior knowledge - 
not saying it's intentional or that I have a strong reason to do that though. 
You seem to have a strong preference for not having out-of-date comments with 
low-ish information value. Just out of curiosity - is it based on any 
particular reason or experience?



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:52
+close(FDWrite);
+close(FDRead);
+  }

gribozavr wrote:
> Since it closes the file descriptors in the destructor, I feel like it should 
> also be responsible for calling `pipe` in the constructor.
I know what you mean - my "oop feel" was telling me it's wrong too. It's not 
just the pipe in this class but also the inotify descriptor in the watcher 
class.

The problem is that the only reasonable way how to communicate failures from 
constructors that I am aware of are exceptions which we don't use in llvm. 
That's why I moved most of the stuff that can fail even before any work is 
actually started to the factory method (with the exception of epoll file 
descriptor as that felt like making its scope unnecessarily larger).

Thinking about it now, I am starting to doubt that it makes life any easier for 
client code as it still has to cope with failure communicated as 
WatcherGotInvalidated event via receiver.

What do you think?



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:153
+  // Not locking - caller has to lock Mtx
+  llvm::Optional Result() const {
+if (ExpectedInitial.empty() && ExpectedNonInitial.empty() &&

gribozavr wrote:
> Please name functions consistently -- there's both `consume()` that starts 
> with lowercase, and `Result()` that starts with uppercase.
> 
> Please refer to the current naming rules in the style guide and apply 
> everywhere in the patch.
> 
> 
Sorry about that. This is definitely my weak point.


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

https://reviews.llvm.org/D58418



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 202467.
jkorous marked 2 inline comments as done.
jkorous added a comment.

Addressed comments.


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

https://reviews.llvm.org/D58418

Files:
  clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
  clang/lib/CMakeLists.txt
  clang/lib/DirectoryWatcher/CMakeLists.txt
  clang/lib/DirectoryWatcher/DirectoryScanner.cpp
  clang/lib/DirectoryWatcher/DirectoryScanner.h
  clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
  clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/DirectoryWatcher/CMakeLists.txt
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- /dev/null
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -0,0 +1,426 @@
+//===- unittests/DirectoryWatcher/DirectoryWatcherTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/DirectoryWatcher/DirectoryWatcher.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Mutex.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace llvm::sys;
+using namespace llvm::sys::fs;
+using namespace clang;
+
+namespace clang {
+static bool operator==(const DirectoryWatcher::Event &lhs,
+   const DirectoryWatcher::Event &rhs) {
+  return lhs.Filename == rhs.Filename &&
+ static_cast(lhs.Kind) == static_cast(rhs.Kind);
+}
+} // namespace clang
+
+namespace {
+
+struct DirectoryWatcherTestFixture {
+  std::string TestRootDir;
+  std::string TestWatchedDir;
+
+  DirectoryWatcherTestFixture() {
+SmallString<128> pathBuf;
+std::error_code UniqDirRes = createUniqueDirectory("dirwatcher", pathBuf);
+assert(!UniqDirRes);
+TestRootDir = pathBuf.str();
+path::append(pathBuf, "watch");
+TestWatchedDir = pathBuf.str();
+std::error_code CreateDirRes = create_directory(TestWatchedDir, false);
+assert(!CreateDirRes);
+  }
+
+  ~DirectoryWatcherTestFixture() { remove_directories(TestRootDir); }
+
+  SmallString<128> getPathInWatched(const std::string &testFile) {
+SmallString<128> pathBuf;
+pathBuf = TestWatchedDir;
+path::append(pathBuf, testFile);
+return pathBuf;
+  }
+
+  void addFile(const std::string &testFile) {
+Expected ft = openNativeFileForWrite(getPathInWatched(testFile),
+ CD_CreateNew, OF_None);
+if (ft) {
+  closeFile(*ft);
+} else {
+  llvm::errs() << llvm::toString(ft.takeError()) << "\n";
+  llvm::errs() << getPathInWatched(testFile) << "\n";
+  llvm_unreachable("Couldn't create test file.");
+}
+  }
+
+  void deleteFile(const std::string &testFile) {
+std::error_code EC =
+remove(getPathInWatched(testFile), /*IgnoreNonExisting=*/false);
+ASSERT_FALSE(EC);
+  }
+};
+
+std::string eventKindToString(const DirectoryWatcher::Event::EventKind K) {
+  switch (K) {
+  case DirectoryWatcher::Event::EventKind::Removed:
+return "Removed";
+  case DirectoryWatcher::Event::EventKind::Modified:
+return "Modified";
+  case DirectoryWatcher::Event::EventKind::WatchedDirRemoved:
+return "WatchedDirRemoved";
+  case DirectoryWatcher::Event::EventKind::WatcherGotInvalidated:
+return "WatcherGotInvalidated";
+  }
+  llvm_unreachable("unknown event kind");
+}
+
+struct VerifyingConsumer {
+  std::vector ExpectedInitial;
+  std::vector ExpectedNonInitial;
+  std::vector OptionalNonInitial;
+  std::vector UnexpectedInitial;
+  std::vector UnexpectedNonInitial;
+  std::mutex Mtx;
+  std::condition_variable ResultIsReady;
+
+  VerifyingConsumer(
+  const std::vector &ExpectedInitial,
+  const std::vector &ExpectedNonInitial,
+  const std::vector &OptionalNonInitial = {})
+  : ExpectedInitial(ExpectedInitial),
+ExpectedNonInitial(ExpectedNonInitial),
+OptionalNonInitial(OptionalNonInitial) {}
+
+  // This method is used by DirectoryWatcher.
+  void consume(DirectoryWatcher::Event E, bool IsInitial) {
+if (IsInitial)
+  consumeInitial(E);
+else
+  consumeNonInitial(E);
+  }
+
+  void consumeInitial(DirectoryWatcher::Event E) {
+std::unique_lock L(Mtx);
+auto It = std::find(ExpectedInitial.begin(), ExpectedInitial.end(), E);
+if (It == ExpectedInitial.end()) {
+  UnexpectedInitial.push_back(E);
+} else {
+  ExpectedInitial.erase(It);
+}
+if (result())
+  ResultIsReady.notify_one();
+  }

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Something I ran into when reviewing https://reviews.llvm.org/D62639 is that on 
AArch64, for varargs functions, we emit floating-point stores when 
noimplicitfloat is specified.  That seems fine for -mno-implicit-float, but 
maybe not for -mgeneral-regs-only?


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

https://reviews.llvm.org/D38479



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

jkorous wrote:
> gribozavr wrote:
> > gribozavr wrote:
> > > Three slashes for doc comments?
> > Don't write what something is used for currently, such comments go out of 
> > date quickly.  (Just delete the last sentence.)
> Sure, no problem.
> 
> I naturally tend to err on the side of over-commenting as I think I'd 
> appreciate it myself if I had to understand the code without prior knowledge 
> - not saying it's intentional or that I have a strong reason to do that 
> though. You seem to have a strong preference for not having out-of-date 
> comments with low-ish information value. Just out of curiosity - is it based 
> on any particular reason or experience?
> is it based on any particular reason or experience?

Yes, primarily working on the Swift compiler and the standard library, when 
everything was changing very quickly.  My current work also confirms the same 
-- a lot of time when I see a comment about the "current usage" or other 
incidental information that is not the contract of the API, it tends to be 
outdated.  Approximately nobody will change such a comment when another user is 
added ("I'm just reusing the code..."), even when the quoted user already 
became non-representative of the usage pattern, or the usage pattern has 
changed.  However, when changing the API contract people typically do change 
the comment.

It also makes sense to me in abstract: reading that X happens to be used for Y 
does not necessarily help understand X better -- it is only a cross-reference 
that I could find myself with an IDE command; I still need to understand the 
design of Y and the interaction with X, and then using my past experience infer 
what X was intended to be.

Saying that X is intended to be used only by Y is a different story of course, 
that's design documentation.

Providing an example of usage is also fine, but it should be phrased as an 
example that can't become stale.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:52
+close(FDWrite);
+close(FDRead);
+  }

jkorous wrote:
> gribozavr wrote:
> > Since it closes the file descriptors in the destructor, I feel like it 
> > should also be responsible for calling `pipe` in the constructor.
> I know what you mean - my "oop feel" was telling me it's wrong too. It's not 
> just the pipe in this class but also the inotify descriptor in the watcher 
> class.
> 
> The problem is that the only reasonable way how to communicate failures from 
> constructors that I am aware of are exceptions which we don't use in llvm. 
> That's why I moved most of the stuff that can fail even before any work is 
> actually started to the factory method (with the exception of epoll file 
> descriptor as that felt like making its scope unnecessarily larger).
> 
> Thinking about it now, I am starting to doubt that it makes life any easier 
> for client code as it still has to cope with failure communicated as 
> WatcherGotInvalidated event via receiver.
> 
> What do you think?
You could add a factory function to SemaphorePipe, but... I feel like trying to 
recover from a failure in the pipe() call is a bit like trying to recover from 
a memory allocation failure.  The process is probably hitting a file descriptor 
limit or something like that, and is likely going to fail anyway.  I'd probably 
trigger a fatal error if pipe() fails.

This class is a lot like pthread_mutex_init -- it can fail "gracefully", but 
there's no way for the caller to recover -- the caller needs a mutex to proceed.


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

https://reviews.llvm.org/D58418



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:317
+
+  if (pipe(InotifyPollingStopperFDs) == -1)
+return nullptr;

Use pipe2() with O_CLOEXEC, to avoid leaking the file descriptors to child 
processes?


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

https://reviews.llvm.org/D58418



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Eugene.Zelenko wrote:
> gribozavr wrote:
> > Eugene.Zelenko wrote:
> > > Return type is not mentioned explicitly, so auto should not be used.
> > An explicit type is not needed for readability here.  The rule is to use 
> > auto when it improves readability, not when the type is not spelled in 
> > immediate vicinity.
> I think it's reasonable to follow modernize-use-auto.
modernize-use-auto is only a heuristic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:17
+  pipe(pipefd);
+  // CHECK-MESSAGES-NOT: warning:
+}

hokein wrote:
> nit: no need to do it explicitly, if a warning is shown unexpectedly, the 
> test will fail.
I somehow never realized this (and there seem to be several places that also 
don't realize it). Thanks for letting us know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54
+
+void e() {
+  int pipefd[2];

gribozavr wrote:
> How is `e` different from `a`?
`e` uses O_CLOEXEC properly with `pipe2()` and makes sure that we don't issue 
additional diagnostics/fixes that are unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62746: msabi: Fix exponential mangling time in certain pathological inputs

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.

Template back references used to be recursively recomputed, add a
memoization cache to cut down on this.

Since there are now two different types of argument maps, rename the
existing TypeBackReferences to FunArgBackReferences, and rename
mangleArgumentType() to mangleFunctionArgumentType().

Fixes PR42091, the input there now takes 50ms instead of 7s to compile.

No intended behavior change.


https://reviews.llvm.org/D62746

Files:
  clang/lib/AST/MicrosoftMangle.cpp

Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -265,7 +265,8 @@
   BackRefVec NameBackReferences;
 
   typedef llvm::DenseMap ArgBackRefMap;
-  ArgBackRefMap TypeBackReferences;
+  ArgBackRefMap FunArgBackReferences;
+  ArgBackRefMap TemplateArgBackReferences;
 
   typedef std::set> PassObjectSizeArgsSet;
   PassObjectSizeArgsSet PassObjectSizeArgs;
@@ -343,7 +344,7 @@
   const TemplateArgumentList &TemplateArgs);
   void mangleObjCMethodName(const ObjCMethodDecl *MD);
 
-  void mangleArgumentType(QualType T, SourceRange Range);
+  void mangleFunctionArgumentType(QualType T, SourceRange Range);
   void manglePassObjectSizeArg(const PassObjectSizeAttr *POSA);
 
   bool isArtificialTagType(QualType T) const;
@@ -793,7 +794,7 @@
 // the X part is aliased. However, if you need to mangle
 //   void foo(A::X, A::X),
 // the A::X<> part is not aliased.
-// That said, from the mangler's perspective we have a structure like this:
+// That is, from the mangler's perspective we have a structure like this:
 //   namespace[s] -> type[ -> template-parameters]
 // but from the Clang perspective we have
 //   type [ -> template-parameters]
@@ -803,12 +804,30 @@
 // the mangled type name as a key to check the mangling of different types
 // for aliasing.
 
-llvm::SmallString<64> TemplateMangling;
-llvm::raw_svector_ostream Stream(TemplateMangling);
-MicrosoftCXXNameMangler Extra(Context, Stream);
-Extra.mangleTemplateInstantiationName(TD, *TemplateArgs);
-
-mangleSourceName(TemplateMangling);
+// It's important to key cache reads off ND, not TD -- the same TD can
+// be used with different TemplateArgs, but ND uniquely identifies
+// TD / TemplateArg pairs.
+ArgBackRefMap::iterator Found = TemplateArgBackReferences.find(ND);
+if (Found == TemplateArgBackReferences.end()) {
+  // Mangle full template name into temporary buffer.
+  llvm::SmallString<64> TemplateMangling;
+  llvm::raw_svector_ostream Stream(TemplateMangling);
+  MicrosoftCXXNameMangler Extra(Context, Stream);
+  Extra.mangleTemplateInstantiationName(TD, *TemplateArgs);
+
+  // Use the string backref vector to possibly get a back reference.
+  mangleSourceName(TemplateMangling);
+
+  // Memoize back reference for this type.
+  BackRefVec::iterator StringFound =
+  llvm::find(NameBackReferences, TemplateMangling);
+  if (StringFound != NameBackReferences.end()) {
+TemplateArgBackReferences[ND] =
+StringFound - NameBackReferences.begin();
+  }
+} else {
+  Out << Found->second;
+}
 return;
   }
 
@@ -1282,11 +1301,13 @@
   // Always start with the unqualified name.
 
   // Templates have their own context for back references.
-  ArgBackRefMap OuterArgsContext;
+  ArgBackRefMap OuterFunArgsContext;
+  ArgBackRefMap OuterTemplateArgsContext;
   BackRefVec OuterTemplateContext;
   PassObjectSizeArgsSet OuterPassObjectSizeArgs;
   NameBackReferences.swap(OuterTemplateContext);
-  TypeBackReferences.swap(OuterArgsContext);
+  FunArgBackReferences.swap(OuterFunArgsContext);
+  TemplateArgBackReferences.swap(OuterTemplateArgsContext);
   PassObjectSizeArgs.swap(OuterPassObjectSizeArgs);
 
   mangleUnscopedTemplateName(TD);
@@ -1294,7 +1315,8 @@
 
   // Restore the previous back reference contexts.
   NameBackReferences.swap(OuterTemplateContext);
-  TypeBackReferences.swap(OuterArgsContext);
+  FunArgBackReferences.swap(OuterFunArgsContext);
+  TemplateArgBackReferences.swap(OuterTemplateArgsContext);
   PassObjectSizeArgs.swap(OuterPassObjectSizeArgs);
 }
 
@@ -1699,8 +1721,8 @@
   }
 }
 
-void MicrosoftCXXNameMangler::mangleArgumentType(QualType T,
- SourceRange Range) {
+void MicrosoftCXXNameMangler::mangleFunctionArgumentType(QualType T,
+ SourceRange Range) {
   // MSVC will backreference two canonically equivalent types that have slightly
   // different manglings when mangled alone.
 
@@ -1730,9 +1752,9 @@
 TypePtr = T.getCanonicalType().getAsOpaquePtr();
   }
 
-  ArgBackRefMap::iterator Found = TypeBackReferences.find(TypePtr);
+  ArgBackRefMap::iterator Found = FunArgBackReferences.find(TypePt

[PATCH] D62746: msabi: Fix exponential mangling time for certain pathological inputs

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

Thanks for this, I remember a user complained about this a long time ago but we 
never did anything.


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

https://reviews.llvm.org/D62746



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

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There was a search for non-prototype declarations for the function, but
we only showed the results for zero-parameter functions. Now we show the
note for functions with parameters as well, but we omit the fix-it hint
suggesting to add `void`.


Repository:
  rC Clang

https://reviews.llvm.org/D62750

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c

Index: test/Sema/warn-missing-prototypes.c
===
--- test/Sema/warn-missing-prototypes.c
+++ test/Sema/warn-missing-prototypes.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wmissing-prototypes -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
-int f();
+int f(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
 
 int f(int x) { return x; } // expected-warning{{no previous prototype for function 'f'}}
 
@@ -15,7 +15,7 @@
 
 void test(void);
 
-int h3();
+int h3(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
 int h4(int);
 int h4();
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12747,8 +12747,9 @@
   Consumer.HandleInlineFunctionDefinition(D);
 }
 
-static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
- const FunctionDecl*& PossibleZeroParamPrototype) {
+static bool
+ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
+const FunctionDecl *&PossiblePrototype) {
   // Don't warn about invalid declarations.
   if (FD->isInvalidDecl())
 return false;
@@ -12785,7 +12786,6 @@
   if (FD->isDeleted())
 return false;
 
-  bool MissingPrototype = true;
   for (const FunctionDecl *Prev = FD->getPreviousDecl();
Prev; Prev = Prev->getPreviousDecl()) {
 // Ignore any declarations that occur in function or method
@@ -12793,13 +12793,11 @@
 if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
   continue;
 
-MissingPrototype = !Prev->getType()->isFunctionProtoType();
-if (FD->getNumParams() == 0)
-  PossibleZeroParamPrototype = Prev;
-break;
+PossiblePrototype = Prev;
+return Prev->getType()->isFunctionNoProtoType();
   }
 
-  return MissingPrototype;
+  return true;
 }
 
 void
@@ -13319,21 +13317,22 @@
 //   prototype declaration. This warning is issued even if the
 //   definition itself provides a prototype. The aim is to detect
 //   global functions that fail to be declared in header files.
-const FunctionDecl *PossibleZeroParamPrototype = nullptr;
-if (ShouldWarnAboutMissingPrototype(FD, PossibleZeroParamPrototype)) {
+const FunctionDecl *PossiblePrototype = nullptr;
+if (ShouldWarnAboutMissingPrototype(FD, PossiblePrototype)) {
   Diag(FD->getLocation(), diag::warn_missing_prototype) << FD;
 
-  if (PossibleZeroParamPrototype) {
+  if (PossiblePrototype) {
 // We found a declaration that is not a prototype,
 // but that could be a zero-parameter prototype
-if (TypeSourceInfo *TI =
-PossibleZeroParamPrototype->getTypeSourceInfo()) {
+if (TypeSourceInfo *TI = PossiblePrototype->getTypeSourceInfo()) {
   TypeLoc TL = TI->getTypeLoc();
   if (FunctionNoProtoTypeLoc FTL = TL.getAs())
-Diag(PossibleZeroParamPrototype->getLocation(),
+Diag(PossiblePrototype->getLocation(),
  diag::note_declaration_not_a_prototype)
-<< PossibleZeroParamPrototype
-<< FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
+<< (FD->getNumParams() != 0)
+<< (FD->getNumParams() == 0
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void")
+: FixItHint{});
 }
   }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4652,7 +4652,8 @@
   "no previous prototype for function %0">,
   InGroup>, DefaultIgnore;
 def note_declaration_not_a_prototype : Note<
-  "this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function">;
+  "this declaration is not a prototype; add %select{'void'|parameter declarations}0 "
+  "to make it %select{a prototype for a zero-parameter function|one}0">;
 def warn_strict_prototypes : Warning<
   "this %select{function declaration is not|block declaration is not|"
   "old-style funct

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:7885
+// By default, restrict the maximum size to 256.
+F->addFnAttr("amdgpu-flat-work-group-size", "128,256");
   }

arsenm wrote:
> b-sumner wrote:
> > Theoretically, shouldn't the minimum be 1?
> That's what I thought, but the backend is defaulting to 2 * wave size now
I don't get it. This attribute indicates the possible workgroup size range this 
kernel may be run with, right? It only depends on how user execute the kernel. 
How is it related to backend defaults?


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

https://reviews.llvm.org/D62739



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

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I've factored out some changes into D62750  
and will rebase this on top if it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 202479.
aaronpuchert added a comment.
This revision is now accepted and ready to land.

Show note suggesting to add `static`, move fix-it to note for functions, remove 
fix-it for variables. Show note even for `extern` function definitions when we 
can't build a proper fix-it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-prototypes.cpp
  test/SemaCXX/warn-missing-variable-declarations.cpp
  test/SemaOpenCL/warn-missing-prototypes.cl

Index: test/SemaOpenCL/warn-missing-prototypes.cl
===
--- test/SemaOpenCL/warn-missing-prototypes.cl
+++ test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@
 template int var_template;
 extern template int var_template;
 template<> int var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
Index: test/SemaCXX/warn-missing-prototypes.cpp
===
--- test/SemaCXX/warn-missing-prototypes.cpp
+++ test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,6 @@
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
Index: test/Sema/

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

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

Remove other change from this one.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-prototypes.cpp
  test/SemaCXX/warn-missing-variable-declarations.cpp
  test/SemaOpenCL/warn-missing-prototypes.cl

Index: test/SemaOpenCL/warn-missing-prototypes.cl
===
--- test/SemaOpenCL/warn-missing-prototypes.cl
+++ test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@
 template int var_template;
 extern template int var_template;
 template<> int var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
Index: test/SemaCXX/warn-missing-prototypes.cpp
===
--- test/SemaCXX/warn-missing-prototypes.cpp
+++ test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,6 @@
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
Index: test/Sema/warn-missing-variable-declarations.c
===
--- test/Sema/warn-missing-variable-declarations.c
+++ test/Sema/warn-missing-variable-declarations

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

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

Add missing CHECK-NOTs for -Wmissing-prototypes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-prototypes.cpp
  test/SemaCXX/warn-missing-variable-declarations.cpp
  test/SemaOpenCL/warn-missing-prototypes.cl

Index: test/SemaOpenCL/warn-missing-prototypes.cl
===
--- test/SemaOpenCL/warn-missing-prototypes.cl
+++ test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@
 template int var_template;
 extern template int var_template;
 template<> int var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
Index: test/SemaCXX/warn-missing-prototypes.cpp
===
--- test/SemaCXX/warn-missing-prototypes.cpp
+++ test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,7 @@
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:{{.*}}-[[@LINE-2]]:{{.*}}}:"{{.*}}"
Index: test/Sema/warn-missing-variable-declarations.c
===
--- t

r362266 - Clarify when fix-it hints on warnings are appropriate

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Fri May 31 14:27:39 2019
New Revision: 362266

URL: http://llvm.org/viewvc/llvm-project?rev=362266&view=rev
Log:
Clarify when fix-it hints on warnings are appropriate

Summary:
This is not a change in the rules, it's meant as a clarification about
warnings. Since the recovery from warnings is a no-op, the fix-it hints
on warnings shouldn't change anything. Anything that doesn't just
suppress the warning and changes the meaning of the code (even if it's
for the better) should be on an additional note.

Reviewers: rsmith, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: cfe-commits, thakis

Tags: #clang

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

Modified:
cfe/trunk/docs/InternalsManual.rst

Modified: cfe/trunk/docs/InternalsManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/InternalsManual.rst?rev=362266&r1=362265&r2=362266&view=diff
==
--- cfe/trunk/docs/InternalsManual.rst (original)
+++ cfe/trunk/docs/InternalsManual.rst Fri May 31 14:27:39 2019
@@ -423,6 +423,9 @@ Fix-it hints on errors and warnings need
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.


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

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362266: Clarify when fix-it hints on warnings are 
appropriate (authored by aaronpuchert, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62470

Files:
  cfe/trunk/docs/InternalsManual.rst


Index: cfe/trunk/docs/InternalsManual.rst
===
--- cfe/trunk/docs/InternalsManual.rst
+++ cfe/trunk/docs/InternalsManual.rst
@@ -423,6 +423,9 @@
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.


Index: cfe/trunk/docs/InternalsManual.rst
===
--- cfe/trunk/docs/InternalsManual.rst
+++ cfe/trunk/docs/InternalsManual.rst
@@ -423,6 +423,9 @@
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62752: Move VarBypassDector.h to include/clang/CodeGen

[Zachary Henkel via Phabricator via cfe-commits]

zahen created this revision.
zahen added a reviewer: chandlerc.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Allow the VarBypassDetector class to be consumed by external tools by exposing 
its header from a standard include path.

If accepted, I will need a user with check-in permission to apply the patch.


Repository:
  rC Clang

https://reviews.llvm.org/D62752

Files:
  clang/include/clang/CodeGen/VarBypassDetector.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/VarBypassDetector.cpp
  clang/lib/CodeGen/VarBypassDetector.h


Index: clang/lib/CodeGen/VarBypassDetector.cpp
===
--- clang/lib/CodeGen/VarBypassDetector.cpp
+++ clang/lib/CodeGen/VarBypassDetector.cpp
@@ -6,7 +6,7 @@
 //
 
//===--===//
 
-#include "VarBypassDetector.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -20,7 +20,6 @@
 #include "CodeGenModule.h"
 #include "CodeGenPGO.h"
 #include "EHScopeStack.h"
-#include "VarBypassDetector.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
 #include "clang/AST/ExprCXX.h"
@@ -32,6 +31,7 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
Index: clang/include/clang/CodeGen/VarBypassDetector.h
===
--- clang/include/clang/CodeGen/VarBypassDetector.h
+++ clang/include/clang/CodeGen/VarBypassDetector.h
@@ -11,8 +11,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
-#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+#ifndef LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
+#define LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
 
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"


Index: clang/lib/CodeGen/VarBypassDetector.cpp
===
--- clang/lib/CodeGen/VarBypassDetector.cpp
+++ clang/lib/CodeGen/VarBypassDetector.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "VarBypassDetector.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -20,7 +20,6 @@
 #include "CodeGenModule.h"
 #include "CodeGenPGO.h"
 #include "EHScopeStack.h"
-#include "VarBypassDetector.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
 #include "clang/AST/ExprCXX.h"
@@ -32,6 +31,7 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
Index: clang/include/clang/CodeGen/VarBypassDetector.h
===
--- clang/include/clang/CodeGen/VarBypassDetector.h
+++ clang/include/clang/CodeGen/VarBypassDetector.h
@@ -11,8 +11,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
-#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+#ifndef LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
+#define LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
 
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54
+
+void e() {
+  int pipefd[2];

srhines wrote:
> gribozavr wrote:
> > How is `e` different from `a`?
> `e` uses O_CLOEXEC properly with `pipe2()` and makes sure that we don't issue 
> additional diagnostics/fixes that are unnecessary.
Thanks -- I see it now.  I think a descriptive test name would help here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

gribozavr wrote:
> Eugene.Zelenko wrote:
> > gribozavr wrote:
> > > Eugene.Zelenko wrote:
> > > > Return type is not mentioned explicitly, so auto should not be used.
> > > An explicit type is not needed for readability here.  The rule is to use 
> > > auto when it improves readability, not when the type is not spelled in 
> > > immediate vicinity.
> > I think it's reasonable to follow modernize-use-auto.
> modernize-use-auto is only a heuristic.
But set of processed situations are very reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62754: [analyzer] Fix JSON dumps for location contexts.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

`lctx_id` is a property of a location context, not of an item within it. It's 
useful to know the id even when there are no items in the context.


Repository:
  rC Clang

https://reviews.llvm.org/D62754

Files:
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c


Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -30,8 +30,8 @@
 // CHECK-NEXT: ]}
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "environment": [
-// CHECK-NEXT: { "location_context": "#0 Call", "calling": "foo", 
"call_line": null, "items": [
-// CHECK-NEXT:   { "lctx_id": 1, "stmt_id": {{[0-9]+}}, "pretty": 
"clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" }
+// CHECK-NEXT: { "lctx_id": 1, "location_context": "#0 Call", "calling": 
"foo", "call_line": null, "items": [
+// CHECK-NEXT:   { "stmt_id": {{[0-9]+}}, "pretty": 
"clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" }
 // CHECK-NEXT: ]}
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "constraints": [
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -18,9 +18,9 @@
   new S;
 }
 
-// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"foo\", 
\"call_line\": null, \"items\": 
[\l\{ \"lctx_id\": 1, 
\"stmt_id\": {{[0-9]+}}, \"kind\": \"construct into local variable\", 
\"argument_index\": null, \"pretty\": \"T t;\", \"value\": \"&t\"
+// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"foo\", 
\"call_line\": null, \"items\": 
[\l\{ \"stmt_id\": {{[0-9]+}}, 
\"kind\": \"construct into local variable\", \"argument_index\": null, 
\"pretty\": \"T t;\", \"value\": \"&t\"
 
-// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"T::T\", 
\"call_line\": \"16\", \"items\": 
[\l\{ \"lctx_id\": 2, 
\"init_id\": {{[0-9]+}}, \"kind\": \"construct into member variable\", 
\"argument_index\": null, \"pretty\": \"s\", \"value\": \"&t-\>s\"
+// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"T::T\", 
\"call_line\": \"16\", \"items\": 
[\l\{ \"init_id\": {{[0-9]+}}, 
\"kind\": \"construct into member variable\", \"argument_index\": null, 
\"pretty\": \"s\", \"value\": \"&t-\>s\"
 
 // CHECK: \"cluster\": \"t\", \"items\": 
[\l\{ \"kind\": \"Default\", 
\"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -149,9 +149,6 @@
 if (!S)
   I = getItem().getCXXCtorInitializer();
 
-// IDs
-Out << "\"lctx_id\": " << getLocationContext()->getID() << ", ";
-
 if (S)
   Out << "\"stmt_id\": " << S->getID(getASTContext());
 else
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -261,8 +261,7 @@
 
   const Stmt *S = I->first.getStmt();
   Indent(Out, InnerSpace, IsDot)
-  << "{ \"lctx_id\": " << LC->getID()
-  << ", \"stmt_id\": " << S->getID(Ctx) << ", \"pretty\": ";
+  << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"pretty\": ";
   S->printJson(Out, nullptr, PP, /*AddQuotes=*/true);
 
   Out << ", \"value\": ";
Index: clang/lib/Analysis/AnalysisDeclContext.cpp
===
--- clang/lib/Analysis/AnalysisDeclContext.cpp
+++ clang/lib/Analysis/AnalysisDeclContext.cpp
@@ -527,7 +527,8 @@
 
   unsigned Frame = 0;
   for (const LocationContext *LCtx = this; LCtx; LCtx = LCtx->getParent()) {
-Indent(Out, Space, IsDot) << "{ \"location_context\": \"";
+Indent(Out, Space, IsDot)
+<< "{ \"lctx_id\": " << LCtx->getID() << ", \"location_context\": \"";
 switch (LCtx->getKind()) {
 case StackFrame:
   Out << '#' << Frame << " Call\", \"calling\": \"";
@@ -541,7 +542,7 @@
   if (const Stmt *S = cast(LCtx)->getCallSite()) {
 Out << '\"';
 printLocation(Out, SM, S->getBeginLoc());
-   Out << '\"';
+Out << '\"';
   } else {
 Out << "null";
   }


Index: clang/test/Analysis/expr-inspection.c
=

[PATCH] D62754: [analyzer] Fix JSON dumps for location contexts.

[Csaba Dabis via Phabricator via cfe-commits]

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

That was my feeling as well, but it has been defined like that. Thanks for the 
clarification!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62754



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