[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1485607 , @jakehehrlich 
wrote:

> The linker doesn't look at section permissions. In fact the only thing it 
> even looks at the section index for is to check for alignment for copy 
> relocations which is something most people don't even use.


Alright, that seems reasonable. Then I will work on emitting something that 
resembles:

  --- !ifo-elf-v1
  Arch: 
  Endian:  
  Symbols:
  # One or more symbols here:
- Name:
  Type:
  
  
  ...

Along with a tool that merges these "ifo" files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1485610 , @jakehehrlich 
wrote:

> > Wanted to mention, from my testing with yaml2obj, I needed the section 
> > flags (SHF_EXECINSTR, SHF_WRITE, SHF_ALLOC, etc) otherwise nm was spitting 
> > the wrong thing. How are you going to handle those?
>
> I'm not sure what you mean. yaml2obj is a different tool that we shouldn't be 
> using IMO. yaml2obj would require the sections to denote that a symbol was 
> defined. When producing an elf file from the .tbe format you have to 
> synthesize sections for this same reason.


I meant, not including the section permissions (when I was emitting yaml2obj 
yaml) resulted in a .so file where nm thought all the symbols were ' "N" The 
symbol is a debugging symbol '


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Ping.  Is this the direction you'd like to go?  Or would you prefer something 
else?  thanks... don


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629



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

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough added a comment.

Gentle ping


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435



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

[PATCH] D61270: [CommandLine] Enable Grouping for short options by default. Part 4 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61270#1483661 , @MaskRay wrote:

> Thank you for the patch series!
>
> >   This is consistent with GNU getopt behavior.
>
> This is not GNU specific :) It is POSIX Utility Syntax Guidelines 5 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html


Thanks for pointing that out.

While not mentioned here, this series of patches also deals with long options 
which I believe is a GNU thing -- at least I think that's true.  In any case, 
the ultimate goal of these patches is to allow GNU compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61270



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Jinsong Ji via Phabricator via cfe-commits]

jsji added inline comments.



Comment at: lib/Headers/ppc_wrappers/mmintrin.h:3
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal

jsji wrote:
> MaskRay wrote:
> > This is not Apache 2.
> Ah.. This was aligned with other headers .. but forgot to update to Apache 2.
> 
> @qiucf Please update it to Apache 2 as well. Thanks.
Updated to Apache 2 in https://reviews.llvm.org/rC359164


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1485611 , @jakehehrlich 
wrote:

> Oh and "ifo" is a good name.


I still would like to keep the clang flag as -emit-ifso; the intermediate 
unmerged files are analogous to object files but the final result should be 
something that resembles a .so file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

> Wanted to mention, from my testing with yaml2obj, I needed the section flags 
> (SHF_EXECINSTR, SHF_WRITE, SHF_ALLOC, etc) otherwise nm was spitting the 
> wrong thing. How are you going to handle those?

I'm not sure what you mean. yaml2obj is a different tool that we shouldn't be 
using IMO. yaml2obj would require the sections to denote that a symbol was 
defined. When producing an elf file from the .tbe format you have to synthesize 
sections for this same reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Oh and "ifo" is a good name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

The linker doesn't look at section permissions. In fact the only thing it even 
looks at the section index for is to check for alignment for copy relocations 
which is something most people don't even use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:29
+
+bool isMessageExpressionInsideMacro(const ObjCMessageExpr *Expr) {
+  SourceLocation ReceiverLocation = Expr->getReceiverRange().getBegin();

Please use static instead anonymous namespace for functions. See LLVM Coding 
Guidelines. Same for other functions.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:107
+
+  Checks for calls to +new or overrides of it, which are prohibited by the
+  Google Objective-C style guide.

Please highlight +new with double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst:4
+google-objc-avoid-nsobject-new
+=
+

Length should be same as title.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst:6
+
+Finds uses of +new to create objects in Objective-C files.
+

Please synchronize first statement with Release Notes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst:9
+The Google Objective-C style guide forbids calling +new or overriding it in
+class implementations, preferring +alloc and -init methods to instantiate
+objects.

Please highlight  +alloc and -init with double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst:19
+
+Instead, code should use +alloc/-init or class factory methods.
+

Please highlight +alloc/-init with double back-ticks.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61350



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1485552 , @jakehehrlich 
wrote:

> Well I think it should be a seperate tool but because the code inside 
> llvm-elfabi isn't a library yet (though it is written to be used like one) we 
> can do that by adding a symlink and changing the behavior of the underlying 
> binary to present as a different tool. Many different tools around llvm do 
> this for similar reasons.


So, I think we could do something like:

  --- !ifo-elf-v1
  Arch: X86_64
  Endian: little
  Sections:
- Name:.text
  Type:SHT_PROGBITS
  Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
- Name:.data
  Type:SHT_PROGBITS
  Flags:   [ SHF_WRITE, SHF_ALLOC ]
  Symbols:
- Name:_dataA
  Type:Object
  Section: .data
  Alignment: 32
- Name:__ZN3qux3barEii
  Type:Function
  Section: .text
  Weak: true
  ...

Notice the "ifo" and not the "ifso"; I think that could be a good way to denote 
that this is the unmerged format.

I could also drop the section parts:

  --- !ifo-elf-v1
  Arch: X86_64
  Endian: little
  Symbols:
- Name:_dataA
  Type:Object
  Alignment: 32
- Name:__ZN3qux3barEii
  Type:Function
  Weak: true
  ...

as soon as we discuss the section flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483479 , @jakehehrlich 
wrote:

> In D60974#1483380 , @plotfi wrote:
>
> > Also, curious on the lack of denoting the sections and which symbols go in 
> > which sections? Do you just assume that "Type: Object" goes into .data?
>
>
> You don't need that information and the linker doesn't use it. It only uses 
> that information to get alignment. In the code that I'm writing right now it 
> happens to put everything in a single rodata section (except for tls which it 
> puts in a TLS rodata section, which isn't a thing that even exists in the 
> wild)


Wanted to mention, from my testing with yaml2obj, I needed the section flags 
(SHF_EXECINSTR, SHF_WRITE, SHF_ALLOC, etc) otherwise nm was spitting the wrong 
thing. How are you going to handle those?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Because diagnostics and their notes are not connected at the API level,
if the error message for an overload is emitted, then the overload
candidates are completed - if a diagnostic is emitted during that work,
the notes related to overload candidates would be attached to the latter
diagnostic, not the original error. Sort of worse, if the latter
diagnostic was disabled, the notes are disabled.


Repository:
  rC Clang

https://reviews.llvm.org/D61357

Files:
  include/clang/AST/TemplateName.h
  include/clang/Basic/PartialDiagnostic.h
  include/clang/Sema/Overload.h
  lib/AST/TemplateName.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaStmt.cpp
  test/SemaCXX/overload-template.cpp

Index: test/SemaCXX/overload-template.cpp
===
--- /dev/null
+++ test/SemaCXX/overload-template.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+enum copy_traits { movable = 1 };
+
+template 
+struct optional_ctor_base {};
+template 
+struct ctor_copy_traits {
+  // this would produce a c++98-compat warning, which would erroneously get the
+  // no-matching-function-call error's notes attached to it (or suppress those
+  // notes if this diagnostic was suppressed, as it is in this case)
+  static constexpr int traits = copy_traits::movable;
+};
+template 
+struct optional : optional_ctor_base::traits> {
+  template 
+  constexpr optional(U&& v);
+};
+struct A {};
+struct XA {
+  XA(const A&);
+};
+struct B {};
+struct XB {
+  XB(const B&);
+  XB(const optional&);
+};
+struct YB : XB {
+  using XB::XB;
+};
+void InsertRow(const XA&, const YB&); // expected-note {{candidate function not viable: no known conversion from 'int' to 'const XA' for 1st argument}}
+void ReproducesBugSimply() {
+  InsertRow(3, B{}); // expected-error {{no matching function for call to 'InsertRow'}}
+}
+
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2224,9 +2224,11 @@
   return Sema::FRS_Success;
 
 case Sema::FRS_NoViableFunction:
-  SemaRef.Diag(BeginRange->getBeginLoc(), diag::err_for_range_invalid)
-  << BeginRange->getType() << BEFFound;
-  CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates, BeginRange);
+  CandidateSet->NoteCandidates(
+  PartialDiagnosticAt(BeginRange->getBeginLoc(),
+  SemaRef.PDiag(diag::err_for_range_invalid)
+  << BeginRange->getType() << BEFFound),
+  SemaRef, OCD_AllCandidates, BeginRange);
   LLVM_FALLTHROUGH;
 
 case Sema::FRS_DiagnosticIssued:
@@ -2523,9 +2525,12 @@
   // Otherwise, emit diagnostics if we haven't already.
   if (RangeStatus == FRS_NoViableFunction) {
 Expr *Range = BEFFailure ? EndRangeRef.get() : BeginRangeRef.get();
-Diag(Range->getBeginLoc(), diag::err_for_range_invalid)
-<< RangeLoc << Range->getType() << BEFFailure;
-CandidateSet.NoteCandidates(*this, OCD_AllCandidates, Range);
+CandidateSet.NoteCandidates(
+PartialDiagnosticAt(Range->getBeginLoc(),
+PDiag(diag::err_for_range_invalid)
+<< RangeLoc << Range->getType()
+<< BEFFailure),
+*this, OCD_AllCandidates, Range);
   }
   // Return an error if no fix was discovered.
   if (RangeStatus != FRS_Success)
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -3504,6 +3504,8 @@
   OverloadingResult OvResult =
 IsUserDefinedConversion(*this, From, ToType, ICS.UserDefined,
 CandidateSet, false, false);
+
+  auto Cands = CandidateSet.CompleteCandidates(*this, OCD_AllCandidates, From);
   if (OvResult == OR_Ambiguous)
 Diag(From->getBeginLoc(), diag::err_typecheck_ambiguous_condition)
 << From->getType() << ToType << From->getSourceRange();
@@ -3515,7 +3517,9 @@
   << false << From->getType() << From->getSourceRange() << ToType;
   } else
 return false;
-  CandidateSet.NoteCandidates(*this, OCD_AllCandidates, From);
+
+  CandidateSet.NoteCandidates(
+  *this, From, Cands);
   return true;
 }
 
@@ -10744,11 +10748,9 @@
   }
 }
 
-/// When overload resolution fails, prints diagnostic messages containing the
-/// candidates in the candidate set.
-void OverloadCandidateSet::NoteCandidates(
+SmallVector OverloadCandidateSet::CompleteCandidates(
 Sema , OverloadCandidateDisplayKind OCD, ArrayRef Args,
-StringRef Opc, 

[PATCH] D61357: SemaOverload: Complete candidates before emitting the error, to ensure diagnostics emitted (or suppressed) during completion don't interfere with the overload notes

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Oh, @rsmith - if there's any better/different testing (if you can figure out 
how to reduce the test case down further, now that we know the cause - or if 
you'd like testing for other codepaths I've touched in this patch) I'm all 
ears. (also naming/API wrangling - my changes were just the minimal sort of 
"this works" based on what we discussed, but totally happy to do more 
involved/different things if there's such to be done)


Repository:
  rC Clang

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

https://reviews.llvm.org/D61357



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Well I think it should be a seperate tool but because the code inside 
llvm-elfabi isn't a library yet (though it is written to be used like one) we 
can do that by adding a symlink and changing the behavior of the underlying 
binary to present as a different tool. Many different tools around llvm do this 
for similar reasons.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1485417 , @jakehehrlich 
wrote:

> Background:
>  There are two parts to this. TextAPI is a public interface of textual 
> representations for these sorts of files that already exists in llvm. There 
> exist ELF and MachO textual representations currently. llvm-elfabi is a tool 
> for 1) Creating Stubs 2) Creating .tbe (the TextAPI fully linked ELF format) 
> files 3) Converting between them and 4) Analyzing the details of the formats.
>
> You asked about the format I proposed here, not the already accepted .tbe 
> format. The format proposed here could be added to TextAPI where it could 
> subsequently be used in both Clang and a tool that would use the llvm-elfabi 
> code to create stubs. I imagine the flags needed for this new tool would be 
> very differnet from what llvm-elfabi uses or would use in the future so we'd 
> probably want a second symlink the behaved very differently.
>
> It can currently read ELF and .tbe files and can currently write .tbe files. 
> I'll have the basics of ELF writing up for review by the end of the day and 
> that will, for the next month or two be my primary work which should likely 
> see it close to stabilized. I'd expect landing the basics of ELF writing to 
> take another week or two since its somewhat involved and I'll likely need to 
> split the patches up from what I have. Given that no such writer currently 
> exists this should put a version of your tool well ahead of schedule. Such 
> are the wonderful benefits of open source.


Good, thanks for confirming this. For the format you proposed here, do you 
think that should be consumed by llvm-elfabi or by a secondary tool that 
generates a final merged tbe to be consumed by llvm-elfabi?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: chandlerc.
rjmccall added a comment.

@chandlerc, I hate to do this to you, but licensing question.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Hmm.  You know, there's another case where the destructor can be called even 
for a non-array: if constructing the object requires a temporary, I believe an 
exception thrown from that temporary's destructor is supposed to go back and 
destroy the variable.  (This is, sadly, not entirely clear under the standard 
since the object is not automatic.)  Now, Clang does not actually get this 
correct — we abort the construction, but we don't destroy the variable — but 
(1) we should fix that someday and (2) in the meantime, we shouldn't implement 
something which will be a problem when we go to fix that.

This doesn't affect non-locals because there the exception will call 
`std::terminate()` as specified in [except.terminate]p1.




Comment at: clang/include/clang/Basic/AttrDocs.td:3893
+~only_no_destroy();
+  }
+

Missing semicolon.


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

https://reviews.llvm.org/D61165



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Follow-up test fix in http://llvm.org/r359636


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

r359636 - Fix auto-init test

[JF Bastien via cfe-commits]

Author: jfb
Date: Tue Apr 30 16:27:28 2019
New Revision: 359636

URL: http://llvm.org/viewvc/llvm-project?rev=359636=rev
Log:
Fix auto-init test

r359628 changed the initialization of padding to follow C, but I didn't update 
the C++ tests.

Modified:
cfe/trunk/test/CodeGenCXX/auto-var-init.cpp

Modified: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/auto-var-init.cpp?rev=359636=359635=359636=diff
==
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp Tue Apr 30 16:27:28 2019
@@ -64,7 +64,7 @@ struct smallpartinit { char c = 42, d; }
 // PATTERN-O1-NOT: @__const.test_nullinit_custom.custom
 struct nullinit { char* null = nullptr; };
 // PATTERN-O0: @__const.test_padded_uninit.uninit = private unnamed_addr 
constant { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 
}, align 4
-// PATTERN-O0: @__const.test_padded_custom.custom = private unnamed_addr 
constant { i8, [3 x i8], i32 } { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, 
align 4
+// PATTERN-O0: @__const.test_padded_custom.custom = private unnamed_addr 
constant { i8, [3 x i8], i32 } { i8 42, [3 x i8] zeroinitializer, i32 13371337 
}, align 4
 // ZERO-O0: @__const.test_padded_custom.custom = private unnamed_addr constant 
{ i8, [3 x i8], i32 } { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, align 4
 // PATTERN-O1-NOT: @__const.test_padded_uninit.uninit
 // PATTERN-O1-NOT: @__const.test_padded_custom.custom
@@ -88,7 +88,7 @@ struct paddedpackedarray { struct padded
 // PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private 
unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } 
{ i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
 struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { 
i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
-// PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] c"\AA\AA\AA", i32 13371338 } }, align 4
+// PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] zeroinitializer, i32 13371338 } }, align 4
 // ZERO: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] zeroinitializer, i32 13371338 } }, align 4
 struct paddednested { struct padded p1, p2; };
 // PATTERN-O0: @__const.test_paddedpackednested_uninit.uninit = private 
unnamed_addr constant %struct.paddedpackednested { %struct.paddedpacked <{ i8 
-86, i32 -1431655766 }>, %struct.paddedpacked <{ i8 -86, i32 -1431655766 }> }, 
align 1
@@ -135,7 +135,7 @@ struct arraytail { int i; int arr[]; };
 // ZERO: @__const.test_intptr4_custom.custom = private unnamed_addr constant 
[4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 
to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 
to i32*)], align 16
 // PATTERN-O0: @__const.test_tailpad4_uninit.uninit = private unnamed_addr 
constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 -21846, i8 
-86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] 
c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, 
i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }], align 16
 // PATTERN-O1-NOT: @__const.test_tailpad4_uninit.uninit
-// PATTERN: @__const.test_tailpad4_custom.custom = private unnamed_addr 
constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 257, i8 1, [1 
x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] c"\AA" }, { 
i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { 
i16 257, i8 1, [1 x i8] c"\AA" }], align 16
+// PATTERN: @__const.test_tailpad4_custom.custom = private unnamed_addr 
constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 257, i8 1, [1 
x i8] zeroinitializer }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] 
zeroinitializer }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] 
zeroinitializer }, { i16, i8, [1 x i8] } { i16 257, i8 1, [1 x i8] 
zeroinitializer }], align 16
 // ZERO: @__const.test_tailpad4_custom.custom = private 

[PATCH] D61318: [Sema] Prevent binding references with mismatching address spaces to temporaries

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaInit.cpp:4836
+  if (T1Quals.hasAddressSpace()) {
+if (!T1Quals.isAddressSpaceSupersetOf(cv1T1IgnoreAS.getQualifiers())) {
+  Sequence.SetFailed(

Isn't `cv1T1IgnoreAS.getQualifiers()` always `LangAS::Default`?


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

https://reviews.llvm.org/D61318



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

Pinging for visibility.


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

https://reviews.llvm.org/D55044



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

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359630: [WebAssembly] Use the wasm32-wasi triple 
in tests (authored by djg, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D61338

Files:
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9577,10 +9577,10 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-unknown \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY32,WEBASSEMBLY-WASI %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
 //
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -24,17 +24,17 @@
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi-musl/c++/v1" "-internal-isystem" 
"/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi-musl" 
"-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" 
"-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" 
"/foo/include"
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -24,20 +24,20 @@
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 197477.
erik.pilkington added a comment.

Add a try/catch block to the docs.


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

https://reviews.llvm.org/D61165

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/no_destroy.cpp
  clang/test/SemaCXX/no_destroy.cpp

Index: clang/test/SemaCXX/no_destroy.cpp
===
--- clang/test/SemaCXX/no_destroy.cpp
+++ clang/test/SemaCXX/no_destroy.cpp
@@ -1,11 +1,13 @@
-// RUN: %clang_cc1 -DNO_DTORS -fno-c++-static-destructors -verify %s
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -DNO_DTORS -DNO_EXCEPTIONS -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -DNO_EXCEPTIONS -verify %s
+// RUN: %clang_cc1 -DNO_DTORS -fexceptions -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -fexceptions -verify %s
 
 struct SecretDestructor {
 #ifndef NO_DTORS
   // expected-note@+2 4 {{private}}
 #endif
-private: ~SecretDestructor(); // expected-note 2 {{private}}
+private: ~SecretDestructor(); // expected-note + {{private}}
 };
 
 SecretDestructor sd1;
@@ -44,3 +46,36 @@
 
 [[clang::no_destroy(0)]] int no_args; // expected-error{{'no_destroy' attribute takes no arguments}}
 [[clang::always_destroy(0)]] int no_args2; // expected-error{{'always_destroy' attribute takes no arguments}}
+
+SecretDestructor arr[10];
+#ifndef NO_DTORS
+// expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+
+void local_arrays() {
+  static SecretDestructor arr2[10];
+#if !defined(NO_DTORS) || !defined(NO_EXCEPTIONS)
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+  thread_local SecretDestructor arr3[10];
+#if !defined(NO_DTORS) || !defined(NO_EXCEPTIONS)
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+}
+
+struct Base {
+  ~Base();
+};
+struct Derived1 {
+  Derived1(int);
+  Base b;
+};
+struct Derived2 {
+  Derived1 b;
+};
+
+void dontcrash() {
+  [[clang::no_destroy]] static Derived2 d2[] = {0, 0};
+}
+
+[[clang::no_destroy]] Derived2 d2[] = {0, 0};
Index: clang/test/CodeGenCXX/no_destroy.cpp
===
--- clang/test/CodeGenCXX/no_destroy.cpp
+++ clang/test/CodeGenCXX/no_destroy.cpp
@@ -1,11 +1,14 @@
-// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,NO_EXCEPTIONS
+// RUN: %clang_cc1 -fexceptions %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,EXCEPTIONS
 
 struct NonTrivial {
   ~NonTrivial();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] NonTrivial nt1;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] thread_local NonTrivial nt2;
 
@@ -13,11 +16,14 @@
   ~NonTrivial2();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
 NonTrivial2 nt21;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN11NonTrivial2D1Ev
 thread_local NonTrivial2 nt22;
 
+// CHECK-LABEL: define void @_Z1fv
 void f() {
   // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
   static NonTrivial2 nt21;
@@ -25,7 +31,50 @@
   thread_local NonTrivial2 nt22;
 }
 
+// CHECK-LABEL: define void @_Z1gv
+void g() {
+  // CHECK-NOT: __cxa_atexit
+  [[clang::no_destroy]] static NonTrivial2 nt21;
+  // CHECK-NOT: _tlv_atexit
+  [[clang::no_destroy]] thread_local NonTrivial2 nt22;
+}
+
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] NonTrivial nt3;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] thread_local NonTrivial nt4;
+
+
+struct NonTrivial3 {
+  NonTrivial3();
+  ~NonTrivial3();
+};
+
+[[clang::no_destroy]] NonTrivial3 arr[10];
+
+// CHECK-LABEL: define internal void @__cxx_global_var_init
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// CHECK-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
+
+void h() {
+  [[clang::no_destroy]] static NonTrivial3 slarr[10];
+}
+
+// CHECK-LABEL: define void @_Z1hv
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// EXCEPTIONS: call void @_ZN11NonTrivial3D1Ev
+// NO_EXCEPTIONS-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
+
+void i() {
+  [[clang::no_destroy]] thread_local NonTrivial3 tlarr[10];
+}

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
 

efriedma wrote:
> richard.townsend.arm wrote:
> > efriedma wrote:
> > > richard.townsend.arm wrote:
> > > > I'm not sure what the IsSizeGreaterThan128 check is doing here - if the 
> > > > return type is over 128 bits, then it will be indirectly returned in X8 
> > > > with this check, which is not always what we want (e.g. in 
> > > > https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects 
> > > > the value returned in X1). Another check of hasMicrosoftABIRestrictions 
> > > > might be OK, but that's also not quite right due to the size check. 
> > > I think the check here is intended to counteract the similar check in 
> > > hasMicrosoftABIRestrictions... but yes, I don't think that works 
> > > correctly.  Looks like we're missing a testcase for a non-aggregate type 
> > > with size greater than 128 bits.
> > Ah, OK. I think the problem is that we need to check whether the return 
> > value meets the rest of aggregate definition (i.e. 
> > !hasMicrosoftABIRestrictions), as well as being more than 128 bits in size. 
> > May be worth splitting the logic up. Will experiment.
> I think you might get the right behavior by just erasing both 
> IsSizeGreaterThan128 checks.  That matches the way the ABI document describes 
> the rules.  Haven't tried it, though.
So I tried that, and everything mostly worked, but there are still some crashes 
in electron. I think the solution will be something like:
1. Remove the size restriction from the hasMicrosoftABIRestrictions and just 
decide if the result is aggregate/not aggregate according to the ABI spec.
2. Replace the check with !(isAggregate && IsSizeGreaterThan128(RD))

If both of those things are are true (i.e. it's an aggregate more than 
>128bits) we should end up calling FI.getReturnInfo().setInReg(false), which 
implies a return in x8. I haven't confirmed that this works yet because I've 
run out of time today, but the result should be in tomorrow. May need similar 
logic on the outer if.


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

https://reviews.llvm.org/D60349



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

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In D61147#1479932 , @rjmccall wrote:

> I would also recommend that you go fix the warning to never fire on virtual 
> C++ methods.


I tried building clang/llvm with  -Wunused-parameter turned on and there are 
lots of places where parameters are unused other than virtual functions, in 
particular template functions and non-template functions that are called by 
other template functions. So disabling the warning on C++ virtual functions is 
probably not enough in order to make this warning more useful.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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

r359630 - [WebAssembly] Use the "wasm32-wasi" triple in tests

[Dan Gohman via cfe-commits]

Author: djg
Date: Tue Apr 30 16:06:07 2019
New Revision: 359630

URL: http://llvm.org/viewvc/llvm-project?rev=359630=rev
Log:
[WebAssembly] Use the "wasm32-wasi" triple in tests

Similar to https://reviews.llvm.org/D61334, update clang tests to use the
"wasm32-wasi" triple, removing the "-musl" environment and omitting the
"-unknown" vendor.

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

Reviewer: sbc100

Modified:
cfe/trunk/test/Driver/wasm-toolchain.c
cfe/trunk/test/Driver/wasm-toolchain.cpp
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=359630=359629=359630=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Tue Apr 30 16:06:07 2019
@@ -24,20 +24,20 @@
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C compile command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
 
 // Thread-related command line tests.
 

Modified: cfe/trunk/test/Driver/wasm-toolchain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.cpp?rev=359630=359629=359630=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.cpp (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.cpp Tue Apr 30 16:06:07 2019
@@ -24,17 +24,17 @@
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo 

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2579
+assert(
+!ES.getExpr() ||
+CXXConstructorDeclBits.HasTrailingExplicitSpecifier &&

Your or needs parens or the disambiguation is wrong.



Comment at: clang/include/clang/Serialization/ASTReader.h:2435
+uint64_t Kind = readInt();
+bool hasExpr = Kind & 0x1;
+Kind = Kind >> 1;

same here.



Comment at: clang/lib/Sema/DeclSpec.cpp:952
+  ExplicitSpec.getExpr()) &&
+ "invalide ExplicitSpecifier");
   // 'explicit explicit' is ok, but warn as this is likely not what the user

typo.



Comment at: clang/test/CXX/temp/temp.deduct.guide/p1.cpp:74
 virtual A(int(&)[28]) -> A; // expected-error {{'virtual' can only appear 
on non-static member functions}}
-const A(int(&)[28]) -> A; // expected-error {{deduction guide cannot be 
declared 'const'}}
+const A(int(&)[31]) -> A; // expected-error {{deduction guide cannot be 
declared 'const'}}
 

Is there a reason why you changed this?


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

https://reviews.llvm.org/D60934



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359628: Variable auto-init: dont initialize aggregate 
padding of all aggregates (authored by jfb, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61280?vs=197188=197476#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61280

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/padding-init.c


Index: test/CodeGen/padding-init.c
===
--- test/CodeGen/padding-init.c
+++ test/CodeGen/padding-init.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown 
-ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero 
%s -emit-llvm -o - | FileCheck %s
+
+// C guarantees that brace-init with fewer initializers than members in the
+// aggregate will initialize the rest of the aggregate as-if it were static
+// initialization. In turn static initialization guarantees that padding is
+// initialized to zero bits.
+
+// CHECK: @__const.partial_init.s = private unnamed_addr constant { i8, [7 x 
i8], i64 } { i8 42, [7 x i8] zeroinitializer, i64 0 }, align 8
+
+// Technically, we could initialize this padding to non-zero because all of the
+// struct's members have initializers.
+
+// CHECK: @__const.init_all.s = private unnamed_addr constant { i8, [7 x i8], 
i64 } { i8 42, [7 x i8] zeroinitializer, i64 -2401053089374216531 }, align 8
+
+struct S {
+  char c;
+  long long l;
+};
+
+void use(struct S*);
+
+// CHECK-LABEL: @empty_braces(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memset{{.*}}(i8* align 8 %[[B]], i8 0,
+// CHECK-NEXT:  call void @use(%struct.S* %s)
+void empty_braces() {
+  struct S s = {};
+  return use();
+}
+
+// CHECK-LABEL: @partial_init(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memcpy{{.*}}(i8* align 8 %[[B]], 
{{.*}}@__const.partial_init.s
+// CHECK-NEXT:  call void @use(%struct.S* %s)
+void partial_init() {
+  struct S s = { .c = 42 };
+  return use();
+}
+
+// CHECK-LABEL: @init_all(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memcpy{{.*}}(i8* align 8 %[[B]], 
{{.*}}@__const.init_all.s
+// CHECK-NEXT:  call void @use(%struct.S* %s)
+void init_all() {
+  struct S s = { .c = 42, .l = 0xdeadbeefc0fedead };
+  return use();
+}
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1787,13 +1787,20 @@
   D.isUsableInConstantExpressions(getContext())) {
 assert(!capturedByInit && "constant init contains a capturing block?");
 constant = ConstantEmitter(*this).tryEmitAbstractForInitializer(D);
-if (constant && trivialAutoVarInit !=
-LangOptions::TrivialAutoVarInitKind::Uninitialized) {
+if (constant && !constant->isZeroValue() &&
+(trivialAutoVarInit !=
+ LangOptions::TrivialAutoVarInitKind::Uninitialized)) {
   IsPattern isPattern =
   (trivialAutoVarInit == LangOptions::TrivialAutoVarInitKind::Pattern)
   ? IsPattern::Yes
   : IsPattern::No;
-  constant = constWithPadding(CGM, isPattern,
+  // C guarantees that brace-init with fewer initializers than members in
+  // the aggregate will initialize the rest of the aggregate as-if it were
+  // static initialization. In turn static initialization guarantees that
+  // padding is initialized to zero bits. We could instead pattern-init if 
D
+  // has any ImplicitValueInitExpr, but that seems to be unintuitive
+  // behavior.
+  constant = constWithPadding(CGM, IsPattern::No,
   replaceUndef(CGM, isPattern, constant));
 }
   }


Index: test/CodeGen/padding-init.c
===
--- test/CodeGen/padding-init.c
+++ test/CodeGen/padding-init.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s
+
+// C guarantees that brace-init with fewer initializers than members in the
+// aggregate will initialize the rest of the aggregate as-if it were static
+// initialization. In turn static initialization guarantees that padding is
+// initialized to zero bits.
+
+// CHECK: @__const.partial_init.s = private unnamed_addr constant { i8, [7 x i8], i64 } { i8 42, [7 x i8] zeroinitializer, i64 0 }, align 8
+
+// Technically, we could initialize this padding to non-zero because all of the
+// 

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

This seems uncontroversial, I'm happy to make follow-up change if you have 
suggestions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 197469.
ahatanak retitled this revision from "[Sema][ObjC] Add a flavor of 
-Wunused-parameter that doesn't diagnose unused parameters of ObjC methods" to 
"[Sema][ObjC] Disable -Wunused-parameter for ObjC methods".
ahatanak edited the summary of this revision.
ahatanak added a comment.

Disable the warning for ObjC methods.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaObjC/method-unused-attribute.m
  test/SemaObjC/unused.m


Index: test/SemaObjC/unused.m
===
--- test/SemaObjC/unused.m
+++ test/SemaObjC/unused.m
@@ -33,7 +33,7 @@
   // expected-note {{introduce a parameter name to make 
'x' part of the selector}} \
   // expected-note {{or insert whitespace before ':' to 
use 'x' as parameter name and have an empty entry in the selector}}
 
-(int)y:  // expected-warning {{unused}}  expected-warning {{'y' used as the 
name of the previous parameter rather than as part of the selector}} \
+(int)y:  // expected-warning {{'y' used as the name of the previous parameter 
rather than as part of the selector}} \
  // expected-note {{introduce a parameter name to make 'y' part of the 
selector}} \
  // expected-note {{or insert whitespace before ':' to use 'y' as 
parameter name and have an empty entry in the selector}}
 (int) __attribute__((unused))z { return x; }
Index: test/SemaObjC/method-unused-attribute.m
===
--- test/SemaObjC/method-unused-attribute.m
+++ test/SemaObjC/method-unused-attribute.m
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1  -fsyntax-only -Wunused-parameter -verify 
-Wno-objc-root-class %s
 
+// -Wunused-parameter ignores ObjC method parameters that are unused.
+
+// expected-no-diagnostics
+
 @interface INTF
 - (void) correct_use_of_unused: (void *) notice : (id)another_arg;
 - (void) will_warn_unused_arg: (void *) notice : (id)warn_unused;
@@ -9,7 +13,7 @@
 @implementation INTF
 - (void) correct_use_of_unused: (void *)  __attribute__((unused)) notice : 
(id) __attribute__((unused)) newarg{
 }
-- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 
(id)warn_unused {}  // expected-warning {{unused parameter 'warn_unused'}}
-- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}  // 
expected-warning {{unused parameter 'will_warn'}}
+- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 
(id)warn_unused {}
+- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}
 @end
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13354,8 +13354,6 @@
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  if (!MD->hasSkippedBody())
-DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 


Index: test/SemaObjC/unused.m
===
--- test/SemaObjC/unused.m
+++ test/SemaObjC/unused.m
@@ -33,7 +33,7 @@
   // expected-note {{introduce a parameter name to make 'x' part of the selector}} \
   // expected-note {{or insert whitespace before ':' to use 'x' as parameter name and have an empty entry in the selector}}
 
-(int)y:  // expected-warning {{unused}}  expected-warning {{'y' used as the name of the previous parameter rather than as part of the selector}} \
+(int)y:  // expected-warning {{'y' used as the name of the previous parameter rather than as part of the selector}} \
  // expected-note {{introduce a parameter name to make 'y' part of the selector}} \
  // expected-note {{or insert whitespace before ':' to use 'y' as parameter name and have an empty entry in the selector}}
 (int) __attribute__((unused))z { return x; }
Index: test/SemaObjC/method-unused-attribute.m
===
--- test/SemaObjC/method-unused-attribute.m
+++ test/SemaObjC/method-unused-attribute.m
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1  -fsyntax-only -Wunused-parameter -verify -Wno-objc-root-class %s
 
+// -Wunused-parameter ignores ObjC method parameters that are unused.
+
+// expected-no-diagnostics
+
 @interface INTF
 - (void) correct_use_of_unused: (void *) notice : (id)another_arg;
 - (void) will_warn_unused_arg: (void *) notice : (id)warn_unused;
@@ -9,7 +13,7 @@
 @implementation INTF
 - (void) correct_use_of_unused: (void *)  __attribute__((unused)) notice : (id) __attribute__((unused)) newarg{
 }
-- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : 

r359628 - Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via cfe-commits]

Author: jfb
Date: Tue Apr 30 15:56:53 2019
New Revision: 359628

URL: http://llvm.org/viewvc/llvm-project?rev=359628=rev
Log:
Variable auto-init: don't initialize aggregate padding of all aggregates

Summary:
C guarantees that brace-init with fewer initializers than members in the
aggregate will initialize the rest of the aggregate as-if it were static
initialization. In turn static initialization guarantees that padding is
initialized to zero bits.

Quoth the Standard:

C17 6.7.9 Initialization ❡21

If there are fewer initializers in a brace-enclosed list than there are elements
or members of an aggregate, or fewer characters in a string literal used to
initialize an array of known size than there are elements in the array, the
remainder of the aggregate shall be initialized implicitly the same as objects
that have static storage duration.

C17 6.7.9 Initialization ❡10

If an object that has automatic storage duration is not initialized explicitly,
its value is indeterminate. If an object that has static or thread storage
duration is not initialized explicitly, then:

 * if it has pointer type, it is initialized to a null pointer;
 * if it has arithmetic type, it is initialized to (positive or unsigned) zero;
 * if it is an aggregate, every member is initialized (recursively) according to
   these rules, and any padding is initialized to zero bits;
 * if it is a union, the first named member is initialized (recursively)
   according to these rules, and any padding is initialized to zero bits;



Reviewers: glider, pcc, kcc, rjmccall, erik.pilkington

Subscribers: jkorous, dexonsmith, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGen/padding-init.c
Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=359628=359627=359628=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Apr 30 15:56:53 2019
@@ -1787,13 +1787,20 @@ void CodeGenFunction::EmitAutoVarInit(co
   D.isUsableInConstantExpressions(getContext())) {
 assert(!capturedByInit && "constant init contains a capturing block?");
 constant = ConstantEmitter(*this).tryEmitAbstractForInitializer(D);
-if (constant && trivialAutoVarInit !=
-LangOptions::TrivialAutoVarInitKind::Uninitialized) {
+if (constant && !constant->isZeroValue() &&
+(trivialAutoVarInit !=
+ LangOptions::TrivialAutoVarInitKind::Uninitialized)) {
   IsPattern isPattern =
   (trivialAutoVarInit == LangOptions::TrivialAutoVarInitKind::Pattern)
   ? IsPattern::Yes
   : IsPattern::No;
-  constant = constWithPadding(CGM, isPattern,
+  // C guarantees that brace-init with fewer initializers than members in
+  // the aggregate will initialize the rest of the aggregate as-if it were
+  // static initialization. In turn static initialization guarantees that
+  // padding is initialized to zero bits. We could instead pattern-init if 
D
+  // has any ImplicitValueInitExpr, but that seems to be unintuitive
+  // behavior.
+  constant = constWithPadding(CGM, IsPattern::No,
   replaceUndef(CGM, isPattern, constant));
 }
   }

Added: cfe/trunk/test/CodeGen/padding-init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/padding-init.c?rev=359628=auto
==
--- cfe/trunk/test/CodeGen/padding-init.c (added)
+++ cfe/trunk/test/CodeGen/padding-init.c Tue Apr 30 15:56:53 2019
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown 
-ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ftrivial-auto-var-init=zero 
%s -emit-llvm -o - | FileCheck %s
+
+// C guarantees that brace-init with fewer initializers than members in the
+// aggregate will initialize the rest of the aggregate as-if it were static
+// initialization. In turn static initialization guarantees that padding is
+// initialized to zero bits.
+
+// CHECK: @__const.partial_init.s = private unnamed_addr constant { i8, [7 x 
i8], i64 } { i8 42, [7 x i8] zeroinitializer, i64 0 }, align 8
+
+// Technically, we could initialize this padding to non-zero because all of the
+// struct's members have initializers.
+
+// CHECK: @__const.init_all.s = private unnamed_addr constant { i8, [7 x i8], 
i64 } { i8 42, [7 x i8] zeroinitializer, i64 -2401053089374216531 }, align 8
+
+struct S {
+  char c;
+  long long l;
+};
+
+void use(struct S*);
+
+// CHECK-LABEL: @empty_braces(
+// CHECK:   %s = alloca
+// CHECK-NEXT:  %[[B:[0-9+]]] = bitcast %struct.S* %s to i8*
+// CHECK-NEXT:  call void @llvm.memset{{.*}}(i8* align 8 %[[B]], i8 0,
+// 

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Background:
There are two parts to this. TextAPI is a public interface of textual 
representations for these sorts of files that already exists in llvm. There 
exist ELF and MachO textual representations currently. llvm-elfabi is a tool 
for 1) Creating Stubs 2) Creating .tbe (the TextAPI fully linked ELF format) 
files 3) Converting between them and 4) Analyzing the details of the formats.

You asked about the format I proposed here, not the already accepted .tbe 
format. The format proposed here could be added to TextAPI where it could 
subsequently be used in both Clang and a tool that would use the llvm-elfabi 
code to create stubs. I imagine the flags needed for this new tool would be 
very differnet from what llvm-elfabi uses or would use in the future so we'd 
probably want a second symlink the behaved very differently.

It can currently read ELF and .tbe files and can currently write .tbe files. 
I'll have the basics of ELF writing up for review by the end of the day and 
that will, for the next month or two be my primary work which should likely see 
it close to stabilized. I'd expect landing the basics of ELF writing to take 
another week or two since its somewhat involved and I'll likely need to split 
the patches up from what I have. Given that no such writer currently exists 
this should put a version of your tool well ahead of schedule. Such are the 
wonderful benefits of open source.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61269: [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 197466.
hintonda added a comment.

- Refactor and fix additional dashes in error messages and tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61269

Files:
  clang/test/Driver/clang-offload-bundler.c
  llvm/lib/Support/CommandLine.cpp
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/Support/check-default-options.txt
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -1042,7 +1042,7 @@
 
   StackOption TestOption(Opt, cl::desc(HelpText),
   OptionAttributes...);
-  printOptionInfo(TestOption, 25);
+  printOptionInfo(TestOption, 26);
   outs().flush();
 }
 auto Buffer = MemoryBuffer::getFile(File.FilePath);
@@ -1069,8 +1069,8 @@
   cl::values(clEnumValN(OptionValue::Val, "v1", "desc1")));
 
   // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n")
+  EXPECT_EQ(Output, ("  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n")
 .str());
   // clang-format on
 }
@@ -1082,9 +1082,9 @@
 
   // clang-format off
   EXPECT_EQ(Output,
-("  -" + Opt + " - " + HelpText + "\n"
- "  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n")
+("  --" + Opt + " - " + HelpText + "\n"
+ "  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n")
 .str());
   // clang-format on
 }
@@ -1095,10 +1095,10 @@
 clEnumValN(OptionValue::Val, "", "desc2")));
 
   // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + " - " + HelpText + "\n"
- "  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n"
- "=   -   desc2\n")
+  EXPECT_EQ(Output, ("  --" + Opt + " - " + HelpText + "\n"
+ "  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n"
+ "=-   desc2\n")
 .str());
   // clang-format on
 }
@@ -1109,8 +1109,8 @@
 clEnumValN(OptionValue::Val, "", "")));
 
   // clang-format off
-  EXPECT_EQ(Output, ("  -" + Opt + "= - " + HelpText + "\n"
- "=v1-   desc1\n"
+  EXPECT_EQ(Output, ("  --" + Opt + "= - " + HelpText + "\n"
+ "=v1 -   desc1\n"
  "=\n")
 .str());
   // clang-format on
@@ -1122,7 +1122,7 @@
 
   // clang-format off
   EXPECT_EQ(Output,
-("  -" + Opt + "= - " + HelpText + "\n"
+("  --" + Opt + "= - " + HelpText + "\n"
  "=v1\n").str());
   // clang-format on
 }
@@ -1147,7 +1147,7 @@
 
 TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) {
   StringRef ArgName("a-long-argument-name");
-  size_t ExpectedStrSize = ("  -" + ArgName + "= - ").str().size();
+  size_t ExpectedStrSize = ("  --" + ArgName + "= - ").str().size();
   EXPECT_EQ(
   runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))),
   ExpectedStrSize);
Index: llvm/test/Support/check-default-options.txt
===
--- llvm/test/Support/check-default-options.txt
+++ llvm/test/Support/check-default-options.txt
@@ -1,18 +1,18 @@
-# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
-# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
-# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
-# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
-# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-objdump --help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj --help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen --help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report --help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump --help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
 # RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
 
 
 # CHECK-OBJDUMP: -h  - Alias for --section-headers
 # CHECK-READOBJ: -h  - Alias for --file-headers
-# CHECK-TBLGEN:  -h  - Alias for -help
-# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-TBLGEN:  -h  - Alias for 

[PATCH] D60937: [clangd] Fix code completion of macros defined in the preamble region of the main file.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Ping :-)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60937



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

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D61165#1479937 , @rjmccall wrote:

> Are you sure these are the right semantics for `nodestroy`?  I think there's 
> a reasonable argument that we should not destroy previous elements of a 
> `nodestroy` array just because an element constructor throws.  It's basically 
> academic for true globals because the exception will terminate the process 
> anyway, and even for `thread_local`s and `static` locals (where I believe the 
> exception is theoretically recoverable) it's at least arguable that we should 
> either decline to destroy (possibly causing leaks) or simply call 
> `std::terminate`.


I think `std::terminate` makes the most sense. Getting teardown correctly is 
always tricky, and I'm willing to bet that teardown caused by an exception in 
construction of an array is even harder and done wrong.




Comment at: clang/include/clang/Basic/AttrDocs.td:3906
+[[clang::no_destroy]]
+static only_no_destroy array[10]; // error, only_no_destroy has a private 
destructor.
+

You probably want a `try`/`catch` here to illustrate why exceptions can matter.


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

https://reviews.llvm.org/D61165



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

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D60605#1483729 , @ilya-biryukov 
wrote:

> Found today:


I'm not able to reproduce this, can you give a complete example?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

[PATCH] D61350: New clang-tidy check calling out uses of +new in Objective-C code

[Michael Wyman via Phabricator via cfe-commits]

mwyman created this revision.
mwyman added reviewers: benhamilton, hokein.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Google's Objective-C style guide forbids calling or overriding +new to 
instantiate objects. This check warns on violations.

Style guide reference: 
https://google.github.io/styleguide/objcguide.html#do-not-use-new


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61350

Files:
  clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
  clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m

Index: clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s google-objc-avoid-nsobject-new %t
+
+@interface NSObject
++ (instancetype)new;
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+// NSDate provides a specific factory method.
+@interface NSDate : NSObject
++ (instancetype)date;
+@end
+
+// For testing behavior with Objective-C Generics.
+@interface NSMutableDictionary<__covariant KeyType, __covariant ObjectType> : NSObject
+@end
+
+@class NSString;
+
+#define ALLOCATE_OBJECT(_Type) [_Type new]
+
+void CheckSpecificInitRecommendations(void) {
+  NSObject *object = [NSObject new];
+  // CHECK-MESSAGES: [[@LINE-1]]:22: warning: do not create objects with +new [google-objc-avoid-nsobject-new]
+  // CHECK-FIXES: [NSObject alloc] init];
+
+  NSDate *correctDate = [NSDate date];
+  NSDate *incorrectDate = [NSDate new];
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: do not create objects with +new [google-objc-avoid-nsobject-new]
+  // CHECK-FIXES: [NSDate date];
+
+  NSObject *macroCreated = ALLOCATE_OBJECT(NSObject);  // Shouldn't warn on macros.
+
+  NSMutableDictionary *dict = [NSMutableDictionary new];
+  // CHECK-MESSAGES: [[@LINE-1]]:31: warning: do not create objects with +new [google-objc-avoid-nsobject-new]
+  // CHECK-FIXES: [NSMutableDictionary alloc] init];
+}
+
+@interface Foo : NSObject
++ (instancetype)new; // Declare again to suppress warning.
+- (instancetype)initWithInt:(int)anInt;
+- (instancetype)init __attribute__((unavailable));
+
+- (id)new;
+@end
+
+@interface Baz : Foo // Check unavailable -init through inheritance.
+@end
+
+void CallNewWhenInitUnavailable(void) {
+  Foo *foo = [Foo new];
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: do not create objects with +new [google-objc-avoid-nsobject-new]
+
+  Baz *baz = [Baz new];
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: do not create objects with +new [google-objc-avoid-nsobject-new]
+
+  // Instance method -new calls may be weird, but are not strictly forbidden.
+  Foo *bar = [[Foo alloc] initWithInt:4];
+  [bar new];
+}
+
+@interface HasNewOverride : NSObject
+@end
+
+@implementation HasNewOverride
++ (instancetype)new {
+  return [[self alloc] init];
+}
+// CHECK-MESSAGES: [[@LINE-3]]:1: warning: classes should not override +new [google-objc-avoid-nsobject-new]
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -133,6 +133,7 @@
google-default-arguments
google-explicit-constructor
google-global-names-in-headers
+   google-objc-avoid-nsobject-new
google-objc-avoid-throwing-exception
google-objc-function-naming
google-objc-global-variable-declaration
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-avoid-nsobject-new
+
+google-objc-avoid-nsobject-new
+=
+
+Finds uses of +new to create objects in Objective-C files.
+
+The Google Objective-C style guide forbids calling +new or overriding it in
+class implementations, preferring +alloc and -init methods to instantiate
+objects.
+
+An example:
+
+.. code-block:: objc
+
+  NSDate *now = [NSDate new];
+  Foo *bar = [Foo new];
+
+Instead, code should use +alloc/-init or class factory methods.
+
+.. code-block:: objc
+
+  NSDate *now = [NSDate date];
+  Foo *bar = [[Foo alloc] init];
+
+The corresponding style guide rule:
+https://google.github.io/styleguide/objcguide.html#do-not-use-new
Index: clang-tools-extra/docs/ReleaseNotes.rst

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/AST/Decl.cpp:738
+  (isa(D) && D->hasAttr()) ||
+  (isa(D) && D->hasAttr())) {
+Visibility Vis = LV.getVisibility();

we also need this for `__constant__` variables.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61274



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

[PATCH] D61349: [clangd] Standard library mapping: prefer "primary" versions of functions over variants.

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61349

Files:
  clangd/StdSymbolMap.inc
  clangd/include-mapping/gen_std.py
  clangd/include-mapping/test.py

Index: clangd/include-mapping/test.py
===
--- clangd/include-mapping/test.py
+++ clangd/include-mapping/test.py
@@ -24,16 +24,17 @@
 
 actual = ParseIndexPage(html)
 expected = [
-  ("abs", "abs.html"),
-  ("abs", "complex/abs.html"),
-  ("acos", "acos.html"),
-  ("acosh", "acosh.html"),
-  ("as_bytes", "as_bytes.html"),
+  ("abs", "abs.html", True),
+  ("abs", "complex/abs.html", True),
+  ("acos", "acos.html", False),
+  ("acosh", "acosh.html", False),
+  ("as_bytes", "as_bytes.html", False),
 ]
 self.assertEqual(len(actual), len(expected))
 for i in range(0, len(actual)):
   self.assertEqual(expected[i][0], actual[i][0])
   self.assertTrue(actual[i][1].endswith(expected[i][1]))
+  self.assertEqual(expected[i][2], actual[i][2])
 
 
   def testParseSymbolPage_SingleHeader(self):
Index: clangd/include-mapping/gen_std.py
===
--- clangd/include-mapping/gen_std.py
+++ clangd/include-mapping/gen_std.py
@@ -28,7 +28,7 @@
gen_std.py -cppreference  > StdSymbolMap.inc
 """
 
-from bs4 import BeautifulSoup
+from bs4 import BeautifulSoup, NavigableString
 
 import argparse
 import collections
@@ -82,15 +82,17 @@
   abs() (int) 
   acos() 
 
-  Returns a list of tuple (symbol_name, relative_path_to_symbol_page).
+  Returns a list of tuple (symbol_name, relative_path_to_symbol_page, variant).
   """
   symbols = []
   soup = BeautifulSoup(index_page_html, "html.parser")
   for symbol_href in soup.select("a[title]"):
+caption = symbol_href.next_sibling
+variant = isinstance(caption, NavigableString) and "(" in caption
 symbol_tt = symbol_href.find("tt")
 if symbol_tt:
   symbols.append((symbol_tt.text.rstrip("<>()"), # strip any trailing <>()
-  symbol_href["href"]))
+  symbol_href["href"], variant))
   return symbols
 
 class Symbol:
@@ -125,7 +127,11 @@
   with open(index_page_path, "r") as f:
 # Read each symbol page in parallel.
 results = [] # (symbol_name, promise of [header...])
-for symbol_name, symbol_page_path in ParseIndexPage(f.read()):
+for symbol_name, symbol_page_path, variant in ParseIndexPage(f.read()):
+  # Variant symbols (e.g. the std::locale version of isalpha) add ambiguity.
+  # FIXME: use these as a fallback rather than ignoring entirely.
+  if variant:
+continue
   path = os.path.join(root_dir, symbol_page_path)
   results.append((symbol_name,
   pool.apply_async(ReadSymbolPage, (path, symbol_name
Index: clangd/StdSymbolMap.inc
===
--- clangd/StdSymbolMap.inc
+++ clangd/StdSymbolMap.inc
@@ -42,6 +42,8 @@
 SYMBOL(UnsignedIntegral, std::, )
 SYMBOL(_Exit, std::, )
 SYMBOL(accumulate, std::, )
+SYMBOL(acos, std::, )
+SYMBOL(acosh, std::, )
 SYMBOL(add_const, std::, )
 SYMBOL(add_const_t, std::, )
 SYMBOL(add_cv, std::, )
@@ -82,8 +84,13 @@
 SYMBOL(array, std::, )
 SYMBOL(as_const, std::, )
 SYMBOL(asctime, std::, )
+SYMBOL(asin, std::, )
+SYMBOL(asinh, std::, )
 SYMBOL(async, std::, )
 SYMBOL(at_quick_exit, std::, )
+SYMBOL(atan, std::, )
+SYMBOL(atan2, std::, )
+SYMBOL(atanh, std::, )
 SYMBOL(atexit, std::, )
 SYMBOL(atof, std::, )
 SYMBOL(atoi, std::, )
@@ -220,6 +227,8 @@
 SYMBOL(copy_if, std::, )
 SYMBOL(copy_n, std::, )
 SYMBOL(copysign, std::, )
+SYMBOL(cos, std::, )
+SYMBOL(cosh, std::, )
 SYMBOL(count, std::, )
 SYMBOL(count_if, std::, )
 SYMBOL(cout, std::, )
@@ -292,6 +301,7 @@
 SYMBOL(exchange, std::, )
 SYMBOL(exclusive_scan, std::, )
 SYMBOL(exit, std::, )
+SYMBOL(exp, std::, )
 SYMBOL(exp2, std::, )
 SYMBOL(expm1, std::, )
 SYMBOL(exponential_distribution, std::, )
@@ -609,20 +619,31 @@
 SYMBOL(is_void_v, std::, )
 SYMBOL(is_volatile, std::, )
 SYMBOL(is_volatile_v, std::, )
+SYMBOL(isalnum, std::, )
+SYMBOL(isalpha, std::, )
+SYMBOL(isblank, std::, )
+SYMBOL(iscntrl, std::, )
+SYMBOL(isdigit, std::, )
 SYMBOL(isfinite, std::, )
+SYMBOL(isgraph, std::, )
 SYMBOL(isgreater, std::, )
 SYMBOL(isgreaterequal, std::, )
 SYMBOL(isinf, std::, )
 SYMBOL(isless, std::, )
 SYMBOL(islessequal, std::, )
 SYMBOL(islessgreater, std::, )
+SYMBOL(islower, std::, )
 SYMBOL(isnan, std::, )
 SYMBOL(isnormal, std::, )
 SYMBOL(ispow2, std::, )
+SYMBOL(isprint, std::, )
+SYMBOL(ispunct, std::, )
+SYMBOL(isspace, std::, )
 SYMBOL(istream_iterator, std::, )
 SYMBOL(istreambuf_iterator, std::, )
 SYMBOL(istringstream, std::, )
 SYMBOL(isunordered, std::, )

r359623 - Remove two unnecessary wrappers of canPassInRegisters

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Apr 30 15:23:20 2019
New Revision: 359623

URL: http://llvm.org/viewvc/llvm-project?rev=359623=rev
Log:
Remove two unnecessary wrappers of canPassInRegisters

These extra layers aren't necessary.

Modified:
cfe/trunk/lib/CodeGen/CGCXXABI.cpp
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=359623=359622=359623=diff
==
--- cfe/trunk/lib/CodeGen/CGCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.cpp Tue Apr 30 15:23:20 2019
@@ -28,12 +28,6 @@ void CGCXXABI::ErrorUnsupportedABI(CodeG
 << S;
 }
 
-bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const {
-  // We can only copy the argument if there exists at least one trivial,
-  // non-deleted copy or move constructor.
-  return RD->canPassInRegisters();
-}
-
 llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {
   return llvm::Constant::getNullValue(CGM.getTypes().ConvertType(T));
 }

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=359623=359622=359623=diff
==
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Tue Apr 30 15:23:20 2019
@@ -136,10 +136,6 @@ public:
 RAA_Indirect
   };
 
-  /// Returns true if C++ allows us to copy the memory of an object of type RD
-  /// when it is passed as an argument.
-  bool canCopyArgument(const CXXRecordDecl *RD) const;
-
   /// Returns how an argument of the given record type should be passed.
   virtual RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const = 0;
 

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=359623=359622=359623=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue Apr 30 15:23:20 2019
@@ -62,13 +62,9 @@ public:
 
   bool classifyReturnType(CGFunctionInfo ) const override;
 
-  bool passClassIndirect(const CXXRecordDecl *RD) const {
-return !canCopyArgument(RD);
-  }
-
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
 // If C++ prohibits us from making a copy, pass by address.
-if (passClassIndirect(RD))
+if (!RD->canPassInRegisters())
   return RAA_Indirect;
 return RAA_Default;
   }
@@ -1093,7 +1089,7 @@ bool ItaniumCXXABI::classifyReturnType(C
 return false;
 
   // If C++ prohibits us from making a copy, return by address.
-  if (passClassIndirect(RD)) {
+  if (!RD->canPassInRegisters()) {
 auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
 FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
 return true;

Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=359623=359622=359623=diff
==
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Tue Apr 30 15:23:20 2019
@@ -811,7 +811,7 @@ MicrosoftCXXABI::getRecordArgABI(const C
 // Use the simple Itanium rules for now.
 // FIXME: This is incompatible with MSVC for arguments with a dtor and no
 // copy ctor.
-return !canCopyArgument(RD) ? RAA_Indirect : RAA_Default;
+return !RD->canPassInRegisters() ? RAA_Indirect : RAA_Default;
 
   case llvm::Triple::x86:
 // All record arguments are passed in memory on x86.  Decide whether to
@@ -820,7 +820,7 @@ MicrosoftCXXABI::getRecordArgABI(const C
 
 // If C++ prohibits us from making a copy, construct the arguments directly
 // into argument memory.
-if (!canCopyArgument(RD))
+if (!RD->canPassInRegisters())
   return RAA_DirectInMemory;
 
 // Otherwise, construct the argument into a temporary and copy the bytes
@@ -829,7 +829,7 @@ MicrosoftCXXABI::getRecordArgABI(const C
 
   case llvm::Triple::x86_64:
   case llvm::Triple::aarch64:
-return !canCopyArgument(RD) ? RAA_Indirect : RAA_Default;
+return !RD->canPassInRegisters() ? RAA_Indirect : RAA_Default;
   }
 
   llvm_unreachable("invalid enum");


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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:55-56
 
+  bool passClassIndirect(const CXXRecordDecl *RD) const {
+return !canCopyArgument(RD);
+  }

These are both trivial wrappers for `RD->canPassInRegisters()` which has the 
real logic. I would just call that to remove some indirection, even if the name 
is the logical opposite of the property we're trying to check.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1079
+return true;
+  for (auto it = RD->ctor_begin(); it != RD->ctor_end(); ++it) {
+if (it->isUserProvided())

Can this be just: `for (const CXXConstructorDecl *Ctor : RD->ctors())`?


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

https://reviews.llvm.org/D60349



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1484744 , @jakehehrlich 
wrote:

> > Does llvm-elfabi consume your proposed Schema format? Has it landed yet?
>
> No, I just proposed it and explained my reasoning. If you wanted to add this 
> format to TextAPI that would be acceptable.


I'm a little bit confused. At what point will llvm-elfabi be capable of 
generating a binary object/shared object file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 197453.
erik.pilkington added a comment.

Address review comments.


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

https://reviews.llvm.org/D61165

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/no_destroy.cpp
  clang/test/SemaCXX/no_destroy.cpp

Index: clang/test/SemaCXX/no_destroy.cpp
===
--- clang/test/SemaCXX/no_destroy.cpp
+++ clang/test/SemaCXX/no_destroy.cpp
@@ -1,11 +1,13 @@
-// RUN: %clang_cc1 -DNO_DTORS -fno-c++-static-destructors -verify %s
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -DNO_DTORS -DNO_EXCEPTIONS -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -DNO_EXCEPTIONS -verify %s
+// RUN: %clang_cc1 -DNO_DTORS -fexceptions -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -fexceptions -verify %s
 
 struct SecretDestructor {
 #ifndef NO_DTORS
   // expected-note@+2 4 {{private}}
 #endif
-private: ~SecretDestructor(); // expected-note 2 {{private}}
+private: ~SecretDestructor(); // expected-note + {{private}}
 };
 
 SecretDestructor sd1;
@@ -44,3 +46,36 @@
 
 [[clang::no_destroy(0)]] int no_args; // expected-error{{'no_destroy' attribute takes no arguments}}
 [[clang::always_destroy(0)]] int no_args2; // expected-error{{'always_destroy' attribute takes no arguments}}
+
+SecretDestructor arr[10];
+#ifndef NO_DTORS
+// expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+
+void local_arrays() {
+  static SecretDestructor arr2[10];
+#if !defined(NO_DTORS) || !defined(NO_EXCEPTIONS)
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+  thread_local SecretDestructor arr3[10];
+#if !defined(NO_DTORS) || !defined(NO_EXCEPTIONS)
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+}
+
+struct Base {
+  ~Base();
+};
+struct Derived1 {
+  Derived1(int);
+  Base b;
+};
+struct Derived2 {
+  Derived1 b;
+};
+
+void dontcrash() {
+  [[clang::no_destroy]] static Derived2 d2[] = {0, 0};
+}
+
+[[clang::no_destroy]] Derived2 d2[] = {0, 0};
Index: clang/test/CodeGenCXX/no_destroy.cpp
===
--- clang/test/CodeGenCXX/no_destroy.cpp
+++ clang/test/CodeGenCXX/no_destroy.cpp
@@ -1,11 +1,14 @@
-// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,NO_EXCEPTIONS
+// RUN: %clang_cc1 -fexceptions %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,EXCEPTIONS
 
 struct NonTrivial {
   ~NonTrivial();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] NonTrivial nt1;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] thread_local NonTrivial nt2;
 
@@ -13,11 +16,14 @@
   ~NonTrivial2();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
 NonTrivial2 nt21;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN11NonTrivial2D1Ev
 thread_local NonTrivial2 nt22;
 
+// CHECK-LABEL: define void @_Z1fv
 void f() {
   // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
   static NonTrivial2 nt21;
@@ -25,7 +31,50 @@
   thread_local NonTrivial2 nt22;
 }
 
+// CHECK-LABEL: define void @_Z1gv
+void g() {
+  // CHECK-NOT: __cxa_atexit
+  [[clang::no_destroy]] static NonTrivial2 nt21;
+  // CHECK-NOT: _tlv_atexit
+  [[clang::no_destroy]] thread_local NonTrivial2 nt22;
+}
+
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] NonTrivial nt3;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] thread_local NonTrivial nt4;
+
+
+struct NonTrivial3 {
+  NonTrivial3();
+  ~NonTrivial3();
+};
+
+[[clang::no_destroy]] NonTrivial3 arr[10];
+
+// CHECK-LABEL: define internal void @__cxx_global_var_init
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// CHECK-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
+
+void h() {
+  [[clang::no_destroy]] static NonTrivial3 slarr[10];
+}
+
+// CHECK-LABEL: define void @_Z1hv
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// EXCEPTIONS: call void @_ZN11NonTrivial3D1Ev
+// NO_EXCEPTIONS-NOT: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
+
+void i() {
+  [[clang::no_destroy]] thread_local NonTrivial3 tlarr[10];
+}
+
+// 

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:257-262
+  bool AttrStartIsInMacro =
+  (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion(
+   StartLoc, SrcMgr, PP.getLangOpts()));
+  bool AttrEndIsInMacro =
+  (EndLoc.isMacroID() &&
+   Lexer::isAtEndOfMacroExpansion(EndLoc, SrcMgr, PP.getLangOpts()));

rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > leonardchan wrote:
> > > > rsmith wrote:
> > > > > I think these checks need to be moved to the last loop of 
> > > > > `FindLocsWithCommonFileID`. Otherwise for a case like:
> > > > > 
> > > > > ```
> > > > > #define THING \
> > > > >   int *p = nullptr;
> > > > >   AS1 int *q = nullptr;
> > > > > 
> > > > > THING
> > > > > ```
> > > > > 
> > > > > ... `FindLocsWithCommonFileID` will pick out the `THING` macro and 
> > > > > return the range from the `int` token to the second semicolon, and 
> > > > > the checks here will fail. Instead, we should pick out the inner 
> > > > > `AS1` expansion, because it's the outermost macro expansion that 
> > > > > starts with `StartLoc` and ends with `EndLoc`.
> > > > Moved, although this doesn't appear to fix this case. On closer 
> > > > inspection, when generating the vector of starting locations, the 
> > > > expansion location for `AS1` doesn't seem to be returned by 
> > > > `getExpansionLocStart`. It goes straight from the location of the 
> > > > `__attribute__` behind `AS1` to `THING` and skips `AS1`. I found this 
> > > > out by just dumping `StartLoc` on every iteration.
> > > > 
> > > > The only way I can generate the location for `AS1` in `THING` is by 
> > > > also watching for the spelling location of each expansion, but this 
> > > > SourceLoc is not a macro ID and would not fulfill these last 2 checks. 
> > > > Is this intended? If it's a bug I don't mind addressing it if you point 
> > > > me in the right direction to start.
> > > Right, sorry, mistake on my part. To deal with this, you need to call 
> > > `isAt{Start,End}OfMacroExpansion` while collecting `StartLocs` and 
> > > `EndLocs`, and stop once you hit a location where they return `false`. 
> > > (That would mean that you never step from the location of `AS1` out to 
> > > the location of `THING`.)
> > Hmm. So I still run into the problem of none of the locations being at the 
> > start or end of the macro expansion. In your example, I only find 2 source 
> > locations at all. Let's say I have:
> > 
> > ```
> > 1 #define AS1 __attribute__((address_space(1)))
> > 2
> > 3 int main() {
> > 4 #define THING \
> > 5   int *p = 0; \
> > 6   AS1 int *q = p;
> > 7
> > 8   THING;
> > 9 }
> > ```
> > 
> > I only see the following expansion source locations:
> > 
> > ```
> > /usr/local/google/home/leonardchan/misc/test.c:8:3 
> > 
> > /usr/local/google/home/leonardchan/misc/test.c:8:3 
> > 
> > ```
> > 
> > And it seems none of them return true for `isAtStartOfMacroExpansion` since 
> > the `__attribute__` keyword isn't the first token of `THING`. I imagine 
> > stopping early would work if the 2nd expansion location instead referred to 
> > `AS1` (on line 6 col 3), but it doesn't seem to be the case here.
> > 
> > I can also see similar results for other examples where the macro is nested 
> > but does not encapsulate the whole macro:
> > 
> > ```
> > #define THING2 int AS1 *q2 = p2;
> > int *p2;
> > THING2;  // Error doesn't print AS1
> > ```
> > 
> > For our case, it seems like the correct thing to do is to get the spelling 
> > location and default to it if the macro itself doesn't contain the whole 
> > attribute. I updated and renamed this function to account for this and we 
> > can now see `AS1` printed. Also added test cases for this.
> > 
> > For cases like `#define AS2 AS1`, `AS1` is still printed, but this is 
> > intended since `AS1` is more immediate of an expansion than `AS2`.
> I'd like to get some (correct) form of this landed, so I wonder if we can do 
> something much simpler for now. Can we just check that the first token of the 
> attribute-specifier `isAtStartOfMacroExpansion` and the last token 
> `isAtEndOfMacroExpansion` and that they're from the same expansion of the 
> same (object-like) macro? That won't find the outermost such macro, nor will 
> it deal with cases where `__attribute__` or the last `)` was itself generated 
> by a macro, but it should not have any false positives.
No problem. Done, and our original test cases still pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 197454.
leonardchan marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Frontend/macro_defined_type.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaObjC/externally-retained.m
  clang/test/SemaObjC/gc-attributes.m
  clang/test/SemaObjC/mrc-weak.m
  clang/test/SemaObjCXX/gc-attributes.mm
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1614,6 +1614,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroQualifiedTypeLoc(MacroQualifiedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaObjCXX/gc-attributes.mm
===
--- clang/test/SemaObjCXX/gc-attributes.mm
+++ clang/test/SemaObjCXX/gc-attributes.mm
@@ -3,7 +3,7 @@
 @interface A
 @end
 
-void f0(__strong A**); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
+void f0(__strong A **); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
 
 void test_f0() {
   A *a;
@@ -12,7 +12,7 @@
   f0(); // expected-error{{no matching function}}
 }
 
-void f1(__weak A**); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
+void f1(__weak A **); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
 
 void test_f1() {
   A *a;
Index: clang/test/SemaObjC/mrc-weak.m
===
--- clang/test/SemaObjC/mrc-weak.m
+++ clang/test/SemaObjC/mrc-weak.m
@@ -62,6 +62,6 @@
 
 void test_cast_qualifier_inference(__weak id *value) {
   __weak id *a = (id*) value;
-  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing 'id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
+  __unsafe_unretained id *b = (id *)value; // expected-error {{initializing '__unsafe_unretained id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
 }
 
Index: clang/test/SemaObjC/gc-attributes.m
===
--- clang/test/SemaObjC/gc-attributes.m
+++ clang/test/SemaObjC/gc-attributes.m
@@ -9,7 +9,7 @@
   A *a;
   static __weak A *a2;
   f0();
-  f0(); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}} 
+  f0(); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}}
 }
 
 void f1(__weak A**); // expected-note{{passing argument to parameter here}}
@@ -18,7 +18,7 @@
   A *a;
   __strong A *a2;
   f1();
-  f1(); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}} 
+  f1(); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}}
 }
 
 // These qualifiers should silently expand to nothing in GC mode.
Index: clang/test/SemaObjC/externally-retained.m
===
--- clang/test/SemaObjC/externally-retained.m
+++ clang/test/SemaObjC/externally-retained.m
@@ -68,6 +68,12 @@
   second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
 };
 
+void (^blk2)(ObjCTy *, ObjCTy *) =
+^(__strong ObjCTy *first, ObjCTy *second) __attribute__((objc_externally_retained)) {
+  first = 0;
+  

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
 

richard.townsend.arm wrote:
> efriedma wrote:
> > richard.townsend.arm wrote:
> > > I'm not sure what the IsSizeGreaterThan128 check is doing here - if the 
> > > return type is over 128 bits, then it will be indirectly returned in X8 
> > > with this check, which is not always what we want (e.g. in 
> > > https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects the 
> > > value returned in X1). Another check of hasMicrosoftABIRestrictions might 
> > > be OK, but that's also not quite right due to the size check. 
> > I think the check here is intended to counteract the similar check in 
> > hasMicrosoftABIRestrictions... but yes, I don't think that works correctly. 
> >  Looks like we're missing a testcase for a non-aggregate type with size 
> > greater than 128 bits.
> Ah, OK. I think the problem is that we need to check whether the return value 
> meets the rest of aggregate definition (i.e. !hasMicrosoftABIRestrictions), 
> as well as being more than 128 bits in size. May be worth splitting the logic 
> up. Will experiment.
I think you might get the right behavior by just erasing both 
IsSizeGreaterThan128 checks.  That matches the way the ABI document describes 
the rules.  Haven't tried it, though.


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

https://reviews.llvm.org/D60349



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

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Having in bitcode something like `@llvm.x86.encls.64` is better than inline 
assembly because we understand the meaning of the bitcode while we don't parse 
assembly and have a very limited understanding of what it is doing.

The use case we need to support is basically

  --- a/clang/test/Modules/compiler_builtins_x86.c
  +++ b/clang/test/Modules/compiler_builtins_x86.c
  @@ -1,6 +1,7 @@
   // RUN: rm -rf %t
   // RUN: %clang_cc1 -triple i686-unknown-unknown -fsyntax-only -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t %s -verify -ffreestanding
   // RUN: %clang_cc1 -triple i686-unknown-unknown -fsyntax-only -fmodules 
-fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s 
-verify -ffreestanding
  +// RUN: %clang_cc1 -triple i686-unknown-unknown -fsyntax-only -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t %s -verify -ffreestanding 
-fno-gnu-inline-asm
   // expected-no-diagnostics
   
   #include

And this case isn't affected by cpuid, so we don't plan to change that as well 
as Windows-specific assembly in immintrin.h.

While writing the response, it occurred to me that another solution might be 
changing `-fno-gnu-inline-asm` to cover only actually called code. If you don't 
call `_pconfig_u32`, it shouldn't matter that in the header it is implemented 
with inline assembly. But this is an early idea, I need to discuss with the 
team if it will work and if there are other constraints I'm missing.


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

https://reviews.llvm.org/D44387



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

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

xbolva00 added inline comments.



Comment at: clang/include/clang/AST/DeclBase.h:1534
 
 /// 25 bits to fit in the remaining availible space.
 /// Note that this makes CXXConstructorDeclBitfields take

Small typo + update comment


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

https://reviews.llvm.org/D60934



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
 

efriedma wrote:
> richard.townsend.arm wrote:
> > I'm not sure what the IsSizeGreaterThan128 check is doing here - if the 
> > return type is over 128 bits, then it will be indirectly returned in X8 
> > with this check, which is not always what we want (e.g. in 
> > https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects the 
> > value returned in X1). Another check of hasMicrosoftABIRestrictions might 
> > be OK, but that's also not quite right due to the size check. 
> I think the check here is intended to counteract the similar check in 
> hasMicrosoftABIRestrictions... but yes, I don't think that works correctly.  
> Looks like we're missing a testcase for a non-aggregate type with size 
> greater than 128 bits.
Ah, OK. I think the problem is that we need to check whether the return value 
meets the rest of aggregate definition (i.e. !hasMicrosoftABIRestrictions), as 
well as being more than 128 bits in size. May be worth splitting the logic up. 
Will experiment.


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

https://reviews.llvm.org/D60349



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
 

richard.townsend.arm wrote:
> I'm not sure what the IsSizeGreaterThan128 check is doing here - if the 
> return type is over 128 bits, then it will be indirectly returned in X8 with 
> this check, which is not always what we want (e.g. in 
> https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects the 
> value returned in X1). Another check of hasMicrosoftABIRestrictions might be 
> OK, but that's also not quite right due to the size check. 
I think the check here is intended to counteract the similar check in 
hasMicrosoftABIRestrictions... but yes, I don't think that works correctly.  
Looks like we're missing a testcase for a non-aggregate type with size greater 
than 128 bits.


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

https://reviews.llvm.org/D60349



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

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

rsmith added a comment.

This is great, thank you so much!




Comment at: clang/include/clang/AST/DeclBase.h:1539-1541
+uint64_t NumCtorInitializers : 64 - NumDeclContextBits -
+NumFunctionDeclBits -
+/*Other used bits in CXXConstructorDecl*/ 3;

Tyker wrote:
> rsmith wrote:
> > I would prefer that we keep an explicit number here so that we can ensure 
> > that this field has the range we desire.
> couldn't we compute the value and static_assert on it. having to modify this 
> each time we modify DeclContextBits or FunctionDeclBits is sad. and there 
> isn't anything reminding us to do so in some cases.
> what would be a reasonable minimum ?
I think it'd be reasonable to reduce this to 20 if you like. (Going down as far 
as 16 or so seems acceptable, but I'd be worried about generated code having 
thousands of members, so I'd be hesitant to go below that.) The existing 
`static_assert` on line ~1721 will then catch if we add too many bit-field 
members.



Comment at: clang/include/clang/AST/DeclCXX.h:2604
 
-  static CXXConstructorDecl *CreateDeserialized(ASTContext , unsigned ID,
-bool InheritsConstructor);
+  static CXXConstructorDecl *CreateDeserialized(ASTContext , unsigned ID, 
uint64_t AllocKind);
   static CXXConstructorDecl *

Please wrap this to 80 columns.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6174
 CXXConversionDecl)) {
+  // FIXEME : it's not clear whether this should match a dependent 
explicit()
   return Node.isExplicit();

Typo "FIXEME" -> "FIXME"



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6174
 CXXConversionDecl)) {
-  return Node.isExplicit();
+  return Node.getExplicitSpecifier().isSpecified();
 }

Tyker wrote:
> rsmith wrote:
> > This will match `explicit(false)` constructors. I think 
> > `getExplicitSpecifier().isExplicit()` would be better, but please also add 
> > a FIXME here: it's not clear whether this should match a dependent 
> > `explicit()`.
> shouldn't this be able to match with deduction guides too ?
Yes, it should. This patch is doing a lot of things already, though, so I'd 
prefer that we fix that as a separate change.



Comment at: clang/lib/Parse/ParseDecl.cpp:3906
   } else
 Diag(Tok, DiagID) << PrevSpec;
 }

These diagnostics need to be updated to use a different location (rather than 
using `Tok`) if `isAlreadyConsumed`.



Comment at: clang/lib/Sema/SemaInit.cpp:9361
 // Only consider converting constructors.
-if (GD->isExplicit())
+if (!GD->isMaybeNotExplicit())
   continue;

Tyker wrote:
> rsmith wrote:
> > Tyker wrote:
> > > rsmith wrote:
> > > > We need to substitute into the deduction guide first to detect whether 
> > > > it forms a "converting constructor", and that will need to be done 
> > > > inside `AddTemplateOverloadCandidate`.
> > > similarly as the previous if. this check removes deduction guide that are 
> > > already resolve to be explicit when we are in a context that doesn't 
> > > allow explicit.
> > > every time the explicitness was checked before my change i replaced it by 
> > > a check that removes already resolved explicit specifiers.
> > Unlike in the previous case, we do //not// yet pass an `AllowExplicit` flag 
> > into `AddTemplateOverloadCandidate` here, so this will incorrectly allow 
> > dependent //explicit-specifier//s that evaluate to `true` in 
> > copy-initialization contexts.
> the default value for `AllowExplicit` is false. so the conversion will be 
> removed in `AddTemplateOverloadCandidate`.
That doesn't sound right: that'd mean we never use explicit deduction guides 
(we never pass `AllowExplicit = true` to `AddTemplateOverloadCandidate`). Do we 
have any test coverage that demonstrates that conditionally-explicit deduction 
guides are handled properly?



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2076
+Method = CXXConstructorDecl::Create(
+SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, 
InstantiatedExplicitSpecifier,
+Constructor->isInlineSpecified(), false, Constructor->isConstexpr());

Please reflow to 80 columns.


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

https://reviews.llvm.org/D60934



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

[PATCH] D61345: Allow 'CodeGenObjC/illegal-UTF8.m' test for 32-bit targets.

[Vlad Vereschaka via Phabricator via cfe-commits]

vvereschaka created this revision.
vvereschaka added a project: clang.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.

The 'CodeGenObjC/illegal-UTF8.m' get failed with Clang built with 32-bit 
targets only (as example ARM-only) with the following error:

error: unable to create target: 'No available targets are compatible with 
triple "< ... any 64-bit target triple ... >"'

I didn't find any 64-bit dependencies for the test and I think removing '-m64' 
option should fix the problem and allow this test for any target specified by 
LLVM_DEFAULT_TARGET_TRIPLE.


Repository:
  rC Clang

https://reviews.llvm.org/D61345

Files:
  test/CodeGenObjC/illegal-UTF8.m


Index: test/CodeGenObjC/illegal-UTF8.m
===
--- test/CodeGenObjC/illegal-UTF8.m
+++ test/CodeGenObjC/illegal-UTF8.m
@@ -1,4 +1,4 @@
-// RUN: %clang %s -S -m64 -o -
+// RUN: %clang %s -S -o -
 
 @class NSString;
 


Index: test/CodeGenObjC/illegal-UTF8.m
===
--- test/CodeGenObjC/illegal-UTF8.m
+++ test/CodeGenObjC/illegal-UTF8.m
@@ -1,4 +1,4 @@
-// RUN: %clang %s -S -m64 -o -
+// RUN: %clang %s -S -o -
 
 @class NSString;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61142: Set LoopInterleaved in the PassManagerBuilder.

[Alina Sbirlea via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359616: Set LoopInterleaved in the PassManagerBuilder. 
(authored by asbirlea, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61142?vs=196696=197445#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61142

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -560,6 +560,9 @@
   PMBuilder.LoopVectorize = CodeGenOpts.VectorizeLoop;
 
   PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops;
+  // Loop interleaving in the loop vectorizer has historically been set to be
+  // enabled when loop unrolling is enabled.
+  PMBuilder.LoopsInterleaved = CodeGenOpts.UnrollLoops;
   PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
   PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -560,6 +560,9 @@
   PMBuilder.LoopVectorize = CodeGenOpts.VectorizeLoop;
 
   PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops;
+  // Loop interleaving in the loop vectorizer has historically been set to be
+  // enabled when loop unrolling is enabled.
+  PMBuilder.LoopsInterleaved = CodeGenOpts.UnrollLoops;
   PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
   PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r359616 - Set LoopInterleaved in the PassManagerBuilder.

[Alina Sbirlea via cfe-commits]

Author: asbirlea
Date: Tue Apr 30 14:29:23 2019
New Revision: 359616

URL: http://llvm.org/viewvc/llvm-project?rev=359616=rev
Log:
Set LoopInterleaved in the PassManagerBuilder.

Summary: Corresponds to D61030.

Subscribers: jlebar, cfe-commits, llvm-commits

Tags: #clang

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=359616=359615=359616=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Apr 30 14:29:23 2019
@@ -560,6 +560,9 @@ void EmitAssemblyHelper::CreatePasses(le
   PMBuilder.LoopVectorize = CodeGenOpts.VectorizeLoop;
 
   PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops;
+  // Loop interleaving in the loop vectorizer has historically been set to be
+  // enabled when loop unrolling is enabled.
+  PMBuilder.LoopsInterleaved = CodeGenOpts.UnrollLoops;
   PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
   PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;


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

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

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

rsmith added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:257-262
+  bool AttrStartIsInMacro =
+  (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion(
+   StartLoc, SrcMgr, PP.getLangOpts()));
+  bool AttrEndIsInMacro =
+  (EndLoc.isMacroID() &&
+   Lexer::isAtEndOfMacroExpansion(EndLoc, SrcMgr, PP.getLangOpts()));

leonardchan wrote:
> rsmith wrote:
> > leonardchan wrote:
> > > rsmith wrote:
> > > > I think these checks need to be moved to the last loop of 
> > > > `FindLocsWithCommonFileID`. Otherwise for a case like:
> > > > 
> > > > ```
> > > > #define THING \
> > > >   int *p = nullptr;
> > > >   AS1 int *q = nullptr;
> > > > 
> > > > THING
> > > > ```
> > > > 
> > > > ... `FindLocsWithCommonFileID` will pick out the `THING` macro and 
> > > > return the range from the `int` token to the second semicolon, and the 
> > > > checks here will fail. Instead, we should pick out the inner `AS1` 
> > > > expansion, because it's the outermost macro expansion that starts with 
> > > > `StartLoc` and ends with `EndLoc`.
> > > Moved, although this doesn't appear to fix this case. On closer 
> > > inspection, when generating the vector of starting locations, the 
> > > expansion location for `AS1` doesn't seem to be returned by 
> > > `getExpansionLocStart`. It goes straight from the location of the 
> > > `__attribute__` behind `AS1` to `THING` and skips `AS1`. I found this out 
> > > by just dumping `StartLoc` on every iteration.
> > > 
> > > The only way I can generate the location for `AS1` in `THING` is by also 
> > > watching for the spelling location of each expansion, but this SourceLoc 
> > > is not a macro ID and would not fulfill these last 2 checks. Is this 
> > > intended? If it's a bug I don't mind addressing it if you point me in the 
> > > right direction to start.
> > Right, sorry, mistake on my part. To deal with this, you need to call 
> > `isAt{Start,End}OfMacroExpansion` while collecting `StartLocs` and 
> > `EndLocs`, and stop once you hit a location where they return `false`. 
> > (That would mean that you never step from the location of `AS1` out to the 
> > location of `THING`.)
> Hmm. So I still run into the problem of none of the locations being at the 
> start or end of the macro expansion. In your example, I only find 2 source 
> locations at all. Let's say I have:
> 
> ```
> 1 #define AS1 __attribute__((address_space(1)))
> 2
> 3 int main() {
> 4 #define THING \
> 5   int *p = 0; \
> 6   AS1 int *q = p;
> 7
> 8   THING;
> 9 }
> ```
> 
> I only see the following expansion source locations:
> 
> ```
> /usr/local/google/home/leonardchan/misc/test.c:8:3 
> 
> /usr/local/google/home/leonardchan/misc/test.c:8:3 
> 
> ```
> 
> And it seems none of them return true for `isAtStartOfMacroExpansion` since 
> the `__attribute__` keyword isn't the first token of `THING`. I imagine 
> stopping early would work if the 2nd expansion location instead referred to 
> `AS1` (on line 6 col 3), but it doesn't seem to be the case here.
> 
> I can also see similar results for other examples where the macro is nested 
> but does not encapsulate the whole macro:
> 
> ```
> #define THING2 int AS1 *q2 = p2;
> int *p2;
> THING2;  // Error doesn't print AS1
> ```
> 
> For our case, it seems like the correct thing to do is to get the spelling 
> location and default to it if the macro itself doesn't contain the whole 
> attribute. I updated and renamed this function to account for this and we can 
> now see `AS1` printed. Also added test cases for this.
> 
> For cases like `#define AS2 AS1`, `AS1` is still printed, but this is 
> intended since `AS1` is more immediate of an expansion than `AS2`.
I'd like to get some (correct) form of this landed, so I wonder if we can do 
something much simpler for now. Can we just check that the first token of the 
attribute-specifier `isAtStartOfMacroExpansion` and the last token 
`isAtEndOfMacroExpansion` and that they're from the same expansion of the same 
(object-like) macro? That won't find the outermost such macro, nor will it deal 
with cases where `__attribute__` or the last `)` was itself generated by a 
macro, but it should not have any false positives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 197436.

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

https://reviews.llvm.org/D60349

Files:
  include/clang/AST/DeclCXX.h
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeGen/arm64-microsoft-arguments.cpp
  test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Index: test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===
--- test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -69,6 +69,11 @@
   int bb;
 };
 
+struct SmallWithPrivate {
+private:
+ int i;
+};
+
 // WIN32: declare dso_local void @"{{.*take_bools_and_chars.*}}"
 // WIN32:   (<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor,
 // WIN32:   i8, [3 x i8], i8, [3 x i8], i32, i8, [3 x i8] }>* inalloca)
@@ -165,7 +170,7 @@
 // WIN64:   call void @"??1SmallWithDtor@@QEAA@XZ"
 // WIN64: }
 // WOA64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(i64 %s.coerce) {{.*}} {
-// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"
+// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA64: }
 
 // FIXME: MSVC incompatible!
@@ -173,6 +178,12 @@
 // WOA:   call arm_aapcs_vfpcc void @"??1SmallWithDtor@@QAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA: }
 
+
+// Test that the eligible non-aggregate is passed directly, but returned
+// indirectly on ARM64 Windows.
+// WOA64: define dso_local void @"?small_arg_with_private_member@@YA?AUSmallWithPrivate@@U1@@Z"(%struct.SmallWithPrivate* inreg noalias sret %agg.result, i64 %s.coerce) {{.*}} {
+SmallWithPrivate small_arg_with_private_member(SmallWithPrivate s) { return s; }
+
 void call_small_arg_with_dtor() {
   small_arg_with_dtor(SmallWithDtor());
 }
Index: test/CodeGen/arm64-microsoft-arguments.cpp
===
--- test/CodeGen/arm64-microsoft-arguments.cpp
+++ test/CodeGen/arm64-microsoft-arguments.cpp
@@ -1,25 +1,187 @@
 // RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
 // RUN: -x c++ -o - %s | FileCheck %s
 
-struct pod { int a, b, c, d, e; };
+// Pass and return for type size <= 8 bytes.
+// CHECK: define {{.*}} i64 @{{.*}}f1{{.*}}()
+// CHECK: call i64 {{.*}}func1{{.*}}(i64 %3)
+struct S1 {
+  int a[2];
+};
+
+S1 func1(S1 x);
+S1 f1() {
+  S1 x;
+  return func1(x);
+}
+
+// Pass and return type size <= 16 bytes.
+// CHECK: define {{.*}} [2 x i64] @{{.*}}f2{{.*}}()
+// CHECK: call [2 x i64] {{.*}}func2{{.*}}([2 x i64] %3)
+struct S2 {
+  int a[4];
+};
+
+S2 func2(S2 x);
+S2 f2() {
+  S2 x;
+  return func2(x);
+}
+
+// Pass and return for type size > 16 bytes.
+// CHECK: define {{.*}} void @{{.*}}f3{{.*}}(%struct.S3* noalias sret %agg.result)
+// CHECK: call void {{.*}}func3{{.*}}(%struct.S3* sret %agg.result, %struct.S3* %agg.tmp)
+struct S3 {
+  int a[5];
+};
+
+S3 func3(S3 x);
+S3 f3() {
+  S3 x;
+  return func3(x);
+}
+
+// Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
+// Passed directly but returned indirectly.
+// CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret %agg.result, [2 x i64] %4)
+struct S4 {
+  int a[3];
+  ~S4();
+};
+
+S4 func4(S4 x);
+S4 f4() {
+  S4 x;
+  return func4(x);
+}
+
+// Pass and return from instance method called from instance method.
+// CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* %this, %class.P1* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* %ref.tmp, %class.P1* inreg sret %agg.result, i8 %0)
+
+class P1 {
+public:
+  P1 foo(P1 x);
+};
+
+class Q1 {
+public:
+  P1 bar();
+};
+
+P1 Q1::bar() {
+  P1 p1;
+  return P1().foo(p1);
+}
+
+// Pass and return from instance method called from free function.
+// CHECK: define {{.*}} void {{.*}}bar{{.*}}()
+// CHECK: call void {{.*}}foo@P2{{.*}}(%class.P2* %ref.tmp, %class.P2* inreg sret %retval, i8 %0)
+class P2 {
+public:
+  P2 foo(P2 x);
+};
+
+P2 bar() {
+  P2 p2;
+  return P2().foo(p2);
+}
+
+// Pass and return an object with a user-provided constructor (passed directly,
+// returned indirectly)
+// CHECK: define {{.*}} void @{{.*}}f5{{.*}}(%struct.S5* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func5{{.*}}(%struct.S5* inreg sret %agg.result, i64 {{.*}})
+struct S5 {
+  S5();
+  int x;
+};
+
+S5 func5(S5 x);
+S5 f5() {
+  S5 x;
+  return func5(x);
+}
+
+// Pass and return an object with a non-trivial explicitly defaulted constructor
+// (passed directly, returned directly)
+// CHECK: define {{.*}} i64 @"?f6@@YA?AUS6@@XZ"()
+// CHECK: call i64 {{.*}}func6{{.*}}(i64 {{.*}})
+struct S6a {
+  S6a();
+};
+
+struct S6 {
+  S6() = default;
+  S6a x;
+};
 
-struct non_pod {
-  int a;
-  non_pod() {}
+S6 func6(S6 x);
+S6 f6() {
+  S6 x;
+  return func6(x);
+}
+

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Richard Townsend (Arm) via Phabricator via cfe-commits]

richard.townsend.arm added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
 

I'm not sure what the IsSizeGreaterThan128 check is doing here - if the return 
type is over 128 bits, then it will be indirectly returned in X8 with this 
check, which is not always what we want (e.g. in 
https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects the value 
returned in X1). Another check of hasMicrosoftABIRestrictions might be OK, but 
that's also not quite right due to the size check. 


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

https://reviews.llvm.org/D60349



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

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

rsmith added a comment.

I'd like to understand more about the intended use cases of this functionality. 
What information do clients want?




Comment at: clang/lib/Lex/Preprocessor.cpp:870-900
+  TokenSource Source;
   do {
+Source = TokenSource();
+
 switch (CurLexerKind) {
 case CLK_Lexer:
+  Source.InDirective = CurLexer->ParsingPreprocessorDirective;

This is a lot of extra stuff to be doing in the main `Lex` loop. Adding one 
(perfectly-predicted) branch on `OnToken` seems like it should be fine, but 
this seems like a bit more added complexity than I'd prefer. I'd like some 
performance measurements of `-cc1 -Eonly` to see if this makes any real 
difference.



Comment at: clang/lib/Lex/Preprocessor.cpp:896
 case CLK_LexAfterModuleImport:
-  ReturnedToken = LexAfterModuleImport(Result);
+  Source.InDirective = true;
+

This isn't a directive; these are normal tokens.



Comment at: clang/lib/Lex/Preprocessor.cpp:956-957
   --LexLevel;
+  if (OnToken)
+OnToken(Result, Source);
 }

This seems like it's going to be extremely hard to use. If you just want the 
expanded token stream, then removing all the other calls to `OnToken` and only 
calling it here when `LexLevel == 0` will give you that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885



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

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  So it sounds like this should either be a device-only rule, with no 
warning in mixed-mode languages like CUDA, or we should take a different 
approach.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61274



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D61280#1485115 , @rjmccall wrote:

> I don't think the implication is supposed to be that padding is 
> zero-initialized or not depending on where in the aggregate it appears, but 
> it doesn't really matter, I don't think we're arguing about the goal of this 
> patch.


The way I read it, C guarantees that an aggregate's padding is zero-initialized 
when you use brace-enclosed list *unless* you've mentioned all the elements / 
members of the aggregate. Fail to mention one → padding must be zero. Mention 
all of them → padding isn't guaranteed.

But then *reading* padding is another story. What triggered this change is C 
code that used `memcmp` on a struct with padding, and was expecting padding to 
be set. Not great code, but AFAICT technically correct.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61304: [OpenCL][PR41609] Deduce static data members to __global addr space

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Presumably a similar rule would apply to thread-locals if you supported them.


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

https://reviews.llvm.org/D61304



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't think the implication is supposed to be that padding is 
zero-initialized or not depending on where in the aggregate it appears, but it 
doesn't really matter, I don't think we're arguing about the goal of this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D61280#1485073 , @rjmccall wrote:

> IIRC, C says *members* are initialized as if they were in static storage, 
> which might mean that their interior padding should be zeroed, but I don't 
> think says anything about the padding in the enclosing aggregate.  But I 
> think zero-initializing padding is probably the right thing to do in general 
> under this flag.


Quoth the Standard:

C17 6.7.9 Initialization ❡21
If there are fewer initializers in a brace-enclosed list than there are 
elements or members of an aggregate, or fewer characters in a string literal 
used to initialize an array of known size than there are elements in the array, 
the remainder of the aggregate shall be initialized implicitly the same as 
objects that have static storage duration.

C17 6.7.9 Initialization ❡10
If an object that has automatic storage duration is not initialized explicitly, 
its value is indeterminate. If an object that has static or thread storage 
duration is not initialized explicitly, then:

- if it has pointer type, it is initialized to a null pointer;
- if it has arithmetic type, it is initialized to (positive or unsigned) zero;
- if it is an aggregate, every member is initialized (recursively) according to 
these rules, and and padding is initialized to zero bits;
- if it is a union, the first named member is initialized (recessively) 
according to these rules, and any padding is initialized to zero bits;




Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D60930: [codeview] Fix symbol names for dynamic initializers and atexit stubs

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In Swift we basically shove *everything* through our corresponding 
linkage-entity structure, so I'm not opposed to the basic idea.

That said, it's unfortunate that this needs to be raised to the AST level.  
Also, you have really been contributing to this project far too long to still 
need to be told to add comments to the new types and functions you're adding. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60930



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

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: dschuff, sbc100, aheejin.
Herald added a subscriber: jgravelle-google.
Herald added a project: clang.

Similar to https://reviews.llvm.org/D61334, update clang tests to use the 
"wasm32-wasi" triple, removing the "-musl" environment and omitting the 
"-unknown" vendor.


Repository:
  rC Clang

https://reviews.llvm.org/D61338

Files:
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9577,10 +9577,10 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-unknown \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY32,WEBASSEMBLY-WASI %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
 //
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -24,17 +24,17 @@
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi-musl/c++/v1" "-internal-isystem" 
"/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi-musl" 
"-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" 
"-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" 
"/foo/include"
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -24,20 +24,20 @@
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: 

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM, I think this makes sense.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

IIRC, C says *members* are initialized as if they were in static storage, which 
might mean that their interior padding should be zeroed, but I don't think says 
anything about the padding in the enclosing aggregate.  But I think 
zero-initializing padding is probably the right thing to do in general under 
this flag.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61280



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

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 197421.
vsapsai added a comment.

- Add a test case with `-disable-free`.

Thanks for the suggestion, Erik.


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

https://reviews.llvm.org/D60848

Files:
  clang/lib/Parse/ParseExpr.cpp
  clang/test/SemaCXX/typo-correction.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- /dev/null
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only 
-Wno-objc-root-class %s -verify -disable-free
+
+@class Dictionary;
+
+@interface Test
+@end
+@implementation Test
+// rdar://problem/47403222
+- (void)rdar47403222:(Dictionary *)opts {
+  [self undeclaredMethod:undeclaredArg];
+  // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  opts[(__bridge id)undeclaredKey] = 0;
+  // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
+}
+@end
Index: clang/test/SemaCXX/typo-correction.cpp
===
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -678,7 +678,7 @@
 struct a0is0 {};
 struct b0is0 {};
 int g() {
-  0 [ // expected-error {{subscripted value is not an array}}
+  0 [
   sizeof(c0is0)]; // expected-error {{use of undeclared identifier}}
 };
 }
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -1582,7 +1582,9 @@
 
   SourceLocation RLoc = Tok.getLocation();
 
-  ExprResult OrigLHS = LHS;
+  LHS = Actions.CorrectDelayedTyposInExpr(LHS);
+  Idx = Actions.CorrectDelayedTyposInExpr(Idx);
+  Length = Actions.CorrectDelayedTyposInExpr(Length);
   if (!LHS.isInvalid() && !Idx.isInvalid() && !Length.isInvalid() &&
   Tok.is(tok::r_square)) {
 if (ColonLoc.isValid()) {
@@ -1594,12 +1596,6 @@
 }
   } else {
 LHS = ExprError();
-  }
-  if (LHS.isInvalid()) {
-(void)Actions.CorrectDelayedTyposInExpr(OrigLHS);
-(void)Actions.CorrectDelayedTyposInExpr(Idx);
-(void)Actions.CorrectDelayedTyposInExpr(Length);
-LHS = ExprError();
 Idx = ExprError();
   }
 


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- /dev/null
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple i386-apple-macosx10.10 -fobjc-arc -fsyntax-only -Wno-objc-root-class %s -verify -disable-free
+
+@class Dictionary;
+
+@interface Test
+@end
+@implementation Test
+// rdar://problem/47403222
+- (void)rdar47403222:(Dictionary *)opts {
+  [self undeclaredMethod:undeclaredArg];
+  // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  opts[(__bridge id)undeclaredKey] = 0;
+  // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
+}
+@end
Index: clang/test/SemaCXX/typo-correction.cpp
===
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -678,7 +678,7 @@
 struct a0is0 {};
 struct b0is0 {};
 int g() {
-  0 [ // expected-error {{subscripted value is not an array}}
+  0 [
   sizeof(c0is0)]; // expected-error {{use of undeclared identifier}}
 };
 }
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -1582,7 +1582,9 @@
 
   SourceLocation RLoc = Tok.getLocation();
 
-  ExprResult OrigLHS = LHS;
+  LHS = Actions.CorrectDelayedTyposInExpr(LHS);
+  Idx = Actions.CorrectDelayedTyposInExpr(Idx);
+  Length = Actions.CorrectDelayedTyposInExpr(Length);
   if (!LHS.isInvalid() && !Idx.isInvalid() && !Length.isInvalid() &&
   Tok.is(tok::r_square)) {
 if (ColonLoc.isValid()) {
@@ -1594,12 +1596,6 @@
 }
   } else {
 LHS = ExprError();
-  }
-  if (LHS.isInvalid()) {
-(void)Actions.CorrectDelayedTyposInExpr(OrigLHS);
-(void)Actions.CorrectDelayedTyposInExpr(Idx);
-(void)Actions.CorrectDelayedTyposInExpr(Length);
-LHS = ExprError();
 Idx = ExprError();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D38061#1484568 , @wenlei wrote:

> In D38061#1484486 , @dblaikie wrote:
>
> > Seems like the right thing would be for the DWARF code that wants a 
> > rendered type name to pass its own printing policy, rather than changing 
> > some relatively global one.
> >
> > (though also I have my doubts about the whole approach - macro expansion 
> > can change the line number as well as the column number, so only 
> > suppressing column number would be insufficient - and this also reduces the 
> > usability for users (because the file/line/column of an anonymous type is a 
> > useful debugging aid). Seems to me like this should be opt-in if it's 
> > supported at all - though ideally build systems would use 
> > -frewrite-includes rather than full preprocessing, and then macros and 
> > lines/columns would be preserved, I think)
>
>
> Line number is ok because preprocessor also inserts #linemarker, and later on 
> these markers are used to recover the original line number before macro 
> expansion. Column is problematic, as there's nothing like a column marker (if 
> possible at all) so there's no way to see through the column change from 
> macro expansion. We tried -frewrite-includes too, but it has its own issues - 
> without macro expansion, it bloats the file size that needs to send to distcc 
> remote machine significantly.


Do you have some data on that growth/size comparison & its overall performance 
impact on the build? I'd be curious.

But, yeah, fair point about the line markers - I'd thought a multiline macro 
would break that, but seems not (it gets stamped out by the preprocessor all on 
one line anyway).

Still - figure this should be opt-in and done at the debug info level, not by 
changing a global printing policy. (& I'd still suggest using 
-frewrite-includes because Clang changes its diagnostic behavior based on the 
presence of macros to reduce diagnostic false positives, so you'll be degrading 
the usability in other ways by compiling fully preprocessed code)


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
Herald added a project: clang.

This revision adds a new kind of rewrite rule, `CompositeRewriteRule`, which
composes multiple subrules into a new rule that allows ordered-choice among its
subrules. With this feature, users can write the rules that appear later in the
list of subrules knowing that previous rules' patterns *have not matched*,
freeing them from reasoning about those cases in the current pattern.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61335

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -116,7 +116,8 @@
 };
   }
 
-  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
+  template 
+  void testRule(R Rule, StringRef Input, StringRef Expected) {
 Transformer T(std::move(Rule), consumer());
 T.registerMatchers();
 compareSnippets(Expected, rewrite(Input));
@@ -375,6 +376,92 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, OrderedRuleUnrelated) {
+  StringRef Flag = "flag";
+  RewriteRule FlagRule = makeRule(
+  cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
+hasName("proto::ProtoCommandLineFlag"
+   .bind(Flag)),
+unless(callee(cxxMethodDecl(hasName("GetProto"),
+  change(Flag, "PROTO"));
+
+  std::string Input = R"cc(
+proto::ProtoCommandLineFlag flag;
+int x = flag.foo();
+int y = flag.GetProto().foo();
+int f(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+proto::ProtoCommandLineFlag flag;
+int x = PROTO.foo();
+int y = flag.GetProto().foo();
+int f(string s) { return REPLACED; }
+  )cc";
+
+  testRule(makeOrderedRule({ruleStrlenSize(), FlagRule}), Input, Expected);
+}
+
+// Version of ruleStrlenSizeAny that inserts a method with a different name than
+// ruleStrlenSize, so we can tell their effect apart.
+RewriteRule ruleStrlenSizeDistinct() {
+  StringRef S;
+  return makeRule(
+  callExpr(callee(functionDecl(hasName("strlen"))),
+   hasArgument(0, cxxMemberCallExpr(
+  on(expr().bind(S)),
+  callee(cxxMethodDecl(hasName("c_str")),
+  change("DISTINCT"));
+}
+
+TEST_F(TransformerTest, OrderedRuleRelated) {
+  std::string Input = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return strlen(s.c_str()); }
+}  // namespace foo
+int g(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return DISTINCT; }
+}  // namespace foo
+int g(string s) { return REPLACED; }
+  )cc";
+
+  testRule(makeOrderedRule({ruleStrlenSize(), ruleStrlenSizeDistinct()}), Input,
+   Expected);
+}
+
+// Change the order of the rules to get a different result.
+TEST_F(TransformerTest, OrderedRuleRelatedSwapped) {
+  std::string Input = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return strlen(s.c_str()); }
+}  // namespace foo
+int g(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return DISTINCT; }
+}  // namespace foo
+int g(string s) { return DISTINCT; }
+  )cc";
+
+  testRule(makeOrderedRule({ruleStrlenSizeDistinct(), ruleStrlenSize()}), Input,
+   Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -28,6 +28,7 @@
 using namespace tooling;
 
 using ast_matchers::MatchFinder;
+using ast_matchers::internal::DynTypedMatcher;
 using ast_type_traits::ASTNodeKind;
 using ast_type_traits::DynTypedNode;
 using llvm::Error;
@@ -171,7 +172,7 @@
   return Transformations;
 }
 
-RewriteRule tooling::makeRule(ast_matchers::internal::DynTypedMatcher M,
+RewriteRule tooling::makeRule(DynTypedMatcher M,
   SmallVector Edits) {
   M.setAllowBind(true);
   // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
@@ -181,6 +182,80 @@
 
 constexpr llvm::StringLiteral RewriteRule::RootId;
 
+// Determines whether A is higher than B in the class hierarchy.
+static bool isHigher(ASTNodeKind A, 

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 197416.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Frontend/macro_defined_type.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaObjC/externally-retained.m
  clang/test/SemaObjC/gc-attributes.m
  clang/test/SemaObjC/mrc-weak.m
  clang/test/SemaObjCXX/gc-attributes.mm
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1614,6 +1614,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroQualifiedTypeLoc(MacroQualifiedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaObjCXX/gc-attributes.mm
===
--- clang/test/SemaObjCXX/gc-attributes.mm
+++ clang/test/SemaObjCXX/gc-attributes.mm
@@ -3,7 +3,7 @@
 @interface A
 @end
 
-void f0(__strong A**); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
+void f0(__strong A **); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
 
 void test_f0() {
   A *a;
@@ -12,7 +12,7 @@
   f0(); // expected-error{{no matching function}}
 }
 
-void f1(__weak A**); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
+void f1(__weak A **); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
 
 void test_f1() {
   A *a;
Index: clang/test/SemaObjC/mrc-weak.m
===
--- clang/test/SemaObjC/mrc-weak.m
+++ clang/test/SemaObjC/mrc-weak.m
@@ -62,6 +62,6 @@
 
 void test_cast_qualifier_inference(__weak id *value) {
   __weak id *a = (id*) value;
-  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing 'id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
+  __unsafe_unretained id *b = (id *)value; // expected-error {{initializing '__unsafe_unretained id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
 }
 
Index: clang/test/SemaObjC/gc-attributes.m
===
--- clang/test/SemaObjC/gc-attributes.m
+++ clang/test/SemaObjC/gc-attributes.m
@@ -9,7 +9,7 @@
   A *a;
   static __weak A *a2;
   f0();
-  f0(); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}} 
+  f0(); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}}
 }
 
 void f1(__weak A**); // expected-note{{passing argument to parameter here}}
@@ -18,7 +18,7 @@
   A *a;
   __strong A *a2;
   f1();
-  f1(); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}} 
+  f1(); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}}
 }
 
 // These qualifiers should silently expand to nothing in GC mode.
Index: clang/test/SemaObjC/externally-retained.m
===
--- clang/test/SemaObjC/externally-retained.m
+++ clang/test/SemaObjC/externally-retained.m
@@ -68,6 +68,12 @@
   second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
 };
 
+void (^blk2)(ObjCTy *, ObjCTy *) =
+^(__strong ObjCTy *first, ObjCTy *second) __attribute__((objc_externally_retained)) {
+  first = 0;
+  second = 0; // expected-error{{variable 

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I think I figured it out. r359604 should fix the bug that caused this to be 
reverted twice.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50515



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Petr Hosek via Phabricator via cfe-commits]

phosek marked an inline comment as not done.
phosek added a subscriber: rsmith.
phosek added inline comments.



Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

jdenny wrote:
> phosek wrote:
> > jdenny wrote:
> > > I naively assumed libunwind would have its own directory so it could be 
> > > selected independently.  Does this mean any library for C++ goes in c++?
> > We could do that, but I'm not sure if it's worth doing without a clear use 
> > case? Do you know of any client that would want to consume libunwind 
> > independently of other C++ libraries? I'm aware of Rust but they build 
> > libunwind independently so there's no need for this.
> I'm afraid I have no idea.  I'm not close enough to these libraries to be an 
> adequate reviewer by myself.  Another reviewer should probably take a look at 
> the new version of the patch.
@EricWF @rsmith is this something you have an opinion on (or can you recommend 
someone who might)?


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

https://reviews.llvm.org/D59168



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:257-262
+  bool AttrStartIsInMacro =
+  (StartLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion(
+   StartLoc, SrcMgr, PP.getLangOpts()));
+  bool AttrEndIsInMacro =
+  (EndLoc.isMacroID() &&
+   Lexer::isAtEndOfMacroExpansion(EndLoc, SrcMgr, PP.getLangOpts()));

rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > I think these checks need to be moved to the last loop of 
> > > `FindLocsWithCommonFileID`. Otherwise for a case like:
> > > 
> > > ```
> > > #define THING \
> > >   int *p = nullptr;
> > >   AS1 int *q = nullptr;
> > > 
> > > THING
> > > ```
> > > 
> > > ... `FindLocsWithCommonFileID` will pick out the `THING` macro and return 
> > > the range from the `int` token to the second semicolon, and the checks 
> > > here will fail. Instead, we should pick out the inner `AS1` expansion, 
> > > because it's the outermost macro expansion that starts with `StartLoc` 
> > > and ends with `EndLoc`.
> > Moved, although this doesn't appear to fix this case. On closer inspection, 
> > when generating the vector of starting locations, the expansion location 
> > for `AS1` doesn't seem to be returned by `getExpansionLocStart`. It goes 
> > straight from the location of the `__attribute__` behind `AS1` to `THING` 
> > and skips `AS1`. I found this out by just dumping `StartLoc` on every 
> > iteration.
> > 
> > The only way I can generate the location for `AS1` in `THING` is by also 
> > watching for the spelling location of each expansion, but this SourceLoc is 
> > not a macro ID and would not fulfill these last 2 checks. Is this intended? 
> > If it's a bug I don't mind addressing it if you point me in the right 
> > direction to start.
> Right, sorry, mistake on my part. To deal with this, you need to call 
> `isAt{Start,End}OfMacroExpansion` while collecting `StartLocs` and `EndLocs`, 
> and stop once you hit a location where they return `false`. (That would mean 
> that you never step from the location of `AS1` out to the location of 
> `THING`.)
Hmm. So I still run into the problem of none of the locations being at the 
start or end of the macro expansion. In your example, I only find 2 source 
locations at all. Let's say I have:

```
1 #define AS1 __attribute__((address_space(1)))
2
3 int main() {
4 #define THING \
5   int *p = 0; \
6   AS1 int *q = p;
7
8   THING;
9 }
```

I only see the following expansion source locations:

```
/usr/local/google/home/leonardchan/misc/test.c:8:3 

/usr/local/google/home/leonardchan/misc/test.c:8:3 

```

And it seems none of them return true for `isAtStartOfMacroExpansion` since the 
`__attribute__` keyword isn't the first token of `THING`. I imagine stopping 
early would work if the 2nd expansion location instead referred to `AS1` (on 
line 6 col 3), but it doesn't seem to be the case here.

I can also see similar results for other examples where the macro is nested but 
does not encapsulate the whole macro:

```
#define THING2 int AS1 *q2 = p2;
int *p2;
THING2;  // Error doesn't print AS1
```

For our case, it seems like the correct thing to do is to get the spelling 
location and default to it if the macro itself doesn't contain the whole 
attribute. I updated and renamed this function to account for this and we can 
now see `AS1` printed. Also added test cases for this.

For cases like `#define AS2 AS1`, `AS1` is still printed, but this is intended 
since `AS1` is more immediate of an expansion than `AS2`.



Comment at: clang/lib/Sema/SemaType.cpp:6967-6972
+  QualType R = T.IgnoreParens().IgnoreMacroDefinitions();
   while (const AttributedType *AT = dyn_cast(R)) {
 if (AT->isCallingConv())
   return true;
-R = AT->getModifiedType().IgnoreParens();
+R = AT->getModifiedType().IgnoreParens().IgnoreMacroDefinitions();
   }

rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > Instead of calling `IgnoreParens()` and `IgnoreMacroDefinitions` here, I 
> > > think we should just be ignoring any type sugar by using 
> > > `getAs()`:
> > > 
> > > ```
> > > bool Sema::hasExplicitCallingConv(QualType T) {
> > >   while (const auto *AT = T->getAs()) {
> > > if (AT->isCallingConv())
> > >   return true;
> > > T = AT->getModifiedType();
> > >   }
> > >   return false;
> > > }
> > > ```
> > > 
> > > (Note also the change from passing `T` by reference -- and then never 
> > > storing to it in the function -- to passing it by value.)
> > When using only `getAs`, the following 3 tests fail:
> > 
> > Clang :: CodeGenCXX/mangle-ms.cpp
> > Clang :: SemaCXX/calling-conv-compat.cpp
> > Clang :: SemaCXX/decl-microsoft-call-conv.cpp
> > 
> > since using `getAs` would involved removing a `TypeDefType` for each of 
> > these. Looking at the comment above this method's declaration, part of the 
> > intent is to retain the 

[PATCH] D51329: [Attribute/Diagnostics] Print macro if definition is an attribute declaration

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 197415.
leonardchan marked 17 inline comments as done.
leonardchan removed a reviewer: martong.
Herald added a reviewer: martong.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Frontend/macro_defined_type.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaObjC/externally-retained.m
  clang/test/SemaObjC/gc-attributes.m
  clang/test/SemaObjC/mrc-weak.m
  clang/test/SemaObjCXX/gc-attributes.mm
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1614,6 +1614,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroQualifiedTypeLoc(MacroQualifiedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaObjCXX/gc-attributes.mm
===
--- clang/test/SemaObjCXX/gc-attributes.mm
+++ clang/test/SemaObjCXX/gc-attributes.mm
@@ -3,7 +3,7 @@
 @interface A
 @end
 
-void f0(__strong A**); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
+void f0(__strong A **); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak ownership, but parameter has __strong ownership}}
 
 void test_f0() {
   A *a;
@@ -12,7 +12,7 @@
   f0(); // expected-error{{no matching function}}
 }
 
-void f1(__weak A**); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
+void f1(__weak A **); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong ownership, but parameter has __weak ownership}}
 
 void test_f1() {
   A *a;
Index: clang/test/SemaObjC/mrc-weak.m
===
--- clang/test/SemaObjC/mrc-weak.m
+++ clang/test/SemaObjC/mrc-weak.m
@@ -62,6 +62,6 @@
 
 void test_cast_qualifier_inference(__weak id *value) {
   __weak id *a = (id*) value;
-  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing 'id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
+  __unsafe_unretained id *b = (id *)value; // expected-error {{initializing '__unsafe_unretained id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
 }
 
Index: clang/test/SemaObjC/gc-attributes.m
===
--- clang/test/SemaObjC/gc-attributes.m
+++ clang/test/SemaObjC/gc-attributes.m
@@ -9,7 +9,7 @@
   A *a;
   static __weak A *a2;
   f0();
-  f0(); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}} 
+  f0(); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}}
 }
 
 void f1(__weak A**); // expected-note{{passing argument to parameter here}}
@@ -18,7 +18,7 @@
   A *a;
   __strong A *a2;
   f1();
-  f1(); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}} 
+  f1(); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}}
 }
 
 // These qualifiers should silently expand to nothing in GC mode.
Index: clang/test/SemaObjC/externally-retained.m
===
--- clang/test/SemaObjC/externally-retained.m
+++ clang/test/SemaObjC/externally-retained.m
@@ -68,6 +68,12 @@
   second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
 };
 
+void (^blk2)(ObjCTy *, ObjCTy *) =
+^(__strong ObjCTy *first, 

[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359603: [Driver] Support compiler-rt crtbegin.o/crtend.o for 
Linux (authored by phosek, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59264?vs=193733=197414#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59264

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-i386.o
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-x86_64.o
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-i386.o
  cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-x86_64.o
  cfe/trunk/test/Driver/linux-ld.c

Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -21,6 +21,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Path.h"
@@ -453,20 +454,29 @@
 
 if (IsIAMCU)
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
-else {
-  const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-}
+else if (HasCRTBeginEndFiles) {
+  std::string P;
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {
+std::string crtbegin = ToolChain.getCompilerRT(Args, "crtbegin",
+   ToolChain::FT_Object);
+if (ToolChain.getVFS().exists(crtbegin))
+  P = crtbegin;
+  }
+  if (P.empty()) {
+const char *crtbegin;
+if (Args.hasArg(options::OPT_static))
+  crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
+else if (Args.hasArg(options::OPT_shared))
+  crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
+else if (IsPIE || IsStaticPIE)
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
+else
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
+P = ToolChain.GetFilePath(crtbegin);
+  }
+  CmdArgs.push_back(Args.MakeArgString(P));
+	  }
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -560,16 +570,27 @@
 }
 
 if (!Args.hasArg(options::OPT_nostartfiles) && !IsIAMCU) {
-  const char *crtend;
-  if (Args.hasArg(options::OPT_shared))
-crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
-  else if (IsPIE || IsStaticPIE)
-crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
-  else
-crtend = isAndroid ? "crtend_android.o" : "crtend.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtend)));
+  if (HasCRTBeginEndFiles) {
+std::string P;
+if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+!isAndroid) {
+  std::string crtend = ToolChain.getCompilerRT(Args, "crtend",
+   ToolChain::FT_Object);
+  if (ToolChain.getVFS().exists(crtend))
+P = crtend;
+}
+if (P.empty()) {
+  const char *crtend;
+  if (Args.hasArg(options::OPT_shared))
+crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
+  else if (IsPIE || IsStaticPIE)
+crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
+  else
+crtend = isAndroid ? "crtend_android.o" : "crtend.o";
+  P = ToolChain.GetFilePath(crtend);
+}
+CmdArgs.push_back(Args.MakeArgString(P));
+  }
   if (!isAndroid)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
 }
Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -2,7 +2,7 @@
 // sysroot to make these tests independent of the host system.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c:8
 int b;
-} FILE;
+} MYFILE;
 

In TestCmodules.py we have `import Darwin` and then `expr *fopen("/dev/zero", 
"w")`. This imports the name `FILE` from the Darwin module. Then when we want 
`expr *myFile`, the two different definition of the `FILE` structs collides.
This happens because now the lookup finds the existing definition for the 
`FILE` in the TU associated with the expression evaluator, and that comes from 
the Darwin module.



Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:640
   }
-
-  DeclContext *decl_context_non_const =

We must remove the below addDeclInternal call, because that may be called from 
another
addDeclInternal:
```
6  clang::CXXConstructorDecl::isDefaultConstructor() const + 87
7  clang::CXXRecordDecl::addedMember(clang::Decl*) + 803
8  clang::DeclContext::addHiddenDecl(clang::Decl*) + 341
9  clang::DeclContext::addDeclInternal(clang::Decl*) + 29
10 lldb_private::ClangASTSource::FindExternalLexicalDecls(clang::DeclContext 
const*, llvm::function_ref, 
llvm::SmallVectorImpl&) + 3917
11 
lldb_private::ClangASTSource::ClangASTSourceProxy::FindExternalLexicalDecls(clang::DeclContext
 const*, llvm::function_ref, 
llvm::SmallVectorImpl&) + 102
12 clang::ExternalASTSource::FindExternalLexicalDecls(clang::DeclContext 
const*, llvm::SmallVectorImpl&) + 101
13 clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const + 269
14 clang::DeclContext::buildLookup() + 336
15 clang::DeclContext::makeDeclVisibleInContextWithFlags(clang::NamedDecl*, 
bool, bool) + 403
16 clang::DeclContext::addDeclInternal(clang::Decl*) + 92
17 clang::ASTNodeImporter::VisitFieldDecl(clang::FieldDecl*) + 3851
```
... and at the second call of addDeclInternal we may add an incomplete Decl 
(CXXConstructorDecl above).
We must always avoid redundant work in lldb which is already handled in 
Clang::ASTImporter.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333



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

r359603 - [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Petr Hosek via cfe-commits]

Author: phosek
Date: Tue Apr 30 12:35:14 2019
New Revision: 359603

URL: http://llvm.org/viewvc/llvm-project?rev=359603=rev
Log:
[Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

When compiler-rt is selected as the runtime library for Linux targets
use its crtbegin.o/crtend.o implemenetation rather than platform one
if available.

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

Added:
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-i386.o

cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-x86_64.o
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-i386.o
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtend-x86_64.o
Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/linux-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=359603=359602=359603=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Apr 30 12:35:14 2019
@@ -21,6 +21,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Path.h"
@@ -453,20 +454,29 @@ void tools::gnutools::Linker::ConstructJ
 
 if (IsIAMCU)
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
-else {
-  const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-}
+else if (HasCRTBeginEndFiles) {
+  std::string P;
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {
+std::string crtbegin = ToolChain.getCompilerRT(Args, "crtbegin",
+   ToolChain::FT_Object);
+if (ToolChain.getVFS().exists(crtbegin))
+  P = crtbegin;
+  }
+  if (P.empty()) {
+const char *crtbegin;
+if (Args.hasArg(options::OPT_static))
+  crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
+else if (Args.hasArg(options::OPT_shared))
+  crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
+else if (IsPIE || IsStaticPIE)
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
+else
+  crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
+P = ToolChain.GetFilePath(crtbegin);
+  }
+  CmdArgs.push_back(Args.MakeArgString(P));
+ }
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -560,16 +570,27 @@ void tools::gnutools::Linker::ConstructJ
 }
 
 if (!Args.hasArg(options::OPT_nostartfiles) && !IsIAMCU) {
-  const char *crtend;
-  if (Args.hasArg(options::OPT_shared))
-crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
-  else if (IsPIE || IsStaticPIE)
-crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
-  else
-crtend = isAndroid ? "crtend_android.o" : "crtend.o";
-
-  if (HasCRTBeginEndFiles)
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtend)));
+  if (HasCRTBeginEndFiles) {
+std::string P;
+if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+!isAndroid) {
+  std::string crtend = ToolChain.getCompilerRT(Args, "crtend",
+   ToolChain::FT_Object);
+  if (ToolChain.getVFS().exists(crtend))
+P = crtend;
+}
+if (P.empty()) {
+  const char *crtend;
+  if (Args.hasArg(options::OPT_shared))
+crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
+  else if (IsPIE || IsStaticPIE)
+crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
+  else
+crtend = isAndroid ? "crtend_android.o" : "crtend.o";
+  P = ToolChain.GetFilePath(crtend);
+}
+CmdArgs.push_back(Args.MakeArgString(P));
+  }
   if (!isAndroid)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
 }

Added: 
cfe/trunk/test/Driver/Inputs/resource_dir/lib/linux/clang_rt.crtbegin-i386.o
URL: 

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: shafik, teemperor, jingham, clayborg, a_sidorin.
Herald added subscribers: lldb-commits, cfe-commits, gamesh411, Szelethus, 
dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added projects: clang, LLDB.

With LLDB we use localUncachedLookup(), however, that fails to find
Decls when a transparent context is involved and the given DC has
external lexical storage.  The solution is to use noload_lookup, which
works well with transparent contexts.  But, we cannot use only the
noload_lookup since the slow case of localUncachedLookup is still needed
in some other cases.

These other cases are handled in ASTImporterLookupTable, but we cannot
use that with LLDB since that traverses through the AST which initiates
the load of external decls again via DC::decls().

We must avoid loading external decls during the import becuase
ExternalASTSource is implemented with ASTImporter, so external loads
during import results in uncontrolled and faulty import.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61333

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/include/lldb/Utility/Logging.h
  lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
  lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
  lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
  lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Symbol/ClangASTImporter.cpp
  lldb/source/Utility/Logging.cpp

Index: lldb/source/Utility/Logging.cpp
===
--- lldb/source/Utility/Logging.cpp
+++ lldb/source/Utility/Logging.cpp
@@ -17,6 +17,7 @@
 
 static constexpr Log::Category g_categories[] = {
   {{"api"}, {"log API calls and return values"}, LIBLLDB_LOG_API},
+  {{"ast"}, {"log AST"}, LIBLLDB_LOG_AST},
   {{"break"}, {"log breakpoints"}, LIBLLDB_LOG_BREAKPOINTS},
   {{"commands"}, {"log command argument parsing"}, LIBLLDB_LOG_COMMANDS},
   {{"comm"}, {"log communication activities"}, LIBLLDB_LOG_COMMUNICATION},
Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -918,6 +918,28 @@
   if (clang::TagDecl *to_tag = dyn_cast(to)) {
 if (clang::TagDecl *from_tag = dyn_cast(from)) {
   to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
+
+  Log *log_ast(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_AST));
+  if (log_ast) {
+std::string name_string;
+if (NamedDecl *from_named_decl = dyn_cast(from)) {
+  llvm::raw_string_ostream name_stream(name_string);
+  from_named_decl->printName(name_stream);
+  name_stream.flush();
+}
+log_ast->Printf(" [ClangASTImporter][TUDecl: %p] Imported "
+"(%sDecl*)%p, named %s (from "
+"(Decl*)%p)",
+static_cast(to->getTranslationUnitDecl()),
+from->getDeclKindName(), static_cast(to),
+name_string.c_str(), static_cast(from));
+
+// Log the AST of the TU.
+std::string ast_string;
+llvm::raw_string_ostream ast_stream(ast_string);
+to->getTranslationUnitDecl()->dump(ast_stream);
+log_ast->Printf("%s", ast_string.c_str());
+  }
 }
   }
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -637,18 +637,6 @@
 
 m_ast_importer_sp->RequireCompleteType(copied_field_type);
   }
-
-  DeclContext *decl_context_non_const =
-  const_cast(decl_context);
-
-  if (copied_decl->getDeclContext() != decl_context) {
-if (copied_decl->getDeclContext()->containsDecl(copied_decl))
-  copied_decl->getDeclContext()->removeDecl(copied_decl);
-copied_decl->setDeclContext(decl_context_non_const);
-  }
-
-  if (!decl_context_non_const->containsDecl(copied_decl))
-decl_context_non_const->addDeclInternal(copied_decl);
 }
   }
 
Index: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
===
--- lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
+++ lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
@@ -5,11 +5,11 @@
 typedef struct {
 int a;
 int b;
-} FILE;
+} MYFILE;
 
 int main()
 {
-FILE *myFile = malloc(sizeof(FILE));
+MYFILE *myFile = malloc(sizeof(MYFILE));
 
 myFile->a = 5;
 myFile->b = 9;
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Your understanding is correct. As far as testing I think the existing testing 
in the original patches is sufficient.

I'm not sure I understand how a target specific intrinsic that only works on 
x86 in the bitcode is substantially better than inline assembly.   Do you plan 
to also change the cpuid intrinsics in cpuid.h that are also implemented in 
inline assembly?


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

https://reviews.llvm.org/D44387



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

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the insightful explanation, Craig. As far as I understand, 
implementing intrinsics with builtins is possible but it is more complex and 
wasn't providing enough value to prefer it over inline assembly. If that is 
correct, I'd like to revive the abandoned implementation. Does it sound 
reasonable? And do I need to do something special for testing? Because I don't 
have access to corresponding hardware and unable to test the intrinsics on the 
actual hardware.


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

https://reviews.llvm.org/D44387



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

r359598 - Add requires amdgpu-registered-target for amdgpu-float16.cpp

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 30 12:06:15 2019
New Revision: 359598

URL: http://llvm.org/viewvc/llvm-project?rev=359598=rev
Log:
Add requires amdgpu-registered-target for amdgpu-float16.cpp

Modified:
cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp

Modified: cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp?rev=359598=359597=359598=diff
==
--- cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp Tue Apr 30 12:06:15 2019
@@ -1,3 +1,4 @@
+// REQUIRES: amdgpu-registered-target
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx701 -S -o - %s | 
FileCheck %s -check-prefix=NOF16
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx803 -S -o - %s | 
FileCheck %s
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx900 -S -o - %s | 
FileCheck %s


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

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61220



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

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

vitalybuka told me that this should repro things locally:

  mkdir /tmp/bot
  cd /tmp/bot
  svn checkout https://llvm.org/svn/llvm-project/zorg
  BUILDBOT_CLOBBER= BUILDBOT_REVISION=YOUR_REV 
zorg/trunk/zorg/buildbot/builders/sanitizers/buildbot_fast.sh 2>&1 | tee 
buildlog.txt

I'm now running that and waiting for it.

In the meantime, 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/31821/steps/check-llvm%20asan/logs/stdio
 has a possibly better error:

  [ RUN  ] Option.FindNearest
  =
  ==64567==ERROR: AddressSanitizer: stack-use-after-scope on address 
0x7f488a118db1 at pc 0x00577ebb bp 0x7ffcd3a9ba90 sp 0x7ffcd3a9ba88
  READ of size 1 at 0x7f488a118db1 thread T0
  #0 0x577eba in unsigned int 
llvm::ComputeEditDistance(llvm::ArrayRef, llvm::ArrayRef, 
bool, unsigned int) 
/b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/edit_distance.h:81:25
  #1 0x56a016 in llvm::opt::OptTable::findNearest(llvm::StringRef, 
std::__1::basic_string, 
std::__1::allocator >&, unsigned int, unsigned int, unsigned int) const 
/b/sanitizer-x86_64-linux-fast/build/llvm/lib/Option/OptTable.cpp:301:21
  #2 0x54d0fa in Option_FindNearest_Test::TestBody() 
/b/sanitizer-x86_64-linux-fast/build/llvm/unittests/Option/OptionParsingTest.cpp:279:3
  #3 0x5f81a1 in HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc
  #4 0x5f81a1 in testing::Test::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474
  #5 0x5fa758 in testing::TestInfo::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
  #6 0x5fbd14 in testing::TestCase::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
  #7 0x619834 in testing::internal::UnitTestImpl::RunAllTests() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
  #8 0x6189af in 
HandleExceptionsInMethodIfSupported 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc
  #9 0x6189af in testing::UnitTest::Run() 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257
  #10 0x5ded86 in RUN_ALL_TESTS 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
  #11 0x5ded86 in main 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
  #12 0x7f488d38a2e0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
  #13 0x42a3a9 in _start 
(/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/unittests/Option/OptionTests+0x42a3a9)
  
  Address 0x7f488a118db1 is located in stack of thread T0 at offset 433 in frame
  #0 0x56935f in llvm::opt::OptTable::findNearest(llvm::StringRef, 
std::__1::basic_string, 
std::__1::allocator >&, unsigned int, unsigned int, unsigned int) const 
/b/sanitizer-x86_64-linux-fast/build/llvm/lib/Option/OptTable.cpp:251


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50515



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D59168#1484754 , @phosek wrote:

> It's the `LIBCXX_ABI_VERSION` CMake option, see 
> https://github.com/llvm/llvm-project/blob/master/libcxx/CMakeLists.txt#L121


Thanks.




Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

phosek wrote:
> jdenny wrote:
> > I naively assumed libunwind would have its own directory so it could be 
> > selected independently.  Does this mean any library for C++ goes in c++?
> We could do that, but I'm not sure if it's worth doing without a clear use 
> case? Do you know of any client that would want to consume libunwind 
> independently of other C++ libraries? I'm aware of Rust but they build 
> libunwind independently so there's no need for this.
I'm afraid I have no idea.  I'm not close enough to these libraries to be an 
adequate reviewer by myself.  Another reviewer should probably take a look at 
the new version of the patch.


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

https://reviews.llvm.org/D59168



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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D60907#1484756 , @jdoerfert wrote:

> I actually don't want to preinclude anything and my arguments are (mostly) 
> for the OpenMP offloading code path not necessarily Cuda.
>  Maybe to clarify, what I want is:
>
> 1. Make sure the `clang/Headers/math.h` is found first if `math.h` is 
> included.
> 2. Use a scheme similar to the one described 
> https://reviews.llvm.org/D47849#1483653 in `clang/Headers/math.h`
> 3. Only add `math.h` function overloads in our `math.h`. **<- This is 
> debatable**


Agreed.

> 4. Include `clang/Headers/math.h` from `__clang_cuda_device_functions.h` to 
> avoid duplication of math function declarations.

This is not needed for CUDA. `math.h` is included early on in 
`__clang_cuda_runtime_wrapper.h` (via ``), so by the time 
`__clang_cuda_device_functions.h` is included, math.h has already been included 
one way or another -- either in step 3 above, or directly by the 
__clang_cuda_runtime_wrapper.h


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Seems like the right thing would be for the DWARF code that wants a rendered 
type name to pass its own printing policy, rather than changing some relatively 
global one.

(though also I have my doubts about the whole approach - macro expansion can 
change the line number as well as the column number, so only suppressing column 
number would be insufficient - and this also reduces the usability for users 
(because the file/line/column of an anonymous type is a useful debugging aid). 
Seems to me like this should be opt-in if it's supported at all - though 
ideally build systems would use -frewrite-includes rather than full 
preprocessing, and then macros and lines/columns would be preserved, I think)


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.

In D38061#1484486 , @dblaikie wrote:

> Seems like the right thing would be for the DWARF code that wants a rendered 
> type name to pass its own printing policy, rather than changing some 
> relatively global one.
>
> (though also I have my doubts about the whole approach - macro expansion can 
> change the line number as well as the column number, so only suppressing 
> column number would be insufficient - and this also reduces the usability for 
> users (because the file/line/column of an anonymous type is a useful 
> debugging aid). Seems to me like this should be opt-in if it's supported at 
> all - though ideally build systems would use -frewrite-includes rather than 
> full preprocessing, and then macros and lines/columns would be preserved, I 
> think)


Line number is ok because preprocessor also inserts #linemarker, and later on 
these markers are used to recover the original line number before macro 
expansion. Column is problematic, as there's nothing like a column marker (if 
possible at all) so there's no way to see through the column change from macro 
expansion. We tried -frewrite-includes too, but it has its own issues - without 
macro expansion, it bloats the file size that needs to send to distcc remote 
machine significantly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D38061



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

r359594 - AMDGPU: Enable _Float16

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 30 11:35:37 2019
New Revision: 359594

URL: http://llvm.org/viewvc/llvm-project?rev=359594=rev
Log:
AMDGPU: Enable _Float16

Added:
cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp
Modified:
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=359594=359593=359594=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Tue Apr 30 11:35:37 2019
@@ -252,6 +252,9 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const
  !isAMDGCN(Triple));
   UseAddrSpaceMapMangling = true;
 
+  HasLegalHalfType = true;
+  HasFloat16 = true;
+
   // Set pointer width and alignment for target address space 0.
   PointerWidth = PointerAlign = DataLayout->getPointerSizeInBits();
   if (getMaxPointerWidth() == 64) {

Added: cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp?rev=359594=auto
==
--- cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/amdgpu-float16.cpp Tue Apr 30 11:35:37 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx701 -S -o - %s | 
FileCheck %s -check-prefix=NOF16
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx803 -S -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx900 -S -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 -S -o - %s | 
FileCheck %s
+void f() {
+  _Float16 x, y, z;
+  // CHECK: v_add_f16_e64
+  // NOF16: v_add_f32_e64
+  z = x + y;
+  // CHECK: v_sub_f16_e64
+  // NOF16: v_sub_f32_e64
+  z = x - y;
+  // CHECK: v_mul_f16_e64
+  // NOF16: v_mul_f32_e64
+  z = x * y;
+  // CHECK: v_div_fixup_f16
+  // NOF16: v_div_fixup_f32
+  z = x / y;
+}


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

[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D60907#1484529 , @hfinkel wrote:

> In D60907#1483660 , @jdoerfert wrote:
>
> > In D60907#1483615 , @hfinkel wrote:
> >
> > > In D60907#1479370 , @gtbercea 
> > > wrote:
> > >
> > > > In D60907#1479142 , @hfinkel 
> > > > wrote:
> > > >
> > > > > In D60907#1479118 , 
> > > > > @gtbercea wrote:
> > > > >
> > > > > > Ping @hfinkel @tra
> > > > >
> > > > >
> > > > > The last two comments in D47849  
> > > > > indicated exploration of a different approach, and one which still 
> > > > > seems superior to this one. Can you please comment on why you're now 
> > > > > pursuing this approach instead?
> > > >
> > > >
> > > > ...
> > > >
> > > > Hal, as far as I can tell, this solution is similar to yours but with a 
> > > > slightly different implementation. If there are particular aspects 
> > > > about this patch you would like to discuss/give feedback on please let 
> > > > me know.
> > >
> > >
> > > The solution I suggested had the advantages of:
> > >
> > > 1. Being able to directly reuse the code in 
> > > `__clang_cuda_device_functions.h`. On the other hand, using this solution 
> > > we need to implement a wrapper function for every math function. When 
> > > `__clang_cuda_device_functions.h` is updated, we need to update the 
> > > OpenMP wrapper as well.
> >
> >
> > I'd even go as far as to argue that `__clang_cuda_device_functions.h` 
> > should include the internal math.h wrapper to get all math functions. See 
> > also the next comment.
> >
> > > 2. Providing access to wrappers for other CUDA intrinsics in a natural 
> > > way (e.g., rnorm3d) [it looks a bit nicer to provide a host version of 
> > > rnorm3d than __nv_rnorm3d in user code].
> >
> > @hfinkel 
> >  I don't see why you want to mix CUDA intrinsics with math.h overloads.
>
>
> What I had in mind was matching non-standard functions in a standard way. For 
> example, let's just say that I have a CUDA kernel that uses the rnorm3d 
> function, or I otherwise have a function that I'd like to write in OpenMP 
> that will make good use of this CUDA function (because it happens to have an 
> efficient device implementation). This is a function that CUDA provides, in 
> the global namespace, although it's not standard.
>
> Then I can do something like this (depending on how we setup the 
> implementation):
>
>   double rnorm3d(double a,  double b, double c) {
> return sqrt(a*a + b*b + c*c);
>   }
>   
>   ...
>   
>   #pragma omp target
>   {
> double a = ..., b = ..., c = ...;
> double r = rnorm3d(a, b, c)
>   }
>   
>
> and, if we use the CUDA math headers for CUDA math-function support, than 
> this might "just work." To be clear, I can see an argument for having this 
> work being a bad idea ;) -- but it has the advantage of providing a way to 
> take advantage of system-specific functions while still writing 
> completely-portable code.


Matching `rnorm3d` and replacing it with some nvvm "intrinsic" is something I 
wouldn't like to see happening if `math.h` was included and not if it was not. 
As you say, in Cuda that is not how it works either. I'm in favor of reusing 
the built-in recognition mechanism:
That is, if the target is nvptx, the name is rnorm3d, we match that name and 
use the appropriate intrinsic, as we do others already for other targets.

>>   I added a rough outline of how I imagined the internal math.h header to 
>> look like as a comment in D47849. Could you elaborate how that differs from 
>> what you imagine and how the other intrinsics come in?
> 
> That looks like what I had in mind (including 
> `__clang_cuda_device_functions.h` to get the device functions.)
> 
>> 
>> 
>>> 3. Being similar to the "declare variant" functionality from OpenMP 5, and 
>>> thus, I suspect, closer to the solution we'll eventually be able to apply 
>>> in a standard way to all targets.
>> 
>> I can see this.
>> 
 This solution is following Alexey's suggestions. This solution allows the 
 optimization of math calls if they apply (example: pow(x,2) => x*x ) which 
 was one of the issues in the previous solution I implemented.
>>> 
>>> So we're also missing that optimization for CUDA code when compiling with 
>>> Clang? Isn't this also something that, regardless, should be fixed?
>> 
>> Maybe through a general built-in recognition and lowering into target 
>> specific implementations/intrinsics late again?
> 
> I suspect that we need to match the intrinsics and perform the optimizations 
> in LLVM at that level in order to get the optimizations for CUDA.

That seems reasonable to me. We could also match other intrinsics, e.g., 
`rnorm3d`, here as well, both by name but also by the computation pattern.

In 

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D59168#1484467 , @jdenny wrote:

> In D59168#1480801 , @phosek wrote:
>
> > In D59168#1480428 , @jdenny wrote:
> >
> > > It seems we have roughly the same situation for their ABIs.  That is, for 
> > > .so files, someone noted that libc++ and libunwind have always used the 
> > > same version, .1, and openmp doesn't use a version (except in Debian 
> > > packages).  In other words, for each of these three, a change in ABI 
> > > compatibility has never been indicated.  For openmp only, it's apparently 
> > > assumed a change never will need to be indicated (except in Debian 
> > > packages).
> >
> >
> > They already use `.2` for ABI v2 which is currently considered unstable but 
> > some project like Fuchsia already use. There's also an ongoing discussion 
> > to stabilize v2 
> > .
>
>
> Thanks, I wasn't aware.  I still see only .1 in my build, so apparently a 
> different config is needed.


It's the `LIBCXX_ABI_VERSION` CMake option, see 
https://github.com/llvm/llvm-project/blob/master/libcxx/CMakeLists.txt#L121


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

https://reviews.llvm.org/D59168



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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

[Petr Hosek via Phabricator via cfe-commits]

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



Comment at: libunwind/CMakeLists.txt:190
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

jdenny wrote:
> I naively assumed libunwind would have its own directory so it could be 
> selected independently.  Does this mean any library for C++ goes in c++?
We could do that, but I'm not sure if it's worth doing without a clear use 
case? Do you know of any client that would want to consume libunwind 
independently of other C++ libraries? I'm aware of Rust but they build 
libunwind independently so there's no need for this.


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

https://reviews.llvm.org/D59168



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

[PATCH] D50515: Re-push "[Option] Fix PR37006 prefix choice in findNearest"

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.
Herald added a project: LLVM.

I re-landed this to get access to the msan output, since the link above has 
long expired. Here's the new link: 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/31821/steps/check-clang%20msan/logs/stdio

Sadly, the bot output isn't super useful:

  Command Output (stderr):
  --
  
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/CodeGen/thinlto_backend.ll:10:18:
 error: CHECK-WARNING: expected string not found in input
  ; CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only 
allowed with '-x ir'
   ^
  :1:1: note: scanning from here
  ==7832==WARNING: MemorySanitizer: use-of-uninitialized-value
  ^
  
  --

I'll see if I can figure out how to do an msan build of llvm.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50515



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

> Does llvm-elfabi consume your proposed Schema format? Has it landed yet?

No, I just proposed it and explained my reasoning. If you wanted to add this 
format to TextAPI that would be acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D61015#1484669 , @thakis wrote:

> This breaks a test:  
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12112/steps/check-llvm%20check-clang%20stage3%2Fmsan/logs/stdio
>
>   [--] 1 test from TransformerTest
>   [ RUN  ] TransformerTest.NodePartNameDeclRefFailure
>   
> /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/unittests/Tooling/TransformerTest.cpp:66:
>  Failure
>   Value of: MaybeActual
> Actual: false
>   Expected: true
>   Rewrite failed. Expecting: 
>   struct Y {
> int operator*();
>   };
>   int neutral(int x) {
> Y y;
> int (Y::*ptr)() = ::operator*;
> return *y + x;
>   }
>
>   [  FAILED  ] TransformerTest.NodePartNameDeclRefFailure (83 ms)
>   [--] 1 test from TransformerTest (83 ms total)
>
>
> Can you take a look?


Fixed in r359578.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61015



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2033
+
+  void setExplicitSpecifier(ExplicitSpecInfo ESI);
+

Tyker wrote:
> Tyker wrote:
> > rsmith wrote:
> > > Generally we don't want to have setters in the AST; the AST is intended 
> > > to be immutable after creation. Is this necessary?
> > this is used in 2 cases:
> > - for deserialization.
> > - for in `Sema::ActOnFunctionDeclarator` to make every declaration of the 
> > same function have the same explicit specifier as the canonical one. there 
> > is probably a better way to do this but i didn't find how.
> > 
> > the second use case will need to be changed because the constructor's 
> > explicit specifier will be tail-allocated so the explicit specifier from 
> > the canonical declaration will need to be recovered before construction to 
> > allocate storage. 
> > 
> > how can we find the canonical declaration of a function before it is 
> > constructed ?
> i found a solution around that issue.
> now setExplicitSpecifier is only used for deserialization.
as it is only used by deserialization and ASTDeclReader is friend to all class 
with a setExplicitSpecifier.
I made setExplicitSpecifier private.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6174
 CXXConversionDecl)) {
-  return Node.isExplicit();
+  return Node.getExplicitSpecifier().isSpecified();
 }

rsmith wrote:
> This will match `explicit(false)` constructors. I think 
> `getExplicitSpecifier().isExplicit()` would be better, but please also add a 
> FIXME here: it's not clear whether this should match a dependent 
> `explicit()`.
shouldn't this be able to match with deduction guides too ?



Comment at: clang/lib/Sema/SemaInit.cpp:9361
 // Only consider converting constructors.
-if (GD->isExplicit())
+if (!GD->isMaybeNotExplicit())
   continue;

rsmith wrote:
> Tyker wrote:
> > rsmith wrote:
> > > We need to substitute into the deduction guide first to detect whether it 
> > > forms a "converting constructor", and that will need to be done inside 
> > > `AddTemplateOverloadCandidate`.
> > similarly as the previous if. this check removes deduction guide that are 
> > already resolve to be explicit when we are in a context that doesn't allow 
> > explicit.
> > every time the explicitness was checked before my change i replaced it by a 
> > check that removes already resolved explicit specifiers.
> Unlike in the previous case, we do //not// yet pass an `AllowExplicit` flag 
> into `AddTemplateOverloadCandidate` here, so this will incorrectly allow 
> dependent //explicit-specifier//s that evaluate to `true` in 
> copy-initialization contexts.
the default value for `AllowExplicit` is false. so the conversion will be 
removed in `AddTemplateOverloadCandidate`.


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

https://reviews.llvm.org/D60934



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

[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks a test:  
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12112/steps/check-llvm%20check-clang%20stage3%2Fmsan/logs/stdio

  [--] 1 test from TransformerTest
  [ RUN  ] TransformerTest.NodePartNameDeclRefFailure
  
/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/unittests/Tooling/TransformerTest.cpp:66:
 Failure
  Value of: MaybeActual
Actual: false
  Expected: true
  Rewrite failed. Expecting: 
  struct Y {
int operator*();
  };
  int neutral(int x) {
Y y;
int (Y::*ptr)() = ::operator*;
return *y + x;
  }

  [  FAILED  ] TransformerTest.NodePartNameDeclRefFailure (83 ms)
  [--] 1 test from TransformerTest (83 ms total)

Can you take a look?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61015



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