[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[MyDeveloperDay via Phabricator via cfe-commits]

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

I can tell from https://zed0.co.uk/clang-format-configurator/ that it's not 
possible to get it to the form you suggested, I'm not opposed to accepting 
this, I'm just not an ObjectiveC person so I haven't used clang-format on code 
like this, I just don't want to break others

requesting changes due to removed line?




Comment at: clang/lib/Format/ContinuationIndenter.cpp:869
   State.Stack[i].BreakBeforeParameter = true;
 
   if (PreviousNonComment &&

I'm a little confused by the code, this feels to me like its covering something 
more than a specific than just the ObjectiveC case, what is meant here by a 
nestedblockspecialcase? this feels like we are turning off BreakBeforeParameter 
by another method?

I get the code below, just wondering why this is also needed.



Comment at: clang/lib/Format/Format.cpp:794
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
-  LLVMStyle.ObjCBlockIndentWidth = 2;
   LLVMStyle.ObjCSpaceAfterProperty = false;

did you mean to remove this line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> `note()`

Of course i mean something like this:

  std::string text = note();
  if (!text.empty())
return text;

(and return something like `""` on the other return path in the inner lambdas).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D70111#1765223 , @dblaikie wrote:

> Looks OK to me - please recommit when you're ready.


Thank you @awpandey . As requested, I've recommited this.


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

https://reviews.llvm.org/D70111



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

[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1054
+ "pointer to __strong expected");
+  EmitStoreOfScalar(*AI++, LV);
+} else

This definitely deserves a comment.

I don't think the assertion is right; the condition is that the type is legal 
for a field in a struct that can be passed directly, and while that does 
exclude `__weak` (because the struct will have to be passed indirectly) and 
`__autoreleasing` (because that's not legal in a struct), it doesn't exclude 
`__unsafe_unretained`.

This function is implementing an operation that's broadly meaningful (it's a 
store-init of an owned value, in contrast to a store-init with an unowned value 
which is what `isInit` is implementing) but not extensively used in the C 
frontend.  On some level, I feel like we should probably teach 
`EmitStoreThroughLValue` to handle that properly, but that's a more significant 
refactor.

It does look like this change isn't enough to handle `__ptrauth`, which will 
assume that the source value is signed appropriately for the unqualified type 
when probably it should be signed appropriately for the qualifier (which, like 
`__weak`, cannot be address-discriminated because it would stop being passed 
directly).  Probably the default case should be to use `EmitStoreOfScalar`, and 
`EmitStoreThroughLValue` should only be used if the l-value is a bit-field (the 
only non-simple case that can actually happen from drilling down to a field).

The same logic applies on the load side in the abstract, except that it is only 
causing problems for `__ptrauth` (well, if we change the behavior above) 
because loads from the ARC qualifiers don't implicitly retain.  Regardless, 
analogously to this, `EmitRValueForField` should only be calling 
`EmitLoadOfLValue` for bit-fields and should otherwise call `EmitLoadOfScalar`. 
 Please add a comment on both sides making it clear that the logic is expected 
to be parallel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70935



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

[clang] f1e3988 - Recommit "[DWARF5]Addition of alignment atrribute in typedef DIE."

[Sourabh Singh Tomar via cfe-commits]

Author: Sourabh Singh Tomar
Date: 2019-12-03T09:51:43+05:30
New Revision: f1e3988aa6016188c376b9bcca1afc7559f9fbc0

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

LOG: Recommit "[DWARF5]Addition of alignment atrribute in typedef DIE."

This revision is revised to update Go-bindings and Release Notes.

The original commit message follows.

This patch, adds support for DW_AT_alignment[DWARF5] attribute, to be emitted 
with typdef DIE.
When explicit alignment is specified.

Patch by Awanish Pandey 

Reviewers: aprantl, dblaikie, jini.susan.george, SouraVX, alok,
deadalinx

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

Added: 
clang/test/CodeGenCXX/debug-info-template-align.cpp
llvm/test/DebugInfo/X86/debug-info-template-align.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
llvm/bindings/go/llvm/dibuilder.go
llvm/docs/ReleaseNotes.rst
llvm/include/llvm-c/DebugInfo.h
llvm/include/llvm/IR/DIBuilder.h
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/lib/IR/DebugInfo.cpp
llvm/tools/llvm-c-test/debuginfo.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index db5893a7b51f..8d6406c02773 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1141,10 +1141,11 @@ llvm::DIType *CGDebugInfo::CreateType(const TypedefType 
*Ty,
   // declared.
   SourceLocation Loc = Ty->getDecl()->getLocation();
 
+  uint32_t Align = getDeclAlignIfRequired(Ty->getDecl(), CGM.getContext());
   // Typedefs are derived from some other type.
   return DBuilder.createTypedef(Underlying, Ty->getDecl()->getName(),
 getOrCreateFile(Loc), getLineNumber(Loc),
-getDeclContextDescriptor(Ty->getDecl()));
+getDeclContextDescriptor(Ty->getDecl()), 
Align);
 }
 
 static unsigned getDwarfCC(CallingConv CC) {

diff  --git a/clang/test/CodeGenCXX/debug-info-template-align.cpp 
b/clang/test/CodeGenCXX/debug-info-template-align.cpp
new file mode 100644
index ..42fdb269a30b
--- /dev/null
+++ b/clang/test/CodeGenCXX/debug-info-template-align.cpp
@@ -0,0 +1,14 @@
+//  Test for debug info related to DW_AT_alignment attribute in the typedef 
operator
+// Supported: -O0, standalone DI
+// RUN: %clang_cc1 -dwarf-version=5  -emit-llvm -triple x86_64-linux-gnu %s -o 
- \
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone \
+// RUN: | FileCheck %s
+
+// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align: 512
+
+typedef char __attribute__((__aligned__(64))) alchar;
+
+int main() {
+  alchar newChar;
+}

diff  --git a/llvm/bindings/go/llvm/dibuilder.go 
b/llvm/bindings/go/llvm/dibuilder.go
index e84536927160..e30a964beaf6 100644
--- a/llvm/bindings/go/llvm/dibuilder.go
+++ b/llvm/bindings/go/llvm/dibuilder.go
@@ -504,6 +504,7 @@ type DITypedef struct {
FileMetadata
Lineint
Context Metadata
+  AlignInBits unit32
 }
 
 // CreateTypedef creates typedef type debug metadata.
@@ -518,6 +519,7 @@ func (d *DIBuilder) CreateTypedef(t DITypedef) Metadata {
t.File.C,
C.unsigned(t.Line),
t.Context.C,
+C.uint32_t(t.AlignInBits),
)
return Metadata{C: result}
 }

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index e85a85053fb9..c27f3bc8b692 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -166,6 +166,16 @@ Changes to the OCaml bindings
 
 Changes to the C API
 
+* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra
+argument ``AlignInBits``, to facilitate / propagate specified Alignment 
information
+present in a ``typedef`` to Debug information in LLVM IR.
+
+
+Changes to the Go bindings
+--
+* Go DebugInfo API ``CreateTypedef`` is updated to include an extra argument 
``AlignInBits``,
+to facilitate / propagate specified Alignment information present in a 
``typedef``
+to Debug information in LLVM IR.
 
 
 Changes to the DAG infrastructure

diff  --git a/llvm/include/llvm-c/DebugInfo.h b/llvm/include/llvm-c/DebugInfo.h
index ab60b88a31f4..731f32741e19 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -874,7 +874,7 @@ LLVMMetadataRef
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope);
+   LLVMMetadataRef Scope, uint32_t AlignInBits);
 
 /**
  * Create debugging information entry to 

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D70725#1765981 , @xazax.hun wrote:

> the same call might both acquire and release handles (there are such calls in 
> Fuchsia), so we might end up adding more than one note for a call for which 
> we would need to add more than one transitions


Hmm, would this be too functional?:

  std::vector notes;
  for (arg : args) {
if (isAcquired(param)) {
  State = State->set(arg, Acquired);
  notes.push_back([](Report) {
if (Report.isInteresting(arg))
  return "Handle acquired here";
  });
}
if (isReleased(param)) {
  State = State->set(arg, Released);
  notes.push_back([](Report) {
if (Report.isInteresting(arg))
  return "Handle released here";
  });
}
  }
  
  C.addTransition(State, C.getNoteTag(
  // We might as well add a C.getNoteTag() overload
  // to do this automatically.
  [std::move(notes)](Report) {
for (note : notes)
  note();
  }));

Or, well, yeah, chain the nodes together; you anyway have to do this 
occasionally due to clumsiness of the rest of the API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I can't speak for the pragma authors, but I strongly doubt the pragma is 
intended to force all affected globals to go into a single section unit, since 
the division into section units is purely an object-file representation issue.

Looks to me like `MCContext::getELFSection` should be including the entry size 
and flags as part of the uniquing key.


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

https://reviews.llvm.org/D68101



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

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

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



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

mibintc wrote:
> michele.scandale wrote:
> > Here the state of the floating point options seems unchanged except for 
> > `FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would 
> > expect the state of the floating point options to match the one of 
> > `-fno-fast-math` except for `FPContract` which you want to be set to "fast".
> > 
> > I think you might need to replicate the reset for all the option here as 
> > well, so at this point I don't know how much worth is to use the optID 
> > reset trick for the "fast" case only.
> @michele.scandale Thanks for your helpful review, I think I fixed the things 
> that you remarked on. I also added a test case for the assertion fail that 
> you saw.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D70936: [clang-scan-deps] do not skip empty #if/#elif in the minimize to avoid missing `__has_include` dependencies

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
arphaman marked an inline comment as done.
Closed by commit rG389530524be1: [clang-scan-deps] do not skip empty #if/#elif 
in the minimizer to avoid missing… (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D70936?vs=231804=231810#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70936

Files:
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
  clang/test/ClangScanDeps/has_include_if_elif.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -328,12 +328,17 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#ifdef A\n"
+"void skip();\n"
 "#elif B\n"
 "#elif C\n"
 "#else D\n"
 "#endif\n",
 Out));
-  EXPECT_STREQ("", Out.data());
+  EXPECT_STREQ("#ifdef A\n"
+   "#elif B\n"
+   "#elif C\n"
+   "#endif\n",
+   Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Pragma) {
@@ -507,6 +512,12 @@
   for (auto Source : {
"#warning \\\n#include \n",
"#error \\\n#include \n",
+   }) {
+ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+EXPECT_STREQ("", Out.data());
+  }
+
+  for (auto Source : {
"#if MACRO\n#warning '\n#endif\n",
"#if MACRO\n#warning \"\n#endif\n",
"#if MACRO\n#warning /*\n#endif\n",
@@ -515,7 +526,7 @@
"#if MACRO\n#error /*\n#endif\n",
}) {
 ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
-EXPECT_STREQ("", Out.data());
+EXPECT_STREQ("#if MACRO\n#endif\n", Out.data());
   }
 }
 
@@ -543,7 +554,7 @@
 #include 
 )";
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
-  EXPECT_STREQ("#include \n", Out.data());
+  EXPECT_STREQ("#if DEBUG\n#endif\n#include \n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixU) {
Index: clang/test/ClangScanDeps/has_include_if_elif.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/has_include_if_elif.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/has_include_if_elif2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header3.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header4.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/has_include_if_elif.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
+// RUN:   FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
+// RUN:   FileCheck %s
+
+#if __has_include("header.h")
+#endif
+
+#if 0
+#elif __has_include("header2.h")
+#endif
+
+#define H3 __has_include("header3.h")
+#if H3
+#endif
+
+#define H4 __has_include("header4.h")
+
+#if 0
+#elif H4
+#endif
+
+// CHECK: has_include_if_elif2.cpp
+// CHECK-NEXT: Inputs{{/|\\}}header.h
+// CHECK-NEXT: Inputs{{/|\\}}header2.h
+// CHECK-NEXT: Inputs{{/|\\}}header3.h
+// CHECK-NEXT: Inputs{{/|\\}}header4.h
Index: clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/has_include_if_elif2.cpp -IInputs",
+  "file": "DIR/has_include_if_elif2.cpp"
+}
+]
Index: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -763,12 +763,13 @@
   if (top() == pp_else)
 popToken();
 
-  // Strip out "#elif" if they're empty.
-  while (top() == pp_elif)
-popToken();
-
-  // If "#if" is empty, strip it and skip the "#endif".
-  if (top() == pp_if || top() == pp_ifdef || top() == pp_ifndef) {
+  // If "#ifdef" is empty, strip it and skip the "#endif".
+  //
+  // FIXME: Once/if Clang starts disallowing __has_include in 

[clang] 3895305 - [clang-scan-deps] do not skip empty #if/#elif in the minimizer to avoid missing `__has_include` dependencies

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2019-12-02T18:47:22-08:00
New Revision: 389530524be1715e97947810514f3a75dfe73975

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

LOG: [clang-scan-deps] do not skip empty #if/#elif in the minimizer to avoid 
missing `__has_include` dependencies

This patch makes the minimizer more conservative to avoid missing dependency 
files that are brought in by __has_include
PP expressions that occur in a condition of an #if/#elif that was previously 
skipped. The __has_include PP expressions
can be used in an #if/#elif either directly, or through macro expansion, so we 
can't detect them at the time of minimization.

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

Added: 
clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
clang/test/ClangScanDeps/has_include_if_elif.cpp

Modified: 
clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Removed: 




diff  --git a/clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp 
b/clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
index f063ed711c44..029bfe1cd600 100644
--- a/clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ b/clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -763,12 +763,13 @@ bool Minimizer::lexEndif(const char *, const char 
*const End) {
   if (top() == pp_else)
 popToken();
 
-  // Strip out "#elif" if they're empty.
-  while (top() == pp_elif)
-popToken();
-
-  // If "#if" is empty, strip it and skip the "#endif".
-  if (top() == pp_if || top() == pp_ifdef || top() == pp_ifndef) {
+  // If "#ifdef" is empty, strip it and skip the "#endif".
+  //
+  // FIXME: Once/if Clang starts disallowing __has_include in macro expansions,
+  // we can skip empty `#if` and `#elif` blocks as well after scanning for a
+  // literal __has_include in the condition.  Even without that rule we could
+  // drop the tokens if we scan for identifiers in the condition and find none.
+  if (top() == pp_ifdef || top() == pp_ifndef) {
 popToken();
 skipLine(First, End);
 return false;

diff  --git a/clang/test/ClangScanDeps/Inputs/has_include_if_elif.json 
b/clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
new file mode 100644
index ..36ca006b0329
--- /dev/null
+++ b/clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/has_include_if_elif2.cpp -IInputs",
+  "file": "DIR/has_include_if_elif2.cpp"
+}
+]

diff  --git a/clang/test/ClangScanDeps/has_include_if_elif.cpp 
b/clang/test/ClangScanDeps/has_include_if_elif.cpp
new file mode 100644
index ..dd56ecac69db
--- /dev/null
+++ b/clang/test/ClangScanDeps/has_include_if_elif.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/has_include_if_elif2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header3.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header4.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/has_include_if_elif.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode 
preprocess-minimized-sources | \
+// RUN:   FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
+// RUN:   FileCheck %s
+
+#if __has_include("header.h")
+#endif
+
+#if 0
+#elif __has_include("header2.h")
+#endif
+
+#define H3 __has_include("header3.h")
+#if H3
+#endif
+
+#define H4 __has_include("header4.h")
+
+#if 0
+#elif H4
+#endif
+
+// CHECK: has_include_if_elif2.cpp
+// CHECK-NEXT: Inputs{{/|\\}}header.h
+// CHECK-NEXT: Inputs{{/|\\}}header2.h
+// CHECK-NEXT: Inputs{{/|\\}}header3.h
+// CHECK-NEXT: Inputs{{/|\\}}header4.h

diff  --git a/clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp 
b/clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
index ed44cd86b3e4..b5bba30db78d 100644
--- a/clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -328,12 +328,17 @@ TEST(MinimizeSourceToDependencyDirectivesTest, 
EmptyIfdef) {
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#ifdef A\n"
+"void skip();\n"
 "#elif B\n"
 "#elif C\n"
 "#else D\n"
 "#endif\n",
 Out));
-  EXPECT_STREQ("", 

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[ben via Phabricator via cfe-commits]

bd1976llvm added a comment.

In D68101#1766151 , @nickdesaulniers 
wrote:

> In D68101#1765665 , @rjmccall wrote:
>
> > Given all that, this patch seems far too aggressive.  While mergeable 
> > sections can be useful for optimizing arbitrary code that might not use a 
> > section, they are also extremely useful for optimizing the sorts of global 
> > tables that programmers frequently use explicit sections for.  It seems to 
> > me that the right fix is to find the place that ensures that we don't put 
> > mergeable and non-mergeable objects in the same section unit (or at least 
> > conservatively makes the section unit non-mergeable) and fix it to consider 
> > entry size as well.  That should be straightforward unless that place 
> > doesn't exist, in which case we have very serious problems.
>
>
> Disagree (but I spent all day thinking about this).  Going back to 
> https://bugs.llvm.org/show_bug.cgi?id=31828, I think I solved the problem 
> there incorrectly; we should be not marking the explicit section merge-able.


Thanks for the comments!

I am convinced that the current patch is too pessimistic, size optimizations 
can be functional requirements in memory constrained areas.

There are two frontend attributes involved:

pragma clang section ... - 
https://clang.llvm.org/docs/LanguageExtensions.html#specifying-section-names-for-global-objects-pragma-clang-section
_attribute_ ((section ("section-name"))) - 
https://clang.llvm.org/docs/AttributeReference.html#section-declspec-allocate

At the IR level you can query for explicit sections assignment with code like:

  static bool inExplicitSection(const GlobalObject *GO, SectionKind Kind) {
    if (GO->hasSection())
      return true;
  
    if (auto *GVar = dyn_cast(GO)) {
      auto Attrs = GVar->getAttributes();
      if ((Attrs.hasAttribute("bss-section") && Kind.isBSS()) ||
          (Attrs.hasAttribute("data-section") && Kind.isData()) ||
          (Attrs.hasAttribute("rodata-section") && Kind.isReadOnly())) {
        return true;
      }
    }
  
    if (auto *F = dyn_cast(GO)) {
      if (F->hasFnAttribute("implicit-section-name"))
        return true;
    }
  
    return false;
  }

The section of a global is used for _attribute_ ((section ("section-name"))) 
but it is also set in LLVM e.g. by optimization passes.
Attributes "bss-section", "data-section", "rodata-section" and 
"implicit-section-name" are only used for "clang section" pragmas.

As I commented upthread I would like input from the authors of the "clang 
section" pragma to understand whether the semantics of that pragma would allow 
for generating multiple output sections (with the same name). Assuming that 
this pragma implies a single output section I have an new proposal for a less 
pessimistic fix:

1. Sections generated for "clang section" pragmas are never made mergeable.
2. It is an error if incompatible globals are assigned to a section with the 
same name.

This works as the user can easily fix any "_attribute_ ((section 
("section-name")))" in their source code and internal uses in LLVM code can 
also make sure that incompatible globals are assigned to differently named 
sections. Thoughts? I'll update the patch shortly unless there are objections.


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

https://reviews.llvm.org/D68101



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

[PATCH] D70936: [clang-scan-deps] do not skip empty #if/#elif in the minimize to avoid missing `__has_include` dependencies

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

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

LGTM, although I have a suggested change for one of the comments (and I'm still 
sad about this).




Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:766-769
+  // If "#ifdef" is empty, strip it and skip the "#endif".
+  // Avoid stripping "#if" / "#elif" as they could contain a "__has_include"
+  // (either directly, or through a macro expansion) without an "#include"
+  // inside the "#if/elif", which could affect the dependency output.

I'd prefer to structure this comment differently, something like:
```
// If "#ifdef" is empty, strip it and skip the "#endif".
//
// TODO: Once/if Clang starts disallowing __has_include in macro expansions,
// we can skip empty `#if` and `#elif` blocks as well after scanning for a
// literal __has_include in the condition.  Even without that rule we could
// drop the tokens if we scan for identifiers in the condition and find none.
```
This rephrases it as a potential future optimization instead of one that has to 
be skipped, which I think makes it easier to reason about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70936



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

[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Note that passing `isInit=true` to `EmitStoreThroughLValue` to make it emit 
`llvm.objc.retain` would be incorrect since the callee destructs the struct 
argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70935



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

[PATCH] D70936: [clang-scan-deps] do not skip empty #if/#elif in the minimize to avoid missing `__has_include` dependencies

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: Bigcheese, dexonsmith, kousikk.
Herald added subscribers: ributzka, tschuett, jkorous.
Herald added a project: clang.

This patch makes the minimizer more conservative to avoid missing dependency 
files that are brought in by `__has_include` PP expressions that occur in a 
condition of an `#if`/`#elif` that was previously skipped. The `__has_include` 
PP expressions can be used in an `#if`/`#elif` either directly, or through 
macro expansion, so we can't detect them at the time of minimization.

It'll be possible to skip certain `#if`/`#elif` in the future by doing more 
minimizer optimizations (e.g. skip `#if 0`). Furthermore, @Bigcheese is working 
on clarifying the allowed usage of  `__has_include` in the standard, as it 
seems that this kind of usage should actually be disallowed. This will 
hopefully allow us skip these `#if`/`#elif` again in the future.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70936

Files:
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
  clang/test/ClangScanDeps/has_include_if_elif.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -328,12 +328,17 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#ifdef A\n"
+"void skip();\n"
 "#elif B\n"
 "#elif C\n"
 "#else D\n"
 "#endif\n",
 Out));
-  EXPECT_STREQ("", Out.data());
+  EXPECT_STREQ("#ifdef A\n"
+   "#elif B\n"
+   "#elif C\n"
+   "#endif\n",
+   Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Pragma) {
@@ -507,6 +512,12 @@
   for (auto Source : {
"#warning \\\n#include \n",
"#error \\\n#include \n",
+   }) {
+ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+EXPECT_STREQ("", Out.data());
+  }
+
+  for (auto Source : {
"#if MACRO\n#warning '\n#endif\n",
"#if MACRO\n#warning \"\n#endif\n",
"#if MACRO\n#warning /*\n#endif\n",
@@ -515,7 +526,7 @@
"#if MACRO\n#error /*\n#endif\n",
}) {
 ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
-EXPECT_STREQ("", Out.data());
+EXPECT_STREQ("#if MACRO\n#endif\n", Out.data());
   }
 }
 
@@ -543,7 +554,7 @@
 #include 
 )";
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
-  EXPECT_STREQ("#include \n", Out.data());
+  EXPECT_STREQ("#if DEBUG\n#endif\n#include \n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, CharacterLiteralPrefixU) {
Index: clang/test/ClangScanDeps/has_include_if_elif.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/has_include_if_elif.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/has_include_if_elif2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header3.h
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header4.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/has_include_if_elif.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
+// RUN:   FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
+// RUN:   FileCheck %s
+
+#if __has_include("header.h")
+#endif
+
+#if 0
+#elif __has_include("header2.h")
+#endif
+
+#define H3 __has_include("header3.h")
+#if H3
+#endif
+
+#define H4 __has_include("header4.h")
+
+#if 0
+#elif H4
+#endif
+
+// CHECK: has_include_if_elif2.cpp
+// CHECK-NEXT: Inputs{{/|\\}}header.h
+// CHECK-NEXT: Inputs{{/|\\}}header2.h
+// CHECK-NEXT: Inputs{{/|\\}}header3.h
+// CHECK-NEXT: Inputs{{/|\\}}header4.h
Index: clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/has_include_if_elif.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/has_include_if_elif2.cpp -IInputs",
+  "file": "DIR/has_include_if_elif2.cpp"
+}
+]
Index: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: ributzka, dexonsmith, jkorous.

This fixes a bug in IRGen where a call to `llvm.objc.storeStrong` was being 
emitted to initialize a __strong field of an uninitialized temporary struct, 
which caused crashes at runtime.
rdar://problem/51807365


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70935

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenObjC/nontrivial-struct-param-init.m


Index: clang/test/CodeGenObjC/nontrivial-struct-param-init.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/nontrivial-struct-param-init.m
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-apple-watchos6.0-simulator -emit-llvm -fblocks 
-fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+typedef struct {
+  id x;
+} S;
+
+// CHECK: define void @test0(i8* %[[A_0:.*]])
+// CHECK: %[[A:.*]] = alloca %[[STRUCT_S]], align 4
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_S]], %[[STRUCT_S]]* 
%[[A]], i32 0, i32 0
+// CHECK: store i8* %[[A_0]], i8** %[[X]], align 4
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_S]]* %[[A]] to i8**
+// CHECK: call void @__destructor_4_s0(i8** %[[V0]]) #2
+
+void test0(S a) {
+}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1048,7 +1048,12 @@
 EmitStoreOfComplex(ComplexPairTy(realValue, imagValue), LV, /*init*/ true);
   } else {
 assert(isa(Exp.get()));
-EmitStoreThroughLValue(RValue::get(*AI++), LV);
+if (LV.getQuals().getObjCLifetime()) {
+  assert(LV.getQuals().getObjCLifetime() == Qualifiers::OCL_Strong &&
+ "pointer to __strong expected");
+  EmitStoreOfScalar(*AI++, LV);
+} else
+  EmitStoreThroughLValue(RValue::get(*AI++), LV);
   }
 }
 


Index: clang/test/CodeGenObjC/nontrivial-struct-param-init.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/nontrivial-struct-param-init.m
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-apple-watchos6.0-simulator -emit-llvm -fblocks -fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+typedef struct {
+  id x;
+} S;
+
+// CHECK: define void @test0(i8* %[[A_0:.*]])
+// CHECK: %[[A:.*]] = alloca %[[STRUCT_S]], align 4
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_S]], %[[STRUCT_S]]* %[[A]], i32 0, i32 0
+// CHECK: store i8* %[[A_0]], i8** %[[X]], align 4
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_S]]* %[[A]] to i8**
+// CHECK: call void @__destructor_4_s0(i8** %[[V0]]) #2
+
+void test0(S a) {
+}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1048,7 +1048,12 @@
 EmitStoreOfComplex(ComplexPairTy(realValue, imagValue), LV, /*init*/ true);
   } else {
 assert(isa(Exp.get()));
-EmitStoreThroughLValue(RValue::get(*AI++), LV);
+if (LV.getQuals().getObjCLifetime()) {
+  assert(LV.getQuals().getObjCLifetime() == Qualifiers::OCL_Strong &&
+ "pointer to __strong expected");
+  EmitStoreOfScalar(*AI++, LV);
+} else
+  EmitStoreThroughLValue(RValue::get(*AI++), LV);
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

> So I don't think we should even be trying to mark sections as mergeable 
> unless we walk all elements in the section and make sure it's even safe to 
> apply these.

That's the conservative fix, yes.  You can either:

1. emit global objects with the same section name into a single section unit 
and then set the mergeable flag and entry size for that unit only if all the 
objects were mergeable and the same size, or
2. emit global objects with the same section name into different section units 
based on their mergeability and entry size.

If the code isn't already set up to emit multiple section units with a name — 
which implies that there may also be an existing bug based on having mergeable 
and non-mergeable symbols with the same section name — then the former is 
probably much simpler.  But if it's already set up to split into two section 
units, I can't imagine it'd be that difficult to split into more than two.


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

https://reviews.llvm.org/D68101



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

[PATCH] D69543: [clangd] Add a tweak refactoring to wrap Objective-C string literals in `NSLocalizedString` macros

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang-tools-extra/clangd/ParsedAST.h:80
 
+  const LangOptions () const {
+return getASTContext().getLangOpts();

kadircet wrote:
> can we introduce this helper in a new patch, while changing other occurrences 
> in clangd?
Sounds good, I'll do that.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp:34
+/// After:
+///   NSLocalizedString(@"description", "")
+class ObjCLocalizeStringLiteral : public Tweak {

kadircet wrote:
>  NSLocalizedString(@"description", `@`"")
Great catch, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69543



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

[PATCH] D69543: [clangd] Add a tweak refactoring to wrap Objective-C string literals in `NSLocalizedString` macros

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 231799.
arphaman marked 6 inline comments as done.
arphaman added a comment.

Fixed nits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69543

Files:
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -122,6 +122,25 @@
   EXPECT_EQ(apply(Input), Output);
 }
 
+TWEAK_TEST(ObjCLocalizeStringLiteral);
+TEST_F(ObjCLocalizeStringLiteralTest, Test) {
+  ExtraArgs.push_back("-x");
+  ExtraArgs.push_back("objective-c");
+
+  // Ensure the the action can be initiated in the string literal.
+  EXPECT_AVAILABLE(R"(id x = ^[[@[[^"^t^est^";)");
+
+  // Ensure that the action can't be initiated in other places.
+  EXPECT_UNAVAILABLE(R"([[i^d ^[[x]] ^= @"test";^]])");
+
+  // Ensure that the action is not available for regular C strings.
+  EXPECT_UNAVAILABLE(R"(const char * x= "^test";)");
+
+  const char *Input = R"(id x = [[@"test"]];)";
+  const char *Output = R"(id x = NSLocalizedString(@"test", @"");)";
+  EXPECT_EQ(apply(Input), Output);
+}
+
 TWEAK_TEST(DumpAST);
 TEST_F(DumpASTTest, Test) {
   EXPECT_AVAILABLE("^int f^oo() { re^turn 2 ^+ 2; }");
Index: clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp
@@ -0,0 +1,85 @@
+//===--- ObjcLocalizeStringLiteral.cpp ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Logger.h"
+#include "ParsedAST.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+/// Wraps an Objective-C string literal with the NSLocalizedString macro.
+/// Before:
+///   @"description"
+///   ^^^
+/// After:
+///   NSLocalizedString(@"description", @"")
+class ObjCLocalizeStringLiteral : public Tweak {
+public:
+  const char *id() const override final;
+  Intent intent() const override { return Intent::Refactor; }
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) override;
+  std::string title() const override;
+
+private:
+  const clang::ObjCStringLiteral *Str = nullptr;
+};
+
+REGISTER_TWEAK(ObjCLocalizeStringLiteral)
+
+bool ObjCLocalizeStringLiteral::prepare(const Selection ) {
+  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
+  if (!N)
+return false;
+  // Allow the refactoring even if the user selected only the C string part
+  // of the expression.
+  if (N->ASTNode.get()) {
+if (N->Parent)
+  N = N->Parent;
+  }
+  Str = dyn_cast_or_null(N->ASTNode.get());
+  return Str;
+}
+
+Expected
+ObjCLocalizeStringLiteral::apply(const Selection ) {
+  auto  = Inputs.AST.getSourceManager();
+  auto  = Inputs.AST.getASTContext().getLangOpts();
+  auto Reps = tooling::Replacements(tooling::Replacement(
+  SM, CharSourceRange::getCharRange(Str->getBeginLoc()),
+  "NSLocalizedString(", LangOpts));
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+  Str->getEndLoc(), 0, Inputs.AST.getSourceManager(), LangOpts);
+  if (auto Err = Reps.add(tooling::Replacement(
+  SM, CharSourceRange::getCharRange(EndLoc), ", @\"\")", LangOpts)))
+return std::move(Err);
+  return Effect::mainFileEdit(SM, std::move(Reps));
+}
+
+std::string ObjCLocalizeStringLiteral::title() const {
+  return "Wrap in NSLocalizedString";
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
===
--- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -19,6 +19,7 @@
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
+  ObjCLocalizeStringLiteral.cpp
   RawStringLiteral.cpp
   

[PATCH] D70931: [MS] Emit exported complete/vbase destructors

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added a reviewer: hans.
Herald added a project: clang.

Fixes PR44205

I checked, and deleting destructors are not affected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70931

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp


Index: clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
@@ -19,9 +19,9 @@
   virtual ~ImportOverrideVDtor() {}
 };
 
-// Virtually inherits from a non-dllimport base class. This time we need to 
call
-// the complete destructor and emit it inline. It's not exported from the DLL,
-// and it must be emitted.
+// Virtually inherits from a non-dllimport base class. In this case, we can
+// expect the DLL to provide a definition of the complete dtor. See
+// dllexport-dtor-thunks.cpp.
 struct __declspec(dllimport) ImportVBaseOverrideVDtor
 : public virtual BaseClass {
   virtual ~ImportVBaseOverrideVDtor() {}
Index: clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 -mconstructor-aliases -fms-extensions %s -emit-llvm -o - 
-triple x86_64-windows-msvc | FileCheck %s
 
+namespace test1 {
+struct A { ~A(); };
+struct __declspec(dllexport) B : virtual A { };
+// CHECK: define weak_odr dso_local dllexport void @"??1B@test1@@QEAA@XZ"
+// CHECK: define weak_odr dso_local dllexport void @"??_DB@test1@@QEAAXXZ"
+}
+
 struct __declspec(dllexport) A { virtual ~A(); };
 struct __declspec(dllexport) B { virtual ~B(); };
 struct __declspec(dllexport) C : A, B { virtual ~C(); };
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1343,6 +1343,13 @@
   // The TU defining a dtor is only guaranteed to emit a base destructor.  All
   // other destructor variants are delegating thunks.
   CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+
+  // If the class is dllexported, emit the complete (vbase) destructor wherever
+  // the base dtor is emitted.
+  // FIXME: To match MSVC, this should only be done when the class was
+  // dllexported inlines are being exported.
+  if (D->getParent()->getNumVBases() > 0 && D->hasAttr())
+CGM.EmitGlobal(GlobalDecl(D, Dtor_Complete));
 }
 
 CharUnits


Index: clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
@@ -19,9 +19,9 @@
   virtual ~ImportOverrideVDtor() {}
 };
 
-// Virtually inherits from a non-dllimport base class. This time we need to call
-// the complete destructor and emit it inline. It's not exported from the DLL,
-// and it must be emitted.
+// Virtually inherits from a non-dllimport base class. In this case, we can
+// expect the DLL to provide a definition of the complete dtor. See
+// dllexport-dtor-thunks.cpp.
 struct __declspec(dllimport) ImportVBaseOverrideVDtor
 : public virtual BaseClass {
   virtual ~ImportVBaseOverrideVDtor() {}
Index: clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 -mconstructor-aliases -fms-extensions %s -emit-llvm -o - -triple x86_64-windows-msvc | FileCheck %s
 
+namespace test1 {
+struct A { ~A(); };
+struct __declspec(dllexport) B : virtual A { };
+// CHECK: define weak_odr dso_local dllexport void @"??1B@test1@@QEAA@XZ"
+// CHECK: define weak_odr dso_local dllexport void @"??_DB@test1@@QEAAXXZ"
+}
+
 struct __declspec(dllexport) A { virtual ~A(); };
 struct __declspec(dllexport) B { virtual ~B(); };
 struct __declspec(dllexport) C : A, B { virtual ~C(); };
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1343,6 +1343,13 @@
   // The TU defining a dtor is only guaranteed to emit a base destructor.  All
   // other destructor variants are delegating thunks.
   CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+
+  // If the class is dllexported, emit the complete (vbase) destructor wherever
+  // the base dtor is emitted.
+  // FIXME: To match MSVC, this should only be done when the class was
+  // dllexported inlines are being exported.
+  if (D->getParent()->getNumVBases() > 0 && D->hasAttr())
+CGM.EmitGlobal(GlobalDecl(D, Dtor_Complete));

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

Thank you for rebasing on current master.

I have ran it today on our code base and found three classes of false positives:

1. Writing to a bitfield of a struct. The struct still is suggested it should 
be const.
2. Using a variable with an ostream extraction; like "int foo; cin >> foo;", 
except it was a template on the stream instance.
3. In a for loop, with a somewhat strange pair (for auto [foo, bar] = std::pair 
{}; foo < big_foo; ++ foo).

Let me know if you can't create test cases from these descriptions and I can 
try to extract the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D45444



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

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

[Zachary Turner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG64f74bf72eb4: [clang-tidy] Rewrite modernize-avoid-bind 
check. (authored by zturner).

Changed prior to commit:
  https://reviews.llvm.org/D70368?vs=230661=231793#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70368

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-bind.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind-permissive-parameter-list.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D68101#1765665 , @rjmccall wrote:

> Given all that, this patch seems far too aggressive.  While mergeable 
> sections can be useful for optimizing arbitrary code that might not use a 
> section, they are also extremely useful for optimizing the sorts of global 
> tables that programmers frequently use explicit sections for.  It seems to me 
> that the right fix is to find the place that ensures that we don't put 
> mergeable and non-mergeable objects in the same section unit (or at least 
> conservatively makes the section unit non-mergeable) and fix it to consider 
> entry size as well.  That should be straightforward unless that place doesn't 
> exist, in which case we have very serious problems.


Disagree (but I spent all day thinking about this).  Going back to 
https://bugs.llvm.org/show_bug.cgi?id=31828, I think I solved the problem there 
incorrectly; we should be not marking the explicit section merge-able.

Consider: https://godbolt.org/z/6sF_kc (GCC will put `a` and `c` in a 
non-mergeable section).

https://www.sco.com/developers/gabi/2003-12-17/ch4.sheader.html mentions:

  Each element in the section is compared against other elements in sections 
with the same name, type and flags. Elements that would have identical values 
at program run-time may be merged. Relocations referencing elements of such 
sections must be resolved to the merged locations of the referenced values. 
Note that any relocatable values, including values that would result in 
run-time relocations, must be analyzed to determine whether the run-time values 
would actually be identical.

So I don't think we should even be trying to mark sections as mergeable unless 
we walk all elements in the section and make sure it's even safe to apply these.

In D68101#1684828 , @jmolloy wrote:

> This feels like it could cause a pretty serious regression. This essentially 
> disables global merging with -fdata-sections, which I know at least one 
> linker relies upon for code size.


Does it? Surely there's a test that would fail with this patch applied?




Comment at: llvm/test/CodeGen/X86/explict-section-mergeable.ll:21
+; CHECK: explicit2:
+; CHECK-NOT: .section
+; CHECK: explicit3:

These `CHECK-NOT`s should be more explicit that the the section is not 
merge-able (`M`).



Comment at: llvm/test/CodeGen/X86/explict-section-mergeable.ll:22
+; CHECK-NOT: .section
+; CHECK: explicit3:

Any new tests should be added to 
`llvm/test/CodeGen/Generic/section_mergeable_size.ll` IMO. I assume this test 
changes that test's behavior?


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

https://reviews.llvm.org/D68101



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

[clang-tools-extra] 64f74bf - [clang-tidy] Rewrite modernize-avoid-bind check.

[Zachary Turner via cfe-commits]

Author: Zachary Turner
Date: 2019-12-02T15:36:26-08:00
New Revision: 64f74bf72eb484aa32e1104050cb54745116decf

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

LOG: [clang-tidy] Rewrite modernize-avoid-bind check.

This represents largely a full re-write of modernize-avoid-bind, adding
significant new functionality in the process. In particular:

* Both boost::bind and std::bind are now supported
* Function objects are supported in addition to functions
* Member functions are supported
* Nested calls are supported using capture-init syntax
* std::ref() and boost::ref() are now recognized, and will capture by reference.
* Rather than capturing with a global =, we now build up an individual
  capture list that is both necessary and sufficient for the call.
* Fixits are supported in a much larger variety of scenarios than before.

All previous tests pass under the re-write, but a large number of new
tests have been added as well.

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind-permissive-parameter-list.cpp

Modified: 
clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-bind.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
index 2d4475c991ca..d1994073bd07 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -14,11 +14,12 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -34,45 +35,270 @@ namespace modernize {
 namespace {
 
 enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other };
+enum CaptureMode { CM_None, CM_ByRef, CM_ByValue, CM_InitExpression };
+
+enum CallableType {
+  CT_Other,  // unknown
+  CT_Function,   // global or static function
+  CT_MemberFunction, // member function with implicit this
+  CT_Object, // object with operator()
+};
+
+enum CallableMaterializationKind {
+  CMK_Other,   // unknown
+  CMK_Function,// callable is the name of a member or non-member function.
+  CMK_VariableRef, // callable is a simple expression involving a global or
+   // local variable.
+  CMK_CallExpression, // callable is obtained as the result of a call 
expression
+};
 
 struct BindArgument {
-  StringRef Tokens;
+  // A rough classification of the type of expression this argument was.
   BindArgumentKind Kind = BK_Other;
+
+  // If this argument required a capture, a value indicating how it was
+  // captured.
+  CaptureMode CaptureMode = CM_None;
+
+  // The exact spelling of this argument in the source code.
+  StringRef SourceTokens;
+
+  // The identifier of the variable within the capture list.  This may be
+  // 
diff erent from UsageIdentifier for example in the expression *d, where the
+  // variable is captured as d, but referred to as *d.
+  std::string CaptureIdentifier;
+
+  // If this is a placeholder or capture init expression, contains the tokens
+  // used to refer to this parameter from within the body of the lambda.
+  std::string UsageIdentifier;
+
+  // If Kind == BK_Placeholder, the index of the placeholder.
   size_t PlaceHolderIndex = 0;
+
+  // True if the argument is used inside the lambda, false otherwise.
+  bool IsUsed = false;
+
+  // The actual Expr object representing this expression.
+  const Expr *E = nullptr;
+};
+
+struct CallableInfo {
+  CallableType Type = CT_Other;
+  CallableMaterializationKind Materialization = CMK_Other;
+  CaptureMode CaptureMode = CM_None;
+  StringRef SourceTokens;
+  std::string CaptureIdentifier;
+  std::string UsageIdentifier;
+  StringRef CaptureInitializer;
+  const FunctionDecl *Decl = nullptr;
+};
+
+struct LambdaProperties {
+  CallableInfo Callable;
+  SmallVector BindArguments;
+  StringRef BindNamespace;
+  bool IsFixitSupported = false;
 };
 
 } // end namespace
 
+static const Expr *ignoreTemporariesAndPointers(const Expr *E) {
+  if (const auto *T = dyn_cast(E))
+return ignoreTemporariesAndPointers(T->getSubExpr());
+
+  const Expr *F = 

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 231792.
Tyker added a comment.

improved based on aaron's comment.


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

https://reviews.llvm.org/D70638

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Index/pragma-diag-reparse.c
  clang/test/Misc/warning-wall.c
  clang/test/Parser/warn-misleading-indentation.cpp
  clang/test/Preprocessor/pragma_diagnostic_sections.cpp

Index: clang/test/Preprocessor/pragma_diagnostic_sections.cpp
===
--- clang/test/Preprocessor/pragma_diagnostic_sections.cpp
+++ clang/test/Preprocessor/pragma_diagnostic_sections.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -Wno-misleading-indentation -verify %s
 
 // rdar://8365684
 struct S {
Index: clang/test/Parser/warn-misleading-indentation.cpp
===
--- /dev/null
+++ clang/test/Parser/warn-misleading-indentation.cpp
@@ -0,0 +1,208 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation -DWITH_WARN %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN -DCXX17 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -Wno-misleading-indentation -DCXX17 %s
+
+#ifndef WITH_WARN
+// expected-no-diagnostics
+#endif
+
+void f0(int i) {
+  if (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1;
+int x = 0;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+  return;
+#ifdef CXX17
+  if constexpr (false)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = 0;
+i += 1;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+#endif
+}
+
+void f1(int i) {
+  for (;i;)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1;
+i *= 2;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}}
+#endif
+  return;
+}
+
+void f2(int i) {
+  while (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1; i *= 2;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'while'}}
+#endif
+  return;
+}
+
+void f3(int i) {
+  if (i)
+i = i + 1;
+  else
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+const int x = 0;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}}
+#endif
+}
+
+#ifdef CXX17
+struct Range {
+  int *begin() {return nullptr;}
+  int *end() {return nullptr;}
+};
+#endif
+
+void f4(int i) {
+  if (i)
+  i *= 2;
+  return;
+  if (i)
+i *= 2;
+;
+  if (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+typedef int Int;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+#ifdef CXX17
+  Range R;
+  for (auto e : R)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+using Int2 = int;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}}
+#endif
+#endif
+}
+
+int bar(void);
+
+int foo(int* dst)
+{   
+if (dst)
+   return
+bar();
+  if (dst)
+dst = dst + \
+bar();
+  return 0;
+}
+
+void g(int i) {
+  if (1)
+i = 2;
+  else
+ if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-3 {{here}}
+#endif
+i = 4;
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+}
+
+// Or this
+#define TEST i = 5
+void g0(int i) {
+  if (1)
+i = 2;
+  else
+i = 5;
+TEST;
+}
+
+void g1(int i) {
+  if (1)
+i = 2;
+  else if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+  i = 4;
+  i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+}
+
+void g2(int i) {
+  if (1)
+i = 2;
+  else
+if (i == 3)
+{i = 4;}
+i = 5;
+}
+
+void g6(int i) {
+if (1)
+if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = 4;
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+}
+
+void g7(int i) {
+  if (1)

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 231790.
ghvg1313 added a comment.

- Update documents


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1,6 +1,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCDontBreakBeforeNestedBlockParam = true;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCDontBreakBeforeNestedBlockParam",
+   Style.ObjCDontBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -791,7 +793,7 @@
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
-  LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCDontBreakBeforeNestedBlockParam = false;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -863,7 +863,7 @@
   bool NestedBlockSpecialCase =
   !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
   State.Stack[State.Stack.size() - 2].NestedBlockInlined;
-  if (!NestedBlockSpecialCase)
+  if (!NestedBlockSpecialCase && !Style.ObjCDontBreakBeforeNestedBlockParam)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
 
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+!Style.ObjCDontBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1646,6 +1646,10 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// \brief Dont break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  bool ObjCDontBreakBeforeNestedBlockParam;
+
   /// Add a space in front of an Objective-C protocol list, i.e. use
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
@@ -2126,6 +2130,7 @@
NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
ObjCBlockIndentWidth == 

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

You're right indeed Russell, my bad, this gain is not as important as I 
initially claimed -- however there's a gain.

Several factors came into play:

1. The latest ninja 1.9.0 doesn't use all cpu sockets on some Windows systems 
(see this ), and as it uses +2 
cores in addition to the detected # of cores, this possibly induces some 
unwanted side-effects (such as the speed-up seen in the summary).
2. I was using Clang 9.0 **RC4,** which is compiled with 
-DLLVM_ENABLE_ASSERTIONS=ON. When @hans sent the email for 9.0.1 I realized 
this flag is turned off only for the final release :-(
3. I was using all sorts of options (/MT /GS- /arch:AVX) which actually 
improved build times, when compared with the baseline Clang release (see graphs 
below)

We indeed have a security Windows service running on our machines which we 
//cannot// disable.

I have re-run the tests on 6 different configurations (see graph for specs)
Overall, this patch seems to reduce build times by about **30 sec** on 
many-cores machines, and about **15-20 sec** on 6-cores. The 28-core machine 
did not see any improvements, I cannot explain why.
The variability is also reduced when this patch is applied. This seems to be 
consistent across all machines.

WARNING: the X coordinate (time) does not start at zero

F10949529: cc1_4-core.png 

F10949531: cc1_6-core_w7.png 

F10949533: cc1_6-core_w10.png 

F10949535: cc1_28-core_ws2016.png 

F10949537: cc1_36-core-ws2016.png 

F10949539: cc1_36-core-w10.png 

In my sense, the patch remains valid overall:

- It is easier to debug with just one process.
- Later on, I'd like to put forward a patch which (optionally) replaces the CRT 
allocator with rpmalloc, and not having this current patch would have less 
impact.
- I have an alternate proposal for D52193  and 
D69582  which uses a thread pool instead of a 
process pool, and this patch is needed for that purpose (however that requires 
reviving & solving the thread-safe cl::opt RFC).


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

https://reviews.llvm.org/D69825



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 231786.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

clang-format change (from @aprantl) , add a little details into summary related 
to recent discussion,
move tests from CodeGenCXX to CodeGen as they are all C tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696

Files:
  clang/include/clang/AST/ASTConsumer.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/debug-info-extern-duplicate-2.c
  clang/test/CodeGen/debug-info-extern-duplicate.c
  clang/test/CodeGen/debug-info-extern-unused.c
  clang/test/CodeGen/debug-info-extern-var-char-2.c
  clang/test/CodeGen/debug-info-extern-var-char.c
  clang/test/CodeGen/debug-info-extern-var-func.c
  clang/test/CodeGen/debug-info-extern-var-multi.c
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/unittests/Transforms/Utils/CloningTest.cpp

Index: llvm/unittests/Transforms/Utils/CloningTest.cpp
===
--- llvm/unittests/Transforms/Utils/CloningTest.cpp
+++ llvm/unittests/Transforms/Utils/CloningTest.cpp
@@ -764,7 +764,7 @@
 
 DBuilder.createGlobalVariableExpression(
 Subprogram, "unattached", "unattached", File, 1,
-DBuilder.createNullPtrType(), false, Expr);
+DBuilder.createNullPtrType(), false, true, Expr);
 
 auto *Entry = BasicBlock::Create(C, "", F);
 IBuilder.SetInsertPoint(Entry);
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1290,7 +1290,7 @@
   return wrap(unwrap(Builder)->createGlobalVariableExpression(
   unwrapDI(Scope), {Name, NameLen}, {Linkage, LinkLen},
   unwrapDI(File), LineNo, unwrapDI(Ty), LocalToUnit,
-  unwrap(Expr), unwrapDI(Decl),
+  true, unwrap(Expr), unwrapDI(Decl),
   nullptr, AlignInBits));
 }
 
Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -639,13 +639,14 @@
 
 DIGlobalVariableExpression *DIBuilder::createGlobalVariableExpression(
 DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *F,
-unsigned LineNumber, DIType *Ty, bool isLocalToUnit, DIExpression *Expr,
+unsigned LineNumber, DIType *Ty, bool isLocalToUnit,
+bool isDefined, DIExpression *Expr,
 MDNode *Decl, MDTuple *templateParams, uint32_t AlignInBits) {
   checkGlobalVariableScope(Context);
 
   auto *GV = DIGlobalVariable::getDistinct(
   VMContext, cast_or_null(Context), Name, LinkageName, F,
-  LineNumber, Ty, isLocalToUnit, true, cast_or_null(Decl),
+  LineNumber, Ty, isLocalToUnit, isDefined, cast_or_null(Decl),
   templateParams, AlignInBits);
   if (!Expr)
 Expr = createExpression();
Index: llvm/include/llvm/IR/DIBuilder.h
===
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -581,7 +581,7 @@
 ///specified)
 DIGlobalVariableExpression *createGlobalVariableExpression(
 DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
-unsigned LineNo, DIType *Ty, bool isLocalToUnit,
+unsigned LineNo, DIType *Ty, bool isLocalToUnit, bool isDefined = true,
 DIExpression *Expr = nullptr, MDNode *Decl = nullptr,
 MDTuple *templateParams = nullptr, uint32_t AlignInBits = 0);
 
Index: clang/test/CodeGen/debug-info-extern-var-multi.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-var-multi.c
@@ -0,0 +1,13 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  extern short sh;
+  return ch + sh;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[Tch:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: distinct !DIGlobalVariable(name: "sh",{{.*}} type: ![[Tsh:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: ![[Tsh]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+// CHECK: ![[Tch]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
Index: clang/test/CodeGen/debug-info-extern-var-func.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-var-func.c
@@ -0,0 +1,13 

[PATCH] D70302: [CodeGen] Fix clang crash on aggregate initialization of array of labels

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70302#1764261 , @johannes wrote:

> I see, apologies for the premature commit.


No problem, LLVM's code review practices are somewhat opaque.




Comment at: clang/test/CodeGen/label-array-aggregate-init.c:1
+// RUN: %clang -cc1 -emit-llvm %s -o /dev/null
+

johannes wrote:
> rnk wrote:
> > It's best practice to filecheck for something, even if this used to crash.
> I guess the test should make sure that the array is constructed successfully 
> after codegen.
> I'm unsure about the best way to test that. 
I went ahead and did it in rG536cedaecbe586ec9cf86d5102872adc27e6ea23.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70302



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

[clang] 536ceda - FileCheck IR output for blockaddress in new test

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-12-02T15:05:50-08:00
New Revision: 536cedaecbe586ec9cf86d5102872adc27e6ea23

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

LOG: FileCheck IR output for blockaddress in new test

Minor improvement to a test added in 1ac700cdef787383ad49a

Added: 


Modified: 
clang/test/CodeGen/label-array-aggregate-init.c

Removed: 




diff  --git a/clang/test/CodeGen/label-array-aggregate-init.c 
b/clang/test/CodeGen/label-array-aggregate-init.c
index 6821fd355ec1..5cefd8d270c0 100644
--- a/clang/test/CodeGen/label-array-aggregate-init.c
+++ b/clang/test/CodeGen/label-array-aggregate-init.c
@@ -1,6 +1,10 @@
-// RUN: %clang -cc1 -emit-llvm %s -o /dev/null
+// RUN: %clang -cc1 -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck 
%s
+
+// CHECK: @constinit = private global [3 x i8*] [i8* blockaddress(@main, %L), 
i8* null, i8* null]
+
+void receivePtrs(void **);
 
 int main() {
 L:
-  (void)(void *[]){ &, 0, 0 };
+  receivePtrs((void *[]){ &, 0, 0 });
 }



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

[PATCH] D70748: [clang test] Do not assume default target

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre closed this revision.
thopre added a comment.

Committed as 717140a0dcc651ca2fec23248d1675fb2d388b9c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70748



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

[PATCH] D69088: [Lex] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69088



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

[clang] 717140a - [clang test] Do not assume default target

[Thomas Preud'homme via cfe-commits]

Author: Thomas Preud'homme
Date: 2019-12-02T22:57:30Z
New Revision: 717140a0dcc651ca2fec23248d1675fb2d388b9c

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

LOG: [clang test] Do not assume default target

Summary:
clang test Driver/darwin-opt-record.c assumes the default target is
x86_64 by its uses of the -arch x86_64 and -arch x86_64h and thus fail
on systems where it is not the case. Adding a target
x86_64-apple-darwin10 reveals another problem: the driver refuses 2
-arch for an assembly output so this commit also changes the output to
be an object file.

Reviewers: thegameg

Reviewed By: thegameg

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/test/Driver/darwin-opt-record.c

Removed: 




diff  --git a/clang/test/Driver/darwin-opt-record.c 
b/clang/test/Driver/darwin-opt-record.c
index ca0fad7ee16d..7c674819663a 100644
--- a/clang/test/Driver/darwin-opt-record.c
+++ b/clang/test/Driver/darwin-opt-record.c
@@ -1,6 +1,6 @@
 // REQUIRES: system-darwin
 
-// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
+// RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO 
-fsave-optimization-record -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-MULTIPLE-ARCH
 //
 // CHECK-MULTIPLE-ARCH: "-cc1"
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"



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

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

@aaron.ballman

Type attributes are definitely more flexible as in they can be applied in more 
contexts. These attributes, however, make no sense outside of a function 
declaration, or maybe a type alias. So I feel like we could argue on both 
sides. I made a minimal version of the attribute so it can potentially unblock 
dependent revisions (the static analyzer changes) and plan to add more 
diagnostics in follow-up patches. What do you think?


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

https://reviews.llvm.org/D70469



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

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 231784.
xazax.hun added a comment.

- Convert to type attributes.


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

https://reviews.llvm.org/D70469

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-handles.cpp

Index: clang/test/Sema/attr-handles.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-handles.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+void f(int __attribute__((acquire_handle)) * a);
+void (*fp)(int __attribute__((use_handle)) handle);
+auto lambda = [](int __attribute__((use_handle)) handle) -> 
+int __attribute__((acquire_handle)) { return 0; };
+void g(int __attribute__((acquire_handle)) a); // TODO: diagnose this. The acquire attribute only makes sense for outputs.
+void h(int __attribute__((acquire_handle(1))) *a); // expected-error {{'acquire_handle' attribute takes no arguments}}
+int i() __attribute__((release_handle)); // expected-warning {{'release_handle' only applies to non-function types; type here is 'int ()'}}
+int j() __attribute__((use_handle)); // expected-warning {{'use_handle' only applies to non-function types; type here is 'int ()'}}
+int __attribute__((acquire_handle)) a; // TODO: diagnose this. The type attribute only makes sense for function parameters, return types, type aliases.
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7403,6 +7403,20 @@
   }
 }
 
+template 
+static void handleHandleAttr(TypeProcessingState , QualType ,
+ ParsedAttr ) {
+  if (CurType->isFunctionType()) {
+State.getSema().Diag(Attr.getLoc(),
+ diag::warn_type_attribute_wrong_type_str)
+<< Attr.getAttrName()->getName() << "non-function" << CurType;
+return;
+  }
+  ASTContext  = State.getSema().Context;
+  CurType = State.getAttributedType(createSimpleAttr(Ctx, Attr),
+CurType, CurType);
+  Attr.setUsedAsTypeAttr();
+}
 
 static void processTypeAttrs(TypeProcessingState , QualType ,
  TypeAttrLocation TAL,
@@ -7600,6 +7614,15 @@
   else if (!handleFunctionTypeAttr(state, attr, type))
 distributeFunctionTypeAttr(state, attr, type);
   break;
+case ParsedAttr::AT_UseHandle:
+  handleHandleAttr(state, type, attr);
+  break;
+case ParsedAttr::AT_AcquireHandle:
+  handleHandleAttr(state, type, attr);
+  break;
+case ParsedAttr::AT_ReleaseHandle:
+  handleHandleAttr(state, type, attr);
+  break;
 }
 
 // Handle attributes that are defined in a macro. We do not want this to be
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3042,6 +3042,9 @@
   "'%0' only applies to %select{function|pointer|"
   "Objective-C object or block pointer}1 types; type here is %2">,
   InGroup;
+def warn_type_attribute_wrong_type_str : Warning<
+  "'%0' only applies to %1 types; type here is %2">,
+  InGroup;
 def warn_incomplete_encoded_type : Warning<
   "encoding of %0 type is incomplete because %1 component has unknown encoding">,
   InGroup>;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4565,3 +4565,62 @@
   }
   }];
 }
+
+def HandleDocs : DocumentationCategory<"Handle Attributes"> {
+  let Content = [{
+Handles are a way to identify resources like files, sockets, and processes.
+They are more opaque than pointers and widely used in system programming. They
+have similar risks such as never releasing a resource associated with a handle,
+attempting to use a handle that was already released, or trying to release a
+handle twice. Using the annotations below it is possible to make the ownership
+of the handles clear: whose responsibility is to release them. They can also
+aid static analysis tools to find bugs.
+  }];
+}
+
+def AcquireHandleDocs : Documentation {
+  let Category = HandleDocs;
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed
+to fill the corresponding argument with a new handle.
+
+.. code-block:: c++
+
+  // Output arguments from Zircon.
+  zx_status_t zx_socket_create(uint32_t options,
+   zx_handle_t __attribute__((acquire_handle))* out0,
+   zx_handle_t __attribute__((acquire_handle))* out1);
+
+
+  // 

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you update the ClangStyleOption.rst (using the tool in docs/tools)

and as you are adding a new option would you  add a line or two in the 
docs/ReleaseNotes.rst in the clang-format section


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70923: Fix comment to more accurately describe C++ language requirements around tail padding.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG711c669ae926: Fix comment to more accurately describe C++ 
language requirements around tail… (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70923

Files:
  clang/include/clang/Basic/TargetCXXABI.h


Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -277,27 +277,18 @@
   /// padding of a base class?
   ///
   /// This decision cannot be changed without breaking platform ABI
-  /// compatibility, and yet it is tied to language guarantees which
-  /// the committee has so far seen fit to strengthen no less than
-  /// three separate times:
-  ///   - originally, there were no restrictions at all;
-  ///   - C++98 declared that objects could not be allocated in the
-  /// tail padding of a POD type;
-  ///   - C++03 extended the definition of POD to include classes
-  /// containing member pointers; and
-  ///   - C++11 greatly broadened the definition of POD to include
-  /// all trivial standard-layout classes.
-  /// Each of these changes technically took several existing
-  /// platforms and made them permanently non-conformant.
+  /// compatibility. In ISO C++98, tail padding reuse was only permitted for
+  /// non-POD base classes, but that restriction was removed retroactively by
+  /// DR 43, and tail padding reuse is always permitted in all de facto C++
+  /// language modes. However, many platforms use a variant of the old C++98
+  /// rule for compatibility.
   enum TailPaddingUseRules {
 /// The tail-padding of a base class is always theoretically
-/// available, even if it's POD.  This is not strictly conforming
-/// in any language mode.
+/// available, even if it's POD.
 AlwaysUseTailPadding,
 
 /// Only allocate objects in the tail padding of a base class if
 /// the base class is not POD according to the rules of C++ TR1.
-/// This is non-strictly conforming in C++11 mode.
 UseTailPaddingUnlessPOD03,
 
 /// Only allocate objects in the tail padding of a base class if


Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -277,27 +277,18 @@
   /// padding of a base class?
   ///
   /// This decision cannot be changed without breaking platform ABI
-  /// compatibility, and yet it is tied to language guarantees which
-  /// the committee has so far seen fit to strengthen no less than
-  /// three separate times:
-  ///   - originally, there were no restrictions at all;
-  ///   - C++98 declared that objects could not be allocated in the
-  /// tail padding of a POD type;
-  ///   - C++03 extended the definition of POD to include classes
-  /// containing member pointers; and
-  ///   - C++11 greatly broadened the definition of POD to include
-  /// all trivial standard-layout classes.
-  /// Each of these changes technically took several existing
-  /// platforms and made them permanently non-conformant.
+  /// compatibility. In ISO C++98, tail padding reuse was only permitted for
+  /// non-POD base classes, but that restriction was removed retroactively by
+  /// DR 43, and tail padding reuse is always permitted in all de facto C++
+  /// language modes. However, many platforms use a variant of the old C++98
+  /// rule for compatibility.
   enum TailPaddingUseRules {
 /// The tail-padding of a base class is always theoretically
-/// available, even if it's POD.  This is not strictly conforming
-/// in any language mode.
+/// available, even if it's POD.
 AlwaysUseTailPadding,
 
 /// Only allocate objects in the tail padding of a base class if
 /// the base class is not POD according to the rules of C++ TR1.
-/// This is non-strictly conforming in C++11 mode.
 UseTailPaddingUnlessPOD03,
 
 /// Only allocate objects in the tail padding of a base class if
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 711c669 - Fix comment to more accurately describe C++ language requirements around tail padding.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-02T14:41:27-08:00
New Revision: 711c669ae92658aecc6fabccc583594924bac6d7

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

LOG: Fix comment to more accurately describe C++ language requirements around 
tail padding.

Summary:
As of C++ core issue 43 (http://wg21.link/cwg43), which was voted into
the C++ working draft in 1999, it is not permissible to memcpy a base
class subobject, even if it's of POD type, so there is no problem with
reusing the tail padding of a base class. That issue was voted into the
standard in DR status, so it applies retroactively to C++98 (and is in
any case part of C++03).

So stop suggesting that AlwaysUseTailPadding mode is non-conforming.

Reviewers: rjmccall

Reviewed By: rjmccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Basic/TargetCXXABI.h

Removed: 




diff  --git a/clang/include/clang/Basic/TargetCXXABI.h 
b/clang/include/clang/Basic/TargetCXXABI.h
index b1be40272513..60343fe99c1d 100644
--- a/clang/include/clang/Basic/TargetCXXABI.h
+++ b/clang/include/clang/Basic/TargetCXXABI.h
@@ -277,27 +277,18 @@ class TargetCXXABI {
   /// padding of a base class?
   ///
   /// This decision cannot be changed without breaking platform ABI
-  /// compatibility, and yet it is tied to language guarantees which
-  /// the committee has so far seen fit to strengthen no less than
-  /// three separate times:
-  ///   - originally, there were no restrictions at all;
-  ///   - C++98 declared that objects could not be allocated in the
-  /// tail padding of a POD type;
-  ///   - C++03 extended the definition of POD to include classes
-  /// containing member pointers; and
-  ///   - C++11 greatly broadened the definition of POD to include
-  /// all trivial standard-layout classes.
-  /// Each of these changes technically took several existing
-  /// platforms and made them permanently non-conformant.
+  /// compatibility. In ISO C++98, tail padding reuse was only permitted for
+  /// non-POD base classes, but that restriction was removed retroactively by
+  /// DR 43, and tail padding reuse is always permitted in all de facto C++
+  /// language modes. However, many platforms use a variant of the old C++98
+  /// rule for compatibility.
   enum TailPaddingUseRules {
 /// The tail-padding of a base class is always theoretically
-/// available, even if it's POD.  This is not strictly conforming
-/// in any language mode.
+/// available, even if it's POD.
 AlwaysUseTailPadding,
 
 /// Only allocate objects in the tail padding of a base class if
 /// the base class is not POD according to the rules of C++ TR1.
-/// This is non-strictly conforming in C++11 mode.
 UseTailPaddingUnlessPOD03,
 
 /// Only allocate objects in the tail padding of a base class if



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

[PATCH] D70923: Fix comment to more accurately describe C++ language requirements around tail padding.

[John McCall via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70923



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

[PATCH] D70926: Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 created this revision.
ghvg1313 added a reviewer: klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

>From `clang-format` version 3.7.0 and up, , there is no way to keep following 
>format of ObjectiveC block:

  - (void)_aMethod
  {
  [self.test1 t:self w:self callback:^(typeof(self) self, NSNumber *u, 
NSNumber *v) {
  u = c;
  }]
  }

Regardless of the change in `.clang-format` configuration file, all parameters 
will be lined up so that colons will be on the same column, like following:

  - (void)_aMethod
  {
  [self.test1 t:self
  w:self
   callback:^(typeof(self) self, NSNumber *u, NSNumber *v) {
   u = c;
   }]
  }

Considering with ObjectiveC, the first code style is cleaner & more readable 
for some people, I've added a config option: 
`ObjCDontBreakBeforeNestedBlockParam` (boolean) so that if it is enable, the 
first code style will be favored.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70926

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1,6 +1,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCDontBreakBeforeNestedBlockParam = true;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCDontBreakBeforeNestedBlockParam",
+   Style.ObjCDontBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -791,7 +793,7 @@
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
-  LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCDontBreakBeforeNestedBlockParam = false;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -863,7 +863,7 @@
   bool NestedBlockSpecialCase =
   !Style.isCpp() && Current.is(tok::r_brace) && State.Stack.size() > 1 &&
   State.Stack[State.Stack.size() - 2].NestedBlockInlined;
-  if (!NestedBlockSpecialCase)
+  if (!NestedBlockSpecialCase && !Style.ObjCDontBreakBeforeNestedBlockParam)
 for (unsigned i = 0, e = State.Stack.size() - 1; i != e; ++i)
   State.Stack[i].BreakBeforeParameter = true;
 
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+!Style.ObjCDontBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@dblaikie  The concern for that users using -fstandalone-debug may suddenly see 
a bloat of external var debug info is totally valid. Let me just stick to BPF 
use case for now. If somebody else ever wants this information with 
`-fstandalone-debug`, the restriction can be evaluated and relaxed then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[clang-tools-extra] 936de1c - Remove extraneous semicolon.

[Bill Wendling via cfe-commits]

Author: Bill Wendling
Date: 2019-12-02T14:09:21-08:00
New Revision: 936de1c5bc2dfbec25318985cddb7345d989a7ee

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

LOG: Remove extraneous semicolon.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 7d74641be719..28d4c432683b 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -283,7 +283,7 @@ Range toRange(const SymbolLocation ) {
   R.end.line = L.End.line();
   R.end.character = L.End.column();
   return R;
-};
+}
 
 // Return all rename occurrences (per the index) outside of the main file,
 // grouped by the absolute file path.



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

[PATCH] D70923: Fix comment to more accurately describe C++ language requirements around tail padding.

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

rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added a project: clang.

As of C++ core issue 43 (http://wg21.link/cwg43), which was voted into
the C++ working draft in 1999, it is not permissible to memcpy a base
class subobject, even if it's of POD type, so there is no problem with
reusing the tail padding of a base class. That issue was voted into the
standard in DR status, so it applies retroactively to C++98 (and is in
any case part of C++03).

So stop suggesting that AlwaysUseTailPadding mode is non-conforming.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70923

Files:
  clang/include/clang/Basic/TargetCXXABI.h


Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -277,27 +277,18 @@
   /// padding of a base class?
   ///
   /// This decision cannot be changed without breaking platform ABI
-  /// compatibility, and yet it is tied to language guarantees which
-  /// the committee has so far seen fit to strengthen no less than
-  /// three separate times:
-  ///   - originally, there were no restrictions at all;
-  ///   - C++98 declared that objects could not be allocated in the
-  /// tail padding of a POD type;
-  ///   - C++03 extended the definition of POD to include classes
-  /// containing member pointers; and
-  ///   - C++11 greatly broadened the definition of POD to include
-  /// all trivial standard-layout classes.
-  /// Each of these changes technically took several existing
-  /// platforms and made them permanently non-conformant.
+  /// compatibility. In ISO C++98, tail padding reuse was only permitted for
+  /// non-POD base classes, but that restriction was removed retroactively by
+  /// DR 43, and tail padding reuse is always permitted in all de facto C++
+  /// language modes. However, many platforms use a variant of the old C++98
+  /// rule for compatibility.
   enum TailPaddingUseRules {
 /// The tail-padding of a base class is always theoretically
-/// available, even if it's POD.  This is not strictly conforming
-/// in any language mode.
+/// available, even if it's POD.
 AlwaysUseTailPadding,
 
 /// Only allocate objects in the tail padding of a base class if
 /// the base class is not POD according to the rules of C++ TR1.
-/// This is non-strictly conforming in C++11 mode.
 UseTailPaddingUnlessPOD03,
 
 /// Only allocate objects in the tail padding of a base class if


Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -277,27 +277,18 @@
   /// padding of a base class?
   ///
   /// This decision cannot be changed without breaking platform ABI
-  /// compatibility, and yet it is tied to language guarantees which
-  /// the committee has so far seen fit to strengthen no less than
-  /// three separate times:
-  ///   - originally, there were no restrictions at all;
-  ///   - C++98 declared that objects could not be allocated in the
-  /// tail padding of a POD type;
-  ///   - C++03 extended the definition of POD to include classes
-  /// containing member pointers; and
-  ///   - C++11 greatly broadened the definition of POD to include
-  /// all trivial standard-layout classes.
-  /// Each of these changes technically took several existing
-  /// platforms and made them permanently non-conformant.
+  /// compatibility. In ISO C++98, tail padding reuse was only permitted for
+  /// non-POD base classes, but that restriction was removed retroactively by
+  /// DR 43, and tail padding reuse is always permitted in all de facto C++
+  /// language modes. However, many platforms use a variant of the old C++98
+  /// rule for compatibility.
   enum TailPaddingUseRules {
 /// The tail-padding of a base class is always theoretically
-/// available, even if it's POD.  This is not strictly conforming
-/// in any language mode.
+/// available, even if it's POD.
 AlwaysUseTailPadding,
 
 /// Only allocate objects in the tail padding of a base class if
 /// the base class is not POD according to the rules of C++ TR1.
-/// This is non-strictly conforming in C++11 mode.
 UseTailPaddingUnlessPOD03,
 
 /// Only allocate objects in the tail padding of a base class if
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:261
+
+  // TODO: do we want to emit notes for escapes?
+  //   do we want to emit notes for for error checks? I.e. on which branch

NoQ wrote:
> Will we ever emit a report against an escaped symbol? If not, there will 
> never be a report to attach the note to.
I think there might be cases where we go from an escaped symbol to released and 
so on. But you are right, it would not contribute to understanding the actual 
bug report.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:262-263
+  // TODO: do we want to emit notes for escapes?
+  //   do we want to emit notes for for error checks? I.e. on which branch
+  //   do we consider the acquire/release success or fail.
+

NoQ wrote:
> If the function is implemented as an eager state split, having a note is 
> great and easy to implement.
> 
> If it's implemented as a Schrödinger state split, then i think we should 
> still emit a note at the collapse point (especially given that it may happen 
> in a deeper stack frame than the call that produced the symbol). But this 
> makes me recognize the need for note tags in `evalAssume`.
I agree.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:521
+  if (Type.isSuppressOnSink()) {
+const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
+if (AcquireNode) {

NoQ wrote:
> I wonder how terrible would it be to allow bug visitors (or note tags, which 
> are just an "API sugar" for bug visitors) to mutate the uniqueing location. 
> Because such information is naturally available in the visitor.
> 
> It most likely won't work because the information is necessary before the 
> visitors are run. But it would have been nice though, so i feel as if we're 
> missing an opportunity here.
On one hand, it would be nice :) On the other hand we could have more than one 
visitor and it would be a hell to debug when the visitors disagree an the 
uniqueing location. If we can come up with an API that is not that easy to 
misuse, I am in favor of a change like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725



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

[PATCH] D70701: Fix more VFS tests on Windows

[Adrian McCarthy via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078
+std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl 
) const {
+  if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
+  llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows))
+return {};

rnk wrote:
> I think Posix-style paths are considered absolute, even on Windows. The 
> opposite isn't true, a path with a drive letter is considered to be relative 
> if the default path style is Posix. But, I don't think that matters. We only 
> end up in this mixed path style situation on Windows.
> 
> Does this change end up being necessary? I would expect the existing 
> implementation of makeAbsolute to do the right thing on Windows as is. I 
> think the other change seems to be what matters.
Yes, it's necessary.  The Posix-style path `\tests` is not considered absolute 
on Windows.  Thus the original makeAboslute would merge it with the current 
working directory, which gives it a drive letter, like `D:\tests\`.  The drive 
letter component then causes comparisons to fail.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1431
 
-if (IsRootEntry && !sys::path::is_absolute(Name)) {
-  assert(NameValueNode && "Name presence should be checked earlier");

rnk wrote:
> Is there a way to unit test this? I see some existing tests in 
> llvm/unittests/Support/VirtualFileSystemTest.cpp.
> 
> I looked at the yaml files in the VFS tests this fixes, and I see entries 
> like this:
> { 'name': '/tests', 'type': 'directory', ... },
> { 'name': '/indirect-vfs-root-files', 'type': 'directory', ... },
> { 'name': 'C:/src/llvm-project/clang/test/VFS/Inputs/Broken.framework', 
> 'type': 'directory', ... },
> { 'name': 
> 'C:/src/llvm-project/build/tools/clang/test/VFS/Output/vfsroot-with-overlay.c.tmp/cache',
>  'type': 'directory', ... },
> 
> So it makes sense to me that we need to handle both kinds of absolute path.
> Is there a way to unit test this?

What do you mean by "this"?  The `/tests` and `/indirect-vfs-root-files` 
entries in that yaml are the ones causing several tests to fail without this 
fix, so I take it that this is already being tested.  But perhaps you meant 
testing something more specific?



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1448-1449
+  // which style we have, and use it consistently.
+  if (sys::path::is_absolute(Name, sys::path::Style::posix)) {
+path_style = sys::path::Style::posix;
+  } else if (sys::path::is_absolute(Name, sys::path::Style::windows)) {

rnk wrote:
> I wonder if there's a simpler fix here. If the working directory is an 
> absolute Windows-style path, we could prepend the drive letter of the working 
> directory onto any absolute Posix style paths read from YAML files. That's 
> somewhat consistent with what native Windows tools do. In cmd, you can run 
> `cd \Windows`, and that will mean `C:\Windows` if the active driver letter is 
> C. I think on Windows every drive has an active directory, but that's not 
> part of the file system model.
I'm not seeing how this would be simpler.

I don't understand the analogy of your proposal to what the native Windows 
tools do.  When you say, `cd \Windows`, the `\Windows` is _not_ an absolute 
path.  It's relative to the current drive.

I could be wrong, but I don't think prepending the drive onto absolution Posix 
style paths read from YAML would work.  That would give us something like 
`D:/tests` (which is what was happening in makeAbsolute) and that won't match 
paths generated from non-YAML sources, which will still come out as 
`/tests/foo.h`.

> I think on Windows every drive has an active directory 

Yep.  I think of it as "every drive has a _current_ directory" and each process 
has a current drive.  When you want the current working directory, you get the 
current directory of the current drive.


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

https://reviews.llvm.org/D70701



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

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/test/Misc/permissions.cpp:8
 
 // RUN: umask 002
 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t

If you change this to `umask 022`, does that result in `rw-r-`? That would 
make the test meaningful on your system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70854



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Sorry, my previous comment might not be clear. The point is, the same call 
might both acquire and release handles (there are such calls in Fuchsia), so we 
might end up adding more than one note for a call for which we would need to 
add more than one transitions.  If you think it is still cleaner that way, I 
could do that as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725



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

[PATCH] D63276: [AST] Add FunctionDecl::getParametersSourceRange()

[Nicolas Manichon via Phabricator via cfe-commits]

nicolas added a comment.

Hi, if this patch still LGTY, could you commit it on my behalf?
I don't have commit access.
Thanks a lot!


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

https://reviews.llvm.org/D63276



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I was thinking about note tags, but since we iterate on the argument/parameter 
pairs and might change the state in each iteration we would need to add a new 
transition after each change. I was wondering if using note tags would worth 
the cost (both in performance due to the additional number of nodes and in code 
complexity due to making transitions more often).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> I feel like the 2. is a better solution. Of course, that change might have a 
> performance impact as well.

Yes, i'm all for '2.'. There's no need to make this callback more complicated 
than it already is.

As for performance, it's messy and suffers from a deeper problem: the number of 
escaped symbols is potentially infinite. The following false positive 
illustrates that well:

  void invalidate(int **x);
  
  void foo(int **x) {
    int *y = *x;
    if (*y == 0) {
  // **x should be invalidated here!
  invalidate(x);
    }
// Should not warn about division by zero!
    1 / *y;
  }

Therefore one does not simply compose a list of escaped symbols. We need 
something similar to `SymbolReaper` but for invalidation/escapes. And //then// 
we'll talk about performance.


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

https://reviews.llvm.org/D70470



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

[PATCH] D69840: [Basic] Make SourceLocation usable as key in hash maps, NFCI

[Adrian Prantl via Phabricator via cfe-commits]

aprantl resigned from this revision.
aprantl added a comment.

@rsmith can you take a look at this to see if this is going into the right 
direction?


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

https://reviews.llvm.org/D69840



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

[PATCH] D70048: [LLD] Add NetBSD support as a new flavor of LLD (nb.lld)

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> If we could get this patch merged, we could build and link the whole LLVM 
> with lld on NetBSD and it would increase the productivity of the bot (better 
> build times). Right now we need to maintain hacks to link at most with 2/3 
> cores, while 5/6 ones are idling doing nothing due to enormous RAM 
> consumption of GNU ld.

I will be very happy to see the productivity of NetBSD build bot gets improved. 
My comments below are about the approach you choose in this patch.

> FreeBSD/OpenBSD ship with mutations of the behavior of ELF/GNU/Linux in 
> certain nits, we do the same with our driver.

The FreeBSD specific code is just the following 4 lines in ELF/Driver.cpp:

  if (s.endswith("_fbsd")) {
s = s.drop_back(5);
osabi = ELFOSABI_FREEBSD;
  }

`PT_OPENBSD_*` extensions are related to 
https://bugs.llvm.org//show_bug.cgi?id=31288 , which predated my involvement 
into lld.
It was not a good example of issue reports. Neither the bug nor the link it 
referenced provided rationale why such extensions were needed. CC @bradsmith 
here. I will also ask in their IRC channel and/or send an email to their 
mailing list.

The NetBSD extension does much more than FreeBSD (4 lines of code) and OpenBSD 
(7 lines of code modulo comments).
On the contrary, the NetBSD patch introduces another toplevel driver nb.lld, 
and includes ~200 lines of change.
Moreoever, I anticipate you will post another patch with probably than more 
than 200 lines of code that does 
https://blog.netbsd.org/tnf/entry/the_first_report_on_lld#handling-of-indirect-shared-library-dependencies
 , which many people (probably even the GNU maintainers) feel no longer 
appropriate nowadays.

> the linker MUST work in the standalone mode

By standalone mode, do you mean $(LD) or ld? I think for most such use cases, 
we should probably change them to do `gcc/clang -nostdlib` instead. In most 
cases users are not supposed to call ld directly. On FreeBSD (and Google 
servers and Android, if you consider them distributions), it has been proved 
many packages don't need ld customization to function properly.

If you can't migrate off those packages right now, a shell script that gets 
installed at "/usr/bin/ld" will probably work.

> clang NetBSD driver shall not hardcode LLD specific options

Several toolchains in the clang/lib/Driver/ToolChains detect lld and make lld 
specific decisions there. While I hope linker specific options should be kept 
at a minimum, I understand that sometimes it is unavoidable. When we have no 
choice but to add linker specific options, adding such code to clangDriver is 
an established practice followed by almost every toolchain (see Android, 
Fuchsia, MinGW, MSVC, etc). Doing the driver thing in two places will more 
likely to cause conflicts.

> the linker must have support for cross-building

I think this patch will actually harm cross building that targets NetBSD. 
Before, ld.lld is shared by all ELF platforms: Linux, FreeBSD, Fuchsia, 
Android, ChromeOS, etc. If an ELF change works on Linux, it will assuredly work 
on other platforms. This patch will introduce a new driver that naturally gets 
less test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70048



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

[clang-tools-extra] 93f7761 - Revert "[clangd] repair mac tests for 88bccded8fa1"

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-12-02T22:13:29+01:00
New Revision: 93f77617abba512d2861e2fc50ce385883f587b6

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

LOG: Revert "[clangd] repair mac tests for 88bccded8fa1"

Revert "[clangd] Try harder to find a plausible `clang` as argv0, particularly 
on Mac."

Added: 


Modified: 
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 8e78fedf44bb..ed3b86f0f55b 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -18,9 +18,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Program.h"
 #include 
 #include 
 #include 
@@ -29,113 +27,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-// Query apple's `xcrun` launcher, which is the source of truth for "how 
should"
-// clang be invoked on this system.
-llvm::Optional queryXcrun(llvm::ArrayRef Argv) {
-  auto Xcrun = llvm::sys::findProgramByName("xcrun");
-  if (!Xcrun) {
-log("Couldn't find xcrun. Hopefully you have a non-apple toolchain...");
-return llvm::None;
-  }
-  llvm::SmallString<64> OutFile;
-  llvm::sys::fs::createTemporaryFile("clangd-xcrun", "", OutFile);
-  llvm::FileRemover OutRemover(OutFile);
-  llvm::Optional Redirects[3] = {
-  /*stdin=*/{""}, /*stdout=*/{OutFile}, /*stderr=*/{""}};
-  vlog("Invoking {0} to find clang installation", *Xcrun);
-  int Ret = llvm::sys::ExecuteAndWait(*Xcrun, Argv,
-  /*Env=*/llvm::None, Redirects,
-  /*SecondsToWait=*/10);
-  if (Ret != 0) {
-log("xcrun exists but failed with code {0}. "
-"If you have a non-apple toolchain, this is OK. "
-"Otherwise, try xcode-select --install.",
-Ret);
-return llvm::None;
-  }
-
-  auto Buf = llvm::MemoryBuffer::getFile(OutFile);
-  if (!Buf) {
-log("Can't read xcrun output: {0}", Buf.getError().message());
-return llvm::None;
-  }
-  StringRef Path = Buf->get()->getBuffer().trim();
-  if (Path.empty()) {
-log("xcrun produced no output");
-return llvm::None;
-  }
-  return Path.str();
-}
-
-// On Mac, `which clang` is /usr/bin/clang. It runs `xcrun clang`, which knows
-// where the real clang is kept. We need to do the same thing,
-// because cc1 (not the driver!) will find libc++ relative to argv[0].
-llvm::Optional queryMacClangPath() {
-#ifndef __APPLE__
-  return llvm::None;
-#endif
-
-  return queryXcrun({"xcrun", "--find", "clang"});
-}
-
-// Resolve symlinks if possible.
-std::string resolve(std::string Path) {
-  llvm::SmallString<128> Resolved;
-  if (llvm::sys::fs::real_path(Path, Resolved))
-return Path; // On error;
-  return Resolved.str();
-}
-
-// Get a plausible full `clang` path.
-// This is used in the fallback compile command, or when the CDB returns a
-// generic driver with no path.
-llvm::StringRef getFallbackClangPath() {
-  static const std::string  = [&]() -> std::string {
-// The driver and/or cc1 sometimes depend on the binary name to compute
-// useful things like the standard library location.
-// We need to emulate what clang on this system is likely to see.
-// cc1 in particular looks at the "real path" of the running process, and
-// so if /usr/bin/clang is a symlink, it sees the resolved path.
-// clangd doesn't have that luxury, so we resolve symlinks ourselves.
-
-// /usr/bin/clang on a mac is a program that redirects to the right clang.
-// We resolve it as if it were a symlink.
-if (auto MacClang = queryMacClangPath())
-  return resolve(std::move(*MacClang));
-// On other platforms, just look for compilers on the PATH.
-for (const char* Name : {"clang", "gcc", "cc"})
-  if (auto PathCC = llvm::sys::findProgramByName(Name))
-return resolve(std::move(*PathCC));
-// Fallback: a nonexistent 'clang' binary next to clangd.
-static int Dummy;
-std::string ClangdExecutable =
-llvm::sys::fs::getMainExecutable("clangd", (void *));
-SmallString<128> ClangPath;
-ClangPath = llvm::sys::path::parent_path(ClangdExecutable);
-llvm::sys::path::append(ClangPath, "clang");
-return ClangPath.str();
-  }();
-  return MemoizedFallbackPath;
-}
-
-// On mac, /usr/bin/clang sets SDKROOT and then invokes the real clang.
-// The effect of this is to 

[PATCH] D70701: Fix more VFS tests on Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078
+std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl 
) const {
+  if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
+  llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows))
+return {};

I think Posix-style paths are considered absolute, even on Windows. The 
opposite isn't true, a path with a drive letter is considered to be relative if 
the default path style is Posix. But, I don't think that matters. We only end 
up in this mixed path style situation on Windows.

Does this change end up being necessary? I would expect the existing 
implementation of makeAbsolute to do the right thing on Windows as is. I think 
the other change seems to be what matters.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1431
 
-if (IsRootEntry && !sys::path::is_absolute(Name)) {
-  assert(NameValueNode && "Name presence should be checked earlier");

Is there a way to unit test this? I see some existing tests in 
llvm/unittests/Support/VirtualFileSystemTest.cpp.

I looked at the yaml files in the VFS tests this fixes, and I see entries like 
this:
{ 'name': '/tests', 'type': 'directory', ... },
{ 'name': '/indirect-vfs-root-files', 'type': 'directory', ... },
{ 'name': 'C:/src/llvm-project/clang/test/VFS/Inputs/Broken.framework', 
'type': 'directory', ... },
{ 'name': 
'C:/src/llvm-project/build/tools/clang/test/VFS/Output/vfsroot-with-overlay.c.tmp/cache',
 'type': 'directory', ... },

So it makes sense to me that we need to handle both kinds of absolute path.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1448-1449
+  // which style we have, and use it consistently.
+  if (sys::path::is_absolute(Name, sys::path::Style::posix)) {
+path_style = sys::path::Style::posix;
+  } else if (sys::path::is_absolute(Name, sys::path::Style::windows)) {

I wonder if there's a simpler fix here. If the working directory is an absolute 
Windows-style path, we could prepend the drive letter of the working directory 
onto any absolute Posix style paths read from YAML files. That's somewhat 
consistent with what native Windows tools do. In cmd, you can run `cd 
\Windows`, and that will mean `C:\Windows` if the active driver letter is C. I 
think on Windows every drive has an active directory, but that's not part of 
the file system model.


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

https://reviews.llvm.org/D70701



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

[PATCH] D70863: [clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D70863#1765668 , @arphaman wrote:

> @sammccall Sam, it looks like the tests are failing on the darwin bots:
>
> http://lab.llvm.org:8080/green/job/clang-stage1-RA/4243/consoleFull


Thanks, and sorry! Fingers crossed that 
82039cbc8d2a0f6fb5995f54e0e4291bcfd0 
 will fix 
it...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70863



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

[clang-tools-extra] 82039cb - [clangd] repair mac tests for 88bccded8fa1

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-12-02T21:55:54+01:00
New Revision: 82039cbc8d2a0f6fb5995f54e0e4291bcfd0

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

LOG: [clangd] repair mac tests for 88bccded8fa1

Added: 


Modified: 
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp 
b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index 01f0ba1b4044..ee7ba4355c05 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -527,6 +527,13 @@ TEST_F(BackgroundIndexTest, UncompilableFiles) {
   }
 }
 
+MATCHER_P(HasPrefix, Prefix, "") {
+  auto Arg = arg; // Force copy.
+  if (Arg.size() > Prefix.size())
+Arg.resize(Prefix.size());
+  return Arg == Prefix;
+}
+
 TEST_F(BackgroundIndexTest, CmdLineHash) {
   MockFSProvider FS;
   llvm::StringMap Storage;
@@ -552,7 +559,8 @@ TEST_F(BackgroundIndexTest, CmdLineHash) {
 
   {
 tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
-EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
+// Accept prefix because -isysroot gets added on mac.
+EXPECT_THAT(CmdStored.CommandLine, HasPrefix(Cmd.CommandLine));
 EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
   }
 
@@ -566,6 +574,7 @@ TEST_F(BackgroundIndexTest, CmdLineHash) {
 
   {
 tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
+EXPECT_THAT(CmdStored.CommandLine, HasPrefix(Cmd.CommandLine));
 EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
 EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
   }

diff  --git 
a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp 
b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index 15f628825b13..857bf26f00dc 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -102,12 +102,20 @@ TEST_F(OverlayCDBTest, GetCompileCommand) {
   Contains("-DA=3"));
 }
 
+// Remove -isysroot injected on mac, if present, to simplify tests.
+std::vector stripSysroot(std::vector Cmd) {
+  // Allow -isysroot injection on Mac.
+  if (Cmd.size() > 2 && Cmd[Cmd.size() - 2] == "-isysroot")
+Cmd.resize(Cmd.size() - 2);
+  return Cmd;
+}
+
 TEST_F(OverlayCDBTest, GetFallbackCommand) {
   OverlayCDB CDB(Base.get(), {"-DA=4"});
-  EXPECT_THAT(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine,
-  ElementsAre(EndsWith("clang"), "-DA=2", testPath("bar.cc"),
-  "-DA=4", "-fsyntax-only",
-  StartsWith("-resource-dir")));
+  EXPECT_THAT(
+  stripSysroot(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine),
+  ElementsAre(EndsWith("clang"), "-DA=2", testPath("bar.cc"), "-DA=4",
+  "-fsyntax-only", StartsWith("-resource-dir")));
 }
 
 TEST_F(OverlayCDBTest, NoBase) {
@@ -118,9 +126,10 @@ TEST_F(OverlayCDBTest, NoBase) {
   EXPECT_THAT(CDB.getCompileCommand(testPath("bar.cc"))->CommandLine,
   Contains("-DA=5"));
 
-  EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine,
-  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6",
-  "-fsyntax-only"));
+  EXPECT_THAT(
+  stripSysroot(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine),
+  ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6",
+  "-fsyntax-only"));
 }
 
 TEST_F(OverlayCDBTest, Watch) {



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

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

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

kpn added inline comments.



Comment at: llvm/include/llvm/IR/IRBuilder.h:268
   I->addAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
-setConstrainedFPFunctionAttr();
   }

mibintc wrote:
> @kpn I got rid of this line because the function attribute is being set in 
> CodeGen
Makes sense.



Comment at: llvm/unittests/IR/IRBuilderTest.cpp:186
   Builder.setIsFPConstrained(true);
+  auto Parent = BB->getParent();
+  Parent->addFnAttr(Attribute::StrictFP);

mibintc wrote:
> @kpn I changed the test to create the function attribute a priori since it 
> will be set in CodeGen before passing to IRBuilder
Right, of course.

I'm not going to quibble over the use of auto. It's fine I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

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

Thanks

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

[PATCH] D69223: WDocumentation: Implement the \anchor.

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 231766.
Mordante marked 2 inline comments as done.
Mordante added a comment.

Addresses review comments.


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

https://reviews.llvm.org/D69223

Files:
  clang/bindings/xml/comment-xml-schema.rng
  clang/include/clang-c/Documentation.h
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/lib/AST/CommentSema.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Index/CommentToXML.cpp
  clang/test/Index/Inputs/CommentXML/valid-inline-command-01.xml
  clang/test/Index/comment-to-html-xml-conversion.cpp
  clang/test/Index/comment-xml-schema.c
  clang/test/Sema/warn-documentation.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXComment.cpp

Index: clang/tools/libclang/CXComment.cpp
===
--- clang/tools/libclang/CXComment.cpp
+++ clang/tools/libclang/CXComment.cpp
@@ -159,6 +159,9 @@
 
   case InlineCommandComment::RenderEmphasized:
 return CXCommentInlineCommandRenderKind_Emphasized;
+
+  case InlineCommandComment::RenderAnchor:
+return CXCommentInlineCommandRenderKind_Anchor;
   }
   llvm_unreachable("unknown InlineCommandComment::RenderKind");
 }
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -497,6 +497,9 @@
 case CXCommentInlineCommandRenderKind_Emphasized:
   printf(" RenderEmphasized");
   break;
+case CXCommentInlineCommandRenderKind_Anchor:
+  printf(" RenderAnchor");
+  break;
 }
 for (i = 0, e = clang_InlineCommandComment_getNumArgs(Comment);
  i != e; ++i) {
Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1057,6 +1057,13 @@
 /// \a A
 int test_inline_no_argument_a_good(int);
 
+// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+/// \anchor
+int test_inline_no_argument_anchor_bad(int);
+
+/// \anchor A
+int test_inline_no_argument_anchor_good(int);
+
 // expected-warning@+1 {{'@b' command does not have a valid word argument}}
 /// @b
 int test_inline_no_argument_b_bad(int);
Index: clang/test/Index/comment-xml-schema.c
===
--- clang/test/Index/comment-xml-schema.c
+++ clang/test/Index/comment-xml-schema.c
@@ -33,6 +33,8 @@
 // RUN: xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng %S/Inputs/CommentXML/valid-enum-01.xml
 //
 // RUN: xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng %S/Inputs/CommentXML/valid-para-kind-01.xml
+//
+// RUN: xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng %S/Inputs/CommentXML/valid-inline-command-01.xml
 
 // RUN: not xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng %S/Inputs/CommentXML/invalid-function-01.xml 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID
 // RUN: not xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng %S/Inputs/CommentXML/invalid-function-02.xml 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID
Index: clang/test/Index/comment-to-html-xml-conversion.cpp
===
--- clang/test/Index/comment-to-html-xml-conversion.cpp
+++ clang/test/Index/comment-to-html-xml-conversion.cpp
@@ -734,6 +734,16 @@
 // CHECK-NEXT: (CXComment_Text Text=[Aaa])
 // CHECK-NEXT: (CXComment_HTMLEndTag Name=[h1])))]
 
+/// \anchor A
+void comment_to_html_conversion_37();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_37:{{.*}} FullCommentAsHTML=[ ] FullCommentAsXML=[comment_to_html_conversion_37c:@F@comment_to_html_conversion_37#void comment_to_html_conversion_37() ]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ ] IsWhitespace)
+// CHECK-NEXT: (CXComment_InlineCommand CommandName=[anchor] RenderAnchor Arg[0]=A)))]
+
 
 /// Aaa.
 class comment_to_xml_conversion_01 {
Index: clang/test/Index/Inputs/CommentXML/valid-inline-command-01.xml
===
--- /dev/null
+++ clang/test/Index/Inputs/CommentXML/valid-inline-command-01.xml
@@ -0,0 +1,9 @@
+
+
+aaa
+
+  
+
+  
+
+
Index: clang/lib/Index/CommentToXML.cpp
===
--- clang/lib/Index/CommentToXML.cpp
+++ clang/lib/Index/CommentToXML.cpp
@@ -297,6 +297,10 @@
 appendToResultWithHTMLEscaping(Arg0);
 Result << "";
 return;
+  case 

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

[Sid Manning via Phabricator via cfe-commits]

sidneym updated this revision to Diff 231765.
sidneym added a comment.

Update testcase.


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

https://reviews.llvm.org/D70919

Files:
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/test/Driver/hexagon-toolchain-elf.c


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -536,3 +536,25 @@
 // RUN:   | FileCheck -check-prefix=CHECK080 %s
 // CHECK080:  "-cc1"
 // CHECK080:  "-Wreturn-type"
+
+// 
-
+// Default, not passing -fuse-ld
+// 
-
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK081 %s
+// CHECK081:  "-march=hexagon"
+// CHECK081:  "-mcpu=hexagonv60"
+// 
-
+// Passing -fuse-ld=lld
+// 
-
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK082 %s
+// CHECK082-NOT:  "-march="
+// CHECK082-NOT:  "-mcpu="
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -209,7 +209,9 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  bool UseLLD = 
Args.getLastArgValue(options::OPT_fuse_ld_EQ).startswith("lld");
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   
//
   // Silence warnings for various options
@@ -232,9 +234,10 @@
   for (const auto  : HTC.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());
 
-  CmdArgs.push_back("-march=hexagon");
-  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
-  CmdArgs.push_back(Args.MakeArgString("-mcpu=hexagon" + CpuVer));
+  if (!UseLLD) {
+CmdArgs.push_back("-march=hexagon");
+CmdArgs.push_back(Args.MakeArgString("-mcpu=hexagon" + CpuVer));
+  }
 
   if (IsShared) {
 CmdArgs.push_back("-shared");


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -536,3 +536,25 @@
 // RUN:   | FileCheck -check-prefix=CHECK080 %s
 // CHECK080:  "-cc1"
 // CHECK080:  "-Wreturn-type"
+
+// -
+// Default, not passing -fuse-ld
+// -
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK081 %s
+// CHECK081:  "-march=hexagon"
+// CHECK081:  "-mcpu=hexagonv60"
+// -
+// Passing -fuse-ld=lld
+// -
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK082 %s
+// CHECK082-NOT:  "-march="
+// CHECK082-NOT:  "-mcpu="
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -209,7 +209,9 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  bool UseLLD = Args.getLastArgValue(options::OPT_fuse_ld_EQ).startswith("lld");
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   //
   // Silence warnings for various options
@@ -232,9 +234,10 @@
   for (const auto  : HTC.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());
 
-  CmdArgs.push_back("-march=hexagon");
-  StringRef CpuVer = 

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 9 inline comments as done.
mibintc added a comment.

I added inline comments describing what I did in this version of the patch to 
address the bug https://bugs.llvm.org/show_bug.cgi?id=44048




Comment at: clang/include/clang/AST/DeclBase.h:1539
+/// Indicates if the function uses Floating Point Constrained Intrinsics
+uint64_t UsesFPIntrin : 1;
   };

This corresponds to "strictfp" LLVM attribute. I add this here because I want 
to collect the information during Sema and set the attribute during CodeGen. 
The next thing I want to do is to add support for modifying float_control via 
pragma within function bodies (enable floating point control at the block 
level).  If I wasn't preparing to support floating_control via statement-level 
pragma then setting the bit could be accomplished entirely within CodeGen.



Comment at: clang/include/clang/AST/DeclBase.h:1560
 /// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-uint64_t NumCtorInitializers : 23;
+uint64_t NumCtorInitializers : 22;
 uint64_t IsInheritingConstructor : 1;

Need to adjust the number of bits here, because it's at the threshold of 
overrunning 64 bits.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444
 
+def warn_drv_experimental_fp_control_incomplete_opt : Warning<
+  "Support for floating point control option %0 is incomplete and 
experimental">,

@kpn thought it would be a good idea to add a Warning that the implementation 
of float control is experimental and partially implemented.  That's what this 
is for.



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

michele.scandale wrote:
> Here the state of the floating point options seems unchanged except for 
> `FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would 
> expect the state of the floating point options to match the one of 
> `-fno-fast-math` except for `FPContract` which you want to be set to "fast".
> 
> I think you might need to replicate the reset for all the option here as 
> well, so at this point I don't know how much worth is to use the optID reset 
> trick for the "fast" case only.
@michele.scandale Thanks for your helpful review, I think I fixed the things 
that you remarked on. I also added a test case for the assertion fail that you 
saw.



Comment at: clang/test/CodeGen/fpconstrained.cpp:10
+
+  template 
+  class  {

This is the test case from the bug report (null deref/segfault/in IRBuilder)



Comment at: llvm/include/llvm/IR/IRBuilder.h:268
   I->addAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
-setConstrainedFPFunctionAttr();
   }

@kpn I got rid of this line because the function attribute is being set in 
CodeGen



Comment at: llvm/unittests/IR/IRBuilderTest.cpp:186
   Builder.setIsFPConstrained(true);
+  auto Parent = BB->getParent();
+  Parent->addFnAttr(Attribute::StrictFP);

@kpn I changed the test to create the function attribute a priori since it will 
be set in CodeGen before passing to IRBuilder


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

So... note tags ?




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:261
+
+  // TODO: do we want to emit notes for escapes?
+  //   do we want to emit notes for for error checks? I.e. on which branch

Will we ever emit a report against an escaped symbol? If not, there will never 
be a report to attach the note to.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:262-263
+  // TODO: do we want to emit notes for escapes?
+  //   do we want to emit notes for for error checks? I.e. on which branch
+  //   do we consider the acquire/release success or fail.
+

If the function is implemented as an eager state split, having a note is great 
and easy to implement.

If it's implemented as a Schrödinger state split, then i think we should still 
emit a note at the collapse point (especially given that it may happen in a 
deeper stack frame than the call that produced the symbol). But this makes me 
recognize the need for note tags in `evalAssume`.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:521
+  if (Type.isSuppressOnSink()) {
+const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
+if (AcquireNode) {

I wonder how terrible would it be to allow bug visitors (or note tags, which 
are just an "API sugar" for bug visitors) to mutate the uniqueing location. 
Because such information is naturally available in the visitor.

It most likely won't work because the information is necessary before the 
visitors are run. But it would have been nice though, so i feel as if we're 
missing an opportunity here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725



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

[PATCH] D53250: [ToolChain] Use default linker if the toolchain uses a custom one

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Sorry about the late response, I was mostly out last week. This recently came 
up again so I'd like to look into it but I'll have to rethink the change and 
come up with a better approach.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53250



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70696#1765823 , @SouraVX wrote:

> @dblaikie  , If you try switching to C compiler. then the DW_TAG_variable is 
> their. for C++ . It's not generated.  Please check godbolt again. -- though 
> strange


Hmm - I don't seem to see it in C either: https://godbolt.org/z/EUufYE (it's 
present in 9.2 as it is with C++, but not present in GCC trunk)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D70696#1765843 , @dblaikie wrote:

> In D70696#1765823 , @SouraVX wrote:
>
> > @dblaikie  , If you try switching to C compiler. then the DW_TAG_variable 
> > is their. for C++ . It's not generated.  Please check godbolt again. -- 
> > though strange
>
>
> Hmm - I don't seem to see it in C either: https://godbolt.org/z/EUufYE (it's 
> present in 9.2 as it is with C++, but not present in GCC trunk)


Ahh,, missed the trunk -- Sorry!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

[Krzysztof Parzyszek via Phabricator via cfe-commits]

kparzysz added inline comments.



Comment at: clang/test/Driver/hexagon-toolchain-elf.c:560
+// CHECK082-NOT:  "-march=hexagon"
+// CHECK082-NOT:  "-mcpu=hexagon"

I don't think there will ever be `"-mcpu=hexagon"` (including the quotation 
marks). Maybe this should just check for any `-march` and `-mcpu`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70919



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

[PATCH] D70535: [clangd] Define out-of-line qualify return value

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:153
+  // Get new begin and end positions for the qualified body.
+  auto OrigFuncRange = toHalfOpenFileRange(
+  SM, FD->getASTContext().getLangOpts(), FD->getSourceRange());

kadircet wrote:
> hokein wrote:
> > we have similar code in define-inline as well, would be nice to have them 
> > in a single place in the long term. probably a FIXME?
> they're quite similar but, different in nature. one of them returns the full 
> function definition, including template parameter lists, whereas the other 
> only operates on function body.
yes, the only difference is the range, right? the logic of applying 
replacements, getting correct begin/end offset, and getting the interesting 
content is the same. we could abstract a function that taking the range into 
the parameter.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1810
+})cpp",
+   "a::Foo::Bar foo() { return {}; }\n"},
+  {R"cpp(

kadircet wrote:
> hokein wrote:
> > oh, this is very tricky case (I think you meant to test the public 
> > members), note that Bar and foo are private member of Foo, we can't move 
> > the body out of the class `Foo`, for this case I think we should disallow 
> > the tweak.
> > 
> > No need to do it in this patch, but please update the test here (to test 
> > public members).
> I don't follow, the following compiles nicely:
> ```
> namespace a {
> class Foo {
>   class Bar {};
>   Bar foo();
> };
> }  // namespace a
> 
> a::Foo::Bar a::Foo::foo() { return {}; }
> ```
> 
> the problem here is we are not qualifying the function name, which is handled 
> in the follow up patch D70656
ah, you are right. I think I was somehow confused with the `a::Foo::Bar foo()`.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:2007
+Bar foo() ;
+  }
+})cpp",

nit: ";"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70535



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

@dblaikie  , If you try switching to C compiler. then the DW_TAG_variable is 
their. for C++ . It's not generated.  Please check godbolt again. -- though 
strange


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 231758.
mibintc added a comment.

This commit was reverted in 30e7ee3c4bac 
 because a 
null deref error occurred in IRBuilder.h when setting strictfp attribute, see 
https://bugs.llvm.org/show_bug.cgi?id=44048 for information about that bug.
This patch moves setting strictfp from IRBuilder into clang/CodeGen. Also 
addresses some code review comments that were received after the revert.  I'll 
add some inline comments next.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/unittests/IR/IRBuilderTest.cpp

Index: llvm/unittests/IR/IRBuilderTest.cpp
===
--- llvm/unittests/IR/IRBuilderTest.cpp
+++ llvm/unittests/IR/IRBuilderTest.cpp
@@ -183,6 +183,8 @@
   // See if we get constrained intrinsics instead of non-constrained
   // instructions.
   Builder.setIsFPConstrained(true);
+  auto Parent = BB->getParent();
+  Parent->addFnAttr(Attribute::StrictFP);
 
   V = Builder.CreateFAdd(V, V);
   ASSERT_TRUE(isa(V));
@@ -233,7 +235,8 @@
   AttributeSet CallAttrs = II->getAttributes().getFnAttributes();
   EXPECT_EQ(CallAttrs.hasAttribute(Attribute::StrictFP), true);
 
-  // Verify attributes on the containing function are created automatically.
+  // Verify attributes on the containing function are created when requested.
+  Builder.setConstrainedFPFunctionAttr();
   AttributeList Attrs = BB->getParent()->getAttributes();
   AttributeSet FnAttrs = Attrs.getFnAttributes();
   EXPECT_EQ(FnAttrs.hasAttribute(Attribute::StrictFP), true);
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -265,7 +265,6 @@
   void setConstrainedFPCallAttr(CallInst *I) {
 if (!I->hasFnAttr(Attribute::StrictFP))
   I->addAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
-setConstrainedFPFunctionAttr();
   }
 
   //======//
Index: clang/test/Driver/fp-model.c
===
--- /dev/null
+++ clang/test/Driver/fp-model.c
@@ -0,0 +1,137 @@
+// Test that incompatible combinations of -ffp-model= options
+// and other floating point options get a warning diagnostic.
+//
+// REQUIRES: clang-driver
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN %s
+// WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN1 %s
+// WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN2 %s
+// WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN3 %s
+// WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
+
+// RUN: %clang -### 

[PATCH] D70779: AArch64: add support for newer Apple CPUs

[Ahmed Bougacha via Phabricator via cfe-commits]

ab added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64.td:587
+"Apple A10", [
+ProcAppleA7,
+FeatureCRC,

I'm not sure we want to reuse the features:
- everything will get stuck with FeatureZCZeroingFPWorkaround, right? (but 
maybe we can remove features in this list?  I don't think so)
- it probably becomes harder to tune later chips, but that's admittedly a 
theoretical problem at this point
- some of the features can be generation-specific


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

https://reviews.llvm.org/D70779



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

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

[Sid Manning via Phabricator via cfe-commits]

sidneym created this revision.
sidneym added reviewers: kparzysz, bcain, ruiu.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
sidneym added a subscriber: llvm-commits.

Avoid passing unsupported options to lld when -fuse-ld=lld is used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70919

Files:
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/test/Driver/hexagon-toolchain-elf.c


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -536,3 +536,25 @@
 // RUN:   | FileCheck -check-prefix=CHECK080 %s
 // CHECK080:  "-cc1"
 // CHECK080:  "-Wreturn-type"
+
+// 
-
+// Default, not passing -fuse-ld
+// 
-
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK081 %s
+// CHECK081:  "-march=hexagon"
+// CHECK081:  "-mcpu=hexagonv60"
+// 
-
+// Passing -fuse-ld=lld
+// 
-
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK082 %s
+// CHECK082-NOT:  "-march=hexagon"
+// CHECK082-NOT:  "-mcpu=hexagon"
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -209,7 +209,9 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  bool UseLLD = 
Args.getLastArgValue(options::OPT_fuse_ld_EQ).startswith("lld");
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   
//
   // Silence warnings for various options
@@ -232,9 +234,10 @@
   for (const auto  : HTC.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());
 
-  CmdArgs.push_back("-march=hexagon");
-  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
-  CmdArgs.push_back(Args.MakeArgString("-mcpu=hexagon" + CpuVer));
+  if (!UseLLD) {
+CmdArgs.push_back("-march=hexagon");
+CmdArgs.push_back(Args.MakeArgString("-mcpu=hexagon" + CpuVer));
+  }
 
   if (IsShared) {
 CmdArgs.push_back("-shared");


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -536,3 +536,25 @@
 // RUN:   | FileCheck -check-prefix=CHECK080 %s
 // CHECK080:  "-cc1"
 // CHECK080:  "-Wreturn-type"
+
+// -
+// Default, not passing -fuse-ld
+// -
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK081 %s
+// CHECK081:  "-march=hexagon"
+// CHECK081:  "-mcpu=hexagonv60"
+// -
+// Passing -fuse-ld=lld
+// -
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK082 %s
+// CHECK082-NOT:  "-march=hexagon"
+// CHECK082-NOT:  "-mcpu=hexagon"
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -209,7 +209,9 @@
   bool IncStartFiles = !Args.hasArg(options::OPT_nostartfiles);
   bool IncDefLibs = !Args.hasArg(options::OPT_nodefaultlibs);
   bool UseG0 = false;
+  bool UseLLD = Args.getLastArgValue(options::OPT_fuse_ld_EQ).startswith("lld");
   bool UseShared = IsShared && !IsStatic;
+  StringRef CpuVer = toolchains::HexagonToolChain::GetTargetCPUVersion(Args);
 
   //
   // Silence warnings for various options
@@ 

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70696#1765789 , @dblaikie wrote:

> In D70696#1765784 , @probinson wrote:
>
> > In D70696#1765671 , @dblaikie 
> > wrote:
> >
> > > In D70696#1765637 , @probinson 
> > > wrote:
> > >
> > > > For the case:
> > > >
> > > >   cat ref.c
> > > >   extern int global_var;
> > > >   int main() { return global_var; }
> > > >
> > > >
> > > > I *do* expect to see debug info for the declaration of global_var.
> > >
> > >
> > > FWIW I'd only expect it there with -fstandalone-debug - with 
> > > -fno-standalone-debug I'd expect this code to rely on the assumption that 
> > > def.c is also compiled with debug info.
> >
> >
> > When global_var is defined in a separate .so, you might not have the symbol 
> > at all.  It's helpful to be able to report "that symbol is defined outside 
> > this executable" (and IIRC, gdb can look it up by mangled name and actually 
> > show it to you anyway).  Without even the declaration, you get "that symbol 
> > that you can see right there in your source? It doesn't exist, haha."  So, 
> > I would definitely rather see a declaration for a referenced global.
> >  Even if we currently don't do that.
>
>
> Same would be true of all functions as well though, right? Neither LLVM nor 
> GCC emit declarations for all called functions (& I expect that'd produce 
> significant size growth - I'm worried enough about global variable 
> declarations that are referenced by unused inline functions or other similar 
> things, and pull in complex type hierarchies, etc - let alone all functions 
> called too).


(but in general, that's what -fstandalone-debug is for: If you want this file's 
debug info to be complete (for some value of complete) even if other parts of 
the program (shared libraries, static libraries, however they're done) are 
compiled without debug info - though I still suspect emitting debug info for 
all variables and functions called from this file would be a pretty big size 
cost)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70696#1765784 , @probinson wrote:

> In D70696#1765671 , @dblaikie wrote:
>
> > In D70696#1765637 , @probinson 
> > wrote:
> >
> > > For the case:
> > >
> > >   cat ref.c
> > >   extern int global_var;
> > >   int main() { return global_var; }
> > >
> > >
> > > I *do* expect to see debug info for the declaration of global_var.
> >
> >
> > FWIW I'd only expect it there with -fstandalone-debug - with 
> > -fno-standalone-debug I'd expect this code to rely on the assumption that 
> > def.c is also compiled with debug info.
>
>
> When global_var is defined in a separate .so, you might not have the symbol 
> at all.  It's helpful to be able to report "that symbol is defined outside 
> this executable" (and IIRC, gdb can look it up by mangled name and actually 
> show it to you anyway).  Without even the declaration, you get "that symbol 
> that you can see right there in your source? It doesn't exist, haha."  So, I 
> would definitely rather see a declaration for a referenced global.
>  Even if we currently don't do that.


Same would be true of all functions as well though, right? Neither LLVM nor GCC 
emit declarations for all called functions (& I expect that'd produce 
significant size growth - I'm worried enough about global variable declarations 
that are referenced by unused inline functions or other similar things, and 
pull in complex type hierarchies, etc - let alone all functions called too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D70696#1765671 , @dblaikie wrote:

> In D70696#1765637 , @probinson wrote:
>
> > For the case:
> >
> >   cat ref.c
> >   extern int global_var;
> >   int main() { return global_var; }
> >
> >
> > I *do* expect to see debug info for the declaration of global_var.
>
>
> FWIW I'd only expect it there with -fstandalone-debug - with 
> -fno-standalone-debug I'd expect this code to rely on the assumption that 
> def.c is also compiled with debug info.


When global_var is defined in a separate .so, you might not have the symbol at 
all.  It's helpful to be able to report "that symbol is defined outside this 
executable" (and IIRC, gdb can look it up by mangled name and actually show it 
to you anyway).  Without even the declaration, you get "that symbol that you 
can see right there in your source? It doesn't exist, haha."  So, I would 
definitely rather see a declaration for a referenced global.
Even if we currently don't do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70696#1765675 , @yonghong-song 
wrote:

> @probinson for the question,
>
> > Does bpf require debug info for the declaration of global_var in noref.c ?
>
> No, bpf only cares the referenced external global variables. So my current 
> implementation does not emit debug info
>  for external global_var in noref.c.
>
> It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions 
> as well) emits the debuginfo (encoded in dwarf)
>  even if the external variable is not used in the current compilation unit. 
> Not sure what is the rationale behind it.


FWIW, looks like GCC trunk doesn't do this anymore: 
https://godbolt.org/z/EP5mWF (try it with GCC trunk V GCC 9.2 - 9.2 mentions 
"glbl_var" and trunk does not).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D69223: WDocumentation: Implement the \anchor.

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/test/Index/comment-xml-schema.c:38
+// RUN: xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng 
%S/Inputs/CommentXML/valid-inline-command-01.xml
 
 // RUN: not xmllint --noout --relaxng 
%S/../../bindings/xml/comment-xml-schema.rng 
%S/Inputs/CommentXML/invalid-function-01.xml 2>&1 | FileCheck %s 
-check-prefix=CHECK-INVALID

Mordante wrote:
> Is this what you meant? Or did you mean something different?
Yes, this line. Sorry, I missed it during review. Thanks for pointing out!


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

https://reviews.llvm.org/D69223



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70696#1765714 , @yonghong-song 
wrote:

> @dblaikie Good points. I will guard external variable debug info generation 
> under `-fstandalone-debug` flag.


Oh, I figured given your needs this'd be guarded behind the target being BPF? 
While I don't mind it being also enabled by -fstandalone-debug (or, if you 
like, I guess maybe BPF is a "standalone-debug by default" target (like 
MacOS/Darwin, I think - due to some LLDB limitations at the moment) & maybe 
that captures all the BPF quirks in this regard?) - though I wouldn't leap to 
it unless someone else is already interested in that feature in 
-fstandalone-debug. (I'd be a bit worried about debug info growth and how "is 
this global variable referenced" is computed - eg: it could be referenced from 
a dead (uncalled) inline function)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

In D70696#1765675 , @yonghong-song 
wrote:

> @probinson for the question,
>
> > Does bpf require debug info for the declaration of global_var in noref.c ?
>
> No, bpf only cares the referenced external global variables. So my current 
> implementation does not emit debug info
>  for external global_var in noref.c.
>
> It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions 
> as well) emits the debuginfo (encoded in dwarf)
>  even if the external variable is not used in the current compilation unit. 
> Not sure what is the rationale behind it.


I'm using gcc 9.2.0 -- it's generating, for `noref.c`{Not used in compilation 
unit} case also.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D70696#1765675 , @yonghong-song 
wrote:

> It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions 
> as well) emits the debuginfo (encoded in dwarf)
>  even if the external variable is not used in the current compilation unit. 
> Not sure what is the rationale behind it.


If it were me, it would be because the declaration is available in the current 
CU, therefore if the debugger is stopped somewhere in the CU, the symbol should 
be available to the debugger.  Having the external declaration means you don't 
need to go rooting around in other CUs for the symbol.

This is just one of those trade-offs that producers and consumers make, 
regarding completeness of information versus bloat of unnecessary information.  
There is no "right" answer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70780: [WebAssembly] Find wasm-opt with GetProgramPath

[sunfishcode via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f1e2151b8e9: [WebAssembly] Find wasm-opt with 
GetProgramPath (authored by sunfishcode).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70780

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -92,10 +92,10 @@
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
 
-  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-if (llvm::ErrorOr WasmOptPath =
-   llvm::sys::findProgramByName("wasm-opt")) {
+auto WasmOptPath = getToolChain().GetProgramPath("wasm-opt");
+if (WasmOptPath != "wasm-opt") {
   StringRef OOpt = "s";
   if (A->getOption().matches(options::OPT_O4) ||
   A->getOption().matches(options::OPT_Ofast))
@@ -106,7 +106,7 @@
 OOpt = A->getValue();
 
   if (OOpt != "0") {
-const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+const char *WasmOpt = Args.MakeArgString(WasmOptPath);
 ArgStringList CmdArgs;
 CmdArgs.push_back(Output.getFilename());
 CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -92,10 +92,10 @@
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
 
-  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-if (llvm::ErrorOr WasmOptPath =
-   llvm::sys::findProgramByName("wasm-opt")) {
+auto WasmOptPath = getToolChain().GetProgramPath("wasm-opt");
+if (WasmOptPath != "wasm-opt") {
   StringRef OOpt = "s";
   if (A->getOption().matches(options::OPT_O4) ||
   A->getOption().matches(options::OPT_Ofast))
@@ -106,7 +106,7 @@
 OOpt = A->getValue();
 
   if (OOpt != "0") {
-const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+const char *WasmOpt = Args.MakeArgString(WasmOptPath);
 ArgStringList CmdArgs;
 CmdArgs.push_back(Output.getFilename());
 CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8f1e215 - [WebAssembly] Find wasm-opt with GetProgramPath

[Dan Gohman via cfe-commits]

Author: Dan Gohman
Date: 2019-12-02T11:48:36-08:00
New Revision: 8f1e2151b8e923345a18aa3025a7d074e134768b

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

LOG: [WebAssembly] Find wasm-opt with GetProgramPath

Instead of just searching for wasm-opt in PATH, use GetProgramPath, which
checks the `COMPILER_PATH` environment variable, -B paths, and `PATH`.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/WebAssembly.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 55b82592c09f..1bb7c35d0c52 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -92,10 +92,10 @@ void wasm::Linker::ConstructJob(Compilation , const 
JobAction ,
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
 
-  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-if (llvm::ErrorOr WasmOptPath =
-   llvm::sys::findProgramByName("wasm-opt")) {
+auto WasmOptPath = getToolChain().GetProgramPath("wasm-opt");
+if (WasmOptPath != "wasm-opt") {
   StringRef OOpt = "s";
   if (A->getOption().matches(options::OPT_O4) ||
   A->getOption().matches(options::OPT_Ofast))
@@ -106,7 +106,7 @@ void wasm::Linker::ConstructJob(Compilation , const 
JobAction ,
 OOpt = A->getValue();
 
   if (OOpt != "0") {
-const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+const char *WasmOpt = Args.MakeArgString(WasmOptPath);
 ArgStringList CmdArgs;
 CmdArgs.push_back(Output.getFilename());
 CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@dblaikie Good points. I will guard external variable debug info generation 
under `-fstandalone-debug` flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D69223: WDocumentation: Implement the \anchor.

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 6 inline comments as done.
Mordante added inline comments.



Comment at: clang/lib/Index/CommentToXML.cpp:650
+assert(C->getNumArgs() == 1);
+Result << "";
+appendToResultWithXMLEscaping(Arg0);

gribozavr2 wrote:
> Sholudn't this code be producing ""?
Good catch, will update it in the next patch.



Comment at: clang/test/Index/Inputs/CommentXML/valid-inline-command-01.xml:1
+
+

gribozavr2 wrote:
> Please add a line to `clang/test/Index/comment-xml-schema.c` that executes 
> this test.
I already added a line.



Comment at: clang/test/Index/comment-xml-schema.c:38
+// RUN: xmllint --noout --relaxng %S/../../bindings/xml/comment-xml-schema.rng 
%S/Inputs/CommentXML/valid-inline-command-01.xml
 
 // RUN: not xmllint --noout --relaxng 
%S/../../bindings/xml/comment-xml-schema.rng 
%S/Inputs/CommentXML/invalid-function-01.xml 2>&1 | FileCheck %s 
-check-prefix=CHECK-INVALID

Is this what you meant? Or did you mean something different?


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

https://reviews.llvm.org/D69223



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

[clang] 1d45873 - [AArch64] Attempt to fixup test line. NFC

[David Green via cfe-commits]

Author: David Green
Date: 2019-12-02T19:30:54Z
New Revision: 1d4587346f51ca5cc5741337cadfaeb208ca59ad

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

LOG: [AArch64] Attempt to fixup test line. NFC

The test is complaining on some of the builders. This attempts to
adjust the run line to be more line the others in the same folder, using
clang_cc1 as opposed to the driver.

Added: 


Modified: 
clang/test/CodeGen/aarch64-neon-vcadd.c

Removed: 




diff  --git a/clang/test/CodeGen/aarch64-neon-vcadd.c 
b/clang/test/CodeGen/aarch64-neon-vcadd.c
index 6f1b3dcd4015..2d721f187fe6 100644
--- a/clang/test/CodeGen/aarch64-neon-vcadd.c
+++ b/clang/test/CodeGen/aarch64-neon-vcadd.c
@@ -1,4 +1,6 @@
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.3-a+fp16 %s -S 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon \
+// RUN:  -target-feature +v8.3a -target-feature +fullfp16 -S -emit-llvm -o - 
%s \
+// RUN:  | FileCheck %s
 
 #include 
 



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

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi @arthurp, I can review the libclang part of the patch.

Could you please remove the changes that are just code formatting? You can land 
those as a separate NFC commit.




Comment at: clang/tools/libclang/CIndex.cpp:259
+
+  std::pair Begin = SM.getDecomposedLoc(
+  SM.getFileLoc(RegionOfInterest.getBegin())),

This seems like just a clang-format change. Maybe we could separate these as a 
NFC commit?



Comment at: clang/tools/libclang/CIndex.cpp:436
 
-  bool OnlyLocalDecls
-= !AU->isMainFileAST() && AU->getOnlyLocalDecls(); 
-  
+  bool OnlyLocalDecls = !AU->isMainFileAST() && AU->getOnlyLocalDecls();
+

This seems like just a clang-format change. Maybe we could separate these as a 
NFC commit?



Comment at: clang/tools/libclang/CIndex.cpp:1369
 
-bool 
-CursorVisitor::VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc Qualifier) {
+bool CursorVisitor::VisitNestedNameSpecifierLoc(
+NestedNameSpecifierLoc Qualifier) {

This seems like just a clang-format change. Maybe we could separate these as a 
NFC commit?

Could you please leave out all such changes from this patch? It would be easier 
to review. (It seems to me a bunch of changes below are of this nature.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10833



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor updated this revision to Diff 231742.
ptaylor added a comment.

drop comment about vscode cuda syntax highlighting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,14 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+{ scheme: 'file', language: 'cuda' },
 { scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
-{ scheme: 'file', pattern: cudaFilePattern },
+{ scheme: 'file', language: 'objective-cpp'}
 ],
 synchronize: !syncFileEvents ? undefined : {
 // FIXME: send sync file events when clangd provides implemenatations.
@@ -111,10 +104,10 @@
 serverOptions, clientOptions);
   if (getConfig('semanticHighlighting')) {
 const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-context);
+new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+ context);
 context.subscriptions.push(
-  vscode.Disposable.from(semanticHighlightingFeature));
+vscode.Disposable.from(semanticHighlightingFeature));
 clangdClient.registerFeature(semanticHighlightingFeature);
   }
   console.log('Clang Language Server is now active!');
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -23,6 +23,7 @@
 "activationEvents": [
 "onLanguage:c",
 "onLanguage:cpp",
+"onLanguage:cuda",
 "onLanguage:objective-c",
 "onLanguage:objective-cpp",
 "onCommand:clangd-vscode.activate"
@@ -64,6 +65,13 @@
 "**/MSVC/*/include/**"
 ],
 "firstLine": "^/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+},
+{
+"id": "cuda",
+"extensions": [
+".cu",
+".cuh"
+]
 }
 ],
 "configuration": {


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,14 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+{ scheme: 'file', language: 'cuda' },
 { scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
-{ scheme: 'file', pattern: cudaFilePattern },
+{ scheme: 'file', language: 'objective-cpp'}
 ],
 synchronize: 

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a subscriber: andreadb.
spatel added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:580
+  /// This checks for presence of the -Ofast, -ffast-math or -funsafe-math 
flags.
+  virtual bool FastMathRuntimeIsAvailable(
+const llvm::opt::ArgList , std::string ) const;

Formatting nit - prefer to start with verb and lower-case: 
isFastMathRuntimeAvailable() or hasFastMathRuntime().



Comment at: clang/include/clang/Driver/ToolChain.h:587
   /// This checks for presence of the -Ofast, -ffast-math or -funsafe-math 
flags.
-  virtual bool AddFastMathRuntimeIfAvailable(
-  const llvm::opt::ArgList , llvm::opt::ArgStringList ) const;
+  bool AddFastMathRuntimeIfAvailable(
+const llvm::opt::ArgList , llvm::opt::ArgStringList ) const;

Add -> add



Comment at: clang/lib/Driver/ToolChains/PS4CPU.h:95-96
+const llvm::fltSemantics *FPType) const override {
+// DAZ and FTZ are on by default.
+return llvm::DenormalMode::getPreserveSign();
+  }

@probinson / @andreadb - is this correct for PS4? or is there some equivalent 
to the Linux startup file?


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

https://reviews.llvm.org/D69979



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@probinson for the question,

> Does bpf require debug info for the declaration of global_var in noref.c ?

No, bpf only cares the referenced external global variables. So my current 
implementation does not emit debug info
for external global_var in noref.c.

It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions 
as well) emits the debuginfo (encoded in dwarf)
even if the external variable is not used in the current compilation unit. Not 
sure what is the rationale behind it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70696#1765637 , @probinson wrote:

> For the case:
>
>   cat def.c
>   int global_var = 2;
>
>
> def.o should have debug info for the definition of global_var.
>  For the case:
>
>   cat noref.c
>   extern int global_var;
>   int main() {}
>
>
> I would not expect to see debug info for the declaration of global_var.
>  For the case:
>
>   cat ref.c
>   extern int global_var;
>   int main() { return global_var; }
>
>
> I *do* expect to see debug info for the declaration of global_var.


FWIW I'd only expect it there with -fstandalone-debug - with 
-fno-standalone-debug I'd expect this code to rely on the assumption that def.c 
is also compiled with debug info.

(as it stands today, Clang/LLVM never produces debug info for global_var in 
ref.c, even with -fstandalone-debug & I'm not too fussed about that, but would 
be OK if someone wanted to fix/improve that)

> Does bpf require debug info for the declaration of global_var in `noref.c` ?

Yeah, +1, I'm still curious to know more/trying to understand this ^ 
requirement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70863: [clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@sammccall Sam, it looks like the tests are failing on the darwin bots:

http://lab.llvm.org:8080/green/job/clang-stage1-RA/4243/consoleFull

  Value of: CDB.getFallbackCommand(testPath("bar.cc")).CommandLine
  Expected: has 6 elements where
  element #0 ends with "clang",
  element #1 is equal to "-DA=2",
  element #2 is equal to "/clangd-test/bar.cc",
  element #3 is equal to "-DA=4",
  element #4 is equal to "-fsyntax-only",
  element #5 starts with "-resource-dir"
Actual: { 
"/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang",
 "-DA=2", "/clangd-test/bar.cc", "-DA=4", "-fsyntax-only", 
"-resource-dir=/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/tools/extra/clangd/lib/clang/10.0.0",
 "-isysroot", 
"/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk"
 }, which has 8 elements

Can you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70863



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song marked an inline comment as done.
yonghong-song added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1140
 
+  for (auto D: ExternalDeclarations) {
+if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed())

aprantl wrote:
> clang-format
Thanks for the review! Will update the patch today after running clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Let me try to restate what's happening here so that I can see if I understand 
it.

There are two concepts of "section" in play here:

- a unit in an object file with a particular section name, which need not be 
unique within the object file, and which can have interesting per-unit 
attributes like mergeability; and
- the high-level layout of the final linked image, where all of the units with 
the same section name (within a single image) are supposed to be contiguous, 
and where the image will contain metadata describing the location and size of 
the section.

I'll call the first a "section unit" and the second an "image section" just for 
clarity; my apologies if there's more standard jargon.

Marking a section unit as mergeable in ELF opts in to a link-time optimization 
where the linker can avoid unnecessary duplication in the image section by 
combining identical data that would have ended up in it.  Essentially, the 
section unit is broken up into entries according to an entry size that's part 
of the mergeable attribute, and if the linker sees that two entries will be 
identical (whether they come from different section units or not), it can 
simply remove the redundant entry from the final image section.  (Presumably 
there's some rule about the order of entries, but it doesn't really matter for 
this analysis.)  This is done as a relatively late pass; any sort of mandatory 
"merging" from e.g. COMDAT, weak, and common symbols will already have been 
applied, so we don't need to worry about this interfering with 
language-mandated symbol coalescing.

When LLVM is emitting ELF, it will try to place an object in a mergeable 
section unit if the object is `unnamed_addr`.  It will also generally emit 
objects into the same section unit if they share the same section name.  I 
assume this takes attributes into account to at least some degree, or else we 
might be lumping non-`unnamed_addr` into a mergeable section just because the 
first object we processed with that section name was `unnamed_addr`.  But it 
must not take entry size into account, because PR 43457 shows us clearly 
emitting a single section unit for objects of different sizes.

Given all that, this patch seems far too aggressive.  While mergeable sections 
can be useful for optimizing arbitrary code that might not use a section, they 
are also extremely useful for optimizing the sorts of global tables that 
programmers frequently use explicit sections for.  It seems to me that the 
right fix is to find the place that ensures that we don't put mergeable and 
non-mergeable objects in the same section unit (or at least conservatively 
makes the section unit non-mergeable) and fix it to consider entry size as 
well.  That should be straightforward unless that place doesn't exist, in which 
case we have very serious problems.


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

https://reviews.llvm.org/D68101



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

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2925
+return C.getQualifiedType(T.getTypePtr(), Quals);
+  }
 case Type::DeducedTemplateSpecialization: {

aprantl wrote:
> You need to mark this `LLVM_FALLTHROUGH` now or you'll get a warning.
The FALLTHROUGH must come right before the next `case`



Comment at: clang/test/CodeGenCXX/debug-info-auto-return.cpp:7
+
+// CHECK: !DISubprogram(name: "findMax",{{.*}}, type: !18
+// CHECK: !18 =  !DISubroutineType(types: !19)

aprantl wrote:
> Please don't hardcode the MDNode numbers, they will inevitably change over 
> time. Instead use variables:
> ```
> type: ![[SUBROUTINE_TYPE:[0-9]+]]`
> // CHECK: ![[SUBROUTINE_TYPE]] =  !DISubroutineType(types: ![[ARGS:[0-9]+)
> ```
> etc
you still need to make sure that the two nodes CHECKed are being connected. 
Please use variables like in the example I posted.


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

https://reviews.llvm.org/D70524



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

[clang] f17a1d8 - [OPENMP]Use cast instead dyn_cast, NFC.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-12-02T14:16:52-05:00
New Revision: f17a1d8b283d227dcbc88caf94acf55abc91c1f9

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

LOG: [OPENMP]Use cast instead dyn_cast, NFC.

Here the expression is always a DeclRefExpr, no need to use dyn_cast.

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 2773efcf3dae..50d9ab974dd4 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -5445,7 +5445,7 @@ void 
Sema::markOpenMPDeclareVariantFuncsReferenced(SourceLocation Loc,
  Func->specific_attrs()) {
   // TODO: add checks for active OpenMP context where possible.
   Expr *VariantRef = A->getVariantFuncRef();
-  auto *DRE = dyn_cast(VariantRef->IgnoreParenImpCasts());
+  auto *DRE = cast(VariantRef->IgnoreParenImpCasts());
   auto *F = cast(DRE->getDecl());
   if (!F->isDefined() && F->isTemplateInstantiation())
 InstantiateFunctionDefinition(Loc, F->getFirstDecl());



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

[clang] 478541a - [OPENMP]Fix PR44133: Emit definitions of used constructors/functions.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-12-02T14:07:29-05:00
New Revision: 478541a6da59fa3eadab98cabdcb0126fad3fdb5

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

LOG: [OPENMP]Fix PR44133: Emit definitions of used constructors/functions.

Need to fully rebuild the initializer/combiner when instatiating the
declare reduction constrcut to properly emit used functions.

Added: 


Modified: 
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/OpenMP/declare_reduction_codegen.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 9a6c7b5277b5..e9cb9f89e0a2 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3070,20 +3070,11 @@ Decl 
*TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
   } else {
 SubstReductionType = D->getType();
   }
-  Expr *Combiner = D->getCombiner();
-  Expr *Init = D->getInitializer();
-  const bool CombinerRequiresInstantiation =
-  Combiner &&
-  (Combiner->isValueDependent() || Combiner->isInstantiationDependent() ||
-   Combiner->isTypeDependent() ||
-   Combiner->containsUnexpandedParameterPack());
-  const bool InitRequiresInstantiation =
-  Init &&
-  (Init->isValueDependent() || Init->isInstantiationDependent() ||
-   Init->isTypeDependent() || Init->containsUnexpandedParameterPack());
   if (SubstReductionType.isNull())
 return nullptr;
-  bool IsCorrect = !SubstReductionType.isNull();
+  Expr *Combiner = D->getCombiner();
+  Expr *Init = D->getInitializer();
+  bool IsCorrect = true;
   // Create instantiated copy.
   std::pair ReductionTypes[] = {
   std::make_pair(SubstReductionType, D->getLocation())};
@@ -3098,79 +3089,53 @@ Decl 
*TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
   PrevDeclInScope);
   auto *NewDRD = cast(DRD.get().getSingleDecl());
   SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewDRD);
-  if (!RequiresInstantiation && !CombinerRequiresInstantiation &&
-  !InitRequiresInstantiation) {
-if (Combiner) {
-  NewDRD->setCombinerData(D->getCombinerIn(), D->getCombinerOut());
-  NewDRD->setCombiner(Combiner);
-  if (Init) {
-NewDRD->setInitializerData(D->getInitOrig(), D->getInitPriv());
-NewDRD->setInitializer(Init, D->getInitializerKind());
-  }
-}
-(void)SemaRef.ActOnOpenMPDeclareReductionDirectiveEnd(
-/*S=*/nullptr, DRD, IsCorrect && !D->isInvalidDecl());
-return NewDRD;
-  }
   Expr *SubstCombiner = nullptr;
   Expr *SubstInitializer = nullptr;
   // Combiners instantiation sequence.
   if (Combiner) {
-if (!CombinerRequiresInstantiation) {
-  NewDRD->setCombinerData(D->getCombinerIn(), D->getCombinerOut());
-  NewDRD->setCombiner(Combiner);
-} else {
-  SemaRef.ActOnOpenMPDeclareReductionCombinerStart(
-  /*S=*/nullptr, NewDRD);
-  SemaRef.CurrentInstantiationScope->InstantiatedLocal(
-  cast(D->getCombinerIn())->getDecl(),
-  cast(NewDRD->getCombinerIn())->getDecl());
-  SemaRef.CurrentInstantiationScope->InstantiatedLocal(
-  cast(D->getCombinerOut())->getDecl(),
-  cast(NewDRD->getCombinerOut())->getDecl());
-  auto *ThisContext = dyn_cast_or_null(Owner);
-  Sema::CXXThisScopeRAII ThisScope(SemaRef, ThisContext, Qualifiers(),
-   ThisContext);
-  SubstCombiner = SemaRef.SubstExpr(Combiner, TemplateArgs).get();
-  SemaRef.ActOnOpenMPDeclareReductionCombinerEnd(NewDRD, SubstCombiner);
-}
+SemaRef.ActOnOpenMPDeclareReductionCombinerStart(
+/*S=*/nullptr, NewDRD);
+SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+cast(D->getCombinerIn())->getDecl(),
+cast(NewDRD->getCombinerIn())->getDecl());
+SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+cast(D->getCombinerOut())->getDecl(),
+cast(NewDRD->getCombinerOut())->getDecl());
+auto *ThisContext = dyn_cast_or_null(Owner);
+Sema::CXXThisScopeRAII ThisScope(SemaRef, ThisContext, Qualifiers(),
+ ThisContext);
+SubstCombiner = SemaRef.SubstExpr(Combiner, TemplateArgs).get();
+SemaRef.ActOnOpenMPDeclareReductionCombinerEnd(NewDRD, SubstCombiner);
   }
   // Initializers instantiation sequence.
   if (Init) {
-if (!InitRequiresInstantiation) {
-  NewDRD->setInitializerData(D->getInitOrig(), D->getInitPriv());
-  NewDRD->setInitializer(Init, D->getInitializerKind());
+VarDecl *OmpPrivParm = SemaRef.ActOnOpenMPDeclareReductionInitializerStart(
+/*S=*/nullptr, NewDRD);
+SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

For the case:

  cat def.c
  int global_var = 2;

def.o should have debug info for the definition of global_var.
For the case:

  cat noref.c
  extern int global_var;
  int main() {}

I would not expect to see debug info for the declaration of global_var.
For the case:

  cat ref.c
  extern int global_var;
  int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

Does bpf require debug info for the declaration of global_var in `noref.c` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[clang] 9ec6d71 - [clang][modules] Add support for merging lifetime-extended temporaries

[via cfe-commits]

Author: Tyker
Date: 2019-12-02T19:55:13+01:00
New Revision: 9ec6d7121132d30db68818e4f684910f76307fdf

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

LOG: [clang][modules] Add support for merging lifetime-extended temporaries

Summary: Add support for merging lifetime-extended temporaries

Reviewers: rsmith

Reviewed By: rsmith

Subscribers: xbolva00, cfe-commits

Tags: #clang

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

Added: 
clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h
clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h
clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h
clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap
clang/test/Modules/merge-lifetime-extended-temporary.cpp

Modified: 
clang/include/clang/AST/DeclCXX.h
clang/include/clang/AST/TextNodeDumper.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/AST/TextNodeDumper.cpp
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 63d67bd3f55b..0f2018fb9e8c 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -3041,7 +3041,9 @@ class NamespaceAliasDecl : public NamedDecl,
 
 /// Implicit declaration of a temporary that was materialized by
 /// a MaterializeTemporaryExpr and lifetime-extended by a declaration
-class LifetimeExtendedTemporaryDecl final : public Decl {
+class LifetimeExtendedTemporaryDecl final
+: public Decl,
+  public Mergeable {
   friend class MaterializeTemporaryExpr;
   friend class ASTDeclReader;
 

diff  --git a/clang/include/clang/AST/TextNodeDumper.h 
b/clang/include/clang/AST/TextNodeDumper.h
index 0ff5a614a864..d293ea190aa4 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -346,6 +346,8 @@ class TextNodeDumper
   void VisitObjCPropertyImplDecl(const ObjCPropertyImplDecl *D);
   void VisitBlockDecl(const BlockDecl *D);
   void VisitConceptDecl(const ConceptDecl *D);
+  void
+  VisitLifetimeExtendedTemporaryDecl(const LifetimeExtendedTemporaryDecl *D);
 };
 
 } // namespace clang

diff  --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index f0b5e9933823..b6dae68b3413 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -551,6 +551,14 @@ class ASTReader
   llvm::DenseMap>
 AnonymousDeclarationsForMerging;
 
+  /// Key used to identify LifetimeExtendedTemporaryDecl for merging,
+  /// containing the lifetime-extending declaration and the mangling number.
+  using LETemporaryKey = std::pair;
+
+  /// Map of already deserialiazed temporaries.
+  llvm::DenseMap
+  LETemporaryForMerging;
+
   struct FileDeclsInfo {
 ModuleFile *Mod = nullptr;
 ArrayRef Decls;

diff  --git a/clang/lib/AST/TextNodeDumper.cpp 
b/clang/lib/AST/TextNodeDumper.cpp
index 0ff95213118f..561c76a45cbc 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1338,6 +1338,17 @@ void TextNodeDumper::VisitFunctionDecl(const 
FunctionDecl *D) {
 OS << " <>>";
 }
 
+void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl(
+const LifetimeExtendedTemporaryDecl *D) {
+  OS << " extended by ";
+  dumpBareDeclRef(D->getExtendingDecl());
+  OS << " mangling ";
+  {
+ColorScope Color(OS, ShowColors, ValueColor);
+OS << D->getManglingNumber();
+  }
+}
+
 void TextNodeDumper::VisitFieldDecl(const FieldDecl *D) {
   dumpName(D);
   dumpType(D->getType());

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 8991a39a7067..3f7a1ed7fd5c 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -424,6 +424,8 @@ namespace clang {
 template
 void mergeMergeable(Mergeable *D);
 
+void mergeMergeable(LifetimeExtendedTemporaryDecl *D);
+
 void mergeTemplatePattern(RedeclarableTemplateDecl *D,
   RedeclarableTemplateDecl *Existing,
   DeclID DsID, bool IsKeyDecl);
@@ -2358,6 +2360,7 @@ void ASTDeclReader::VisitLifetimeExtendedTemporaryDecl(
   if (Record.readInt())
 D->Value = new (D->getASTContext()) APValue(Record.readAPValue());
   D->ManglingNumber = Record.readInt();
+  mergeMergeable(D);
 }
 
 std::pair
@@ -2555,6 +2558,25 @@ static bool allowODRLikeMergeInC(NamedDecl *ND) {
   return false;
 }
 
+/// Attempts to merge LifetimeExtendedTemporaryDecl with
+/// identical class definitions from two 
diff erent modules.
+void 

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I guess first I'm confused about why the type would be undeduced in the first 
place, given that it is actually instantiated.
And if undeduced is correct, wouldn't we rather emit these with 
DW_TAG_unspecified_type?




Comment at: clang/test/CodeGenCXX/pr42710.cpp:2
+// RUN: %clang %s -DTYPE=auto -emit-llvm -S -g -o - -std=c++17
+// expected-no-diagnostics
+// RUN: %clang %s -DTYPE=int -emit-llvm -S -g -o - -std=c++17

I believe you would use `expected-no-diagnostics` only if you run clang with 
`-verify`.  So, please remove those two directives.


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

https://reviews.llvm.org/D70537



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

[PATCH] D70912: [Analyzer] Iterator Modeling: Print Container Data and Iterator Positions when printing the Program State

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

baloghadamsoftware updated this revision to Diff 231738.
baloghadamsoftware added a comment.

Wrong diff uploaded.


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

https://reviews.llvm.org/D70912

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/test/Analysis/iterator-modelling.cpp

Index: clang/test/Analysis/iterator-modelling.cpp
===
--- clang/test/Analysis/iterator-modelling.cpp
+++ clang/test/Analysis/iterator-modelling.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.DebugIteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.DebugIteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
 
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true %s 2>&1 | FileCheck %s
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 template 
@@ -1970,3 +1973,29 @@
 clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}} expected-warning@-1 0-1{{TRUE}} FIXME: should only expect FALSE in every case
   }
 }
+
+void clang_analyzer_printState();
+
+void print_state(std::vector ) {
+  const auto i0 = V.cbegin();
+  clang_analyzer_printState();
+
+// CHECK:  "checker_messages": [
+// CHECK-NEXT:   { "checker": "alpha.cplusplus.IteratorModeling", "messages": [
+// CHECK-NEXT: "Container Data :",
+// CHECK-NEXT: "SymRegion{reg_$[[#]] & V>} : [ conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]} ..  ]",
+// CHECK-NEXT: "Iterator Positions :",
+// CHECK-NEXT: "i0 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
+// CHECK-NEXT:   ]}
+
+  const auto i1 = V.cend();
+  clang_analyzer_printState();
+  
+// CHECK:  "checker_messages": [
+// CHECK-NEXT:   { "checker": "alpha.cplusplus.IteratorModeling", "messages": [
+// CHECK-NEXT: "Container Data :",
+// CHECK-NEXT: "SymRegion{reg_$[[#]] & V>} : [ conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]} .. conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]} ]",
+// CHECK-NEXT: "Iterator Positions :",
+// CHECK-NEXT: "i1 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
+// CHECK-NEXT:   ]}
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -121,6 +121,9 @@
   void handleEraseAfter(CheckerContext , const SVal ) const;
   void handleEraseAfter(CheckerContext , const SVal ,
 const SVal ) const;
+  void printState(raw_ostream , ProgramStateRef State, const char *NL,
+  const char *Sep) const override;
+
 public:
   IteratorModeling() {}
 
@@ -1080,6 +1083,58 @@
   C.addTransition(State);
 }
 
+void IteratorModeling::printState(raw_ostream , ProgramStateRef State,
+  const char *NL, const char *Sep) const {
+
+  auto ContMap = State->get();
+
+  if (!ContMap.isEmpty()) {
+Out << Sep << "Container Data :" << NL;
+for (const auto Cont : ContMap) {
+  Cont.first->dumpToStream(Out);
+  Out << " : [ ";
+  const auto CData = Cont.second;
+  if (CData.getBegin())
+CData.getBegin()->dumpToStream(Out);
+  else
+Out << "";
+  Out << " .. ";
+  if (CData.getEnd())
+CData.getEnd()->dumpToStream(Out);
+  else
+Out << "";
+  Out << " ]" << NL;
+}
+  }
+
+  auto SymbolMap = State->get();
+  auto RegionMap = State->get();
+
+  if (!SymbolMap.isEmpty() || !RegionMap.isEmpty()) {
+Out << Sep << "Iterator Positions :" << NL;
+for (const auto Sym : SymbolMap) {
+  Sym.first->dumpToStream(Out);
+  Out << " : ";
+  const auto Pos = Sym.second;
+  Out << (Pos.isValid() ? "Valid" : "Invalid") << " ; Container == ";
+  Pos.getContainer()->dumpToStream(Out);
+  Out<<" ; Offset == ";
+  Pos.getOffset()->dumpToStream(Out);
+}
+
+for (const auto Reg : RegionMap) {
+  Reg.first->dumpToStream(Out);
+  Out << " : ";
+  const auto Pos = Reg.second;
+  Out << (Pos.isValid() ? "Valid" : "Invalid") << " ; Container == ";
+  Pos.getContainer()->dumpToStream(Out);
+  Out<<" ; Offset == ";
+  Pos.getOffset()->dumpToStream(Out);
+}
+  }
+}
+
+
 namespace {
 
 const CXXRecordDecl *getCXXRecordDecl(ProgramStateRef State,

[PATCH] D70911: [clangd] Switch Hover.All to structured tests

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - 
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70911



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

[PATCH] D70911: [clangd] Switch Hover.All to structured tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70911

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -11,9 +11,14 @@
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -532,252 +537,333 @@
   }
 }
 
-TEST(Hover, All) {
-  struct OneTest {
-StringRef Input;
-StringRef ExpectedHover;
-  };
-
-  OneTest Tests[] = {
-  {
-  R"cpp(// No hover
-^int main() {
+TEST(Hover, NoHover) {
+  llvm::StringRef Tests[] = {
+  "^int main() {}",
+  "void foo() {^}",
+  R"cpp(// structured binding. Not supported yet
+struct Bar {};
+void foo() {
+  Bar a[2];
+  ^auto [x,y] = a;
+}
+  )cpp",
+  R"cpp(// Template auto parameter. Nothing (Not useful).
+template<^auto T>
+void func() {
+}
+void foo() {
+   func<1>();
 }
   )cpp",
-  "",
-  },
+  };
+
+  for (const auto  : Tests) {
+SCOPED_TRACE(Test);
+
+Annotations T(Test);
+TestTU TU = TestTU::withCode(T.code());
+TU.ExtraArgs.push_back("-std=c++17");
+auto AST = TU.build();
+ASSERT_TRUE(AST.getDiagnostics().empty());
+
+auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ASSERT_FALSE(H);
+  }
+}
+
+TEST(Hover, All) {
+  struct {
+const char *const Code;
+const std::function ExpectedBuilder;
+  } Cases[] = {
   {
   R"cpp(// Local variable
 int main() {
   int bonjour;
-  ^bonjour = 2;
+  ^[[bonjour]] = 2;
   int test1 = bonjour;
 }
   )cpp",
-  "text[Declared in]code[main]\n"
-  "codeblock(cpp) [\n"
-  "int bonjour\n"
-  "]",
-  },
+  [](HoverInfo ) {
+HI.Name = "bonjour";
+HI.Kind = index::SymbolKind::Variable;
+HI.NamespaceScope = "";
+HI.LocalScope = "main::";
+HI.Type = "int";
+HI.Definition = "int bonjour";
+  }},
   {
   R"cpp(// Local variable in method
 struct s {
   void method() {
 int bonjour;
-^bonjour = 2;
+^[[bonjour]] = 2;
   }
 };
   )cpp",
-  "text[Declared in]code[s::method]\n"
-  "codeblock(cpp) [\n"
-  "int bonjour\n"
-  "]",
-  },
+  [](HoverInfo ) {
+HI.Name = "bonjour";
+HI.Kind = index::SymbolKind::Variable;
+HI.NamespaceScope = "";
+HI.LocalScope = "s::method::";
+HI.Type = "int";
+HI.Definition = "int bonjour";
+  }},
   {
   R"cpp(// Struct
 namespace ns1 {
   struct MyClass {};
 } // namespace ns1
 int main() {
-  ns1::My^Class* Params;
+  ns1::[[My^Class]]* Params;
 }
   )cpp",
-  "text[Declared in]code[ns1]\n"
-  "codeblock(cpp) [\n"
-  "struct MyClass {}\n"
-  "]",
-  },
+  [](HoverInfo ) {
+HI.Name = "MyClass";
+HI.Kind = index::SymbolKind::Struct;
+HI.NamespaceScope = "ns1::";
+HI.Definition = "struct MyClass {}";
+  }},
   {
   R"cpp(// Class
 namespace ns1 {
   class MyClass {};
 } // namespace ns1
 int main() {
-  ns1::My^Class* Params;
+  ns1::[[My^Class]]* Params;
 }
   )cpp",
-  "text[Declared in]code[ns1]\n"
-  "codeblock(cpp) [\n"
-  "class MyClass {}\n"
-  "]",
-  },
+  [](HoverInfo ) {
+HI.Name = "MyClass";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns1::";
+HI.Definition = "class MyClass {}";
+  }},
   {
   R"cpp(// Union
 namespace ns1 {
   union MyUnion { int x; int y; };
 } // namespace ns1
 int main() {
-  ns1::My^Union Params;
+  ns1::[[My^Union]] Params;
 }
   )cpp",
-  "text[Declared in]code[ns1]\n"
-