[PATCH] D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

I ended up adding a lot to this patch, so feel free to let me know if I should 
split this up if it's more difficult to review.

Changes:

- Remove default ctors for `APFixedPoint` and `FixedPointSemantics`. The main 
reason I had these was so in the `EvaluateAsFixedPoint*` functions, I could 
pass in an `APFixedPoint` by reference that could be assigned on successful 
evaluation. But it feels more correct now to instead just use an `APValue`.
- Allow `APFixedPoint` to be contained in `APValue`. This takes up a bulk of 
the new changes and could be it's own patch.
- `FixedPointExprEvaluator` now calculates an `APValue` that holds an 
`APFixedPoint`.
- Removed unused `Success` methods in `FixedPointExprEvaluator`.
- Added `EvaluateAsFixedPoint` functions and method to `Expr`.
- Added tests for constant-evaluating fixed point conversions.
- Added warning for potential overflow conversions when assigning to fixed 
point types (as a part of adding tests for conversions)
- `APFixedPoint` `convert()` and `add()` methods take an optional second 
argument for indicating if the operation resulted in an overflow (although this 
could be changed for a clearer way to check for operator overflow)




Comment at: clang/include/clang/Basic/FixedPoint.h:98
  public:
+   APFixedPoint() = default;
APFixedPoint(const llvm::APInt , const FixedPointSemantics )

ebevhan wrote:
> rjmccall wrote:
> > This should be documented to describe what it does.  Does it create a value 
> > with impossible semantics?  Some reasonable default value in some 
> > reasonable default semantics?
> As it is, it doesn't seem to create anything useful; neither does the one for 
> the `FixedPointSemantics`. It would be a zero-width, zero-scale value.
The idea behind this was to enable a value to later be reassigned to an 
APFixedPoint, similar to how APValues are assigned in `EvaluateInteger` on 
success. I suppose the correct thing to do in this case would be to instead 
allow for APValue to take an APFixedPoint instead and pass that to 
`EvaluateFixedPoint*` to make it consistent with the other Evaluate methods and 
allow for APFixedPoint to be initialized with a default value.



Comment at: clang/lib/AST/ExprConstant.cpp:9953
+  return false;
+APFixedPoint Result = Src.convert(DestFXSema);
+return Success(Result, E);

rjmccall wrote:
> Can a fixed-point conversion ever have undefined behavior?  If so, you might 
> need to flag that as a failed case, unless we want to give it defined 
> semantics in Clang.
Yup. Overflow can happen for some non-saturating operations. Added.



Comment at: clang/lib/AST/ExprConstant.cpp:9955
+return Success(Result, E);
+  }
+  default:

ebevhan wrote:
> rjmccall wrote:
> > Do you not have a separate `CK_IntegralToFixedPoint`?  It's convenient 
> > here, but still, it's curious.
> > 
> > You also need a `CK_FloatingToFixedPoint`, I think.  (Obviously you don't 
> > need to implement that right now.)
> Both of those should exist. I think that the `FixedPointCast` evaluation 
> should only use `EvaluateFixedPoint`.
I have not yet added either of those but will do so in other patches.



Comment at: clang/lib/AST/ExprConstant.cpp:9959
+  }
+  llvm_unreachable("unknown cast resulting in fixed point value");
+}

rjmccall wrote:
> This is obviously unreachable because of the `default` case in the `switch`.  
> Should this be moved into the `default` case, and then you can put your 
> `Error` call in cases for the known-missing fixed-point conversions?
> 
> You should go ahead and add cases for `CK_NoOp` and `CK_LValueToRValue`, they 
> should be obvious from the other evaluators.
> 
> You should add tests for constant-evaluating fixed-point conversions.
I think it would be better to keep it as an error since elsewhere, we emit 
errors indicating a particular operation or conversion has not yet been 
implemented.



Comment at: clang/lib/AST/ExprConstant.cpp:9977
+APFixedPoint Result = LHSFX.add(RHSFX).convert(ResultFXSema);
+return Success(Result, E);
+  }

ebevhan wrote:
> I understand the benefit of placing the actual operation implementation in 
> the `APFixedPoint` class, but it means that any intermediate information is 
> sort of lost. For example, if we want to warn the user about overflow (into 
> the padding bit, or just in general) we can't, because that information was 
> hidden.
> 
> I guess it could be done by checking if the result of the `add` is equal to 
> the result of the `convert` for non-saturating `ResultFXSema`? The 
> `APFixedPoint` comparison is value-based. Not sure how it would work with the 
> padding bit in unsigned types, though.
We could do something like having a separate method that checks for overflow or 
like `bool addCanOverflow(APFixedPoint LHS, APFixedPoint RHS)`, or an 

[PATCH] D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 181421.
leonardchan marked 13 inline comments as done.

Repository:
  rC Clang

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

https://reviews.llvm.org/D55868

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Frontend/fixed_point_add.c
  clang/test/Frontend/fixed_point_conversions.c
  clang/test/Frontend/fixed_point_errors.c

Index: clang/test/Frontend/fixed_point_errors.c
===
--- clang/test/Frontend/fixed_point_errors.c
+++ clang/test/Frontend/fixed_point_errors.c
@@ -232,3 +232,8 @@
   auto auto_accum = 0k;  // expected-error{{invalid suffix 'k' on integer constant}}
  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
 }
+
+// Overflow
+short _Accum sa_const = 256.0k;   // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}
+short _Fract sf_const = 1.0hk;// expected-warning{{implicit conversion from 1.0 cannot fit within the range of values for 'short _Fract'}}
+unsigned _Accum ua_const = -1.0k; // expected-warning{{implicit conversion from -1.0 cannot fit within the range of values for 'unsigned _Accum'}}
Index: clang/test/Frontend/fixed_point_conversions.c
===
--- clang/test/Frontend/fixed_point_conversions.c
+++ clang/test/Frontend/fixed_point_conversions.c
@@ -1,5 +1,39 @@
-// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+// Between different fixed point types
+short _Accum sa_const = 2.5hk; // DEFAULT-DAG: @sa_const  = {{.*}}global i16 320, align 2
+_Accum a_const = 2.5hk;// DEFAULT-DAG: @a_const   = {{.*}}global i32 81920, align 4
+short _Accum sa_const2 = 2.5k; // DEFAULT-DAG: @sa_const2 = {{.*}}global i16 320, align 2
+
+short _Accum sa_from_f_const = 0.5r; // DEFAULT-DAG: sa_from_f_const = {{.*}}global i16 64, align 2
+_Fract f_from_sa_const = 0.5hk;  // DEFAULT-DAG: f_from_sa_const = {{.*}}global i16 16384, align 2
+
+unsigned short _Accum usa_const = 2.5uk;
+unsigned _Accum ua_const = 2.5uhk;
+// DEFAULT-DAG: @usa_const  = {{.*}}global i16 640, align 2
+// DEFAULT-DAG: @ua_const   = {{.*}}global i32 163840, align 4
+// SAME-DAG:@usa_const  = {{.*}}global i16 320, align 2
+// SAME-DAG:@ua_const   = {{.*}}global i32 81920, align 4
+
+// Signedness
+unsigned short _Accum usa_const2 = 2.5hk;
+// DEFAULT-DAG: @usa_const2  = {{.*}}global i16 640, align 2
+// SAME-DAG:@usa_const2  = {{.*}}global i16 320, align 2
+short _Accum sa_const3 = 2.5hk; // DEFAULT-DAG: @sa_const3 = {{.*}}global i16 320, align 2
+
+// Overflow (this is undefined but allowed)
+short _Accum sa_const4 = 256.0k;
+
+// Saturation
+_Sat short _Accum sat_sa_const = 2.5hk;   // DEFAULT-DAG: @sat_sa_const  = {{.*}}global i16 320, align 2
+_Sat short _Accum sat_sa_const2 = 256.0k; // DEFAULT-DAG: @sat_sa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = -1.0hk;
+// DEFAULT-DAG: @sat_usa_const = {{.*}}global i16 0, align 2
+// SAME-DAG:@sat_usa_const = {{.*}}global i16 0, align 2
+_Sat unsigned short _Accum sat_usa_const2 = 256.0k;
+// DEFAULT-DAG: @sat_usa_const2 = {{.*}}global i16 -1, align 2
+// SAME-DAG:@sat_usa_const2 = {{.*}}global i16 32767, align 2
 
 void TestFixedPointCastSameType() {
   _Accum a = 2.5k;
Index: clang/test/Frontend/fixed_point_add.c
===
--- clang/test/Frontend/fixed_point_add.c
+++ clang/test/Frontend/fixed_point_add.c
@@ -1,5 +1,48 @@
-// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
-// RUN: %clang_cc1 -ffixed-point -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | 

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56554#1354885 , @ruiu wrote:

> In D56554#1353572 , @krytarowski 
> wrote:
>
> > What do you think about this new logic:
> >
> > 1. specified `-z execstack` -> do not emit GNU STACK segment
> > 2. specified `-z noexecstack` -> emit GNU STACK segment
> > 3. no option specified -> detect `findSection(".note.GNU-stack")` (I might 
> > miss offhand some details here to be sure if it is reliable) 3.1. detected 
> > -> emit GNU STACK segment 3.2. not detected -> do not emit GNU STACK segment
> >
> >   Additionally we will specify explicitly in the clang driver for Linux and 
> > FreeBSD `-z noexecstack`.
>
>
> I think I don't like the very idea of using "marker sections" in input object 
> files to change the linker behavior. That's too subtle and seems error-prone 
> to me.
>
> After thinking for a while, I started thinking that the first version of this 
> patch is probably exactly what we want. I had been thinking that `-z 
> {no,}execstack` and `-z {no,}gnustack` are four different options, but what 
> we actually want to get is tri-state:
>
> - emit PT_GNU_STACK with RWX permission
> - emit PT_GNU_STACK with RW permission
> - do not emit PT_GNU_STACK
>
>   So we could map them to `-z {execstack,noexecstack,nognustack}`, 
> respectively, with default set to `-z noexecstack`. You guys can pass `-z 
> nognustack` to the linker to tell the linker to not emit it at all. That's 
> exactly this patch does.


Actually after a longer thought, I recommend to hardcode inside lld a check for 
`TargetTriple.isOSNetBSD()`. Using `-z nognustack` isn't portable to other 
linkers.




Comment at: ELF/Writer.cpp:1979
 
-  // PT_GNU_STACK is a special section to tell the loader to make the
-  // pages for the stack non-executable. If you really want an executable
-  // stack, you can pass -z execstack, but that's not recommended for
-  // security reasons.
-  unsigned Perm = PF_R | PF_W;
-  if (Config->ZExecstack)
-Perm |= PF_X;
-  AddHdr(PT_GNU_STACK, Perm)->p_memsz = Config->ZStackSize;
+  if (!Config->ZNognustack) {
+// PT_GNU_STACK is a special section to tell the loader to make the

Please go for `if (!Config->TargetTriple.isOSNetBSD()) {`


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56554



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

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

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Updated according to your comments.




Comment at: lib/Sema/SemaDeclAttr.cpp:3481
+  llvm::StringMap NameIdxMapping;
+  NameIdxMapping["__"] = -1;
+

aaron.ballman wrote:
> This doesn't match the documentation -- I assume you switched from `?` to 
> `__` because `__` at least parses as a valid identifier, whereas `?` would 
> require extra parsing support? If so, that's fine by me.
Yes, `__`, and `__this` where chosen because they work without lexer/parser 
changes and are in the implementation namespace. I forgot to update the 
documentation though. Will be fixed.



Comment at: lib/Sema/SemaDeclAttr.cpp:3483
+
+  NameIdxMapping["__this"] = 0;
+

aaron.ballman wrote:
> This doesn't match the documentation either, but I'm less clear on why the 
> double underscores are used.
If you use `this`, the lexer will generate the special "this" token. That one 
is checked explicitly to be only used inside of non-static class methods. If 
you have an idea how to avoid this check or make it consider uses in the 
attribute as OK, please let me know.



Comment at: lib/Sema/SemaDeclAttr.cpp:3492
+  SmallVector EncodingIndices;
+  for (unsigned u = 0, e = AL.getNumArgs(); u < e; u++) {
+

aaron.ballman wrote:
> Identifiers don't match the usual naming conventions.  Prefer `++U` as well.
OK.


> Prefer ++U as well.

Out of curiosity, why?



Comment at: lib/Sema/SemaDeclAttr.cpp:3493
+  for (unsigned u = 0, e = AL.getNumArgs(); u < e; u++) {
+
+SourceRange SR;

aaron.ballman wrote:
> Spurious newline
That was intentional but OK.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483



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

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

[Johannes Doerfert via Phabricator via cfe-commits]

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

Style changes, clang-format, documentation improvements


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483

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

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -775,6 +775,11 @@
 }
   };
 
+  struct VariadicParamOrParamIdxArgument : public VariadicArgument {
+VariadicParamOrParamIdxArgument(const Record , StringRef Attr)
+: VariadicArgument(Arg, Attr, "int") {}
+  };
+
   // Unique the enums, but maintain the original declaration ordering.
   std::vector
   uniqueEnumsInOrder(const std::vector ) {
@@ -1283,6 +1288,8 @@
 Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "VariadicParamIdxArgument")
 Ptr = llvm::make_unique(Arg, Attr);
+  else if (ArgName == "VariadicParamOrParamIdxArgument")
+Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "ParamIdxArgument")
 Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
   else if (ArgName == "VariadicIdentifierArgument")
@@ -2116,6 +2123,7 @@
  llvm::StringSwitch(
  Arg->getSuperClasses().back().first->getName())
  .Case("VariadicIdentifierArgument", true)
+ .Case("VariadicParamOrParamIdxArgument", true)
  .Default(false);
 }
 
Index: test/SemaCXX/attr-callback.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-callback.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+class C_in_class {
+#include "../Sema/attr-callback.c"
+};
+
+struct Base {
+
+  void no_args_1(void (*callback)(void));
+  __attribute__((callback(1))) void no_args_2(void (*callback)(void));
+  __attribute__((callback(callback))) void no_args_3(void (*callback)(void)) {}
+
+  __attribute__((callback(1, 0))) virtual void
+  this_tr(void (*callback)(Base *));
+
+  __attribute__((callback(1, __this, __, __this))) virtual void
+  this_unknown_this(void (*callback)(Base *, Base *, Base *));
+
+  __attribute__((callback(1))) virtual void
+  virtual_1(void (*callback)(void));
+
+  __attribute__((callback(callback))) virtual void
+  virtual_2(void (*callback)(void));
+
+  __attribute__((callback(1))) virtual void
+  virtual_3(void (*callback)(void));
+};
+
+__attribute__((callback(1))) void
+Base::no_args_1(void (*callback)(void)) {
+}
+
+void Base::no_args_2(void (*callback)(void)) {
+}
+
+struct Derived_1 : public Base {
+
+  __attribute__((callback(1, 0))) virtual void
+  this_tr(void (*callback)(Base *)) override;
+
+  __attribute__((callback(1))) virtual void
+  virtual_1(void (*callback)(void)) override {}
+
+  virtual void
+  virtual_3(void (*callback)(void)) override {}
+};
+
+struct Derived_2 : public Base {
+
+  __attribute__((callback(callback))) virtual void
+  virtual_1(void (*callback)(void)) override;
+
+  virtual void
+  virtual_2(void (*callback)(void)) override;
+
+  virtual void
+  virtual_3(void (*callback)(void)) override;
+};
+
+void Derived_2::virtual_1(void (*callback)(void)) {}
+
+__attribute__((callback(1))) void
+Derived_2::virtual_2(void (*callback)(void)) {}
+
+void Derived_2::virtual_3(void (*callback)(void)) {}
Index: test/SemaCXX/attr-callback-broken.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-callback-broken.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+class C_in_class {
+#define HAS_THIS
+#include "../Sema/attr-callback-broken.c"
+#undef HAS_THIS
+};
Index: test/Sema/attr-callback.c
===
--- /dev/null
+++ test/Sema/attr-callback.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+__attribute__((callback(1))) void no_args(void (*callback)(void));
+__attribute__((callback(1, 2, 3))) void args_1(void 

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 181419.
NoQ marked an inline comment as done.
NoQ added a comment.

Prettify the unittest a bit, especially the `ASTMatcher` part of it.


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

https://reviews.llvm.org/D56632

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  test/Analysis/diagnostics/dtors.cpp
  test/Analysis/symbol-reaper.cpp
  unittests/StaticAnalyzer/CMakeLists.txt
  unittests/StaticAnalyzer/SymbolReaperTest.cpp

Index: unittests/StaticAnalyzer/SymbolReaperTest.cpp
===
--- /dev/null
+++ unittests/StaticAnalyzer/SymbolReaperTest.cpp
@@ -0,0 +1,121 @@
+//===- unittests/StaticAnalyzer/SymbolReaperTest.cpp --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+using namespace ast_matchers;
+
+// A re-usable consumer that constructs ExprEngine out of CompilerInvocation.
+// TODO: Actually re-use it when we write our second test.
+class ExprEngineConsumer : public ASTConsumer {
+protected:
+  CompilerInstance 
+
+private:
+  // We need to construct all of these in order to construct ExprEngine.
+  CheckerManager ChkMgr;
+  cross_tu::CrossTranslationUnitContext CTU;
+  PathDiagnosticConsumers Consumers;
+  AnalysisManager AMgr;
+  SetOfConstDecls VisitedCallees;
+  FunctionSummariesTy FS;
+
+protected:
+  ExprEngine Eng;
+
+  // Find a declaration in the current AST by name. This has nothing to do
+  // with ExprEngine but turns out to be handy.
+  // TODO: There's probably a better place for it.
+  template 
+  const T *findDeclByName(const Decl *Where, StringRef Name) {
+auto Matcher = decl(hasDescendant(namedDecl(hasName(Name)).bind("d")));
+auto Matches = match(Matcher, *Where, Eng.getContext());
+assert(Matches.size() == 1 && "Ambiguous name!");
+for (BoundNodes  : Matches)
+  return M.getNodeAs("d");
+llvm_unreachable("Unable to find declaration!");
+  }
+
+public:
+  ExprEngineConsumer(CompilerInstance )
+  : C(C), ChkMgr(C.getASTContext(), *C.getAnalyzerOpts()), CTU(C),
+Consumers(),
+AMgr(C.getASTContext(), C.getDiagnostics(), Consumers,
+ CreateRegionStoreManager, CreateRangeConstraintManager, ,
+ *C.getAnalyzerOpts()),
+VisitedCallees(), FS(),
+Eng(CTU, AMgr, , , ExprEngine::Inline_Regular) {}
+};
+
+class SuperRegionLivenessConsumer : public ExprEngineConsumer {
+  void performTest(const Decl *D) {
+const auto *FD = findDeclByName(D, "x");
+const auto *VD = findDeclByName(D, "s");
+assert(FD && VD);
+
+// The variable must belong to a stack frame,
+// otherwise SymbolReaper would think it's a global.
+const StackFrameContext *SFC =
+Eng.getAnalysisDeclContextManager().getStackFrame(D);
+
+// Create regions for 's' and 's.x'.
+const VarRegion *VR = Eng.getRegionManager().getVarRegion(VD, SFC);
+const FieldRegion *FR = Eng.getRegionManager().getFieldRegion(FD, VR);
+
+// Pass a null location context to the SymbolReaper so that
+// it was thinking that the variable is dead.
+SymbolReaper SymReaper((StackFrameContext *)nullptr, (Stmt *)nullptr,
+   Eng.getSymbolManager(), Eng.getStoreManager());
+
+SymReaper.markLive(FR);
+EXPECT_TRUE(SymReaper.isLiveRegion(VR));
+  }
+
+public:
+  SuperRegionLivenessConsumer(CompilerInstance ) : ExprEngineConsumer(C) {}
+  ~SuperRegionLivenessConsumer() override {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (auto D: DG)
+  performTest(D);
+return true;
+  }
+};
+
+class SuperRegionLivenessAction: public ASTFrontendAction {
+public:
+  SuperRegionLivenessAction() {}
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef File) override {
+auto Consumer = llvm::make_unique(Compiler);
+return Consumer;
+  }
+};
+
+// Test that marking s.x as live would also make s live.
+TEST(SymbolReaper, SuperRegionLiveness) {
+  EXPECT_TRUE(tooling::runToolOnCode(new SuperRegionLivenessAction,
+ "void foo() { struct 

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

[Artem Dergachev via Phabricator via cfe-commits]

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

This is a follow-up to D56042 .

When a memory region is live, all its sub-regions and super-regions are 
automatically live (and vice versa), so it is only necessary to track liveness 
of base regions. This is exactly how we imagined this to work, but it turns out 
that it didn't.

The reason why it works correctly most of the time because the reachable symbol 
scanner is automatically marks all parent regions as reachable - and therefore 
they are marked as live by adding all of them to region roots every time a 
sub-region is marked as live. However, enumerating all *child* regions this way 
is problematic (there may be infinitely many of them).

In the test from D56042 , the symbol for `p.x` 
dies because when `.get()` is called for the last time only `p` is kept alive 
by the Environment, but not `p.x`. Due to that,  `reg_$0` is believed to 
be dead - recall that `SymbolRegionValue` is kept alive by its parent region, 
and additionally notice that there are no explicit bindings anywhere else to 
keep it alive (because `SymbolRegionValue` is simply assumed to be there as 
long as the parent region is alive, rather than bound explicitly).

Now, when the `RegionRoots` test fails, `isLiveRegion()` falls through to see 
if the region is "lexically" alive. Here it correctly jumps to the base region 
`p` and looks if live variables analysis thinks it's alive. It doesn't! Because 
there's no need to compute expression 'p' anymore anywhere in the program.

What `isLiveRegion()` should have done is look up the base region in 
`RegionRoots`. But it doesn't. Hence the patch.

The newly added `test_A` demonstrates the problem even more clearly: having the 
symbol for `a.x` die before the call to `a.foo()` is definitely incorrect.

The other test, `test_B`, is an attempt to figure out whether the problem is 
also there "in the opposite direction". That is, when `b.a` is marked live, is 
`b` marked live automatically? Otherwise the lookup in `RegionRoots` would 
still fail.

The answer is yes, it does work correctly, because `scanReachableSymbols` 
always scans the whole super-region chain of every region. Which means that 
every time the Environment or the Store marks a region as live, all of its 
super-regions are added to `RegionRoots`. However, i would still like to add 
conversion to the base region into `markLive()`, because this behavior is 
something that should be guaranteed by `SymbolReaper` rather implied by callers 
manually, even if the callers have to do that anyway.

So for now the change in `markLive()` does not affect functionality at all, but 
it will be important when checkers use the `checkLiveSymbols()` functionality 
more aggressively. Additionally it slightly decreases the size of the 
`RegionRoots` map for faster lookups but adds an extra time overhead for 
marking something as live (need to ascend to the base region). I didn't try to 
figure out whether it gives a net gain in performance.

For that reason the unit test as well. Also a few convenient getters were added 
to `ExprEngine` in order to make the test more concise.


Repository:
  rC Clang

https://reviews.llvm.org/D56632

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  test/Analysis/diagnostics/dtors.cpp
  test/Analysis/symbol-reaper.cpp
  unittests/StaticAnalyzer/CMakeLists.txt
  unittests/StaticAnalyzer/SymbolReaperTest.cpp

Index: unittests/StaticAnalyzer/SymbolReaperTest.cpp
===
--- /dev/null
+++ unittests/StaticAnalyzer/SymbolReaperTest.cpp
@@ -0,0 +1,119 @@
+//===- unittests/StaticAnalyzer/SymbolReaperTest.cpp --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+using namespace ast_matchers;
+
+class SuperRegionLivenessConsumer : public ASTConsumer {
+  CompilerInstance 
+
+  // We need to construct all of 

[PATCH] D56631: [MSVC Compat] Fix typo correction for inclusion directives.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: christylee, compnerd.
Herald added subscribers: dexonsmith, jkorous.

In MSVC compatibility mode we were checking not the typo corrected
filename but the original filename.


https://reviews.llvm.org/D56631

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/include-likely-typo.c


Index: clang/test/Preprocessor/include-likely-typo.c
===
--- clang/test/Preprocessor/include-likely-typo.c
+++ clang/test/Preprocessor/include-likely-typo.c
@@ -1,3 +1,4 @@
 // RUN: %clang_cc1 %s -verify
+// RUN: %clang_cc1 -fms-compatibility %s -verify
 
 #include "" // expected-error 
{{'' file not found, did you mean 
'empty_file_to_include.h'?}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1813,9 +1813,17 @@
   return Filename;
 };
 StringRef TypoCorrectionName = CorrectTypoFilename(Filename);
+SmallString<128> NormalizedTypoCorrectionPath;
+if (LangOpts.MSVCCompat) {
+  NormalizedTypoCorrectionPath = TypoCorrectionName.str();
+#ifndef _WIN32
+  llvm::sys::path::native(NormalizedTypoCorrectionPath);
+#endif
+}
 File = LookupFile(
 FilenameLoc,
-LangOpts.MSVCCompat ? NormalizedPath.c_str() : TypoCorrectionName,
+LangOpts.MSVCCompat ? NormalizedTypoCorrectionPath.c_str()
+: TypoCorrectionName,
 isAngled, LookupFrom, LookupFromFile, CurDir,
 Callbacks ?  : nullptr,
 Callbacks ?  : nullptr, , );


Index: clang/test/Preprocessor/include-likely-typo.c
===
--- clang/test/Preprocessor/include-likely-typo.c
+++ clang/test/Preprocessor/include-likely-typo.c
@@ -1,3 +1,4 @@
 // RUN: %clang_cc1 %s -verify
+// RUN: %clang_cc1 -fms-compatibility %s -verify
 
 #include "" // expected-error {{'' file not found, did you mean 'empty_file_to_include.h'?}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1813,9 +1813,17 @@
   return Filename;
 };
 StringRef TypoCorrectionName = CorrectTypoFilename(Filename);
+SmallString<128> NormalizedTypoCorrectionPath;
+if (LangOpts.MSVCCompat) {
+  NormalizedTypoCorrectionPath = TypoCorrectionName.str();
+#ifndef _WIN32
+  llvm::sys::path::native(NormalizedTypoCorrectionPath);
+#endif
+}
 File = LookupFile(
 FilenameLoc,
-LangOpts.MSVCCompat ? NormalizedPath.c_str() : TypoCorrectionName,
+LangOpts.MSVCCompat ? NormalizedTypoCorrectionPath.c_str()
+: TypoCorrectionName,
 isAngled, LookupFrom, LookupFromFile, CurDir,
 Callbacks ?  : nullptr,
 Callbacks ?  : nullptr, , );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Out of interest, what's the motivation for this? It seems to add way more code 
than it removes, so there must be some other advantage, but the patch 
description doesn't say.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55491



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

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from nits that aren't complete yet, LGTM.




Comment at: include/clang/AST/TemplateArgumentVisitor.h:24
+/// A simple visitor class that helps create template argument visitors.
+template  class Ref, typename ImplClass,
+  typename RetTy = void, typename... ParamTys>

Missed a `typename` in there.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55491



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 marked 2 inline comments as done.
jyu2 added inline comments.



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

jyu2 wrote:
> efriedma wrote:
> > jyu2 wrote:
> > > efriedma wrote:
> > > > jyu2 wrote:
> > > > > efriedma wrote:
> > > > > > This looks suspicious; an AddrLabelExpr could be an input or 
> > > > > > output, e.g. `"r"(&)`.
> > > > > Syntax for asm goto:
> > > > >  Syntax:
> > > > >asm [volatile] goto ( AssemblerTemplate
> > > > >:
> > > > >: InputOperands
> > > > >: Clobbers
> > > > >: GotoLabels)
> > > > > 
> > > > >  Only input is allowed.  Output is not allowed
> > > > > 
> > > > That doesn't really address my point here... ignore the "or output" 
> > > > part of the comment.
> > > Sorry did not realize that.  Thank you so much for catching that.  Need 
> > > to add other condition "ConstraintIdx > NS->getNumInputs() - 1", change 
> > > to :
> > > 
> > > if (NS->isGCCAsmGoto() && ConstraintIdx > NS->getNumInputs() - 1 &&
> > > Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
> > >   break;
> > > 
> > > Is this ok with you?  Thanks
> > That's the right idea. But I still see a few issues at that point:
> > 
> > 1. The AddrLabelExprClass check is redundant.
> > 2. "NS->getNumInputs() - 1" can overflow; probably should use 
> > "ConstraintIdx >= NS->getNumInputs()".
> > 3. "break" exits the loop completely (so it skips validating all 
> > constraints written after the label).
> > 4. The code needs to verify that the user correctly specified the "l" 
> > constraint modifier.
> Sorry not done yet.  
For you comment 4:

The code needs to verify that the user correctly specified the "l" constraint 
modifier.  We already emit error like following?

Do you mean, we need more checking here?  Thanks. 

n.c:4:35: error: unknown symbolic operand name in inline assembly string
  asm goto ("frob %%r5, %1; jc %l[error]; mov (%2), %%r5"
~~^~~
n.c:8:15: error: use of undeclared label 'error1'
: error1);

Test is:
int frob(int x)
{
  int y;
  asm goto ("frob %%r5, %1; jc %l[error]; mov (%2), %%r5"
: /* No outputs. */
: "r"(x), "r"()
: "memory"
: error1);
  return y;
error:
  return -1;
}




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

https://reviews.llvm.org/D56571



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

Re: r350984 - NFC: Port loop to cxx_range_for

[Aaron Ballman via cfe-commits]

On Fri, Jan 11, 2019 at 7:46 PM Stephen Kelly via cfe-commits
 wrote:
>
> Author: steveire
> Date: Fri Jan 11 16:42:59 2019
> New Revision: 350984
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350984=rev
> Log:
> NFC: Port loop to cxx_range_for

Thanks for the cleanup!

>
> Modified:
> cfe/trunk/lib/AST/ASTDumper.cpp
>
> Modified: cfe/trunk/lib/AST/ASTDumper.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=350984=350983=350984=diff
> ==
> --- cfe/trunk/lib/AST/ASTDumper.cpp (original)
> +++ cfe/trunk/lib/AST/ASTDumper.cpp Fri Jan 11 16:42:59 2019
> @@ -709,9 +709,8 @@ void ASTDumper::dumpTemplateArgument(con
>break;
>  case TemplateArgument::Pack:
>OS << " pack";
> -  for (TemplateArgument::pack_iterator I = A.pack_begin(), E = 
> A.pack_end();
> -   I != E; ++I)
> -dumpTemplateArgument(*I);
> +  for (const auto& TArg : A.pack_elements())

The formatting above is incorrect -- clang-format helps to avoid that.

~Aaron

> +dumpTemplateArgument(TArg);
>break;
>  }
>});
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

[Yuanfang Chen via Phabricator via cfe-commits]

tabloid.adroit added a comment.

gentle ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D56353



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

> Apologies - the value seems to indeed overflow, but I'm still very confused 
> how this was affected by this change.

What's different is that `setRequiresImmediate` is being set on the constraint 
where before it wasn't. If no range values are supplied (like in this patch), 
it defaults to `INT_MIN` and `INT_MAX`. From GNU's documentation, `n` accepts 
an integer, which we're treating as signed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Apologies - the value seems to indeed overflow, but I'm still very confused how 
this was affected by this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Jennifer Yu via Phabricator via cfe-commits]

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



Comment at: lib/Analysis/CFG.cpp:1474
+  appendScopeBegin(JT.block, VD, G);
+  addSuccessor(B, JT.block);
+}

efriedma wrote:
> Please don't copy-paste code.
:-(  changed



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

efriedma wrote:
> jyu2 wrote:
> > efriedma wrote:
> > > jyu2 wrote:
> > > > efriedma wrote:
> > > > > This looks suspicious; an AddrLabelExpr could be an input or output, 
> > > > > e.g. `"r"(&)`.
> > > > Syntax for asm goto:
> > > >  Syntax:
> > > >asm [volatile] goto ( AssemblerTemplate
> > > >:
> > > >: InputOperands
> > > >: Clobbers
> > > >: GotoLabels)
> > > > 
> > > >  Only input is allowed.  Output is not allowed
> > > > 
> > > That doesn't really address my point here... ignore the "or output" part 
> > > of the comment.
> > Sorry did not realize that.  Thank you so much for catching that.  Need to 
> > add other condition "ConstraintIdx > NS->getNumInputs() - 1", change to :
> > 
> > if (NS->isGCCAsmGoto() && ConstraintIdx > NS->getNumInputs() - 1 &&
> > Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
> >   break;
> > 
> > Is this ok with you?  Thanks
> That's the right idea. But I still see a few issues at that point:
> 
> 1. The AddrLabelExprClass check is redundant.
> 2. "NS->getNumInputs() - 1" can overflow; probably should use "ConstraintIdx 
> >= NS->getNumInputs()".
> 3. "break" exits the loop completely (so it skips validating all constraints 
> written after the label).
> 4. The code needs to verify that the user correctly specified the "l" 
> constraint modifier.
Sorry not done yet.  


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 181403.
jyu2 added a comment.

Couple of change to respond Eli’s comments.


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/ExprObjC.h
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/asm-goto.cpp
  test/CodeGen/asm-goto.c
  test/CodeGen/asm.c
  test/CodeGen/inline-asm-mixed-style.c
  test/Coverage/c-language-features.inc
  test/PCH/asm.h
  test/Parser/asm-goto.c
  test/Parser/asm-goto.cpp
  test/Parser/asm.c
  test/Parser/asm.cpp
  test/Sema/asm.c
  test/Sema/inline-asm-validate-tmpl.cpp

Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,21 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+}
Index: test/Parser/asm.cpp
===
--- test/Parser/asm.cpp
+++ test/Parser/asm.cpp
@@ -7,3 +7,28 @@
 int foo5 asm (U"bar5"); // expected-error {{cannot use unicode string literal in 'asm'}}
 int foo6 asm ("bar6"_x); // expected-error {{string literal with user-defined suffix cannot be used here}}
 int foo6 asm ("" L"bar7"); // expected-error {{cannot use wide string literal in 'asm'}}
+
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm goto ("testl %0, %0; jne %l3;" :: "r"(cond)::label_true, loop)
+  // expected-error@+1 {{unknown symbolic operand name in inline assembly string}}
+  asm goto ("decl %0; jnz %l[b]" :: "m"(x) : "memory" : a);
+label_true:
+loop:
+a:
+  return 0;
+}
Index: test/Parser/asm.c
===
--- test/Parser/asm.c
+++ test/Parser/asm.c
@@ -16,6 +16,31 @@
   asm _Atomic (""); // expected-warning {{ignored _Atomic qualifier on asm}}
 }
 
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1 {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1 {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm goto ("testl %0, %0; jne %l3;" :: "r"(cond)::label_true, loop)
+  // 

[PATCH] D55616: Emit ASM input in a constant context

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

@void @efriedma I can't build XNU anymore after this commit, with an error 
message:

  osfmk/i386/genassym.c:298:2: error: value '18446743523953737728' out of range 
for constraint 'n'
  DECLAREULL("KERNEL_BASE", KERNEL_BASE);
  ^~
  osfmk/i386/genassym.c:107:65: note: expanded from macro 'DECLAREULL'
  __asm("DEFINITION__define__" SYM ":\t .ascii \"%0\"" : : "n"  
((unsigned long long)(VAL)))
 
^

The error message looks wrong: the value is in range to fit in `unsigned long 
long` (but I'm not an inline assembly expert).

At a minimum, could we add a test case to this change to show in which cases an 
extra diagnostics would appear?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D56620: [COFF, ARM64] Declare intrinsics: __nop, _byteswap_[ushort/ulong/uint64]

[Tom Tan via Phabricator via cfe-commits]

TomTan added a comment.

We need full definition for __nop in intrin.h.




Comment at: lib/Headers/intrin.h:569
+unsigned __int64 _byteswap_uint64 (unsigned __int64 val);
+void __nop();
 #endif

efriedma wrote:
> Isn't there already a declaration of __nop in intrin.h?  (Line 100.)
For __nop, we probably need full definition instead of declaration, just like 
__nop for x86/x64 in this file.


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

https://reviews.llvm.org/D56620



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

[PATCH] D56620: [COFF, ARM64] Declare intrinsics: __nop, _byteswap_[ushort/ulong/uint64]

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/Headers/intrin.h:569
+unsigned __int64 _byteswap_uint64 (unsigned __int64 val);
+void __nop();
 #endif

Isn't there already a declaration of __nop in intrin.h?  (Line 100.)


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

https://reviews.llvm.org/D56620



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

r350984 - NFC: Port loop to cxx_range_for

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Fri Jan 11 16:42:59 2019
New Revision: 350984

URL: http://llvm.org/viewvc/llvm-project?rev=350984=rev
Log:
NFC: Port loop to cxx_range_for

Modified:
cfe/trunk/lib/AST/ASTDumper.cpp

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=350984=350983=350984=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Fri Jan 11 16:42:59 2019
@@ -709,9 +709,8 @@ void ASTDumper::dumpTemplateArgument(con
   break;
 case TemplateArgument::Pack:
   OS << " pack";
-  for (TemplateArgument::pack_iterator I = A.pack_begin(), E = 
A.pack_end();
-   I != E; ++I)
-dumpTemplateArgument(*I);
+  for (const auto& TArg : A.pack_elements())
+dumpTemplateArgument(TArg);
   break;
 }
   });


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

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 181395.
steveire added a comment.

Updates


Repository:
  rC Clang

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

https://reviews.llvm.org/D55491

Files:
  include/clang/AST/TemplateArgumentVisitor.h
  include/clang/AST/TextNodeDumper.h
  lib/AST/ASTDumper.cpp
  lib/AST/TextNodeDumper.cpp

Index: lib/AST/TextNodeDumper.cpp
===
--- lib/AST/TextNodeDumper.cpp
+++ lib/AST/TextNodeDumper.cpp
@@ -64,6 +64,19 @@
   ConstAttrVisitor::Visit(A);
 }
 
+void TextNodeDumper::Visit(const TemplateArgument , SourceRange R,
+   const Decl *From, StringRef Label) {
+  OS << "TemplateArgument";
+  if (R.isValid())
+dumpSourceRange(R);
+
+  if (From) {
+dumpDeclRef(From, Label);
+  }
+
+  ConstTemplateArgumentVisitor::Visit(TA);
+}
+
 void TextNodeDumper::dumpPointer(const void *Ptr) {
   ColorScope Color(OS, ShowColors, AddressColor);
   OS << ' ' << Ptr;
@@ -184,12 +197,12 @@
   OS << ")";
 }
 
-void TextNodeDumper::dumpDeclRef(const Decl *D, const char *Label) {
+void TextNodeDumper::dumpDeclRef(const Decl *D, StringRef Label) {
   if (!D)
 return;
 
   AddChild([=] {
-if (Label)
+if (!Label.empty())
   OS << Label << ' ';
 dumpBareDeclRef(D);
   });
@@ -317,3 +330,45 @@
 const comments::VerbatimLineComment *C, const comments::FullComment *) {
   OS << " Text=\"" << C->getText() << "\"";
 }
+
+void TextNodeDumper::VisitNullTemplateArgument(const TemplateArgument &) {
+  OS << " null";
+}
+
+void TextNodeDumper::VisitTypeTemplateArgument(const TemplateArgument ) {
+  OS << " type";
+  dumpType(TA.getAsType());
+}
+
+void TextNodeDumper::VisitDeclarationTemplateArgument(
+const TemplateArgument ) {
+  OS << " decl";
+  dumpDeclRef(TA.getAsDecl());
+}
+
+void TextNodeDumper::VisitNullPtrTemplateArgument(const TemplateArgument &) {
+  OS << " nullptr";
+}
+
+void TextNodeDumper::VisitIntegralTemplateArgument(const TemplateArgument ) {
+  OS << " integral " << TA.getAsIntegral();
+}
+
+void TextNodeDumper::VisitTemplateTemplateArgument(const TemplateArgument ) {
+  OS << " template ";
+  TA.getAsTemplate().dump(OS);
+}
+
+void TextNodeDumper::VisitTemplateExpansionTemplateArgument(
+const TemplateArgument ) {
+  OS << " template expansion ";
+  TA.getAsTemplateOrTemplatePattern().dump(OS);
+}
+
+void TextNodeDumper::VisitExpressionTemplateArgument(const TemplateArgument &) {
+  OS << " expr";
+}
+
+void TextNodeDumper::VisitPackTemplateArgument(const TemplateArgument &) {
+  OS << " pack";
+}
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/DeclVisitor.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TemplateArgumentVisitor.h"
 #include "clang/AST/TextNodeDumper.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/Builtins.h"
@@ -44,7 +45,8 @@
 public ConstStmtVisitor,
 public ConstCommentVisitor,
 public TypeVisitor,
-public ConstAttrVisitor {
+public ConstAttrVisitor,
+public ConstTemplateArgumentVisitor {
 
 TextNodeDumper NodeDumper;
 
@@ -440,6 +442,14 @@
 // Comments.
 void dumpComment(const Comment *C, const FullComment *FC);
 
+void VisitExpressionTemplateArgument(const TemplateArgument ) {
+  dumpStmt(TA.getAsExpr());
+}
+void VisitPackTemplateArgument(const TemplateArgument ) {
+  for (const auto& TArg : TA.pack_elements())
+dumpTemplateArgument(TArg);
+}
+
 // Implements Visit methods for Attrs.
 #include "clang/AST/AttrNodeTraverse.inc"
   };
@@ -670,49 +680,8 @@
 void ASTDumper::dumpTemplateArgument(const TemplateArgument , SourceRange R,
  const Decl *From, const char *Label) {
   dumpChild([=] {
-OS << "TemplateArgument";
-if (R.isValid())
-  NodeDumper.dumpSourceRange(R);
-
-if (From)
-  NodeDumper.dumpDeclRef(From, Label);
-
-switch (A.getKind()) {
-case TemplateArgument::Null:
-  OS << " null";
-  break;
-case TemplateArgument::Type:
-  OS << " type";
-  NodeDumper.dumpType(A.getAsType());
-  break;
-case TemplateArgument::Declaration:
-  OS << " decl";
-  NodeDumper.dumpDeclRef(A.getAsDecl());
-  break;
-case TemplateArgument::NullPtr:
-  OS << " nullptr";
-  break;
-case TemplateArgument::Integral:
-  OS << " integral " << A.getAsIntegral();
-  break;
-case TemplateArgument::Template:
-  OS << " template ";
-  A.getAsTemplate().dump(OS);
-  break;
-case TemplateArgument::TemplateExpansion:
-  OS << " template expansion ";
-  A.getAsTemplateOrTemplatePattern().dump(OS);
-  break;
-case TemplateArgument::Expression:
-  OS << " expr";
-  dumpStmt(A.getAsExpr());
-  

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I would suggest to rename //contribution// to //Contributing// (see LLVM 
documentation) and //integrations// to //Integrations//.

Will be also good idea to fix D55523  script 
warnings.


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

https://reviews.llvm.org/D54945



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

[PATCH] D56620: [COFF, ARM64] Declare intrinsics: __nop, _byteswap_[short/long/uint64]

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 181390.
mgrang retitled this revision from "[COFF, ARM64] Include stdlib.h in 
arm64intr.h" to "[COFF, ARM64] Declare intrinsics: __nop, 
_byteswap_[short/long/uint64]".
mgrang edited the summary of this revision.

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

https://reviews.llvm.org/D56620

Files:
  lib/Headers/intrin.h


Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -563,6 +563,10 @@
 long _InterlockedAdd(long volatile *Addend, long Value);
 int _ReadStatusReg(int);
 void _WriteStatusReg(int, int);
+unsigned short _byteswap_ushort (unsigned short val);
+unsigned long _byteswap_ulong (unsigned long val);
+unsigned __int64 _byteswap_uint64 (unsigned __int64 val);
+void __nop();
 #endif
 
 
/**\


Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -563,6 +563,10 @@
 long _InterlockedAdd(long volatile *Addend, long Value);
 int _ReadStatusReg(int);
 void _WriteStatusReg(int, int);
+unsigned short _byteswap_ushort (unsigned short val);
+unsigned long _byteswap_ulong (unsigned long val);
+unsigned __int64 _byteswap_uint64 (unsigned __int64 val);
+void __nop();
 #endif
 
 /**\
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56620: [COFF, ARM64] Include stdlib.h in arm64intr.h

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

In D56620#1354979 , @efriedma wrote:

> Have you verified this matches MSVC?  (IIRC the only reason we include 
> stdlib.h on x86 is so we can define _mm_malloc.)


I don't see MSVC intrin.h including stdlib.h. So I guess I will just declare 
the required intrinsics.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56620



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

[PATCH] D56620: [COFF, ARM64] Include stdlib.h in arm64intr.h

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Have you verified this matches MSVC?  (IIRC the only reason we include stdlib.h 
on x86 is so we can define _mm_malloc.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D56620



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

[PATCH] D56620: [COFF, ARM64] Include stdlib.h in arm64intr.h

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.
mgrang added reviewers: rnk, efriedma, ssijaric, TomTan, haripul.
Herald added subscribers: kristof.beyls, javed.absar.

We mimic x86 behavior which includes stdlib.h in x86intr.h.


Repository:
  rC Clang

https://reviews.llvm.org/D56620

Files:
  lib/Headers/arm64intr.h


Index: lib/Headers/arm64intr.h
===
--- lib/Headers/arm64intr.h
+++ lib/Headers/arm64intr.h
@@ -26,6 +26,10 @@
 #include_next 
 #else
 
+#if __STDC_HOSTED__
+#include 
+#endif
+
 #ifndef __ARM64INTR_H
 #define __ARM64INTR_H
 


Index: lib/Headers/arm64intr.h
===
--- lib/Headers/arm64intr.h
+++ lib/Headers/arm64intr.h
@@ -26,6 +26,10 @@
 #include_next 
 #else
 
+#if __STDC_HOSTED__
+#include 
+#endif
+
 #ifndef __ARM64INTR_H
 #define __ARM64INTR_H
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Except one thing, it looks reasonable to me. I'll try to run some tests and 
report back tomorrow.

(Not very familiar with Phabricator either. I still see some comments, 
hopefully the "Collapse" function does something useful here.)




Comment at: test/Driver/prefix-map.S:7
+
+// More tests for this flag in debug-prefix-map.c.

Maybe restore the old file name (debug-prefix-map.S) since this still tests the 
debug prefix functionality? And otherwise this comment needs to be updated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/Analysis/CFG.cpp:1474
+  appendScopeBegin(JT.block, VD, G);
+  addSuccessor(B, JT.block);
+}

Please don't copy-paste code.



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

jyu2 wrote:
> efriedma wrote:
> > jyu2 wrote:
> > > efriedma wrote:
> > > > This looks suspicious; an AddrLabelExpr could be an input or output, 
> > > > e.g. `"r"(&)`.
> > > Syntax for asm goto:
> > >  Syntax:
> > >asm [volatile] goto ( AssemblerTemplate
> > >:
> > >: InputOperands
> > >: Clobbers
> > >: GotoLabels)
> > > 
> > >  Only input is allowed.  Output is not allowed
> > > 
> > That doesn't really address my point here... ignore the "or output" part of 
> > the comment.
> Sorry did not realize that.  Thank you so much for catching that.  Need to 
> add other condition "ConstraintIdx > NS->getNumInputs() - 1", change to :
> 
> if (NS->isGCCAsmGoto() && ConstraintIdx > NS->getNumInputs() - 1 &&
> Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
>   break;
> 
> Is this ok with you?  Thanks
That's the right idea. But I still see a few issues at that point:

1. The AddrLabelExprClass check is redundant.
2. "NS->getNumInputs() - 1" can overflow; probably should use "ConstraintIdx >= 
NS->getNumInputs()".
3. "break" exits the loop completely (so it skips validating all constraints 
written after the label).
4. The code needs to verify that the user correctly specified the "l" 
constraint modifier.


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

https://reviews.llvm.org/D56571



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

r350982 - [analyzer] Support for OSObjects out parameters in RetainCountChecker

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Jan 11 15:35:17 2019
New Revision: 350982

URL: http://llvm.org/viewvc/llvm-project?rev=350982=rev
Log:
[analyzer] Support for OSObjects out parameters in RetainCountChecker

rdar://46357478
rdar://47121327

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

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

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=350982=350981=350982=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Fri Jan 
11 15:35:17 2019
@@ -77,13 +77,21 @@ enum ArgEffectKind {
   IncRef,
 
   /// The argument is a pointer to a retain-counted object; on exit, the new
-  /// value of the pointer is a +0 value or NULL.
+  /// value of the pointer is a +0 value.
   UnretainedOutParameter,
 
   /// The argument is a pointer to a retain-counted object; on exit, the new
-  /// value of the pointer is a +1 value or NULL.
+  /// value of the pointer is a +1 value.
   RetainedOutParameter,
 
+  /// The argument is a pointer to a retain-counted object; on exit, the new
+  /// value of the pointer is a +1 value iff the return code is zero.
+  RetainedOutParameterOnZero,
+
+  /// The argument is a pointer to a retain-counted object; on exit, the new
+  /// value of the pointer is a +1 value iff the return code is non-zero.
+  RetainedOutParameterOnNonZero,
+
   /// The argument is treated as potentially escaping, meaning that
   /// even when its reference count hits 0 it should be treated as still
   /// possibly being alive as someone else *may* be holding onto the object.

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=350982=350981=350982=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Fri Jan 11 15:35:17 2019
@@ -529,38 +529,92 @@ void RetainCountChecker::processSummaryO
   C.addTransition(state);
 }
 
-static ProgramStateRef updateOutParameter(ProgramStateRef State,
-  SVal ArgVal,
-  ArgEffectKind Effect) {
-  auto *ArgRegion = dyn_cast_or_null(ArgVal.getAsRegion());
-  if (!ArgRegion)
-return State;
-
-  QualType PointeeTy = ArgRegion->getValueType();
-  if (!coreFoundation::isCFObjectRef(PointeeTy))
-return State;
-
-  SVal PointeeVal = State->getSVal(ArgRegion);
-  SymbolRef Pointee = PointeeVal.getAsLocSymbol();
-  if (!Pointee)
-return State;
-
-  switch (Effect) {
-  case UnretainedOutParameter:
-State = setRefBinding(State, Pointee,
-  RefVal::makeNotOwned(ObjKind::CF, PointeeTy));
-break;
-  case RetainedOutParameter:
-// Do nothing. Retained out parameters will either point to a +1 reference
-// or NULL, but the way you check for failure differs depending on the API.
-// Consequently, we don't have a good way to track them yet.
-break;
+static bool shouldEscapeRegion(const MemRegion *R) {
 
-  default:
-llvm_unreachable("only for out parameters");
+  // We do not currently model what happens when a symbol is
+  // assigned to a struct field, so be conservative here and let the symbol
+  // go. TODO: This could definitely be improved upon.
+  return !R->hasStackStorage() || !isa(R);
+}
+
+static SmallVector
+updateOutParameters(ProgramStateRef State, const RetainSummary ,
+const CallEvent ) {
+
+  SVal L = CE.getReturnValue();
+
+  // Splitting is required to support out parameters,
+  // as out parameters might be created only on the "success" branch.
+  // We want to avoid eagerly splitting unless out parameters are actually
+  // needed.
+  bool SplitNecessary = false;
+  for (auto  : Summ.getArgEffects())
+if (P.second.getKind() == RetainedOutParameterOnNonZero ||
+P.second.getKind() == RetainedOutParameterOnZero)
+  SplitNecessary = true;
+
+  ProgramStateRef AssumeNonZeroReturn = State;
+ 

r350981 - [analyzer] Introduce a convenience method for getting a CallEvent from an arbitrary Stmt

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Jan 11 15:35:04 2019
New Revision: 350981

URL: http://llvm.org/viewvc/llvm-project?rev=350981=rev
Log:
[analyzer] Introduce a convenience method for getting a CallEvent from an 
arbitrary Stmt

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h?rev=350981=350980=350981=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h Fri 
Jan 11 15:35:04 2019
@@ -1138,9 +1138,16 @@ class CallEventManager {
 public:
   CallEventManager(llvm::BumpPtrAllocator ) : Alloc(alloc) {}
 
+  /// Gets an outside caller given a callee context.
   CallEventRef<>
   getCaller(const StackFrameContext *CalleeCtx, ProgramStateRef State);
 
+  /// Gets a call event for a function call, Objective-C method call,
+  /// or a 'new' call.
+  CallEventRef<>
+  getCall(const Stmt *S, ProgramStateRef State,
+  const LocationContext *LC);
+
   CallEventRef<>
   getSimpleCall(const CallExpr *E, ProgramStateRef State,
 const LocationContext *LCtx);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=350981=350980=350981=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Fri Jan 11 15:35:04 2019
@@ -1369,28 +1369,23 @@ CallEventManager::getCaller(const StackF
   const Stmt *CallSite = CalleeCtx->getCallSite();
 
   if (CallSite) {
-if (const CallExpr *CE = dyn_cast(CallSite))
-  return getSimpleCall(CE, State, CallerCtx);
+if (CallEventRef<> Out = getCall(CallSite, State, CallerCtx))
+  return Out;
 
-switch (CallSite->getStmtClass()) {
-case Stmt::CXXConstructExprClass:
-case Stmt::CXXTemporaryObjectExprClass: {
-  SValBuilder  = State->getStateManager().getSValBuilder();
-  const auto *Ctor = cast(CalleeCtx->getDecl());
-  Loc ThisPtr = SVB.getCXXThis(Ctor, CalleeCtx);
-  SVal ThisVal = State->getSVal(ThisPtr);
-
-  return getCXXConstructorCall(cast(CallSite),
-   ThisVal.getAsRegion(), State, CallerCtx);
-}
-case Stmt::CXXNewExprClass:
-  return getCXXAllocatorCall(cast(CallSite), State, CallerCtx);
-case Stmt::ObjCMessageExprClass:
-  return getObjCMethodCall(cast(CallSite),
-   State, CallerCtx);
-default:
-  llvm_unreachable("This is not an inlineable statement.");
-}
+Stmt::StmtClass SC = CallSite->getStmtClass();
+
+// All other cases are handled by getCall.
+assert(SC == Stmt::CXXConstructExprClass ||
+   SC == Stmt::CXXTemporaryObjectExprClass &&
+   "This is not an inlineable statement");
+
+SValBuilder  = State->getStateManager().getSValBuilder();
+const auto *Ctor = cast(CalleeCtx->getDecl());
+Loc ThisPtr = SVB.getCXXThis(Ctor, CalleeCtx);
+SVal ThisVal = State->getSVal(ThisPtr);
+
+return getCXXConstructorCall(cast(CallSite),
+ ThisVal.getAsRegion(), State, CallerCtx);
   }
 
   // Fall back to the CFG. The only thing we haven't handled yet is
@@ -1417,3 +1412,16 @@ CallEventManager::getCaller(const StackF
   E.getAs().hasValue(), State,
   CallerCtx);
 }
+
+CallEventRef<> CallEventManager::getCall(const Stmt *S, ProgramStateRef State,
+ const LocationContext *LC) {
+  if (const auto *CE = dyn_cast(S)) {
+return getSimpleCall(CE, State, LC);
+  } else if (const auto *NE = dyn_cast(S)) {
+return getCXXAllocatorCall(NE, State, LC);
+  } else if (const auto *ME = dyn_cast(S)) {
+return getObjCMethodCall(ME, State, LC);
+  } else {
+return nullptr;
+  }
+}


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

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

In D56554#1353572 , @krytarowski wrote:

> What do you think about this new logic:
>
> 1. specified `-z execstack` -> do not emit GNU STACK segment
> 2. specified `-z noexecstack` -> emit GNU STACK segment
> 3. no option specified -> detect `findSection(".note.GNU-stack")` (I might 
> miss offhand some details here to be sure if it is reliable) 3.1. detected -> 
> emit GNU STACK segment 3.2. not detected -> do not emit GNU STACK segment
>
>   Additionally we will specify explicitly in the clang driver for Linux and 
> FreeBSD `-z noexecstack`.


I think I don't like the very idea of using "marker sections" in input object 
files to change the linker behavior. That's too subtle and seems error-prone to 
me.

After thinking for a while, I started thinking that the first version of this 
patch is probably exactly what we want. I had been thinking that `-z 
{no,}execstack` and `-z {no,}gnustack` are four different options, but what we 
actually want to get is tri-state:

- emit PT_GNU_STACK with RWX permission
- emit PT_GNU_STACK with RW permission
- do not emit PT_GNU_STACK

So we could map them to `-z {execstack,noexecstack,nognustack}`, respectively, 
with default set to `-z noexecstack`. You guys can pass `-z nognustack` to the 
linker to tell the linker to not emit it at all. That's exactly this patch does.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56554



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

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

[Alexander Kornienko via Phabricator via cfe-commits]

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

Awesome! Thanks a lot ! LG


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

https://reviews.llvm.org/D54945



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

[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

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

rsmith added a comment.

In D55500#1347493 , @EricWF wrote:

> @rsmith, what sorts of additional tests are needed?


Let's see...

- test that we use the constant initializer for suitably-constant-initialized 
locals even if we can't constant-fold the reference to them:

  bool f(int k) {
const bool n = is_constant_evaluated();
const bool *volatile p = n;
return *p;
  }



- test that we suitably handle local references initialized by constant 
expressions (and local `constexpr` variables), and non-`const` globals
- test that `__builtin_is_constant_evaluated()` returns false in contexts that 
IR generation might constant-fold (eg, the operand of a non-`constexpr` `if` or 
the left-hand side of a `?:` expression)
- ensure that the right value is returned in all the cases that are 
syntactically //constant-expression//s in the C++ grammar:
  - second or subsequent array bound in a //new-expression//
  - `case` labels
  - operand of `static_assert`
  - array bounds (but be careful around the treatment of VLAs! for them we want 
the "try evaluating it as a constant expression and fall back to treating it as 
a non-constant expression" rule that we have for the initializers of globals)
  - values of enumerators
  - operand of `alignas`
  - width of a bit-field
  - template arguments (but bizarrely not default template arguments! I think 
that's a bug and I'll bring it up on the core reflector.)
  - operand of `noexcept` (and eventually `explicit` -- please write a test for 
the latter with a FIXME to update the expectations, so we don't forget)




Comment at: include/clang/Basic/Builtins.def:758
 
+// Random C++ builtins.
+LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG)

This should be grouped with `__builtin_launder`, though maybe moving that one 
here makes more sense than grouping them both under "GCC builtins" since 
they're directly implementing C++ stdlib functions.



Comment at: include/clang/Basic/Builtins.def:759
+// Random C++ builtins.
+LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG)
+

bruno wrote:
> EricWF wrote:
> > EricWF wrote:
> > > bruno wrote:
> > > > Name bikeshedding : perhaps the builtin name could be detached from the 
> > > > std:: name? Suggestion: `__builtin_in_constant_evaluation_context`
> > > I'm not sure detaching it from the `std::` name is desirable. Most 
> > > importantly it should match w/e GCC does/decides to do.
> > > 
> > > But if it is, we should name in deference to the standardese it 
> > > implements. Specifically weither an expression or conversion is 
> > > //manifestly constant-evaluated// 
> > > [[expr.const](http://eel.is/c++draft/expr.const#11)]p11.
> > > 
> > > Therefore I proffer  `__builtin_is_manifestly_constant_evaluated()` or 
> > > `__builtin_is_being_manifestly_constant_evaluated()`.
> > > 
> > > 
> > Actually, GCC has `__builtin_is_constant_evaluated` so we should use that 
> > name too.
> Agreed!
I don't think the "u" flag has any meaning here (there are no arguments, so no 
unevaluated arguments), and it would reduce the complexity of this declaration 
if you remove it.

I don't think the "c" flag is formally correct -- a program can contain two 
calls to `__builtin_is_constant_evaluated()` that return different values (even 
"U" appears to be incorrect). It would, for example, be reasonable for Clang to 
warn on:

```
const bool b = pure_fn();
return b == pure_fn();
```

... on the basis that the comparison must always evaluate to `true`. But such a 
warning would be incorrect for `__builtin_is_constant_evaluated`, so it's not a 
pure function.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55500



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

[PATCH] D56555: Add Attribute to define nonlazy objc classes

[Joe via Phabricator via cfe-commits]

joedaniels29 updated this revision to Diff 181371.
joedaniels29 added a comment.

Tried to fix Aaron Ballman's feedback. Thanks Aaron!


Repository:
  rC Clang

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

https://reviews.llvm.org/D56555

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenObjC/non-lazy-classes.m

Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -1,11 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
 // RUN: FileCheck %s
-// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [1 x {{.*}}] {{.*}}@"OBJC_CLASS_$_A"{{.*}}, section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
+// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [2 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
 // CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [1 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
 
-@interface A @end
+@interface A
+@end
 @implementation A
-+(void) load {
++ (void)load {
 }
 @end
 
@@ -27,6 +28,13 @@
 }
 @end
 
-@interface C : A @end
+@interface C : A
+@end
 @implementation C
 @end
+
+__attribute__((objc_nonlazy_class))
+@interface D @end
+
+@implementation D
+@end
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6374,6 +6374,9 @@
   case ParsedAttr::AT_ObjCRootClass:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_ObjCNonLazyClass:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_ObjCSubclassingRestricted:
 handleSimpleAttribute(S, D, AL);
 break;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6253,9 +6253,10 @@
   return GV;
 }
 
-bool
-CGObjCNonFragileABIMac::ImplementationIsNonLazy(const ObjCImplDecl *OD) const {
-  return OD->getClassMethod(GetNullarySelector("load")) != nullptr;
+bool CGObjCNonFragileABIMac::ImplementationIsNonLazy(
+const ObjCImplDecl *OD) const {
+  return OD->getClassMethod(GetNullarySelector("load")) != nullptr ||
+ OD->getClassInterface()->hasAttr();
 }
 
 void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCImplementationDecl *OID,
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3498,6 +3498,14 @@
   }];
 }
 
+def ObjCNonLazyClassDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{
+This attribute can be added to an Objective-C ``@interface`` declaration to
+add the class to the list of non-lazily initialized classes. These are classes
+that are realized at objc_init() time rather than when first referenced. 
+  }];
+}
 
 def SelectAnyDocs : Documentation {
   let Category = DocCatType;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1690,6 +1690,13 @@
   let Documentation = [Undocumented];
 }
 
+def ObjCNonLazyClass : Attr {
+  let Spellings = [Clang<"objc_nonlazy_class">];
+  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let LangOpts = [ObjC]
+  let Documentation = [Undocumented];
+}
+
 def ObjCSubclassingRestricted : InheritableAttr {
   let Spellings = [Clang<"objc_subclassing_restricted">];
   let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1350179 , @ruiu wrote:

> To be honest, I don't think I would lgtm a patch that changes lld's default 
> behavior depending on host/target only for NetBSD, and it seems like a 
> NetBSD's issue rather than an lld's issue. As I said, could you please make 
> an effort to pass the flags you need from the compiler driver to the linker 
> just like other systems do? That is easy to do, doesn't hurt anyone, probably 
> a good thing to do  anyway as it makes options explicit rather than implicit, 
> and solves at least most of the problems you have.


We will add proper mapping for other ELF targets and everybody will benefit 
from a standalone linker.

As mentioned previously it will be more than standalone as we need to introduce 
NetBSD specific customization into target binaries that is not easily mappable 
from linker options (unless someone wants to switch to Linux style ELF files on 
NetBSD, but it won't happen). From bigger changes we plan to change DT_NEEDED 
semantics for NetBSD and keep catching further integration issues.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Jennifer Yu via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

efriedma wrote:
> jyu2 wrote:
> > efriedma wrote:
> > > This looks suspicious; an AddrLabelExpr could be an input or output, e.g. 
> > > `"r"(&)`.
> > Syntax for asm goto:
> >  Syntax:
> >asm [volatile] goto ( AssemblerTemplate
> >:
> >: InputOperands
> >: Clobbers
> >: GotoLabels)
> > 
> >  Only input is allowed.  Output is not allowed
> > 
> That doesn't really address my point here... ignore the "or output" part of 
> the comment.
Sorry did not realize that.  Thank you so much for catching that.  Need to add 
other condition "ConstraintIdx > NS->getNumInputs() - 1", change to :

if (NS->isGCCAsmGoto() && ConstraintIdx > NS->getNumInputs() - 1 &&
Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
  break;

Is this ok with you?  Thanks


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

https://reviews.llvm.org/D56571



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Actually probably Linux MIPS used it and some proprietary OSes.. but we skip 
them for now. Only FreeBSD sets OSABI in lld and uses an emulation name 
detection hack. Once we will get merged TripleTarget detection we can switch it 
to use proper Triple check.


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

https://reviews.llvm.org/D56215



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 4 inline comments as done.
dankm added a comment.

In D49466#1354468 , @Lekensteyn wrote:

> Could you add more tests to check the error message for bad options (missing 
> `=`):
>
>   -fdebug-prefix-map=bad
>   -fmacro-prefix-map=bad
>   -ffile-prefix-map=bad
>


Some more got added with the latest diff

> FWIW, GCC emits two errors for `-ffile-prefix-map=bad`.

Yes, this does too. It looked odd to me, but it's not a huge deal.

> Another edge case is `-ffile-prefix-map==foo/`, GCC currently uses this to 
> prepend `foo/` to every path. Not sure if that is intentional, but that is 
> the current behavior (one which is also replicated by this patch I believe).

Yes, with this patch it does that for file-prefix-map and macro-prefix-map. It 
already did that (sort-of) for debug-prefix-map, but seems to add it twice for 
some debugging information, but I'll fix that later since it's done that since 
at least version 5.0.

> Could you also mark review comments that are completed as "done"? It should 
> make the diff easier to read (I hope) :)

Yes, I tried to do that with this comment. I'm new to phabricator.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 181363.
dankm added a comment.

renamed err_drv_invalid_argument_to_prefix_map to 
err_drv_invalid_argument_to_option
added more frontend tests for macro-prefix-map and file-prefix-map.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Lex/PreprocessorOptions.h
  lib/CodeGen/CGDebugInfo.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/Gnu.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp
  test/CodeGen/debug-prefix-map.c
  test/Driver/debug-prefix-map.S
  test/Driver/debug-prefix-map.c
  test/Driver/prefix-map.S
  test/Driver/prefix-map.c
  test/Preprocessor/file_test.c
  test/Preprocessor/file_test.h

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// RUN: %clang -E -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fmacro-prefix-map=%p/= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/prefix-map.c
===
--- /dev/null
+++ test/Driver/prefix-map.c
@@ -0,0 +1,28 @@
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
+
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-DEBUG-EMPTY: fdebug-prefix-map=old=
+// CHECK-MACRO-EMPTY: fmacro-prefix-map=old=
Index: test/Driver/prefix-map.S
===
--- /dev/null
+++ test/Driver/prefix-map.S
@@ -0,0 +1,7 @@
+// RUN: %clang -### -g -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s
+// RUN: %clang -### -g -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s
+
+// CHECK: cc1as
+// CHECK-SAME: -fdebug-prefix-map=old=new
+
+// More tests for this flag in debug-prefix-map.c.
Index: 

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Right, I'm not aware of anyone but FreeBSD really using the OSABI field. For 
FreeBSD it was a long standing hack around limitations in the ELF kernel 
loader. I'm not even sure if FreeBSD use(d) to set the OSABI field for the 
intermediate object files.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350970: [Darwin][Driver] Dont pass a file as 
object_path_lto during ThinLTO (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56608?vs=181325=181361#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/darwin-ld-lto.c


Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation , StringRef BaseName) const;
 
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {


Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return 

r350970 - [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via cfe-commits]

Author: steven_wu
Date: Fri Jan 11 13:16:04 2019
New Revision: 350970

URL: http://llvm.org/viewvc/llvm-project?rev=350970=rev
Log:
[Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

Summary:
After r327851, Driver::GetTemporaryPath will create the file rather than
just create a potientially unqine filename. If clang driver pass the
file as parameter as -object_path_lto, ld64 will pass it back to libLTO
as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
is not a directory.
Now during thinLTO, pass a temp directory path to linker instread.

rdar://problem/47194182

Reviewers: arphaman, dexonsmith

Reviewed By: arphaman

Subscribers: mehdi_amini, inglorion, jkorous, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Driver.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld-lto.c

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=350970=350969=350970=diff
==
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Fri Jan 11 13:16:04 2019
@@ -505,6 +505,10 @@ public:
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation , StringRef BaseName) const;
 

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=350970=350969=350970=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Fri Jan 11 13:16:04 2019
@@ -4478,6 +4478,17 @@ std::string Driver::GetTemporaryPath(Str
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=350970=350969=350970=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Fri Jan 11 13:16:04 2019
@@ -224,13 +224,20 @@ void darwin::Linker::AddLinkArgs(Compila
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);

Modified: cfe/trunk/test/Driver/darwin-ld-lto.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld-lto.c?rev=350970=350969=350970=diff
==
--- cfe/trunk/test/Driver/darwin-ld-lto.c (original)
+++ cfe/trunk/test/Driver/darwin-ld-lto.c Fri Jan 11 13:16:04 2019
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir 

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:375
+switch (Config->EMachine) {
+  case EM_386:
+Config->SearchPaths.push_back("=/usr/lib/i386");

As we have TargetTriple now, I would use it here instead of `Config->EMachine`, 
it will be easier to map knowledge from the Clang driver and handle special ABI 
cases.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1354603 , @krytarowski wrote:

> @mgorny could you check if we can get crossbuilding functional for:
>
> - to !NetBSD from NetBSD
> - from !NetBSD to NetBSD.
> - from NetBSD/amd64 to NetBSD/aarch64
>
>   I wonder whether it can work if we will keep using 'ld' file name for a 
> linker.


Actually we have discussed it internally with @mgorny and we will propose a 
patch in Clang to handle this easier/better.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:830-858
+  if (AteExtraColon || Tok.is(tok::colon)) {
+if (AteExtraColon)
+  AteExtraColon = false;
+else
+  ConsumeToken();
+
+if (!AteExtraColon && Tok.isNot(tok::identifier)) {

jyu2 wrote:
> nickdesaulniers wrote:
> > ```
> > if (x || y) {
> >   if (x) foo();
> >   else bar();
> >   if (!x && ...) baz();
> >   if (!x && ...) quux();
> > ```
> > is maybe more readable as:
> > ```
> > if (x) foo();
> > else if (y)
> >   bar();
> >   baz();
> >   quux();
> > ```
> This is better?
IIUC, it looks like `ConsumeToken()` modifies `Tok`, so the 2 `isNot` cases 
should fail as expected when `!AteExtraColon`, so this change LGTM.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56607: [clang] [NetBSD] Enable additional sanitizer types

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

mgorny updated this revision to Diff 181356.
mgorny added a comment.
Herald added a subscriber: cryptoad.

Updated the tests to account for most of the known sanitizer types.

Notes/TODO:

1. I wasn't able to get a sane match for `-fsanitize=undefined`, so I just 
check if it enables anything.
2. `-fsanitize=efficiency-all` is reported as unsupported even though it's in 
SanitizerKinds.


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

https://reviews.llvm.org/D56607

Files:
  lib/Driver/ToolChains/NetBSD.cpp
  test/Driver/fsanitize.c


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -709,28 +709,67 @@
 // RUN: %clang -target x86_64-unknown-cloudabi -fsanitize=safe-stack %s -### 
2>&1 | FileCheck %s -check-prefix=SAFESTACK-CLOUDABI
 // SAFESTACK-CLOUDABI: "-fsanitize=safe-stack"
 
+
+
+// * NetBSD; please keep ordered as in Sanitizers.def *
+
 // RUN: %clang -target i386--netbsd -fsanitize=address %s -### 2>&1 | 
FileCheck %s -check-prefix=ADDRESS-NETBSD
 // RUN: %clang -target x86_64--netbsd -fsanitize=address %s -### 2>&1 | 
FileCheck %s -check-prefix=ADDRESS-NETBSD
 // ADDRESS-NETBSD: "-fsanitize=address"
 
+// RUN: %clang -target x86_64--netbsd -fsanitize=kernel-address %s -### 2>&1 | 
FileCheck %s -check-prefix=KERNEL-ADDRESS-NETBSD
+// KERNEL-ADDRESS-NETBSD: "-fsanitize=kernel-address"
+
+// RUN: %clang -target x86_64--netbsd -fsanitize=hwaddress %s -### 2>&1 | 
FileCheck %s -check-prefix=HWADDRESS-NETBSD
+// HWADDRESS-NETBSD: "-fsanitize=hwaddress"
+
+// RUN: %clang -target x86_64--netbsd -fsanitize=kernel-hwaddress %s -### 2>&1 
| FileCheck %s -check-prefix=KERNEL-HWADDRESS-NETBSD
+// KERNEL-HWADDRESS-NETBSD: "-fsanitize=kernel-hwaddress"
+
+// RUN: %clang -target x86_64--netbsd -fsanitize=memory %s -### 2>&1 | 
FileCheck %s -check-prefix=MEMORY-NETBSD
+// MEMORY-NETBSD: "-fsanitize=memory"
+
+// RUN: %clang -target x86_64--netbsd -fsanitize=kernel-memory %s -### 2>&1 | 
FileCheck %s -check-prefix=KERNEL-MEMORY-NETBSD
+// KERNEL-MEMORY-NETBSD: "-fsanitize=kernel-memory"
+
+// RUN: %clang -target x86_64--netbsd -fsanitize=thread %s -### 2>&1 | 
FileCheck %s -check-prefix=THREAD-NETBSD
+// THREAD-NETBSD: "-fsanitize=thread"
+
+// RUN: %clang -target i386--netbsd -fsanitize=leak %s -### 2>&1 | FileCheck 
%s -check-prefix=LEAK-NETBSD
+// RUN: %clang -target x86_64--netbsd -fsanitize=leak %s -### 2>&1 | FileCheck 
%s -check-prefix=LEAK-NETBSD
+// LEAK-NETBSD: "-fsanitize=leak"
+
+// RUN: %clang -target i386--netbsd -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-NETBSD
+// RUN: %clang -target x86_64--netbsd -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-NETBSD
+// FUNCTION-NETBSD: "-fsanitize=function"
+
 // RUN: %clang -target i386--netbsd -fsanitize=vptr %s -### 2>&1 | FileCheck 
%s -check-prefix=VPTR-NETBSD
 // RUN: %clang -target x86_64--netbsd -fsanitize=vptr %s -### 2>&1 | FileCheck 
%s -check-prefix=VPTR-NETBSD
 // VPTR-NETBSD: "-fsanitize=vptr"
 
+// RUN: %clang -target x86_64--netbsd -fsanitize=dataflow %s -### 2>&1 | 
FileCheck %s -check-prefix=DATAFLOW-NETBSD
+// DATAFLOW-NETBSD: "-fsanitize=dataflow"
+
+// RUN: %clang -target i386--netbsd -fsanitize=cfi %s -### 2>&1 | FileCheck %s 
-check-prefix=CFI-NETBSD
+// RUN: %clang -target x86_64--netbsd -fsanitize=cfi %s -### 2>&1 | FileCheck 
%s -check-prefix=CFI-NETBSD
+// CFI-NETBSD: 
"-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
+
 // RUN: %clang -target i386--netbsd -fsanitize=safe-stack %s -### 2>&1 | 
FileCheck %s -check-prefix=SAFESTACK-NETBSD
 // RUN: %clang -target x86_64--netbsd -fsanitize=safe-stack %s -### 2>&1 | 
FileCheck %s -check-prefix=SAFESTACK-NETBSD
 // SAFESTACK-NETBSD: "-fsanitize=safe-stack"
 
-// RUN: %clang -target i386--netbsd -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-NETBSD
-// RUN: %clang -target x86_64--netbsd -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-NETBSD
-// FUNCTION-NETBSD: "-fsanitize=function"
+// RUN: %clang -target x86_64--netbsd -fsanitize=shadow-call-stack %s -### 
2>&1 | FileCheck %s -check-prefix=SHADOW-CALL-STACK-NETBSD
+// SHADOW-CALL-STACK-NETBSD: "-fsanitize=shadow-call-stack"
+
+// RUN: %clang -target i386--netbsd -fsanitize=undefined %s -### 2>&1 | 
FileCheck %s -check-prefix=UNDEFINED-NETBSD
+// RUN: %clang -target x86_64--netbsd -fsanitize=undefined %s -### 2>&1 | 
FileCheck %s -check-prefix=UNDEFINED-NETBSD
+// UNDEFINED-NETBSD: "-fsanitize={{.*}}
+
+// RUN: %clang -target i386--netbsd -fsanitize=scudo %s -### 2>&1 | FileCheck 
%s -check-prefix=SCUDO-NETBSD
+// RUN: %clang -target x86_64--netbsd -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s -check-prefix=SCUDO-NETBSD
+// SCUDO-NETBSD: "-fsanitize=scudo"
 
-// RUN: %clang -target i386--netbsd -fsanitize=leak %s -### 2>&1 | FileCheck 
%s -check-prefix=LEAK-NETBSD
-// RUN: 

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

jyu2 wrote:
> efriedma wrote:
> > This looks suspicious; an AddrLabelExpr could be an input or output, e.g. 
> > `"r"(&)`.
> Syntax for asm goto:
>  Syntax:
>asm [volatile] goto ( AssemblerTemplate
>:
>: InputOperands
>: Clobbers
>: GotoLabels)
> 
>  Only input is allowed.  Output is not allowed
> 
That doesn't really address my point here... ignore the "or output" part of the 
comment.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 marked 7 inline comments as done.
jyu2 added inline comments.



Comment at: lib/CodeGen/CGStmt.cpp:2182
+}
+  }
+

nickdesaulniers wrote:
> If this new block was moved closer to the new one on L2227, I assume they 
> could be combined and possibly `IsGCCAsmGoto` be removed?  The code currently 
> in between doesn't appear at first site to depend on info from this block, 
> though maybe I may be missing it.
The labels need be processed before Clobbers



Comment at: lib/Parse/ParseStmtAsm.cpp:830-858
+  if (AteExtraColon || Tok.is(tok::colon)) {
+if (AteExtraColon)
+  AteExtraColon = false;
+else
+  ConsumeToken();
+
+if (!AteExtraColon && Tok.isNot(tok::identifier)) {

nickdesaulniers wrote:
> ```
> if (x || y) {
>   if (x) foo();
>   else bar();
>   if (!x && ...) baz();
>   if (!x && ...) quux();
> ```
> is maybe more readable as:
> ```
> if (x) foo();
> else if (y)
>   bar();
>   baz();
>   quux();
> ```
This is better?



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

efriedma wrote:
> This looks suspicious; an AddrLabelExpr could be an input or output, e.g. 
> `"r"(&)`.
Syntax for asm goto:
 Syntax:
   asm [volatile] goto ( AssemblerTemplate
   :
   : InputOperands
   : Clobbers
   : GotoLabels)

 Only input is allowed.  Output is not allowed



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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 181342.
jyu2 added a comment.

1>  I add code for CFG.cpp and a test for that, as efriedman request.
2>  changes are respond the comments I received


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/ExprObjC.h
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/asm-goto.cpp
  test/CodeGen/asm-goto.c
  test/CodeGen/asm.c
  test/CodeGen/inline-asm-mixed-style.c
  test/Coverage/c-language-features.inc
  test/PCH/asm.h
  test/Parser/asm-goto.c
  test/Parser/asm-goto.cpp
  test/Parser/asm.c
  test/Parser/asm.cpp
  test/Sema/asm.c
  test/Sema/inline-asm-validate-tmpl.cpp

Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,21 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+}
Index: test/Parser/asm.cpp
===
--- test/Parser/asm.cpp
+++ test/Parser/asm.cpp
@@ -7,3 +7,28 @@
 int foo5 asm (U"bar5"); // expected-error {{cannot use unicode string literal in 'asm'}}
 int foo6 asm ("bar6"_x); // expected-error {{string literal with user-defined suffix cannot be used here}}
 int foo6 asm ("" L"bar7"); // expected-error {{cannot use wide string literal in 'asm'}}
+
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm goto ("testl %0, %0; jne %l3;" :: "r"(cond)::label_true, loop)
+  // expected-error@+1 {{unknown symbolic operand name in inline assembly string}}
+  asm goto ("decl %0; jnz %l[b]" :: "m"(x) : "memory" : a);
+label_true:
+loop:
+a:
+  return 0;
+}
Index: test/Parser/asm.c
===
--- test/Parser/asm.c
+++ test/Parser/asm.c
@@ -16,6 +16,31 @@
   asm _Atomic (""); // expected-warning {{ignored _Atomic qualifier on asm}}
 }
 
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1 {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1 {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm 

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/AST/DeclOpenMP.cpp:164
+  if (NumClauses) {
+Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+setClauses(CL);

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > No, bad idea. Use tail allocation for the clauses. Check the 
> > > > > > implementation of `OMPRequiresDecl`
> > > > > I think it is possible to use TrailingObjects for clause storage when 
> > > > > the number of clauses are known before creating the directive (e.g., 
> > > > > for OMPRequiresDecl and OMPExecutableDirective). 
> > > > > 
> > > > > The reason that I had to create OMPDeclareMapperDecl before parsing 
> > > > > map clauses, is the mapper variable (AA in the example below) needs 
> > > > > to be declared within OMPDeclareMapperDecl, because the following map 
> > > > > clauses will use it.
> > > > > 
> > > > > ```
> > > > > #pragma omp declare mapper(struct S AA) map(AA.field1)
> > > > > ```
> > > > > 
> > > > > A possible way to get around this is to count the number of map 
> > > > > clauses before hand. But this solution is not trivial since the 
> > > > > normal method for parsing map clauses cannot be used (e.g., it does 
> > > > > not know AA when parsing map(AA.field1)). A customized and complex 
> > > > > (because it needs to handle all possible situations) parsing method 
> > > > > needs to be created, just for counting clause number. I think it's 
> > > > > not worthy to do this compared with allocating map clause space later.
> > > > > 
> > > > > I checked the code for OMPDeclareReductionDecl that you wrote. It 
> > > > > also has to be created before parsing the combiner and initializer. 
> > > > > It does not have a variable number of clauses though.
> > > > > 
> > > > > Any suggestions?
> > > > Instead, you can introduce special DeclContext-based declaration and 
> > > > keep the reference to this declaration inside of the 
> > > > `OMPDeclareMapperDecl`.
> > > Hi Alexey,
> > > 
> > > Thanks a lot for your quick response! I don't think I understand your 
> > > idea. Can you establish more on that?
> > > 
> > > In my current implementation, OMPDeclareMapperDecl is used as the 
> > > DeclConext of the variable AA in the above example, and it already 
> > > includes the reference to AA's declaration.
> > > 
> > > My problem is, I need to create OMPDeclareMapperDecl before parsing map 
> > > clauses. But before parsing map clauses, I don't know the number of 
> > > clauses. Using TrailingObject requires to know how many clauses there are 
> > > when creating OMPDeclareMapperDecl. So I couldn't use TrailingObject.
> > > 
> > > My current solution is to create OMPDeclareMapperDecl before parsing map 
> > > clauses, and to create the clause storage after parsing finishes.
> > What I meant, that you don't need to use `OMPDeclareMapperDecl` for this, 
> > instead you can add another (very simple) special declaration based on 
> > `DeclContext` to use it as the parent declaration for the variable. In the 
> > `OMPDeclareMapperDecl` you can keep the reference to this special 
> > declaration.
> Thanks for your response! Please let me know if my understanding below is 
> correct:
> 
> `OMPDeclareMapperDecl` no longer inherits from `DeclContext`. Instead, we 
> create something like `OMPDeclareMapperDeclContext` which inherits from 
> `DeclContext`, and `OMPDeclareMapperDecl` keeps a pointer that points to this 
> `OMPDeclareMapperDeclContext`.  AA and map clauses are parsed within 
> `OMPDeclareMapperDeclContext`.
> 
> This sounds a bit more complex, but if you believe it's better, I can change 
> the code. Please share your thoughts.
Yes, something like this.


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

https://reviews.llvm.org/D56326



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

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/AST/DeclOpenMP.cpp:164
+  if (NumClauses) {
+Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+setClauses(CL);

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > No, bad idea. Use tail allocation for the clauses. Check the 
> > > > > implementation of `OMPRequiresDecl`
> > > > I think it is possible to use TrailingObjects for clause storage when 
> > > > the number of clauses are known before creating the directive (e.g., 
> > > > for OMPRequiresDecl and OMPExecutableDirective). 
> > > > 
> > > > The reason that I had to create OMPDeclareMapperDecl before parsing map 
> > > > clauses, is the mapper variable (AA in the example below) needs to be 
> > > > declared within OMPDeclareMapperDecl, because the following map clauses 
> > > > will use it.
> > > > 
> > > > ```
> > > > #pragma omp declare mapper(struct S AA) map(AA.field1)
> > > > ```
> > > > 
> > > > A possible way to get around this is to count the number of map clauses 
> > > > before hand. But this solution is not trivial since the normal method 
> > > > for parsing map clauses cannot be used (e.g., it does not know AA when 
> > > > parsing map(AA.field1)). A customized and complex (because it needs to 
> > > > handle all possible situations) parsing method needs to be created, 
> > > > just for counting clause number. I think it's not worthy to do this 
> > > > compared with allocating map clause space later.
> > > > 
> > > > I checked the code for OMPDeclareReductionDecl that you wrote. It also 
> > > > has to be created before parsing the combiner and initializer. It does 
> > > > not have a variable number of clauses though.
> > > > 
> > > > Any suggestions?
> > > Instead, you can introduce special DeclContext-based declaration and keep 
> > > the reference to this declaration inside of the `OMPDeclareMapperDecl`.
> > Hi Alexey,
> > 
> > Thanks a lot for your quick response! I don't think I understand your idea. 
> > Can you establish more on that?
> > 
> > In my current implementation, OMPDeclareMapperDecl is used as the 
> > DeclConext of the variable AA in the above example, and it already includes 
> > the reference to AA's declaration.
> > 
> > My problem is, I need to create OMPDeclareMapperDecl before parsing map 
> > clauses. But before parsing map clauses, I don't know the number of 
> > clauses. Using TrailingObject requires to know how many clauses there are 
> > when creating OMPDeclareMapperDecl. So I couldn't use TrailingObject.
> > 
> > My current solution is to create OMPDeclareMapperDecl before parsing map 
> > clauses, and to create the clause storage after parsing finishes.
> What I meant, that you don't need to use `OMPDeclareMapperDecl` for this, 
> instead you can add another (very simple) special declaration based on 
> `DeclContext` to use it as the parent declaration for the variable. In the 
> `OMPDeclareMapperDecl` you can keep the reference to this special declaration.
Thanks for your response! Please let me know if my understanding below is 
correct:

`OMPDeclareMapperDecl` no longer inherits from `DeclContext`. Instead, we 
create something like `OMPDeclareMapperDeclContext` which inherits from 
`DeclContext`, and `OMPDeclareMapperDecl` keeps a pointer that points to this 
`OMPDeclareMapperDeclContext`.  AA and map clauses are parsed within 
`OMPDeclareMapperDeclContext`.

This sounds a bit more complex, but if you believe it's better, I can change 
the code. Please share your thoughts.


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

https://reviews.llvm.org/D56326



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

[PATCH] D56463: [SEH] Pass the frame pointer from SEH finally to finally functions

[Sanjin Sijaric via Phabricator via cfe-commits]

ssijaric updated this revision to Diff 181337.
ssijaric added a comment.

Address formatting comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56463

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


Index: test/CodeGen/exceptions-seh-nested-finally.c
===
--- /dev/null
+++ test/CodeGen/exceptions-seh-nested-finally.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -triple x86_64-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 %s -triple aarch64-windows -fms-extensions -emit-llvm -o - \
+// RUN: | FileCheck %s
+
+// Check that the first finally block passes the enclosing function's frame
+// pointer to the second finally block, instead of generating it via localaddr.
+
+// CHECK-LABEL: define internal void @"?fin$0@0@main@@"({{i8( zeroext)?}} 
%abnormal_termination, i8* %frame_pointer)
+// CHECK: call void @"?fin$1@0@main@@"({{i8( zeroext)?}} 0, i8* %frame_pointer)
+int
+main() {
+  int Check = 0;
+  __try {
+Check = 3;
+  } __finally {
+__try {
+  Check += 2;
+} __finally {
+  Check += 4;
+}
+  }
+  return Check;
+}
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1627,8 +1627,16 @@
 
 // Compute the two argument values.
 QualType ArgTys[2] = {Context.UnsignedCharTy, Context.VoidPtrTy};
-llvm::Value *LocalAddrFn = CGM.getIntrinsic(llvm::Intrinsic::localaddress);
-llvm::Value *FP = CGF.Builder.CreateCall(LocalAddrFn);
+llvm::Value *FP = nullptr;
+// If CFG.IsOutlinedSEHHelper is true, then we are within a finally block.
+if (CGF.IsOutlinedSEHHelper) {
+  FP = >arg_begin()[1];
+} else {
+  llvm::Value *LocalAddrFn =
+  CGM.getIntrinsic(llvm::Intrinsic::localaddress);
+  FP = CGF.Builder.CreateCall(LocalAddrFn);
+}
+
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
 Args.add(RValue::get(IsForEH), ArgTys[0]);


Index: test/CodeGen/exceptions-seh-nested-finally.c
===
--- /dev/null
+++ test/CodeGen/exceptions-seh-nested-finally.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -triple x86_64-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 %s -triple aarch64-windows -fms-extensions -emit-llvm -o - \
+// RUN: | FileCheck %s
+
+// Check that the first finally block passes the enclosing function's frame
+// pointer to the second finally block, instead of generating it via localaddr.
+
+// CHECK-LABEL: define internal void @"?fin$0@0@main@@"({{i8( zeroext)?}} %abnormal_termination, i8* %frame_pointer)
+// CHECK: call void @"?fin$1@0@main@@"({{i8( zeroext)?}} 0, i8* %frame_pointer)
+int
+main() {
+  int Check = 0;
+  __try {
+Check = 3;
+  } __finally {
+__try {
+  Check += 2;
+} __finally {
+  Check += 4;
+}
+  }
+  return Check;
+}
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1627,8 +1627,16 @@
 
 // Compute the two argument values.
 QualType ArgTys[2] = {Context.UnsignedCharTy, Context.VoidPtrTy};
-llvm::Value *LocalAddrFn = CGM.getIntrinsic(llvm::Intrinsic::localaddress);
-llvm::Value *FP = CGF.Builder.CreateCall(LocalAddrFn);
+llvm::Value *FP = nullptr;
+// If CFG.IsOutlinedSEHHelper is true, then we are within a finally block.
+if (CGF.IsOutlinedSEHHelper) {
+  FP = >arg_begin()[1];
+} else {
+  llvm::Value *LocalAddrFn =
+  CGM.getIntrinsic(llvm::Intrinsic::localaddress);
+  FP = CGF.Builder.CreateCall(LocalAddrFn);
+}
+
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
 Args.add(RValue::get(IsForEH), ArgTys[0]);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D56608



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

[PATCH] D53541: [COFF, ARM64] Do not emit x86_seh_recoverfp intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 181333.

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

https://reviews.llvm.org/D53541

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


Index: test/CodeGen/exceptions-seh.c
===
--- test/CodeGen/exceptions-seh.c
+++ test/CodeGen/exceptions-seh.c
@@ -99,8 +99,7 @@
 // X64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture 
to i8*), i8* %[[fp]], i32 0)
 //
 // ARM64-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"(i8* 
%exception_pointers, i8* %frame_pointer)
-// ARM64: %[[fp:[^ ]*]] = call i8* @llvm.x86.seh.recoverfp(i8* bitcast (i32 
()* @filter_expr_capture to i8*), i8* %frame_pointer)
-// ARM64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* 
@filter_expr_capture to i8*), i8* %[[fp]], i32 0)
+// ARM64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* 
@filter_expr_capture to i8*), i8* %frame_pointer, i32 0)
 //
 // X86-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"()
 // X86: %[[ebp:[^ ]*]] = call i8* @llvm.frameaddress(i32 1)
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1783,7 +1783,8 @@
   }
 
   llvm::Value *ParentFP = EntryFP;
-  if (IsFilter) {
+  if (IsFilter &&
+  CGM.getTarget().getTriple().getArch() != llvm::Triple::aarch64) {
 // Given whatever FP the runtime provided us in EntryFP, recover the true
 // frame pointer of the parent function. We only need to do this in 
filters,
 // since finally funclets recover the parent FP for us.


Index: test/CodeGen/exceptions-seh.c
===
--- test/CodeGen/exceptions-seh.c
+++ test/CodeGen/exceptions-seh.c
@@ -99,8 +99,7 @@
 // X64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %[[fp]], i32 0)
 //
 // ARM64-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"(i8* %exception_pointers, i8* %frame_pointer)
-// ARM64: %[[fp:[^ ]*]] = call i8* @llvm.x86.seh.recoverfp(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %frame_pointer)
-// ARM64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %[[fp]], i32 0)
+// ARM64: call i8* @llvm.localrecover(i8* bitcast (i32 ()* @filter_expr_capture to i8*), i8* %frame_pointer, i32 0)
 //
 // X86-LABEL: define internal i32 @"?filt$0@0@filter_expr_capture@@"()
 // X86: %[[ebp:[^ ]*]] = call i8* @llvm.frameaddress(i32 1)
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1783,7 +1783,8 @@
   }
 
   llvm::Value *ParentFP = EntryFP;
-  if (IsFilter) {
+  if (IsFilter &&
+  CGM.getTarget().getTriple().getArch() != llvm::Triple::aarch64) {
 // Given whatever FP the runtime provided us in EntryFP, recover the true
 // frame pointer of the parent function. We only need to do this in filters,
 // since finally funclets recover the parent FP for us.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55492: Implement Attr dumping in terms of visitors

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350958: Implement Attr dumping in terms of visitors 
(authored by steveire, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55492?vs=181182=181334#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55492

Files:
  include/clang/AST/AttrVisitor.h
  include/clang/AST/CMakeLists.txt
  include/clang/AST/TextNodeDumper.h
  lib/AST/ASTDumper.cpp
  lib/AST/TextNodeDumper.cpp
  utils/TableGen/ClangAttrEmitter.cpp
  utils/TableGen/TableGen.cpp
  utils/TableGen/TableGenBackends.h

Index: include/clang/AST/AttrVisitor.h
===
--- include/clang/AST/AttrVisitor.h
+++ include/clang/AST/AttrVisitor.h
@@ -0,0 +1,76 @@
+//===- AttrVisitor.h - Visitor for Attr subclasses --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the AttrVisitor interface.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_ATTRVISITOR_H
+#define LLVM_CLANG_AST_ATTRVISITOR_H
+
+#include "clang/AST/Attr.h"
+
+namespace clang {
+
+namespace attrvisitor {
+
+/// A simple visitor class that helps create attribute visitors.
+template  class Ptr, typename ImplClass,
+  typename RetTy = void, class... ParamTys>
+class Base {
+public:
+#define PTR(CLASS) typename Ptr::type
+#define DISPATCH(NAME) \
+  return static_cast(this)->Visit##NAME(static_cast(A))
+
+  RetTy Visit(PTR(Attr) A) {
+switch (A->getKind()) {
+
+#define ATTR(NAME) \
+  case attr::NAME: \
+DISPATCH(NAME##Attr);
+#include "clang/Basic/AttrList.inc"
+}
+llvm_unreachable("Attr that isn't part of AttrList.inc!");
+  }
+
+  // If the implementation chooses not to implement a certain visit
+  // method, fall back to the parent.
+#define ATTR(NAME) \
+  RetTy Visit##NAME##Attr(PTR(NAME##Attr) A) { DISPATCH(Attr); }
+#include "clang/Basic/AttrList.inc"
+
+  RetTy VisitAttr(PTR(Attr)) { return RetTy(); }
+
+#undef PTR
+#undef DISPATCH
+};
+
+} // namespace attrvisitor
+
+/// A simple visitor class that helps create attribute visitors.
+///
+/// This class does not preserve constness of Attr pointers (see
+/// also ConstAttrVisitor).
+template 
+class AttrVisitor : public attrvisitor::Base {};
+
+/// A simple visitor class that helps create attribute visitors.
+///
+/// This class preserves constness of Attr pointers (see also
+/// AttrVisitor).
+template 
+class ConstAttrVisitor
+: public attrvisitor::Base {};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_ATTRVISITOR_H
Index: include/clang/AST/CMakeLists.txt
===
--- include/clang/AST/CMakeLists.txt
+++ include/clang/AST/CMakeLists.txt
@@ -8,10 +8,15 @@
   SOURCE ../Basic/Attr.td
   TARGET ClangAttrImpl)
 
-clang_tablegen(AttrDump.inc -gen-clang-attr-dump
+clang_tablegen(AttrTextNodeDump.inc -gen-clang-attr-text-node-dump
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
   SOURCE ../Basic/Attr.td
-  TARGET ClangAttrDump)
+  TARGET ClangAttrTextDump)
+
+clang_tablegen(AttrNodeTraverse.inc -gen-clang-attr-node-traverse
+  -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
+  SOURCE ../Basic/Attr.td
+  TARGET ClangAttrTraverse)
 
 clang_tablegen(AttrVisitor.inc -gen-clang-attr-ast-visitor
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
Index: include/clang/AST/TextNodeDumper.h
===
--- include/clang/AST/TextNodeDumper.h
+++ include/clang/AST/TextNodeDumper.h
@@ -16,6 +16,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDumperUtils.h"
+#include "clang/AST/AttrVisitor.h"
 #include "clang/AST/CommentCommandTraits.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/ExprCXX.h"
@@ -121,7 +122,8 @@
 class TextNodeDumper
 : public TextTreeStructure,
   public comments::ConstCommentVisitor {
+   const comments::FullComment *>,
+  public ConstAttrVisitor {
   raw_ostream 
   const bool ShowColors;
 
@@ -146,6 +148,8 @@
 
   void Visit(const comments::Comment *C, const comments::FullComment *FC);
 
+  void Visit(const Attr *A);
+
   void dumpPointer(const void *Ptr);
   void dumpLocation(SourceLocation Loc);
   void dumpSourceRange(SourceRange R);
@@ -179,6 +183,9 @@
 const comments::FullComment *);
   void 

r350958 - Implement Attr dumping in terms of visitors

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Fri Jan 11 11:16:01 2019
New Revision: 350958

URL: http://llvm.org/viewvc/llvm-project?rev=350958=rev
Log:
Implement Attr dumping in terms of visitors

Remove now-vestigial dumpType and dumpBareDeclRef methods. The old
tablegen generated code used to expect them to be present, but the new
generated code has no such requirement.

Reviewers: aaron.ballman

Subscribers: mgorny, cfe-commits

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

Added:
cfe/trunk/include/clang/AST/AttrVisitor.h
Modified:
cfe/trunk/include/clang/AST/CMakeLists.txt
cfe/trunk/include/clang/AST/TextNodeDumper.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/TextNodeDumper.cpp
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Added: cfe/trunk/include/clang/AST/AttrVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/AttrVisitor.h?rev=350958=auto
==
--- cfe/trunk/include/clang/AST/AttrVisitor.h (added)
+++ cfe/trunk/include/clang/AST/AttrVisitor.h Fri Jan 11 11:16:01 2019
@@ -0,0 +1,76 @@
+//===- AttrVisitor.h - Visitor for Attr subclasses --*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the AttrVisitor interface.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_ATTRVISITOR_H
+#define LLVM_CLANG_AST_ATTRVISITOR_H
+
+#include "clang/AST/Attr.h"
+
+namespace clang {
+
+namespace attrvisitor {
+
+/// A simple visitor class that helps create attribute visitors.
+template  class Ptr, typename ImplClass,
+  typename RetTy = void, class... ParamTys>
+class Base {
+public:
+#define PTR(CLASS) typename Ptr::type
+#define DISPATCH(NAME) 
\
+  return static_cast(this)->Visit##NAME(static_cast(A))
+
+  RetTy Visit(PTR(Attr) A) {
+switch (A->getKind()) {
+
+#define ATTR(NAME) 
\
+  case attr::NAME: 
\
+DISPATCH(NAME##Attr);
+#include "clang/Basic/AttrList.inc"
+}
+llvm_unreachable("Attr that isn't part of AttrList.inc!");
+  }
+
+  // If the implementation chooses not to implement a certain visit
+  // method, fall back to the parent.
+#define ATTR(NAME) 
\
+  RetTy Visit##NAME##Attr(PTR(NAME##Attr) A) { DISPATCH(Attr); }
+#include "clang/Basic/AttrList.inc"
+
+  RetTy VisitAttr(PTR(Attr)) { return RetTy(); }
+
+#undef PTR
+#undef DISPATCH
+};
+
+} // namespace attrvisitor
+
+/// A simple visitor class that helps create attribute visitors.
+///
+/// This class does not preserve constness of Attr pointers (see
+/// also ConstAttrVisitor).
+template 
+class AttrVisitor : public attrvisitor::Base {};
+
+/// A simple visitor class that helps create attribute visitors.
+///
+/// This class preserves constness of Attr pointers (see also
+/// AttrVisitor).
+template 
+class ConstAttrVisitor
+: public attrvisitor::Base {};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_ATTRVISITOR_H

Modified: cfe/trunk/include/clang/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CMakeLists.txt?rev=350958=350957=350958=diff
==
--- cfe/trunk/include/clang/AST/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/AST/CMakeLists.txt Fri Jan 11 11:16:01 2019
@@ -8,10 +8,15 @@ clang_tablegen(AttrImpl.inc -gen-clang-a
   SOURCE ../Basic/Attr.td
   TARGET ClangAttrImpl)
 
-clang_tablegen(AttrDump.inc -gen-clang-attr-dump
+clang_tablegen(AttrTextNodeDump.inc -gen-clang-attr-text-node-dump
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
   SOURCE ../Basic/Attr.td
-  TARGET ClangAttrDump)
+  TARGET ClangAttrTextDump)
+
+clang_tablegen(AttrNodeTraverse.inc -gen-clang-attr-node-traverse
+  -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
+  SOURCE ../Basic/Attr.td
+  TARGET ClangAttrTraverse)
 
 clang_tablegen(AttrVisitor.inc -gen-clang-attr-ast-visitor
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../

Modified: cfe/trunk/include/clang/AST/TextNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TextNodeDumper.h?rev=350958=350957=350958=diff
==
--- cfe/trunk/include/clang/AST/TextNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/TextNodeDumper.h Fri Jan 11 11:16:01 2019
@@ -16,6 +16,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include 

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/AST/DeclOpenMP.cpp:164
+  if (NumClauses) {
+Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+setClauses(CL);

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > No, bad idea. Use tail allocation for the clauses. Check the 
> > > > implementation of `OMPRequiresDecl`
> > > I think it is possible to use TrailingObjects for clause storage when the 
> > > number of clauses are known before creating the directive (e.g., for 
> > > OMPRequiresDecl and OMPExecutableDirective). 
> > > 
> > > The reason that I had to create OMPDeclareMapperDecl before parsing map 
> > > clauses, is the mapper variable (AA in the example below) needs to be 
> > > declared within OMPDeclareMapperDecl, because the following map clauses 
> > > will use it.
> > > 
> > > ```
> > > #pragma omp declare mapper(struct S AA) map(AA.field1)
> > > ```
> > > 
> > > A possible way to get around this is to count the number of map clauses 
> > > before hand. But this solution is not trivial since the normal method for 
> > > parsing map clauses cannot be used (e.g., it does not know AA when 
> > > parsing map(AA.field1)). A customized and complex (because it needs to 
> > > handle all possible situations) parsing method needs to be created, just 
> > > for counting clause number. I think it's not worthy to do this compared 
> > > with allocating map clause space later.
> > > 
> > > I checked the code for OMPDeclareReductionDecl that you wrote. It also 
> > > has to be created before parsing the combiner and initializer. It does 
> > > not have a variable number of clauses though.
> > > 
> > > Any suggestions?
> > Instead, you can introduce special DeclContext-based declaration and keep 
> > the reference to this declaration inside of the `OMPDeclareMapperDecl`.
> Hi Alexey,
> 
> Thanks a lot for your quick response! I don't think I understand your idea. 
> Can you establish more on that?
> 
> In my current implementation, OMPDeclareMapperDecl is used as the DeclConext 
> of the variable AA in the above example, and it already includes the 
> reference to AA's declaration.
> 
> My problem is, I need to create OMPDeclareMapperDecl before parsing map 
> clauses. But before parsing map clauses, I don't know the number of clauses. 
> Using TrailingObject requires to know how many clauses there are when 
> creating OMPDeclareMapperDecl. So I couldn't use TrailingObject.
> 
> My current solution is to create OMPDeclareMapperDecl before parsing map 
> clauses, and to create the clause storage after parsing finishes.
What I meant, that you don't need to use `OMPDeclareMapperDecl` for this, 
instead you can add another (very simple) special declaration based on 
`DeclContext` to use it as the parent declaration for the variable. In the 
`OMPDeclareMapperDecl` you can keep the reference to this special declaration.


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

https://reviews.llvm.org/D56326



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

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

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350957: [ASTDump] Add utility for dumping a label with child 
nodes (authored by steveire, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55488?vs=180935=181331#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55488

Files:
  include/clang/AST/TextNodeDumper.h
  lib/AST/ASTDumper.cpp
  test/AST/ast-dump-stmt.cpp


Index: include/clang/AST/TextNodeDumper.h
===
--- include/clang/AST/TextNodeDumper.h
+++ include/clang/AST/TextNodeDumper.h
@@ -41,6 +41,12 @@
 public:
   /// Add a child of the current node.  Calls DoAddChild without arguments
   template  void AddChild(Fn DoAddChild) {
+return AddChild("", DoAddChild);
+  }
+
+  /// Add a child of the current node with an optional label.
+  /// Calls DoAddChild without arguments.
+  template  void AddChild(StringRef Label, Fn DoAddChild) {
 // If we're at the top level, there's nothing interesting to do; just
 // run the dumper.
 if (TopLevel) {
@@ -56,7 +62,10 @@
   return;
 }
 
-auto DumpWithIndent = [this, DoAddChild](bool IsLastChild) {
+// We need to capture an owning-string in the lambda because the lambda
+// is invoked in a deferred manner.
+std::string LabelStr = Label;
+auto DumpWithIndent = [this, DoAddChild, LabelStr](bool IsLastChild) {
   // Print out the appropriate tree structure and work out the prefix for
   // children of this node. For instance:
   //
@@ -73,6 +82,9 @@
 OS << '\n';
 ColorScope Color(OS, ShowColors, IndentColor);
 OS << Prefix << (IsLastChild ? '`' : '|') << '-';
+if (!LabelStr.empty())
+  OS << LabelStr << ": ";
+
 this->Prefix.push_back(IsLastChild ? ' ' : '|');
 this->Prefix.push_back(' ');
   }
Index: test/AST/ast-dump-stmt.cpp
===
--- test/AST/ast-dump-stmt.cpp
+++ test/AST/ast-dump-stmt.cpp
@@ -91,8 +91,7 @@
   U us[3] = {1};
 // CHECK: VarDecl {{.+}}  col:5 us 'U [3]' cinit
 // CHECK-NEXT: `-InitListExpr {{.+}}  'U [3]'
-// CHECK-NEXT:   |-array filler
-// CHECK-NEXT:   | `-InitListExpr {{.+}}  'U' field Field {{.+}} 'i' 
'int'
+// CHECK-NEXT:   |-array_filler: InitListExpr {{.+}}  'U' field Field 
{{.+}} 'i' 'int'
 // CHECK-NEXT:   `-InitListExpr {{.+}}  'U' field Field {{.+}} 'i' 
'int'
 // CHECK-NEXT: `-IntegerLiteral {{.+}}  'int' 1
 }
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -61,6 +61,9 @@
 template void dumpChild(Fn DoDumpChild) {
   NodeDumper.AddChild(DoDumpChild);
 }
+template  void dumpChild(StringRef Label, Fn DoDumpChild) {
+  NodeDumper.AddChild(Label, DoDumpChild);
+}
 
   public:
 ASTDumper(raw_ostream , const CommandTraits *Traits,
@@ -80,7 +83,7 @@
 void setDeserialize(bool D) { Deserialize = D; }
 
 void dumpDecl(const Decl *D);
-void dumpStmt(const Stmt *S);
+void dumpStmt(const Stmt *S, StringRef Label = {});
 
 // Utilities
 void dumpType(QualType T) { NodeDumper.dumpType(T); }
@@ -1685,8 +1688,8 @@
 //  Stmt dumping methods.
 
//===--===//
 
-void ASTDumper::dumpStmt(const Stmt *S) {
-  dumpChild([=] {
+void ASTDumper::dumpStmt(const Stmt *S, StringRef Label) {
+  dumpChild(Label, [=] {
 if (!S) {
   ColorScope Color(OS, ShowColors, NullColor);
   OS << "<<>>";
@@ -1957,10 +1960,7 @@
 NodeDumper.dumpBareDeclRef(Field);
   }
   if (auto *Filler = ILE->getArrayFiller()) {
-dumpChild([=] {
-  OS << "array filler";
-  dumpStmt(Filler);
-});
+dumpStmt(Filler, "array_filler");
   }
 }
 


Index: include/clang/AST/TextNodeDumper.h
===
--- include/clang/AST/TextNodeDumper.h
+++ include/clang/AST/TextNodeDumper.h
@@ -41,6 +41,12 @@
 public:
   /// Add a child of the current node.  Calls DoAddChild without arguments
   template  void AddChild(Fn DoAddChild) {
+return AddChild("", DoAddChild);
+  }
+
+  /// Add a child of the current node with an optional label.
+  /// Calls DoAddChild without arguments.
+  template  void AddChild(StringRef Label, Fn DoAddChild) {
 // If we're at the top level, there's nothing interesting to do; just
 // run the dumper.
 if (TopLevel) {
@@ -56,7 +62,10 @@
   return;
 }
 
-auto DumpWithIndent = [this, DoAddChild](bool IsLastChild) {
+// We need to capture an owning-string in the lambda because the lambda
+// is invoked in a deferred manner.
+std::string LabelStr = Label;
+auto DumpWithIndent = [this, DoAddChild, LabelStr](bool IsLastChild) {
   // Print out the appropriate 

r350957 - [ASTDump] Add utility for dumping a label with child nodes

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Fri Jan 11 11:11:17 2019
New Revision: 350957

URL: http://llvm.org/viewvc/llvm-project?rev=350957=rev
Log:
[ASTDump] Add utility for dumping a label with child nodes

Summary:
Use it to add optional label nodes to Stmt dumps.  This preserves
behavior of InitExprList dump:

// CHECK-NEXT: `-InitListExpr {{.+}}  'U [3]'
// CHECK-NEXT:   |-array_filler: InitListExpr {{.+}}  'U' field Field 
{{.+}} 'i' 'int'
// CHECK-NEXT:   `-InitListExpr {{.+}}  'U' field Field {{.+}} 'i' 'int'
// CHECK-NEXT: `-IntegerLiteral {{.+}}  'int' 1

Reviewers: aaron.ballman

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/TextNodeDumper.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/test/AST/ast-dump-stmt.cpp

Modified: cfe/trunk/include/clang/AST/TextNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TextNodeDumper.h?rev=350957=350956=350957=diff
==
--- cfe/trunk/include/clang/AST/TextNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/TextNodeDumper.h Fri Jan 11 11:11:17 2019
@@ -41,6 +41,12 @@ class TextTreeStructure {
 public:
   /// Add a child of the current node.  Calls DoAddChild without arguments
   template  void AddChild(Fn DoAddChild) {
+return AddChild("", DoAddChild);
+  }
+
+  /// Add a child of the current node with an optional label.
+  /// Calls DoAddChild without arguments.
+  template  void AddChild(StringRef Label, Fn DoAddChild) {
 // If we're at the top level, there's nothing interesting to do; just
 // run the dumper.
 if (TopLevel) {
@@ -56,7 +62,10 @@ public:
   return;
 }
 
-auto DumpWithIndent = [this, DoAddChild](bool IsLastChild) {
+// We need to capture an owning-string in the lambda because the lambda
+// is invoked in a deferred manner.
+std::string LabelStr = Label;
+auto DumpWithIndent = [this, DoAddChild, LabelStr](bool IsLastChild) {
   // Print out the appropriate tree structure and work out the prefix for
   // children of this node. For instance:
   //
@@ -73,6 +82,9 @@ public:
 OS << '\n';
 ColorScope Color(OS, ShowColors, IndentColor);
 OS << Prefix << (IsLastChild ? '`' : '|') << '-';
+if (!LabelStr.empty())
+  OS << LabelStr << ": ";
+
 this->Prefix.push_back(IsLastChild ? ' ' : '|');
 this->Prefix.push_back(' ');
   }

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=350957=350956=350957=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Fri Jan 11 11:11:17 2019
@@ -61,6 +61,9 @@ namespace  {
 template void dumpChild(Fn DoDumpChild) {
   NodeDumper.AddChild(DoDumpChild);
 }
+template  void dumpChild(StringRef Label, Fn DoDumpChild) {
+  NodeDumper.AddChild(Label, DoDumpChild);
+}
 
   public:
 ASTDumper(raw_ostream , const CommandTraits *Traits,
@@ -80,7 +83,7 @@ namespace  {
 void setDeserialize(bool D) { Deserialize = D; }
 
 void dumpDecl(const Decl *D);
-void dumpStmt(const Stmt *S);
+void dumpStmt(const Stmt *S, StringRef Label = {});
 
 // Utilities
 void dumpType(QualType T) { NodeDumper.dumpType(T); }
@@ -1685,8 +1688,8 @@ void ASTDumper::VisitBlockDecl(const Blo
 //  Stmt dumping methods.
 
//===--===//
 
-void ASTDumper::dumpStmt(const Stmt *S) {
-  dumpChild([=] {
+void ASTDumper::dumpStmt(const Stmt *S, StringRef Label) {
+  dumpChild(Label, [=] {
 if (!S) {
   ColorScope Color(OS, ShowColors, NullColor);
   OS << "<<>>";
@@ -1957,10 +1960,7 @@ void ASTDumper::VisitInitListExpr(const
 NodeDumper.dumpBareDeclRef(Field);
   }
   if (auto *Filler = ILE->getArrayFiller()) {
-dumpChild([=] {
-  OS << "array filler";
-  dumpStmt(Filler);
-});
+dumpStmt(Filler, "array_filler");
   }
 }
 

Modified: cfe/trunk/test/AST/ast-dump-stmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-stmt.cpp?rev=350957=350956=350957=diff
==
--- cfe/trunk/test/AST/ast-dump-stmt.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-stmt.cpp Fri Jan 11 11:11:17 2019
@@ -91,8 +91,7 @@ void TestUnionInitList()
   U us[3] = {1};
 // CHECK: VarDecl {{.+}}  col:5 us 'U [3]' cinit
 // CHECK-NEXT: `-InitListExpr {{.+}}  'U [3]'
-// CHECK-NEXT:   |-array filler
-// CHECK-NEXT:   | `-InitListExpr {{.+}}  'U' field Field {{.+}} 'i' 
'int'
+// CHECK-NEXT:   |-array_filler: InitListExpr {{.+}}  'U' field Field 
{{.+}} 'i' 'int'
 // CHECK-NEXT:   `-InitListExpr {{.+}}  'U' field Field {{.+}} 'i' 
'int'
 // CHECK-NEXT: `-IntegerLiteral {{.+}}  'int' 1
 }

[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350955: [Bug 39548][Clang] PGO bootstrap fails with python3: 
errors in perf-helper.py (authored by serge_sans_paille, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54071?vs=181140=181327#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54071

Files:
  cfe/trunk/utils/perf-training/perf-helper.py


Index: cfe/trunk/utils/perf-training/perf-helper.py
===
--- cfe/trunk/utils/perf-training/perf-helper.py
+++ cfe/trunk/utils/perf-training/perf-helper.py
@@ -114,7 +114,7 @@
   # Find the cc1 command used by the compiler. To do this we execute the
   # compiler with '-###' to figure out what it wants to do.
   cmd = cmd + ['-###']
-  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, 
env=env).strip()
+  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env, 
universal_newlines=True).strip()
   cc_commands = []
   for ln in cc_output.split('\n'):
   # Filter out known garbage.
@@ -340,7 +340,7 @@
   # If the user gave us a binary, get all the symbols in the binary by
   # snarfing 'nm' output.
   if opts.binary_path is not None:
- output = subprocess.check_output(['nm', '-P', opts.binary_path])
+ output = subprocess.check_output(['nm', '-P', opts.binary_path], 
universal_newlines=True)
  lines = output.split("\n")
  all_symbols = [ln.split(' ',1)[0]
 for ln in lines


Index: cfe/trunk/utils/perf-training/perf-helper.py
===
--- cfe/trunk/utils/perf-training/perf-helper.py
+++ cfe/trunk/utils/perf-training/perf-helper.py
@@ -114,7 +114,7 @@
   # Find the cc1 command used by the compiler. To do this we execute the
   # compiler with '-###' to figure out what it wants to do.
   cmd = cmd + ['-###']
-  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env).strip()
+  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env, universal_newlines=True).strip()
   cc_commands = []
   for ln in cc_output.split('\n'):
   # Filter out known garbage.
@@ -340,7 +340,7 @@
   # If the user gave us a binary, get all the symbols in the binary by
   # snarfing 'nm' output.
   if opts.binary_path is not None:
- output = subprocess.check_output(['nm', '-P', opts.binary_path])
+ output = subprocess.check_output(['nm', '-P', opts.binary_path], universal_newlines=True)
  lines = output.split("\n")
  all_symbols = [ln.split(' ',1)[0]
 for ln in lines
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56318: [HIP] Fix size_t for MSVC environment

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 181326.
yaxunl added a comment.
Herald added a subscriber: jfb.

Copy type information from AuxTarget.


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

https://reviews.llvm.org/D56318

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Frontend/CompilerInstance.cpp
  test/SemaCUDA/amdgpu-size_t.cu

Index: test/SemaCUDA/amdgpu-size_t.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-size_t.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef unsigned __int64 size_t;
+typedef __int64 intptr_t;
+typedef unsigned __int64 uintptr_t;
+
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -929,6 +929,8 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
+  getTarget().copyAuxTarget(getAuxTarget());
+
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().ProgramAction == frontend::RewriteObjC)
 getTarget().noSignedCharForObjCBool();
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -260,6 +260,7 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  ShouldCopyAuxTarget = true;
 }
 
 void AMDGPUTargetInfo::adjust(LangOptions ) {
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -130,6 +130,7 @@
   // Default to an unknown platform name.
   PlatformName = "unknown";
   PlatformMinVersion = VersionTuple();
+  ShouldCopyAuxTarget = false;
 }
 
 // Out of line virtual dtor for TargetInfo.
@@ -796,3 +797,81 @@
   assert(getAccumIBits() >= getUnsignedAccumIBits());
   assert(getLongAccumIBits() >= getUnsignedLongAccumIBits());
 }
+
+void TargetInfo::copyAuxTarget(TargetInfo *Aux) {
+  if (!ShouldCopyAuxTarget)
+return;
+
+  PointerWidth = Aux->PointerWidth;
+  PointerAlign = Aux->PointerAlign;
+  BoolWidth = Aux->BoolWidth;
+  BoolAlign = Aux->BoolAlign;
+  IntWidth = Aux->IntWidth;
+  IntAlign = Aux->IntAlign;
+  LongWidth = Aux->LongWidth;
+  LongAlign = Aux->LongAlign;
+  LongLongWidth = Aux->LongLongWidth;
+  LongLongAlign = Aux->LongLongAlign;
+
+  // Fixed point default bit widths
+  ShortAccumWidth = Aux->ShortAccumWidth;
+  ShortAccumAlign = Aux->ShortAccumAlign;
+  AccumWidth = Aux->AccumWidth;
+  AccumAlign = Aux->AccumAlign;
+  LongAccumWidth = Aux->LongAccumWidth;
+  LongAccumAlign = Aux->LongAccumAlign;
+  ShortFractWidth = Aux->ShortFractWidth;
+  ShortFractAlign = Aux->ShortFractAlign;
+  FractWidth = Aux->FractWidth;
+  FractAlign = Aux->FractAlign;
+  LongFractWidth = Aux->LongFractWidth;
+  LongFractAlign = Aux->LongFractAlign;
+
+  // Fixed point default integral and fractional bit sizes
+  PaddingOnUnsignedFixedPoint = Aux->PaddingOnUnsignedFixedPoint;
+  ShortAccumScale = Aux->ShortAccumScale;
+  AccumScale = Aux->AccumScale;
+  LongAccumScale = Aux->LongAccumScale;
+
+  SuitableAlign = Aux->SuitableAlign;
+  DefaultAlignForAttributeAligned = Aux->DefaultAlignForAttributeAligned;
+  MinGlobalAlign = Aux->MinGlobalAlign;
+
+  NewAlign = Aux->NewAlign;
+
+  HalfWidth = Aux->HalfWidth;
+  HalfAlign = Aux->HalfAlign;
+  FloatWidth = Aux->FloatWidth;
+  FloatAlign = Aux->FloatAlign;
+  DoubleWidth = Aux->DoubleWidth;
+  DoubleAlign = Aux->DoubleAlign;
+  LongDoubleWidth = Aux->LongDoubleWidth;
+  LongDoubleAlign = Aux->LongDoubleAlign;
+  Float128Align = Aux->Float128Align;
+  LargeArrayMinWidth = Aux->LargeArrayMinWidth;
+  LargeArrayAlign = Aux->LargeArrayAlign;
+  MaxVectorAlign = Aux->MaxVectorAlign;
+  MaxTLSAlign = Aux->MaxTLSAlign;
+
+  SizeType = Aux->SizeType;
+  PtrDiffType = Aux->PtrDiffType;
+  IntMaxType = Aux->IntMaxType;
+  IntPtrType = Aux->IntPtrType;
+  WCharType = Aux->WCharType;
+  WIntType = Aux->WIntType;
+  Char16Type = Aux->Char16Type;
+  Char32Type = Aux->Char32Type;
+  Int64Type = Aux->Int64Type;
+  SigAtomicType = Aux->SigAtomicType;
+  ProcessIDType = Aux->ProcessIDType;
+  UseSignedCharForObjCBool = Aux->UseSignedCharForObjCBool;
+  UseBitFieldTypeAlignment = Aux->UseBitFieldTypeAlignment;
+  UseZeroLengthBitfieldAlignment = Aux->UseZeroLengthBitfieldAlignment;
+  UseExplicitBitFieldAlignment = Aux->UseExplicitBitFieldAlignment;
+  ZeroLengthBitfieldBoundary = Aux->ZeroLengthBitfieldBoundary;
+  HalfFormat = Aux->HalfFormat;
+  FloatFormat = Aux->FloatFormat;
+  DoubleFormat = Aux->DoubleFormat;
+  LongDoubleFormat = Aux->LongDoubleFormat;
+  Float128Format 

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 181325.
steven_wu added a comment.

Fix the comment


Repository:
  rC Clang

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

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/darwin-ld-lto.c


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation , StringRef BaseName) const;
 


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" "{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s 

r350955 - [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Fri Jan 11 11:04:48 2019
New Revision: 350955

URL: http://llvm.org/viewvc/llvm-project?rev=350955=rev
Log:
[Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

Current clang fail to bootstrap in PGO mode when only python3 is available,
because perf-helper.py is not compatible with python3.

Commited on behalf of  Romain Geissler.

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

Modified:
cfe/trunk/utils/perf-training/perf-helper.py

Modified: cfe/trunk/utils/perf-training/perf-helper.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/perf-training/perf-helper.py?rev=350955=350954=350955=diff
==
--- cfe/trunk/utils/perf-training/perf-helper.py (original)
+++ cfe/trunk/utils/perf-training/perf-helper.py Fri Jan 11 11:04:48 2019
@@ -114,7 +114,7 @@ def get_cc1_command_for_args(cmd, env):
   # Find the cc1 command used by the compiler. To do this we execute the
   # compiler with '-###' to figure out what it wants to do.
   cmd = cmd + ['-###']
-  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, 
env=env).strip()
+  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env, 
universal_newlines=True).strip()
   cc_commands = []
   for ln in cc_output.split('\n'):
   # Filter out known garbage.
@@ -340,7 +340,7 @@ def genOrderFile(args):
   # If the user gave us a binary, get all the symbols in the binary by
   # snarfing 'nm' output.
   if opts.binary_path is not None:
- output = subprocess.check_output(['nm', '-P', opts.binary_path])
+ output = subprocess.check_output(['nm', '-P', opts.binary_path], 
universal_newlines=True)
  lines = output.split("\n")
  all_symbols = [ln.split(' ',1)[0]
 for ln in lines


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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

@mgorny could you check if we can get crossbuilding functional for:

- to !NetBSD from NetBSD
- from !NetBSD to NetBSD.
- from NetBSD/amd64 to NetBSD/aarch64

I wonder whether it can work if we will keep using 'ld' file name for a linker.




Comment at: ELF/Driver.cpp:369
 
+void LinkerDriver::appendDefaultSearchPaths() {
+  if (Config->TargetTriple.isOSNetBSD()) {

From a stylistic point of view, I would introduce a dedicated file for the 
NetBSD driver (`DriverNetBSD.cpp`?) that overloads generic ` 
LinkerDriver::appendDefaultSearchPaths()`.

I have no opinion what C++ semantics to use for it.

The generic driver could be empty or use use 
`Config->SearchPaths.push_back("=/usr/lib");`. Probably empty is better as it 
would be more generic.


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

https://reviews.llvm.org/D56215



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

r350954 - [test] Update support for Exynos M4 (NFC)

[Evandro Menezes via cfe-commits]

Author: evandro
Date: Fri Jan 11 10:54:41 2019
New Revision: 350954

URL: http://llvm.org/viewvc/llvm-project?rev=350954=rev
Log:
[test] Update support for Exynos M4 (NFC)

Update test cases for Exynos M4.

Modified:
cfe/trunk/test/CodeGen/arm-target-features.c
cfe/trunk/test/Driver/aarch64-cpus.c
cfe/trunk/test/Driver/arm-cortex-cpus.c
cfe/trunk/test/Preprocessor/aarch64-target-features.c

Modified: cfe/trunk/test/CodeGen/arm-target-features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm-target-features.c?rev=350954=350953=350954=diff
==
--- cfe/trunk/test/CodeGen/arm-target-features.c (original)
+++ cfe/trunk/test/CodeGen/arm-target-features.c Fri Jan 11 10:54:41 2019
@@ -28,9 +28,11 @@
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m1 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m3 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8
-// RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m4 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8
 // CHECK-BASIC-V8: 
"target-features"="+armv8-a,+crc,+crypto,+dsp,+fp-armv8,+hwdiv,+hwdiv-arm,+neon,+thumb-mode"
 
+// RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m4 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
+// CHECK-BASIC-V82: 
"target-features"="+armv8.2-a,+crc,+crypto,+dotprod,+dsp,+fp-armv8,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode"
+
 // RUN: %clang_cc1 -triple armv8-linux-gnueabi -target-cpu cortex-a53 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8-ARM
 // CHECK-BASIC-V8-ARM: 
"target-features"="+armv8-a,+crc,+crypto,+dsp,+fp-armv8,+hwdiv,+hwdiv-arm,+neon,-thumb-mode"
 

Modified: cfe/trunk/test/Driver/aarch64-cpus.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-cpus.c?rev=350954=350953=350954=diff
==
--- cfe/trunk/test/Driver/aarch64-cpus.c (original)
+++ cfe/trunk/test/Driver/aarch64-cpus.c Fri Jan 11 10:54:41 2019
@@ -169,8 +169,9 @@
 // RUN: %clang -target aarch64_be -mtune=exynos-m4 -### -c %s 2>&1 | FileCheck 
-check-prefix=M4-TUNE %s
 // RUN: %clang -target aarch64 -mbig-endian -mtune=exynos-m4 -### -c %s 2>&1 | 
FileCheck -check-prefix=M4-TUNE %s
 // RUN: %clang -target aarch64_be -mbig-endian -mtune=exynos-m4 -### -c %s 
2>&1 | FileCheck -check-prefix=M4-TUNE %s
-// M4: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "exynos-m4"
+// M4: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "exynos-m4" 
"-target-feature" "+v8.2a"
 // M4-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// M4-TUNE-NOT: "+v8.2a"
 
 // RUN: %clang -target arm64 -mcpu=exynos-m1 -### -c %s 2>&1 | FileCheck 
-check-prefix=ARM64-M1 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=exynos-m1 -### -c %s 2>&1 | 
FileCheck -check-prefix=ARM64-M1 %s
@@ -197,8 +198,9 @@
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=exynos-m4 -### -c %s 2>&1 | 
FileCheck -check-prefix=ARM64-M4 %s
 // RUN: %clang -target arm64 -mtune=exynos-m4 -### -c %s 2>&1 | FileCheck 
-check-prefix=ARM64-M4-TUNE %s
 // RUN: %clang -target arm64 -mlittle-endian -mtune=exynos-m4 -### -c %s 2>&1 
| FileCheck -check-prefix=ARM64-M4-TUNE %s
-// ARM64-M4: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "exynos-m4"
+// ARM64-M4: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "exynos-m4" 
"-target-feature" "+v8.2a"
 // ARM64-M4-TUNE: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
+// ARM64-M4-TUNE-NOT: "+v8.2a"
 
 // RUN: %clang -target aarch64 -mcpu=falkor -### -c %s 2>&1 | FileCheck 
-check-prefix=FALKOR %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=falkor -### -c %s 2>&1 | 
FileCheck -check-prefix=FALKOR %s
@@ -338,8 +340,9 @@
 // RUN: %clang -target aarch64_be -mtune=exynos-m4 -### -c %s 2>&1 | FileCheck 
-check-prefix=M4-BE-TUNE %s
 // RUN: %clang -target aarch64 -mbig-endian -mtune=exynos-m4 -### -c %s 2>&1 | 
FileCheck -check-prefix=M4-BE-TUNE %s
 // RUN: %clang -target aarch64_be -mbig-endian -mtune=exynos-m4 -### -c %s 
2>&1 | FileCheck -check-prefix=M4-BE-TUNE %s
-// M4-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "exynos-m4"
+// M4-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "exynos-m4" 
"-target-feature" "+v8.2a"
 // M4-BE-TUNE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" 
"generic"
+// M4-BE-TUNE-NOT: "+v8.2a"
 
 // RUN: %clang -target aarch64_be -mcpu=thunderx2t99 -### -c %s 2>&1 | 
FileCheck -check-prefix=THUNDERX2T99-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -mcpu=thunderx2t99 -### -c %s 2>&1 
| FileCheck 

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Driver/Driver.h:508
 
+  /// GetTemporaryPath - Return the pathname of a temporary directory to use
+  /// as part of compilation; the directory will have the given prefix.

Old function name in the comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56608



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

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 181324.
steven_wu added a comment.

I was planning to add a test but I am not sure how to check the file type of 
temporary files.

I add a test to check for temp file names because I do create file and 
directory with different prefix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/darwin-ld-lto.c


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryPath - Return the pathname of a temporary directory to use
+  /// as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation , StringRef BaseName) const;
 


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s

r350952 - [MergeFunc] Update clang test for r350939

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Fri Jan 11 10:51:02 2019
New Revision: 350952

URL: http://llvm.org/viewvc/llvm-project?rev=350952=rev
Log:
[MergeFunc] Update clang test for r350939

In r350939, the MergeFunc pass learned to erase duplicate functions
which are discardable if unused.

Modified:
cfe/trunk/test/CodeGenCXX/merge-functions.cpp

Modified: cfe/trunk/test/CodeGenCXX/merge-functions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/merge-functions.cpp?rev=350952=350951=350952=diff
==
--- cfe/trunk/test/CodeGenCXX/merge-functions.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/merge-functions.cpp Fri Jan 11 10:51:02 2019
@@ -1,5 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -O1 -fmerge-functions 
-emit-llvm -o - -x c++ < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -O1 -fmerge-functions 
-emit-llvm -o - -x c++ < %s | FileCheck %s -implicit-check-not=_ZN1A1gEiPi
 
 // Basic functionality test. Function merging doesn't kick in on functions that
 // are too simple.
@@ -9,6 +9,4 @@ struct A {
   virtual int g(int x, int *p) { return x ? *p : 1; }
 } a;
 
-// CHECK: define {{.*}} @_ZN1A1gEiPi
-// CHECK-NEXT: tail call i32 @_ZN1A1fEiPi
-// CHECK-NEXT: ret
+// CHECK: define {{.*}} @_ZN1A1fEiPi


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

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

@ruiu actually if we can get D56215  merged we 
will be able to tune it specifically for NetBSD (with `if 
(Config->TargetTriple.isOSNetBSD()) {`) and retain intact the current 
Linux-biased logic for everybody who deserves to use it.

We will need to add similar NetBSD adjustments in other parts of lld.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56554



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

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I've added a few sample actions, please take a look.

The major thing that's missing from the design is how we can define an 
interface for actions to get the nodes they interested in from the AST without 
doing an AST traversal in each of the actions separately. I have a few options 
in mind, will be happy to explore more of this.
The other major problem that I've run into while playing around with the action 
in VSCode is increased latency: my suspicion is that the codeAction requests 
are not getting cancelled, I'll confirm and file an issue against VSCode if 
that's the case.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267



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

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 181321.
ilya-biryukov added a comment.

- Remove 'using namespace llvm'


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeActions.cpp
  clangd/CodeActions.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/ActionProvider.cpp
  clangd/refactor/ActionProvider.h
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -25,7 +25,8 @@
 # CHECK-NEXT:  "documentSymbolProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.applyCodeAction"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
Index: clangd/refactor/ActionProvider.h
===
--- /dev/null
+++ clangd/refactor/ActionProvider.h
@@ -0,0 +1,115 @@
+//===--- ActionProvider.h *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// The code actions are small refactoring-like actions that run over the AST and
+// produce the set of edits as a result. They are local, i.e. they should take
+// the current editor context, e.g. the cursor position and selection into
+// account.
+// The actions are executed in two stages:
+//   - Stage 1 should check whether the action is available in a current
+// context. It should be cheap and fast to compute as it is executed for all
+// available actions on every client request, which happen quite frequently.
+//   - Stage 2 is performed after stage 1 and can be more expensive to compute.
+// It is performed when the user actually chooses the action.
+// To avoid extra round-trips and AST reads, actions can also produce results on
+// stage 1 in cases when the actions are fast to compute.
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H
+
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+namespace clang {
+namespace clangd {
+
+class PreparedAction;
+struct ActionInputs;
+
+/// Base interface for writing a code action.
+class ActionProvider {
+public:
+  virtual ~ActionProvider() = default;
+
+  /// Run the first stage of the action. The non-None result indicates that the
+  /// action is available and should be shown to the user. Returns None if the
+  /// action is not available.
+  /// This function should be fast, if the action requires non-trivial work it
+  /// should be moved into the second stage, i.e. the continuation function
+  /// provided in the resulting PreparedAction.
+  virtual llvm::Optional
+  prepare(const ActionInputs ) = 0;
+};
+
+/// A context used by the actions to identify
+struct ActionInputs {
+  static llvm::Optional create(llvm::StringRef File,
+ llvm::StringRef Code,
+ ParsedAST , Range Selection);
+
+  /// The path of an active document the action was invoked in.
+  llvm::StringRef File;
+  /// The text of the active document.
+  llvm::StringRef Code;
+  /// Parsed AST of the active file.
+  ParsedAST 
+  /// A location of the cursor in the editor.
+  SourceLocation Cursor;
+  // FIXME: add selection when there are checks relying on it.
+  // FIXME: provide a way to get sources and ASTs for other files.
+  // FIXME: cache some commonly required information (e.g. AST nodes under
+  //cursor) to avoid redundant AST visit in every action.
+};
+
+using ActionID = size_t;
+/// Result of executing a first stage of the action. If computing the resulting
+/// WorkspaceEdit is fast, the actions should produce it right away.
+/// For non-trivial actions, a continuation function to compute the resulting
+/// edits should be provided instead. It is expected that PreparedAction is
+/// consumed immediately when created, so continuations can reference the AST,
+/// so it's not safe to store the PreparedAction for long spans of time.
+class PreparedAction {
+public:
+  PreparedAction(std::string Title,
+ llvm::unique_function()>
+   

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:770
+  // Start with a default initial triple
+  Config->TargetTriple = llvm::Triple(getDefaultTargetTriple());
+

krytarowski wrote:
> arichardson wrote:
> > arichardson wrote:
> > > If I invoke an unprefixed ld.lld on NetBSD but want to target a different 
> > > operating system, this will cause all the NetBSD defaults to apply to 
> > > that binary and will possibly cause it to crash at runtime.
> > > 
> > > I think any config changes based on a triple would need to use an 
> > > explicit --target= flag instead. As @ruiu says, LLD's behaviour should 
> > > not change depending on the host OS/default LLVM triple. Given the same 
> > > input files and command line options the resulting binary should be 
> > > identical on any host.
> > There needs to be a way to override the target triple that is not creating 
> > prefixed a symlink to ld.lld. Otherwise I can't use NetBSD ld.lld to build 
> > a non-NetBSD target without giving a value for every config option that lld 
> > supports.
> > 
> > I think there should be a command line option to override the triple (e.g. 
> > --triple= or --target=).
> > Also how will the default this interact with input files that have the 
> > OSABI field set? I feel like the options based on the target OSABI should 
> > be used instead of the default triple.
> OSABI field is not reliable way to detect OS/ABI. Everybody except FreeBSD 
> sets UNIX SystemV.
Actually there is a FreeBSD specific hack to detect emulation name, and it has 
suffix `fbsd`.. if it is detected it sets FreeBSD OSABI.

We don't have a chance to use a similar hack for NetBSD in other configuration 
options.


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

https://reviews.llvm.org/D56215



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

[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350949: [LTO] Add option to enable LTOUnit splitting, and 
disable unless needed (authored by tejohnson, committed by ).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D53891

Files:
  cfe/trunk/include/clang/Basic/CodeGenOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/SanitizerArgs.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  cfe/trunk/test/CodeGen/thinlto-distributed-cfi.ll
  cfe/trunk/test/CodeGenCXX/no-lto-unit.cpp
  cfe/trunk/test/CodeGenCXX/type-metadata-thinlto.cpp
  cfe/trunk/test/Driver/split-lto-unit.c

Index: cfe/trunk/include/clang/Driver/SanitizerArgs.h
===
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h
@@ -81,6 +81,7 @@
 
   bool requiresPIE() const;
   bool needsUnwindTables() const;
+  bool needsLTO() const;
   bool linkCXXRuntimes() const { return LinkCXXRuntimes; }
   bool hasCrossDsoCfi() const { return CfiCrossDso; }
   bool hasAnySanitizer() const { return !Sanitizers.empty(); }
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1776,6 +1776,11 @@
   HelpText<"Enables whole-program vtable optimization. Requires -flto">;
 def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, Group,
   Flags<[CoreOption]>;
+def fsplit_lto_unit : Flag<["-"], "fsplit-lto-unit">, Group,
+  Flags<[CoreOption, CC1Option]>,
+  HelpText<"Enables splitting of the LTO unit.">;
+def fno_split_lto_unit : Flag<["-"], "fno-split-lto-unit">, Group,
+  Flags<[CoreOption]>;
 def fforce_emit_vtables : Flag<["-"], "fforce-emit-vtables">, Group,
 Flags<[CC1Option]>,
 HelpText<"Emits more virtual tables to improve devirtualization">;
Index: cfe/trunk/include/clang/Basic/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Basic/CodeGenOptions.def
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.def
@@ -116,6 +116,10 @@
  ///< compile step.
 CODEGENOPT(LTOUnit, 1, 0) ///< Emit IR to support LTO unit features (CFI, whole
   ///< program vtable opt).
+CODEGENOPT(EnableSplitLTOUnit, 1, 0) ///< Enable LTO unit splitting to support
+ /// CFI and traditional whole program
+ /// devirtualization that require whole
+ /// program IR support.
 CODEGENOPT(IncrementalLinkerCompatible, 1, 0) ///< Emit an object file which can
   ///< be used with an incremental
   ///< linker.
Index: cfe/trunk/test/CodeGenCXX/type-metadata-thinlto.cpp
===
--- cfe/trunk/test/CodeGenCXX/type-metadata-thinlto.cpp
+++ cfe/trunk/test/CodeGenCXX/type-metadata-thinlto.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
+// RUN: %clang_cc1 -flto=thin -flto-unit -fsplit-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+// RUN: llvm-modextract -b -o - -n 1 %t | llvm-bcanalyzer -dump | FileCheck %s --check-prefix=LTOUNIT
+// LTOUNIT: 
 
 // CHECK: @_ZTV1A = linkonce_odr
 class A {
Index: cfe/trunk/test/CodeGenCXX/no-lto-unit.cpp
===
--- cfe/trunk/test/CodeGenCXX/no-lto-unit.cpp
+++ cfe/trunk/test/CodeGenCXX/no-lto-unit.cpp
@@ -2,6 +2,8 @@
 // RUN: llvm-dis -o - %t | FileCheck %s
 // RUN: %clang_cc1 -flto=thin -flto-unit -fno-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-dis -o - %t | FileCheck %s
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s --check-prefix=NOLTOUNIT
+// NOLTOUNIT: 
 
 // CHECK-NOT: !type
 class A {
Index: cfe/trunk/test/Driver/split-lto-unit.c
===
--- cfe/trunk/test/Driver/split-lto-unit.c
+++ cfe/trunk/test/Driver/split-lto-unit.c
@@ -0,0 +1,10 @@
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fsplit-lto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang 

r350949 - [LTO] Add option to enable LTOUnit splitting, and disable unless needed

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Fri Jan 11 10:32:07 2019
New Revision: 350949

URL: http://llvm.org/viewvc/llvm-project?rev=350949=rev
Log:
[LTO] Add option to enable LTOUnit splitting, and disable unless needed

Summary:
Adds a new -f[no]split-lto-unit flag that is disabled by default to
control module splitting during ThinLTO. It is automatically enabled
for -fsanitize=cfi and -fwhole-program-vtables.

The new EnableSplitLTOUnit codegen flag is passed down to llvm
via a new module flag of the same name.

Depends on D53890.

Reviewers: pcc

Subscribers: ormris, mehdi_amini, inglorion, eraman, steven_wu, dexonsmith, 
cfe-commits, llvm-commits

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

Added:
cfe/trunk/test/Driver/split-lto-unit.c
Modified:
cfe/trunk/include/clang/Basic/CodeGenOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
cfe/trunk/test/CodeGen/thinlto-distributed-cfi.ll
cfe/trunk/test/CodeGenCXX/no-lto-unit.cpp
cfe/trunk/test/CodeGenCXX/type-metadata-thinlto.cpp

Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.def?rev=350949=350948=350949=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.def Fri Jan 11 10:32:07 2019
@@ -116,6 +116,10 @@ CODEGENOPT(PrepareForThinLTO , 1, 0) ///
  ///< compile step.
 CODEGENOPT(LTOUnit, 1, 0) ///< Emit IR to support LTO unit features (CFI, whole
   ///< program vtable opt).
+CODEGENOPT(EnableSplitLTOUnit, 1, 0) ///< Enable LTO unit splitting to support
+/// CFI and traditional whole program
+/// devirtualization that require whole
+/// program IR support.
 CODEGENOPT(IncrementalLinkerCompatible, 1, 0) ///< Emit an object file which 
can
   ///< be used with an incremental
   ///< linker.

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=350949=350948=350949=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Jan 11 10:32:07 2019
@@ -1776,6 +1776,11 @@ def fwhole_program_vtables : Flag<["-"],
   HelpText<"Enables whole-program vtable optimization. Requires -flto">;
 def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, 
Group,
   Flags<[CoreOption]>;
+def fsplit_lto_unit : Flag<["-"], "fsplit-lto-unit">, Group,
+  Flags<[CoreOption, CC1Option]>,
+  HelpText<"Enables splitting of the LTO unit.">;
+def fno_split_lto_unit : Flag<["-"], "fno-split-lto-unit">, Group,
+  Flags<[CoreOption]>;
 def fforce_emit_vtables : Flag<["-"], "fforce-emit-vtables">, Group,
 Flags<[CC1Option]>,
 HelpText<"Emits more virtual tables to improve devirtualization">;

Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=350949=350948=350949=diff
==
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original)
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Fri Jan 11 10:32:07 2019
@@ -81,6 +81,7 @@ class SanitizerArgs {
 
   bool requiresPIE() const;
   bool needsUnwindTables() const;
+  bool needsLTO() const;
   bool linkCXXRuntimes() const { return LinkCXXRuntimes; }
   bool hasCrossDsoCfi() const { return CfiCrossDso; }
   bool hasAnySanitizer() const { return !Sanitizers.empty(); }

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=350949=350948=350949=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Fri Jan 11 10:32:07 2019
@@ -814,6 +814,8 @@ void EmitAssemblyHelper::EmitAssembly(Ba
 if (!ThinLinkOS)
   return;
   }
+  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+   CodeGenOpts.EnableSplitLTOUnit);
   PerModulePasses.add(createWriteThinLTOBitcodePass(
   *OS, ThinLinkOS ? >os() : nullptr));
 } else {
@@ -824,12 +826,15 @@ void 

Re: [PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Nick Desaulniers via cfe-commits]

On Thu, Jan 10, 2019 at 6:11 PM Yu, Jennifer  wrote:
>
> Syntax for asm goto:
> Syntax:
>   asm [volatile] goto ( AssemblerTemplate
>   :
>   : InputOperands
>   : Clobbers
>   : GotoLabels)
>
> Only input is allowed.  Output is not allowed

To match parity with GCC, yes.  Support for output is requested by
kernel developers, but it can understandably be left out of v1.

> Thanks.
> Jennifer
>
> -Original Message-
> From: Eli Friedman via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Thursday, January 10, 2019 5:58 PM
> To: craig.top...@gmail.com; Keane, Erich ; 
> ndesaulni...@google.com; chandl...@gmail.com; Yu, Jennifer 
> ; syaghm...@apple.com
> Cc: efrie...@codeaurora.org; srhi...@google.com; era...@google.com; 
> cfe-commits@lists.llvm.org
> Subject: [PATCH] D56571: [RFC prototype] Implementation of asm-goto support 
> in LLVM
>
> efriedma added a comment.
>
> Missing changes to lib/Analysis/CFG.cpp.
>
>
>
> 
> Comment at: lib/Sema/SemaStmtAsm.cpp:470
> +if (NS->isGCCAsmGoto() &&
> +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
> +  break;
> 
> This looks suspicious; an AddrLabelExpr could be an input or output, e.g. 
> `"r"(&)`.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D56571/new/
>
> https://reviews.llvm.org/D56571
>
>
>


-- 
Thanks,
~Nick Desaulniers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56612: [clangd] A code action to remove 'using namespace'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

The most complicated of the sample actions, requires considerable work and 
thorough testing before it can be landed.
Serves the purpose of illustrating how to write the two-stage actions.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56612



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

[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 181317.
tejohnson marked 4 inline comments as done.
tejohnson added a comment.

Address comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53891

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto-distributed-cfi-devirt.ll
  test/CodeGen/thinlto-distributed-cfi.ll
  test/CodeGenCXX/no-lto-unit.cpp
  test/CodeGenCXX/type-metadata-thinlto.cpp
  test/Driver/split-lto-unit.c

Index: test/Driver/split-lto-unit.c
===
--- /dev/null
+++ test/Driver/split-lto-unit.c
@@ -0,0 +1,10 @@
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fsplit-lto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=ERROR1 %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit -fsanitize=cfi 2>&1 | FileCheck --check-prefix=ERROR2 %s
+
+// UNIT: "-fsplit-lto-unit"
+// NOUNIT-NOT: "-fsplit-lto-unit"
+// ERROR1: error: invalid argument '-fno-split-lto-unit' not allowed with '-fwhole-program-vtables'
+// ERROR2: error: invalid argument '-fno-split-lto-unit' not allowed with '-fsanitize=cfi'
Index: test/CodeGenCXX/type-metadata-thinlto.cpp
===
--- test/CodeGenCXX/type-metadata-thinlto.cpp
+++ test/CodeGenCXX/type-metadata-thinlto.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
+// RUN: %clang_cc1 -flto=thin -flto-unit -fsplit-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+// RUN: llvm-modextract -b -o - -n 1 %t | llvm-bcanalyzer -dump | FileCheck %s --check-prefix=LTOUNIT
+// LTOUNIT: 
 
 // CHECK: @_ZTV1A = linkonce_odr
 class A {
Index: test/CodeGenCXX/no-lto-unit.cpp
===
--- test/CodeGenCXX/no-lto-unit.cpp
+++ test/CodeGenCXX/no-lto-unit.cpp
@@ -2,6 +2,8 @@
 // RUN: llvm-dis -o - %t | FileCheck %s
 // RUN: %clang_cc1 -flto=thin -flto-unit -fno-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-dis -o - %t | FileCheck %s
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s --check-prefix=NOLTOUNIT
+// NOLTOUNIT: 
 
 // CHECK-NOT: !type
 class A {
Index: test/CodeGen/thinlto-distributed-cfi.ll
===
--- test/CodeGen/thinlto-distributed-cfi.ll
+++ test/CodeGen/thinlto-distributed-cfi.ll
@@ -2,7 +2,7 @@
 
 ; Backend test for distribute ThinLTO with CFI.
 
-; RUN: opt -thinlto-bc -o %t.o %s
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
 ; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
 ; RUN:   -o %t2.index \
Index: test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -4,7 +4,7 @@
 ; It additionally enables -fwhole-program-vtables to get more information in
 ; TYPE_IDs of GLOBALVAL_SUMMARY_BLOCK.
 
-; RUN: opt -thinlto-bc -o %t.o %s
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
 ; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436.
 ; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -907,6 +907,7 @@
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S;
   }
   Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false);
+  Opts.EnableSplitLTOUnit = Args.hasArg(OPT_fsplit_lto_unit);
   if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) {
 if (IK.getLanguage() != InputKind::LLVM_IR)
   Diags.Report(diag::err_drv_argument_only_allowed_with)
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5226,6 +5226,17 @@
 CmdArgs.push_back("-fwhole-program-vtables");
   }
 
+  bool RequiresSplitLTOUnit = WholeProgramVTables || Sanitize.needsLTO();

[PATCH] D56612: [clangd] A code action to remove 'using namespace'

[Ilya Biryukov via Phabricator via cfe-commits]

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

Only available in the source files to fit into the model of single-file
actions. Doing the same in headers would require more complicated
machinery, which is out of scope of code actions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56612

Files:
  clangd/AST.cpp
  clangd/AST.h
  clangd/CMakeLists.txt
  clangd/CodeActions.cpp
  clangd/refactor/actions/RemoveUsingNamespace.cpp
  clangd/refactor/actions/RemoveUsingNamespace.h

Index: clangd/refactor/actions/RemoveUsingNamespace.h
===
--- /dev/null
+++ clangd/refactor/actions/RemoveUsingNamespace.h
@@ -0,0 +1,32 @@
+//===--- RemoveUsingNamespace.h --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that removes the 'using namespace' under the cursor and
+// qualifies all accesses in the current file. E.g.,
+//   using namespace std;
+//   vector foo(std::map);
+// Would become:
+//   std::vector foo(std::map);
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_REMOVEUSINGNAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_REMOVEUSINGNAMESPACE_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class RemoveUsingNamespace : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/refactor/actions/RemoveUsingNamespace.cpp
===
--- /dev/null
+++ clangd/refactor/actions/RemoveUsingNamespace.cpp
@@ -0,0 +1,189 @@
+//===--- RemoveUsingNamespace.cpp *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "RemoveUsingNamespace.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+class FindNodeUnderCursor : public RecursiveASTVisitor {
+public:
+  FindNodeUnderCursor(SourceLocation SearchedLoc, UsingDirectiveDecl *)
+  : SearchedLoc(SearchedLoc), Result(Result) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getUsingLoc() != SearchedLoc)
+  return true;
+
+Result = D;
+return false;
+  }
+
+private:
+  SourceLocation SearchedLoc;
+  UsingDirectiveDecl *
+};
+
+class FindSameUsings : public RecursiveASTVisitor {
+public:
+  FindSameUsings(UsingDirectiveDecl ,
+ std::vector )
+  : TargetNS(Target.getNominatedNamespace()),
+TargetCtx(Target.getDeclContext()), Results(Results) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getNominatedNamespace() != TargetNS ||
+D->getDeclContext() != TargetCtx)
+  return true;
+
+Results.push_back(D);
+return true;
+  }
+
+private:
+  NamespaceDecl *TargetNS;
+  DeclContext *TargetCtx;
+  std::vector 
+};
+
+class FindIdentsToQualify
+: public tooling::RecursiveSymbolVisitor {
+public:
+  FindIdentsToQualify(ASTContext , NamespaceDecl ,
+  std::vector )
+  : RecursiveSymbolVisitor(Ctx.getSourceManager(), Ctx.getLangOpts()),
+Ctx(Ctx), TargetNS(TargetNS), ResultIdents(ResultIdents) {}
+
+  bool visitSymbolOccurrence(const NamedDecl *D,
+ llvm::ArrayRef NameRanges) {
+if (!D || D->getCanonicalDecl() == TargetNS.getCanonicalDecl())
+  return true;
+if (!D->getDeclName().isIdentifier() ||
+D->getDeclName().getNameKind() == DeclarationName::CXXOperatorName)
+  return true; // do not add qualifiers for non-idents, e.g. 'operator+'.
+// Check the symbol is unqualified and references something inside our
+// namespace.
+// FIXME: add a check it's unqualified.
+if (!TargetNS.InEnclosingNamespaceSetOf(D->getDeclContext()))
+  return true;
+// FIXME: handle more tricky cases, e.g. we don't need the qualifier if we
+//have the using decls for some entities, we might have qualified
+//references that need updating too.
+for (auto R : NameRanges) {
+  

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

A deliberately simple syntactic transformation. Missing tests, but should work 
very reliably. To serve as an reference point for writing similar actions.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56611



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

[PATCH] D56463: [SEH] Pass the frame pointer from SEH finally to finally functions

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.



Comment at: lib/CodeGen/CGException.cpp:1635
+else {
+  llvm::Value *LocalAddrFn = 
CGM.getIntrinsic(llvm::Intrinsic::localaddress);
+  FP = CGF.Builder.CreateCall(LocalAddrFn);

80 char limit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56463



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

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/readability-identifier-naming.rst:34
+
+When defined, the checker will ensure abstract class names conform to the
+selected casing.

MyDeveloperDay wrote:
> Eugene.Zelenko wrote:
> > check, please. Same in other places.
> I assumed you mean replace "ensure" with "check", now I'm not sure do you 
> mean?
> 
> A) "the checker will check"
> B) "the check will ensure"
> C) "the check will check"
> 
I'm sorry for been ambiguous. I meant replacing checker with check.  Actually 
this wider problem: //check// is in Clang-tidy; //checker// - in Static Code 
Analyzer.


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

https://reviews.llvm.org/D56563



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

Re: r350768 - [ObjC] Allow the use of implemented unavailable methods from within

[Alex L via cfe-commits]

Thanks, we might have similar cases in our code base as well. We'll see if
we can fix that too.

On Fri, 11 Jan 2019 at 06:13, Nico Weber  wrote:

> Here's some user feedback on this new feature.
>
> It looks like the warning is only suppressed if `init` has a definition in
> the @interface block. In the 4 cases where we saw this warning fire after
> r349841, it still fires after this change because in all 4 cases a class
> marked init as unavailable in the @interface but didn't add a definition of
> it in the classes @implementation (instead relying on calling the
> superclass's (NSObject) init).
>
> It's pretty easy to just add
>
> - (instancetype)init { return [super init]; }
>
> to these 4 classes, but:
> a) it doesn't Just Work
> b) the diagnostic doesn't make it clear that adding a definition of init
> will suppress the warning.
>
> Up to you to decide what (if anything) to do with this feedback :-)
>
> (https://bugs.chromium.org/p/chromium/issues/detail?id=917351#c17 has a
> full reduced code snippet.)
>
> On Wed, Jan 9, 2019 at 5:35 PM Alex Lorenz via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: arphaman
>> Date: Wed Jan  9 14:31:37 2019
>> New Revision: 350768
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=350768=rev
>> Log:
>> [ObjC] Allow the use of implemented unavailable methods from within
>> the @implementation context
>>
>> In Objective-C, it's common for some frameworks to mark some methods like
>> init
>> as unavailable in the @interface to prohibit their usage. However, these
>> frameworks then often implemented said method and refer to it in another
>> method
>> that acts as a factory for that object. The recent change to how messages
>> to
>> self are type checked in clang (r349841) introduced a regression which
>> started
>> to prohibit this pattern with an X is unavailable error. This commit
>> addresses
>> the aforementioned regression.
>>
>> rdar://47134898
>>
>> Differential Revision: https://reviews.llvm.org/D56469
>>
>> Added:
>> cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
>> Modified:
>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=350768=350767=350768=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jan  9 14:31:37 2019
>> @@ -7269,9 +7269,10 @@ ShouldDiagnoseAvailabilityOfDecl(Sema 
>>  /// whether we should emit a diagnostic for \c K and \c DeclVersion in
>>  /// the context of \c Ctx. For example, we should emit an unavailable
>> diagnostic
>>  /// in a deprecated context, but not the other way around.
>> -static bool ShouldDiagnoseAvailabilityInContext(Sema ,
>> AvailabilityResult K,
>> -VersionTuple DeclVersion,
>> -Decl *Ctx) {
>> +static bool
>> +ShouldDiagnoseAvailabilityInContext(Sema , AvailabilityResult K,
>> +VersionTuple DeclVersion, Decl *Ctx,
>> +const NamedDecl *OffendingDecl) {
>>assert(K != AR_Available && "Expected an unavailable declaration
>> here!");
>>
>>// Checks if we should emit the availability diagnostic in the context
>> of C.
>> @@ -7280,9 +7281,22 @@ static bool ShouldDiagnoseAvailabilityIn
>>if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, C))
>>  if (AA->getIntroduced() >= DeclVersion)
>>return true;
>> -} else if (K == AR_Deprecated)
>> +} else if (K == AR_Deprecated) {
>>if (C->isDeprecated())
>>  return true;
>> +} else if (K == AR_Unavailable) {
>> +  // It is perfectly fine to refer to an 'unavailable' Objective-C
>> method
>> +  // when it's actually defined and is referenced from within the
>> +  // @implementation itself. In this context, we interpret
>> unavailable as a
>> +  // form of access control.
>> +  if (const auto *MD = dyn_cast(OffendingDecl)) {
>> +if (const auto *Impl = dyn_cast(C)) {
>> +  if (MD->getClassInterface() == Impl->getClassInterface() &&
>> +  MD->isDefined())
>> +return true;
>> +}
>> +  }
>> +}
>>
>>  if (C->isUnavailable())
>>return true;
>> @@ -7471,7 +7485,8 @@ static void DoEmitAvailabilityWarning(Se
>>if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context,
>> OffendingDecl))
>>  DeclVersion = AA->getIntroduced();
>>
>> -  if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx))
>> +  if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx,
>> +   OffendingDecl))
>>  return;
>>
>>SourceLocation Loc = Locs.front();
>> @@ -7955,7 +7970,8 @@ void DiagnoseUnguardedAvailability::Diag
>>
>>  

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56611

Files:
  clangd/CMakeLists.txt
  clangd/CodeActions.cpp
  clangd/refactor/actions/SwapIfBranches.cpp
  clangd/refactor/actions/SwapIfBranches.h

Index: clangd/refactor/actions/SwapIfBranches.h
===
--- /dev/null
+++ clangd/refactor/actions/SwapIfBranches.h
@@ -0,0 +1,31 @@
+//===--- SwapIfBrances.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that swaps the 'then' and 'else' branch of the if statement.
+// Before:
+//   if (foo) { return 10; } else { continue; }
+// After:
+//   if (foo) { continue; } else { return 10; }
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_SWAPIFBRANCHES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_SWAPIFBRANCHES_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class SwapIfBranches : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/SwapIfBranches.cpp
===
--- /dev/null
+++ clangd/refactor/actions/SwapIfBranches.cpp
@@ -0,0 +1,94 @@
+//===--- SwapIfBranches.cpp --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "SwapIfBranches.h"
+#include "ClangdUnit.h"
+#include "CodeActions.h"
+#include "Logger.h"
+#include "SourceCode.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+class FindIfUnderCursor : public RecursiveASTVisitor {
+public:
+  FindIfUnderCursor(ASTContext , SourceLocation CursorLoc, IfStmt *)
+  : Ctx(Ctx), CursorLoc(CursorLoc), Result(Result) {}
+
+  bool VisitIfStmt(IfStmt *If) {
+auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(),
+ SourceRange(If->getIfLoc()));
+if (!R)
+  return true;
+if (!halfOpenRangeContains(Ctx.getSourceManager(), *R, CursorLoc))
+  return true;
+Result = If;
+return false;
+  }
+
+private:
+  ASTContext 
+  SourceLocation CursorLoc;
+  IfStmt *
+};
+
+llvm::Optional
+SwapIfBranches::prepare(const ActionInputs ) {
+  auto  = Inputs.AST.getASTContext();
+  auto  = Ctx.getSourceManager();
+  IfStmt *If = nullptr;
+  FindIfUnderCursor(Ctx, Inputs.Cursor, If).TraverseAST(Ctx);
+  if (!If)
+return llvm::None;
+
+  // avoid dealing with single-statement brances, they require careful handling
+  // to avoid changing semantics of the code (i.e. dangling else).
+  if (!llvm::dyn_cast_or_null(If->getThen()) ||
+  !llvm::dyn_cast_or_null(If->getElse()))
+return llvm::None;
+
+  auto ThenRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getThen()->getSourceRange());
+  if (!ThenRng)
+return llvm::None;
+  auto ElseRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getElse()->getSourceRange());
+  if (!ElseRng)
+return llvm::None;
+
+  llvm::StringRef ThenCode = toSourceCode(SrcMgr, *ThenRng);
+  llvm::StringRef ElseCode = toSourceCode(SrcMgr, *ElseRng);
+
+  tooling::Replacements R;
+  if (auto Err = R.add(tooling::Replacement(SrcMgr, ThenRng->getBegin(),
+ThenCode.size(), ElseCode))) {
+llvm::consumeError(std::move(Err));
+return llvm::None;
+  }
+  if (auto Err = R.add(tooling::Replacement(SrcMgr, ElseRng->getBegin(),
+ElseCode.size(), ThenCode))) {
+llvm::consumeError(std::move(Err));
+return llvm::None;
+  }
+  return PreparedAction("Swap if branches", std::move(R));
+}
+} // namespace clangd
+} // 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This is a somewhat simple action to illustrate the use of code action APIs.
Still missing tests and trying to figure out what information we want to expose 
in order to avoid walking over ASTs in each of the actions, so this is not 
final. Should be a good reference point for how the code of the action would 
look like, though.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610



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

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 181312.
ilya-biryukov added a comment.

- Add some forgotten helpers


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/CodeActions.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/actions/QualifyName.cpp
  clangd/refactor/actions/QualifyName.h

Index: clangd/refactor/actions/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.h
@@ -0,0 +1,32 @@
+//===--- QualifyNameAction.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that fully qualifies a name under cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.cpp
@@ -0,0 +1,150 @@
+//===--- QualifyNameAction.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+return nullptr; // FIXME: handle this situation, retrieve the thing
+// referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+   // TranslationUnitDecl.
+return nullptr;
+return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+  return false;
+return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (NNS.getPrefix())
+  return true; // we want only unqualified names.
+auto  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+if (!Target)
+  return true; // continue traversal to recurse into types, if any.
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Target;
+return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (E->hasQualifier())
+  return true; // we want only unqualified names.
+auto  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 181311.
ilya-biryukov added a comment.

- Add the code to actually instantiate a code action


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/CodeActions.cpp
  clangd/refactor/actions/QualifyName.cpp
  clangd/refactor/actions/QualifyName.h

Index: clangd/refactor/actions/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.h
@@ -0,0 +1,32 @@
+//===--- QualifyNameAction.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that fully qualifies a name under cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.cpp
@@ -0,0 +1,150 @@
+//===--- QualifyNameAction.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+return nullptr; // FIXME: handle this situation, retrieve the thing
+// referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+   // TranslationUnitDecl.
+return nullptr;
+return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+  return false;
+return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (NNS.getPrefix())
+  return true; // we want only unqualified names.
+auto  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+if (!Target)
+  return true; // continue traversal to recurse into types, if any.
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Target;
+return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (E->hasQualifier())
+  return true; // we want only unqualified names.
+auto  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;

[PATCH] D56607: [clang] [NetBSD] Enable additional sanitizer types

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Please check these options in regression test-suite. There are also some 
missing entries and please add them too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56607



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

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/refactor/actions/QualifyName.cpp
  clangd/refactor/actions/QualifyName.h

Index: clangd/refactor/actions/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.h
@@ -0,0 +1,32 @@
+//===--- QualifyNameAction.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that fully qualifies a name under cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.cpp
@@ -0,0 +1,150 @@
+//===--- QualifyNameAction.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+return nullptr; // FIXME: handle this situation, retrieve the thing
+// referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+   // TranslationUnitDecl.
+return nullptr;
+return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+  return false;
+return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (NNS.getPrefix())
+  return true; // we want only unqualified names.
+auto  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+if (!Target)
+  return true; // continue traversal to recurse into types, if any.
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Target;
+return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (E->hasQualifier())
+  return true; // we want only unqualified names.
+auto  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = 

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 181309.
ilya-biryukov added a comment.

- Put more AST-centric information into ActionInputs
- Restructure and refactor the code


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeActions.cpp
  clangd/CodeActions.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/ActionProvider.cpp
  clangd/refactor/ActionProvider.h
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -25,7 +25,8 @@
 # CHECK-NEXT:  "documentSymbolProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.applyCodeAction"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
Index: clangd/refactor/ActionProvider.h
===
--- /dev/null
+++ clangd/refactor/ActionProvider.h
@@ -0,0 +1,115 @@
+//===--- ActionProvider.h *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// The code actions are small refactoring-like actions that run over the AST and
+// produce the set of edits as a result. They are local, i.e. they should take
+// the current editor context, e.g. the cursor position and selection into
+// account.
+// The actions are executed in two stages:
+//   - Stage 1 should check whether the action is available in a current
+// context. It should be cheap and fast to compute as it is executed for all
+// available actions on every client request, which happen quite frequently.
+//   - Stage 2 is performed after stage 1 and can be more expensive to compute.
+// It is performed when the user actually chooses the action.
+// To avoid extra round-trips and AST reads, actions can also produce results on
+// stage 1 in cases when the actions are fast to compute.
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H
+
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+namespace clang {
+namespace clangd {
+
+class PreparedAction;
+struct ActionInputs;
+
+/// Base interface for writing a code action.
+class ActionProvider {
+public:
+  virtual ~ActionProvider() = default;
+
+  /// Run the first stage of the action. The non-None result indicates that the
+  /// action is available and should be shown to the user. Returns None if the
+  /// action is not available.
+  /// This function should be fast, if the action requires non-trivial work it
+  /// should be moved into the second stage, i.e. the continuation function
+  /// provided in the resulting PreparedAction.
+  virtual llvm::Optional
+  prepare(const ActionInputs ) = 0;
+};
+
+/// A context used by the actions to identify
+struct ActionInputs {
+  static llvm::Optional create(llvm::StringRef File,
+ llvm::StringRef Code,
+ ParsedAST , Range Selection);
+
+  /// The path of an active document the action was invoked in.
+  llvm::StringRef File;
+  /// The text of the active document.
+  llvm::StringRef Code;
+  /// Parsed AST of the active file.
+  ParsedAST 
+  /// A location of the cursor in the editor.
+  SourceLocation Cursor;
+  // FIXME: add selection when there are checks relying on it.
+  // FIXME: provide a way to get sources and ASTs for other files.
+  // FIXME: cache some commonly required information (e.g. AST nodes under
+  //cursor) to avoid redundant AST visit in every action.
+};
+
+using ActionID = size_t;
+/// Result of executing a first stage of the action. If computing the resulting
+/// WorkspaceEdit is fast, the actions should produce it right away.
+/// For non-trivial actions, a continuation function to compute the resulting
+/// edits should be provided instead. It is expected that PreparedAction is
+/// consumed immediately when created, so continuations can reference the AST,
+/// so it's not safe to store the PreparedAction for long spans of time.
+class PreparedAction {
+public:
+  PreparedAction(std::string Title,
+ 

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

The code looks good.  Can you add a test too?  Might need to require “shell”.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56608



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:770
+  // Start with a default initial triple
+  Config->TargetTriple = llvm::Triple(getDefaultTargetTriple());
+

arichardson wrote:
> arichardson wrote:
> > If I invoke an unprefixed ld.lld on NetBSD but want to target a different 
> > operating system, this will cause all the NetBSD defaults to apply to that 
> > binary and will possibly cause it to crash at runtime.
> > 
> > I think any config changes based on a triple would need to use an explicit 
> > --target= flag instead. As @ruiu says, LLD's behaviour should not change 
> > depending on the host OS/default LLVM triple. Given the same input files 
> > and command line options the resulting binary should be identical on any 
> > host.
> There needs to be a way to override the target triple that is not creating 
> prefixed a symlink to ld.lld. Otherwise I can't use NetBSD ld.lld to build a 
> non-NetBSD target without giving a value for every config option that lld 
> supports.
> 
> I think there should be a command line option to override the triple (e.g. 
> --triple= or --target=).
> Also how will the default this interact with input files that have the OSABI 
> field set? I feel like the options based on the target OSABI should be used 
> instead of the default triple.
OSABI field is not reliable way to detect OS/ABI. Everybody except FreeBSD sets 
UNIX SystemV.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/AST/DeclOpenMP.cpp:164
+  if (NumClauses) {
+Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+setClauses(CL);

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > No, bad idea. Use tail allocation for the clauses. Check the 
> > > implementation of `OMPRequiresDecl`
> > I think it is possible to use TrailingObjects for clause storage when the 
> > number of clauses are known before creating the directive (e.g., for 
> > OMPRequiresDecl and OMPExecutableDirective). 
> > 
> > The reason that I had to create OMPDeclareMapperDecl before parsing map 
> > clauses, is the mapper variable (AA in the example below) needs to be 
> > declared within OMPDeclareMapperDecl, because the following map clauses 
> > will use it.
> > 
> > ```
> > #pragma omp declare mapper(struct S AA) map(AA.field1)
> > ```
> > 
> > A possible way to get around this is to count the number of map clauses 
> > before hand. But this solution is not trivial since the normal method for 
> > parsing map clauses cannot be used (e.g., it does not know AA when parsing 
> > map(AA.field1)). A customized and complex (because it needs to handle all 
> > possible situations) parsing method needs to be created, just for counting 
> > clause number. I think it's not worthy to do this compared with allocating 
> > map clause space later.
> > 
> > I checked the code for OMPDeclareReductionDecl that you wrote. It also has 
> > to be created before parsing the combiner and initializer. It does not have 
> > a variable number of clauses though.
> > 
> > Any suggestions?
> Instead, you can introduce special DeclContext-based declaration and keep the 
> reference to this declaration inside of the `OMPDeclareMapperDecl`.
Hi Alexey,

Thanks a lot for your quick response! I don't think I understand your idea. Can 
you establish more on that?

In my current implementation, OMPDeclareMapperDecl is used as the DeclConext of 
the variable AA in the above example, and it already includes the reference to 
AA's declaration.

My problem is, I need to create OMPDeclareMapperDecl before parsing map 
clauses. But before parsing map clauses, I don't know the number of clauses. 
Using TrailingObject requires to know how many clauses there are when creating 
OMPDeclareMapperDecl. So I couldn't use TrailingObject.

My current solution is to create OMPDeclareMapperDecl before parsing map 
clauses, and to create the clause storage after parsing finishes.


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

https://reviews.llvm.org/D56326



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

[PATCH] D56581: [ASTImporter] Set the described template if not set

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Can you add a test?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56581



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

r350942 - [attributes] Extend os_returns_(not_?)_retained attributes to parameters

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Jan 11 10:02:08 2019
New Revision: 350942

URL: http://llvm.org/viewvc/llvm-project?rev=350942=rev
Log:
[attributes] Extend os_returns_(not_?)_retained attributes to parameters

When applied to out-parameters, the attributes specify the expected lifetime of 
the written-into object.

Additionally, introduce OSReturnsRetainedOn(Non)Zero attributes, which
specify that an ownership transfer happens depending on a return code.

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/Sema/attr-osobject.cpp
cfe/trunk/test/Sema/attr-osobject.mm

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=350942=350941=350942=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Jan 11 10:02:08 2019
@@ -841,13 +841,25 @@ def OSConsumed : InheritableParamAttr {
 
 def OSReturnsRetained : InheritableAttr {
   let Spellings = [Clang<"os_returns_retained">];
-  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
+  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty, ParmVar]>;
   let Documentation = [RetainBehaviorDocs];
 }
 
 def OSReturnsNotRetained : InheritableAttr {
   let Spellings = [Clang<"os_returns_not_retained">];
-  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
+  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty, ParmVar]>;
+  let Documentation = [RetainBehaviorDocs];
+}
+
+def OSReturnsRetainedOnZero : InheritableAttr {
+  let Spellings = [Clang<"os_returns_retained_on_zero">];
+  let Subjects = SubjectList<[ParmVar]>;
+  let Documentation = [RetainBehaviorDocs];
+}
+
+def OSReturnsRetainedOnNonZero : InheritableAttr {
+  let Spellings = [Clang<"os_returns_retained_on_non_zero">];
+  let Subjects = SubjectList<[ParmVar]>;
   let Documentation = [RetainBehaviorDocs];
 }
 

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=350942=350941=350942=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Fri Jan 11 10:02:08 2019
@@ -888,6 +888,21 @@ Similar to ``__attribute__((ns_consumes_
 ``__attribute__((os_consumes_this))`` specifies that the method call consumes
 the reference to "this" (e.g., when attaching it to a different object supplied
 as a parameter).
+Out parameters (parameters the function is meant to write into,
+either via pointers-to-pointers or references-to-pointers)
+may be annotated with ``__attribute__((os_returns_retained))``
+or ``__attribute__((os_returns_not_retained))`` which specifies that the object
+written into the out parameter should (or respectively should not) be released
+after use.
+Since often out parameters may or may not be written depending on the exit
+code of the function,
+annotations ``__attribute__((os_returns_retained_on_zero))``
+and ``__attribute__((os_returns_retained_on_non_zero))`` specify that
+an out parameter at ``+1`` is written if and only if the function returns a 
zero
+(respectively non-zero) error code.
+Observe that return-code-dependent out parameter annotations are only
+available for retained out parameters, as non-retained object do not have to be
+released by the callee.
 These attributes are only used by the Clang Static Analyzer.
 
 The family of attributes ``X_returns_X_retained`` can be added to functions,

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=350942=350941=350942=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 11 10:02:08 
2019
@@ -3425,7 +3425,7 @@ def err_ns_attribute_wrong_parameter_typ
   "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">;
 def warn_ns_attribute_wrong_parameter_type : Warning<
   "%0 attribute only applies to "
-  "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">,
+  "%select{Objective-C 
object|pointer|pointer-to-CF-pointer|pointer/reference-to-OSObject-pointer}1 
parameters">,
   InGroup;
 def warn_objc_requires_super_protocol : Warning<
   "%0 attribute cannot be applied to %select{methods in protocols|dealloc}1">,

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 

r350941 - Fix a pair of Wfallthrough warnings in ScanfFormatString.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Jan 11 10:01:40 2019
New Revision: 350941

URL: http://llvm.org/viewvc/llvm-project?rev=350941=rev
Log:
Fix a pair of Wfallthrough warnings in ScanfFormatString.

Change-Id: Ia73a34fdd93fc974224583505f9e6432493cb0da

Modified:
cfe/trunk/lib/AST/ScanfFormatString.cpp

Modified: cfe/trunk/lib/AST/ScanfFormatString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ScanfFormatString.cpp?rev=350941=350940=350941=diff
==
--- cfe/trunk/lib/AST/ScanfFormatString.cpp (original)
+++ cfe/trunk/lib/AST/ScanfFormatString.cpp Fri Jan 11 10:01:40 2019
@@ -264,6 +264,7 @@ ArgType ScanfSpecifier::getArgType(ASTCo
 case LengthModifier::AsWide:
   return ArgType::Invalid();
   }
+  llvm_unreachable("Unsupported LenghtModifier Type");
 
 // Unsigned int.
 case ConversionSpecifier::oArg:
@@ -303,6 +304,7 @@ ArgType ScanfSpecifier::getArgType(ASTCo
 case LengthModifier::AsWide:
   return ArgType::Invalid();
   }
+  llvm_unreachable("Unsupported LenghtModifier Type");
 
 // Float.
 case ConversionSpecifier::aArg:


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

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM

Thank you for adding the additional test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53699



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Could you add more tests to check the error message for bad options (missing 
`=`):

  -fdebug-prefix-map=bad
  -fmacro-prefix-map=bad
  -ffile-prefix-map=bad

FWIW, GCC emits two errors for `-ffile-prefix-map=bad`.

Another edge case is `-ffile-prefix-map==foo/`, GCC currently uses this to 
prepend `foo/` to every path. Not sure if that is intentional, but that is the 
current behavior (one which is also replicated by this patch I believe).

Could you also mark review comments that are completed as "done"? It should 
make the diff easier to read (I hope) :)




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:118
   "invalid deployment target for -stdlib=libc++ (requires %0 or later)">;
-def err_drv_invalid_argument_to_fdebug_prefix_map : Error<
-  "invalid argument '%0' to -fdebug-prefix-map">;
+def err_drv_invalid_argument_to_prefix_map : Error<
+  "invalid argument '%1' to -%0">;

Maybe rename `_to_prefix_map` to `_to_option`? (And maybe swap the order of 
parameters so `%0` comes before `%1`?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Alexander Richardson via Phabricator via cfe-commits]

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



Comment at: ELF/Driver.cpp:770
+  // Start with a default initial triple
+  Config->TargetTriple = llvm::Triple(getDefaultTargetTriple());
+

arichardson wrote:
> If I invoke an unprefixed ld.lld on NetBSD but want to target a different 
> operating system, this will cause all the NetBSD defaults to apply to that 
> binary and will possibly cause it to crash at runtime.
> 
> I think any config changes based on a triple would need to use an explicit 
> --target= flag instead. As @ruiu says, LLD's behaviour should not change 
> depending on the host OS/default LLVM triple. Given the same input files and 
> command line options the resulting binary should be identical on any host.
There needs to be a way to override the target triple that is not creating 
prefixed a symlink to ld.lld. Otherwise I can't use NetBSD ld.lld to build a 
non-NetBSD target without giving a value for every config option that lld 
supports.

I think there should be a command line option to override the triple (e.g. 
--triple= or --target=).
Also how will the default this interact with input files that have the OSABI 
field set? I feel like the options based on the target OSABI should be used 
instead of the default triple.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: arphaman, dexonsmith.
Herald added subscribers: jkorous, inglorion, mehdi_amini.

After r327851, Driver::GetTemporaryPath will create the file rather than
just create a potientially unqine filename. If clang driver pass the
file as parameter as -object_path_lto, ld64 will pass it back to libLTO
as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
is not a directory.
Now during thinLTO, pass a temp directory path to linker instread.

rdar://problem/47194182


Repository:
  rC Clang

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp


Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryPath - Return the pathname of a temporary directory to use
+  /// as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation , StringRef BaseName) const;
 


Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath =